Hi,
On Thu, Oct 10, 2013 at 11:46 AM, Guillaume "Louis-Marie" Delhumeau <
gdelhumeau(a)xwiki.com> wrote:
Hi Eduard,
2013/10/9 Eduard Moraru <enygma2002(a)gmail.com>
Hi,
On Tue, Oct 8, 2013 at 4:41 PM, Guillaume "Louis-Marie" Delhumeau <
gdelhumeau(a)xwiki.com> wrote:
> Thanks for all your answers.
>
> I continue to iterate until we reach something nice. You can see the
work
https://github.com/gdelhumeau/xwiki-platform/tree/feature-wiki-properties-g…
Side note: I`ve noticed that you have 2 branches where you work, both
feature-wiki-properties-group that you`ve linked to now and new-wiki-api
that you`ve linked to in the initial post. Please use only one since it`s
already quite confusing and hard to follow.
>
> I think my mistakes until now was the fact that I didn't want to
deviate
too much
from the wiki-descriptor module of Vincent. But now, I
understand
> it is better to make something new.
>
> Now, the API has a complete javadoc.
>
> = Modularity =
> == Reduce responsibility ==
>
> The first advantage of creating several modules is to reduce the
> responsibility of each module. I want one API to handle the wiki
creation
and basic
management, one API for the template feature (which could be
optional as soon as we have flavors),
This transition will most certainly require some considerable refactoring
and possibly some method deprecating, for which we already have a
strategy,
at least for the workspaces UI.
Currently, the workspaces UI is a kind of a hack. WorkspaceManager.Install
create a new template where we install a standard XAR UI + Workspace
Template Feature (that overwrites some standard pages) + delete some pages
that we don't need, etc... It forces us to ask the user to create
workspacetemplate first, it does not work out of the box.
Neither does wiki manager work out of the box. The current technical
limitations require that the user first creates a template wiki that is
used in the creation of new wikis/workspaces.
And we need to
have 2 distinct XARs to handle the 2 use-cases: farm and workspaces. It is
definitively not clean.
When talking with Vincent about this, we decided that it would be better to
create a unique XAR that handle the 2 use-cases.
But again, this does not fix the "out-of-the-box" issue.
It will just look at the
wiki configuration to know if we have local users or not, what is the
membership type of the wiki, etc...
The idea was that an admin could come with his own customized XAR that
would then be "adapted" to be used as a workspace template, if the admin
did not already do that. It was not desired that the user be limited to the
standard XE xar, since that would have been rather easy to implement and
automatically "install".
So, I apologize to not having talk here about that. I
think that we should,
indeed, refactor a lot of things to have a very clean multiwiki feature in
5.3.
Basically, you`d right now be using a
$services.wiki.createWiki('newWiki')
to create the wiki followed by a
$services.wikiTemplate.apply('someWikiTemplateName', 'newWiki') to
initialize the wiki with content.
When we`d have flavours, you`d probably follow the creation step with a
$services.extension.install('someFlavourExtensionID', 'newWiki') to
initialize the wiki with the content of a flavour extension.
We`d probably also want to keep the option of importing the content from
a
XAR, so that might be a option too.
So this wikiTemplate service would probably contain some methods like:
- create/convert (since the argument would probably be a wikiID to mark
as
template. The content of that wiki would have
previously be initialized
from an extension of from a XAR)
I have proposed setTemplate(boolean template);
Maybe setTemplate(String wikiID) as in,
$services.wikiTemplate.setTemplate('someWiki')
...or even setTemplate(String wikiID, boolean isTemplate) to avoid having a
second removeTemplate(String wikiID) method.
- list/getAll (wikis marked as templates)
- delete/revert (remove the template mark, resulting in a regular wiki
once
again)
- isTemplate (wikiID)
- apply (as seen above, copy the content to an empty wiki)
I was thinking about createWikiFromTemplate() instead, but maybe it does
not provide enough flexibility.
Didn`t you mention something earlier about separation of concerns? :)
Side Note1: It could get a bit ugly fast when having to list wikis. What
do
you do with templates? What do users see, what do
admins see if you plan
to
have a single view(UI) for everything? Probably 2
UIs would fit best
(similar to what we have not with WikiManagerUI and WorkspacesUI). The
WikiManager part would probably go in the main wiki's administration (for
admins) and the WorkspacesUI part would be in the main wiki's WikiIndex
(for users).
That is an interesting question. I did not think that much about the UI
until now. I wanted to have a clean API first.
Side Note2: While thinking about templates, I thought that we might need
some "copy" and "rename" methods for the WikiManager API, since they
might
operations that the user might want to perform
without having to use
templates or some other weird operations.
- Rename would be hard to implement (copy the wiki and delete it?)
No, just rename the database and the descriptor. Is there any limitation to
that?
- What "copy a wiki" means? Should we copy
the history of every pages too?
Should we copy the recycle bin? I did not want to provide a copy feature
because of these questions. The template feature actually does a copy
operation but with its own opinion concerning this aspects : no history
copied, no recycle bin.
1-to-1 copy of the source wiki. This means everything.
Sure, there might be some applications that use custom mapping and add some
tables to the source database that might not be copied in the process. But,
if we have no way of detecting and copying this data, I think it's not a
bit problem. History and recycle bin are core notions of which we have full
control so they should not be an issue.
Anyway, since we`re reviewing these processes and thinking about them now,
maybe we should do it right. Copy and Rename are basic operations that
should not miss from any data model. We should *at least* include them in
the API and implement them when time allows (and expose them in the UI when
implemented). It would be a shame to have yet another half baked API.
and one API for the ability to
manager users (both the farm and the workspaces
use-cases).
Do you mean operations like:
- join
- leave
- invite
- acceptInvitation
- cancelInvitation
- getMembers
- isMember
- etc, all the stuff that it's now written as velocity code in the
workspaces-ui pages?
If so, than this could be indeed a useful script service to lighten up
the
velocity pages that are full of duplicate code.
I think it's the only way to have a clean code, and will probably make it
easier to have only 1 XAR for the 2 use-cases.
However, for the farm use case I don`t see any
applications of this
service, since the farm use case would be using the same tools that are
currently available to the main wiki.
I think it's a
good thing to avoid creating a new oldcore. I
know this module is too
simple yet to be comparable with oldcore, but I try to design things in
this way.
== Reduce dependencies ==
The second advantage, is that I try to reduce the dependencies for each
module. That means, if a module only needs the list of wikis (and a lot
of
modules needs it), they won't have plenty of
dependencies.
For example, the extension manager needs the list of all wikis, but
doesn't
> care about theirs users, the templates, etc... So it does not need to
> depend on the users module, etc...
>
> == About WikiPropertiesGroup ==
>
> The idea behind this new class is to easily add custom properties in
the
> descriptor for new modules. Each module
provides its own
> WikiPropertiesGroupProvider that load and save custom properties for
the
wiki.
Then, it would be easy to get a property:
wikiDescriptor.get("template").get("isTemplate") or (in velocity)
$wikiDescriptor.template.isTemplate. But we can also create typed
wrappers
that access to theses properties.
Care to provide other (practical) examples of where such properties might
be useful?
Imagine a new Activity Stream that needs to store information about each
wiki, like "the main AS should index the events of this wiki" or "users
of
that wiki can send a message to main users", etc... Where AS should store
this information? WikiPropertyGroup is good for that.
The membership type and the "enable local users" could be stored there too.
Ludo told me that clients would like to disable extension manager in
(sub)wiki, or to restrict it to main admins only. If we decide to implement
this, we could store the EM configuration in the property groups...
I think it is easy to implement and could be very useful for a lot of
use-cases that we don't even imagine yet. In my opinion, having this
flexibility can not be bad.
Technical note: Looking at the code, I can`t see these WikiPropertyGroups
being handled anywhere. I imagine that you would have to delegate the task
of creating WikiDescriptor instances to the WikiManager which will, in
turn, be in charge of querying all the WikiPropertyGroupProviders and
populating the new WikiDescriptor with these properties before returning it
to the caller.
You speak of WikiPropertyGroup as a storage location. However, in the code,
I see that each provider is supposed to save the properties itself, so it
is in charge of picking a physical location for these properties to be
stored. The WikiDescriptor would only be a logical location where
applications might store and read information/properties **about** the
wiki. When an application would store a new property for a wiki in a
certain property group, that group's provider will be in charge of
physically storing the value in the location where that group's properties
are physically stored.
It would be an interesting idea, but I find that it would be much more
productive as a generic service of its own and not just limited to wikis.
It is easy to imagine the need for such a service in the case of users.
Applications might want to store/query properties for the current user,
maybe for the current space and so on. For users, right now we`re storing
stuff in the user profile. For wikis, we`d probably store it in
XWikiPreferences, SpacePreferences for spaces and so on.
Maybe something a bit like what we do with ConfiguratinSource, but targeted
on certain entities (wikis, users, etc)
https://github.com/xwiki/xwiki-commons/blob/master/xwiki-commons-core/xwiki…
...however, what I don`t like about ConfigurationSource is that it is
ReadOnly.
Would be a shame to spend the effort and not to make it a generic solution.
WDYT?
Actually, in the WikiManager, we have a cache that avoid to look at the
database every time a module needs a descriptor. By using theses
properties
> groups, we can extend what a descriptor is and make good use of the
cache
without
re-implementing it in every modules.
= Misc =
* I have replace getByWikiId() by getById() and so on...
* getAll() will now return the main wiki as well.
* Aliases are now normal string, and the default alias is stored in the
same list than the other alias.
* WikiDescriptor is now the name used, not "Wiki".
* I have added an hidden field in the descriptor.
* I have added an ownerId field in the descriptor.
* I have added a main page field in the descriptor.
* We may need a WikiManager#getMainWiki(), WDYT?
We have $xcontext.mainWikiName that we use everywhere. Maybe it won`t
hurt
to have a "cleaner" way of getting the
main wiki's descriptor/name
without
relying on the deprecated XWikiContext.
I agree.
Thanks,
Eduard
P.S.: Excuse my rant above on technical details/API. In the absence of
better details, I had to imagine/picture it for myself to better
understand
what you meant :)
I am new to this way of working. I am used to work alone, and for very
specific use cases. I try to improve my communication.
It is not very easy, because some peoples suggest me something while others
advise me the opposite!
One solution would be to use a common communication channel, like the
mailing list, instead of 1-to-1 talks. This way suggestions can be
discussed and evaluated, mine included :)
And, as usual, sometimes you just have to bite the bullet and make the
decision for yourself instead of waiting forever for answers/solutions from
the list/someone. Things can always be improved in future iterations, but
having nothing to show is never a good thing :)
-Eduard
Thanks for your concern,
Louis-Marie
* There is no implementation for all of this, right now, I'm waiting for
> the API to be defined.
>
> Thanks,
> Louis-Marie
>
>
>
>
> 2013/10/8 Thomas Mortagne <thomas.mortagne(a)xwiki.com>
>
> > On Tue, Oct 8, 2013 at 8:53 AM, Marius Dumitru Florea
> > <mariusdumitru.florea(a)xwiki.com> wrote:
> > > Hi Guillaume,
> > >
> > > I also don't see the point of a typed WikiAlias for now and in
> > > Wiki#getWikiId() the "Wiki" is redundant. Why not simply
Wiki#getId()
> > > and Wiki#getAlias()?
> >
> > > And what is the difference between 'wikiAlias'
> > > and 'descriptorAliases' (no javadoc)? If a wiki can have multiple
> > > aliases then why not keep just a list and handle the first item
> > > differently if needed (like ServletRequest#getParameter() vs.
> > > ServletRequest#getParameterValues()).
> >
> > Same for me, this does not make much sense. The wiki has a list of
> > aliases and the URL factory happen to use the first one. Either you
> > keep only the list of you rename wikiAlias into default alias to be
> > extra safe on what should be used when generating a URL but in all
> > cases the list should contain all aliases, the default/first one
> > included.
> >
> > >
> > > In WikiManager#getAll() javadoc I see "(except the main one)".Is
this
> the only method that must treat (by contract) the
main wiki
> differently? How do getById() and exists() behave for instance
> regarding the main wiki?
> The javadoc on an interface / API is very
> important as it indicates what the implementation should do. So it
> must state very clear any special use cases.
+1 the most important is the Javadoc, you would probably get less
questions ;)
>
> Hope this helps,
> Marius
>
> On Mon, Oct 7, 2013 at 6:17 PM, Guillaume "Louis-Marie" Delhumeau
> <gdelhumeau(a)xwiki.com> wrote:
>> Thanks to your advices (Eddy and Thomas), I propose you a clean
version:
>>
https://github.com/gdelhumeau/xwiki-platform/tree/new-wiki-api/xwiki-platfo…
>
>>
> >> I have only changed the API, not the implementation.
> >>
> >> I think it starts to be clean.
> >>
> >>
> >> 2013/10/7 Guillaume "Louis-Marie" Delhumeau
<gdelhumeau(a)xwiki.com
> > >>
> > >>> Hi Eddy.
> > >>>
> > >>> First, thanks for your answer.
> > >>>
> > >>> 2013/10/7 Eduard Moraru <enygma2002(a)gmail.com>
> > >>>
> > >>>> Hi Guilllaume,
> > >>>>
> > >>>> 1) What is the point of the WikiDescriptorAlias class if it
just
> > holds and
> > >>>> returns a string?
> > >>>>
> > >>>
> > >>>> Also, the only method in the WikiDescriptorManager that works
with
> > aliases
> > >>>> accepts a String instead of the WikiDescriptorAlias class, so
this
>
means
> >>>> that the class is useless:
> >>>>
> >>>> WikiDescriptor getByWikiAlias(String wikiAlias) throws
> >>>> WikiManagerException;
> >>>>
> >>>
> >>> I did not really think about it, I just take the work done by
Vincent
> > on
> > >>> wiki descriptors.
> > >>>
> > >>>
> > >>>> 2) WikiManager:
> > >>>>
> > >>>> The methods wikiIdExists and and isWikiIdAvailable do the
same
> thing,
> > they
> > >>>> just call it differently. Use only one, like wikiExists().
> > >>>>
> > >>>
> > >>> I should have added the javadoc about that. This is what I have
in
> > mind :
> > >>> - wikiExists(String wikiOd) as you said
> > >>> - wikiAvailable(String wikiId) that checks if the wiki id is
valid
> and
> > >>> free (that means that there is no existing database with that
name
--
> > not
> > >>> necessary related to XWiki).
> > >>>
> > >>> It is not implemented yet.
> > >>>
> > >>>
> > >>>>
> > >>>> Also, since it's a WikiManager, so the "wiki"
part is in the
name
of
the
>>>> class, we get it that it deals with wikis. You could remove the
"wiki"
> >>>> part
> >>>> from method names, like:
> >>>> - create instead of createWiki
> >>>> - delete instead of deleteWiki
> >>>> - exists instead of existsWiki
> >>>> - so on...
> >>>>
> >>>
> >>> I agree. I'll change it ;)
> >>>
> >>>
> >>>>
> >>>> Note on the naming of methods:
> >>>> I`ve noticed a rather annoying thing in the way the methods are
> named. So
> >>>> you have a class that is say... WikiDescriptor.
> >>>>
> >>>
> >>> Indeed, but a Wiki class would not contain something else than a
> >>> descriptor. I don't know what to do. Renaming WikiDescriptor to
Wiki?
> > >>>
> > >>>
> > >>>>
> > >>>> 3) WikiManager:
> > >>>>
> > >>>> What is the point of these 2 methods?
> > >>>>
> > >>>> void setDescriptor(WikiDescriptor descriptor);
> > >>>>
> > >>>> void removeDescriptor(WikiDescriptor descriptor);
> > >>>>
> > >>>> Looking at the implementation, I see that you`re handling
> descriptors
> > >>>> (setting/deleting) whithout corelating it to the wikis. I
mean,
if
you
> >>>> remove a descriptor, then you must remove the wiki too, and that
is
> the
> >>>> task of the WikiManager.delete() method.
> >>>>
> >>>
> >>> I have keeped them because it is used in the descriptors
> implementation of
> >>> Vincent. But I agree that it is weird to have it in the API.
> >>>
> >>>
> >>>>
> >>>> 4) Don`t use implementation details. Improve the interface (API)
if
there
>>>> is information you`re missing:
>>>>
>>>>
https://github.com/gdelhumeau/xwiki-platform/blob/new-wiki-api/xwiki-platfo…
>
>>>>
> >>>
> >>> I disagree.
> >>> In this code, I need to have the document related to the
descriptor.
It
> >>> depends on the current implementation, because we can imagine an
> >>> implementation where the descriptors are not stored in the wiki.
So I
> > won't
> > >>> add a getDocument() in the API, but I need it in my
implementation.
> > >>>
> > >>>
> > >>>>
> > >>>> 5) DefaultWikiManager.createDescritptor
> > >>>> Since you have a WikiDescriptorBuilder class, maybe that is
where
> the
> > >>>> logic
> > >>>> of creating/building the descriptor's details (objects,
document)
and
>>>> where
>>>> the logic of extracting the document name from wiki name and
viceversa
>>>> should be located.
>>>>
>>>>
https://github.com/gdelhumeau/xwiki-platform/blob/new-wiki-api/xwiki-platfo…
> > >>>>
> > >>>
> > >>> I agree, I will move that code into WikiDescriptorBuilder.
> > >>>
> > >>>
> > >>>>
> > >>>> 6) DefaultWikiManager.getByWikiId
> > >>>> Again seeing the logic of wikiID-to-document. It should be
located
in
> a
> >>>> single place and not spread in multiple places. Use a method of
the
> >>>> WikiManager class or of the
DescriptorBuilder class.
> >>>>
> >>>
> >>> OK.
> >>>
> >>>
> >>>>
> >>>> 7) WikiManagerScriptService:
> >>>> - deleteWiki: Why do you need Admin right to delete a wiki when
you
> did
> >>>> not
> >>>> need it in order to create it? That`s bad. We should probably
have a
> > >>>> deleteWiki right as well.
> > >>>
> > >>>
> > >>> It is a temporary solution in my implementation.
> > >>> The problem is: who has the right to remove a wiki ?
> > >>> - his owner? But we don't have the notion of owner in this
API.
It
>
belongs
> >>> to the wiki-users module to handle this concept.
> >>> - someone who has the CREATE_WIKI right? That means every person
who
> has
> >>> the CREATE_WIKI right can also delete any existing wiki?
> >>> - someone who as the DELETE_WIKI right? Why not, but we need to
make
> a
> > >>> vote for adding this new right.
> > >>>
> > >>> So, in my first implementation, I have used the admin right.
> > >>>
> > >>>
> > >>>> Also, consider the workspaces use case when user
> > >>>> create and delete wikis/workspaces when they want to and when
they
are
> >>>> done
> >>>> with them.
> >>>>
> >>>
> >>> To me, it belongs to the future wiki-user module.
> >>>
> >>>
> >>>> - context key for exceptions "lastexception2" is not the
best of
> choices.
> >>>> Try something like "wikiException" instead, or even just
> "lastexception"
> >>>> like it was before (if we consider this as a best practice).
What's
> > the
> > >>>> reason for "lastexception2"?
> > >>>>
> > >>>>
> > >>> It's a bad commit. I have setted this name for debugging
reason.
> > >>>
> > >>>
> > >>>>
> > >>>> As a general note, I fail to see the added value of this
module
at
> > this
> > >>>> point. I mean, the whole point was to have both the
functionality
of
> > >>>> WikiManager and Workspaces. I get it that it's a work in
progress
and
> that
> >>>> it's at the first stages, but you need to touch those points
before
> it
> > >>>> becomes relevant. Not that it's bad, just that it's
not much
there
> to
> > see
> > >>>> at this point.
> > >>>>
> > >>>
> > >>> The idea is to split in different simple modules, that enable us
to
>
easily
> >>> add a new feature or to depends on a module without depending on
the
> > whole
> > >>> core. I try to reduce the dependencies while I design the
modules.
>
>>>
> >>>
> >>>>
> >>>> Hope this helps,
> >>>> Eduard
> >>>>
> >>>>
> >>> Thanks Eduard!
> >>>
> >>> Louis-Marie
> >>>
> >>>
> >>>>
> >>>> On Mon, Oct 7, 2013 at 11:49 AM, Guillaume "Louis-Marie"
Delhumeau <
> >>>> gdelhumeau(a)xwiki.com>
wrote:
> >>>>
> >>>> > Hi.
> >>>> >
> >>>> > In this thread, I want to propose you the tiniest API that we
need
to
>>>> > handle multiwiki. All other features (users, templates,
workspaces...)
>>>> will
>>>> > be on other modules, because I think it is better for the
extensibility.
>>>> >
>>>> > The new module will be based on the
xwiki-platform-wiki-descriptor-api
>>>> > module, that I will rename to xwiki-platform-wiki-api. The
>>>> > WikiDescriptorManager becomes WikiManager and handle both
descriptors
>>>> and
>>>> > databases.
>>>> >
>>>> > This API will permit:
>>>> > * to create a wiki
>>>> > * to remove a wiki
>>>> > * to list all wikis (returning a list of descriptors)
>>>> > * ...
>>>> >
>>>> > You can see the code of that proposal there:
>>>> >
>>>> >
>>>>
https://github.com/gdelhumeau/xwiki-platform/tree/new-wiki-api/xwiki-platfo…
> > >>>> >
> > >>>> > The most important is to decide what the API must look
like.
The
>>>> > implementation can still be
modified afterwards.
>>>> >
>>>> > I hope you like it,
>>>> >
>>>> > Louis-Marie
>>>> > _______________________________________________
>>>> > devs mailing list
>>>> > devs(a)xwiki.org
>>>> >
http://lists.xwiki.org/mailman/listinfo/devs
>>>> >
>>>> _______________________________________________
>>>> devs mailing list
>>>> devs(a)xwiki.org
>>>>
http://lists.xwiki.org/mailman/listinfo/devs
>>>>
>>>
>>>
>> _______________________________________________
>> devs mailing list
>> devs(a)xwiki.org
>>
http://lists.xwiki.org/mailman/listinfo/devs
> _______________________________________________
> devs mailing list
> devs(a)xwiki.org
>
http://lists.xwiki.org/mailman/listinfo/devs
--
Thomas Mortagne
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs
_______________________________________________
devs mailing list
devs(a)xwiki.org
http://lists.xwiki.org/mailman/listinfo/devs