add tests. feedback. minor cleanups.

Author: zhumin8

oauth2_http/java/com/google/auth/oauth2/LoggingConfigs.java

@@ -1,3 +1,34 @@
+/*
+ * Copyright 2024 Google LLC
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google LLC nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
 package com.google.auth.oauth2;
 
 import org.slf4j.ILoggerFactory;
@@ -24,12 +55,11 @@ static Logger getLogger(Class<?> clazz) {
 
   // constructor with LoggerFactoryProvider to make testing easier
   static Logger getLogger(Class<?> clazz, LoggerFactoryProvider factoryProvider) {
-    if (loggingEnabled) {
-      return factoryProvider.getLoggerFactory().getLogger(clazz.getName());
-    } else {
+    if (!loggingEnabled) {
       //  use SLF4j's NOP logger regardless of bindings
       return NO_OP_LOGGER;
     }
+    return factoryProvider.getLoggerFactory().getLogger(clazz.getName());
   }
 
   static boolean isLoggingEnabled() {

oauth2_http/java/com/google/auth/oauth2/LoggingUtils.java

@@ -1,3 +1,34 @@
+/*
+ * Copyright 2024 Google LLC
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google LLC nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
 package com.google.auth.oauth2;
 
 import com.google.api.client.http.HttpRequest;
@@ -43,8 +74,8 @@ static void logWithMDC(
   }
 
   static void logRequest(HttpRequest request, Logger logger, String message) {
-    if (logger.isInfoEnabled()) {
-      try {
+    try {
+      if (logger.isInfoEnabled()) {
         Map<String, String> loggingDataMap = new HashMap<>();
         loggingDataMap.put("request.method", request.getRequestMethod());
         loggingDataMap.put("request.url", request.getUrl().toString());
@@ -75,39 +106,37 @@ static void logRequest(HttpRequest request, Logger logger, String message) {
 
           logWithMDC(logger, org.slf4j.event.Level.INFO, loggingDataMap, message);
         }
-      } catch (Exception e) {
-        logger.error("Error logging request: ", e);
       }
+    } catch (Exception e) {
+      logger.error("Error logging request: ", e);
     }
   }
 
   static void logResponse(HttpResponse response, Logger logger, String message) {
-    if (logger.isInfoEnabled()) {
-      try {
+    try {
+      if (logger.isInfoEnabled()) {
         Map<String, String> responseLogDataMap = new HashMap<>();
         responseLogDataMap.put("response.status", String.valueOf(response.getStatusCode()));
         responseLogDataMap.put("response.status.message", response.getStatusMessage());
 
-        Map<String, Object> headers = new HashMap<>();
-        response.getHeaders().forEach((key, val) -> headers.put(key, val));
+        Map<String, Object> headers = new HashMap<>(response.getHeaders());
         responseLogDataMap.put("response.headers", headers.toString());
         logWithMDC(logger, org.slf4j.event.Level.INFO, responseLogDataMap, message);
-      } catch (Exception e) {
-
-        logger.error("Error logging response: ", e);
       }
+    } catch (Exception e) {
+
+      logger.error("Error logging response: ", e);
     }
   }
 
   static void logGenericData(GenericData genericData, Logger logger, String message) {
-    if (logger.isDebugEnabled()) {
-      try {
-
+    try {
+      if (logger.isDebugEnabled()) {
         Map<String, String> contextMap = parseGenericData(genericData);
         logWithMDC(logger, org.slf4j.event.Level.DEBUG, contextMap, message);
-      } catch (Exception e) {
-        logger.error("Error logging GenericData: ", e);
       }
+    } catch (Exception e) {
+      logger.error("Error logging GenericData: ", e);
     }
   }
 

oauth2_http/javatests/com/google/auth/TestAppender.java

@@ -1,3 +1,34 @@
+/*
+ * Copyright 2024 Google LLC
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google LLC nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
 package com.google.auth;
 
 import ch.qos.logback.classic.spi.ILoggingEvent;
@@ -11,6 +42,8 @@ public class TestAppender extends AppenderBase<ILoggingEvent> {
 
   @Override
   protected void append(ILoggingEvent eventObject) {
+    // triggering Logback to capture the current MDC context and store it with the log event
+    eventObject.getMDCPropertyMap();
     events.add(eventObject);
   }
 

oauth2_http/javatests/com/google/auth/oauth2/LoggingConfigsTest.java

@@ -1,21 +1,47 @@
+/*
+ * Copyright 2024 Google LLC
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google LLC nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
 package com.google.auth.oauth2;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import ch.qos.logback.classic.LoggerContext;
-import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
-import ch.qos.logback.classic.spi.ILoggingEvent;
-import ch.qos.logback.core.ConsoleAppender;
-import com.google.auth.TestAppender;
 import com.google.auth.oauth2.LoggingConfigs.LoggerFactoryProvider;
-import java.util.HashMap;
-import java.util.Map;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -33,26 +59,6 @@ public class LoggingConfigsTest {
   @Before
   public void setup() {
     testEnvironmentProvider = new TestEnvironmentProvider();
-    // need to setup a ConsoleAppender and attach to root logger because TestAppender
-    // does not correctly capture MDC info
-    LoggerContext loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory();
-
-    PatternLayoutEncoder patternLayoutEncoder = new PatternLayoutEncoder();
-    patternLayoutEncoder.setPattern("%-4relative [%thread] %-5level %logger{35} - %msg%n");
-    patternLayoutEncoder.setContext(loggerContext);
-
-    patternLayoutEncoder.start();
-
-    ConsoleAppender<ILoggingEvent> consoleAppender = new ConsoleAppender<>();
-    consoleAppender.setEncoder(patternLayoutEncoder);
-
-    consoleAppender.setContext(loggerContext);
-    consoleAppender.setName("CONSOLE");
-
-    consoleAppender.start();
-
-    ch.qos.logback.classic.Logger rootLogger = loggerContext.getLogger(Logger.ROOT_LOGGER_NAME);
-    rootLogger.addAppender(consoleAppender);
   }
 
   @After
@@ -66,8 +72,8 @@ public void testGetLogger_loggingEnabled_slf4jBindingPresent() {
     testEnvironmentProvider.setEnv("GOOGLE_SDK_JAVA_LOGGING", "true");
     LoggingConfigs.setEnvironmentProvider(testEnvironmentProvider);
     Logger logger = LoggingConfigs.getLogger(LoggingConfigsTest.class);
-    assertTrue(logger instanceof org.slf4j.Logger);
-    assertNotEquals(logger.getClass(), NOPLogger.class);
+    assertNotNull(logger);
+    assertNotEquals(NOPLogger.class, logger.getClass());
   }
 
   @Test
@@ -115,19 +121,4 @@ public void testIsLoggingEnabled_defaultToFalse() {
     LoggingConfigs.setEnvironmentProvider(testEnvironmentProvider);
     assertFalse(LoggingConfigs.isLoggingEnabled());
   }
-
-  @Test
-  public void testLogWithMDC_slf4jLogger() {
-    TestAppender.clearEvents();
-    Map<String, String> contextMap = new HashMap<>();
-    contextMap.put("key", "value");
-    LoggingUtils.logWithMDC(LOGGER, org.slf4j.event.Level.DEBUG, contextMap, "test message");
-
-    assertEquals(1, TestAppender.events.size());
-    assertEquals("test message", TestAppender.events.get(0).getFormattedMessage());
-
-    // Verify MDC content
-    ILoggingEvent event = TestAppender.events.get(0);
-    assertEquals("value", event.getMDCPropertyMap().get("key"));
-  }
 }

oauth2_http/javatests/com/google/auth/oauth2/LoggingUtilsTest.java

@@ -0,0 +1,85 @@
+/*
+ * Copyright 2024 Google LLC
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google LLC nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package com.google.auth.oauth2;
+
+import static org.junit.Assert.*;
+
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import com.google.api.client.util.GenericData;
+import com.google.auth.TestAppender;
+import java.util.HashMap;
+import java.util.Map;
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class LoggingUtilsTest {
+  private static final Logger LOGGER = LoggerFactory.getLogger(LoggingUtilsTest.class);
+
+  @Test
+  public void testLogWithMDC_slf4jLogger() {
+    TestAppender.clearEvents();
+    Map<String, String> contextMap = new HashMap<>();
+    contextMap.put("key1", "value1");
+    contextMap.put("key2", "value2");
+    LoggingUtils.logWithMDC(LOGGER, org.slf4j.event.Level.DEBUG, contextMap, "test message");
+
+    assertEquals(1, TestAppender.events.size());
+    assertEquals("test message", TestAppender.events.get(0).getFormattedMessage());
+
+    // Verify MDC content
+    ILoggingEvent event = TestAppender.events.get(0);
+    assertEquals(2, event.getMDCPropertyMap().size());
+    assertEquals("value1", event.getMDCPropertyMap().get("key1"));
+    assertEquals("value2", event.getMDCPropertyMap().get("key2"));
+  }
+
+  @Test
+  public void testLogGenericData() {
+    TestAppender.clearEvents();
+    GenericData genericData = Mockito.mock(GenericData.class);
+
+    GenericData data = new GenericData();
+    data.put("key1", "value1");
+    data.put("token", "value2");
+
+    LoggingUtils.logGenericData(data, LOGGER, "test generic data");
+
+    assertEquals(1, TestAppender.events.size());
+    Map<String, String> mdcPropertyMap = TestAppender.events.get(0).getMDCPropertyMap();
+    assertEquals(2, mdcPropertyMap.size());
+    assertEquals("value1", mdcPropertyMap.get("key1"));
+    assertNotNull(mdcPropertyMap.get("token"));
+    assertNotEquals("value2", mdcPropertyMap.get("token"));
+  }
+}