portletspec3
  1. portletspec3
  2. PORTLETSPEC3-7

Errata: Clarify Inconsistency about Removing Shared Render Parameters

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Component/s: None
    • Labels:
      None

      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.

        Activity

        Hide
        Neil Griffin added a comment -

        @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

        Show
        Neil Griffin added a comment - @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
        Show
        Neil Griffin added a comment - Test Portlet: https://github.com/ngriffin7a/portletbox/tree/master/issues/PORTLETSPEC3-7-portlet
        Hide
        msnicklous added a comment -

        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.

        Show
        msnicklous added a comment - 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.
        Hide
        msnicklous added a comment -

        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.
        
        
        Show
        msnicklous added a comment - 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.
        Hide
        Neil Griffin added a comment -

        +1

        Show
        Neil Griffin added a comment - +1
        Hide
        msnicklous added a comment -

        Updated apidocs as suggested.

        Show
        msnicklous added a comment - Updated apidocs as suggested.
        Hide
        msnicklous added a comment -

        reviewed on 20130730 and can be closed.

        Show
        msnicklous added a comment - reviewed on 20130730 and can be closed.
        Hide
        Neil Griffin added a comment -

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

        Show
        Neil Griffin added a comment - Update to my +1 vote: The changes to the apidocs will require no changes to Liferay Portal's implementation.

          People

          • Assignee:
            msnicklous
            Reporter:
            msnicklous
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: