What I propose for the extensibility: - anyone can extends the Wiki class. - each module will have its own XWikiServerClassDocumentInitializer, that add custom fields into the XWikiServerClass document. - the minimal XWikiServerClassDocumentInitializer will only contain a few fields.
2013/10/7 Thomas Mortagne <[email protected]> > Before writing extensions the API need to be extensible and we don't > see it in your current API ;) This needs to be there before validating > this API. > > On Mon, Oct 7, 2013 at 5:17 PM, Guillaume "Louis-Marie" Delhumeau > <[email protected]> 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-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-api/src/main/java/org/xwiki/wiki > > > > I have only changed the API, not the implementation. > > > > I think it starts to be clean. > > > > > > 2013/10/7 Guillaume "Louis-Marie" Delhumeau <[email protected]> > > > >> Hi Eddy. > >> > >> First, thanks for your answer. > >> > >> 2013/10/7 Eduard Moraru <[email protected]> > >> > >>> 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-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-default/src/main/java/org/xwiki/wiki/descriptor/internal/DefaultWikiManager.java#L209 > >>> > >> > >> 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-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-default/src/main/java/org/xwiki/wiki/descriptor/internal/DefaultWikiManager.java#L126 > >>> > >> > >> 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 < > >>> [email protected]> 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-platform-core/xwiki-platform-wiki > >>> > > >>> > 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 > >>> > [email protected] > >>> > http://lists.xwiki.org/mailman/listinfo/devs > >>> > > >>> _______________________________________________ > >>> devs mailing list > >>> [email protected] > >>> http://lists.xwiki.org/mailman/listinfo/devs > >>> > >> > >> > > _______________________________________________ > > devs mailing list > > [email protected] > > http://lists.xwiki.org/mailman/listinfo/devs > > > > -- > Thomas Mortagne > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
