[PORTLETSPEC3-7] Errata: Clarify Inconsistency about Removing Shared Render Parameters Created: 02/May/13  Updated: 05/Aug/13  Resolved: 26/Jul/13

Status: Closed
Project: portletspec3
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

Referring to Working Document 3, Section "PLT 11.1.2 Public Render Paramaters" states on
page 77, beginning on line 22 the following:

"If a portlet wants to delete a public render parameter it needs to use the
removePublicRenderParameter method on the StateAwareResponse or the
PortletURL."

Judging from this section, it would seem like public render parameters can only
be removed using this method.

However, in the javadoc for StateAwareResponse.setRenderParameters it is stated:

-------------------------
setRenderParameters

void setRenderParameters(java.util.Map<java.lang.String,java.lang.String[]> parameters)

    Sets a parameter map for the render request.

    All previously set render parameters are cleared.

    These parameters will be accessible in all sub-sequent render calls via the PortletRequest.getParameter call 
    until a new request is targeted to the portlet.

    The given parameters do not need to be encoded prior to calling this method.

    The portlet should not modify the map any further after calling this method.

    Parameters:
        parameters - Map containing parameter names for the render phase as keys and parameter values as map values. 
        The keys in the parameter map must be of type String. The values in the parameter map must be of type String array (String[]). 
    Throws:
        java.lang.IllegalArgumentException - if parameters is null, if any of the keys in the Map are null, 
        if any of the keys is not a String, or if any of the values is not a String array. 
        java.lang.IllegalStateException - if the method is invoked after sendRedirect has been called.
-------------------------

In particular, it states that "All previously set render parameters are cleared." without
explicitly taking public and private parameters into account.

What happens when the new map provided to setRenderParameters changes or maybe even does not contain
a public render parameter that was set on the corresponding request? Should that public render parameter
be changed or removed?

Also, can individual entries in the value string[] be null? if so, what would that mean?
In general, can parameters have a value of null?

We need to decide how it should behave. My take would be to update the javadoc for
StateAwareResponse.setParameters as follows:

Corrected:

-------------------------
setRenderParameters

void setRenderParameters(java.util.Map<java.lang.String,java.lang.String[]> parameters)

    Sets a parameter map for the render request.
MSN UPDATE==>    The new paramater map applies to public as well as to private render parameters.

    All previously set render parameters are cleared.

    These parameters will be accessible in all sub-sequent render calls via the PortletRequest.getParameter call 
    until a new request is targeted to the portlet.

    The given parameters do not need to be encoded prior to calling this method.

    The portlet should not modify the map any further after calling this method.

    Parameters:
        parameters - Map containing parameter names for the render phase as keys and parameter values as map values. 
        The keys in the parameter map must be of type String. The values in the parameter map must be of type String array (String[]). 
    Throws:
        java.lang.IllegalArgumentException - if parameters is null, if any of the keys in the Map are null, 
        if any of the keys is not a String, or if any of the values is not a String array
MSN UPDATE==>        , or and entries in the values String array are null. 
        java.lang.IllegalStateException - if the method is invoked after sendRedirect has been called.
-------------------------

And the above-mentioned section in the spec needs to be changed as follows:

Corrected:
-------------------------
If a portlet wants to delete a public render parameter it needs to use the
removePublicRenderParameter method or the setRenderParameters method on the StateAwareResponse or the
PortletURL.
-------------------------

However, I'm not sure that this meets the original intent and I am not convinced
these changes alone would make for a consistent description.

More work needs to be done in this area.



 Comments   
Comment by Neil Griffin [ 13/May/13 ]

@msnicklous: I checked the Pluto portlet container implementation, and it interprets the requirements according to the clarifications you suggest.

However, I just checked the Liferay portlet container implementation, it it interprets:

"All previously set render parameters are cleared"

to mean:

"All previously set [non-public] render parameters are cleared"

If you feel that the clarifications are essential, then please let me know since it would introduce a change into the way Liferay works.

Thanks,

Neil

Comment by Neil Griffin [ 15/May/13 ]

Test Portlet: https://github.com/ngriffin7a/portletbox/tree/master/issues/PORTLETSPEC3-7-portlet

Comment by msnicklous [ 18/Jul/13 ]

Using the PORTLETSPEC3-7 portlet, you can see that you can delete parameters by specifying an empty map "actionResponse.setRenderParameters(new HashMap<String, String[]>());". This works for all private parameters that have been previously set. With public parameters, there are two cases:

1) If the public parameter was set previously during execution of the same action request, it can be deleted using an empty map.

2) If the public parameter was set during a previous request, it remains unchanged when attempting to delete it using an empty map, and no exception is thrown.

However, a public render parameter can be set to a new value using setParameters() regardless of whether the public parameter already existed or not.

Both Pluto and WebSphere show this behavior.

Comment by msnicklous [ 19/Jul/13 ]

I think that it should not be permissible to remove public render parameters by setting them to null or by leaving them out of the map parameter of the setRenderParameters() methods.

I suggest updating the description for StateAwareResponse#setRenderParameters() method as follows:

StateAwareResponse

setRenderParameters

void setRenderParameters(java.util.Map<java.lang.String,java.lang.String[]> parameters)

Sets a parameter map for the render request.

This method can be used to set both public and private render parameters. 

These parameters will be accessible in all subsequent render calls via the PortletRequest.getParameter call
until a new request is targeted to the portlet.

Any previously set private render parameter that is not contained in the new map is removed. However, public render 
parameters cannot be removed by excluding them from the map. Public render parameters that are not included 
in the map remain unchanged.

The given parameters do not need to be encoded prior to calling this method.

The portlet should not modify the map any further after calling this method.

Parameters:
parameters - Map containing parameter names for the render phase as keys and parameter values as map values.
The keys in the parameter map must be of type String and may not be null or the null string (""). The values in the parameter 
map must be of type String array (String[]). Neither the values array nor any of its elements may be null; however,
the null string ("") is allowed. 

Throws:
java.lang.IllegalArgumentException - if parameters is null, if any of the keys in the Map are null or the null string,
if any of the keys is not a String, if any of the values is not a String array, or if any of the Sting array elements are null.
java.lang.IllegalStateException - if the method is invoked after sendRedirect has been called.

Comment by Neil Griffin [ 23/Jul/13 ]

+1

Comment by msnicklous [ 26/Jul/13 ]

Updated apidocs as suggested.

Comment by msnicklous [ 02/Aug/13 ]

reviewed on 20130730 and can be closed.

Comment by Neil Griffin [ 05/Aug/13 ]

Update to my +1 vote: The changes to the apidocs will require no changes to Liferay Portal's implementation.

Generated at Sat Aug 29 10:40:55 UTC 2015 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.