Persist: enforce that null array elements are legal (#7741)

versioned/storage/bigtable/src/main/java/org/projectnessie/versioned/storage/bigtable/BigTablePersist.java

@@ -485,8 +485,10 @@ public void deleteObjs(@Nonnull @jakarta.annotation.Nonnull ObjId[] ids) {
     try (Batcher<RowMutationEntry, Void> batcher =
         backend.client().newBulkMutationBatcher(backend.tableObjs)) {
       for (ObjId id : ids) {
-        ByteString key = dbKey(id);
-        batcher.add(RowMutationEntry.create(key).deleteRow());
+        if (id != null) {
+          ByteString key = dbKey(id);
+          batcher.add(RowMutationEntry.create(key).deleteRow());
+        }
       }
     } catch (ApiException e) {
       throw apiException(e);
@@ -534,6 +536,9 @@ public void upsertObjs(@Nonnull @jakarta.annotation.Nonnull Obj[] objs)
     try (Batcher<RowMutationEntry, Void> batcher =
         backend.client().newBulkMutationBatcher(backend.tableObjs)) {
       for (Obj obj : objs) {
+        if (obj == null) {
+          continue;
+        }
         ObjId id = obj.id();
         checkArgument(id != null, "Obj to store must have a non-null ID");
 

versioned/storage/common-tests/src/main/java/org/projectnessie/versioned/storage/commontests/AbstractBasePersistTests.java

@@ -1001,6 +1001,20 @@ public void createObjectsWithUpsertThenFetchAndScan() throws Exception {
     }
   }
 
+  @Test
+  public void nullHandlingInArrays() throws ObjTooLargeException, ObjNotFoundException {
+    soft.assertThat(persist.storeObjs(new Obj[] {null})).containsExactly(false);
+    soft.assertThatCode(() -> persist.upsertObjs(new Obj[] {null})).doesNotThrowAnyException();
+    soft.assertThat(persist.fetchObjs(new ObjId[] {null})).containsExactly((Obj) null);
+    soft.assertThatCode(() -> persist.deleteObjs(new ObjId[] {null})).doesNotThrowAnyException();
+    soft.assertThat(persist.storeObjs(new Obj[] {null, null})).containsExactly(false, false);
+    soft.assertThatCode(() -> persist.upsertObjs(new Obj[] {null, null}))
+        .doesNotThrowAnyException();
+    soft.assertThat(persist.fetchObjs(new ObjId[] {null, null})).containsExactly(null, null);
+    soft.assertThatCode(() -> persist.deleteObjs(new ObjId[] {null, null}))
+        .doesNotThrowAnyException();
+  }
+
   public static String randomContentId() {
     return randomUUID().toString();
   }

versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/persist/Persist.java

@@ -265,6 +265,8 @@ boolean storeObj(@Nonnull @jakarta.annotation.Nonnull Obj obj, boolean ignoreSof
    * <p>In case an object failed to be stored, it is undefined whether other objects have been
    * stored or not.
    *
+   * @param objs array with {@link Obj}s to store. {@code null} array elements are legal, the
+   *     corresponding elements in the returned array will be {@code false}.
    * @return an array with {@code boolean}s indicating whether the corresponding objects were
    *     created ({@code true}) or already present ({@code false}), see {@link #storeObj(Obj)}
    * @throws ObjTooLargeException thrown when a hard database row/item size limit has been hit, or a
@@ -282,6 +284,8 @@ boolean storeObj(@Nonnull @jakarta.annotation.Nonnull Obj obj, boolean ignoreSof
    *
    * <p>In case an object failed to be deleted, it is undefined whether other objects have been
    * deleted or not.
+   *
+   * @param ids array with {@link ObjId}s to delete. {@code null} array elements are legal.
    */
   void deleteObjs(@Nonnull @jakarta.annotation.Nonnull ObjId[] ids);
 
@@ -302,6 +306,7 @@ boolean storeObj(@Nonnull @jakarta.annotation.Nonnull Obj obj, boolean ignoreSof
    * <p>In case an object failed to be updated, it is undefined whether other objects have been
    * updated or not.
    *
+   * @param objs array with {@link Obj}s to upsert. {@code null} array elements are legal.
    * @see #upsertObj( Obj)
    */
   void upsertObjs(@Nonnull @jakarta.annotation.Nonnull Obj[] objs) throws ObjTooLargeException;

versioned/storage/dynamodb/src/main/java/org/projectnessie/versioned/storage/dynamodb/DynamoDBPersist.java

@@ -541,7 +541,9 @@ public void deleteObj(@Nonnull @jakarta.annotation.Nonnull ObjId id) {
   public void deleteObjs(@Nonnull @jakarta.annotation.Nonnull ObjId[] ids) {
     try (BatchWrite batchWrite = new BatchWrite(backend, backend.tableObjs)) {
       for (ObjId id : ids) {
-        batchWrite.addDelete(objKey(id));
+        if (id != null) {
+          batchWrite.addDelete(objKey(id));
+        }
       }
     }
   }
@@ -571,12 +573,14 @@ public void upsertObjs(@Nonnull @jakarta.annotation.Nonnull Obj[] objs)
     // DynamoDB does not support "PUT IF NOT EXISTS" in a BatchWriteItemRequest/PutItem
     try (BatchWrite batchWrite = new BatchWrite(backend, backend.tableObjs)) {
       for (Obj obj : objs) {
-        ObjId id = obj.id();
-        checkArgument(id != null, "Obj to store must have a non-null ID");
+        if (obj != null) {
+          ObjId id = obj.id();
+          checkArgument(id != null, "Obj to store must have a non-null ID");
 
-        Map<String, AttributeValue> item = objToItem(obj, id, false);
+          Map<String, AttributeValue> item = objToItem(obj, id, false);
 
-        batchWrite.addPut(item);
+          batchWrite.addPut(item);
+        }
       }
     }
   }

versioned/storage/inmemory/src/main/java/org/projectnessie/versioned/storage/inmemory/InmemoryPersist.java

@@ -274,7 +274,9 @@ public boolean[] storeObjs(@Nonnull @jakarta.annotation.Nonnull Obj[] objs)
       throws ObjTooLargeException {
     boolean[] r = new boolean[objs.length];
     for (int i = 0; i < objs.length; i++) {
-      r[i] = storeObj(objs[i]);
+      if (objs[i] != null) {
+        r[i] = storeObj(objs[i]);
+      }
     }
     return r;
   }
@@ -287,7 +289,9 @@ public void deleteObj(@Nonnull @jakarta.annotation.Nonnull ObjId id) {
   @Override
   public void deleteObjs(@Nonnull @jakarta.annotation.Nonnull ObjId[] ids) {
     for (ObjId id : ids) {
-      deleteObj(id);
+      if (id != null) {
+        deleteObj(id);
+      }
     }
   }
 
@@ -301,7 +305,9 @@ public void upsertObj(@Nonnull @jakarta.annotation.Nonnull Obj obj) throws ObjTo
   public void upsertObjs(@Nonnull @jakarta.annotation.Nonnull Obj[] objs)
       throws ObjTooLargeException {
     for (Obj obj : objs) {
-      upsertObj(obj);
+      if (obj != null) {
+        upsertObj(obj);
+      }
     }
   }
 

versioned/storage/jdbc/src/main/java/org/projectnessie/versioned/storage/jdbc/AbstractJdbcPersist.java

@@ -531,7 +531,8 @@ private boolean[] upsertObjs(
       // Sadly an INSERT INTO ... ON CONFLICT DO UPDATE SET ... does not work with parameters in the
       // UPDATE SET clause. Since the JDBC connection is configured with auto-commit=false, we can
       // just DELETE the updates to be upserted and INSERT them again.
-      deleteObjs(conn, stream(objs).map(Obj::id).toArray(ObjId[]::new));
+      deleteObjs(
+          conn, stream(objs).map(obj -> obj == null ? null : obj.id()).toArray(ObjId[]::new));
     }
 
     try (PreparedStatement ps = conn.prepareStatement(databaseSpecific.wrapInsert(STORE_OBJ))) {
@@ -634,6 +635,9 @@ protected final void deleteObjs(
       int batchSize = 0;
 
       for (ObjId id : ids) {
+        if (id == null) {
+          continue;
+        }
         ps.setString(1, config.repositoryId());
         serializeObjId(ps, 2, id);
         ps.addBatch();

versioned/storage/rocksdb/src/main/java/org/projectnessie/versioned/storage/rocksdb/RocksDBPersist.java

@@ -17,7 +17,6 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.Collections.singleton;
-import static org.projectnessie.versioned.storage.common.persist.Reference.reference;
 import static org.projectnessie.versioned.storage.rocksdb.RocksDBBackend.keyPrefix;
 import static org.projectnessie.versioned.storage.rocksdb.RocksDBBackend.rocksDbException;
 import static org.projectnessie.versioned.storage.serialize.ProtoSerialization.deserializeObj;
@@ -424,7 +423,9 @@ public void deleteObj(@Nonnull @jakarta.annotation.Nonnull ObjId id) {
   @Override
   public void deleteObjs(@Nonnull @jakarta.annotation.Nonnull ObjId[] ids) {
     for (ObjId id : ids) {
-      deleteObj(id);
+      if (id != null) {
+        deleteObj(id);
+      }
     }
   }
 
@@ -455,7 +456,9 @@ public void upsertObj(@Nonnull @jakarta.annotation.Nonnull Obj obj) throws ObjTo
   public void upsertObjs(@Nonnull @jakarta.annotation.Nonnull Obj[] objs)
       throws ObjTooLargeException {
     for (Obj obj : objs) {
-      upsertObj(obj);
+      if (obj != null) {
+        upsertObj(obj);
+      }
     }
   }