First of all, you should probably come up with some plausible use cases for extending this API and then build it with them in mind. Maybe I`ve missed some discussions, but right now I just don`t see it.
On Mon, Oct 7, 2013 at 7:44 PM, Guillaume "Louis-Marie" Delhumeau < [email protected]> wrote: > 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 > _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
