From f70712e99dad4a63541d5972458b6234758d3467 Mon Sep 17 00:00:00 2001 From: fickludd Date: Thu, 15 Sep 2016 10:29:20 +0200 Subject: [PATCH] Added PR feedback, including lazy remote host lookup --- .../org/neo4j/kernel/api/ExecutingQuery.java | 2 +- .../impl/query/Neo4jTransactionalContext.java | 5 ----- .../neo4j/kernel/impl/query/QuerySource.java | 22 +++---------------- .../query/Neo4jTransactionalContextTest.java | 6 ----- .../rest/web/InternalJettyServletRequest.java | 10 +++++---- .../server/rest/web/ServerQuerySession.java | 2 +- .../rest/batch/BatchOperationsTest.java | 5 +++-- .../query/QueryLoggerKernelExtension.java | 6 ----- 8 files changed, 14 insertions(+), 44 deletions(-) diff --git a/community/kernel/src/main/java/org/neo4j/kernel/api/ExecutingQuery.java b/community/kernel/src/main/java/org/neo4j/kernel/api/ExecutingQuery.java index 4c107bbc81cb2..5f27f8a31d1c6 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/api/ExecutingQuery.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/api/ExecutingQuery.java @@ -123,6 +123,6 @@ public String toString() return format( "ExecutingQuery{queryId=%d, querySource='%s', username='%s', queryText='%s', queryParameters=%s, " + "startTime=%d}", - queryId, querySource.toString( ':' ), usernameAsString(), queryText, queryParameters, startTime ); + queryId, querySource.toString( ":" ), usernameAsString(), queryText, queryParameters, startTime ); } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContext.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContext.java index 0044d63fe1d97..59919b24b6dfc 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContext.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContext.java @@ -267,9 +267,4 @@ public AccessMode accessMode() { return mode; } - - public static Function describe() - { - return transactionalContext -> "We didn't really care to describe this for you in this test"; - } } diff --git a/community/kernel/src/main/java/org/neo4j/kernel/impl/query/QuerySource.java b/community/kernel/src/main/java/org/neo4j/kernel/impl/query/QuerySource.java index 3c0183727fba0..c6a1c7c4e995e 100644 --- a/community/kernel/src/main/java/org/neo4j/kernel/impl/query/QuerySource.java +++ b/community/kernel/src/main/java/org/neo4j/kernel/impl/query/QuerySource.java @@ -41,27 +41,11 @@ public QuerySource append( String newPart ) @Override public String toString() { - return toString( '\t' ); + return toString( "\t" ); } - public String toString( Character sep ) + public String toString( String sep ) { - StringBuilder builder = new StringBuilder( ); - - boolean isFirst = true; - for ( String part : parts ) - { - if ( isFirst ) - { - isFirst = false; - } - else - { - builder.append( sep ); - } - builder.append( part ); - } - - return builder.toString(); + return String.join( sep, parts ); } } diff --git a/community/kernel/src/test/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContextTest.java b/community/kernel/src/test/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContextTest.java index 987c8f17f630a..c7100727cf0e0 100644 --- a/community/kernel/src/test/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContextTest.java +++ b/community/kernel/src/test/java/org/neo4j/kernel/impl/query/Neo4jTransactionalContextTest.java @@ -69,18 +69,12 @@ public void checkKernelStatementOnCheck() throws Exception KernelTransaction.Type transactionType = null; AccessMode transactionMode = null; ExecutingQuery executingQuery = null; - PropertyContainerLocker locker = null; DbmsOperations.Factory dbmsOperationsFactory = null; ThreadToStatementContextBridge txBridge = null; Neo4jTransactionalContext transactionalContext = -//<<<<<<< HEAD -// new Neo4jTransactionalContext( -// databaseQueryService, transaction, statement, "", Collections.emptyMap(), propertyContainerLocker ); -//======= new Neo4jTransactionalContext( databaseQueryService, transaction, transactionType, transactionMode, statement, executingQuery, propertyContainerLocker, txBridge, dbmsOperationsFactory, guard ); -//>>>>>>> b556d13... Moved functionality from QuerySession to TransactionContext transactionalContext.check(); diff --git a/community/server/src/main/java/org/neo4j/server/rest/web/InternalJettyServletRequest.java b/community/server/src/main/java/org/neo4j/server/rest/web/InternalJettyServletRequest.java index 4f47208892dd1..e0e032acaa4aa 100644 --- a/community/server/src/main/java/org/neo4j/server/rest/web/InternalJettyServletRequest.java +++ b/community/server/src/main/java/org/neo4j/server/rest/web/InternalJettyServletRequest.java @@ -35,6 +35,7 @@ import java.util.Enumeration; import java.util.HashMap; import java.util.Map; +import java.util.function.Supplier; import javax.servlet.DispatcherType; import javax.servlet.ReadListener; import javax.servlet.ServletInputStream; @@ -43,6 +44,7 @@ import javax.servlet.http.HttpServletResponse; import javax.ws.rs.core.MediaType; +import org.neo4j.function.Suppliers; import org.neo4j.string.UTF8; public class InternalJettyServletRequest extends Request @@ -216,7 +218,7 @@ public String getRemoteAddr() @Override public String getRemoteHost() { - return requestData.remoteHost; + return requestData.remoteHost.get(); } @Override @@ -362,7 +364,7 @@ public HttpChannelState getHttpChannelState() public static class RequestData { public final String remoteAddress; - public final String remoteHost; + public final Supplier remoteHost; public final boolean isSecure; public final int remotePort; public final String localName; @@ -372,7 +374,7 @@ public static class RequestData public RequestData( String remoteAddress, - String remoteHost, + Supplier remoteHost, boolean isSecure, int remotePort, String localName, @@ -394,7 +396,7 @@ public static RequestData from( HttpServletRequest req ) { return new RequestData( req.getRemoteAddr(), - req.getRemoteHost(), + Suppliers.lazySingleton( req::getRemoteHost ), req.isSecure(), req.getRemotePort(), req.getLocalName(), diff --git a/community/server/src/main/java/org/neo4j/server/rest/web/ServerQuerySession.java b/community/server/src/main/java/org/neo4j/server/rest/web/ServerQuerySession.java index 79689640d6e35..c45e216cdf0d4 100644 --- a/community/server/src/main/java/org/neo4j/server/rest/web/ServerQuerySession.java +++ b/community/server/src/main/java/org/neo4j/server/rest/web/ServerQuerySession.java @@ -29,6 +29,6 @@ public static QuerySource describe( HttpServletRequest request ) { return request == null ? new QuerySource( "server-session" ) : - new QuerySource( "server-session", "http", request.getRemoteAddr(), request.getRequestURI() ); + new QuerySource( "server-session", request.getScheme(), request.getRemoteAddr(), request.getRequestURI() ); } } diff --git a/community/server/src/test/java/org/neo4j/server/rest/batch/BatchOperationsTest.java b/community/server/src/test/java/org/neo4j/server/rest/batch/BatchOperationsTest.java index 8c8d3aa9beb5c..6c6920bf1e6e0 100644 --- a/community/server/src/test/java/org/neo4j/server/rest/batch/BatchOperationsTest.java +++ b/community/server/src/test/java/org/neo4j/server/rest/batch/BatchOperationsTest.java @@ -34,6 +34,7 @@ import org.neo4j.server.rest.web.InternalJettyServletResponse; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; public class BatchOperationsTest { @@ -83,7 +84,7 @@ public void shouldForwardMetadataFromRequestData() throws Exception { // Given RequestData mock = new RequestData( - "127.0.0.1", "localhost", true, 1, + "127.0.0.1", () -> "localhost", true, 1, "TheLocalName", "129.0.0.1", 2, "authorization/auth" ); InternalJettyServletRequest req = new InternalJettyServletRequest( "POST", @@ -93,7 +94,7 @@ public void shouldForwardMetadataFromRequestData() throws Exception // When & then assertEquals( "127.0.0.1", req.getRemoteAddr()); assertEquals( "localhost", req.getRemoteHost()); - assertEquals( true, req.isSecure() ); + assertTrue( req.isSecure() ); assertEquals( 1, req.getRemotePort()); assertEquals( "TheLocalName", req.getLocalName()); assertEquals( "129.0.0.1", req.getLocalAddr()); diff --git a/enterprise/query-logging/src/main/java/org/neo4j/kernel/impl/query/QueryLoggerKernelExtension.java b/enterprise/query-logging/src/main/java/org/neo4j/kernel/impl/query/QueryLoggerKernelExtension.java index 2fc9451a958a6..105f8ce8b6d2e 100644 --- a/enterprise/query-logging/src/main/java/org/neo4j/kernel/impl/query/QueryLoggerKernelExtension.java +++ b/enterprise/query-logging/src/main/java/org/neo4j/kernel/impl/query/QueryLoggerKernelExtension.java @@ -225,11 +225,5 @@ private static String mapAsString( Map params ) return builder.toString(); } - - @SuppressWarnings( "unchecked" ) - private static Class> paramsClass() - { - return (Class>) (Object) Map.class; - } } }