Skip to content

Commit

Permalink
Avoid lambdas in GraphDatabaseFacade.
Browse files Browse the repository at this point in the history
Always start transaction with timeout (default or custom).
Remove method to start transaction without timeout from SPI.
  • Loading branch information
MishaDemianenko committed Sep 7, 2016
1 parent b18ef55 commit 4dc5824
Show file tree
Hide file tree
Showing 10 changed files with 62 additions and 37 deletions.
Expand Up @@ -28,7 +28,6 @@
import org.neo4j.graphdb.Result; import org.neo4j.graphdb.Result;
import org.neo4j.graphdb.event.KernelEventHandler; import org.neo4j.graphdb.event.KernelEventHandler;
import org.neo4j.graphdb.event.TransactionEventHandler; import org.neo4j.graphdb.event.TransactionEventHandler;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.graphdb.security.URLAccessValidationError; import org.neo4j.graphdb.security.URLAccessValidationError;
import org.neo4j.kernel.GraphDatabaseQueryService; import org.neo4j.kernel.GraphDatabaseQueryService;
import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.KernelTransaction;
Expand All @@ -54,15 +53,13 @@ class ClassicCoreSPI implements GraphDatabaseFacade.SPI
private final DataSourceModule dataSource; private final DataSourceModule dataSource;
private final Logger msgLog; private final Logger msgLog;
private final CoreAPIAvailabilityGuard availability; private final CoreAPIAvailabilityGuard availability;
private final long defaultTransactionTimeout;


public ClassicCoreSPI(PlatformModule platform, DataSourceModule dataSource, Logger msgLog, CoreAPIAvailabilityGuard availability ) public ClassicCoreSPI(PlatformModule platform, DataSourceModule dataSource, Logger msgLog, CoreAPIAvailabilityGuard availability )
{ {
this.platform = platform; this.platform = platform;
this.dataSource = dataSource; this.dataSource = dataSource;
this.msgLog = msgLog; this.msgLog = msgLog;
this.availability = availability; this.availability = availability;
defaultTransactionTimeout = platform.config.get( GraphDatabaseSettings.transaction_timeout );
} }


@Override @Override
Expand Down Expand Up @@ -167,12 +164,6 @@ public void shutdown()
} }
} }


@Override
public KernelTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode )
{
return beginTransaction( type, accessMode, defaultTransactionTimeout );
}

@Override @Override
public KernelTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode, long timeout ) public KernelTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode, long timeout )
{ {
Expand Down
Expand Up @@ -24,7 +24,6 @@
import java.util.Collections; import java.util.Collections;
import java.util.Map; import java.util.Map;
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;


import org.neo4j.collection.primitive.PrimitiveLongCollections; import org.neo4j.collection.primitive.PrimitiveLongCollections;
import org.neo4j.collection.primitive.PrimitiveLongIterator; import org.neo4j.collection.primitive.PrimitiveLongIterator;
Expand All @@ -44,6 +43,7 @@
import org.neo4j.graphdb.TransactionTerminatedException; import org.neo4j.graphdb.TransactionTerminatedException;
import org.neo4j.graphdb.event.KernelEventHandler; import org.neo4j.graphdb.event.KernelEventHandler;
import org.neo4j.graphdb.event.TransactionEventHandler; import org.neo4j.graphdb.event.TransactionEventHandler;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.graphdb.index.IndexManager; import org.neo4j.graphdb.index.IndexManager;
import org.neo4j.graphdb.schema.Schema; import org.neo4j.graphdb.schema.Schema;
import org.neo4j.graphdb.security.URLAccessValidationError; import org.neo4j.graphdb.security.URLAccessValidationError;
Expand All @@ -67,6 +67,7 @@
import org.neo4j.kernel.api.legacyindex.AutoIndexing; import org.neo4j.kernel.api.legacyindex.AutoIndexing;
import org.neo4j.kernel.api.properties.Property; import org.neo4j.kernel.api.properties.Property;
import org.neo4j.kernel.api.security.AccessMode; import org.neo4j.kernel.api.security.AccessMode;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.impl.api.TokenAccess; import org.neo4j.kernel.impl.api.TokenAccess;
import org.neo4j.kernel.impl.api.legacyindex.InternalAutoIndexing; import org.neo4j.kernel.impl.api.legacyindex.InternalAutoIndexing;
import org.neo4j.kernel.impl.api.operations.KeyReadOperations; import org.neo4j.kernel.impl.api.operations.KeyReadOperations;
Expand Down Expand Up @@ -115,6 +116,7 @@ public class GraphDatabaseFacade implements GraphDatabaseAPI
private NodeProxy.NodeActions nodeActions; private NodeProxy.NodeActions nodeActions;
private RelationshipProxy.RelationshipActions relActions; private RelationshipProxy.RelationshipActions relActions;
private SPI spi; private SPI spi;
private long defaultTransactionTimeout;


/** /**
* This is what you need to implemenent to get your very own {@link GraphDatabaseFacade}. This SPI exists as a thin * This is what you need to implemenent to get your very own {@link GraphDatabaseFacade}. This SPI exists as a thin
Expand All @@ -141,14 +143,6 @@ public interface SPI


void shutdown(); void shutdown();


/**
* Begin a new kernel transaction. If a transaction is already associated to the current context
* (meaning, non-null is returned from {@link #currentTransaction()}), this should fail.
*
* @throws org.neo4j.graphdb.TransactionFailureException if unable to begin, or a transaction already exists.
*/
KernelTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode );

/** /**
* Begin a new kernel transaction with specified timeout in milliseconds. * Begin a new kernel transaction with specified timeout in milliseconds.
* If a transaction is already associated to the current context * If a transaction is already associated to the current context
Expand Down Expand Up @@ -200,8 +194,9 @@ public GraphDatabaseFacade()
/** /**
* Create a new Core API facade, backed by the given SPI. * Create a new Core API facade, backed by the given SPI.
*/ */
public void init( SPI spi ) public void init( SPI spi, Config config )
{ {
defaultTransactionTimeout = config.get( GraphDatabaseSettings.transaction_timeout );
IndexProviderImpl idxProvider = new IndexProviderImpl( this, spi::currentStatement ); IndexProviderImpl idxProvider = new IndexProviderImpl( this, spi::currentStatement );


this.spi = spi; this.spi = spi;
Expand Down Expand Up @@ -350,15 +345,17 @@ public Transaction beginTx( long timeout, TimeUnit unit )
return beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.FULL, timeout, unit ); return beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.FULL, timeout, unit );
} }


@Override
public InternalTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode ) public InternalTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode )
{ {
return beginTransaction( () -> spi.beginTransaction( type, accessMode ) ); return beginTransactionInternal( type, accessMode, defaultTransactionTimeout );
} }


@Override
public InternalTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode, public InternalTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode,
long timeout, TimeUnit unit ) long timeout, TimeUnit unit )
{ {
return beginTransaction( () -> spi.beginTransaction( type, accessMode, unit.toMillis( timeout ) ) ); return beginTransactionInternal( type, accessMode, unit.toMillis( timeout ) );
} }


@Override @Override
Expand Down Expand Up @@ -543,14 +540,15 @@ public ResourceIterator<Node> findNodes( final Label myLabel )
return allNodesWithLabel( myLabel ); return allNodesWithLabel( myLabel );
} }


private InternalTransaction beginTransaction( Supplier<KernelTransaction> transactionSupplier ) private InternalTransaction beginTransactionInternal( KernelTransaction.Type type, AccessMode accessMode,
long timeoutMillis )
{ {
if ( spi.isInOpenTransaction() ) if ( spi.isInOpenTransaction() )
{ {
// FIXME: perhaps we should check that the new type and access mode are compatible with the current tx // FIXME: perhaps we should check that the new type and access mode are compatible with the current tx
return new PlaceboTransaction( spi::currentTransaction, spi::currentStatement ); return new PlaceboTransaction( spi::currentTransaction, spi::currentStatement );
} }
return new TopLevelTransaction( transactionSupplier.get(), spi::currentStatement ); return new TopLevelTransaction( spi.beginTransaction( type, accessMode, timeoutMillis ), spi::currentStatement );
} }


private ResourceIterator<Node> nodesByLabelAndProperty( Label myLabel, String key, Object value ) private ResourceIterator<Node> nodesByLabelAndProperty( Label myLabel, String key, Object value )
Expand Down
Expand Up @@ -138,7 +138,8 @@ public GraphDatabaseFacade initFacade( File storeDir, Map<String,String> params,
CoreAPIAvailabilityGuard coreAPIAvailabilityGuard = edition.coreAPIAvailabilityGuard; CoreAPIAvailabilityGuard coreAPIAvailabilityGuard = edition.coreAPIAvailabilityGuard;


// Start it // Start it
graphDatabaseFacade.init( new ClassicCoreSPI( platform, dataSource, msgLog, coreAPIAvailabilityGuard ) ); graphDatabaseFacade.init( new ClassicCoreSPI( platform, dataSource, msgLog, coreAPIAvailabilityGuard ),
platform.config );


Throwable error = null; Throwable error = null;
try try
Expand Down
Expand Up @@ -195,12 +195,6 @@ public void shutdown()
throw new UnsupportedOperationException(); throw new UnsupportedOperationException();
} }


@Override
public KernelTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode )
{
throw new UnsupportedOperationException();
}

@Override @Override
public KernelTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode, long timeout ) public KernelTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode, long timeout )
{ {
Expand Down
Expand Up @@ -40,6 +40,7 @@


public class ProcedureGDSFactory implements ThrowingFunction<CallableProcedure.Context,GraphDatabaseService,ProcedureException> public class ProcedureGDSFactory implements ThrowingFunction<CallableProcedure.Context,GraphDatabaseService,ProcedureException>
{ {
private Config config;
private final File storeDir; private final File storeDir;
private final DependencyResolver resolver; private final DependencyResolver resolver;
private final Supplier<StoreId> storeId; private final Supplier<StoreId> storeId;
Expand All @@ -55,6 +56,7 @@ public ProcedureGDSFactory( Config config,
CoreAPIAvailabilityGuard availability, CoreAPIAvailabilityGuard availability,
URLAccessRule urlAccessRule ) URLAccessRule urlAccessRule )
{ {
this.config = config;
this.storeDir = storeDir; this.storeDir = storeDir;
this.resolver = resolver; this.resolver = resolver;
this.storeId = storeId; this.storeId = storeId;
Expand All @@ -70,7 +72,7 @@ public GraphDatabaseService apply( CallableProcedure.Context context ) throws Pr
Thread owningThread = context.get( CallableProcedure.Context.THREAD ); Thread owningThread = context.get( CallableProcedure.Context.THREAD );
GraphDatabaseFacade facade = new GraphDatabaseFacade(); GraphDatabaseFacade facade = new GraphDatabaseFacade();
facade.init( new ProcedureGDBFacadeSPI( owningThread, transaction, queryExecutor, storeDir, resolver, facade.init( new ProcedureGDBFacadeSPI( owningThread, transaction, queryExecutor, storeDir, resolver,
AutoIndexing.UNSUPPORTED, storeId, availability, urlValidator ) ); AutoIndexing.UNSUPPORTED, storeId, availability, urlValidator ), config );
return facade; return facade;
} }


Expand Down
Expand Up @@ -20,6 +20,7 @@
package org.neo4j.kernel.internal; package org.neo4j.kernel.internal;


import java.net.URL; import java.net.URL;
import java.util.concurrent.TimeUnit;


import org.neo4j.graphdb.DependencyResolver; import org.neo4j.graphdb.DependencyResolver;
import org.neo4j.graphdb.GraphDatabaseService; import org.neo4j.graphdb.GraphDatabaseService;
Expand Down Expand Up @@ -53,5 +54,21 @@ public interface GraphDatabaseAPI extends GraphDatabaseService


String getStoreDir(); String getStoreDir();


/**
* Begin internal transaction with specified type and access mode
* @param type transaction type
* @param accessMode transaction access mode
* @return internal transaction
*/
InternalTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode ); InternalTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode );

/**
* Begin internal transaction with specified type, access mode and timeout
* @param type transaction type
* @param accessMode transaction access mode
* @param timeout transaction timeout
* @param unit time unit of timeout argument
* @return internal transaction
*/
InternalTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode, long timeout, TimeUnit unit );
} }
Expand Up @@ -28,9 +28,11 @@
import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeUnit;


import org.neo4j.graphdb.DependencyResolver; import org.neo4j.graphdb.DependencyResolver;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.kernel.GraphDatabaseQueryService; import org.neo4j.kernel.GraphDatabaseQueryService;
import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.KernelTransaction;
import org.neo4j.kernel.api.security.AccessMode; 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.core.ThreadToStatementContextBridge;


import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
Expand All @@ -45,6 +47,7 @@ public class GraphDatabaseFacadeTest
private GraphDatabaseFacade graphDatabaseFacade = new GraphDatabaseFacade(); private GraphDatabaseFacade graphDatabaseFacade = new GraphDatabaseFacade();
private GraphDatabaseQueryService queryService; private GraphDatabaseQueryService queryService;
private DependencyResolver dependencyResolver; private DependencyResolver dependencyResolver;
private Config defaultConfig;


@Before @Before
public void setUp() public void setUp()
Expand All @@ -55,9 +58,12 @@ public void setUp()


when( spi.queryService() ).thenReturn( queryService ); when( spi.queryService() ).thenReturn( queryService );
when( queryService.getDependencyResolver() ).thenReturn( dependencyResolver ); when( queryService.getDependencyResolver() ).thenReturn( dependencyResolver );
when( dependencyResolver.resolveDependency( ThreadToStatementContextBridge.class ) ).thenReturn( contextBridge ); when( dependencyResolver.resolveDependency( ThreadToStatementContextBridge.class ) )
.thenReturn( contextBridge );


graphDatabaseFacade.init( spi ); defaultConfig = Config.defaults();

graphDatabaseFacade.init( spi, defaultConfig );
} }


@Test @Test
Expand All @@ -73,7 +79,8 @@ public void beginTransaction()
{ {
graphDatabaseFacade.beginTx(); graphDatabaseFacade.beginTx();


verify( spi ).beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.FULL ); long timeout = defaultConfig.get( GraphDatabaseSettings.transaction_timeout );
verify( spi ).beginTransaction( KernelTransaction.Type.explicit, AccessMode.Static.FULL, timeout );
} }


@Test @Test
Expand All @@ -94,6 +101,7 @@ public void executeQueryStartDefaultTransaction()
graphDatabaseFacade.execute( "create (n)" ); graphDatabaseFacade.execute( "create (n)" );
graphDatabaseFacade.execute( "create (n)", new HashMap<>() ); graphDatabaseFacade.execute( "create (n)", new HashMap<>() );


verify( spi, times( 2 ) ).beginTransaction( KernelTransaction.Type.implicit, AccessMode.Static.FULL ); long timeout = defaultConfig.get( GraphDatabaseSettings.transaction_timeout );
verify( spi, times( 2 ) ).beginTransaction( KernelTransaction.Type.implicit, AccessMode.Static.FULL, timeout );
} }
} }
Expand Up @@ -155,6 +155,13 @@ public InternalTransaction beginTransaction( KernelTransaction.Type type, Access
return getGraphDatabaseAPI().beginTransaction( type, accessMode ); return getGraphDatabaseAPI().beginTransaction( type, accessMode );
} }


@Override
public InternalTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode, long timeout,
TimeUnit unit )
{
return getGraphDatabaseAPI().beginTransaction( type, accessMode, timeout, unit );
}

@Override @Override
public Transaction beginTx() public Transaction beginTx()
{ {
Expand Down
Expand Up @@ -62,13 +62,13 @@ public void setUp()
@Test @Test
public void startTransactionWithCustomTimeout() throws Exception public void startTransactionWithCustomTimeout() throws Exception
{ {
when( databaseFacade.beginTransaction( type, accessMode, 10, TimeUnit.SECONDS ) ).thenReturn( internalTransaction ); when( databaseFacade.beginTransaction( type, accessMode, 10, TimeUnit.MILLISECONDS ) ).thenReturn( internalTransaction );


TransitionalPeriodTransactionMessContainer transactionMessContainer = TransitionalPeriodTransactionMessContainer transactionMessContainer =
new TransitionalPeriodTransactionMessContainer( databaseFacade ); new TransitionalPeriodTransactionMessContainer( databaseFacade );
transactionMessContainer.create( queryService, type, accessMode, 10, request ); transactionMessContainer.create( queryService, type, accessMode, 10, request );


verify( databaseFacade ).beginTransaction( type, accessMode, 10, TimeUnit.SECONDS ); verify( databaseFacade ).beginTransaction( type, accessMode, 10, TimeUnit.MILLISECONDS );
} }


@Test @Test
Expand Down
Expand Up @@ -98,6 +98,13 @@ public InternalTransaction beginTransaction( KernelTransaction.Type type, Access
return actual.beginTransaction( type, accessMode ); return actual.beginTransaction( type, accessMode );
} }


@Override
public InternalTransaction beginTransaction( KernelTransaction.Type type, AccessMode accessMode, long timeout,
TimeUnit unit )
{
return actual.beginTransaction( type, accessMode, timeout, unit );
}

@Override @Override
public Transaction beginTx() public Transaction beginTx()
{ {
Expand Down

0 comments on commit 4dc5824

Please sign in to comment.