Skip to content

Commit

Permalink
Cleanup based on review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
thobe committed Jan 17, 2017
1 parent ac7d019 commit ddb0c21
Show file tree
Hide file tree
Showing 14 changed files with 54 additions and 29 deletions.
Expand Up @@ -21,7 +21,6 @@


import org.junit.Test; import org.junit.Test;


import org.neo4j.bolt.v1.runtime.CypherAdapterStream;
import org.neo4j.bolt.v1.runtime.spi.Record; import org.neo4j.bolt.v1.runtime.spi.Record;
import org.neo4j.bolt.v1.runtime.spi.BoltResult; import org.neo4j.bolt.v1.runtime.spi.BoltResult;
import org.neo4j.graphdb.ExecutionPlanDescription; import org.neo4j.graphdb.ExecutionPlanDescription;
Expand Down
Expand Up @@ -34,9 +34,9 @@
import org.neo4j.kernel.impl.coreapi.InternalTransaction; import org.neo4j.kernel.impl.coreapi.InternalTransaction;
import org.neo4j.kernel.impl.coreapi.PropertyContainerLocker; import org.neo4j.kernel.impl.coreapi.PropertyContainerLocker;
import org.neo4j.kernel.impl.query.Neo4jTransactionalContextFactory; import org.neo4j.kernel.impl.query.Neo4jTransactionalContextFactory;
import org.neo4j.kernel.impl.query.QueryEngineProvider;
import org.neo4j.kernel.impl.query.TransactionalContext; import org.neo4j.kernel.impl.query.TransactionalContext;
import org.neo4j.kernel.impl.query.TransactionalContextFactory; import org.neo4j.kernel.impl.query.TransactionalContextFactory;
import org.neo4j.kernel.impl.query.clientconnection.ClientConnectionInfo;
import org.neo4j.logging.NullLogProvider; import org.neo4j.logging.NullLogProvider;
import org.neo4j.test.rule.DatabaseRule; import org.neo4j.test.rule.DatabaseRule;
import org.neo4j.test.rule.ImpermanentDatabaseRule; import org.neo4j.test.rule.ImpermanentDatabaseRule;
Expand Down Expand Up @@ -79,6 +79,6 @@ private TransactionalContext createTransactionContext( GraphDatabaseQueryService
{ {
PropertyContainerLocker locker = new PropertyContainerLocker(); PropertyContainerLocker locker = new PropertyContainerLocker();
TransactionalContextFactory contextFactory = Neo4jTransactionalContextFactory.create( graph, locker ); TransactionalContextFactory contextFactory = Neo4jTransactionalContextFactory.create( graph, locker );
return contextFactory.newContext( QueryEngineProvider.describe(), tx, query, Collections.emptyMap() ); return contextFactory.newContext( ClientConnectionInfo.EMBEDDED_CONNECTION, tx, query, Collections.emptyMap() );
} }
} }
Expand Up @@ -92,9 +92,9 @@
import org.neo4j.kernel.impl.coreapi.TopLevelTransaction; import org.neo4j.kernel.impl.coreapi.TopLevelTransaction;
import org.neo4j.kernel.impl.coreapi.schema.SchemaImpl; import org.neo4j.kernel.impl.coreapi.schema.SchemaImpl;
import org.neo4j.kernel.impl.query.Neo4jTransactionalContextFactory; import org.neo4j.kernel.impl.query.Neo4jTransactionalContextFactory;
import org.neo4j.kernel.impl.query.QueryEngineProvider;
import org.neo4j.kernel.impl.query.TransactionalContext; import org.neo4j.kernel.impl.query.TransactionalContext;
import org.neo4j.kernel.impl.query.TransactionalContextFactory; import org.neo4j.kernel.impl.query.TransactionalContextFactory;
import org.neo4j.kernel.impl.query.clientconnection.ClientConnectionInfo;
import org.neo4j.kernel.impl.store.StoreId; import org.neo4j.kernel.impl.store.StoreId;
import org.neo4j.kernel.impl.traversal.BidirectionalTraversalDescriptionImpl; import org.neo4j.kernel.impl.traversal.BidirectionalTraversalDescriptionImpl;
import org.neo4j.kernel.impl.traversal.MonoDirectionalTraversalDescription; import org.neo4j.kernel.impl.traversal.MonoDirectionalTraversalDescription;
Expand Down Expand Up @@ -422,7 +422,7 @@ public Result execute( InternalTransaction transaction, String query, Map<String
throws QueryExecutionException throws QueryExecutionException
{ {
TransactionalContext context = TransactionalContext context =
contextFactory.newContext( QueryEngineProvider.describe(), transaction, query, parameters ); contextFactory.newContext( ClientConnectionInfo.EMBEDDED_CONNECTION, transaction, query, parameters );
return spi.executeQuery( query, parameters, context ); return spi.executeQuery( query, parameters, context );
} }


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


import org.neo4j.helpers.Service; import org.neo4j.helpers.Service;
import org.neo4j.kernel.impl.query.clientconnection.ClientConnectionInfo;
import org.neo4j.kernel.impl.util.Dependencies; import org.neo4j.kernel.impl.util.Dependencies;
import org.neo4j.kernel.internal.GraphDatabaseAPI; import org.neo4j.kernel.internal.GraphDatabaseAPI;


Expand Down Expand Up @@ -60,9 +59,4 @@ public static QueryExecutionEngine noEngine()
{ {
return NoQueryEngine.INSTANCE; return NoQueryEngine.INSTANCE;
} }

public static ClientConnectionInfo describe()
{
return ClientConnectionInfo.EMBEDDED_CONNECTION;
}
} }
Expand Up @@ -22,6 +22,9 @@
import java.net.InetSocketAddress; import java.net.InetSocketAddress;
import java.net.SocketAddress; import java.net.SocketAddress;


/**
* @see ClientConnectionInfo Parent class for documentation and tests.
*/
public class BoltConnectionInfo extends ClientConnectionInfo public class BoltConnectionInfo extends ClientConnectionInfo
{ {
private final String principalName; private final String principalName;
Expand Down
Expand Up @@ -19,6 +19,11 @@
*/ */
package org.neo4j.kernel.impl.query.clientconnection; package org.neo4j.kernel.impl.query.clientconnection;


/**
* This is implemented as an abstract class in order to support different formatting for {@link #asConnectionDetails()},
* when this method is no longer needed, and we move to a standardized format across all types of connections, we can
* turn this class into a simpler value type that just holds the fields that are actually used.
*/
public abstract class ClientConnectionInfo public abstract class ClientConnectionInfo
{ {
/** /**
Expand All @@ -31,19 +36,42 @@ public ClientConnectionInfo withUsername( String username )
return new ConnectionInfoWithUsername( this, username ); return new ConnectionInfoWithUsername( this, username );
} }


/**
* This method provides the custom format for each type of connection.
* <p>
* Preferably we would not need to have a custom format for each type of connection, but this is provided for
* backwards compatibility reasons.
*
* @return a custom log-line format describing this type of connection.
*/
@Deprecated @Deprecated
public abstract String asConnectionDetails(); public abstract String asConnectionDetails();


/**
* This method is overridden in the subclasses where this information is available.
*
* @return the scheme used for connecting to the server, or {@code null} if no scheme is available.
*/
public String requestScheme() public String requestScheme()
{ {
return null; return null;
} }


/**
* This method is overridden in the subclasses where this information is available.
*
* @return the address of the client. or {@code null} if the address is not available.
*/
public String clientAddress() public String clientAddress()
{ {
return null; return null;
} }


/**
* This method is overridden in the subclasses where this information is available.
*
* @return the URI of this server that the client connected to, or {@code null} if the URI is not available.
*/
public String requestURI() public String requestURI()
{ {
return null; return null;
Expand All @@ -58,6 +86,9 @@ public String asConnectionDetails()
} }
}; };


/**
* Should be removed along with {@link #withUsername(String)} and {@link #asConnectionDetails()}.
*/
@Deprecated @Deprecated
private static class ConnectionInfoWithUsername extends ClientConnectionInfo private static class ConnectionInfoWithUsername extends ClientConnectionInfo
{ {
Expand Down
Expand Up @@ -21,6 +21,9 @@


import java.net.InetSocketAddress; import java.net.InetSocketAddress;


/**
* @see ClientConnectionInfo Parent class for documentation and tests.
*/
public class HttpConnectionInfo extends ClientConnectionInfo public class HttpConnectionInfo extends ClientConnectionInfo
{ {
private final String scheme; private final String scheme;
Expand Down
Expand Up @@ -21,6 +21,9 @@


import java.io.Serializable; import java.io.Serializable;


/**
* @see ClientConnectionInfo Parent class for documentation and tests.
*/
public class ShellConnectionInfo extends ClientConnectionInfo public class ShellConnectionInfo extends ClientConnectionInfo
{ {
private final Serializable id; private final Serializable id;
Expand Down
Expand Up @@ -36,7 +36,7 @@
import org.neo4j.kernel.impl.query.TransactionalContextFactory; import org.neo4j.kernel.impl.query.TransactionalContextFactory;
import org.neo4j.kernel.lifecycle.LifecycleAdapter; import org.neo4j.kernel.lifecycle.LifecycleAdapter;
import org.neo4j.logging.LogProvider; import org.neo4j.logging.LogProvider;
import org.neo4j.server.rest.web.ServerQuerySession; import org.neo4j.server.rest.web.HttpConnectionInfoFactory;


import org.neo4j.logging.Log; import org.neo4j.logging.Log;


Expand Down Expand Up @@ -84,7 +84,7 @@ public TransactionalContext createTransactionContext( String query, Map<String,
HttpServletRequest request ) HttpServletRequest request )
{ {
InternalTransaction tx = getInternalTransaction( request ); InternalTransaction tx = getInternalTransaction( request );
return contextFactory.newContext( ServerQuerySession.describe( request ), tx, query, parameters ); return contextFactory.newContext( HttpConnectionInfoFactory.create( request ), tx, query, parameters );
} }


private InternalTransaction getInternalTransaction( HttpServletRequest request ) private InternalTransaction getInternalTransaction( HttpServletRequest request )
Expand Down
Expand Up @@ -33,7 +33,7 @@
import org.neo4j.kernel.impl.query.clientconnection.ClientConnectionInfo; import org.neo4j.kernel.impl.query.clientconnection.ClientConnectionInfo;
import org.neo4j.kernel.impl.query.TransactionalContext; import org.neo4j.kernel.impl.query.TransactionalContext;
import org.neo4j.kernel.impl.query.TransactionalContextFactory; import org.neo4j.kernel.impl.query.TransactionalContextFactory;
import org.neo4j.server.rest.web.ServerQuerySession; import org.neo4j.server.rest.web.HttpConnectionInfoFactory;


public class TransitionalPeriodTransactionMessContainer public class TransitionalPeriodTransactionMessContainer
{ {
Expand Down Expand Up @@ -68,7 +68,7 @@ public TransactionalContext create(
Map<String, Object> queryParameters) Map<String, Object> queryParameters)
{ {
TransactionalContextFactory contextFactory = Neo4jTransactionalContextFactory.create( service, locker ); TransactionalContextFactory contextFactory = Neo4jTransactionalContextFactory.create( service, locker );
ClientConnectionInfo clientConnection = ServerQuerySession.describe( request ); ClientConnectionInfo clientConnection = HttpConnectionInfoFactory.create( request );
InternalTransaction transaction = service.beginTransaction( type, securityContext ); InternalTransaction transaction = service.beginTransaction( type, securityContext );
return contextFactory.newContext( clientConnection, transaction, query, queryParameters ); return contextFactory.newContext( clientConnection, transaction, query, queryParameters );
} }
Expand Down
Expand Up @@ -27,9 +27,9 @@


import static javax.ws.rs.core.HttpHeaders.USER_AGENT; import static javax.ws.rs.core.HttpHeaders.USER_AGENT;


public class ServerQuerySession public class HttpConnectionInfoFactory
{ {
public static ClientConnectionInfo describe( HttpServletRequest request ) public static ClientConnectionInfo create( HttpServletRequest request )
{ {
return new HttpConnectionInfo( return new HttpConnectionInfo(
request.getScheme(), request.getScheme(),
Expand Down
Expand Up @@ -35,7 +35,6 @@
import org.neo4j.kernel.impl.query.Neo4jTransactionalContextFactory; import org.neo4j.kernel.impl.query.Neo4jTransactionalContextFactory;
import org.neo4j.kernel.impl.query.QueryExecutionEngine; import org.neo4j.kernel.impl.query.QueryExecutionEngine;
import org.neo4j.kernel.impl.query.QueryExecutionKernelException; import org.neo4j.kernel.impl.query.QueryExecutionKernelException;
import org.neo4j.kernel.impl.query.clientconnection.ClientConnectionInfo;
import org.neo4j.kernel.impl.query.TransactionalContext; import org.neo4j.kernel.impl.query.TransactionalContext;
import org.neo4j.kernel.impl.query.TransactionalContextFactory; import org.neo4j.kernel.impl.query.TransactionalContextFactory;
import org.neo4j.kernel.impl.query.clientconnection.ShellConnectionInfo; import org.neo4j.kernel.impl.query.clientconnection.ShellConnectionInfo;
Expand Down Expand Up @@ -200,18 +199,10 @@ private TransactionalContext createTransactionContext( String queryText, Map<Str
InternalTransaction transaction = InternalTransaction transaction =
graph.beginTransaction( KernelTransaction.Type.implicit, SecurityContext.AUTH_DISABLED ); graph.beginTransaction( KernelTransaction.Type.implicit, SecurityContext.AUTH_DISABLED );
return contextFactory.newContext( return contextFactory.newContext(
ShellQuerySession.describe( session ), new ShellConnectionInfo( session.getId() ),
transaction, transaction,
queryText, queryText,
queryParameters queryParameters
); );
} }

private static class ShellQuerySession
{
public static ClientConnectionInfo describe( Session session )
{
return new ShellConnectionInfo( session.getId() );
}
}
} }
Expand Up @@ -144,7 +144,7 @@ public void shouldContainSpecificConnectionDetails() throws Exception
// then // then
assertThat( data, hasKey( "requestScheme" ) ); assertThat( data, hasKey( "requestScheme" ) );
assertThat( data, hasKey( "clientAddress" ) ); assertThat( data, hasKey( "clientAddress" ) );
assertThat( data, hasKey( "requestURI" ) ); assertThat( data, hasKey( "requestUri" ) );
} }


private Map<String,Object> getQueryListing( String query ) private Map<String,Object> getQueryListing( String query )
Expand Down
Expand Up @@ -49,6 +49,7 @@
import org.neo4j.kernel.enterprise.api.security.EnterpriseSecurityContext; import org.neo4j.kernel.enterprise.api.security.EnterpriseSecurityContext;
import org.neo4j.kernel.impl.coreapi.InternalTransaction; import org.neo4j.kernel.impl.coreapi.InternalTransaction;
import org.neo4j.kernel.impl.factory.GraphDatabaseFacade; import org.neo4j.kernel.impl.factory.GraphDatabaseFacade;
import org.neo4j.kernel.impl.query.clientconnection.ClientConnectionInfo;
import org.neo4j.logging.AssertableLogProvider; import org.neo4j.logging.AssertableLogProvider;
import org.neo4j.server.security.enterprise.auth.EmbeddedInteraction; import org.neo4j.server.security.enterprise.auth.EmbeddedInteraction;
import org.neo4j.test.TestEnterpriseGraphDatabaseFactory; import org.neo4j.test.TestEnterpriseGraphDatabaseFactory;
Expand Down Expand Up @@ -226,7 +227,7 @@ public void shouldLogParametersWhenNestedMap() throws Exception


private String clientConnectionInfo() private String clientConnectionInfo()
{ {
return QueryEngineProvider.describe().withUsername( AUTH_DISABLED.username() ).asConnectionDetails(); return ClientConnectionInfo.EMBEDDED_CONNECTION.withUsername( AUTH_DISABLED.username() ).asConnectionDetails();
} }


@Test @Test
Expand Down

0 comments on commit ddb0c21

Please sign in to comment.