Minimal RocksJava compliance with Java 8 language level (EB 1046) (#1…

…0951)

Summary:
Apply a small (and automatic) set of IntelliJ Java inspections/repairs to the Java interface to RocksDB Java and its tests.
Partly enabled by the fact that we now (from RocksDB7) require java 8.

Explicit <p> in empty lines in javadoc comments.

Parameters and variables made final where possible.
Anonymous subclasses converted lambdas.

Some tests which previously used other assertion models were converted to assertj, e.g. (assertThat(actual).isEqualTo(expected)

In a very few cases tests were found to be inoperative or broken, and were repaired. No problems with actual RocksDB behaviour were observed.

This PR is intended to replace #9618 - that PR was not merged, and attempts to rebase it have yielded a questionable looking diff, so we choose to go back to square 1 here, and implement a conservative set of changes.

Pull Request resolved: #10951

Reviewed By: anand1976

Differential Revision: D45057849

Pulled By: ajkr

fbshipit-source-id: e4ea46bfc80518ae86f37702b03ca9352bc11c3d

java/src/main/java/org/rocksdb/AbstractCompactionFilter.java

@@ -7,8 +7,8 @@
 /**
  * A CompactionFilter allows an application to modify/delete a key-value at
  * the time of compaction.
- *
- * At present we just permit an overriding Java class to wrap a C++
+ * <p>
+ * At present, we just permit an overriding Java class to wrap a C++
  * implementation
  */
 public abstract class AbstractCompactionFilter<T extends AbstractSlice<?>>
@@ -49,10 +49,10 @@ protected AbstractCompactionFilter(final long nativeHandle) {
 
   /**
    * Deletes underlying C++ compaction pointer.
-   *
+   * <p>
    * Note that this function should be called only after all
    * RocksDB instances referencing the compaction filter are closed.
-   * Otherwise an undefined behavior will occur.
+   * Otherwise, an undefined behavior will occur.
    */
   @Override
   protected final native void disposeInternal(final long handle);

java/src/main/java/org/rocksdb/AbstractCompactionFilterFactory.java

@@ -15,7 +15,7 @@ public abstract class AbstractCompactionFilterFactory<T extends AbstractCompacti
     extends RocksCallbackObject {
 
   public AbstractCompactionFilterFactory() {
-    super(null);
+    super(0L);
   }
 
   @Override
@@ -55,7 +55,7 @@ public abstract T createCompactionFilter(
 
   /**
    * A name which identifies this compaction filter
-   *
+   * <p>
    * The name will be printed to the LOG file on start up for diagnosis
    *
    * @return name which identifies this compaction filter.

java/src/main/java/org/rocksdb/AbstractComparator.java

@@ -31,7 +31,7 @@ protected long initializeNative(final long... nativeParameterHandles) {
 
   /**
    * Get the type of this comparator.
-   *
+   * <p>
    * Used for determining the correct C++ cast in native code.
    *
    * @return The type of the comparator.
@@ -44,11 +44,11 @@ ComparatorType getComparatorType() {
    * The name of the comparator.  Used to check for comparator
    * mismatches (i.e., a DB created with one comparator is
    * accessed using a different comparator).
-   *
+   * <p>
    * A new name should be used whenever
    * the comparator implementation changes in a way that will cause
    * the relative ordering of any two keys to change.
-   *
+   * <p>
    * Names starting with "rocksdb." are reserved and should not be used.
    *
    * @return The name of this comparator implementation

java/src/main/java/org/rocksdb/AbstractComparatorJniBridge.java

@@ -18,108 +18,102 @@
  * {@link org.rocksdb.AbstractComparator} clean.
  */
 class AbstractComparatorJniBridge {
+  /**
+   * Only called from JNI.
+   * <p>
+   * Simply a bridge to calling
+   * {@link AbstractComparator#compare(ByteBuffer, ByteBuffer)},
+   * which ensures that the byte buffer lengths are correct
+   * before and after the call.
+   *
+   * @param comparator the comparator object on which to
+   *     call {@link AbstractComparator#compare(ByteBuffer, ByteBuffer)}
+   * @param a buffer access to first key
+   * @param aLen the length of the a key,
+   *     may be smaller than the buffer {@code a}
+   * @param b buffer access to second key
+   * @param bLen the length of the b key,
+   *     may be smaller than the buffer {@code b}
+   *
+   * @return the result of the comparison
+   */
+  private static int compareInternal(final AbstractComparator comparator, final ByteBuffer a,
+      final int aLen, final ByteBuffer b, final int bLen) {
+    if (aLen != -1) {
+      a.mark();
+      a.limit(aLen);
+    }
+    if (bLen != -1) {
+      b.mark();
+      b.limit(bLen);
+    }
 
-    /**
-     * Only called from JNI.
-     *
-     * Simply a bridge to calling
-     * {@link AbstractComparator#compare(ByteBuffer, ByteBuffer)},
-     * which ensures that the byte buffer lengths are correct
-     * before and after the call.
-     *
-     * @param comparator the comparator object on which to
-     *     call {@link AbstractComparator#compare(ByteBuffer, ByteBuffer)}
-     * @param a buffer access to first key
-     * @param aLen the length of the a key,
-     *     may be smaller than the buffer {@code a}
-     * @param b buffer access to second key
-     * @param bLen the length of the b key,
-     *     may be smaller than the buffer {@code b}
-     *
-     * @return the result of the comparison
-     */
-    private static int compareInternal(
-            final AbstractComparator comparator,
-            final ByteBuffer a, final int aLen,
-            final ByteBuffer b, final int bLen) {
-        if (aLen != -1) {
-            a.mark();
-            a.limit(aLen);
-        }
-        if (bLen != -1) {
-            b.mark();
-            b.limit(bLen);
-        }
+    final int c = comparator.compare(a, b);
 
-        final int c = comparator.compare(a, b);
+    if (aLen != -1) {
+      a.reset();
+    }
+    if (bLen != -1) {
+      b.reset();
+    }
 
-        if (aLen != -1) {
-            a.reset();
-        }
-        if (bLen != -1) {
-            b.reset();
-        }
+    return c;
+  }
 
-        return c;
+  /**
+   * Only called from JNI.
+   * <p>
+   * Simply a bridge to calling
+   * {@link AbstractComparator#findShortestSeparator(ByteBuffer, ByteBuffer)},
+   * which ensures that the byte buffer lengths are correct
+   * before the call.
+   *
+   * @param comparator the comparator object on which to
+   *     call {@link AbstractComparator#findShortestSeparator(ByteBuffer, ByteBuffer)}
+   * @param start buffer access to the start key
+   * @param startLen the length of the start key,
+   *     may be smaller than the buffer {@code start}
+   * @param limit buffer access to the limit key
+   * @param limitLen the length of the limit key,
+   *     may be smaller than the buffer {@code limit}
+   *
+   * @return either {@code startLen} if the start key is unchanged, otherwise
+   *     the new length of the start key
+   */
+  private static int findShortestSeparatorInternal(final AbstractComparator comparator,
+      final ByteBuffer start, final int startLen, final ByteBuffer limit, final int limitLen) {
+    if (startLen != -1) {
+      start.limit(startLen);
     }
-
-    /**
-     * Only called from JNI.
-     *
-     * Simply a bridge to calling
-     * {@link AbstractComparator#findShortestSeparator(ByteBuffer, ByteBuffer)},
-     * which ensures that the byte buffer lengths are correct
-     * before the call.
-     *
-     * @param comparator the comparator object on which to
-     *     call {@link AbstractComparator#findShortestSeparator(ByteBuffer, ByteBuffer)}
-     * @param start buffer access to the start key
-     * @param startLen the length of the start key,
-     *     may be smaller than the buffer {@code start}
-     * @param limit buffer access to the limit key
-     * @param limitLen the length of the limit key,
-     *     may be smaller than the buffer {@code limit}
-     *
-     * @return either {@code startLen} if the start key is unchanged, otherwise
-     *     the new length of the start key
-     */
-    private static int findShortestSeparatorInternal(
-            final AbstractComparator comparator,
-            final ByteBuffer start, final int startLen,
-            final ByteBuffer limit, final int limitLen) {
-        if (startLen != -1) {
-            start.limit(startLen);
-        }
-        if (limitLen != -1) {
-            limit.limit(limitLen);
-        }
-        comparator.findShortestSeparator(start, limit);
-        return start.remaining();
+    if (limitLen != -1) {
+      limit.limit(limitLen);
     }
+    comparator.findShortestSeparator(start, limit);
+    return start.remaining();
+  }
 
-    /**
-     * Only called from JNI.
-     *
-     * Simply a bridge to calling
-     * {@link AbstractComparator#findShortestSeparator(ByteBuffer, ByteBuffer)},
-     * which ensures that the byte buffer length is correct
-     * before the call.
-     *
-     * @param comparator the comparator object on which to
-     *     call {@link AbstractComparator#findShortSuccessor(ByteBuffer)}
-     * @param key buffer access to the key
-     * @param keyLen the length of the key,
-     *     may be smaller than the buffer {@code key}
-     *
-     * @return either keyLen if the key is unchanged, otherwise the new length of the key
-     */
-    private static int findShortSuccessorInternal(
-            final AbstractComparator comparator,
-            final ByteBuffer key, final int keyLen) {
-        if (keyLen != -1) {
-            key.limit(keyLen);
-        }
-        comparator.findShortSuccessor(key);
-        return key.remaining();
+  /**
+   * Only called from JNI.
+   * <p>
+   * Simply a bridge to calling
+   * {@link AbstractComparator#findShortestSeparator(ByteBuffer, ByteBuffer)},
+   * which ensures that the byte buffer length is correct
+   * before the call.
+   *
+   * @param comparator the comparator object on which to
+   *     call {@link AbstractComparator#findShortSuccessor(ByteBuffer)}
+   * @param key buffer access to the key
+   * @param keyLen the length of the key,
+   *     may be smaller than the buffer {@code key}
+   *
+   * @return either keyLen if the key is unchanged, otherwise the new length of the key
+   */
+  private static int findShortSuccessorInternal(
+      final AbstractComparator comparator, final ByteBuffer key, final int keyLen) {
+    if (keyLen != -1) {
+      key.limit(keyLen);
     }
+    comparator.findShortSuccessor(key);
+    return key.remaining();
+  }
 }

java/src/main/java/org/rocksdb/AbstractEventListener.java

@@ -71,8 +71,8 @@ static EnabledEventCallback fromValue(final byte value) {
 
   /**
    * Creates an Event Listener that will
-   * received all callbacks from C++.
-   *
+   * receive all callbacks from C++.
+   * <p>
    * If you don't need all callbacks, it is much more efficient to
    * just register for the ones you need by calling
    * {@link #AbstractEventListener(EnabledEventCallback...)} instead.
@@ -106,8 +106,8 @@ protected AbstractEventListener(final EnabledEventCallback... enabledEventCallba
    */
   private static long packToLong(final EnabledEventCallback... enabledEventCallbacks) {
     long l = 0;
-    for (int i = 0; i < enabledEventCallbacks.length; i++) {
-      l |= 1 << enabledEventCallbacks[i].getValue();
+    for (final EnabledEventCallback enabledEventCallback : enabledEventCallbacks) {
+      l |= 1L << enabledEventCallback.getValue();
     }
     return l;
   }

java/src/main/java/org/rocksdb/AbstractMutableOptions.java

@@ -53,25 +53,23 @@ public String toString() {
     return buffer.toString();
   }
 
-  public static abstract class AbstractMutableOptionsBuilder<
-      T extends AbstractMutableOptions,
-      U extends AbstractMutableOptionsBuilder<T, U, K>,
-      K extends MutableOptionKey> {
-
+  public abstract static class AbstractMutableOptionsBuilder<
+      T extends AbstractMutableOptions, U extends AbstractMutableOptionsBuilder<T, U, K>, K
+          extends MutableOptionKey> {
     private final Map<K, MutableOptionValue<?>> options = new LinkedHashMap<>();
     private final List<OptionString.Entry> unknown = new ArrayList<>();
 
     protected abstract U self();
 
     /**
-     * Get all of the possible keys
+     * Get all the possible keys
      *
      * @return A map of all keys, indexed by name.
      */
     protected abstract Map<String, K> allKeys();
 
     /**
-     * Construct a sub-class instance of {@link AbstractMutableOptions}.
+     * Construct a subclass instance of {@link AbstractMutableOptions}.
      *
      * @param keys the keys
      * @param values the values
@@ -224,7 +222,7 @@ protected <N extends Enum<N>> N getEnum(final K key)
     private long parseAsLong(final String value) {
       try {
         return Long.parseLong(value);
-      } catch (NumberFormatException nfe) {
+      } catch (final NumberFormatException nfe) {
         final double doubleValue = Double.parseDouble(value);
         if (doubleValue != Math.round(doubleValue))
           throw new IllegalArgumentException("Unable to parse or round " + value + " to long");
@@ -242,7 +240,7 @@ private long parseAsLong(final String value) {
     private int parseAsInt(final String value) {
       try {
         return Integer.parseInt(value);
-      } catch (NumberFormatException nfe) {
+      } catch (final NumberFormatException nfe) {
         final double doubleValue = Double.parseDouble(value);
         if (doubleValue != Math.round(doubleValue))
           throw new IllegalArgumentException("Unable to parse or round " + value + " to int");
@@ -271,7 +269,7 @@ protected U fromParsed(final List<OptionString.Entry> options, final boolean ign
             throw new IllegalArgumentException("options string is invalid: " + option);
           }
           fromOptionString(option, ignoreUnknown);
-        } catch (NumberFormatException nfe) {
+        } catch (final NumberFormatException nfe) {
           throw new IllegalArgumentException(
               "" + option.key + "=" + option.value + " - not a valid value for its type", nfe);
         }
@@ -287,7 +285,7 @@ protected U fromParsed(final List<OptionString.Entry> options, final boolean ign
      * @param ignoreUnknown if this is not set, throw an exception when a key is not in the known
      *     set
      * @return the same object, after adding options
-     * @throws IllegalArgumentException if the key is unkown, or a value has the wrong type/form
+     * @throws IllegalArgumentException if the key is unknown, or a value has the wrong type/form
      */
     private U fromOptionString(final OptionString.Entry option, final boolean ignoreUnknown)
         throws IllegalArgumentException {
@@ -299,7 +297,7 @@ private U fromOptionString(final OptionString.Entry option, final boolean ignore
         unknown.add(option);
         return self();
       } else if (key == null) {
-        throw new IllegalArgumentException("Key: " + key + " is not a known option key");
+        throw new IllegalArgumentException("Key: " + null + " is not a known option key");
       }
 
       if (!option.value.isList()) {
@@ -341,7 +339,7 @@ private U fromOptionString(final OptionString.Entry option, final boolean ignore
           return setIntArray(key, value);
 
         case ENUM:
-          String optionName = key.name();
+          final String optionName = key.name();
           if (optionName.equals("prepopulate_blob_cache")) {
             final PrepopulateBlobCache prepopulateBlobCache =
                 PrepopulateBlobCache.getFromInternal(valueStr);

java/src/main/java/org/rocksdb/AbstractNativeReference.java

@@ -16,8 +16,9 @@
  * <a
  * href="https://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html">try-with-resources</a>
  * statement, when you are finished with the object. It is no longer
- * called automatically during the regular Java GC process via
+ * called automatically during the regular Java GC process via finalization
  * {@link AbstractNativeReference#finalize()}.</p>
+ * which is deprecated from Java 9.
  * <p>
  * Explanatory note - When or if the Garbage Collector calls {@link Object#finalize()}
  * depends on the JVM implementation and system conditions, which the programmer