You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by GitBox <gi...@apache.org> on 2019/01/14 02:43:21 UTC

[logging-log4j2] Diff for: [GitHub] cakofony closed pull request #250: LOG4J2-2530 Generalize check for MapMessage

diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
index 38319d23da..607568e802 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/MapMessage.java
@@ -16,6 +16,7 @@
  */
 package org.apache.logging.log4j.message;
 
+import java.util.AbstractMap;
 import java.util.Collections;
 import java.util.Map;
 import java.util.TreeMap;
@@ -64,8 +65,15 @@
         /** The map should be formatted as JSON. */
         JSON,
         
-        /** The map should be formatted the same as documented by java.util.AbstractMap.toString(). */
-        JAVA;
+        /** The map should be formatted the same as documented by {@link AbstractMap#toString()}. */
+        JAVA,
+
+        /**
+         * The map should be formatted the same as documented by {@link AbstractMap#toString()} but without quotes.
+         *
+         * @since 2.11.2
+         */
+        JAVA_UNQUOTED;
 
         /**
          * Maps a format name to an {@link MapFormat} while ignoring case.
@@ -77,6 +85,7 @@ public static MapFormat lookupIgnoreCase(final String format) {
             return XML.name().equalsIgnoreCase(format) ? XML //
                     : JSON.name().equalsIgnoreCase(format) ? JSON //
                     : JAVA.name().equalsIgnoreCase(format) ? JAVA //
+                    : JAVA_UNQUOTED.name().equalsIgnoreCase(format) ? JAVA_UNQUOTED //
                     : null;
         }
 
@@ -86,7 +95,7 @@ public static MapFormat lookupIgnoreCase(final String format) {
          * @return All {@code MapFormat} names.
          */
         public static String[] names() {
-            return new String[] {XML.name(), JSON.name(), JAVA.name()};
+            return new String[] {XML.name(), JSON.name(), JAVA.name(), JAVA_UNQUOTED.name()};
         }
     }
 
@@ -324,6 +333,9 @@ private StringBuilder format(final MapFormat format, final StringBuilder sb) {
                     asJava(sb);
                     break;
                 }
+                case JAVA_UNQUOTED:
+                    asJavaUnquoted(sb);
+                    break;
                 default : {
                     appendMap(sb);
                 }
@@ -418,16 +430,28 @@ protected void asJson(final StringBuilder sb) {
         sb.append('}');
     }
 
+    protected void asJavaUnquoted(final StringBuilder sb) {
+        asJava(sb, false);
+    }
 
     protected void asJava(final StringBuilder sb) {
+        asJava(sb, true);
+    }
+
+    private void asJava(final StringBuilder sb, boolean quoted) {
         sb.append('{');
         for (int i = 0; i < data.size(); i++) {
             if (i > 0) {
                 sb.append(", ");
             }
-            sb.append(data.getKeyAt(i)).append(Chars.EQ).append(Chars.DQUOTE);
+            sb.append(data.getKeyAt(i)).append(Chars.EQ);
+            if (quoted) {
+                sb.append(Chars.DQUOTE);
+            }
             ParameterFormatter.recursiveDeepToString(data.getValueAt(i), sb, null);
-            sb.append(Chars.DQUOTE);
+            if (quoted) {
+                sb.append(Chars.DQUOTE);
+            }
         }
         sb.append('}');
     }
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MapPatternConverter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MapPatternConverter.java
index 6f018bd2ca..05bbcdfba1 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MapPatternConverter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/MapPatternConverter.java
@@ -18,8 +18,10 @@
 
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.config.plugins.Plugin;
-import org.apache.logging.log4j.message.StringMapMessage;
-import org.apache.logging.log4j.util.IndexedReadOnlyStringMap;
+import org.apache.logging.log4j.message.MapMessage;
+import org.apache.logging.log4j.message.MapMessage.MapFormat;
+
+import java.util.Objects;
 
 /**
  * Able to handle the contents of the LogEvent's MapMessage and either
@@ -30,29 +32,53 @@
 @Plugin(name = "MapPatternConverter", category = PatternConverter.CATEGORY)
 @ConverterKeys({ "K", "map", "MAP" })
 public final class MapPatternConverter extends LogEventPatternConverter {
+
+    private static final String JAVA_UNQUOTED = MapFormat.JAVA_UNQUOTED.name();
+
     /**
      * Name of property to output.
      */
     private final String key;
 
+    /**
+     * Format to use when no key is provided.
+     *
+     * @see MapFormat
+     * @since 2.11.2
+     */
+    private final String[] format;
+
     /**
      * Private constructor.
      *
      * @param options options, may be null.
      */
-    private MapPatternConverter(final String[] options) {
+    private MapPatternConverter(final String[] options, String... format) {
         super(options != null && options.length > 0 ? "MAP{" + options[0] + '}' : "MAP", "map");
         key = options != null && options.length > 0 ? options[0] : null;
+        this.format = format;
     }
 
     /**
-     * Obtains an instance of PropertiesPatternConverter.
+     * Obtains an instance of {@link MapPatternConverter}.
      *
      * @param options options, may be null or first element contains name of property to format.
-     * @return instance of PropertiesPatternConverter.
+     * @return instance of {@link MapPatternConverter}.
      */
     public static MapPatternConverter newInstance(final String[] options) {
-        return new MapPatternConverter(options);
+        return new MapPatternConverter(options, JAVA_UNQUOTED);
+    }
+
+    /**
+     * Obtain an instance of {@link MapPatternConverter}.
+     *
+     * @param options options, may be null or first element contains name of property to format.
+     * @param format the format to use if no options are given (i.e., options is null). Ignored if options is non-null.
+     * @return instance of {@link MapPatternConverter}.
+     * @since 2.11.2
+     */
+    public static MapPatternConverter newInstance(final String[] options, final MapFormat format) {
+        return new MapPatternConverter(options, Objects.toString(format, JAVA_UNQUOTED));
     }
 
     /**
@@ -60,31 +86,19 @@ public static MapPatternConverter newInstance(final String[] options) {
      */
     @Override
     public void format(final LogEvent event, final StringBuilder toAppendTo) {
-        StringMapMessage msg;
-        if (event.getMessage() instanceof StringMapMessage) {
-            msg = (StringMapMessage) event.getMessage();
+        MapMessage msg;
+        if (event.getMessage() instanceof MapMessage) {
+            msg = (MapMessage) event.getMessage();
         } else {
             return;
         }
-        final IndexedReadOnlyStringMap sortedMap = msg.getIndexedReadOnlyStringMap();
         // if there is no additional options, we output every single
         // Key/Value pair for the Map in a similar format to Hashtable.toString()
         if (key == null) {
-            if (sortedMap.isEmpty()) {
-                toAppendTo.append("{}");
-                return;
-            }
-            toAppendTo.append("{");
-            for (int i = 0; i < sortedMap.size(); i++) {
-                if (i > 0) {
-                    toAppendTo.append(", ");
-                }
-                toAppendTo.append(sortedMap.getKeyAt(i)).append('=').append((String)sortedMap.getValueAt(i));
-            }
-            toAppendTo.append('}');
+            msg.formatTo(format, toAppendTo);
         } else {
             // otherwise they just want a single key output
-            final String val = sortedMap.getValue(key);
+            final String val = msg.get(key);
 
             if (val != null) {
                 toAppendTo.append(val);
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/MapPatternConverterTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/MapPatternConverterTest.java
index 811924a093..a841b32b59 100644
--- a/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/MapPatternConverterTest.java
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/pattern/MapPatternConverterTest.java
@@ -19,6 +19,7 @@
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.core.LogEvent;
 import org.apache.logging.log4j.core.impl.Log4jLogEvent;
+import org.apache.logging.log4j.message.MapMessage;
 import org.apache.logging.log4j.message.StringMapMessage;
 import org.junit.Test;
 
@@ -74,4 +75,30 @@ public void testConverterWithKey() {
         final String expected = "Log4j";
         assertEquals(expected, str);
     }
+
+    @Test
+    public void testConverterWithJavaFormat() {
+
+        final StringMapMessage msg = new StringMapMessage();
+        msg.put("subject", "I");
+        msg.put("verb", "love");
+        msg.put("object", "Log4j");
+        final MapPatternConverter converter = MapPatternConverter.newInstance(null, MapMessage.MapFormat.JAVA);
+        final LogEvent event = Log4jLogEvent.newBuilder() //
+                .setLoggerName("MyLogger") //
+                .setLevel(Level.DEBUG) //
+                .setMessage(msg) //
+                .build();
+        final StringBuilder sb = new StringBuilder();
+        converter.format(event, sb);
+        final String str = sb.toString();
+        String expected = "subject=\"I\"";
+        assertTrue("Missing or incorrect subject. Expected " + expected + ", actual " + str, str.contains(expected));
+        expected = "verb=\"love\"";
+        assertTrue("Missing or incorrect verb", str.contains(expected));
+        expected = "object=\"Log4j\"";
+        assertTrue("Missing or incorrect object", str.contains(expected));
+
+        assertEquals("{object=\"Log4j\", subject=\"I\", verb=\"love\"}", str);
+    }
 }
diff --git a/log4j-samples/log4j-samples-loggerProperties/src/main/java/org/apache/logging/log4j/lookup/MapMessageLookup.java b/log4j-samples/log4j-samples-loggerProperties/src/main/java/org/apache/logging/log4j/lookup/MapMessageLookup.java
index 11843822e1..5c444cb4b2 100644
--- a/log4j-samples/log4j-samples-loggerProperties/src/main/java/org/apache/logging/log4j/lookup/MapMessageLookup.java
+++ b/log4j-samples/log4j-samples-loggerProperties/src/main/java/org/apache/logging/log4j/lookup/MapMessageLookup.java
@@ -23,6 +23,7 @@
 import org.apache.logging.log4j.core.config.plugins.Plugin;
 import org.apache.logging.log4j.core.lookup.AbstractLookup;
 import org.apache.logging.log4j.core.lookup.StrLookup;
+import org.apache.logging.log4j.message.MapMessage;
 import org.apache.logging.log4j.message.StringMapMessage;
 import org.apache.logging.log4j.message.Message;
 import org.apache.logging.log4j.status.StatusLogger;
@@ -52,26 +53,13 @@
     @Override
     public String lookup(final LogEvent event, final String key) {
         final Message msg = event.getMessage();
-        if (msg instanceof StringMapMessage) {
+        if (msg instanceof MapMessage) {
             try {
-                final Map<String, String> properties = ((StringMapMessage) msg).getData();
-                if (properties == null) {
-                    return "";
-                }
+                MapMessage<?, ?> mapMessage = (MapMessage) msg;
                 if (key == null || key.length() == 0 || key.equals("*")) {
-                    final StringBuilder sb = new StringBuilder("{");
-                    boolean first = true;
-                    for (final Map.Entry<String, String> entry : properties.entrySet()) {
-                        if (!first) {
-                            sb.append(", ");
-                        }
-                        sb.append(entry.getKey()).append("=").append(entry.getValue());
-                        first = false;
-                    }
-                    sb.append("}");
-                    return sb.toString();
+                    return mapMessage.asString(MapMessage.MapFormat.JAVA_UNQUOTED.name());
                 }
-                return properties.get(key);
+                return mapMessage.get(key);
             } catch (final Exception ex) {
                 LOGGER.warn(LOOKUP, "Error while getting property [{}].", key, ex);
                 return null;
diff --git a/log4j-samples/log4j-samples-loggerProperties/src/test/java/org/apache/logging/log4j/MapMessageLookupTest.java b/log4j-samples/log4j-samples-loggerProperties/src/test/java/org/apache/logging/log4j/MapMessageLookupTest.java
new file mode 100644
index 0000000000..d2f55ccb4e
--- /dev/null
+++ b/log4j-samples/log4j-samples-loggerProperties/src/test/java/org/apache/logging/log4j/MapMessageLookupTest.java
@@ -0,0 +1,89 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache license, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the license for the specific language governing permissions and
+ * limitations under the license.
+ */
+
+package org.apache.logging.log4j;
+
+import static org.junit.Assert.assertEquals;
+
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.impl.Log4jLogEvent;
+import org.apache.logging.log4j.lookup.MapMessageLookup;
+import org.apache.logging.log4j.message.MapMessage;
+import org.apache.logging.log4j.message.StringMapMessage;
+import org.apache.logging.log4j.message.StructuredDataMessage;
+import org.junit.Test;
+
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Tests {@link MapMessageLookup}
+ */
+public class MapMessageLookupTest
+{
+    @Test
+    public void testStructuredDataMessageLookup() {
+        // GIVEN: A StructuredDataMessage object
+        final StructuredDataMessage message = new StructuredDataMessage("id", "msg", "type");
+
+        message.put("A", "a");
+        message.put("B", "b");
+        message.put("C", "c");
+
+        // AND: An event with that message
+        final LogEvent event = Log4jLogEvent.newBuilder().setLevel(Level.DEBUG).setMessage(message).build();
+
+        // AND: A MapMessageLookup object
+        final MapMessageLookup lookup = new MapMessageLookup();
+
+        // WHEN: Lookup is performed
+        final String a = lookup.lookup(event, "A");
+        final String b = lookup.lookup(event, "B");
+        final String c = lookup.lookup(event, "C");
+
+        // THEN: The looked up values are correct
+        assertEquals("a", a);
+        assertEquals("b", b);
+        assertEquals("c", c);
+    }
+
+    @Test
+    public void testStringMapMessageLookup() {
+        // GIVEN: A StringMapMessage object
+        final Map<String, String> values = new HashMap<>(3);
+        values.put("A", "a");
+        values.put("B", "b");
+        values.put("C", "c");
+        final MapMessage message = new StringMapMessage(values);
+
+        // AND: An event with that message
+        final LogEvent event = Log4jLogEvent.newBuilder().setLevel(Level.DEBUG).setMessage(message).build();
+
+        // AND: A MapMessageLookup object
+        final MapMessageLookup lookup = new MapMessageLookup();
+
+        // WHEN: Lookup is performed
+        final String a = lookup.lookup(event, "A");
+        final String b = lookup.lookup(event, "B");
+        final String c = lookup.lookup(event, "C");
+
+        // THEN: The looked up values are correct
+        assertEquals("a", a);
+        assertEquals("b", b);
+        assertEquals("c", c);
+    }
+}


With regards,
Apache Git Services