From 8a1a071824ee971bdfe3dc7954d99c632e7c57ed Mon Sep 17 00:00:00 2001 From: Mikhaylo Demianenko Date: Thu, 25 Aug 2016 18:57:37 +0200 Subject: [PATCH] Add additional IT tests for new api methods. Cleanup. --- .../impl/factory/GraphDatabaseFacadeTest.java | 96 +++++++++++++++++++ .../org/neo4j/server/AbstractNeoServer.java | 2 +- .../neo4j/server/database/CypherExecutor.java | 3 +- .../server/database/CypherExecutorTest.java | 22 +---- .../server/rest/CypherSessionDocTest.java | 3 +- .../TransactionGuardIntegrationTest.java | 89 +++++++++++++++-- 6 files changed, 184 insertions(+), 31 deletions(-) create mode 100644 community/kernel/src/test/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacadeTest.java diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacadeTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacadeTest.java new file mode 100644 index 000000000000..bd8eb490bf97 --- /dev/null +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/factory/GraphDatabaseFacadeTest.java @@ -0,0 +1,96 @@ +/* + * Copyright (c) 2002-2016 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Neo4j is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.neo4j.kernel.impl.factory; + + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; + +import java.util.HashMap; + +import org.neo4j.graphdb.DependencyResolver; +import org.neo4j.kernel.GraphDatabaseQueryService; +import org.neo4j.kernel.api.KernelTransaction; +import org.neo4j.kernel.api.security.AccessMode; +import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class GraphDatabaseFacadeTest +{ + + private GraphDatabaseFacade.SPI spi = Mockito.mock( GraphDatabaseFacade.SPI.class, Mockito.RETURNS_DEEP_STUBS ); + private GraphDatabaseFacade graphDatabaseFacade = new GraphDatabaseFacade(); + private GraphDatabaseQueryService queryService; + private DependencyResolver dependencyResolver; + + @Before + public void setUp() + { + queryService = mock( GraphDatabaseQueryService.class ); + dependencyResolver = mock( DependencyResolver.class ); + ThreadToStatementContextBridge contextBridge = mock( ThreadToStatementContextBridge.class ); + + when( spi.queryService() ).thenReturn( queryService ); + when( queryService.getDependencyResolver() ).thenReturn( dependencyResolver ); + when( dependencyResolver.resolveDependency( ThreadToStatementContextBridge.class ) ).thenReturn( contextBridge ); + + graphDatabaseFacade.init( spi ); + } + + @Test + public void beginTransactionWithCustomTimeout() throws Exception + { + graphDatabaseFacade.beginTx( 10L ); + + verify( spi ).beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.FULL, 10L ); + } + + @Test + public void beginTransaction() + { + graphDatabaseFacade.beginTx(); + + verify( spi ).beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.FULL ); + } + + @Test + public void executeQueryWithCustomTimeoutShouldStartTransactionWithRequestedTimeout() + { + graphDatabaseFacade.execute( "create (n)", 157L ); + verify( spi ).beginTransaction( KernelTransaction.Type.implicit, AccessMode.Static.FULL, 157L ); + + graphDatabaseFacade.execute( "create (n)", new HashMap<>(), 247L ); + verify( spi ).beginTransaction( KernelTransaction.Type.implicit, AccessMode.Static.FULL, 247L ); + } + + @Test + public void executeQueryStartDefaultTransaction() + { + graphDatabaseFacade.execute( "create (n)" ); + graphDatabaseFacade.execute( "create (n)", new HashMap<>() ); + + verify( spi, times( 2 ) ).beginTransaction( KernelTransaction.Type.implicit, AccessMode.Static.FULL ); + } +} diff --git a/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java b/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java index 9684f8d999a5..7f391acb8b3f 100644 --- a/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java +++ b/community/server/src/main/java/org/neo4j/server/AbstractNeoServer.java @@ -201,7 +201,7 @@ public void start() throws ServerStartupException transactionFacade = createTransactionalActions(); - cypherExecutor = new CypherExecutor( database, config, logProvider ); + cypherExecutor = new CypherExecutor( database, logProvider ); configureWebServer(); diff --git a/community/server/src/main/java/org/neo4j/server/database/CypherExecutor.java b/community/server/src/main/java/org/neo4j/server/database/CypherExecutor.java index 49607def5d16..a4e8ee1026ac 100644 --- a/community/server/src/main/java/org/neo4j/server/database/CypherExecutor.java +++ b/community/server/src/main/java/org/neo4j/server/database/CypherExecutor.java @@ -27,7 +27,6 @@ import org.neo4j.kernel.GraphDatabaseQueryService; import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.security.AccessMode; -import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge; import org.neo4j.kernel.impl.coreapi.InternalTransaction; import org.neo4j.kernel.impl.coreapi.PropertyContainerLocker; @@ -52,7 +51,7 @@ public class CypherExecutor extends LifecycleAdapter private static final PropertyContainerLocker locker = new PropertyContainerLocker(); private final Log log; - public CypherExecutor( Database database, Config config, LogProvider logProvider ) + public CypherExecutor( Database database, LogProvider logProvider ) { this.database = database; log = logProvider.getLog( getClass() ); diff --git a/community/server/src/test/java/org/neo4j/server/database/CypherExecutorTest.java b/community/server/src/test/java/org/neo4j/server/database/CypherExecutorTest.java index d3906f2c9508..09d69be9e9f2 100644 --- a/community/server/src/test/java/org/neo4j/server/database/CypherExecutorTest.java +++ b/community/server/src/test/java/org/neo4j/server/database/CypherExecutorTest.java @@ -26,13 +26,10 @@ import org.neo4j.cypher.internal.javacompat.ExecutionEngine; import org.neo4j.graphdb.DependencyResolver; -import org.neo4j.graphdb.factory.GraphDatabaseSettings; -import org.neo4j.helpers.collection.MapUtil; import org.neo4j.kernel.GraphDatabaseQueryService; import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.Statement; import org.neo4j.kernel.api.security.AccessMode; -import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge; import org.neo4j.kernel.impl.coreapi.InternalTransaction; import org.neo4j.kernel.impl.coreapi.TopLevelTransaction; @@ -70,8 +67,7 @@ public void setUp() @Test public void startDefaultTransaction() throws Throwable { - Config config = getConfiguredGuardConfig(); - CypherExecutor cypherExecutor = new CypherExecutor( database, config, logProvider ); + CypherExecutor cypherExecutor = new CypherExecutor( database, logProvider ); cypherExecutor.start(); cypherExecutor.createSession( request ); @@ -86,8 +82,7 @@ public void startTransactionWithCustomTimeout() throws Throwable when( request.getHeader( CypherExecutor.MAX_EXECUTION_TIME_HEADER ) ) .thenReturn( String.valueOf( CUSTOM_TRANSACTION_TIMEOUT ) ); - Config config = getConfiguredGuardConfig(); - CypherExecutor cypherExecutor = new CypherExecutor( database, config, logProvider ); + CypherExecutor cypherExecutor = new CypherExecutor( database, logProvider ); cypherExecutor.start(); cypherExecutor.createSession( request ); @@ -103,8 +98,7 @@ public void startDefaultTransactionWhenHeaderHasIncorrectValue() throws Throwabl when( request.getHeader( CypherExecutor.MAX_EXECUTION_TIME_HEADER ) ) .thenReturn( "not a number" ); - Config config = getConfiguredGuardConfig(); - CypherExecutor cypherExecutor = new CypherExecutor( database, config, logProvider ); + CypherExecutor cypherExecutor = new CypherExecutor( database, logProvider ); cypherExecutor.start(); cypherExecutor.createSession( request ); @@ -120,8 +114,7 @@ public void startDefaultTransactionIfTimeoutIsNegative() throws Throwable when( request.getHeader( CypherExecutor.MAX_EXECUTION_TIME_HEADER ) ) .thenReturn( "-2" ); - Config config = getConfiguredGuardConfig(); - CypherExecutor cypherExecutor = new CypherExecutor( database, config, logProvider ); + CypherExecutor cypherExecutor = new CypherExecutor( database, logProvider ); cypherExecutor.start(); cypherExecutor.createSession( request ); @@ -136,7 +129,7 @@ public void startDefaultTransactionIfExecutionGuardDisabled() throws Throwable when( request.getHeader( CypherExecutor.MAX_EXECUTION_TIME_HEADER ) ) .thenReturn( String.valueOf( CUSTOM_TRANSACTION_TIMEOUT ) ); - CypherExecutor cypherExecutor = new CypherExecutor( database, Config.defaults(), logProvider ); + CypherExecutor cypherExecutor = new CypherExecutor( database, logProvider ); cypherExecutor.start(); cypherExecutor.createSession( request ); @@ -150,11 +143,6 @@ private void initLogProvider() logProvider = new AssertableLogProvider( true ); } - private Config getConfiguredGuardConfig() - { - return new Config( MapUtil.stringMap( GraphDatabaseSettings.transaction_timeout.name(), "60s" ) ); - } - private void setUpMocks() { database = mock( Database.class ); diff --git a/community/server/src/test/java/org/neo4j/server/rest/CypherSessionDocTest.java b/community/server/src/test/java/org/neo4j/server/rest/CypherSessionDocTest.java index ea2f8dc80680..f85eb6acf8d4 100644 --- a/community/server/src/test/java/org/neo4j/server/rest/CypherSessionDocTest.java +++ b/community/server/src/test/java/org/neo4j/server/rest/CypherSessionDocTest.java @@ -24,7 +24,6 @@ import javax.servlet.http.HttpServletRequest; import org.neo4j.helpers.collection.Pair; -import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.impl.factory.GraphDatabaseFacade; import org.neo4j.logging.NullLogProvider; import org.neo4j.server.database.CypherExecutor; @@ -44,7 +43,7 @@ public void shouldReturnASingleNode() throws Throwable { GraphDatabaseFacade graphdb = (GraphDatabaseFacade) new TestGraphDatabaseFactory().newImpermanentDatabase(); Database database = new WrappedDatabase( graphdb ); - CypherExecutor executor = new CypherExecutor( database, Config.defaults(), NullLogProvider.getInstance() ); + CypherExecutor executor = new CypherExecutor( database, NullLogProvider.getInstance() ); executor.start(); try { diff --git a/integrationtests/src/test/java/org/neo4j/TransactionGuardIntegrationTest.java b/integrationtests/src/test/java/org/neo4j/TransactionGuardIntegrationTest.java index 58cb824437e8..98a09dd23cc1 100644 --- a/integrationtests/src/test/java/org/neo4j/TransactionGuardIntegrationTest.java +++ b/integrationtests/src/test/java/org/neo4j/TransactionGuardIntegrationTest.java @@ -67,6 +67,7 @@ import org.neo4j.test.TestGraphDatabaseFactoryState; import org.neo4j.test.rule.CleanupRule; import org.neo4j.test.server.HTTP; +import org.neo4j.time.Clocks; import org.neo4j.time.FakeClock; import static java.util.Collections.singletonList; @@ -84,19 +85,17 @@ public class TransactionGuardIntegrationTest @Rule public CleanupRule cleanupRule = new CleanupRule(); private FakeClock clock; - private GraphDatabaseAPI database; @Before public void setUp() { - clock = new FakeClock(); - Map,String> configMap = getSettingsMap(); - database = startCustomDatabase( clock, configMap ); + clock = Clocks.fakeClock(); } @Test public void terminateLongRunningTransaction() { + GraphDatabaseAPI database = startDatabaseWithTimeout(); try ( Transaction transaction = database.beginTx() ) { clock.forward( 3, TimeUnit.SECONDS ); @@ -112,9 +111,35 @@ public void terminateLongRunningTransaction() assertDatabaseDoesNotHaveNodes( database ); } + @Test + public void terminateTransactionWithCustomTimeoutWithoutConfiguredDefault() + { + GraphDatabaseAPI database = startDatabaseWithoutTimeout(); + try ( Transaction transaction = database.beginTx( TimeUnit.SECONDS.toMillis( 27 ) ) ) + { + clock.forward( 26, TimeUnit.SECONDS ); + database.createNode(); + transaction.failure(); + } + + try ( Transaction transaction = database.beginTx( TimeUnit.SECONDS.toMillis( 27 ) ) ) + { + clock.forward( 28, TimeUnit.SECONDS ); + database.createNode(); + fail( "Transaction should be already terminated." ); + } + catch ( GuardTimeoutException e ) + { + assertThat( e.getMessage(), startsWith( "Transaction timeout." ) ); + } + + assertDatabaseDoesNotHaveNodes( database ); + } + @Test public void terminateLongRunningQueryTransaction() { + GraphDatabaseAPI database = startDatabaseWithTimeout(); try ( Transaction transaction = database.beginTx() ) { clock.forward( 3, TimeUnit.SECONDS ); @@ -130,9 +155,36 @@ public void terminateLongRunningQueryTransaction() assertDatabaseDoesNotHaveNodes( database ); } + @Test + public void terminateLongRunningQueryWithCustomTimeoutWithoutConfiguredDefault() + { + GraphDatabaseAPI database = startDatabaseWithoutTimeout(); + try ( Transaction transaction = database.beginTx( TimeUnit.SECONDS.toMillis( 5 ) ) ) + { + clock.forward( 4, TimeUnit.SECONDS ); + database.execute( "create (n)" ); + transaction.failure(); + } + + try ( Transaction transaction = database.beginTx( TimeUnit.SECONDS.toMillis( 6 ) ) ) + { + clock.forward( 7, TimeUnit.SECONDS ); + transaction.success(); + database.execute( "create (n)" ); + fail( "Transaction should be already terminated." ); + } + catch ( GuardTimeoutException e ) + { + assertThat( e.getMessage(), startsWith( "Transaction timeout." ) ); + } + + assertDatabaseDoesNotHaveNodes( database ); + } + @Test public void terminateLongRunningShellQuery() throws Exception { + GraphDatabaseAPI database = startDatabaseWithTimeout(); GraphDatabaseShellServer shellServer = getGraphDatabaseShellServer( database ); try { @@ -155,6 +207,7 @@ public void terminateLongRunningShellQuery() throws Exception @Test public void terminateLongRunningRestQuery() throws Exception { + GraphDatabaseAPI database = startDatabaseWithTimeout(); CommunityNeoServer neoServer = startNeoServer( (GraphDatabaseFacade) database ); String transactionEndPoint = HTTP.POST( transactionUri(neoServer), singletonList( map( "statement", "create (n)" ) ) ).location(); @@ -170,6 +223,7 @@ public void terminateLongRunningRestQuery() throws Exception @Test public void terminateLongRunningDriverQuery() throws Exception { + GraphDatabaseAPI database = startDatabaseWithTimeout(); CommunityNeoServer neoServer = startNeoServer( (GraphDatabaseFacade) database ); org.neo4j.driver.v1.Config driverConfig = getDriverConfig(); @@ -194,6 +248,18 @@ public void terminateLongRunningDriverQuery() throws Exception assertDatabaseDoesNotHaveNodes( database ); } + protected GraphDatabaseAPI startDatabaseWithTimeout() + { + Map,String> configMap = getSettingsWithTransactionTimeout(); + return startCustomDatabase( clock, configMap ); + } + + protected GraphDatabaseAPI startDatabaseWithoutTimeout() + { + Map,String> configMap = getSettingsWithTransactionTimeout(); + return startCustomDatabase( clock, configMap ); + } + private org.neo4j.driver.v1.Config getDriverConfig() { return org.neo4j.driver.v1.Config.build() @@ -215,14 +281,20 @@ private CommunityNeoServer startNeoServer( GraphDatabaseFacade database ) throws return neoServer; } - private Map,String> getSettingsMap() + private Map,String> getSettingsWithTransactionTimeout() + { + Map,String> settingMap = getSettingsWithoutTransactionTimeout(); + settingMap.put( GraphDatabaseSettings.transaction_timeout, "2s" ); + return settingMap; + } + + private Map,String> getSettingsWithoutTransactionTimeout() { GraphDatabaseSettings.BoltConnector boltConnector = boltConnector( "0" ); return MapUtil.genericMap( boltConnector.type, "BOLT", boltConnector.enabled, "true", boltConnector.encryption_level, GraphDatabaseSettings.BoltConnector.EncryptionLevel.DISABLED.name(), - GraphDatabaseSettings.transaction_timeout, "2s", GraphDatabaseSettings.auth_enabled, "false" ); } @@ -272,7 +344,6 @@ private GraphDatabaseAPI startCustomDatabase( Clock clock, Map,String private class GuardingServerBuilder extends CommunityServerBuilder { - private GraphDatabaseFacade graphDatabaseFacade; final LifecycleManagingDatabase.GraphFactory PRECREATED_FACADE_FACTORY = ( config, dependencies ) -> graphDatabaseFacade; @@ -335,14 +406,14 @@ private class GuardCommunityFacadeFactory extends GraphDatabaseFacadeFactory protected DataSourceModule createDataSource( Dependencies dependencies, PlatformModule platformModule, EditionModule editionModule ) { - return new GuardDataSourceModule( dependencies, platformModule, editionModule, clock ); + return new GuardDataSourceModule( dependencies, platformModule, editionModule ); } private class GuardDataSourceModule extends DataSourceModule { GuardDataSourceModule( GraphDatabaseFacadeFactory.Dependencies dependencies, - PlatformModule platformModule, EditionModule editionModule, Clock clock ) + PlatformModule platformModule, EditionModule editionModule ) { super( dependencies, platformModule, editionModule ); }