Hello guys, In preparation of the meeting about the Gadget Container Cloud Service, I've gone through the current implementation of our open social code. Obviously, we should carefully design the actual REST services we want to expose when people start using this and look at the internal design of this project. This design is currently not on our wiki as far as I can tell.
I started looking at the opensocial.shindig module, and the first thing that came to mind is that, with the new multi tenancy model coming up in the Amdatu Platform 0.3 release, it might make sense to start refactoring the project to use it soon, since we only implemented multi tenancy as a quick fix anyway (see http://jira.amdatu.org/jira/browse/AMDATUOPENSOCIAL-42 for more background, it defines the work that still needs to be done in http://jira.amdatu.org/jira/browse/AMDATUOPENSOCIAL-113 but I would argue that it does not make sense to do this in the "old" multi tenancy model anymore). I do think we need the "proper" model in place if we want to provide this as a cloud service. Having all tenants in the same Shindig container does not feel right. Some other remarks about this module: PersonServiceDispatcher.getPerson() has a rather disturbing TODO in it that mentions: "For now we just loop over all tenants and see if one of them can identify the person we are looking for.". That method also fails to check for nulls, and does not unget the services it gets. OAuthTenantAwareStoreImple contains a comment: "this doesn't work properly in flexible environment where gadgets are added and removed from the store continuously". That needs to be addressed as we will do that. Like the platform does, this project should default to the bundle data area for storing information, instead of "work/shindig" (see ConfigurationAdminGuiceModule). Related, in AmdatuContainerConfig we should drop the system property "amdatu.dir". Next up, I looked at opensocial.gadgetmanagement, where I started with the REST services, which seem to be responsible for two things: 1) providing a REST API to the dashboard 2) providing a REST CRUD API for gadgets These seem to be provided by a combination of two classes and a base class both of them inherit: BaseRESTServiceImpl that is extended by both GadgetManagementRESTServiceImpl and WidgetRESTServiceImpl. The first contains: /gadgetstore /status /categories /gadgets/all /gadgets?uri=... (GET/DELETE) /gadgets?uri=...&category=... (POST) /authorized The second contains: /widgets /mine Again, some remarks about this design: the split seems rather arbitrary to me, and does not align with the two different responsibilities (dashboard API and CRUD API for gadgets) why use both the term "gadgets" and "widgets"? what is the difference? there seem to be no CRUD operations for gadget categories. We also seem to use different methods of registering gadgets right now. You can do that whiteboard style, or by directly adding it to a store. I would propose to change this so we always use the whiteboard. The gadget management component would then listen to two different "services": GadgetDefinition, which probably should be an interface, and which sounds more logical to me than the GadgetDefinitionProvider that is now picked up. I would just drop the latter completely, it simply lists GadgetDefinitions, which is not that useful. GadgetCategory, which should also be an interface, and we should probably try to eliminate any "hardcoded" categories here. So the gadget container itself listens to both, and registers all definitions and categories it sees. It does not need to persist anything. All it needs to do is react to the services. Using this as a basis, we can add two optional bundles to do: REST style registration: Via the REST API there are CRUD operations for both gadget definitions and categories. Somehow these operations need to be persisted (optionally). Eventually, these will lead to GadgetDefinitions and GadgetCategories that are registered whiteboard style. External gadgets via configuration: Those will also be registered whiteboard style. A question I have: Why did we introduce gadget categories? Is it essential to have them or can we also work without them, or with some other "grouping" mechanism such as the tags that GMail uses? Another question: OpenSocialContainer suggests independence from Shindig, but can we really implement this interface differently? To be honest, when reading the documentation of the interface, I don't really understand how to implement it. Final question about this bundle: Why doesn't the whiteboard currently work for external gadgets? It complains that it supports only relative URLs, but why? On to opensocial.profile, where I have a couple of smaller remarks: PersonService.setFriends() this is the only method that uses an ID of a friend, everywhere else we use either "Person" or "User". Also, everywhere else we use List<X> instead of X[] Why is SerializablePersonImpl part of the API? What's the point of a Friend (since a Friend is a Person)? PersonServiceImpl uses serialization, which will fail when you update the bundle with classes that contain incompatible changes. I know that can be fixed partially by only doing compatible changes, but bundles can be upgraded and downgraded at will, which will eventually break this model. And finally, opensocial.dashboardplugin: It contains a readme that uses yuicompressor, which compacts javascript. Why is this not part of the normal Maven build? Greetings, Marcel
_______________________________________________ Amdatu-developers mailing list [email protected] http://lists.amdatu.org/mailman/listinfo/amdatu-developers
