Re: [gatein-dev] GateIn Api Discussion

On May 25, 2011, at 8:11 AM, Thomas Heute wrote: > > Thanks for sharing this Matt, see inline > > On 05/25/2011 05:08 AM, Matt Wringe wrote: >> There have been some discussion on creating an api package to create a >> simple way for people to interact and manipulate a gatein portal. >> >> Example situations would be things like creating a gadget, adding an >> application to a category, creating a page and adding it to a node, >> adding applications to a page, managing users, etc... > > First I want to reiterate that for the scope of next release we said we > stick with: > - application registry features > - page/navigation node metadata > So > - creating a page > - adding apps to a page > - managing users > is out of scope for the coming release. (will be for the release after)

>> Two different solutions have been created, both are a bit different in >> approach. These are preliminary and are not meant to be complete, it is >> more meant to start of a discussion: >> >> https://github.com/metacosm/gatein-api > > Some rough notes I took while looking into it, I mainly looked at the > testcases/usecases > > https://github.com/metacosm/gatein-api/blob/master/src/test/java/org/gate... > Looks like 2 first tests are contradicting themselves: > assert portal.getContentRegistry().getCategory("inexistent") == null; > assert portal.getContentRegistry().getCategory("inexistent") != null; Yes, I noticed that as well and this has already been addressed in the code. Note that I'm still debating wether getOrCreate methods are useful or not. They are useful for users who just want to retrieve something whether it previously existed or not. I'm not sure whether that's a predominant use case but I personally thought that it makes writing code against the API a little cleaner by avoiding a lot of boilerplate code. It also maps reasonably well to REST with the contract that GET will never fail (though I need to think this one through more) :) > ************ > public void assigningContentToAWindowOnAPage() > is out of scope of what we will provide in that version, we agreed to > have read-only info about pages/navigation nodes information without the > info about page compositions. I added this use case because this is actually something I need for WSRP (and I suspect Nick would need as well) import/export so I wanted to see how that particular scenario could work out. > ************ > public void assigningContentToACategory() > > registry.getOrCreateCategory("category"); > I prefer getCategory() and createCategory() to be distinct (with proper > exceptions), plus if there is a need for a getOrCreate or > getCategory(String name, boolen create) we can add it later Like I said, I'm still debating the usefulness of such methods. You'll see that some use cases use the check-for-existence/create or get pattern whether others use getOrCreate directly. It's true that we can add the getOrCreate methods later if needed. > category.getKnownManagedContentIds(); > Meaning ? Confused about the testcase. What's a ManagedContent ? Basically Content is read-only: portlet, remote portlet, gadget, etc. When you add it to a category, it becomes managed and some of its properties can then be modified (display name, title, etc). Basically, you need something that is a wrapper around the read-only content as a portlet can be added to different categories with different names (and presumably at some point, different preferences though it's not currently available via the application registry). I first had implemented a version that didn't require ManagedContent but then it made using Content a lot more complex because you would need to change the reference you held to a given Content after you added it to a category to be able to manipulate it. Basically, I previously had: Application addedApp = category.add(application); assert category.contains(name); assert addedApp.equals(category.getApplication(name)); assert addedApp.isManaged(); assert !application.equals(category.getApplication(name)) : "The added application should be wrapped so that it can be modified"; assert application.equals(addedApp.getDeployedParent()); List<Application> applications = registry.getManagedApplications(Filter.ALL, Filter.ALL, Application.SORT_BY_NAME); assert applications.size() == 1 : "There should only be one application matching"; assert applications.contains(addedApp) : "Application should have become managed"; assert !applications.contains(application) : "Original application should have been left untouched"; try { application.setDisplayName("displayName"); Assert.fail("Shouldn't be possible to modify the original application. Use the result of Category.add instead."); } catch (ApplicationException e) { // expected } See: https://github.com/metacosm/gatein-api/blob/5df7e9a7feacb13ad2198414b540f... for the full version. So things were a lot trickier to deal with and I decided to make the concept explicit rather than implicit. Matt's version deals with this the same way I used to: //NOTE: the returned application is _not_ the same object as the one passed. The returned application // should be mostly a clone of the passed application (but maybe slightly different, eg the name passed may change something like the display-name). public Application addApplication(String name, Application application); but that opens a can of worms in my opinion as users might not understand why they need to change the reference to their application to be able to use it so I decided against transparent wrapping and in favor of explicit one.

Even though it makes for a slightly more complex API at first glance, I believe it's actually easier to deal with because things are explicit and you keep the original content read-only.

> I was actually expecting something more like: > Portlet portlet = new Portlet(...); > Category category = new Category(...); > category.add(portlet); > contentRegistry.save(category); The less detached the API is, the better in my opinion. I would rather also have an API that is persistent by default, i.e. you don't need to do call save or update methods to persist the changes you made to "detached" entities. I understand, though, that it might be tricky to do so if we want to have large batches of modifications. I guess it boils down to whether we want the API to be mainly read-oriented or write-oriented or a mix of both. :)

> ************ > https://github.com/metacosm/gatein-api/blob/master/src/test/java/org/gate... > > getSingleResult in Hibernate isn't the best thing created, it actually > didn't make it through the JPA API. > I would rather remove this method from the API, and potentially add it > in some form later if really needed. > - Nodes.getSingleOrFail(Query.<Dashboard>builder().where(filter).build()); Yes, this is just syntactic sugar but an API is also a lot about syntactic sugar (making your users' life easier). However, it's probably a good idea to have a minimal version of the API first and then add the methods to smooth out the desire paths (http://en.wikipedia.org/wiki/Desire_path) once we've had some feedback on the API. Cordialement / Best, Chris == Principal Software Engineer / JBoss Enterprise Middleware Red Hat, Inc. Follow GateIn: http://blog.gatein.org / http://twitter.com/gatein Follow me: http://metacosm.info/metacosm / http://twitter.com/metacosm