From 4ac2027a915f7c190960583ecf75e5ace3a26f7b Mon Sep 17 00:00:00 2001 From: Anton Persson Date: Mon, 10 Jul 2017 11:47:53 +0200 Subject: [PATCH] Cleanup combined index tests --- .../schema/combined/CombinedIndexUpdater.java | 14 +- .../combined/CombinedIndexAccessorTest.java | 121 ++++-------------- .../combined/CombinedIndexPopulatorTest.java | 46 ++++--- .../combined/CombinedIndexTestHelp.java | 107 ++++++++++++++++ 4 files changed, 169 insertions(+), 119 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexUpdater.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexUpdater.java index edc6af97f52b..cf5e8a3f4209 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexUpdater.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexUpdater.java @@ -71,11 +71,13 @@ public void process( IndexEntryUpdate update ) throws IOException, IndexEntryCon { from.process( IndexEntryUpdate.remove( update.getEntityId(), update.indexKey(), update.beforeValues() ) ); - from.process( IndexEntryUpdate.add( + to.process( IndexEntryUpdate.add( update.getEntityId(), update.indexKey(), update.values() ) ); } + break; case REMOVED: select( update.values(), boostUpdater, fallbackUpdater ).process( update ); + break; default: throw new IllegalArgumentException( "Unknown update mode" ); } @@ -84,7 +86,13 @@ public void process( IndexEntryUpdate update ) throws IOException, IndexEntryCon @Override public void close() throws IOException, IndexEntryConflictException { - boostUpdater.close(); - fallbackUpdater.close(); + try + { + boostUpdater.close(); + } + finally + { + fallbackUpdater.close(); + } } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexAccessorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexAccessorTest.java index 5710e29f4466..e8e262a7553d 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexAccessorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexAccessorTest.java @@ -19,8 +19,8 @@ */ package org.neo4j.kernel.impl.index.schema.combined; +import org.junit.Before; import org.junit.Test; -import org.mockito.Mockito; import java.io.IOException; @@ -35,17 +35,29 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.internal.verification.VerificationModeFactory.times; +import static org.neo4j.kernel.impl.index.schema.combined.CombinedIndexTestHelp.verifyCombinedThrowIfBothThrow; +import static org.neo4j.kernel.impl.index.schema.combined.CombinedIndexTestHelp.verifyFailOnSingleCloseFailure; +import static org.neo4j.kernel.impl.index.schema.combined.CombinedIndexTestHelp.verifyOtherIsClosedOnSingleThrow; public class CombinedIndexAccessorTest { + private IndexAccessor boostAccessor; + private IndexAccessor fallbackAccessor; + private CombinedIndexAccessor combinedIndexAccessor; + + @Before + public void setup() + { + boostAccessor = mock( IndexAccessor.class ); + fallbackAccessor = mock( IndexAccessor.class ); + combinedIndexAccessor = new CombinedIndexAccessor( boostAccessor, fallbackAccessor ); + } + + /* drop */ + @Test public void dropMustDropBoostAndFallback() throws Exception { - // given - IndexAccessor boostAccessor = mock( IndexAccessor.class ); - IndexAccessor fallbackAccessor = mock( IndexAccessor.class ); - CombinedIndexAccessor combinedIndexAccessor = new CombinedIndexAccessor( boostAccessor, fallbackAccessor ); - // when // ... both drop successful combinedIndexAccessor.drop(); @@ -57,11 +69,6 @@ public void dropMustDropBoostAndFallback() throws Exception @Test public void dropMustThrowIfDropBoostFail() throws Exception { - // given - IndexAccessor boostAccessor = mock( IndexAccessor.class ); - IndexAccessor fallbackAccessor = mock( IndexAccessor.class ); - CombinedIndexAccessor combinedIndexAccessor = new CombinedIndexAccessor( boostAccessor, fallbackAccessor ); - // when verifyFailOnSingleDropFailure( boostAccessor, combinedIndexAccessor ); } @@ -69,11 +76,6 @@ public void dropMustThrowIfDropBoostFail() throws Exception @Test public void dropMustThrowIfDropFallbackFail() throws Exception { - // given - IndexAccessor boostAccessor = mock( IndexAccessor.class ); - IndexAccessor fallbackAccessor = mock( IndexAccessor.class ); - CombinedIndexAccessor combinedIndexAccessor = new CombinedIndexAccessor( boostAccessor, fallbackAccessor ); - // when verifyFailOnSingleDropFailure( fallbackAccessor, combinedIndexAccessor ); } @@ -98,9 +100,6 @@ private void verifyFailOnSingleDropFailure( IndexAccessor failingAccessor, Combi public void dropMustThrowIfBothFail() throws Exception { // given - IndexAccessor boostAccessor = mock( IndexAccessor.class ); - IndexAccessor fallbackAccessor = mock( IndexAccessor.class ); - CombinedIndexAccessor combinedIndexAccessor = new CombinedIndexAccessor( boostAccessor, fallbackAccessor ); IOException boostFailure = new IOException( "boost" ); IOException fallbackFailure = new IOException( "fallback" ); doThrow( boostFailure ).when( boostAccessor ).drop(); @@ -119,17 +118,15 @@ public void dropMustThrowIfBothFail() throws Exception } } + /* close */ + @Test public void closeMustCloseBoostAndFallback() throws Exception { - // given - IndexAccessor boostAccessor = mock( IndexAccessor.class ); - IndexAccessor fallbackAccessor = mock( IndexAccessor.class ); - CombinedIndexAccessor combinedIndexAccessor = new CombinedIndexAccessor( boostAccessor, fallbackAccessor ); - // when // ... both drop successful combinedIndexAccessor.close(); + // then verify( boostAccessor, times( 1 ) ).close(); verify( fallbackAccessor, times( 1 ) ).close(); @@ -138,104 +135,30 @@ public void closeMustCloseBoostAndFallback() throws Exception @Test public void closeMustThrowIfFallbackThrow() throws Exception { - // given - IndexAccessor boostAccessor = mock( IndexAccessor.class ); - IndexAccessor fallbackAccessor = mock( IndexAccessor.class ); - CombinedIndexAccessor combinedIndexAccessor = new CombinedIndexAccessor( boostAccessor, fallbackAccessor ); - verifyFailOnSingleCloseFailure( fallbackAccessor, combinedIndexAccessor ); } @Test public void closeMustThrowIfBoostThrow() throws Exception { - // given - IndexAccessor boostAccessor = mock( IndexAccessor.class ); - IndexAccessor fallbackAccessor = mock( IndexAccessor.class ); - CombinedIndexAccessor combinedIndexAccessor = new CombinedIndexAccessor( boostAccessor, fallbackAccessor ); - verifyFailOnSingleCloseFailure( boostAccessor, combinedIndexAccessor ); } @Test public void closeMustCloseBoostIfFallbackThrow() throws Exception { - // given - IndexAccessor boostAccessor = mock( IndexAccessor.class ); - IndexAccessor fallbackAccessor = mock( IndexAccessor.class ); - CombinedIndexAccessor combinedIndexAccessor = new CombinedIndexAccessor( boostAccessor, fallbackAccessor ); - verifyOtherIsClosedOnSingleThrow( fallbackAccessor, boostAccessor, combinedIndexAccessor ); } @Test public void closeMustCloseFallbackIfBoostThrow() throws Exception { - // given - IndexAccessor boostAccessor = mock( IndexAccessor.class ); - IndexAccessor fallbackAccessor = mock( IndexAccessor.class ); - CombinedIndexAccessor combinedIndexAccessor = new CombinedIndexAccessor( boostAccessor, fallbackAccessor ); - verifyOtherIsClosedOnSingleThrow( boostAccessor, fallbackAccessor, combinedIndexAccessor ); } - private void verifyOtherIsClosedOnSingleThrow( IndexAccessor failingAccessor, IndexAccessor successfulAccessor, - CombinedIndexAccessor combinedIndexAccessor ) throws IOException - { - IOException failure = new IOException( "fail" ); - doThrow( failure ).when( failingAccessor ).close(); - - // when - try - { - combinedIndexAccessor.close(); - } - catch ( IOException ignore ) - { - } - - // then - verify( successfulAccessor, Mockito.times( 1 ) ).close(); - } - - private void verifyFailOnSingleCloseFailure( IndexAccessor failingAccessor, CombinedIndexAccessor combinedIndexAccessor ) - throws IOException - { - IOException expectedFailure = new IOException( "fail" ); - doThrow( expectedFailure ).when( failingAccessor ).close(); - try - { - combinedIndexAccessor.close(); - fail( "Should have failed" ); - } - catch ( IOException e ) - { - assertSame( expectedFailure, e ); - } - } - @Test public void closeMustThrowIfBothFail() throws Exception { - // given - IndexAccessor boostAccessor = mock( IndexAccessor.class ); - IndexAccessor fallbackAccessor = mock( IndexAccessor.class ); - CombinedIndexAccessor combinedIndexAccessor = new CombinedIndexAccessor( boostAccessor, fallbackAccessor ); - IOException boostFailure = new IOException( "boost" ); - IOException fallbackFailure = new IOException( "fallback" ); - doThrow( boostFailure ).when( boostAccessor ).close(); - doThrow( fallbackFailure ).when( fallbackAccessor ).close(); - - try - { - // when - combinedIndexAccessor.close(); - fail( "Should have failed" ); - } - catch ( IOException e ) - { - // then0 - assertThat( e, anyOf( sameInstance( boostFailure ), sameInstance( fallbackFailure ) ) ); - } + verifyCombinedThrowIfBothThrow( boostAccessor, fallbackAccessor, combinedIndexAccessor ); } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexPopulatorTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexPopulatorTest.java index c87d07f44751..7725fce39537 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexPopulatorTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexPopulatorTest.java @@ -28,9 +28,12 @@ import org.neo4j.kernel.api.index.IndexEntryUpdate; import org.neo4j.kernel.api.index.IndexPopulator; import org.neo4j.kernel.api.schema.LabelSchemaDescriptor; -import org.neo4j.kernel.api.schema.SchemaDescriptorFactory; import org.neo4j.values.storable.Value; +import static org.hamcrest.Matchers.sameInstance; +import static org.hamcrest.core.AnyOf.anyOf; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyString; @@ -38,6 +41,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.neo4j.kernel.impl.index.schema.combined.CombinedIndexTestHelp.add; import static org.neo4j.kernel.impl.index.schema.combined.CombinedIndexTestHelp.verifyCallFail; public class CombinedIndexPopulatorTest @@ -45,8 +49,6 @@ public class CombinedIndexPopulatorTest private IndexPopulator boostPopulator; private IndexPopulator fallbackPopulator; private CombinedIndexPopulator combinedIndexPopulator; - private LabelSchemaDescriptor indexKey = SchemaDescriptorFactory.forLabel( 0, 0 ); - private LabelSchemaDescriptor compositeIndexKey = SchemaDescriptorFactory.forLabel( 0, 0, 1 ); @Before public void mockComponents() @@ -173,25 +175,12 @@ public void addMustSelectCorrectPopulator() throws Exception private void verifyAddWithCorrectPopulator( IndexPopulator correctPopulator, IndexPopulator wrongPopulator, Value... numberValues ) throws IndexEntryConflictException, IOException { - IndexEntryUpdate update = indexEntryUpdate( numberValues ); + IndexEntryUpdate update = add( numberValues ); combinedIndexPopulator.add( update ); verify( correctPopulator, times( 1 ) ).add( update ); verify( wrongPopulator, times( 0 ) ).add( update ); } - private IndexEntryUpdate indexEntryUpdate( Value... value ) - { - switch ( value.length ) - { - case 1: - return IndexEntryUpdate.add( 0, indexKey, value ); - case 2: - return IndexEntryUpdate.add( 0, compositeIndexKey, value ); - default: - return null; - } - } - /* verifyDeferredConstraints */ @Test @@ -287,6 +276,7 @@ public void closeMustCloseBoostIfFallbackThrow() throws Exception try { combinedIndexPopulator.close( true ); + fail( "Should have failed" ); } catch ( IOException ignore ) { @@ -307,6 +297,7 @@ public void closeMustCloseFallbackIfBoostThrow() throws Exception try { combinedIndexPopulator.close( true ); + fail( "Should have failed" ); } catch ( IOException ignore ) { @@ -314,7 +305,28 @@ public void closeMustCloseFallbackIfBoostThrow() throws Exception // then verify( fallbackPopulator, times( 1 ) ).close( true ); + } + @Test + public void closeMustThrowIfBothThrow() throws Exception + { + // given + IOException boostFailure = new IOException( "boost" ); + IOException fallbackFailure = new IOException( "fallback" ); + doThrow( boostFailure ).when( boostPopulator ).close( anyBoolean() ); + doThrow( fallbackFailure ).when( fallbackPopulator).close( anyBoolean() ); + + try + { + // when + combinedIndexPopulator.close( anyBoolean() ); + fail( "Should have failed" ); + } + catch ( IOException e ) + { + // then + assertThat( e, anyOf( sameInstance( boostFailure ), sameInstance( fallbackFailure ) ) ); + } } /* markAsFailed */ diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexTestHelp.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexTestHelp.java index 27ce73b02493..78b5c4a4f7f4 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexTestHelp.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/index/schema/combined/CombinedIndexTestHelp.java @@ -20,17 +20,30 @@ package org.neo4j.kernel.impl.index.schema.combined; import org.apache.commons.lang3.ArrayUtils; +import org.mockito.Mockito; +import java.io.IOException; import java.util.concurrent.Callable; +import org.neo4j.kernel.api.index.IndexEntryUpdate; +import org.neo4j.kernel.api.schema.LabelSchemaDescriptor; +import org.neo4j.kernel.api.schema.SchemaDescriptorFactory; import org.neo4j.values.storable.Value; import org.neo4j.values.storable.Values; +import static org.hamcrest.Matchers.sameInstance; +import static org.hamcrest.core.AnyOf.anyOf; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; class CombinedIndexTestHelp { + private static LabelSchemaDescriptor indexKey = SchemaDescriptorFactory.forLabel( 0, 0 ); + private static LabelSchemaDescriptor compositeIndexKey = SchemaDescriptorFactory.forLabel( 0, 0, 1 ); + private static final Value[] numberValues = new Value[] { Values.byteValue( (byte) 1 ), @@ -84,4 +97,98 @@ static void verifyCallFail( Exception expectedFailure, Callable failingCall ) th assertSame( expectedFailure, e ); } } + + static IndexEntryUpdate add( Value... value ) + { + switch ( value.length ) + { + case 1: + return IndexEntryUpdate.add( 0, indexKey, value ); + case 2: + return IndexEntryUpdate.add( 0, compositeIndexKey, value ); + default: + return null; + } + } + + static IndexEntryUpdate remove( Value... value ) + { + switch ( value.length ) + { + case 1: + return IndexEntryUpdate.remove( 0, indexKey, value ); + case 2: + return IndexEntryUpdate.remove( 0, compositeIndexKey, value ); + default: + return null; + } + } + + static IndexEntryUpdate change( Value[] before, Value[] after ) + { + return IndexEntryUpdate.change( 0, compositeIndexKey, before, after ); + } + + static IndexEntryUpdate change( Value before, Value after ) + { + return IndexEntryUpdate.change( 0, indexKey, before, after ); + } + + static void verifyOtherIsClosedOnSingleThrow( AutoCloseable failingCloseable, AutoCloseable successfulCloseable, AutoCloseable combinedCloseable ) + throws Exception + { + IOException failure = new IOException( "fail" ); + doThrow( failure ).when( failingCloseable ).close(); + + // when + try + { + combinedCloseable.close(); + fail( "Should have failed" ); + } + catch ( IOException ignore ) + { + } + + // then + verify( successfulCloseable, Mockito.times( 1 ) ).close(); + } + + static void verifyFailOnSingleCloseFailure( AutoCloseable failingCloseable, AutoCloseable combinedCloseable ) + throws Exception + { + IOException expectedFailure = new IOException( "fail" ); + doThrow( expectedFailure ).when( failingCloseable ).close(); + try + { + combinedCloseable.close(); + fail( "Should have failed" ); + } + catch ( IOException e ) + { + assertSame( expectedFailure, e ); + } + } + + static void verifyCombinedThrowIfBothThrow( AutoCloseable boostCloseable, AutoCloseable fallbackCloseable, AutoCloseable combinedCloseable ) + throws Exception + { + // given + IOException boostFailure = new IOException( "boost" ); + IOException fallbackFailure = new IOException( "fallback" ); + doThrow( boostFailure ).when( boostCloseable ).close(); + doThrow( fallbackFailure ).when( fallbackCloseable ).close(); + + try + { + // when + combinedCloseable.close(); + fail( "Should have failed" ); + } + catch ( IOException e ) + { + // then + assertThat( e, anyOf( sameInstance( boostFailure ), sameInstance( fallbackFailure ) ) ); + } + } }