Hi, I took a look at the new model proposal and there are a few questions and suggested changes.
https://github.com/xwiki/xwiki-platform/blob/feature-newmodel/xwiki-platform-core/xwiki-platform-model/xwiki-platform-model-api/src/main/java/org/xwiki/model/Entity.java#L32 Should this be a String rather than a Java UUID type? https://github.com/xwiki/xwiki-platform/blob/feature-newmodel/xwiki-platform-core/xwiki-platform-model/xwiki-platform-model-api/src/main/java/org/xwiki/model/Entity.java#L42 If we use getChildren() rather than getChildReferences(), we will be mangling the model with the database code which violates the MVC principle. Also we run the risk of encouraging users to walk the tree looking for the entity it wants rather than creating a reference to it and using that. Walking the tree would obviously incur enormous unnecessary database load. https://github.com/xwiki/xwiki-platform/blob/feature-newmodel/xwiki-platform-core/xwiki-platform-model/xwiki-platform-model-api/src/main/java/org/xwiki/model/ModelContext.java Do we want to continue with the concept of the global variable representing the "current" document? I'm currently not brimming with alternative ideas but the heavy reliance on the XWikiContext has lead to a lot of inter-modular communication which makes oldcore difficult to maintain, perhaps we can prevent this in the new model. https://github.com/xwiki/xwiki-platform/blob/feature-newmodel/xwiki-platform-core/xwiki-platform-model/xwiki-platform-model-api/src/main/java/org/xwiki/model/Object.java I would rather this have a name which doesn't collide with a java.lang class. When I see Object in a .java file I would like it to always mean java.lang.Object. https://github.com/xwiki/xwiki-platform/blob/feature-newmodel/xwiki-platform-core/xwiki-platform-model/xwiki-platform-model-api/src/main/java/org/xwiki/model/ObjectDefinition.java I like XClass better for this purpose, it is essentially a class and creating a new name which nobody has ever heard of will just steepen the learning curve and make things hard on newcomers. https://github.com/xwiki/xwiki-platform/blob/feature-newmodel/xwiki-platform-core/xwiki-platform-model/xwiki-platform-model-api/src/main/java/org/xwiki/model/ObjectProperty.java Why not give the user direct access to the property without the intervening wrapper? https://github.com/xwiki/xwiki-platform/blob/feature-newmodel/xwiki-platform-core/xwiki-platform-model/xwiki-platform-model-api/src/main/java/org/xwiki/model/Persistable.java#L34 I don't like this. A Persistable is saved in a store by a user. Who is the user and where are they saving it? What if I want to have multiple stores? Must there be a global variable representing the store which is implied by this function? The comment and minorEdit fields are also a bit dubious, perhaps in the future they will make no sense, perhaps they should really be attributes of the Document, also the "attributes" parameter reminds me of this: http://geek-and-poke.com/2012/06/how-to-create-a-stable-api.html Finally, giving the document awareness of where and how it is stored violates the MVC boundries. I think we should not worry about the API which stores the Persistable when we are creating the model, the store API should be separate so that it can evolve while the model stays stable. https://github.com/xwiki/xwiki-platform/blob/feature-newmodel/xwiki-platform-core/xwiki-platform-model/xwiki-platform-model-api/src/main/java/org/xwiki/model/Session.java A session is essentially a transaction? If so why not call it Transaction? Also why does it extend Persistable? How do you persist a session? https://github.com/xwiki/xwiki-platform/blob/feature-newmodel/xwiki-platform-core/xwiki-platform-model/xwiki-platform-model-api/src/main/java/org/xwiki/model/Space.java#L41 Why does addSpace() not take a Space parameter? it makes sense to me that you would do: parent = storeApi.get(wikiRef, "SomeSpace", user); child = new DefaultSpace(); child.addDocument("ADocument", new DefaultDocument("This is content.")); parent.addSpace("ChildSpace", child); storeApi.save(parent, user); I'm just brainstorming here but this seems like a reasonable approach.. I noticed the lack of a User data type, is that just something which is planned for the future or is it intentionally missing? Thanks, Caleb _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
