From 4c17382a4345eb88a3aed4135e234ba6b4e5cebd Mon Sep 17 00:00:00 2001
From: zpencer <spencerfang@google.com>
Date: Thu, 14 Dec 2017 17:27:52 -0800
Subject: [PATCH] services: let BinaryLog factory return null (#3868)

---
 .../main/java/io/grpc/services/BinaryLog.java | 23 ++++++++++---------
 .../java/io/grpc/services/BinaryLogTest.java  | 11 ++++-----
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/services/src/main/java/io/grpc/services/BinaryLog.java b/services/src/main/java/io/grpc/services/BinaryLog.java
index ef1c61095f..07b9dc7021 100644
--- a/services/src/main/java/io/grpc/services/BinaryLog.java
+++ b/services/src/main/java/io/grpc/services/BinaryLog.java
@@ -34,8 +34,6 @@ import javax.annotation.Nullable;
  */
 final class BinaryLog {
   private static final Logger logger = Logger.getLogger(BinaryLog.class.getName());
-  private static final BinaryLog NOOP_LOG =
-      new BinaryLog(/*maxHeaderBytes=*/ 0, /*maxMessageBytes=*/ 0);
   private final int maxHeaderBytes;
   private final int maxMessageBytes;
 
@@ -68,15 +66,10 @@ final class BinaryLog {
   }
 
   private static final Factory DEFAULT_FACTORY;
-  private static final Factory NOOP_FACTORY = new Factory() {
-    @Override
-    public BinaryLog getLog(String fullMethodName) {
-      return NOOP_LOG;
-    }
-  };
+  private static final Factory NULL_FACTORY = new NullFactory();
 
   static {
-    Factory defaultFactory = NOOP_FACTORY;
+    Factory defaultFactory = NULL_FACTORY;
     try {
       String configStr = System.getenv("GRPC_BINARY_LOG_CONFIG");
       if (configStr != null && configStr.length() > 0) {
@@ -84,7 +77,7 @@ final class BinaryLog {
       }
     } catch (Throwable t) {
       logger.log(Level.SEVERE, "Failed to initialize binary log. Disabling binary log.", t);
-      defaultFactory = NOOP_FACTORY;
+      defaultFactory = NULL_FACTORY;
     }
     DEFAULT_FACTORY = defaultFactory;
   }
@@ -98,6 +91,7 @@ final class BinaryLog {
   }
 
   interface Factory {
+    @Nullable
     BinaryLog getLog(String fullMethodName);
   }
 
@@ -167,7 +161,7 @@ final class BinaryLog {
           logger.info(String.format("Method binlog: method=%s log=%s", methodOrSvc, binLog));
         }
       }
-      this.globalLog = globalLog == null ? NOOP_LOG : globalLog;
+      this.globalLog = globalLog;
       this.perServiceLogs = Collections.unmodifiableMap(perServiceLogs);
       this.perMethodLogs = Collections.unmodifiableMap(perMethodLogs);
     }
@@ -243,4 +237,11 @@ final class BinaryLog {
       return input.endsWith("/*");
     }
   }
+
+  private static final class NullFactory implements Factory {
+    @Override
+    public BinaryLog getLog(String fullMethodName) {
+      return null;
+    }
+  }
 }
diff --git a/services/src/test/java/io/grpc/services/BinaryLogTest.java b/services/src/test/java/io/grpc/services/BinaryLogTest.java
index 6be9cf6b3b..92eb37b4a6 100644
--- a/services/src/test/java/io/grpc/services/BinaryLogTest.java
+++ b/services/src/test/java/io/grpc/services/BinaryLogTest.java
@@ -27,7 +27,6 @@ import org.junit.runners.JUnit4;
 /** Tests for {@link BinaryLog}. */
 @RunWith(JUnit4.class)
 public final class BinaryLogTest {
-  private static final BinaryLog NONE = new Builder().build();
   private static final BinaryLog HEADER_FULL = new Builder().header(Integer.MAX_VALUE).build();
   private static final BinaryLog HEADER_256 = new Builder().header(256).build();
   private static final BinaryLog MSG_FULL = new Builder().msg(Integer.MAX_VALUE).build();
@@ -73,7 +72,7 @@ public final class BinaryLogTest {
 
   @Test
   public void configBinLog_method_absent() throws Exception {
-    assertEquals(NONE, new FactoryImpl("p.s/m").getLog("p.s/absent"));
+    assertNull(new FactoryImpl("p.s/m").getLog("p.s/absent"));
   }
 
   @Test
@@ -95,7 +94,7 @@ public final class BinaryLogTest {
 
   @Test
   public void configBinLog_service_absent() throws Exception {
-    assertEquals(NONE, new FactoryImpl("p.s/*").getLog("p.other/m"));
+    assertNull(new FactoryImpl("p.s/*").getLog("p.other/m"));
   }
 
   @Test
@@ -157,7 +156,7 @@ public final class BinaryLogTest {
         "package.both256/*{h:256;m:256},"
         + "package.service1/both128{h:128;m:128},"
         + "package.service2/method_messageOnly{m}");
-    assertEquals(NONE, factory.getLog("otherpackage.service/method"));
+    assertNull(factory.getLog("otherpackage.service/method"));
 
     assertEquals(BOTH_256, factory.getLog("package.both256/method1"));
     assertEquals(BOTH_256, factory.getLog("package.both256/method2"));
@@ -166,11 +165,11 @@ public final class BinaryLogTest {
     assertEquals(
         new Builder().header(128).msg(128).build(), factory.getLog("package.service1/both128"));
     // no global config in effect
-    assertEquals(NONE, factory.getLog("package.service1/absent"));
+    assertNull(factory.getLog("package.service1/absent"));
 
     assertEquals(MSG_FULL, factory.getLog("package.service2/method_messageOnly"));
     // no global config in effect
-    assertEquals(NONE, factory.getLog("package.service2/absent"));
+    assertNull(factory.getLog("package.service2/absent"));
   }
 
   @Test