From df3041ad01c6044a0117baebd016680ced791e06 Mon Sep 17 00:00:00 2001 From: lutovich Date: Wed, 21 Oct 2015 22:48:12 +0200 Subject: [PATCH] Improved testability of BufferReusingChunkingChannelBuffer Now it accepts a factory of ChannelBuffers which makes testing more pleasant compared to method overriding. Rewritten corresponding test to use this new ability. --- .../BufferReusingChunkingChannelBuffer.java | 27 ++++++++++---- ...ufferReusingChunkingChannelBufferTest.java | 35 ++++++++++++++----- 2 files changed, 48 insertions(+), 14 deletions(-) diff --git a/enterprise/backup/src/main/java/org/neo4j/backup/BufferReusingChunkingChannelBuffer.java b/enterprise/backup/src/main/java/org/neo4j/backup/BufferReusingChunkingChannelBuffer.java index 11a1c9ea21914..9dbc77507ed79 100644 --- a/enterprise/backup/src/main/java/org/neo4j/backup/BufferReusingChunkingChannelBuffer.java +++ b/enterprise/backup/src/main/java/org/neo4j/backup/BufferReusingChunkingChannelBuffer.java @@ -20,6 +20,7 @@ package org.neo4j.backup; import org.jboss.netty.buffer.ChannelBuffer; +import org.jboss.netty.buffer.ChannelBuffers; import org.jboss.netty.channel.Channel; import org.jboss.netty.channel.ChannelFuture; import org.jboss.netty.channel.ChannelFutureListener; @@ -28,6 +29,7 @@ import java.util.concurrent.LinkedBlockingQueue; import org.neo4j.com.ChunkingChannelBuffer; +import org.neo4j.function.Factory; /** * {@linkplain ChunkingChannelBuffer Chunking buffer} that is able to reuse up to {@link #MAX_WRITE_AHEAD_CHUNKS} @@ -43,19 +45,37 @@ */ class BufferReusingChunkingChannelBuffer extends ChunkingChannelBuffer { + private static final Factory DEFAULT_CHANNEL_BUFFER_FACTORY = new Factory() + { + @Override + public ChannelBuffer newInstance() + { + return ChannelBuffers.dynamicBuffer(); + } + }; + + private final Factory bufferFactory; private final Queue freeBuffers = new LinkedBlockingQueue<>( MAX_WRITE_AHEAD_CHUNKS ); BufferReusingChunkingChannelBuffer( ChannelBuffer initialBuffer, Channel channel, int capacity, byte internalProtocolVersion, byte applicationProtocolVersion ) + { + this( initialBuffer, DEFAULT_CHANNEL_BUFFER_FACTORY, channel, capacity, internalProtocolVersion, + applicationProtocolVersion ); + } + + BufferReusingChunkingChannelBuffer( ChannelBuffer initialBuffer, Factory bufferFactory, + Channel channel, int capacity, byte internalProtocolVersion, byte applicationProtocolVersion ) { super( initialBuffer, channel, capacity, internalProtocolVersion, applicationProtocolVersion ); + this.bufferFactory = bufferFactory; } @Override protected ChannelBuffer newChannelBuffer() { ChannelBuffer buffer = freeBuffers.poll(); - return (buffer == null) ? createNewChannelBuffer() : buffer; + return (buffer == null) ? bufferFactory.newInstance() : buffer; } @Override @@ -72,9 +92,4 @@ public void operationComplete( ChannelFuture future ) throws Exception } }; } - - ChannelBuffer createNewChannelBuffer() - { - return super.newChannelBuffer(); - } } diff --git a/enterprise/backup/src/test/java/org/neo4j/backup/BufferReusingChunkingChannelBufferTest.java b/enterprise/backup/src/test/java/org/neo4j/backup/BufferReusingChunkingChannelBufferTest.java index f2747f14e2cd3..66bd5e3131ee3 100644 --- a/enterprise/backup/src/test/java/org/neo4j/backup/BufferReusingChunkingChannelBufferTest.java +++ b/enterprise/backup/src/test/java/org/neo4j/backup/BufferReusingChunkingChannelBufferTest.java @@ -25,31 +25,37 @@ import org.jboss.netty.channel.ChannelFuture; import org.junit.Test; +import org.neo4j.function.Factory; + +import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.anyObject; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class BufferReusingChunkingChannelBufferTest { @Test + @SuppressWarnings( "unchecked" ) public void newBuffersAreCreatedIfNoFreeBuffersAreAvailable() { - BufferReusingChunkingChannelBuffer buffer = newBufferReusingChunkingChannelBufferSpy( 10 ); + CountingChannelBufferFactory bufferFactory = new CountingChannelBufferFactory(); + BufferReusingChunkingChannelBuffer buffer = newBufferReusingChunkingChannelBuffer( 10, bufferFactory ); buffer.writeLong( 1 ); buffer.writeLong( 2 ); buffer.writeLong( 3 ); - verify( buffer, times( 3 ) ).createNewChannelBuffer(); + assertEquals( 3, bufferFactory.instancesCreated ); } @Test + @SuppressWarnings( "unchecked" ) public void freeBuffersAreReused() throws Exception { - BufferReusingChunkingChannelBuffer buffer = newBufferReusingChunkingChannelBufferSpy( 10 ); + CountingChannelBufferFactory bufferFactory = new CountingChannelBufferFactory(); + BufferReusingChunkingChannelBuffer buffer = newBufferReusingChunkingChannelBuffer( 10, bufferFactory ); buffer.writeLong( 1 ); buffer.writeLong( 2 ); @@ -62,16 +68,17 @@ public void freeBuffersAreReused() throws Exception buffer.writeLong( 4 ); // 2 buffers were created - verify( buffer, times( 2 ) ).createNewChannelBuffer(); + assertEquals( 2, bufferFactory.instancesCreated ); // and 2 buffers were reused verify( reusedBuffer1 ).writeLong( 3 ); verify( reusedBuffer2 ).writeLong( 4 ); } - private static BufferReusingChunkingChannelBuffer newBufferReusingChunkingChannelBufferSpy( int capacity ) + private static BufferReusingChunkingChannelBuffer newBufferReusingChunkingChannelBuffer( int capacity, + CountingChannelBufferFactory bufferFactory ) { - ChannelBuffer initialBuffer = ChannelBuffers.dynamicBuffer(); + ChannelBuffer buffer = ChannelBuffers.dynamicBuffer(); Channel channel = mock( Channel.class ); ChannelFuture channelFuture = mock( ChannelFuture.class ); @@ -80,7 +87,7 @@ private static BufferReusingChunkingChannelBuffer newBufferReusingChunkingChanne when( channel.isBound() ).thenReturn( true ); when( channel.write( anyObject() ) ).thenReturn( channelFuture ); - return spy( new BufferReusingChunkingChannelBuffer( initialBuffer, channel, capacity, (byte) 1, (byte) 1 ) ); + return new BufferReusingChunkingChannelBuffer( buffer, bufferFactory, channel, capacity, (byte) 1, (byte) 1 ); } private static ChannelBuffer triggerOperationCompleteCallback( BufferReusingChunkingChannelBuffer buffer ) @@ -95,4 +102,16 @@ private static ChannelBuffer triggerOperationCompleteCallback( BufferReusingChun buffer.newChannelFutureListener( reusedBuffer ).operationComplete( channelFuture ); return reusedBuffer; } + + private static class CountingChannelBufferFactory implements Factory + { + int instancesCreated; + + @Override + public ChannelBuffer newInstance() + { + instancesCreated++; + return ChannelBuffers.dynamicBuffer(); + } + } }