As one of the spec-leads of the Portlet Specification 1.0 I'm glad to see a new version
is in the works taking care of a much needed revamping.
I did a quick read of the draft while coming to work (I take the school bus) and following are my first impressions of it.
Before going to the point, I'm fully aware that this is an early draft and that some of my coments will be addressed once things are iron out. And please keep them coming with track changes
on between drafts.AA-1)
PLT.5.2.3 End of Service (P28/L8..29): I'm not sure this is a good idea, this breaks with the previous assumption that the init() method is called once sometime before the first request is sent to it, and that the destroy is called when the application is going down. This will most likely create some funny situations in cases things are coded with this assumption.AA-2)
PLT.5.4 Request Handling (P32/L12..13): "In addition to these portlet initiated events the portal/portlet container may issue portal/portlet container specific events.". I wonder what kind of events the portal/portlet container would generate that are useful for the portlet application. I was under the impression that eventing was for 'inter-portlet communication'. If a portal/portlet-container wants to provide some info to the portlet this can be done via request attributes. It sounds like there would be another programmable API somewhere in the portal that would receive/generate events from/for portlets, if that is the case please spell it out.AA-3)
PLT.5.4 Request Handling (P32/L15..25,P33/L1..4): a portlet serving a resource? That looks like the Servlet.service() method to me. A portlet is a portlet, not a mutant portlet-servlet. I'd rather go for defining a path in the portlet configuration (deployment descriptor) that indicates the path for a resources servlet and that servlet when invoked by the container should receive the context of the portlet as request attributes (note that this can easily be implemented as a servlet filter).AA-4)
PLT.5.4.5 GenericPortlet (P36/L25...33): Not sure using annotations to avoid a switch or if then else in a single processEvent() method is a good idea. From the container/portlet API contract perspective I'd think a well defined set of methods is better than several ones with arbitrary names. I'd rather implement in the GenericPortlet a dispatching mechanism as render() does with doView(), doEdit() and doHelp().AA-5)
PLT.220.127.116.11 Render Request Parameters (P56/L19): talking about 'non-shared parameters', they have not been defined yet, things should be re-arranged or it should be mentioned that they are defined in the next section.AA-6)
PLT.18.104.22.168 Shared Render Parameters (P58/L3..34): I don't understand the reason for depriving portlets of shared-parameters only because they were not the target of a render URL? It seems at odds with the idea of shared-parameters, shared but only sometimes? I guess this is because pre-caching of generated content and portlet URL, but it is definitely strange and not intuitive to the developer. Either share parameters are there always or they are not, but only sometimes, under certain conditions, it will be highly error prone. Also, another thing with shared parameters, they are independent of the portlet and the portlet URL that set them, they are sticky, what happens when there are race conditions, 2 portlets setting different values for the same shared parameters, one portlet setting them the other deleting it?AA-7)
PLT.11.2 ClientHttpRequest (P62/L1..7): Serving resources based on what is described before in the spec can be done only via GET calls, why this interface that is to be used for resources has methods for uploading data?AA-8)
PLT.11.5 EventRequest Interface (P63/L32..38): Event payload is defined as a serializable object or as JAXB mapping. Wouldn't be more consistent to use the same paradigm as for request/response IO, streams, and on top of it, depending on the application needs the proper reader/writer is used. I guess still would make sense to include a alternate signature that sets and gets a Java object, that when eventing among collocated portlets, could share objects by reference (still this could cause problems if the object in question is distributed to many portlets simultaneously and it is a mutable object, not to mention classloader issues when being across portlet-apps).AA-9)
PLT.12. StateAwareResponse Interface (P66/L14..19): Not clear what is this about but it seems to me that the request/response interfaces are being broken in to many specialized interface layers thus cluttering the API and making more difficult for a developer to look at it in the javadocs (yes, stupid as it may sound think how often developers looks at the javadocs).AA-10)
PLT.13 Resource Service (P72..74): same as #AA-3. Again, a servlet should be used for this, not need to define a new (and handicapped) way of doing it.AA-11)
PLT.17.5 Shared session attributes (P92/L7..P93/L19): P92/L16 defeats the purpose of this, what is the point then? Not that I like it, until now portlet container implementations could get a free ride on high availability, fail over and scalability by leveraging the underlying servlet container. By adding shared session data across web-apps (portlet-apps) the portlet container implementation must implement its own mechanisms for such functionality and in doing so it will have to do it for the regular session, including servlet session, in order to ensure data consistency, else you may end up in a situation that the data failover strategy is inconsistent.AA-12)
PLT.17.6.1 Process action and process event phase (P95/L16..18): atomic transactions? there is not database in here. I guess the intention is to say that session operations must be thread-safe.AA-13)
PLT.18.1 Obtaining a PortletRequestDispatcher (P97/L20..21): again, this is reinventing a handicapped servlet container within the portlet container, let a servlet do the resource serving.AA-14)
PLT.19.2.1 Filter Lifecycle (P1004/L(no more numbers in the spec): having a single doFilter() method handling all request types (action, event, render, resource) is will make filters (unnecessary) error prone, I'd rather have one filter method per type of request.