I can add the possibility to have custom data stored in a map. But how should we store it in the wiki?
2013/10/7 Thomas Mortagne <[email protected]> > On Mon, Oct 7, 2013 at 5:38 PM, Guillaume "Louis-Marie" Delhumeau > <[email protected]> wrote: > > 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. > > That's not extension. I was expecting a generic way to access and > modify descriptor properties. The "extensions" you are talking about > are going to be completely independents modules that directly > manipulate xobjects and they will have to provide their own complete > API, do there own caching of descriptors etc, extending Wiki class is > pretty useless for them. > > > > > > > 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 > > > > -- > 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
