fixed feedback - errormessages should show only detail.

neo4j · Sep 4, 2017 · 48ed114 · 48ed114
1 parent 376fb39
commit 48ed114
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 35 deletions.
diff --git a/community/bolt/src/main/java/org/neo4j/bolt/logging/BoltMessageLogger.java b/community/bolt/src/main/java/org/neo4j/bolt/logging/BoltMessageLogger.java
@@ -38,7 +38,7 @@ public interface BoltMessageLogger
 
     void serverError( String eventName, String errorMessage );
 
-    void serverError( String eventName, Status status, String errorMessage );
+    void serverError( String eventName, Status status );
 
     void logInit( String userAgent, Map<String,Object> authToken );
 
@@ -54,7 +54,7 @@ public interface BoltMessageLogger
 
     void logSuccess( Object metadata );
 
-    void logFailure( Status status, String errorMessage );
+    void logFailure( Status status );
 
     void logIgnored();
 }
diff --git a/community/bolt/src/main/java/org/neo4j/bolt/logging/BoltMessageLoggerImpl.java b/community/bolt/src/main/java/org/neo4j/bolt/logging/BoltMessageLoggerImpl.java
@@ -41,7 +41,7 @@
 class BoltMessageLoggerImpl implements BoltMessageLogger
 {
     private static final ObjectMapper jsonObjectMapper = new ObjectMapper();
-    public static final Supplier<String> PLACEHOLDER_DETAIL_SUPPLIER = () -> "-";
+    private static final Supplier<String> PLACEHOLDER_DETAIL_SUPPLIER = () -> "-";
     private final BoltMessageLog messageLog;
 
     private final String remoteAddress;
@@ -67,7 +67,7 @@ public void clientEvent( String eventName )
     @Override
     public void clientEvent( String eventName, Supplier<String> detailsSupplier )
     {
-        infoLoggerWithDetails().accept( format( "C: %s", eventName ), detailsSupplier.get() );
+        infoLogger().accept( format( "C: %s", eventName ), detailsSupplier.get() );
     }
 
     @Override
@@ -85,7 +85,7 @@ public void serverEvent( String eventName )
     @Override
     public void serverEvent( String eventName, Supplier<String> detailsSupplier )
     {
-        infoLoggerWithDetails().accept( format( "S: %s", eventName ), detailsSupplier.get() );
+        infoLogger().accept( format( "S: %s", eventName ), detailsSupplier.get() );
     }
 
     @Override
@@ -95,9 +95,9 @@ public void serverError( String eventName, String errorMessage )
     }
 
     @Override
-    public void serverError( String eventName, Status status, String errorMessage )
+    public void serverError( String eventName, Status status )
     {
-        errorLoggerWithArgs( errorMessage ).accept( format( "S: <%s>", eventName ), status.code().serialize() );
+        serverError( eventName, status.code().serialize() );
     }
 
     @Override
@@ -144,9 +144,9 @@ public void logSuccess( Object metadata )
     }
 
     @Override
-    public void logFailure( Status status, String errorMessage )
+    public void logFailure( Status status )
     {
-        serverEvent( "FAILURE", () -> format( "%s %s", status.code().serialize(), errorMessage ) );
+        serverEvent( "FAILURE", () -> status.code().serialize() );
     }
 
     @Override
@@ -164,15 +164,15 @@ private Consumer<String> errorLogger( String errorMessage )
 
         String boltCorrelationId = channel.attr( CORRELATION_ATTRIBUTE_KEY ).get();
         return formatMessageWithEventName ->
-                messageLog.error( remoteAddress, errorMessage, formatMessageWithEventName, boltCorrelationId );
+                messageLog.error( remoteAddress, boltCorrelationId, formatMessageWithEventName, errorMessage );
     }
 
     private String randomCorrelationIdGenerator()
     {
         return UUID.randomUUID().toString();
     }
 
-    private BiConsumer<String, String> infoLoggerWithDetails()
+    private BiConsumer<String, String> infoLogger()
     {
         if ( !channel.hasAttr( CORRELATION_ATTRIBUTE_KEY ) )
         {

diff --git a/community/bolt/src/main/java/org/neo4j/bolt/logging/NullBoltMessageLogger.java b/community/bolt/src/main/java/org/neo4j/bolt/logging/NullBoltMessageLogger.java
@@ -68,7 +68,7 @@ public void serverError( String eventName, String errorMessage )
     }
 
     @Override
-    public void serverError( String eventName, Status status, String errorMessage )
+    public void serverError( String eventName, Status status )
     {
     }
 
@@ -108,7 +108,7 @@ public void logSuccess( Object metadata )
     }
 
     @Override
-    public void logFailure( Status status, String errorMessage )
+    public void logFailure( Status status )
     {
     }
 

diff --git a/community/bolt/src/main/java/org/neo4j/bolt/v1/messaging/BoltResponseMessageWriter.java b/community/bolt/src/main/java/org/neo4j/bolt/v1/messaging/BoltResponseMessageWriter.java
@@ -97,7 +97,7 @@ public void onIgnored() throws IOException
     @Override
     public void onFailure( Status status, String errorMessage ) throws IOException
     {
-        messageLogger.logFailure( status, errorMessage );
+        messageLogger.logFailure( status );
         packer.packStructHeader( 1, FAILURE.signature() );
         packer.packMapHeader( 2 );
 
@@ -113,7 +113,7 @@ public void onFailure( Status status, String errorMessage ) throws IOException
     @Override
     public void onFatal( Status status, String errorMessage ) throws IOException
     {
-        messageLogger.serverError( "FATAL", status, errorMessage );
+        messageLogger.serverError( "FATAL", status);
         onFailure( status, errorMessage );
         flush();
     }

diff --git a/community/bolt/src/test/java/org/neo4j/bolt/logging/BoltMessageLoggerImplTest.java b/community/bolt/src/test/java/org/neo4j/bolt/logging/BoltMessageLoggerImplTest.java
@@ -26,6 +26,9 @@ public class BoltMessageLoggerImplTest
 
     private static final String REMOTE_ADDRESS = "localhost/127.0.0.1:60297";
     private static final String CORRELATION_ID = "Bolt-X-CorrelationId-1234";
+    private static String errorMessage = "Oh my woes!";
+    private static Neo4jError error = Neo4jError.from( new DeadlockDetectedException( errorMessage ) );
+
     @Mock
     private Channel channel;
     @Mock
@@ -149,23 +152,17 @@ public void logIgnored() throws Exception
     @Test
     public void logFailure() throws Exception
     {
-        // given
-        String errorMessage = "Oh my woes!";
-        Neo4jError error = Neo4jError.from( new DeadlockDetectedException( errorMessage ) );
         // when
-        boltMessageLogger.logFailure( error.status(), errorMessage );
+        boltMessageLogger.logFailure( error.status() );
         // then
         verify( boltMessageLog ).info( REMOTE_ADDRESS, CORRELATION_ID,
-                "S: FAILURE Neo.TransientError.Transaction.DeadlockDetected Oh my woes!" );
+                "S: FAILURE Neo.TransientError.Transaction.DeadlockDetected" );
 
     }
 
     @Test
     public void logCorrelationIdClientEvent() throws Exception
     {
-        // given
-        when( channel.hasAttr( CORRELATION_ATTRIBUTE_KEY ) ).thenReturn( true );
-
         // when
         boltMessageLogger.clientEvent( "TEST" );
 
@@ -184,22 +181,28 @@ public void logCorrelationIdClientErrorWithDetails() throws Exception
         boltMessageLogger.clientError( "TEST", "errorMessage", () -> "details" );
 
         // then
-        verify( boltMessageLog ).error( REMOTE_ADDRESS, "errorMessage", "C: <TEST> details",
-                CORRELATION_ID );
+        verify( boltMessageLog ).error( REMOTE_ADDRESS, CORRELATION_ID, "C: <TEST> details", "errorMessage" );
     }
 
     @Test
     public void logCorrelationIdServerError() throws Exception
     {
-        // given
-        when( channel.hasAttr( CORRELATION_ATTRIBUTE_KEY ) ).thenReturn( true );
-
         // when
         boltMessageLogger.serverError( "TEST", "errorMessage" );
 
         // then
-        verify( boltMessageLog ).error( REMOTE_ADDRESS, "errorMessage", "S: <TEST>",
-                CORRELATION_ID );
+        verify( boltMessageLog ).error( REMOTE_ADDRESS, CORRELATION_ID, "S: <TEST>", "errorMessage" );
+    }
+
+    @Test
+    public void logServerErrorWithStatus() throws Exception
+    {
+        // when
+        boltMessageLogger.serverError( "TEST", error.status() );
+
+        // then
+        verify( boltMessageLog ).error( REMOTE_ADDRESS, CORRELATION_ID, "S: <TEST>",
+                "Neo.TransientError.Transaction.DeadlockDetected" );
     }
 
     @Test
@@ -244,7 +247,6 @@ public void createCorrelationIdIfNotAvailableInErrorLogger() throws Exception
 
         // then
         verify( correlationIdAttribute ).set( anyString() );
-        verify( boltMessageLog ).error( REMOTE_ADDRESS, "errorMessage", "S: <TEST>",
-                CORRELATION_ID );
+        verify( boltMessageLog ).error( REMOTE_ADDRESS, CORRELATION_ID, "S: <TEST>", "errorMessage" );
     }
 }
diff --git a/integrationtests/src/test/java/org/neo4j/bolt/BoltMessageLoggingIT.java b/integrationtests/src/test/java/org/neo4j/bolt/BoltMessageLoggingIT.java
@@ -193,9 +193,8 @@ private Config config()
 
     private Driver newDriver()
     {
-        org.neo4j.driver.v1.Config driverConfig = org.neo4j.driver.v1.Config.build().
-                withEncryptionLevel( org.neo4j.driver.v1.Config.EncryptionLevel.NONE )
-                .toConfig();
+        org.neo4j.driver.v1.Config driverConfig = org.neo4j.driver.v1.Config.build()
+                .withoutEncryption().toConfig();
 
         return GraphDatabase.driver( boltUri(), driverConfig );
     }