From 7071399d15d3f5de0a63c429f2c94094c87cf1cb Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Tue, 11 Jun 2024 21:01:30 +0200 Subject: [PATCH] Persist: introduce type-safe bulk-fetch functions (#8763) This change introduces `Persist.fetchTypedObjs()` and `Persist.fetchTypedObjsIfExist()` as bulk-couterparts to `Persist.fetchTypedObj()`. Also refactor the code paths to let the untyped functions use the typed functions to unify/keep common code paths. --- .../storage/batching/BatchingPersistImpl.java | 49 ++++------- .../storage/bigtable/BigTablePersist.java | 70 +++++---------- .../storage/cache/CachingPersistImpl.java | 50 +++++++++++ .../storage/cache/TestNegativeCaching.java | 80 +++++++++++++++++ .../storage/cassandra/CassandraBackend.java | 10 ++- .../storage/cassandra/CassandraPersist.java | 36 ++++---- .../commontests/AbstractBasePersistTests.java | 10 +++ .../storage/common/logic/CommitLogicImpl.java | 11 +-- .../common/logic/IndexesLogicImpl.java | 7 +- .../common/persist/ObservingPersist.java | 24 +++++- .../storage/common/persist/Persist.java | 52 +++++++++-- .../storage/dynamodb/DynamoDBPersist.java | 70 +++++++-------- .../storage/inmemory/InmemoryPersist.java | 38 ++++---- .../storage/jdbc/AbstractJdbcPersist.java | 65 ++++---------- .../versioned/storage/jdbc/JdbcPersist.java | 23 ++--- .../storage/mongodb/MongoDBPersist.java | 86 +++++++++---------- .../storage/rocksdb/RocksDBPersist.java | 47 +++++----- .../versionstore/RepositoryConfigBackend.java | 11 +-- .../TransplantIndividualImpl.java | 15 ++-- .../storage/versionstore/PersistDelegate.java | 17 +++- .../transfer/AbstractExportImport.java | 18 +++- 21 files changed, 458 insertions(+), 331 deletions(-) diff --git a/versioned/storage/batching/src/main/java/org/projectnessie/versioned/storage/batching/BatchingPersistImpl.java b/versioned/storage/batching/src/main/java/org/projectnessie/versioned/storage/batching/BatchingPersistImpl.java index b100dc16710..47f554ae8ca 100644 --- a/versioned/storage/batching/src/main/java/org/projectnessie/versioned/storage/batching/BatchingPersistImpl.java +++ b/versioned/storage/batching/src/main/java/org/projectnessie/versioned/storage/batching/BatchingPersistImpl.java @@ -20,6 +20,7 @@ import com.google.common.annotations.VisibleForTesting; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import java.lang.reflect.Array; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -178,23 +179,6 @@ public void upsertObjs(@Nonnull @javax.annotation.Nonnull Obj[] objs) } } - @Override - @Nonnull - @javax.annotation.Nonnull - public Obj fetchObj(@Nonnull @javax.annotation.Nonnull ObjId id) throws ObjNotFoundException { - readLock(); - try { - Obj r = pendingObj(id); - if (r != null) { - return r; - } - } finally { - readUnlock(); - } - - return delegate().fetchObj(id); - } - private Obj pendingObj(ObjId id) { Obj r = pendingUpserts.get(id); if (r == null) { @@ -207,13 +191,13 @@ private Obj pendingObj(ObjId id) { @Nonnull @javax.annotation.Nonnull public T fetchTypedObj( - @Nonnull @javax.annotation.Nonnull ObjId id, ObjType type, Class typeClass) + @Nonnull @javax.annotation.Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { readLock(); try { Obj r = pendingObj(id); if (r != null) { - if (!r.type().equals(type)) { + if (type != null && !r.type().equals(type)) { throw new ObjNotFoundException(id); } @SuppressWarnings("unchecked") @@ -246,11 +230,12 @@ public ObjType fetchObjType(@Nonnull @javax.annotation.Nonnull ObjId id) @Override @Nonnull @javax.annotation.Nonnull - public Obj[] fetchObjs(@Nonnull @javax.annotation.Nonnull ObjId[] ids) - throws ObjNotFoundException { + public T[] fetchTypedObjs( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { ObjId[] backendIds = null; - Obj[] r = new Obj[ids.length]; + @SuppressWarnings("unchecked") + T[] r = (T[]) Array.newInstance(typeClass, ids.length); backendIds = fetchObjsPre(ids, r, backendIds); @@ -258,11 +243,11 @@ public Obj[] fetchObjs(@Nonnull @javax.annotation.Nonnull ObjId[] ids) return r; } - Obj[] backendResult = delegate().fetchObjs(backendIds); + T[] backendResult = delegate().fetchTypedObjs(backendIds, type, typeClass); return fetchObjsPost(backendResult, r); } - private ObjId[] fetchObjsPre(ObjId[] ids, Obj[] r, ObjId[] backendIds) { + private ObjId[] fetchObjsPre(ObjId[] ids, T[] r, ObjId[] backendIds) { readLock(); try { for (int i = 0; i < ids.length; i++) { @@ -272,7 +257,9 @@ private ObjId[] fetchObjsPre(ObjId[] ids, Obj[] r, ObjId[] backendIds) { } Obj o = pendingObj(id); if (o != null) { - r[i] = o; + @SuppressWarnings("unchecked") + T typed = (T) o; + r[i] = typed; } else { if (backendIds == null) { backendIds = new ObjId[ids.length]; @@ -286,9 +273,9 @@ private ObjId[] fetchObjsPre(ObjId[] ids, Obj[] r, ObjId[] backendIds) { return backendIds; } - private static Obj[] fetchObjsPost(Obj[] backendResult, Obj[] r) { + private static T[] fetchObjsPost(T[] backendResult, T[] r) { for (int i = 0; i < backendResult.length; i++) { - Obj o = backendResult[i]; + T o = backendResult[i]; if (o != null) { r[i] = o; } @@ -299,10 +286,12 @@ private static Obj[] fetchObjsPost(Obj[] backendResult, Obj[] r) { @Override @Nonnull @javax.annotation.Nonnull - public Obj[] fetchObjsIfExist(@Nonnull @javax.annotation.Nonnull ObjId[] ids) { + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { ObjId[] backendIds = null; - Obj[] r = new Obj[ids.length]; + @SuppressWarnings("unchecked") + T[] r = (T[]) Array.newInstance(typeClass, ids.length); backendIds = fetchObjsPre(ids, r, backendIds); @@ -310,7 +299,7 @@ public Obj[] fetchObjsIfExist(@Nonnull @javax.annotation.Nonnull ObjId[] ids) { return r; } - Obj[] backendResult = delegate().fetchObjsIfExist(backendIds); + T[] backendResult = delegate().fetchTypedObjsIfExist(backendIds, type, typeClass); return fetchObjsPost(backendResult, r); } diff --git a/versioned/storage/bigtable/src/main/java/org/projectnessie/versioned/storage/bigtable/BigTablePersist.java b/versioned/storage/bigtable/src/main/java/org/projectnessie/versioned/storage/bigtable/BigTablePersist.java index b20a49a2244..caa08a3d3dc 100644 --- a/versioned/storage/bigtable/src/main/java/org/projectnessie/versioned/storage/bigtable/BigTablePersist.java +++ b/versioned/storage/bigtable/src/main/java/org/projectnessie/versioned/storage/bigtable/BigTablePersist.java @@ -59,8 +59,8 @@ import com.google.common.collect.ImmutableMap; import com.google.protobuf.ByteString; import jakarta.annotation.Nonnull; +import java.lang.reflect.Array; import java.nio.ByteBuffer; -import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -305,13 +305,20 @@ private static Reference referenceFromRow(Row row) { @Override @Nonnull - public Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { + public T fetchTypedObj( + @Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { try { ByteString key = dbKey(id); Row row = backend.client().readRow(backend.tableObjsId, key); if (row != null) { - return objFromRow(row); + Obj obj = objFromRow(row); + if (type != null && !type.equals(obj.type())) { + throw new ObjNotFoundException(id); + } + @SuppressWarnings("unchecked") + T r = (T) obj; + return r; } throw new ObjNotFoundException(id); } catch (ApiException e) { @@ -320,54 +327,23 @@ public Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { } @Override - @Nonnull - public T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeClass) - throws ObjNotFoundException { - Obj obj = fetchObj(id); - if (!obj.type().equals(type)) { - throw new ObjNotFoundException(id); - } - @SuppressWarnings("unchecked") - T r = (T) obj; - return r; - } - - @Override - @Nonnull - public ObjType fetchObjType(@Nonnull ObjId id) throws ObjNotFoundException { - return fetchObj(id).type(); - } - - @Override - @Nonnull - public Obj[] fetchObjs(@Nonnull ObjId[] ids) throws ObjNotFoundException { + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { try { - Obj[] r = new Obj[ids.length]; - List notFound = new ArrayList<>(); - bulkFetch(backend.tableObjsId, ids, r, this::dbKey, this::objFromRow, notFound::add); - - if (!notFound.isEmpty()) { - throw new ObjNotFoundException(notFound); - } + @SuppressWarnings("unchecked") + T[] r = (T[]) Array.newInstance(typeClass, ids.length); - return r; - } catch (ExecutionException | TimeoutException e) { - throw new RuntimeException(e); - } catch (ApiException e) { - throw apiException(e); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new RuntimeException(e); - } - } - - @Override - @Nonnull - public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { - try { - Obj[] r = new Obj[ids.length]; bulkFetch(backend.tableObjsId, ids, r, this::dbKey, this::objFromRow, x -> {}); + if (type != null) { + for (int i = 0; i < r.length; i++) { + Obj o = r[i]; + if (o != null && !type.equals(o.type())) { + r[i] = null; + } + } + } + return r; } catch (ExecutionException | TimeoutException e) { throw new RuntimeException(e); diff --git a/versioned/storage/cache/src/main/java/org/projectnessie/versioned/storage/cache/CachingPersistImpl.java b/versioned/storage/cache/src/main/java/org/projectnessie/versioned/storage/cache/CachingPersistImpl.java index d2adafaf904..cd279b9a9ea 100644 --- a/versioned/storage/cache/src/main/java/org/projectnessie/versioned/storage/cache/CachingPersistImpl.java +++ b/versioned/storage/cache/src/main/java/org/projectnessie/versioned/storage/cache/CachingPersistImpl.java @@ -19,6 +19,9 @@ import static org.projectnessie.versioned.storage.cache.CacheBackend.NOT_FOUND_OBJ_SENTINEL; import jakarta.annotation.Nonnull; +import java.lang.reflect.Array; +import java.util.ArrayList; +import java.util.List; import java.util.Set; import org.projectnessie.versioned.storage.common.config.StoreConfig; import org.projectnessie.versioned.storage.common.exceptions.ObjNotFoundException; @@ -129,6 +132,53 @@ public Obj[] fetchObjs(@Nonnull ObjId[] ids) throws ObjNotFoundException { return fetchObjsPost(backendIds, backendResult, r, null); } + @Nonnull + @Override + public T[] fetchTypedObjs( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { + @SuppressWarnings("unchecked") + T[] r = (T[]) Array.newInstance(typeClass, ids.length); + + ObjId[] backendIds = fetchObjsPre(ids, r, type, typeClass); + + if (backendIds != null) { + T[] backendResult = persist.fetchTypedObjsIfExist(backendIds, type, typeClass); + r = fetchObjsPost(backendIds, backendResult, r, type); + } + + List notFound = null; + for (int i = 0; i < ids.length; i++) { + ObjId id = ids[i]; + if (r[i] == null && id != null) { + if (notFound == null) { + notFound = new ArrayList<>(); + } + notFound.add(id); + } + } + if (notFound != null) { + throw new ObjNotFoundException(notFound); + } + + return r; + } + + @Override + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { + @SuppressWarnings("unchecked") + T[] r = (T[]) Array.newInstance(typeClass, ids.length); + + ObjId[] backendIds = fetchObjsPre(ids, r, type, typeClass); + + if (backendIds == null) { + return r; + } + + T[] backendResult = persist.fetchTypedObjsIfExist(backendIds, type, typeClass); + return fetchObjsPost(backendIds, backendResult, r, type); + } + private ObjId[] fetchObjsPre( ObjId[] ids, T[] r, ObjType type, @Nonnull Class typeClass) { ObjId[] backendIds = null; diff --git a/versioned/storage/cache/src/test/java/org/projectnessie/versioned/storage/cache/TestNegativeCaching.java b/versioned/storage/cache/src/test/java/org/projectnessie/versioned/storage/cache/TestNegativeCaching.java index 5b54e4520ce..299765668c6 100644 --- a/versioned/storage/cache/src/test/java/org/projectnessie/versioned/storage/cache/TestNegativeCaching.java +++ b/versioned/storage/cache/src/test/java/org/projectnessie/versioned/storage/cache/TestNegativeCaching.java @@ -15,6 +15,8 @@ */ package org.projectnessie.versioned.storage.cache; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -56,6 +58,7 @@ public void nonEffectiveNegativeCache() throws Exception { .isInstanceOf(ObjNotFoundException.class); verify(cachedPersist).fetchObj(id); verify(backing).fetchObj(id); + verify(backing).fetchTypedObj(eq(id), any(), any()); verifyNoMoreInteractions(backing, cachedPersist); reset(backing, cachedPersist); @@ -64,6 +67,7 @@ public void nonEffectiveNegativeCache() throws Exception { .isInstanceOf(ObjNotFoundException.class); verify(cachedPersist).fetchObj(id); verify(backing).fetchObj(id); + verify(backing).fetchTypedObj(eq(id), any(), any()); verifyNoMoreInteractions(backing, cachedPersist); reset(backing, cachedPersist); } @@ -123,6 +127,28 @@ private void negativeCacheFetchRepeat(Persist cachedPersist, Persist backing, Ob verifyNoMoreInteractions(backing, cachedPersist); reset(backing, cachedPersist); + soft.assertThatThrownBy( + () -> + cachedPersist.fetchTypedObjs( + new ObjId[] {id}, NegativeCachingObj.TYPE, NegativeCachingObj.class)) + .isInstanceOf(ObjNotFoundException.class); + verify(cachedPersist) + .fetchTypedObjs(new ObjId[] {id}, NegativeCachingObj.TYPE, NegativeCachingObj.class); + // backing.fetch*() must not be invoked + verifyNoMoreInteractions(backing, cachedPersist); + reset(backing, cachedPersist); + + soft.assertThat( + cachedPersist.fetchTypedObjsIfExist( + new ObjId[] {id}, NegativeCachingObj.TYPE, NegativeCachingObj.class)) + .hasSize(1) + .containsOnlyNulls(); + verify(cachedPersist) + .fetchTypedObjsIfExist(new ObjId[] {id}, NegativeCachingObj.TYPE, NegativeCachingObj.class); + // backing.fetch*() must not be invoked + verifyNoMoreInteractions(backing, cachedPersist); + reset(backing, cachedPersist); + soft.assertThat(cachedPersist.fetchObjsIfExist(new ObjId[] {id})) .hasSize(1) .containsOnlyNulls(); @@ -131,4 +157,58 @@ private void negativeCacheFetchRepeat(Persist cachedPersist, Persist backing, Ob verifyNoMoreInteractions(backing, cachedPersist); reset(backing, cachedPersist); } + + @Test + public void negativeCacheFetchTypedObjs() throws Exception { + Persist backing = spy(persist); + CacheBackend cacheBackend = + PersistCaches.newBackend(CacheConfig.builder().capacityMb(16).build()); + Persist cachedPersist = spy(cacheBackend.wrap(backing)); + + verify(backing).config(); + reset(backing); + + ObjId id = randomObjId(); + + soft.assertThatThrownBy( + () -> + cachedPersist.fetchTypedObjs( + new ObjId[] {id}, NegativeCachingObj.TYPE, NegativeCachingObj.class)) + .isInstanceOf(ObjNotFoundException.class); + verify(cachedPersist) + .fetchTypedObjs(new ObjId[] {id}, NegativeCachingObj.TYPE, NegativeCachingObj.class); + verify(backing) + .fetchTypedObjsIfExist(new ObjId[] {id}, NegativeCachingObj.TYPE, NegativeCachingObj.class); + verifyNoMoreInteractions(backing, cachedPersist); + reset(backing, cachedPersist); + + negativeCacheFetchRepeat(cachedPersist, backing, id); + } + + @Test + public void negativeCacheFetchTypedObjsIfExist() throws Exception { + Persist backing = spy(persist); + CacheBackend cacheBackend = + PersistCaches.newBackend(CacheConfig.builder().capacityMb(16).build()); + Persist cachedPersist = spy(cacheBackend.wrap(backing)); + + verify(backing).config(); + reset(backing); + + ObjId id = randomObjId(); + + soft.assertThat( + cachedPersist.fetchTypedObjsIfExist( + new ObjId[] {id}, NegativeCachingObj.TYPE, NegativeCachingObj.class)) + .hasSize(1) + .containsOnlyNulls(); + verify(cachedPersist) + .fetchTypedObjsIfExist(new ObjId[] {id}, NegativeCachingObj.TYPE, NegativeCachingObj.class); + verify(backing) + .fetchTypedObjsIfExist(new ObjId[] {id}, NegativeCachingObj.TYPE, NegativeCachingObj.class); + verifyNoMoreInteractions(backing, cachedPersist); + reset(backing, cachedPersist); + + negativeCacheFetchRepeat(cachedPersist, backing, id); + } } diff --git a/versioned/storage/cassandra/src/main/java/org/projectnessie/versioned/storage/cassandra/CassandraBackend.java b/versioned/storage/cassandra/src/main/java/org/projectnessie/versioned/storage/cassandra/CassandraBackend.java index ae48f68186a..3c32f681033 100644 --- a/versioned/storage/cassandra/src/main/java/org/projectnessie/versioned/storage/cassandra/CassandraBackend.java +++ b/versioned/storage/cassandra/src/main/java/org/projectnessie/versioned/storage/cassandra/CassandraBackend.java @@ -213,10 +213,12 @@ public Object apply(AsyncResultSet rs, Throwable ex) { try { for (Row row : rs.currentPage()) { R resultItem = rowToResult.apply(row); - K id = idExtractor.apply(resultItem); - int i = idToIndex.getValue(id); - if (i != -1) { - result.set(i, resultItem); + if (resultItem != null) { + K id = idExtractor.apply(resultItem); + int i = idToIndex.getValue(id); + if (i != -1) { + result.set(i, resultItem); + } } } diff --git a/versioned/storage/cassandra/src/main/java/org/projectnessie/versioned/storage/cassandra/CassandraPersist.java b/versioned/storage/cassandra/src/main/java/org/projectnessie/versioned/storage/cassandra/CassandraPersist.java index 074cee9d6c7..a1bf4af11f7 100644 --- a/versioned/storage/cassandra/src/main/java/org/projectnessie/versioned/storage/cassandra/CassandraPersist.java +++ b/versioned/storage/cassandra/src/main/java/org/projectnessie/versioned/storage/cassandra/CassandraPersist.java @@ -234,23 +234,15 @@ public Reference updateReferencePointer(@Nonnull Reference reference, @Nonnull O @SuppressWarnings("unused") @Override @Nonnull - public T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeClass) - throws ObjNotFoundException { - Obj obj = fetchObjsIfExist(new ObjId[] {id})[0]; + public T fetchTypedObj( + @Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { + T obj = fetchTypedObjsIfExist(new ObjId[] {id}, type, typeClass)[0]; - if (obj == null || !obj.type().equals(type)) { + if (obj == null || (type != null && !type.equals(obj.type()))) { throw new ObjNotFoundException(id); } - @SuppressWarnings("unchecked") - T r = (T) obj; - return r; - } - - @Override - @Nonnull - public Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { - return fetchObjs(new ObjId[] {id})[0]; + return obj; } @Override @@ -269,7 +261,8 @@ public ObjType fetchObjType(@Nonnull ObjId id) throws ObjNotFoundException { @Nonnull @Override - public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { Function, List> idsToStrings = queryIds -> queryIds.stream().map(ObjId::toString).collect(Collectors.toList()); @@ -279,17 +272,22 @@ public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { backend.buildStatement( FIND_OBJS, true, config.repositoryId(), idsToStrings.apply(keys))); - Function rowMapper = + Function rowMapper = row -> { ObjType objType = ObjTypes.forName(requireNonNull(row.getString(COL_OBJ_TYPE.name()))); + if (type != null && !type.equals(objType)) { + return null; + } ObjId id = deserializeObjId(row.getString(COL_OBJ_ID.name())); String versionToken = row.getString(COL_OBJ_VERS.name()); - return ObjSerializers.forType(objType).deserialize(row, objType, id, versionToken); + @SuppressWarnings("unchecked") + T typed = (T) ObjSerializers.forType(objType).deserialize(row, objType, id, versionToken); + return typed; }; - Obj[] r; - try (BatchedQuery batchedQuery = - backend.newBatchedQuery(queryFunc, rowMapper, Obj::id, ids.length, Obj.class)) { + T[] r; + try (BatchedQuery batchedQuery = + backend.newBatchedQuery(queryFunc, rowMapper, Obj::id, ids.length, typeClass)) { for (int i = 0; i < ids.length; i++) { ObjId id = ids[i]; diff --git a/versioned/storage/common-tests/src/main/java/org/projectnessie/versioned/storage/commontests/AbstractBasePersistTests.java b/versioned/storage/common-tests/src/main/java/org/projectnessie/versioned/storage/commontests/AbstractBasePersistTests.java index 4277a7569a3..88864af700a 100644 --- a/versioned/storage/common-tests/src/main/java/org/projectnessie/versioned/storage/commontests/AbstractBasePersistTests.java +++ b/versioned/storage/common-tests/src/main/java/org/projectnessie/versioned/storage/commontests/AbstractBasePersistTests.java @@ -833,6 +833,12 @@ public void fetchNonExistingObj() { .extracting(ObjNotFoundException::objIds, list(ObjId.class)) .containsExactly(id); + soft.assertThatThrownBy(() -> persist.fetchTypedObj(id, null, Obj.class)) + .isInstanceOf(ObjNotFoundException.class) + .asInstanceOf(type(ObjNotFoundException.class)) + .extracting(ObjNotFoundException::objIds, list(ObjId.class)) + .containsExactly(id); + soft.assertThatThrownBy(() -> persist.fetchObjs(new ObjId[] {EMPTY_OBJ_ID, id})) .isInstanceOf(ObjNotFoundException.class) .asInstanceOf(type(ObjNotFoundException.class)) @@ -843,6 +849,10 @@ public void fetchNonExistingObj() { .hasSize(2) .containsOnlyNulls(); + soft.assertThat(persist.fetchTypedObjsIfExist(new ObjId[] {EMPTY_OBJ_ID, id}, null, Obj.class)) + .hasSize(2) + .containsOnlyNulls(); + ObjId id2 = randomObjId(); soft.assertThatThrownBy(() -> persist.fetchObjs(new ObjId[] {EMPTY_OBJ_ID, id, id2})) diff --git a/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/logic/CommitLogicImpl.java b/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/logic/CommitLogicImpl.java index 2d85aeb3739..08db6a3440a 100644 --- a/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/logic/CommitLogicImpl.java +++ b/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/logic/CommitLogicImpl.java @@ -16,7 +16,6 @@ package org.projectnessie.versioned.storage.common.logic; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.Maps.newHashMapWithExpectedSize; import static com.google.common.collect.Sets.newHashSetWithExpectedSize; import static java.util.Collections.emptyIterator; @@ -880,19 +879,11 @@ public CommitObj[] fetchCommits(@Nonnull ObjId startCommitId, @Nonnull ObjId end CommitObj[] r = new CommitObj[2]; if (startCommitId != null || endCommitId != null) { - Obj[] objs = persist.fetchObjs(new ObjId[] {startCommitId, endCommitId}); - r[0] = castToCommitObj(objs[0]); - r[1] = castToCommitObj(objs[1]); + r = persist.fetchTypedObjs(new ObjId[] {startCommitId, endCommitId}, COMMIT, CommitObj.class); } return r; } - private static CommitObj castToCommitObj(Obj obj) { - checkState( - obj == null || obj instanceof CommitObj, "Expected a Commit object, but got %s", obj); - return (CommitObj) obj; - } - @Nonnull @Override public DiffPagedResult diff(@Nonnull DiffQuery diffQuery) { diff --git a/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/logic/IndexesLogicImpl.java b/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/logic/IndexesLogicImpl.java index 0f20b606fe7..98251e73d0c 100644 --- a/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/logic/IndexesLogicImpl.java +++ b/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/logic/IndexesLogicImpl.java @@ -270,13 +270,12 @@ private StoreIndex loadIndexSegment(@Nonnull ObjId indexId) { private StoreIndex[] loadIndexSegments(@Nonnull ObjId[] indexes) { try { - Obj[] objs = persist.fetchObjs(indexes); + IndexObj[] objs = persist.fetchTypedObjs(indexes, INDEX, IndexObj.class); @SuppressWarnings("unchecked") StoreIndex[] r = new StoreIndex[indexes.length]; for (int i = 0; i < objs.length; i++) { - Obj obj = objs[i]; - if (obj != null) { - IndexObj index = (IndexObj) obj; + IndexObj index = objs[i]; + if (index != null) { r[i] = deserializeIndex(index.index()).setObjId(indexes[i]); } } diff --git a/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/persist/ObservingPersist.java b/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/persist/ObservingPersist.java index c8afdf9d9b9..f7f849bcb41 100644 --- a/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/persist/ObservingPersist.java +++ b/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/persist/ObservingPersist.java @@ -160,8 +160,8 @@ public Obj getImmediate(@Nonnull ObjId id) { @Counted(PREFIX) @Timed(value = PREFIX, histogram = true) @Nonnull - public T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeClass) - throws ObjNotFoundException { + public T fetchTypedObj( + @Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { return delegate.fetchTypedObj(id, type, typeClass); } @@ -192,6 +192,26 @@ public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { return delegate.fetchObjsIfExist(ids); } + @WithSpan + @Override + @Counted(PREFIX) + @Timed(value = PREFIX, histogram = true) + @Nonnull + public T[] fetchTypedObjs( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { + return delegate.fetchTypedObjs(ids, type, typeClass); + } + + @WithSpan + @Override + @Counted(PREFIX) + @Timed(value = PREFIX, histogram = true) + @Nonnull + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { + return delegate.fetchTypedObjsIfExist(ids, type, typeClass); + } + @WithSpan @Override @Counted(PREFIX) diff --git a/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/persist/Persist.java b/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/persist/Persist.java index 56404b4590d..ff0c7fa8957 100644 --- a/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/persist/Persist.java +++ b/versioned/storage/common/src/main/java/org/projectnessie/versioned/storage/common/persist/Persist.java @@ -189,7 +189,9 @@ default Reference[] fetchReferencesForUpdate(@Nonnull String[] names) { * @see #fetchObjs(ObjId[]) */ @Nonnull - Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException; + default Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { + return fetchTypedObj(id, null, Obj.class); + } default Obj getImmediate(@Nonnull ObjId id) { return null; @@ -206,8 +208,10 @@ default Obj getImmediate(@Nonnull ObjId id) { * @see #fetchObjs(ObjId[]) */ @Nonnull - T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeClass) - throws ObjNotFoundException; + default T fetchTypedObj( + @Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { + return fetchTypedObjs(new ObjId[] {id}, type, typeClass)[0]; + } /** * Retrieves the type of the object with ID {@code id}. @@ -219,7 +223,9 @@ T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeCl * @see #fetchObjs(ObjId[]) */ @Nonnull - ObjType fetchObjType(@Nonnull ObjId id) throws ObjNotFoundException; + default ObjType fetchObjType(@Nonnull ObjId id) throws ObjNotFoundException { + return fetchObj(id).type(); + } /** * Like {@link #fetchObj(ObjId)}, but finds multiple objects by name at once, leveraging bulk @@ -240,10 +246,29 @@ T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeCl * @see #fetchTypedObj(ObjId, ObjType, Class) * @see #fetchObj(ObjId) * @see #fetchObjsIfExist(ObjId[]) + * @see #fetchTypedObjs(ObjId[], ObjType, Class) + * @see #fetchTypedObjsIfExist(ObjId[], ObjType, Class) */ @Nonnull default Obj[] fetchObjs(@Nonnull ObjId[] ids) throws ObjNotFoundException { - Obj[] r = fetchObjsIfExist(ids); + return fetchTypedObjs(ids, null, Obj.class); + } + + /** + * Type-safe variant of {@link #fetchObjs(ObjId[])}. + * + * @param ids IDs of the objects to fetch. + * @param type The expected type of objects to fetch. Can be {@code null}, meaning that any object + * type is fine, must use {@code typeClass=Obj.class}. + * @param typeClass The Java type that corresponds to {@code type}. Must not be {@code null}, use + * {@code Obj.class} for "any" object type in combination with {@code type=null}. + * @throws ObjNotFoundException If any of the given {@link ObjId}s does not exist or has not the + * requested {@code type} + */ + @Nonnull + default T[] fetchTypedObjs( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { + T[] r = fetchTypedObjsIfExist(ids, type, typeClass); List notFound = null; for (int i = 0; i < ids.length; i++) { @@ -266,7 +291,22 @@ default Obj[] fetchObjs(@Nonnull ObjId[] ids) throws ObjNotFoundException { * Same as {@link #fetchObjs(ObjId[])}, does not throw an {@link ObjNotFoundException} but returns * {@code null} instead. */ - Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids); + default Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { + return fetchTypedObjsIfExist(ids, null, Obj.class); + } + + /** + * Same as {@link #fetchTypedObjs(ObjId[], ObjType, Class)}, but returns {@code null} for objects + * that do not exist instead of throwing an {@link ObjNotFoundException}. + * + * @param ids IDs of the objects to fetch. + * @param type The expected type of objects to fetch. Can be {@code null}, meaning that any object + * type is fine, must use {@code typeClass=Obj.class}. + * @param typeClass The Java type that corresponds to {@code type}. Must not be {@code null}, use + * {@code Obj.class} for "any" object type in combination with {@code type=null}. + */ + T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass); /** * Stores the given object as a new record. diff --git a/versioned/storage/dynamodb/src/main/java/org/projectnessie/versioned/storage/dynamodb/DynamoDBPersist.java b/versioned/storage/dynamodb/src/main/java/org/projectnessie/versioned/storage/dynamodb/DynamoDBPersist.java index a1b5ac166c7..4d723842c29 100644 --- a/versioned/storage/dynamodb/src/main/java/org/projectnessie/versioned/storage/dynamodb/DynamoDBPersist.java +++ b/versioned/storage/dynamodb/src/main/java/org/projectnessie/versioned/storage/dynamodb/DynamoDBPersist.java @@ -51,6 +51,7 @@ import com.google.common.collect.AbstractIterator; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import java.lang.reflect.Array; import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; @@ -313,7 +314,8 @@ private List attributeToPreviousPointers( @Override @Nonnull - public Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { + public T fetchTypedObj( + @Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { GetItemResponse item; try { item = backend.client().getItem(b -> b.tableName(backend.tableObjs).key(objKeyMap(id))); @@ -324,31 +326,11 @@ public Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { throw new ObjNotFoundException(id); } - return itemToObj(item.item()); - } - - @Override - @Nonnull - public T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeClass) - throws ObjNotFoundException { - GetItemResponse item; - try { - item = backend.client().getItem(b -> b.tableName(backend.tableObjs).key(objKeyMap(id))); - } catch (RuntimeException e) { - throw unhandledException(e); - } - if (!item.hasItem()) { + T obj = itemToObj(item.item(), type, typeClass); + if (obj == null) { throw new ObjNotFoundException(id); } - - Obj obj = itemToObj(item.item()); - if (!obj.type().equals(type)) { - throw new ObjNotFoundException(id); - } - - @SuppressWarnings("unchecked") - T r = (T) obj; - return r; + return obj; } @Override @@ -376,11 +358,13 @@ public ObjType fetchObjType(@Nonnull ObjId id) throws ObjNotFoundException { @Nonnull @Override - public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { List> keys = new ArrayList<>(Math.min(ids.length, BATCH_GET_LIMIT)); Object2IntHashMap idToIndex = new Object2IntHashMap<>(200, Hashing.DEFAULT_LOAD_FACTOR, -1); - Obj[] r = new Obj[ids.length]; + @SuppressWarnings("unchecked") + T[] r = (T[]) Array.newInstance(typeClass, ids.length); for (int i = 0; i < ids.length; i++) { ObjId id = ids[i]; if (id != null) { @@ -388,7 +372,7 @@ public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { idToIndex.put(id, i); if (keys.size() == BATCH_GET_LIMIT) { - fetchObjsPage(r, keys, idToIndex); + fetchObjsPage(r, keys, idToIndex, type, typeClass); keys.clear(); idToIndex.clear(); } @@ -396,14 +380,18 @@ public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { } if (!keys.isEmpty()) { - fetchObjsPage(r, keys, idToIndex); + fetchObjsPage(r, keys, idToIndex, type, typeClass); } return r; } - private void fetchObjsPage( - Obj[] r, List> keys, Object2IntHashMap idToIndex) { + private void fetchObjsPage( + T[] r, + List> keys, + Object2IntHashMap idToIndex, + ObjType type, + Class typeClass) { Map requestItems = singletonMap(backend.tableObjs, KeysAndAttributes.builder().keys(keys).build()); @@ -417,10 +405,12 @@ private void fetchObjsPage( .get(backend.tableObjs) .forEach( item -> { - Obj obj = itemToObj(item); - int idx = idToIndex.getValue(obj.id()); - if (idx != -1) { - r[idx] = obj; + T obj = itemToObj(item, type, typeClass); + if (obj != null) { + int idx = idToIndex.getValue(obj.id()); + if (idx != -1) { + r[idx] = obj; + } } }); } catch (RuntimeException e) { @@ -613,14 +603,20 @@ public void erase() { backend.eraseRepositories(singleton(config().repositoryId())); } - private Obj itemToObj(Map item) { + private T itemToObj( + Map item, ObjType t, @SuppressWarnings("unused") Class typeClass) { ObjId id = objIdFromString(item.get(KEY_NAME).s().substring(keyPrefix.length())); AttributeValue attributeValue = item.get(COL_OBJ_TYPE); ObjType type = ObjTypes.forShortName(attributeValue.s()); + if (t != null && !t.equals(type)) { + return null; + } ObjSerializer serializer = ObjSerializers.forType(type); Map inner = item.get(serializer.attributeName()).m(); String versionToken = attributeToString(item, COL_OBJ_VERS); - return serializer.fromMap(id, type, inner, versionToken); + @SuppressWarnings("unchecked") + T typed = (T) serializer.fromMap(id, type, inner, versionToken); + return typed; } @Nonnull @@ -772,7 +768,7 @@ protected Obj computeNext() { } Map item = pageIter.next(); - return itemToObj(item); + return itemToObj(item, null, Obj.class); } } catch (RuntimeException e) { throw unhandledException(e); diff --git a/versioned/storage/inmemory/src/main/java/org/projectnessie/versioned/storage/inmemory/InmemoryPersist.java b/versioned/storage/inmemory/src/main/java/org/projectnessie/versioned/storage/inmemory/InmemoryPersist.java index 4d8b1263093..8d6360b6b8d 100644 --- a/versioned/storage/inmemory/src/main/java/org/projectnessie/versioned/storage/inmemory/InmemoryPersist.java +++ b/versioned/storage/inmemory/src/main/java/org/projectnessie/versioned/storage/inmemory/InmemoryPersist.java @@ -20,6 +20,7 @@ import com.google.common.collect.AbstractIterator; import jakarta.annotation.Nonnull; +import java.lang.reflect.Array; import java.util.Iterator; import java.util.Map; import java.util.Set; @@ -179,22 +180,18 @@ public Reference updateReferencePointer(@Nonnull Reference reference, @Nonnull O throw new RefConditionFailedException(result[1]); } - @Override @Nonnull + @Override public Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { - Obj obj = inmemory.objects.get(compositeKey(id)); - if (obj == null) { - throw new ObjNotFoundException(id); - } - return obj; + return fetchTypedObj(id, null, Obj.class); } @Override @Nonnull - public T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeClass) - throws ObjNotFoundException { + public T fetchTypedObj( + @Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { Obj obj = inmemory.objects.get(compositeKey(id)); - if (obj == null || !obj.type().equals(type)) { + if (obj == null || (type != null && !type.equals(obj.type()))) { throw new ObjNotFoundException(id); } @SuppressWarnings("unchecked") @@ -204,24 +201,21 @@ public T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class @Override @Nonnull - public ObjType fetchObjType(@Nonnull ObjId id) throws ObjNotFoundException { - Obj obj = inmemory.objects.get(compositeKey(id)); - if (obj == null) { - throw new ObjNotFoundException(id); - } - return obj.type(); - } - - @Override - @Nonnull - public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { - Obj[] r = new Obj[ids.length]; + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { + @SuppressWarnings("unchecked") + T[] r = (T[]) Array.newInstance(typeClass, ids.length); for (int i = 0; i < ids.length; i++) { ObjId id = ids[i]; if (id == null) { continue; } - r[i] = inmemory.objects.get(compositeKey(id)); + Obj o = inmemory.objects.get(compositeKey(id)); + if (o != null && (type == null || type.equals(o.type()))) { + @SuppressWarnings("unchecked") + T typed = (T) o; + r[i] = typed; + } } return r; } diff --git a/versioned/storage/jdbc/src/main/java/org/projectnessie/versioned/storage/jdbc/AbstractJdbcPersist.java b/versioned/storage/jdbc/src/main/java/org/projectnessie/versioned/storage/jdbc/AbstractJdbcPersist.java index fa678f5d314..b5c4d61d849 100644 --- a/versioned/storage/jdbc/src/main/java/org/projectnessie/versioned/storage/jdbc/AbstractJdbcPersist.java +++ b/versioned/storage/jdbc/src/main/java/org/projectnessie/versioned/storage/jdbc/AbstractJdbcPersist.java @@ -49,6 +49,7 @@ import com.google.common.collect.ImmutableMap.Builder; import jakarta.annotation.Nonnull; import jakarta.annotation.Nullable; +import java.lang.reflect.Array; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -322,18 +323,14 @@ private String referencesDml(String sql, Reference reference) { } protected T fetchTypedObj( - Connection conn, ObjId id, ObjType type, @SuppressWarnings("unused") Class typeClass) - throws ObjNotFoundException { - Obj obj = fetchObjs(conn, new ObjId[] {id}, type)[0]; + Connection conn, ObjId id, ObjType type, Class typeClass) throws ObjNotFoundException { + T obj = fetchTypedObjsIfExist(conn, new ObjId[] {id}, type, typeClass)[0]; - @SuppressWarnings("unchecked") - T r = (T) obj; - return r; - } + if (obj == null) { + throw new ObjNotFoundException(id); + } - protected final Obj fetchObj(@Nonnull Connection conn, @Nonnull ObjId id) - throws ObjNotFoundException { - return fetchObjs(conn, new ObjId[] {id})[0]; + return obj; } protected ObjType fetchObjType(@Nonnull Connection conn, @Nonnull ObjId id) @@ -354,45 +351,15 @@ protected ObjType fetchObjType(@Nonnull Connection conn, @Nonnull ObjId id) } @Nonnull - protected final Obj[] fetchObjs(@Nonnull Connection conn, @Nonnull ObjId[] ids) - throws ObjNotFoundException { - return fetchObjs(conn, ids, null); - } - - @Nonnull - protected final Obj[] fetchObjsIfExist(@Nonnull Connection conn, @Nonnull ObjId[] ids) { - return fetchObjsIfExist(conn, ids, null); - } - - @Nonnull - protected final Obj[] fetchObjs( - @Nonnull Connection conn, @Nonnull ObjId[] ids, @Nullable ObjType type) - throws ObjNotFoundException { - Obj[] r = fetchObjsIfExist(conn, ids, type); - - List notFound = null; - for (int i = 0; i < ids.length; i++) { - ObjId id = ids[i]; - if (r[i] == null && id != null) { - if (notFound == null) { - notFound = new ArrayList<>(); - } - notFound.add(id); - } - } - if (notFound != null) { - throw new ObjNotFoundException(notFound); - } - - return r; - } - - @Nonnull - protected final Obj[] fetchObjsIfExist( - @Nonnull Connection conn, @Nonnull ObjId[] ids, @Nullable ObjType type) { + protected final T[] fetchTypedObjsIfExist( + @Nonnull Connection conn, + @Nonnull ObjId[] ids, + ObjType type, + @SuppressWarnings("unused") Class typeClass) { Object2IntHashMap idToIndex = new Object2IntHashMap<>(200, Hashing.DEFAULT_LOAD_FACTOR, -1); - Obj[] r = new Obj[ids.length]; + @SuppressWarnings("unchecked") + T[] r = (T[]) Array.newInstance(typeClass, ids.length); List keys = new ArrayList<>(); for (int i = 0; i < ids.length; i++) { ObjId id = ids[i]; @@ -424,7 +391,9 @@ protected final Obj[] fetchObjsIfExist( Obj obj = deserializeObj(rs); int i = idToIndex.getValue(obj.id()); if (i != -1) { - r[i] = obj; + @SuppressWarnings("unchecked") + T typed = (T) obj; + r[i] = typed; } } diff --git a/versioned/storage/jdbc/src/main/java/org/projectnessie/versioned/storage/jdbc/JdbcPersist.java b/versioned/storage/jdbc/src/main/java/org/projectnessie/versioned/storage/jdbc/JdbcPersist.java index 7f238062d85..0108e016f15 100644 --- a/versioned/storage/jdbc/src/main/java/org/projectnessie/versioned/storage/jdbc/JdbcPersist.java +++ b/versioned/storage/jdbc/src/main/java/org/projectnessie/versioned/storage/jdbc/JdbcPersist.java @@ -186,14 +186,8 @@ public Reference updateReferencePointer(@Nonnull Reference reference, @Nonnull O @Override @Nonnull - public Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { - return withConnectionException(true, conn -> super.fetchObj(conn, id)); - } - - @Override - @Nonnull - public T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeClass) - throws ObjNotFoundException { + public T fetchTypedObj( + @Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { return withConnectionException(true, conn -> super.fetchTypedObj(conn, id, type, typeClass)); } @@ -204,15 +198,10 @@ public ObjType fetchObjType(@Nonnull ObjId id) throws ObjNotFoundException { } @Override - @Nonnull - public Obj[] fetchObjs(@Nonnull ObjId[] ids) throws ObjNotFoundException { - return withConnectionException(true, conn -> super.fetchObjs(conn, ids)); - } - - @Override - @Nonnull - public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { - return withConnectionException(true, conn -> super.fetchObjsIfExist(conn, ids)); + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { + return withConnectionException( + true, conn -> super.fetchTypedObjsIfExist(conn, ids, type, typeClass)); } @Override diff --git a/versioned/storage/mongodb/src/main/java/org/projectnessie/versioned/storage/mongodb/MongoDBPersist.java b/versioned/storage/mongodb/src/main/java/org/projectnessie/versioned/storage/mongodb/MongoDBPersist.java index 8e2c8cf19e5..7984da31374 100644 --- a/versioned/storage/mongodb/src/main/java/org/projectnessie/versioned/storage/mongodb/MongoDBPersist.java +++ b/versioned/storage/mongodb/src/main/java/org/projectnessie/versioned/storage/mongodb/MongoDBPersist.java @@ -67,6 +67,7 @@ import com.mongodb.client.result.DeleteResult; import com.mongodb.client.result.UpdateResult; import jakarta.annotation.Nonnull; +import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -345,46 +346,27 @@ public Reference[] fetchReferences(@Nonnull String[] names) { @Override @Nonnull - public Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { - FindIterable result; - try { - result = backend.objs().find(eq(ID_PROPERTY_NAME, idObjDoc(id))); - } catch (RuntimeException e) { - throw unhandledException(e); - } - - Document doc = result.first(); - if (doc == null) { - throw new ObjNotFoundException(id); - } - - return docToObj(id, doc); - } - - @Override - @Nonnull - public T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeClass) - throws ObjNotFoundException { + public T fetchTypedObj( + @Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { FindIterable result; try { result = - backend - .objs() - .find(and(eq(ID_PROPERTY_NAME, idObjDoc(id)), eq(COL_OBJ_TYPE, type.shortName()))); + type != null + ? backend + .objs() + .find(and(eq(ID_PROPERTY_NAME, idObjDoc(id)), eq(COL_OBJ_TYPE, type.shortName()))) + : backend.objs().find(eq(ID_PROPERTY_NAME, idObjDoc(id))); } catch (RuntimeException e) { throw unhandledException(e); } Document doc = result.first(); - if (doc == null) { + + T obj = docToObj(id, doc, type, typeClass); + if (obj == null) { throw new ObjNotFoundException(id); } - - Obj obj = docToObj(id, doc); - - @SuppressWarnings("unchecked") - T r = (T) obj; - return r; + return obj; } @Override @@ -405,13 +387,14 @@ public ObjType fetchObjType(@Nonnull ObjId id) throws ObjNotFoundException { return ObjTypes.forShortName(doc.getString(COL_OBJ_TYPE)); } - @Nonnull @Override - public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { List list = new ArrayList<>(ids.length); Object2IntHashMap idToIndex = new Object2IntHashMap<>(ids.length * 2, Hashing.DEFAULT_LOAD_FACTOR, -1); - Obj[] r = new Obj[ids.length]; + @SuppressWarnings("unchecked") + T[] r = (T[]) Array.newInstance(typeClass, ids.length); for (int i = 0; i < ids.length; i++) { ObjId id = ids[i]; if (id != null) { @@ -421,13 +404,18 @@ public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { } if (!list.isEmpty()) { - fetchObjsPage(r, list, idToIndex); + fetchObjsPage(r, list, idToIndex, type, typeClass); } return r; } - private void fetchObjsPage(Obj[] r, List list, Object2IntHashMap idToIndex) { + private void fetchObjsPage( + Obj[] r, + List list, + Object2IntHashMap idToIndex, + ObjType type, + Class typeClass) { FindIterable result; try { result = backend.objs().find(in(ID_PROPERTY_NAME, list)); @@ -435,10 +423,12 @@ private void fetchObjsPage(Obj[] r, List list, Object2IntHashMap T docToObj(Document doc, ObjType type, Class typeClass) { ObjId id = objIdFromDoc(doc); - return docToObj(id, doc); + return docToObj(id, doc, type, typeClass); } - private Obj docToObj(@Nonnull ObjId id, Document doc) { + private T docToObj( + @Nonnull ObjId id, Document doc, ObjType t, @SuppressWarnings("unused") Class typeClass) { + if (doc == null) { + return null; + } ObjType type = ObjTypes.forShortName(doc.getString(COL_OBJ_TYPE)); - ObjSerializer serializer = ObjSerializers.forType(type); + if (t != null && !t.equals(type)) { + return null; + } + @SuppressWarnings("unchecked") + ObjSerializer serializer = (ObjSerializer) ObjSerializers.forType(type); Document inner = doc.get(serializer.fieldName(), Document.class); String versionToken = doc.getString(COL_OBJ_VERS); return serializer.docToObj(id, type, inner, versionToken); @@ -735,7 +733,7 @@ protected Obj computeNext() { try { Document doc = result.next(); - return docToObj(doc); + return docToObj(doc, null, Obj.class); } catch (RuntimeException e) { throw unhandledException(e); } diff --git a/versioned/storage/rocksdb/src/main/java/org/projectnessie/versioned/storage/rocksdb/RocksDBPersist.java b/versioned/storage/rocksdb/src/main/java/org/projectnessie/versioned/storage/rocksdb/RocksDBPersist.java index bd7516e771f..b8c0813dcf6 100644 --- a/versioned/storage/rocksdb/src/main/java/org/projectnessie/versioned/storage/rocksdb/RocksDBPersist.java +++ b/versioned/storage/rocksdb/src/main/java/org/projectnessie/versioned/storage/rocksdb/RocksDBPersist.java @@ -27,6 +27,7 @@ import com.google.common.collect.AbstractIterator; import jakarta.annotation.Nonnull; +import java.lang.reflect.Array; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -257,7 +258,9 @@ public Reference updateReferencePointer(@Nonnull Reference reference, @Nonnull O @Override @Nonnull - public Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { + public T fetchTypedObj( + @Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { + try { RocksDBBackend b = backend; TransactionDB db = b.db(); @@ -268,41 +271,29 @@ public Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { if (obj == null) { throw new ObjNotFoundException(id); } - return deserializeObj(id, obj, null); + Obj o = deserializeObj(id, obj, null); + if (o == null || (type != null && !type.equals(o.type()))) { + throw new ObjNotFoundException(id); + } + @SuppressWarnings("unchecked") + T typed = (T) o; + return typed; } catch (RocksDBException e) { throw rocksDbException(e); } } @Override - @Nonnull - public T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeClass) - throws ObjNotFoundException { - Obj obj = fetchObj(id); - if (!obj.type().equals(type)) { - throw new ObjNotFoundException(id); - } - @SuppressWarnings("unchecked") - T r = (T) obj; - return r; - } - - @Override - @Nonnull - public ObjType fetchObjType(@Nonnull ObjId id) throws ObjNotFoundException { - return fetchObj(id).type(); - } - - @Override - @Nonnull - public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { try { RocksDBBackend b = backend; TransactionDB db = b.db(); ColumnFamilyHandle cf = b.objs(); int num = ids.length; - Obj[] r = new Obj[num]; + @SuppressWarnings("unchecked") + T[] r = (T[]) Array.newInstance(typeClass, num); List handles = new ArrayList<>(num); List keys = new ArrayList<>(num); for (ObjId id : ids) { @@ -319,7 +310,13 @@ public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { if (id != null) { byte[] obj = dbResult.get(ri++); if (obj != null) { - r[i] = deserializeObj(id, obj, null); + Obj o = deserializeObj(id, obj, null); + if (type != null && !type.equals(o.type())) { + o = null; + } + @SuppressWarnings("unchecked") + T typed = (T) o; + r[i] = typed; } } } diff --git a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/RepositoryConfigBackend.java b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/RepositoryConfigBackend.java index 8f1aac50769..94c52e0db2b 100644 --- a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/RepositoryConfigBackend.java +++ b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/RepositoryConfigBackend.java @@ -22,6 +22,7 @@ import static org.projectnessie.versioned.storage.common.logic.Logics.indexesLogic; import static org.projectnessie.versioned.storage.common.logic.Logics.referenceLogic; import static org.projectnessie.versioned.storage.common.logic.Logics.stringLogic; +import static org.projectnessie.versioned.storage.common.objtypes.StandardObjType.STRING; import static org.projectnessie.versioned.storage.common.util.Ser.SHARED_OBJECT_MAPPER; import static org.projectnessie.versioned.storage.versionstore.RefMapping.referenceConflictException; @@ -53,7 +54,6 @@ import org.projectnessie.versioned.storage.common.objtypes.CommitObj; import org.projectnessie.versioned.storage.common.objtypes.CommitOp; import org.projectnessie.versioned.storage.common.objtypes.StringObj; -import org.projectnessie.versioned.storage.common.persist.Obj; import org.projectnessie.versioned.storage.common.persist.ObjId; import org.projectnessie.versioned.storage.common.persist.Persist; import org.projectnessie.versioned.storage.common.persist.Reference; @@ -79,18 +79,19 @@ public List getConfigs(Set repositoryCo CommitObj head = commitLogic(p).headCommit(reference); StoreIndex index = indexesLogic.buildCompleteIndexOrEmpty(head); - Obj[] objs = - p.fetchObjs( + StringObj[] objs = + p.fetchTypedObjs( repositoryConfigTypes.stream() .map(RepositoryConfigBackend::repositoryConfigTypeToStoreKey) .map(storeKey -> valueObjIdFromIndex(index, storeKey)) - .toArray(ObjId[]::new)); + .toArray(ObjId[]::new), + STRING, + StringObj.class); StringLogic stringLogic = stringLogic(p); return Arrays.stream(objs) .filter(Objects::nonNull) - .map(StringObj.class::cast) .map( s -> { try { diff --git a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/TransplantIndividualImpl.java b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/TransplantIndividualImpl.java index c32062f386f..af963d24b42 100644 --- a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/TransplantIndividualImpl.java +++ b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/TransplantIndividualImpl.java @@ -22,6 +22,7 @@ import static org.projectnessie.versioned.storage.common.logic.CreateCommit.newCommitBuilder; import static org.projectnessie.versioned.storage.common.logic.Logics.commitLogic; import static org.projectnessie.versioned.storage.common.logic.Logics.indexesLogic; +import static org.projectnessie.versioned.storage.common.objtypes.StandardObjType.COMMIT; import static org.projectnessie.versioned.storage.common.persist.ObjId.EMPTY_OBJ_ID; import static org.projectnessie.versioned.storage.versionstore.RefMapping.referenceNotFound; import static org.projectnessie.versioned.storage.versionstore.TypeMapping.fromCommitMeta; @@ -181,11 +182,13 @@ MergeTransplantContext loadSourceCommitsForTransplant(VersionStore.TransplantOp branch.getName(), referenceHash.map(Hash::asString).orElse("not specified")); - Obj[] objs; + CommitObj[] objs; try { objs = - persist.fetchObjs( - commitHashes.stream().map(TypeMapping::hashToObjId).toArray(ObjId[]::new)); + persist.fetchTypedObjs( + commitHashes.stream().map(TypeMapping::hashToObjId).toArray(ObjId[]::new), + COMMIT, + CommitObj.class); } catch (ObjNotFoundException e) { throw referenceNotFound(e); } @@ -193,11 +196,7 @@ MergeTransplantContext loadSourceCommitsForTransplant(VersionStore.TransplantOp CommitObj parent = null; CommitLogic commitLogic = commitLogic(persist); for (int i = 0; i < objs.length; i++) { - Obj o = objs[i]; - if (o == null) { - throw RefMapping.hashNotFound(commitHashes.get(i)); - } - CommitObj commit = (CommitObj) o; + CommitObj commit = objs[i]; if (i > 0) { if (!commit.directParent().equals(commits.get(i - 1).id())) { throw new IllegalArgumentException("Sequence of hashes is not contiguous."); diff --git a/versioned/storage/store/src/test/java/org/projectnessie/versioned/storage/versionstore/PersistDelegate.java b/versioned/storage/store/src/test/java/org/projectnessie/versioned/storage/versionstore/PersistDelegate.java index a22b55884c2..e7f7d8e64d5 100644 --- a/versioned/storage/store/src/test/java/org/projectnessie/versioned/storage/versionstore/PersistDelegate.java +++ b/versioned/storage/store/src/test/java/org/projectnessie/versioned/storage/versionstore/PersistDelegate.java @@ -126,8 +126,8 @@ public Obj getImmediate(@Nonnull ObjId id) { @Override @Nonnull - public T fetchTypedObj(@Nonnull ObjId id, ObjType type, Class typeClass) - throws ObjNotFoundException { + public T fetchTypedObj( + @Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { return delegate.fetchTypedObj(id, type, typeClass); } @@ -148,6 +148,19 @@ public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { return delegate.fetchObjsIfExist(ids); } + @Override + @Nonnull + public T[] fetchTypedObjs( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) throws ObjNotFoundException { + return delegate.fetchTypedObjs(ids, type, typeClass); + } + + @Override + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { + return delegate.fetchTypedObjsIfExist(ids, type, typeClass); + } + @Override public boolean storeObj(@Nonnull Obj obj) throws ObjTooLargeException { return delegate.storeObj(obj); diff --git a/versioned/transfer/src/testFixtures/java/org/projectnessie/versioned/transfer/AbstractExportImport.java b/versioned/transfer/src/testFixtures/java/org/projectnessie/versioned/transfer/AbstractExportImport.java index a1c5792baa5..3808476d511 100644 --- a/versioned/transfer/src/testFixtures/java/org/projectnessie/versioned/transfer/AbstractExportImport.java +++ b/versioned/transfer/src/testFixtures/java/org/projectnessie/versioned/transfer/AbstractExportImport.java @@ -344,7 +344,8 @@ public Obj fetchObj(@Nonnull ObjId id) throws ObjNotFoundException { @Nonnull @Override public T fetchTypedObj( - @Nonnull ObjId id, ObjType type, Class typeClass) throws ObjNotFoundException { + @Nonnull ObjId id, ObjType type, @Nonnull Class typeClass) + throws ObjNotFoundException { return inmemory.fetchTypedObj(id, type, typeClass); } @@ -360,12 +361,27 @@ public Obj[] fetchObjs(@Nonnull ObjId[] ids) throws ObjNotFoundException { return inmemory.fetchObjs(ids); } + @Nonnull + @Override + public T[] fetchTypedObjs( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) + throws ObjNotFoundException { + return inmemory.fetchTypedObjs(ids, type, typeClass); + } + @Nonnull @Override public Obj[] fetchObjsIfExist(@Nonnull ObjId[] ids) { return inmemory.fetchObjsIfExist(ids); } + @Nonnull + @Override + public T[] fetchTypedObjsIfExist( + @Nonnull ObjId[] ids, ObjType type, @Nonnull Class typeClass) { + return inmemory.fetchTypedObjsIfExist(ids, type, typeClass); + } + @Override public boolean storeObj(@Nonnull Obj obj, boolean ignoreSoftSizeRestrictions) throws ObjTooLargeException {