You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by cakofony <gi...@git.apache.org> on 2017/11/06 18:16:04 UTC
[GitHub] logging-log4j2 pull request #124: LOG4J2-2101: MapMessage correctly handles ...
GitHub user cakofony opened a pull request:
https://github.com/apache/logging-log4j2/pull/124
LOG4J2-2101: MapMessage correctly handles non-string map values
Previously formatting non-string values worked, however filtering,
which uess the "get" method would fail due to MapMessage attempting
to cast values to String.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cakofony/logging-log4j2 LOG4J2-2101
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/logging-log4j2/pull/124.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #124
----
commit 33af2479457613b0e9d1e560a38b4ad19e23d810
Author: Carter Kozak <c4...@gmail.com>
Date: 2017-11-06T18:14:36Z
LOG4J2-2101: MapMessage correctly handles non-string map values
Previously formatting non-string values worked, however filtering,
which uess the "get" method would fail due to MapMessage attempting
to cast values to String.
----
---
[GitHub] logging-log4j2 pull request #124: LOG4J2-2101: MapMessage correctly handles ...
Posted by cakofony <gi...@git.apache.org>.
Github user cakofony commented on a diff in the pull request:
https://github.com/apache/logging-log4j2/pull/124#discussion_r149968256
--- Diff: log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java ---
@@ -200,7 +200,7 @@ public void put(final String key, final String value) {
* @param map The Map to add.
*/
public void putAll(final Map<String, String> map) {
--- End diff --
perhaps this should take `Map<String, V>` instead. I'd prefer to make that change separately though
---
[GitHub] logging-log4j2 issue #124: LOG4J2-2101: MapMessage correctly handles non-str...
Posted by cakofony <gi...@git.apache.org>.
Github user cakofony commented on the issue:
https://github.com/apache/logging-log4j2/pull/124
I would like to update MapMessage to take advantage of `PatternFormatter.recursiveDeepToString` for more efficient value handling, but I think that makes sense in a follow up commit.
I've got several MapMessage changes in review at the moment, so it's difficult to push other fixes without stomping on my own feet!
---
[GitHub] logging-log4j2 pull request #124: LOG4J2-2101: MapMessage correctly handles ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/logging-log4j2/pull/124
---
[GitHub] logging-log4j2 pull request #124: LOG4J2-2101: MapMessage correctly handles ...
Posted by mikaelstaldal <gi...@git.apache.org>.
Github user mikaelstaldal commented on a diff in the pull request:
https://github.com/apache/logging-log4j2/pull/124#discussion_r149195504
--- Diff: log4j-api/src/test/java/org/apache/logging/log4j/message/MapMessageTest.java ---
@@ -99,4 +102,29 @@ public void testMutableByDesign() { // LOG4J2-763
final String expected2 = "{key1=\"value1\", key2=\"value2\", key3=\"value3\"}";
assertEquals(expected2, result2);
}
+
+ @Test
+ public void testGetNonStringValue() {
+ final String key = "Key";
+ final MapMessage<?, Object> msg = new MapMessage<>()
+ .with(key, 1L);
+ assertEquals("1", msg.get(key));
+ }
+
+ @Test
+ public void testRemoveNonStringValue() {
+ final String key = "Key";
+ final MapMessage<?, Object> msg = new MapMessage<>()
+ .with(key, 1L);
+ assertEquals("1", msg.remove(key));
+ }
+
+ @Test
+ public void testJSONFormatNonStringValue() {
--- End diff --
Would be nice to test XML format of non-String value as well.
---
[GitHub] logging-log4j2 pull request #124: LOG4J2-2101: MapMessage correctly handles ...
Posted by cakofony <gi...@git.apache.org>.
Github user cakofony commented on a diff in the pull request:
https://github.com/apache/logging-log4j2/pull/124#discussion_r149243922
--- Diff: log4j-api/src/test/java/org/apache/logging/log4j/message/MapMessageTest.java ---
@@ -99,4 +102,29 @@ public void testMutableByDesign() { // LOG4J2-763
final String expected2 = "{key1=\"value1\", key2=\"value2\", key3=\"value3\"}";
assertEquals(expected2, result2);
}
+
+ @Test
+ public void testGetNonStringValue() {
+ final String key = "Key";
+ final MapMessage<?, Object> msg = new MapMessage<>()
+ .with(key, 1L);
+ assertEquals("1", msg.get(key));
+ }
+
+ @Test
+ public void testRemoveNonStringValue() {
+ final String key = "Key";
+ final MapMessage<?, Object> msg = new MapMessage<>()
+ .with(key, 1L);
+ assertEquals("1", msg.remove(key));
+ }
+
+ @Test
+ public void testJSONFormatNonStringValue() {
--- End diff --
Updated. Good call, turns out it was broken!
---