Re: [xwiki-devs] [Proposal] Wiki API

Thanks to your advices (Eddy and Thomas), I propose you a clean version:

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

>> returns a string? >> > >> Also, the only method in the WikiDescriptorManager that works with

>> accepts a String instead of the WikiDescriptorAlias class, so this

>> 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,

>> 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 --

> necessary related to XWiki). > > It is not implemented yet. > > >> >> Also, since it's a WikiManager, so the "wiki" part is in the name of

>> 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.

>> 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

> 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

>> is information you`re missing: >> >>

>> > > 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

> 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. >> >>

>> > > 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

> 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

>> Try something like "wikiException" instead, or even just

>> 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

>> 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

>> at this point. >> > > The idea is to split in different simple modules, that enable us to

> add a new feature or to depends on a module without depending on the

> 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&gt; wrote: >> >> > Hi. >> > >> > In this thread, I want to propose you the tiniest API that we need to >> > handle multiwiki. All other features (users, templates,

>> will >> > be on other modules, because I think it is better for the

>> > >> > The new module will be based on the

>> > 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: >> > >> > >>

_______________________________________________ devs mailing list devs(a)xwiki.org http://lists.xwiki.org/mailman/listinfo/devs