Replace get(Uns|S)ignedComparator with getComparator(DbiFlagSet)

Also improve javadoc and refactor some tests to use DbiBuilder. Some tests are failing.
lmdbjava · at055612 · Mar 6, 2025 · Mar 6, 2025 · Mar 6, 2025 · Mar 6, 2025
commit 0234d323244f874ccf12945949e6352879c5a3b3
diff --git a/src/main/java/org/lmdbjava/BufferProxy.java b/src/main/java/org/lmdbjava/BufferProxy.java
@@ -66,26 +66,49 @@
   protected abstract byte[] getBytes(T buffer);
 
   /**
-   * Get a suitable default {@link Comparator} to compare numeric key values as signed.
+   * Get a suitable default {@link Comparator} given the provided flags.
    *
-   * <p>Note: LMDB's default comparator is unsigned so if this is used only for the {@link
-   * CursorIterable} start/stop key comparisons then its behaviour will differ from the iteration
-   * order. Use with caution.
+   * <p>The provided comparator must strictly match the lexicographical order of keys in the native
+   * LMDB database.
    *
+   * @param dbiFlagSet The {@link DbiFlags} set for the database.
    * @return a comparator that can be used (never null)
    */
-  public abstract Comparator<T> getSignedComparator();
+  public abstract Comparator<T> getComparator(final DbiFlagSet dbiFlagSet);
 
   /**
-   * Get a suitable default {@link Comparator} to compare numeric key values as unsigned.
-   * <p>
-   * This should match the behaviour of the LMDB's mdb_cmp comparator as it may be used for
-   * {@link CursorIterable} start/stop keys comparisons, which must match LMDB's insertion order.
-   * </p>
+   * Get a suitable default {@link Comparator}
+   *
+   * <p>The provided comparator must strictly match the lexicographical order of keys in the native
+   * LMDB database.
    *
    * @return a comparator that can be used (never null)
    */
-  public abstract Comparator<T> getUnsignedComparator();
+  public Comparator<T> getComparator() {
+    return getComparator(DbiFlagSet.empty());
+  }
+
+//  /**
+//   * Get a suitable default {@link Comparator} to compare numeric key values as signed.
+//   *
+//   * <p>Note: LMDB's default comparator is unsigned so if this is used only for the {@link
+//   * CursorIterable} start/stop key comparisons then its behaviour will differ from the iteration
+//   * order. Use with caution.
+//   *
+//   * @return a comparator that can be used (never null)
+//   */
+//  public abstract Comparator<T> getSignedComparator();
+//
+//  /**
+//   * Get a suitable default {@link Comparator} to compare numeric key values as unsigned.
+//   * <p>
+//   * This should match the behaviour of the LMDB's mdb_cmp comparator as it may be used for
+//   * {@link CursorIterable} start/stop keys comparisons, which must match LMDB's insertion order.
+//   * </p>
+//   *
+//   * @return a comparator that can be used (never null)
+//   */
+//  public abstract Comparator<T> getUnsignedComparator(final DbiFlagSet dbiFlagSet);
 
   /**
    * Called when the <code>MDB_val</code> should be set to reflect the passed buffer. This buffer

diff --git a/src/main/java/org/lmdbjava/ByteArrayProxy.java b/src/main/java/org/lmdbjava/ByteArrayProxy.java
@@ -104,15 +104,20 @@ protected byte[] getBytes(final byte[] buffer) {
   }
 
   @Override
-  public Comparator<byte[]> getSignedComparator() {
-    return signedComparator;
-  }
-
-  @Override
-  public Comparator<byte[]> getUnsignedComparator() {
+  public Comparator<byte[]> getComparator(final DbiFlagSet dbiFlagSet) {
     return unsignedComparator;
   }
 
+  //  @Override
+//  public Comparator<byte[]> getSignedComparator() {
+//    return signedComparator;
+//  }
+//
+//  @Override
+//  public Comparator<byte[]> getUnsignedComparator() {
+//    return unsignedComparator;
+//  }
+
   @Override
   protected Pointer in(final byte[] buffer, final Pointer ptr) {
     final Pointer pointer = MEM_MGR.allocateDirect(buffer.length);

diff --git a/src/main/java/org/lmdbjava/ByteBufProxy.java b/src/main/java/org/lmdbjava/ByteBufProxy.java
@@ -114,14 +114,19 @@ protected ByteBuf allocate() {
   }
 
   @Override
-  public Comparator<ByteBuf> getSignedComparator() {
+  public Comparator<ByteBuf> getComparator(final DbiFlagSet dbiFlagSet) {
     return comparator;
   }
 
-  @Override
-  public Comparator<ByteBuf> getUnsignedComparator() {
-    return comparator;
-  }
+  //  @Override
+//  public Comparator<ByteBuf> getSignedComparator() {
+//    return comparator;
+//  }
+//
+//  @Override
+//  public Comparator<ByteBuf> getUnsignedComparator() {
+//    return comparator;
+//  }
 
   @Override
   protected void deallocate(final ByteBuf buff) {

diff --git a/src/main/java/org/lmdbjava/ByteBufferProxy.java b/src/main/java/org/lmdbjava/ByteBufferProxy.java
@@ -148,6 +148,35 @@ public static int compareBuff(final ByteBuffer o1, final ByteBuffer o2) {
       return o1.remaining() - o2.remaining();
     }
 
+//    /**
+//     * Possible compareBuff method specifically for 4/8 byte keys when using MDB_INTEGER_KEY
+//     */
+//    public static int compareBuff(final ByteBuffer o1, final ByteBuffer o2) {
+//      requireNonNull(o1);
+//      requireNonNull(o2);
+//      // Both buffers should be same len
+//      final int len1 = o1.limit();
+//      final int len2 = o2.limit();
+//      if (len1 != len2) {
+//        throw new RuntimeException("Length mismatch, len1: " + len1 + ", len2: " + len2
+//            + ". Lengths must be identical and either 4 or 8 bytes.");
+//      }
+//      final boolean reverse1 = o1.order() == LITTLE_ENDIAN;
+//      final boolean reverse2 = o2.order() == LITTLE_ENDIAN;
+//      if (len1 == 8) {
+//          final long lw = reverse1 ? Long.reverseBytes(o1.getLong()) : o1.getLong();
+//          final long rw = reverse2 ? Long.reverseBytes(o2.getLong()) : o2.getLong();
+//          return Long.compareUnsigned(lw, rw);
+//      } else if (len1 == 4) {
+//        final int lw = reverse1 ? Integer.reverseBytes(o1.getInt()) : o1.getInt();
+//        final int rw = reverse2 ? Integer.reverseBytes(o2.getInt()) : o2.getInt();
+//          return Integer.compareUnsigned(lw, rw);
+//      } else {
+//        throw new RuntimeException("Unexpected length len1: " + len1 + ", len2: " + len2
+//            + ". Lengths must be identical and either 4 or 8 bytes.");
+//      }
+//    }
+
     static Field findField(final Class<?> c, final String name) {
       Class<?> clazz = c;
       do {
@@ -182,15 +211,20 @@ protected final ByteBuffer allocate() {
     }
 
     @Override
-    public Comparator<ByteBuffer> getSignedComparator() {
-      return signedComparator;
-    }
-
-    @Override
-    public Comparator<ByteBuffer> getUnsignedComparator() {
+    public Comparator<ByteBuffer> getComparator(DbiFlagSet dbiFlagSet) {
       return unsignedComparator;
     }
 
+    //    @Override
+//    public Comparator<ByteBuffer> getSignedComparator() {
+//      return signedComparator;
+//    }
+//
+//    @Override
+//    public Comparator<ByteBuffer> getUnsignedComparator(final DbiFlagSet dbiFlagSet) {
+//      return unsignedComparator;
+//    }
+
     @Override
     protected final void deallocate(final ByteBuffer buff) {
       buff.order(BIG_ENDIAN);

diff --git a/src/main/java/org/lmdbjava/CursorIterable.java b/src/main/java/org/lmdbjava/CursorIterable.java
@@ -60,10 +60,10 @@ public final class CursorIterable<T> implements Iterable<CursorIterable.KeyVal<T
     this.entry = new KeyVal<>();
 
     if (comparator != null) {
-      // User supplied java-side comparator so use that
+      // User supplied Java-side comparator so use that
       this.rangeComparator = new JavaRangeComparator<>(range, comparator, entry::key);
     } else {
-      // No java-side comparator so call down to LMDB to do the comparison
+      // No Java-side comparator, so call down to LMDB to do the comparison
       this.rangeComparator = new LmdbRangeComparator<>(txn, dbi, cursor, range, proxy);
     }
   }

diff --git a/src/main/java/org/lmdbjava/Dbi.java b/src/main/java/org/lmdbjava/Dbi.java
@@ -51,6 +51,7 @@ public final class Dbi<T> {
 
   private final ComparatorCallback ccb;
   private boolean cleaned;
+  // Used for CursorIterable KeyRange testing and/or native callbacks
   private final Comparator<T> comparator;
   private final Env<T> env;
   private final byte[] name;
@@ -80,6 +81,7 @@ public final class Dbi<T> {
     ptr = dbiPtr.getPointer(0);
     if (nativeCb) {
       requireNonNull(comparator, "comparator cannot be null if nativeCb is set");
+      // LMDB will call back to this comparator for insertion/iteration order
       this.ccb =
           (keyA, keyB) -> {
             final T compKeyA  = proxy.out(proxy.allocate(), keyA);
@@ -465,6 +467,7 @@ private String getNameAsString() {
       return "";
     } else {
       try {
+        // Assume a UTF8 encoding as we don't know, thus swallow if it fails
         return new String(name, StandardCharsets.UTF_8);
       } catch (Exception e) {
         return "?";
@@ -475,8 +478,8 @@ private String getNameAsString() {
   @Override
   public String toString() {
     return "Dbi{" +
-            "name=" + getNameAsString() +
-            ", dbiFlagSet=" + dbiFlagSet +
+            "name='" + getNameAsString() +
+            "', dbiFlagSet=" + dbiFlagSet +
             '}';
   }