Hi,
On Mon, Oct 7, 2013 at 5:33 PM, Guillaume "Louis-Marie" Delhumeau <
gdelhumeau(a)xwiki.com> wrote:
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?
Scratch this note. I did some rephrasing and forgot to remove it. I noticed
you`ve already made the change (as you`ve understood it), but I`m not sure
it's the best thing to do.
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.
Try not to shoot yourself in the leg with this over-zelous separation of
concerns :). In our previous implementation, owners can delete their own
wikis or top level admins (admins on the main wiki). To this, we could add
the deleteWiki right that you`ve mentioned, but, at first, we should stick
to what we had (owners + top admins).
All this information (owner, homepage, etc) should be reflected by the
descriptor.
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.
Why? What exactly do you plan to include in such a 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.
As Vincent mentioned, in order for these modules to make sense, they would
have to be optional. However, the distribution will come with all of them
bundled and they will still be loaded and present.
As mentioned above, care to elaborate more on this plan that you have?
Thanks,
Eduard
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