MyFaces Core
  1. MyFaces Core
  2. MYFACES-655

RequestMap doesn't implement putAll as it should

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.1.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      The JSF description of for the ExternalContext.getRequestMap() method says:

      """Return a mutable Map representing the request scope attributes for the current application. The returned Map must implement the entire contract for a modifiable map as described in the JavaDocs for java.util.Map. Modifications made in the Map must cause the corresponding changes in the set of request scope attributes."""

      yet MyFaces returns a RequestMap class which throws UnsupportedOperationException for both the putAll() and the clear() methods.

      This is a major bug which will break apps (like mine) that depend on being able to add params via putAll(), for example.

        Activity

        Hide
        Matthias Weßendorf added a comment -

        Craig,

        any updates?

        Thx,
        Matthias

        Show
        Matthias Weßendorf added a comment - Craig, any updates? Thx, Matthias
        Hide
        Craig McClanahan added a comment -

        Matthias has a fair question (about why the TCK requires UnsupportedOperattionException) here. Forwarded to the spec leads for further comment.

        Craig

        Show
        Craig McClanahan added a comment - Matthias has a fair question (about why the TCK requires UnsupportedOperattionException) here. Forwarded to the spec leads for further comment. Craig
        Hide
        Mathias Broekelmann added a comment -

        Thanks Craig for giving some light to this.

        I still think there is a difference between optional an mandatory. If clear and putAll is an optional operation for a mutable map why is the tck expecting it to throw an UnsupportedOpperation? IMO this is a tck issue.

        Show
        Mathias Broekelmann added a comment - Thanks Craig for giving some light to this. I still think there is a difference between optional an mandatory. If clear and putAll is an optional operation for a mutable map why is the tck expecting it to throw an UnsupportedOpperation? IMO this is a tck issue.
        Hide
        Craig McClanahan added a comment -

        As Oliver points out, the current MyFaces behavior is definitely spec compliant. Implementation of the clear() and putAll() methods is optional in the java.util.Map contract. However, there is a huge usability benefit in implementing these operations on the relevant maps (i.e. the attributes maps for request, session, and application scope). Therefore, I've encouraged the JSF 1.2 expert group to make the implementation of these methods required in JSF 1.2, and I can report back that the EG has already considered the implementation of clear(), and will now consider the implementation of putAll() as well, to be required for a JSF 1.2 implementation. If both of those come to pass, I would hope that both MyFaces and the java.net RI implementation of JSF 1.1 would both also backport this into their current implementations.

        Show
        Craig McClanahan added a comment - As Oliver points out, the current MyFaces behavior is definitely spec compliant. Implementation of the clear() and putAll() methods is optional in the java.util.Map contract. However, there is a huge usability benefit in implementing these operations on the relevant maps (i.e. the attributes maps for request, session, and application scope). Therefore, I've encouraged the JSF 1.2 expert group to make the implementation of these methods required in JSF 1.2, and I can report back that the EG has already considered the implementation of clear(), and will now consider the implementation of putAll() as well, to be required for a JSF 1.2 implementation. If both of those come to pass, I would hope that both MyFaces and the java.net RI implementation of JSF 1.1 would both also backport this into their current implementations.
        Hide
        Mathias Broekelmann added a comment -

        I don´t agree that we should implement everything like the RI does. The spec clearly says to return a mutable map which implements the entire contract of java.util.Map which inludes putAll and clear. I think we should implement these methods.

        Show
        Mathias Broekelmann added a comment - I don´t agree that we should implement everything like the RI does. The spec clearly says to return a mutable map which implements the entire contract of java.util.Map which inludes putAll and clear. I think we should implement these methods.
        Hide
        Oliver Rossmueller added a comment -

        It's valid behaviour for a Map not to implement putAll() and clear() but to throw UnsupportedOperationException instead. The MyFaces impl follows the JSF RI in this aspect to comply with the spec.

        Show
        Oliver Rossmueller added a comment - It's valid behaviour for a Map not to implement putAll() and clear() but to throw UnsupportedOperationException instead. The MyFaces impl follows the JSF RI in this aspect to comply with the spec.

          People

          • Assignee:
            Mathias Broekelmann
            Reporter:
            Colin Sampaleanu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development