I'm concerned with the implementation of the clearState method and
actually the PortletStateManager in general.

The fact that we cannot use normal get or remove methods to
retrieve/remove items from the map when we have the key implies there
may be something wrong with the implementation.

It seems like we should have a Map keyed on geyKey(portletWindow) and
then the Map stores Maps which are keyed on whatever additional
information was used in the first Key.

Then we can do something like this:
Map map = <whereever we got map from>
String key = getKey(portletWindow) <-- can we even just key on portlet
windows???)
Map portletWindowMap = map.get(key);
portletWindowMap.clear();
map.remove(key);

However this would most likely require other code to be rewritten (and
is incompatible with clearState(HttpServletRequest). What are the use
cases when clearState(HttpServletRequest) would be called vs.
clearState(PortletWindow)

Also, getKey is checking if a session exists (clearState does this
also) but it calls request.getSession() which according to the JavaDoc
does this: "Returns the current session associated with this request, or
if the request does not have a session, creates one."

>
>
>> Also, getKey is checking if a session exists (clearState does this
>> also) but it calls request.getSession() which according to the JavaDoc
>> does this: "Returns the current session associated with this request, or
>> if the request does not have a session, creates one."
>>
>>
>
> This was part of the problem that led to the first memory leak (well second
> actually) in CPortletAdapter. Since the key had previously depended on
> having a valid session, it threw an exception trying to obtain it when the
> HttpServletResponse object had already been committed. To get around this
> we provided a fix that modified getKey to use the window's ID. We had also
> changed CPortletAdapter to set the PortletWindow's ID to include the session
> Id. This does imply that we require PortletWindow IDs to be instance
> unique. I couldn't find anywhere in the spec which indicates a violation.
>
>
My point was that the method was doing additional checks.
HttpServletRequest.getSession() always returns a session, regardless
of whether one originally existed or not (it will create a new one).
Thus an if (session == null) check is redundant.
HttpServletRequest.getSession(false) would need the check.

Just an additional question.. Is there a reason the current state is
not stored with the PortletWindow so you could say something like
portletWindow.getCurrentState() as opposed to checking a map? It seems
like each portletWindow is unique and holds some kind of state (since it
has its own HttpServletRequest). If this was possible, it would
eliminate any need for a map to hold this information.