Skip to content

Commit

Permalink
Fix NPE in dependecy resolver when datasource is null
Browse files Browse the repository at this point in the history
In some cases if you ask for resolving a dependency from the Neo store
data source when the latter is not available a NPE is thrown.  This
change will fix that behaviour by skipping the unavailable data source
from the list of possible dependency providers.
  • Loading branch information
davidegrohmann committed Oct 5, 2016
1 parent 419b959 commit da1c2a2
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 13 deletions.
Expand Up @@ -24,9 +24,7 @@
import java.util.HashMap; import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.function.Supplier;


import org.neo4j.graphdb.DependencyResolver;
import org.neo4j.graphdb.factory.GraphDatabaseSettings; import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.graphdb.security.URLAccessRule; import org.neo4j.graphdb.security.URLAccessRule;
import org.neo4j.helpers.collection.Iterables; import org.neo4j.helpers.collection.Iterables;
Expand Down Expand Up @@ -110,14 +108,10 @@ public PlatformModule( File providedStoreDir, Map<String, String> params, Databa
GraphDatabaseFacadeFactory.Dependencies externalDependencies, GraphDatabaseFacade graphDatabaseFacade ) GraphDatabaseFacadeFactory.Dependencies externalDependencies, GraphDatabaseFacade graphDatabaseFacade )
{ {
this.databaseInfo = databaseInfo; this.databaseInfo = databaseInfo;
dependencies = new org.neo4j.kernel.impl.util.Dependencies( new Supplier<DependencyResolver>() this.dataSourceManager = new DataSourceManager();
{ dependencies = new org.neo4j.kernel.impl.util.Dependencies(
@Override new DataSourceManager.DependencyResolverSupplier( dataSourceManager ) );
public DependencyResolver get()
{
return dataSourceManager.getDataSource().getDependencyResolver();
}
} );
life = dependencies.satisfyDependency( createLife() ); life = dependencies.satisfyDependency( createLife() );
this.graphDatabaseFacade = dependencies.satisfyDependency( graphDatabaseFacade ); this.graphDatabaseFacade = dependencies.satisfyDependency( graphDatabaseFacade );


Expand Down Expand Up @@ -172,7 +166,7 @@ public DependencyResolver get()
// this was the place of the XaDataSourceManager. NeoStoreXaDataSource is create further down than // this was the place of the XaDataSourceManager. NeoStoreXaDataSource is create further down than
// (specifically) KernelExtensions, which creates an interesting out-of-order issue with #doAfterRecovery(). // (specifically) KernelExtensions, which creates an interesting out-of-order issue with #doAfterRecovery().
// Anyways please fix this. // Anyways please fix this.
dataSourceManager = dependencies.satisfyDependency( new DataSourceManager() ); dependencies.satisfyDependency( dataSourceManager );


availabilityGuard = new AvailabilityGuard( Clocks.systemClock(), logging.getInternalLog( availabilityGuard = new AvailabilityGuard( Clocks.systemClock(), logging.getInternalLog(
AvailabilityGuard.class ) ); AvailabilityGuard.class ) );
Expand Down
Expand Up @@ -21,6 +21,7 @@


import java.util.function.Supplier; import java.util.function.Supplier;


import org.neo4j.graphdb.DependencyResolver;
import org.neo4j.helpers.Listeners; import org.neo4j.helpers.Listeners;
import org.neo4j.kernel.NeoStoreDataSource; import org.neo4j.kernel.NeoStoreDataSource;
import org.neo4j.kernel.api.KernelAPI; import org.neo4j.kernel.api.KernelAPI;
Expand Down Expand Up @@ -131,4 +132,25 @@ public KernelAPI get()
{ {
return dataSource.getKernel(); return dataSource.getKernel();
} }

public static class DependencyResolverSupplier implements Supplier<DependencyResolver>
{
private DataSourceManager dataSourceManager;

public DependencyResolverSupplier( DataSourceManager dataSourceManager )
{
this.dataSourceManager = dataSourceManager;
}

@Override
public DependencyResolver get()
{
NeoStoreDataSource dataSource = dataSourceManager.getDataSource();
if ( dataSource == null )
{
return null;
}
return dataSource.getDependencyResolver();
}
}
} }
@@ -0,0 +1,69 @@
/*
* 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 <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.transaction.state;


import org.junit.Test;

import org.neo4j.graphdb.DependencyResolver;
import org.neo4j.kernel.NeoStoreDataSource;
import org.neo4j.kernel.impl.transaction.state.DataSourceManager.DependencyResolverSupplier;

import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

public class DependencyResolverSupplierTest
{
@Test
public void shouldReturnTheDependencyResolveFromTheRegisteredDatasource() throws Exception
{
// given
DataSourceManager dataSourceManager = new DataSourceManager();
DependencyResolverSupplier supplier = new DependencyResolverSupplier( dataSourceManager );
NeoStoreDataSource neoStoreDataSource = mock( NeoStoreDataSource.class );
DependencyResolver dependencyResolver = mock( DependencyResolver.class );
when( neoStoreDataSource.getDependencyResolver() ).thenReturn( dependencyResolver );

// when
dataSourceManager.register( neoStoreDataSource );

// then
assertEquals( dependencyResolver, supplier.get() );
}

@Test
public void shouldReturnNullIfDataSourceHasBeenUnregistered() throws Exception
{
// given
DataSourceManager dataSourceManager = new DataSourceManager();
DependencyResolverSupplier supplier = new DependencyResolverSupplier( dataSourceManager );
NeoStoreDataSource neoStoreDataSource = mock( NeoStoreDataSource.class );
DependencyResolver dependencyResolver = mock( DependencyResolver.class );
when( neoStoreDataSource.getDependencyResolver() ).thenReturn( dependencyResolver );
dataSourceManager.register( neoStoreDataSource );

// when
dataSourceManager.unregister( neoStoreDataSource );

// then
assertEquals( null, supplier.get() );
}
}
Expand Up @@ -36,6 +36,7 @@
import org.neo4j.coreedge.discovery.EdgeClusterMember; import org.neo4j.coreedge.discovery.EdgeClusterMember;
import org.neo4j.coreedge.handlers.ExceptionMonitoringHandler; import org.neo4j.coreedge.handlers.ExceptionMonitoringHandler;
import org.neo4j.kernel.impl.transaction.log.TransactionIdStore; import org.neo4j.kernel.impl.transaction.log.TransactionIdStore;
import org.neo4j.kernel.impl.util.UnsatisfiedDependencyException;
import org.neo4j.kernel.internal.GraphDatabaseAPI; import org.neo4j.kernel.internal.GraphDatabaseAPI;
import org.neo4j.kernel.monitoring.Monitors; import org.neo4j.kernel.monitoring.Monitors;


Expand Down Expand Up @@ -150,7 +151,7 @@ private long txId( ClusterMember member, boolean fail )
return database.getDependencyResolver().resolveDependency( TransactionIdStore.class ) return database.getDependencyResolver().resolveDependency( TransactionIdStore.class )
.getLastClosedTransactionId(); .getLastClosedTransactionId();
} }
catch ( IllegalStateException ex ) catch ( IllegalStateException | UnsatisfiedDependencyException ex )
{ {
return errorValueOrThrow( !isStoreClosed( ex ) || fail, ex ); return errorValueOrThrow( !isStoreClosed( ex ) || fail, ex );
} }
Expand All @@ -168,8 +169,18 @@ private long errorValueOrThrow( boolean fail, RuntimeException error )
} }
} }


private boolean isStoreClosed( IllegalStateException ex ) private boolean isStoreClosed( RuntimeException ex )
{ {
if ( ex instanceof UnsatisfiedDependencyException )
{
return true;
}

if ( !(ex instanceof IllegalStateException) )
{
return false;
}

String message = ex.getMessage(); String message = ex.getMessage();
return message.startsWith( "MetaDataStore for file " ) && message.endsWith( " is closed" ); return message.startsWith( "MetaDataStore for file " ) && message.endsWith( " is closed" );
} }
Expand Down

0 comments on commit da1c2a2

Please sign in to comment.