Refactoring (#1476)

Author: injectives

CONTRIBUTING.md

@@ -47,7 +47,7 @@ If you want to contribute a pull request, we have a little bit of process you'll
 - Take a moment to read and sign our [CLA](http://neo4j.com/developer/cla)
 
 We can't guarantee that we'll accept pull requests and may ask you to make some changes before they go in.
-Occasionally, we might also have logistical, commercial, or legal reasons why we can't accept your work but we'll try to find an alternative way for you to contribute in that case.
+Occasionally, we might also have logistical, commercial, or legal reasons why we can't accept your work, but we'll try to find an alternative way for you to contribute in that case.
 Remember that many community members have become regular contributors and some are now even Neo employees!
 
 

ISSUE_TEMPLATE.md

@@ -15,15 +15,15 @@ To help us understand your issue, please specify important details, primarily:
 
 - Neo4j version: Community/Enterprise X.Y.Z
 - Neo4j Mode: Single instance/HA cluster with X members/Casual cluster with X core Y read-replica
-- Driver version: X lanaguage driver X.Y.Z (If you use some other library that wraps around this driver, you might want to raise an issue there first)
+- Driver version: X language driver X.Y.Z (If you use some other library that wraps around this driver, you might want to raise an issue there first)
 - Operating system: (for example Windows 10/Ubuntu 16.10 on AWS)
 - **Steps to reproduce**
 - Expected behavior
 - Actual behavior
 
 Additionally, include (as appropriate) log-files, stacktraces, and other debug output.
 Always check the server logs to see if there is any stacktrace related to the driver error too.
-Aslo add any solutions you've tried to solve the problem yourself.
+Also, add any solutions you've tried to solve the problem yourself.
 
 ## Example bug report
 
@@ -35,7 +35,7 @@ I got connection reset by peer errors.
 **Operating System:** Ubuntu 16.10 on AWS  
 
 ### Steps to reproduce
-1. Start Neo4j on a AWS instance
+1. Start Neo4j on an AWS instance
 2. Run a query with the driver
 3. Put the driver idle for 2h
 4. Run another query

README.md

@@ -142,7 +142,7 @@ There are 2 ways of running Testkit tests:
 
 ##### Using the testkit-tests module
 
-The `testkit-tests` module will automatically checkout Testkit and run it during Maven build.
+The `testkit-tests` module will automatically check out Testkit and run it during Maven build.
 
 Prerequisites:
 - Docker
@@ -189,7 +189,7 @@ TEST_DOCKER_RMI=true \
 python3 main.py --tests TESTKIT_TESTS UNIT_TESTS --configs 4.3-enterprise
 ````
 
-On Windows or in the abscence of a Bash-compatible environment, the required steps are probably different.
+On Windows or in the absence of a Bash-compatible environment, the required steps are probably different.
 A simple `mvn clean install` will require admin rights on Windows, because our integration tests require admin privileges to install and start a service.
 
 If all of this fails and you only want to try out a local development version of the driver, you could skip all tests like this:

driver/pom.xml

@@ -69,7 +69,7 @@
     <!-- Test dependencies -->
     <dependency>
       <groupId>org.hamcrest</groupId>
-      <artifactId>hamcrest-junit</artifactId>
+      <artifactId>hamcrest</artifactId>
     </dependency>
     <dependency>
       <groupId>org.mockito</groupId>

driver/src/main/java/org/neo4j/driver/Config.java

@@ -390,6 +390,7 @@ public ConfigBuilder withLogging(Logging logging) {
          *
          * @return this builder
          */
+        @SuppressWarnings("deprecation")
         public ConfigBuilder withLeakedSessionsLogging() {
             this.logLeakedSessions = true;
             return this;

driver/src/main/java/org/neo4j/driver/Driver.java

@@ -377,6 +377,7 @@ default AsyncSession asyncSession(SessionConfig sessionConfig) {
      */
     @Experimental
     @Deprecated
+    @SuppressWarnings("SameReturnValue")
     TypeSystem defaultTypeSystem();
 
     /**

driver/src/main/java/org/neo4j/driver/Logging.java

@@ -143,6 +143,7 @@ static Logging console(Level level) {
      *
      * @return new logging implementation.
      */
+    @SuppressWarnings("SameReturnValue")
     static Logging none() {
         return DEV_NULL_LOGGING;
     }

driver/src/main/java/org/neo4j/driver/exceptions/AuthTokenManagerExecutionException.java

@@ -29,7 +29,7 @@
  * <ul>
  *     <li>{@link AuthTokenManager#getToken()} returned {@code null}</li>
  *     <li>{@link AuthTokenManager#getToken()} returned a {@link java.util.concurrent.CompletionStage} that completed with {@code null}</li>
- *     <li>{@link AuthTokenManager#getToken()} returned a {@link java.util.concurrent.CompletionStage} that completed with a token that was not creeated using {@link org.neo4j.driver.AuthTokens}</li>
+ *     <li>{@link AuthTokenManager#getToken()} returned a {@link java.util.concurrent.CompletionStage} that completed with a token that was not created using {@link org.neo4j.driver.AuthTokens}</li>
  *     <li>{@link AuthTokenManager#getToken()} has thrown an exception</li>
  *     <li>{@link AuthTokenManager#getToken()} returned a {@link java.util.concurrent.CompletionStage} that completed exceptionally</li>
  * </ul>

driver/src/main/java/org/neo4j/driver/internal/InternalPath.java

@@ -89,7 +89,7 @@ public InternalPath(List<Entity> alternatingNodeAndRel) {
                 // even index - this should be a node
                 try {
                     lastNode = (Node) entity;
-                    if (nodes.isEmpty() || isEndpoint(lastNode, lastRelationship)) {
+                    if (nodes.isEmpty() || (lastRelationship != null && isEndpoint(lastNode, lastRelationship))) {
                         nodes.add(lastNode);
                     } else {
                         throw new IllegalArgumentException("Node argument " + index

driver/src/main/java/org/neo4j/driver/internal/async/connection/NettyDomainNameResolverGroup.java

@@ -32,6 +32,7 @@ public NettyDomainNameResolverGroup(DomainNameResolver domainNameResolver) {
     }
 
     @Override
+    @SuppressWarnings("resource")
     protected AddressResolver<InetSocketAddress> newResolver(EventExecutor executor) {
         return new NettyDomainNameResolver(executor, domainNameResolver).asAddressResolver();
     }

driver/src/main/java/org/neo4j/driver/internal/cluster/RediscoveryImpl.java

@@ -310,7 +310,7 @@ private CompletionStage<ClusterComposition> lookupOnRouter(
                 });
     }
 
-    @SuppressWarnings("ThrowableNotThrown")
+    @SuppressWarnings({"ThrowableNotThrown", "SameReturnValue"})
     private ClusterComposition handleRoutingProcedureError(
             Throwable error, RoutingTable routingTable, BoltServerAddress routerAddress, Throwable baseError) {
         if (mustAbortDiscovery(error)) {

driver/src/main/java/org/neo4j/driver/internal/logging/DevNullLogging.java

@@ -43,6 +43,7 @@ public Logger getLog(String name) {
     // Reference: https://docs.oracle.com/en/java/javase/17/docs/specs/serialization/input.html#the-readresolve-method
     // andJoshua Bloch, Effective Java 3rd edition
     @Serial
+    @SuppressWarnings("SameReturnValue")
     private Object readResolve() {
         return DEV_NULL_LOGGING;
     }

driver/src/main/java/org/neo4j/driver/internal/packstream/PackStream.java

@@ -516,6 +516,7 @@ public String unpackString() throws IOException {
          * @return null
          * @throws IOException if the unpacked value was not null
          */
+        @SuppressWarnings("SameReturnValue")
         public Object unpackNull() throws IOException {
             final var markerByte = in.readByte();
             if (markerByte != NULL) {

driver/src/main/java/org/neo4j/driver/internal/retry/ExponentialBackoffRetryLogic.java

@@ -186,7 +186,9 @@ private Retry exponentialBackoffRetryRx() {
             if (canRetryOn(error)) {
                 var currentTime = clock.millis();
 
+                @SuppressWarnings("DataFlowIssue")
                 long startTime = contextView.getOrDefault("startTime", currentTime);
+                @SuppressWarnings("DataFlowIssue")
                 long nextDelayMs = contextView.getOrDefault("nextDelayMs", initialRetryDelayMs);
 
                 var elapsedTime = currentTime - startTime;
@@ -213,6 +215,7 @@ private Retry exponentialBackoffRetryRx() {
             }
             addSuppressed(throwable, errors);
 
+            //noinspection DataFlowIssue
             return Mono.error(throwable);
         })));
     }

driver/src/main/java/org/neo4j/driver/reactivestreams/ReactiveResult.java

@@ -51,21 +51,21 @@ public interface ReactiveResult {
      * Returns a cold unicast publisher of records.
      * <p>
      * When the record publisher is {@linkplain Publisher#subscribe(Subscriber) subscribed}, the query is executed and the result is streamed back as a record
-     * stream followed by a result summary. This record publisher publishes all records in the result and signals the completion. However before completion or
+     * stream followed by a result summary. This record publisher publishes all records in the result and signals the completion. However, before completion or
      * error reporting if any, a cleanup of result resources such as network connection will be carried out automatically.
      * <p>
-     * Therefore the {@link Subscriber} of this record publisher shall wait for the termination signal (complete or error) to ensure that the resources used by
+     * Therefore, the {@link Subscriber} of this record publisher shall wait for the termination signal (complete or error) to ensure that the resources used by
      * this result are released correctly. Then the session is ready to be used to run more queries.
      * <p>
      * Cancelling of the record streaming will immediately terminate the propagation of new records. But it will not cancel query execution on the server. When
      * the execution is finished, the {@link Subscriber} will be notified with a termination signal (complete or error).
      * <p>
-     * The record publishing event by default runs in an Network IO thread, as a result no blocking operation is allowed in this thread. Otherwise network IO
+     * The record publishing event by default runs in a Network IO thread, as a result no blocking operation is allowed in this thread. Otherwise, network IO
      * might be blocked by application logic.
      * <p>
      * This publisher can only be subscribed by one {@link Subscriber} once.
      * <p>
-     * If this publisher is subscribed after {@link #keys()}, then the publish of records is carried out after the arrival of keys. If this publisher is
+     * If this publisher is subscribed after {@link #keys()}, then the publishing of records is carried out after the arrival of keys. If this publisher is
      * subscribed after {@link #consume()}, then a {@link ResultConsumedException} will be thrown.
      *
      * @return a cold unicast publisher of records.

driver/src/main/java/org/neo4j/driver/reactivestreams/ReactiveSession.java

@@ -44,7 +44,7 @@ public interface ReactiveSession extends BaseSession, ReactiveQueryRunner {
      * Begin a new <em>unmanaged {@linkplain ReactiveTransaction transaction}</em>. At most one transaction may exist in a session at any point in time. To
      * maintain multiple concurrent transactions, use multiple concurrent sessions.
      * <p>
-     * It by default is executed in a Network IO thread, as a result no blocking operation is allowed in this thread.
+     * It is by default is executed in a Network IO thread, as a result no blocking operation is allowed in this thread.
      *
      * @return a new {@link ReactiveTransaction}
      */
@@ -56,7 +56,7 @@ default Publisher<ReactiveTransaction> beginTransaction() {
      * Begin a new <em>unmanaged {@linkplain ReactiveTransaction transaction}</em> with the specified {@link TransactionConfig configuration}. At most one
      * transaction may exist in a session at any point in time. To maintain multiple concurrent transactions, use multiple concurrent sessions.
      * <p>
-     * It by default is executed in a Network IO thread, as a result no blocking operation is allowed in this thread.
+     * It is by default is executed in a Network IO thread, as a result no blocking operation is allowed in this thread.
      *
      * @param config configuration for the new transaction.
      * @return a new {@link ReactiveTransaction}

driver/src/main/java/org/neo4j/driver/summary/Plan.java

@@ -28,7 +28,7 @@
  * This can be extremely helpful in understanding what a query is doing, and how to optimize it. For more
  * details, see the Neo4j Manual.
  * <p>
- * The plan for the query is a tree of plans - each sub-tree containing zero or more child plans. The query
+ * The plan for the query is a tree of plans - each subtree containing zero or more child plans. The query
  * starts with the root plan. Each sub-plan is of a specific {@link #operatorType() operator type}, which describes
  * what that part of the plan does - for instance, perform an index lookup or filter results. The Neo4j Manual contains
  * a reference of the available operator types, and these may differ across Neo4j versions.

driver/src/main/java/org/neo4j/driver/summary/SummaryCounters.java

@@ -27,7 +27,7 @@
 @Immutable
 public interface SummaryCounters {
     /**
-     * Whether there were any updates at all, eg. any of the counters are greater than 0.
+     * Whether there were any updates at all, e.g. any of the counters are greater than 0.
      * @return true if the query made any updates
      */
     boolean containsUpdates();

driver/src/main/java/org/neo4j/driver/types/MapAccessor.java

@@ -28,7 +28,7 @@
 /**
  * Access the keys, properties and values of an underlying unordered map by key
  * <p>
- * This provides only read methods. Subclasses may chose to provide additional methods
+ * This provides only read methods. Subclasses may choose to provide additional methods
  * for changing the underlying map.
  * @since 1.0
  */
@@ -96,7 +96,7 @@ public interface MapAccessor {
      * as {@link Values#ofBoolean()}, {@link Values#ofList(Function)}.
      * @param <T> the type of map values
      * @see Values for a long list of built-in conversion functions
-     * @return the value as a map from string keys to values of type T obtained from mapping he original map values, if possible
+     * @return the value as a map from string keys to values of type T obtained from mapping the original map values, if possible
      */
     <T> Map<String, T> asMap(Function<Value, T> mapFunction);
 }

driver/src/main/java/org/neo4j/driver/types/MapAccessorWithDefaultValue.java

@@ -24,7 +24,7 @@
 import org.neo4j.driver.Value;
 
 /**
- * Provides methods to access the value of an underlying unordered map by key. When calling the methods, a user need to provides a default value, which will be
+ * Provides methods to access the value of an underlying unordered map by key. When calling the methods, a user need to provide a default value, which will be
  * given back if no match found by the key provided. The default value also servers the purpose of specifying the return type of the value found in map by key.
  * If the type of the value found A differs from the type of the default value B, a cast from A to B would happen automatically. Note: Error might arise if the
  * cast from A to B is not possible.

driver/src/main/java/org/neo4j/driver/types/Point.java

@@ -55,5 +55,6 @@ public interface Point {
      *
      * @return the {@code z} coordinate value or {@link Double#NaN} if not applicable.
      */
+    @SuppressWarnings("SameReturnValue")
     double z();
 }

driver/src/main/java/org/neo4j/driver/types/Relationship.java

@@ -24,7 +24,7 @@
  */
 public interface Relationship extends Entity {
     /**
-     * Id of the node where this relationship starts.
+     * The id of the node where this relationship starts.
      * <p>
      * Please note that depending on server configuration numeric id might not be available and accessing it will result in {@link IllegalStateException}.
      *
@@ -36,14 +36,14 @@ public interface Relationship extends Entity {
     long startNodeId();
 
     /**
-     * Id of the node where this relationship starts.
+     * The id of the node where this relationship starts.
      *
      * @return the node id
      */
     String startNodeElementId();
 
     /**
-     * Id of the node where this relationship ends.
+     * The id of the node where this relationship ends.
      * <p>
      * Please note that depending on server configuration numeric id might not be available and accessing it will result in {@link IllegalStateException}.
      *
@@ -55,7 +55,7 @@ public interface Relationship extends Entity {
     long endNodeId();
 
     /**
-     * Id of the node where this relationship ends.
+     * The id of the node where this relationship ends.
      *
      * @return the node id
      */

driver/src/main/java/org/neo4j/driver/types/Type.java

@@ -22,7 +22,7 @@
 import org.neo4j.driver.util.Immutable;
 
 /**
- * The type of a {@link Value} as defined by the Cypher language
+ * The type of {@link Value} as defined by the Cypher language
  * @since 1.0
  */
 @Immutable

driver/src/main/java/org/neo4j/driver/types/TypeSystem.java

@@ -33,6 +33,7 @@ public interface TypeSystem {
      *
      * @return instance of type system
      */
+    @SuppressWarnings("SameReturnValue")
     static TypeSystem getDefault() {
         return TYPE_SYSTEM;
     }

driver/src/test/java/org/neo4j/driver/AuthTokensTest.java

@@ -19,8 +19,8 @@
 package org.neo4j.driver;
 
 import static java.util.Arrays.asList;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.IsEqual.equalTo;
-import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.neo4j.driver.AuthTokens.basic;

driver/src/test/java/org/neo4j/driver/GraphDatabaseTest.java

@@ -19,8 +19,8 @@
 package org.neo4j.driver;
 
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;

driver/src/test/java/org/neo4j/driver/ParametersTest.java

@@ -20,8 +20,8 @@
 
 import static java.util.Collections.singletonList;
 import static java.util.Collections.singletonMap;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.startsWith;
-import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;

driver/src/test/java/org/neo4j/driver/QueryConfigTest.java

@@ -56,6 +56,7 @@ void shouldUpdateRouting(RoutingControl routing) {
     }
 
     @Test
+    @SuppressWarnings("WriteOnlyObject")
     void shouldNotAllowNullRouting() {
         assertThrows(NullPointerException.class, () -> QueryConfig.builder().withRouting(null));
     }

driver/src/test/java/org/neo4j/driver/QueryTest.java

@@ -18,8 +18,8 @@
  */
 package org.neo4j.driver;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.neo4j.driver.Values.parameters;
 

driver/src/test/java/org/neo4j/driver/integration/ChannelConnectorImplIT.java

@@ -19,9 +19,9 @@
 package org.neo4j.driver.integration;
 
 import static java.util.concurrent.CompletableFuture.runAsync;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.startsWith;
-import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;

driver/src/test/java/org/neo4j/driver/integration/ConnectionPoolIT.java

@@ -19,8 +19,8 @@
 package org.neo4j.driver.integration;
 
 import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
-import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertThrows;

driver/src/test/java/org/neo4j/driver/integration/CredentialsIT.java

@@ -19,9 +19,9 @@
 package org.neo4j.driver.integration;
 
 import static java.util.Collections.singletonMap;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.neo4j.driver.AuthTokens.basic;
 import static org.neo4j.driver.AuthTokens.custom;

driver/src/test/java/org/neo4j/driver/integration/DirectDriverIT.java

@@ -18,9 +18,9 @@
  */
 package org.neo4j.driver.integration;
 
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.core.IsEqual.equalTo;
-import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.neo4j.driver.internal.util.Matchers.directDriverWithAddress;