Skip to content

Commit

Permalink
Fix nullable annotation/tests in logging
Browse files Browse the repository at this point in the history
  • Loading branch information
spacecowboy committed Jan 5, 2017
1 parent 86b617c commit 552d7fc
Show file tree
Hide file tree
Showing 7 changed files with 81 additions and 52 deletions.
Expand Up @@ -30,6 +30,8 @@
import java.util.Arrays;
import java.util.concurrent.TimeUnit;

import javax.annotation.Nonnull;

import org.neo4j.logging.AssertableLogProvider;
import org.neo4j.time.Clocks;
import org.neo4j.time.FakeClock;
Expand All @@ -45,7 +47,8 @@ public class CappedLoggerTest

public interface LogMethod
{
void log( CappedLogger logger, String msg, Throwable cause );
void log( @Nonnull CappedLogger logger, @Nonnull String msg );
void log( @Nonnull CappedLogger logger, @Nonnull String msg, @Nonnull Throwable cause );
}

@Parameterized.Parameters(name = "{0}")
Expand All @@ -54,31 +57,55 @@ public static Iterable<Object[]> parameters()
LogMethod debug = new LogMethod()
{
@Override
public void log( CappedLogger logger, String msg, Throwable cause )
public void log( @Nonnull CappedLogger logger, @Nonnull String msg )
{
logger.debug( msg );
}

@Override
public void log( @Nonnull CappedLogger logger, @Nonnull String msg, @Nonnull Throwable cause )
{
logger.debug( msg, cause );
}
};
LogMethod info = new LogMethod()
{
@Override
public void log( CappedLogger logger, String msg, Throwable cause )
public void log( @Nonnull CappedLogger logger, @Nonnull String msg )
{
logger.debug( msg );
}

@Override
public void log( @Nonnull CappedLogger logger, @Nonnull String msg, @Nonnull Throwable cause )
{
logger.info( msg, cause );
}
};
LogMethod warn = new LogMethod()
{
@Override
public void log( CappedLogger logger, String msg, Throwable cause )
public void log( @Nonnull CappedLogger logger, @Nonnull String msg )
{
logger.debug( msg );
}

@Override
public void log( @Nonnull CappedLogger logger, @Nonnull String msg, @Nonnull Throwable cause )
{
logger.warn( msg, cause );
}
};
LogMethod error = new LogMethod()
{
@Override
public void log( CappedLogger logger, String msg, Throwable cause )
public void log( @Nonnull CappedLogger logger, @Nonnull String msg )
{
logger.debug( msg );
}

@Override
public void log( @Nonnull CappedLogger logger, @Nonnull String msg, @Nonnull Throwable cause )
{
logger.error( msg, cause );
}
Expand Down Expand Up @@ -143,7 +170,7 @@ public String[] logLines( int lineCount, int startAt )
{
String msg = String.format( "### %04d ###", startAt + i );
lines[i] = msg;
logMethod.log( logger, msg, null );
logMethod.log( logger, msg );
}
return lines;
}
Expand Down Expand Up @@ -290,10 +317,10 @@ public void mustNotLogMessagesWithinConfiguredTimeLimit() throws Exception
{
FakeClock clock = getDefaultFakeClock();
logger.setTimeLimit( 1, TimeUnit.MILLISECONDS, clock );
logMethod.log( logger, "### AAA ###", null );
logMethod.log( logger, "### BBB ###", null );
logMethod.log( logger, "### AAA ###" );
logMethod.log( logger, "### BBB ###" );
clock.forward( 1, TimeUnit.MILLISECONDS );
logMethod.log( logger, "### CCC ###", null );
logMethod.log( logger, "### CCC ###" );

logProvider.assertContainsMessageMatching( containsString( "### AAA ###" ) );
logProvider.assertNone( currentLog( inLog( CappedLogger.class ), containsString( "### BBB ###" ) ) );
Expand All @@ -305,13 +332,13 @@ public void unsettingTimeLimitMustLetMessagesThrough() throws Exception
{
FakeClock clock = getDefaultFakeClock();
logger.setTimeLimit( 1, TimeUnit.MILLISECONDS, clock );
logMethod.log( logger, "### AAA ###", null );
logMethod.log( logger, "### BBB ###", null );
logMethod.log( logger, "### AAA ###" );
logMethod.log( logger, "### BBB ###" );
clock.forward( 1, TimeUnit.MILLISECONDS );
logMethod.log( logger, "### CCC ###", null );
logMethod.log( logger, "### DDD ###", null );
logMethod.log( logger, "### CCC ###" );
logMethod.log( logger, "### DDD ###" );
logger.unsetTimeLimit(); // Note that we are not advancing the clock!
logMethod.log( logger, "### EEE ###", null );
logMethod.log( logger, "### EEE ###" );

logProvider.assertContainsMessageMatching( containsString( "### AAA ###" ) );
logProvider.assertNone( currentLog( inLog( CappedLogger.class ), containsString( "### BBB ###" ) ) );
Expand All @@ -325,10 +352,10 @@ public void mustLogAfterResetWithTimeLimit() throws Exception
{
FakeClock clock = getDefaultFakeClock();
logger.setTimeLimit( 1, TimeUnit.MILLISECONDS, clock );
logMethod.log( logger, "### AAA ###", null );
logMethod.log( logger, "### BBB ###", null );
logMethod.log( logger, "### AAA ###" );
logMethod.log( logger, "### BBB ###" );
logger.reset();
logMethod.log( logger, "### CCC ###", null );
logMethod.log( logger, "### CCC ###" );

logProvider.assertContainsMessageMatching( containsString( "### AAA ###" ) );
logProvider.assertNone( currentLog( inLog( CappedLogger.class ), containsString( "### BBB ###" ) ) );
Expand All @@ -341,12 +368,12 @@ public void mustOnlyLogMessagesThatPassBothLimits() throws Exception
FakeClock clock = getDefaultFakeClock();
logger.setCountLimit( 2 );
logger.setTimeLimit( 1, TimeUnit.MILLISECONDS, clock );
logMethod.log( logger, "### AAA ###", null );
logMethod.log( logger, "### BBB ###", null ); // Filtered by the time limit
logMethod.log( logger, "### AAA ###" );
logMethod.log( logger, "### BBB ###" ); // Filtered by the time limit
clock.forward( 1, TimeUnit.MILLISECONDS );
logMethod.log( logger, "### CCC ###", null ); // Filtered by the count limit
logMethod.log( logger, "### CCC ###" ); // Filtered by the count limit
logger.reset();
logMethod.log( logger, "### DDD ###", null );
logMethod.log( logger, "### DDD ###" );

logProvider.assertContainsMessageMatching( containsString( "### AAA ###" ) );
logProvider.assertNone( currentLog( inLog( CappedLogger.class ), containsString( "### BBB ###" ) ) );
Expand All @@ -358,9 +385,9 @@ public void mustOnlyLogMessagesThatPassBothLimits() throws Exception
public void mustFilterDuplicateMessageAndNullException() throws Exception
{
logger.setDuplicateFilterEnabled( true );
logMethod.log( logger, "### AAA ###", null );
logMethod.log( logger, "### AAA ###", null ); // duplicate
logMethod.log( logger, "### BBB ###", null );
logMethod.log( logger, "### AAA ###" );
logMethod.log( logger, "### AAA ###" ); // duplicate
logMethod.log( logger, "### BBB ###" );
String[] lines = new String[]{"### AAA ###", "### BBB ###"};
assertLoggedLines( lines, lines.length );
}
Expand Down Expand Up @@ -393,7 +420,7 @@ public void mustLogSameMessageAndDifferentExceptionWithDuplicateLimit() throws E
public void mustLogSameMessageAndNonNullExceptionWithDuplicateLimit() throws Exception
{
logger.setDuplicateFilterEnabled( true );
logMethod.log( logger, "### AAA ###", null );
logMethod.log( logger, "### AAA ###" );
logMethod.log( logger, "### AAA ###", new ExceptionWithoutStackTrace( null ) ); // Different message
logMethod.log( logger, "### AAA ###", new ExceptionWithoutStackTrace2( null ) ); // Different type

Expand All @@ -407,7 +434,7 @@ public void mustFilterSameMessageAndExceptionWithNullMessage() throws Exception
logger.setDuplicateFilterEnabled( true );
logMethod.log( logger, "### AAA ###", new ExceptionWithoutStackTrace( null ) );
logMethod.log( logger, "### AAA ###", new ExceptionWithoutStackTrace( null ) );
logMethod.log( logger, "### BBB ###", null );
logMethod.log( logger, "### BBB ###" );

String[] messages = new String[]{"### AAA ###", "### BBB ###"};
assertLoggedLines( messages, messages.length );
Expand Down
Expand Up @@ -20,6 +20,7 @@
package org.neo4j.logging;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* An abstract implementation of {@link Log}, providing implementations
Expand All @@ -43,7 +44,7 @@ public void debug( @Nonnull String message, @Nonnull Throwable throwable )
}

@Override
public void debug( @Nonnull String format, @Nonnull Object... arguments )
public void debug( @Nonnull String format, @Nullable Object... arguments )
{
debugLogger().log( format, arguments );
}
Expand All @@ -61,7 +62,7 @@ public void info( @Nonnull String message, @Nonnull Throwable throwable )
}

@Override
public void info( @Nonnull String format, @Nonnull Object... arguments )
public void info( @Nonnull String format, @Nullable Object... arguments )
{
infoLogger().log( format, arguments );
}
Expand All @@ -79,7 +80,7 @@ public void warn( @Nonnull String message, @Nonnull Throwable throwable )
}

@Override
public void warn( @Nonnull String format, @Nonnull Object... arguments )
public void warn( @Nonnull String format, @Nullable Object... arguments )
{
warnLogger().log( format, arguments );
}
Expand All @@ -97,7 +98,7 @@ public void error( @Nonnull String message, @Nonnull Throwable throwable )
}

@Override
public void error( @Nonnull String format, @Nonnull Object... arguments )
public void error( @Nonnull String format, @Nullable Object... arguments )
{
errorLogger().log( format, arguments );
}
Expand Down
Expand Up @@ -23,6 +23,7 @@
import java.util.function.Consumer;
import java.util.function.Supplier;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;

import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -80,7 +81,7 @@ public void log( @Nonnull String message, @Nonnull Throwable throwable )
}

@Override
public void log( @Nonnull String format, @Nonnull Object... arguments )
public void log( @Nonnull String format, @Nullable Object... arguments )
{
requireNonNull( format, "format must not be null" );
if ( arguments == null || arguments.length == 0 )
Expand Down
9 changes: 5 additions & 4 deletions community/logging/src/main/java/org/neo4j/logging/Log.java
Expand Up @@ -21,6 +21,7 @@

import java.util.function.Consumer;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* A log into which various levels of messages can be written
Expand Down Expand Up @@ -59,7 +60,7 @@ public interface Log
* @param format A string format for writing a message
* @param arguments Arguments to substitute into the message according to the format
*/
void debug( @Nonnull String format, @Nonnull Object... arguments );
void debug( @Nonnull String format, @Nullable Object... arguments );

/**
* @return a {@link Logger} instance for writing info messages
Expand Down Expand Up @@ -88,7 +89,7 @@ public interface Log
* @param format A string format for writing a message
* @param arguments Arguments to substitute into the message according to the format
*/
void info( @Nonnull String format, @Nonnull Object... arguments );
void info( @Nonnull String format, @Nullable Object... arguments );

/**
* @return a {@link Logger} instance for writing warn messages
Expand Down Expand Up @@ -117,7 +118,7 @@ public interface Log
* @param format A string format for writing a message
* @param arguments Arguments to substitute into the message according to the format
*/
void warn( @Nonnull String format, @Nonnull Object... arguments );
void warn( @Nonnull String format, @Nullable Object... arguments );

/**
* @return a {@link Logger} instance for writing error messages
Expand Down Expand Up @@ -146,7 +147,7 @@ public interface Log
* @param format A string format for writing a message
* @param arguments Arguments to substitute into the message according to the {@code format}
*/
void error( @Nonnull String format, @Nonnull Object... arguments );
void error( @Nonnull String format, @Nullable Object... arguments );

/**
* Used to temporarily log several messages in bulk. The implementation may choose to
Expand Down
Expand Up @@ -21,6 +21,7 @@

import java.util.function.Consumer;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* A log into which messages can be written
Expand All @@ -42,7 +43,7 @@ public interface Logger
* @param format A string format for writing a message
* @param arguments Arguments to substitute into the message according to the {@code format}
*/
void log( @Nonnull String format, @Nonnull Object... arguments );
void log( @Nonnull String format, @Nullable Object... arguments );

/**
* Used to temporarily write several messages in bulk. The implementation may choose to
Expand Down
Expand Up @@ -21,6 +21,7 @@

import java.util.function.Consumer;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;

/**
* A {@link Log} implementation that discards all messages
Expand Down Expand Up @@ -65,7 +66,7 @@ public void debug( @Nonnull String message, @Nonnull Throwable throwable )
}

@Override
public void debug( @Nonnull String format, @Nonnull Object... arguments )
public void debug( @Nonnull String format, @Nullable Object... arguments )
{
}

Expand All @@ -87,7 +88,7 @@ public void info( @Nonnull String message, @Nonnull Throwable throwable )
}

@Override
public void info( @Nonnull String format, @Nonnull Object... arguments )
public void info( @Nonnull String format, @Nullable Object... arguments )
{
}

Expand All @@ -109,7 +110,7 @@ public void warn( @Nonnull String message, @Nonnull Throwable throwable )
}

@Override
public void warn( @Nonnull String format, @Nonnull Object... arguments )
public void warn( @Nonnull String format, @Nullable Object... arguments )
{
}

Expand All @@ -131,7 +132,7 @@ public void error( @Nonnull String message, @Nonnull Throwable throwable )
}

@Override
public void error( @Nonnull String format, @Nonnull Object... arguments )
public void error( @Nonnull String format, @Nullable Object... arguments )
{
}

Expand Down
Expand Up @@ -204,23 +204,20 @@ public void logShouldWriteNotFormattedMessageWhenNoParametersGiven()
}

@Test
public void logShouldThrowExceptionWhenParametersArrayIsNull()
public void logShouldWriteNotFormattedMessageWhenParametersArrayIsNull()
{
// Given
StringWriter writer = new StringWriter();
Log log = newFormattedLog( writer );

try
{
// When
log.info( "Come with me if you %s to live!", (Object[]) null );
fail( "Should have thrown " + RuntimeException.class );
}
catch ( RuntimeException npe )
{
// Then
assertThat( writer.toString(), equalTo( "" ) );
}
// When
log.info( "Come with me if you %s to live!", (Object[]) null );

// Then
assertThat(
writer.toString(),
equalTo( format( "1984-10-26 04:23:24.343+0000 INFO [test] Come with me if you %%s to live!%n" ) )
);
}

@Test
Expand Down

0 comments on commit 552d7fc

Please sign in to comment.