The method argument type changes are an ABI break, but I recall extending MapMessage within a project in order to support more expressive types -- that may have relied on dangerously casting the result of getIndexedReadOnlyStringMap to an IndexedStringMap. Including a safer Object-valued MapMessage subclass (with overloaded put methods) sounds reasonable to me given at least two of us have run into this!
-Carter On Wed, Mar 24, 2021, at 19:29, Remko Popma wrote: > I called it StringMap because the keys must be Strings. Admittedly not a > great name. :-) > > Not sure exactly, but there may be cases where this change could cause an > issue: > putAll(final Map<String, String> map) -> > putAll(final Map<String, Object> map) > > On Thu, Mar 25, 2021 at 2:12 AM Ralph Goers <ralph.go...@dslextreme.com > <mailto:ralph.goers%40dslextreme.com>> > wrote: > > > I looked the other day and wondered the same thing Volkan did. There are > > no unit tests and the contributor didn’t even indicate that he had tried > > it. > > > > I was initially concerned that the underlying Map wouldn’t support it > > since it has StringMap in its name. It turns out the values are objects. > > > > Technically I don’t think this would break compatibility. Any code > > referencing the put(String, String) would automatically map to put(String, > > Object). He didn’t modify the get method which would have broken > > compatibility. > > > > Ralph > > > > > On Mar 24, 2021, at 8:27 AM, Matt Sicker <boa...@gmail.com > > > <mailto:boards%40gmail.com>> wrote: > > > > > > Pretty sure that would break binary compatibility since it removes the > > > String method. I think it might be addable but not removed like that. > > > > > > On Wed, 24 Mar 2021 at 02:39, Volkan Yazıcı <volkan.yaz...@gmail.com > > > <mailto:volkan.yazici%40gmail.com>> > > wrote: > > >> > > >> Hello, > > >> > > >> Adding non-String-typed value support to MapMessage was also something > > on > > >> my radar too. But this PR replacing String with Object in two lines > > seems > > >> too good to be true to me. Does anybody mind taking a second look at > > this, > > >> please? > > >> > > >> Kind regards. > > >> > > >> ---------- Forwarded message --------- > > >> From: Henry Widd <notificati...@github.com > > >> <mailto:notifications%40github.com>> > > >> Date: Tue, Mar 23, 2021 at 4:58 PM > > >> Subject: [apache/logging-log4j2] MapMessage put methods should not > > mandate > > >> String values (#477) > > >> To: apache/logging-log4j2 <logging-log...@noreply.github.com > > >> <mailto:logging-log4j2%40noreply.github.com>> > > >> Cc: Subscribed <subscri...@noreply.github.com > > >> <mailto:subscribed%40noreply.github.com>> > > >> > > >> > > >> the underlying Map is typed <String,Object> so the put methods on > > >> MapMessage can also be. > > >> ------------------------------ > > >> You can view, comment on, or merge this pull request online at: > > >> > > >> https://github.com/apache/logging-log4j2/pull/477 > > >> Commit Summary > > >> > > >> - MapMessage put methods should not mandate String values > > >> > > >> File Changes > > >> > > >> - *M* > > >> > > log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java > > >> < > > https://github.com/apache/logging-log4j2/pull/477/files#diff-f03ffe9ceefd37c87fd118ce591bd8ad288e43b08cd663dde14441f4e7c117ef > > > > > >> (6) > > >> > > >> Patch Links: > > >> > > >> - https://github.com/apache/logging-log4j2/pull/477.patch > > >> - https://github.com/apache/logging-log4j2/pull/477.diff > > >> > > >> — > > >> You are receiving this because you are subscribed to this thread. > > >> Reply to this email directly, view it on GitHub > > >> <https://github.com/apache/logging-log4j2/pull/477>, or unsubscribe > > >> < > > https://github.com/notifications/unsubscribe-auth/AAARTSKGBRHC4NG637EHA4LTFC3BTANCNFSM4ZVO7L2Q > > > > > >> . > > > > > > > > > >
