portletspec3
  1. portletspec3
  2. PORTLETSPEC3-8

Errata: Clarify inconsistencies regarding getting and setting parameters

    Details

      Description

      Reading the spec document on parameters and comparing it with the javadoc on
      "get...Parameter..." and "set...Parameter..." calls on various Response, Request, and
      URL classes, it seems to me that there are a number of inconsistencies that should
      be addressed, for example:

      1. Is a parameter allowed to have a value of null?
      2. Under which cases (if any) should setting a parameter to a value of null remove the parameter?
      3. In which cases exactly are public parameters get and set?
      4. In which cases are public render parameters set or updated?
      5. In which cases are public render parameters removed?
      6. Should the "getParameterMap" methods always return an immutable map?
      7. probably more that I haven't noticed yet ...

      To #6 -

      The javadoc for PortletRequest.getParameterMap states:
      -------------------------
          Returns:
              an immutable Map containing parameter names as keys and parameter values as map 
              values, or an empty Map if no parameters exist. 
              The keys in the parameter map are of type String. 
              The values in the parameter map are of type String array (String[]).
      -------------------------
      
      While the javadoc for BaseURL.getParameterMap states:
      -------------------------
      Returns:
          Map containing parameter names as keys and parameter values as map 
          values, or an empty Map if no parameters exist. 
          The keys in the parameter map are of type String. 
          The values in the parameter map are of type String array (String[]).
      -------------------------
      

      Shouldn't the returned Map values be consistently immutable?

      If not, and if we want to be able to update the actual parameters by updating the
      Map returned by BaseURL.getParameterMap, it needs to be described clearly.

        Activity

        Hide
        msnicklous added a comment -

        done.

        Show
        msnicklous added a comment - done.
        Hide
        msnicklous added a comment -

        implemented updates for descriptions of BaseURL.getParameterMap() and StateAwareResponse.getRenderParameterMap().

        Show
        msnicklous added a comment - implemented updates for descriptions of BaseURL.getParameterMap() and StateAwareResponse.getRenderParameterMap().
        Hide
        Neil Griffin added a comment -

        @Scott: I received feedback from my colleagues, and the unanimous response was to have StateAwareResponse.getRenderParameterMap() return an immutable map. This would make the API more clear that the only way to set parameters (with a Map) would be to call StateAwareResponse.setRenderParameters(java.util.Map).

        Show
        Neil Griffin added a comment - @Scott: I received feedback from my colleagues, and the unanimous response was to have StateAwareResponse.getRenderParameterMap() return an immutable map. This would make the API more clear that the only way to set parameters (with a Map) would be to call StateAwareResponse.setRenderParameters(java.util.Map) .
        Hide
        andre.hagemeier added a comment -

        Sorry Scott, I guess I mixed up some things here.
        The only question to me is, what to do with the spec and the implementations. I think the implementations cannot be forced to return immutable maps.
        So two possibilities would be left:

        • allow mutable maps in the spec
        • deprecate mutable maps in 362 (but still allow them for backward compatibility) and make immutable ones the default. Therefore, a subclassing approach could be used like above stated.

        Anyway, this issue needs to be documented properly, to make clear why there is a problem at all.

        Show
        andre.hagemeier added a comment - Sorry Scott, I guess I mixed up some things here. The only question to me is, what to do with the spec and the implementations. I think the implementations cannot be forced to return immutable maps. So two possibilities would be left: allow mutable maps in the spec deprecate mutable maps in 362 (but still allow them for backward compatibility) and make immutable ones the default. Therefore, a subclassing approach could be used like above stated. Anyway, this issue needs to be documented properly, to make clear why there is a problem at all.
        Hide
        msnicklous added a comment -

        Andre, sorry, don't really follow you. All methods listed currently exist. In fact, the code in the examples is copy/pasted from test portlets that were run with
        varying degrees of success against several portal implementations. Nothing would be deprecated.

        I don't think we should make assumptions about the internal implementation of a portlet container. We just need to describe external behavior.

        Show
        msnicklous added a comment - Andre, sorry, don't really follow you. All methods listed currently exist. In fact, the code in the examples is copy/pasted from test portlets that were run with varying degrees of success against several portal implementations. Nothing would be deprecated. I don't think we should make assumptions about the internal implementation of a portlet container. We just need to describe external behavior.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: