diff --git a/api/client/src/main/java/org/projectnessie/client/util/v2api/ClientSideUpdateNamespace.java b/api/client/src/main/java/org/projectnessie/client/util/v2api/ClientSideUpdateNamespace.java index ada817d0bc0..5791d1da193 100644 --- a/api/client/src/main/java/org/projectnessie/client/util/v2api/ClientSideUpdateNamespace.java +++ b/api/client/src/main/java/org/projectnessie/client/util/v2api/ClientSideUpdateNamespace.java @@ -74,7 +74,7 @@ public UpdateNamespaceResult updateWithResponse() .branchName(refName) .hash(expectedHash) .commitMeta(CommitMeta.fromMessage("update namespace " + key)) - .operation(Put.of(key, updatedNamespace, oldNamespace)) + .operation(Put.of(key, updatedNamespace)) .commitWithResponse(); return UpdateNamespaceResult.of(updatedNamespace, oldNamespace, commit.getTargetBranch()); diff --git a/api/model/src/main/java/org/projectnessie/model/Operation.java b/api/model/src/main/java/org/projectnessie/model/Operation.java index c6d10db020e..1785ee9b9ee 100644 --- a/api/model/src/main/java/org/projectnessie/model/Operation.java +++ b/api/model/src/main/java/org/projectnessie/model/Operation.java @@ -18,6 +18,7 @@ import com.fasterxml.jackson.annotation.JsonSubTypes; import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.annotation.JsonTypeName; +import com.fasterxml.jackson.annotation.JsonView; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.annotation.JsonSerialize; import javax.annotation.Nullable; @@ -26,10 +27,15 @@ import org.eclipse.microprofile.openapi.annotations.media.DiscriminatorMapping; import org.eclipse.microprofile.openapi.annotations.media.Schema; import org.immutables.value.Value; +import org.projectnessie.model.ser.Views; @Schema( type = SchemaType.OBJECT, title = "Operation", + description = + "Describes an operation to be performed against one content object.\n" + + "\n" + + "The Nessie backend will validate the correctness of the operations.", oneOf = {Operation.Put.class, Operation.Unchanged.class, Operation.Delete.class}, discriminatorMapping = { @DiscriminatorMapping(value = "PUT", schema = Operation.Put.class), @@ -53,9 +59,22 @@ public interface Operation { type = SchemaType.OBJECT, title = "Put-'Content'-operation for a 'ContentKey'.", description = - "Add or replace (put) a 'Content' object for a 'ContentKey'. " - + "If the actual table type tracks the 'global state' of individual tables (Iceberg " - + "as of today), every 'Put'-operation must contain a non-null value for 'expectedContent'.") + "Used to add new content or to update existing content.\n" + + "\n" + + "A new content object is created by populating the `value` field, the " + + "content-id in the content object must not be present (null).\n" + + "\n" + + "A content object is updated by populating the `value` containing the correct " + + "content-id.\n" + + "\n" + + "If the key for a content shall change (aka a rename), then use a `Delete` " + + "operation using the current (old) key and a `Put` operation using the new key " + + "with the `value` having the correct content-id. Both operations must happen " + + "in the same commit.\n" + + "\n" + + "A content object can be replaced (think: `DROP TABLE xyz` + `CREATE TABLE xyz`) " + + "with a `Delete` operation and a `Put` operation for a content using a `value`" + + "representing a new content object, so without a content-id, in the same commit.") @Value.Immutable @JsonSerialize(as = ImmutablePut.class) @JsonDeserialize(as = ImmutablePut.class) @@ -67,12 +86,16 @@ interface Put extends Operation { @Nullable @jakarta.annotation.Nullable + @Deprecated + @SuppressWarnings("DeprecatedIsStillUsed") + @JsonView(Views.V1.class) Content getExpectedContent(); static Put of(ContentKey key, Content content) { return ImmutablePut.builder().key(key).content(content).build(); } + @Deprecated // for removal static Put of(ContentKey key, Content content, Content expectedContent) { return ImmutablePut.builder() .key(key) @@ -82,6 +105,15 @@ static Put of(ContentKey key, Content content, Content expectedContent) { } } + @Schema( + type = SchemaType.OBJECT, + title = "Delete-'Content'-operation for a 'ContentKey'.", + description = + "Used to delete an existing content key.\n" + + "\n" + + "If the key for a content shall change (aka a rename), then use a `Delete` " + + "operation using the current (old) key and a `Put` operation using the new key " + + "with the current `Content` in the the `value` field. See `Put` operation.") @Value.Immutable @JsonSerialize(as = ImmutableDelete.class) @JsonDeserialize(as = ImmutableDelete.class) diff --git a/gc/gc-iceberg/src/main/java/org/projectnessie/gc/iceberg/IcebergContentToFiles.java b/gc/gc-iceberg/src/main/java/org/projectnessie/gc/iceberg/IcebergContentToFiles.java index a0eb86630be..49fbcca191c 100644 --- a/gc/gc-iceberg/src/main/java/org/projectnessie/gc/iceberg/IcebergContentToFiles.java +++ b/gc/gc-iceberg/src/main/java/org/projectnessie/gc/iceberg/IcebergContentToFiles.java @@ -99,7 +99,6 @@ public Stream extractFiles(ContentReference contentReference) { contentReference.snapshotId(), "Iceberg content is expected to have a non-null snapshot-ID"); - // This is to respect Nessie's global state Snapshot snapshot = snapshotId < 0L ? tableMetadata.currentSnapshot() : tableMetadata.snapshot(snapshotId); diff --git a/integrations/deltalake/src/main/scala/org/projectnessie/deltalake/NessieLogStore.scala b/integrations/deltalake/src/main/scala/org/projectnessie/deltalake/NessieLogStore.scala index 9146e75ee05..96c8c41a587 100644 --- a/integrations/deltalake/src/main/scala/org/projectnessie/deltalake/NessieLogStore.scala +++ b/integrations/deltalake/src/main/scala/org/projectnessie/deltalake/NessieLogStore.scala @@ -287,17 +287,8 @@ class NessieLogStore(sparkConf: SparkConf, hadoopConf: Configuration) val currentTable = getTable(path.getParent, targetRef) val table = updateDeltaTable(currentTable, path, targetRef, lastCheckpoint) - val put = currentTable - .map( - Put.of( - DeltaContentKeyUtil.fromHadoopPath(path.getParent), - table, - _ - ) - ) - .getOrElse( - Put.of(DeltaContentKeyUtil.fromHadoopPath(path.getParent), table) - ) + val put = + Put.of(DeltaContentKeyUtil.fromHadoopPath(path.getParent), table) val meta = CommitMeta .builder() .message(message) diff --git a/integrations/iceberg-views/src/main/java/org/apache/iceberg/nessie/NessieViewOperations.java b/integrations/iceberg-views/src/main/java/org/apache/iceberg/nessie/NessieViewOperations.java index 835d447bd61..2b27af63640 100644 --- a/integrations/iceberg-views/src/main/java/org/apache/iceberg/nessie/NessieViewOperations.java +++ b/integrations/iceberg-views/src/main/java/org/apache/iceberg/nessie/NessieViewOperations.java @@ -240,7 +240,7 @@ public void commit( builder.message(buildCommitMsg(base, metadata) + " " + key.getName()); Branch branch = api.commitMultipleOperations() - .operation(Operation.Put.of(key, newView, icebergView)) + .operation(Operation.Put.of(key, newView)) .commitMeta(NessieUtil.catalogOptions(builder, catalogOptions).build()) .branch(reference.getAsBranch()) .commit(); diff --git a/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/CommitToBranchSimulation.scala b/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/CommitToBranchSimulation.scala index 66776dce60c..bb5c35b4c68 100644 --- a/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/CommitToBranchSimulation.scala +++ b/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/CommitToBranchSimulation.scala @@ -90,7 +90,7 @@ class CommitToBranchSimulation extends Simulation { .commitMeta( CommitMeta.fromMessage(s"test-commit $userId $commitNum") ) - .operation(Put.of(key, table, expectedTable.orNull)) + .operation(Put.of(key, table)) .commit() session.set("branch", updatedBranch) diff --git a/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/KeyListSpillingSimulation.scala b/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/KeyListSpillingSimulation.scala index b013b547cc5..c82b456624d 100644 --- a/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/KeyListSpillingSimulation.scala +++ b/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/KeyListSpillingSimulation.scala @@ -80,7 +80,7 @@ class KeyListSpillingSimulation extends Simulation { val key = ContentKey.of(s"$userId $commitIndex $i" + padding) val table = IcebergTable .of(s"metadata_${userId}_${commitNum}_$i", 42, 43, 44, 45) - Put.of(key, table, null) + Put.of(key, table) }) // Call the Nessie client operation to perform a commit with one or more put ops diff --git a/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/MixedWorkloadsSimulation.scala b/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/MixedWorkloadsSimulation.scala index 0dbc3adee06..421242b632e 100644 --- a/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/MixedWorkloadsSimulation.scala +++ b/perftest/simulations/src/gatling/scala/org/projectnessie/perftest/gatling/MixedWorkloadsSimulation.scala @@ -322,7 +322,7 @@ class MixedWorkloadsSimulation extends Simulation { .commitMeta(fromMessage(s"Update table ${tableAndLock._1}")) .operation( Operation.Put - .of(tableAndLock._1, updatedTable, currentTable) + .of(tableAndLock._1, updatedTable) ) .commit() session.set("updated", true) diff --git a/servers/services/src/main/java/org/projectnessie/services/impl/NamespaceApiImpl.java b/servers/services/src/main/java/org/projectnessie/services/impl/NamespaceApiImpl.java index f8dd2481797..936abee2d14 100644 --- a/servers/services/src/main/java/org/projectnessie/services/impl/NamespaceApiImpl.java +++ b/servers/services/src/main/java/org/projectnessie/services/impl/NamespaceApiImpl.java @@ -266,7 +266,7 @@ public void updateProperties( Namespace updatedNamespace = ImmutableNamespace.copyOf(namespace).withProperties(properties); - Put put = Put.of(updatedNamespace.toContentKey(), updatedNamespace, namespace); + Put put = Put.of(updatedNamespace.toContentKey(), updatedNamespace); commit( BranchName.of(refWithHash.getValue().getName()), "update properties for namespace " + updatedNamespace.name(), diff --git a/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java b/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java index 45633beb8a7..a328c179bae 100644 --- a/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java +++ b/servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java @@ -991,9 +991,7 @@ protected static org.projectnessie.versioned.Operation toOp(Operation o) { return Delete.of(key); } else if (o instanceof Operation.Put) { Operation.Put put = (Operation.Put) o; - return put.getExpectedContent() != null - ? Put.of(key, put.getContent(), put.getExpectedContent()) - : Put.of(key, put.getContent()); + return Put.of(key, put.getContent()); } else if (o instanceof Operation.Unchanged) { return Unchanged.of(key); } else { diff --git a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommitLog.java b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommitLog.java index ef3303f8661..dd821d473e8 100644 --- a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommitLog.java +++ b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommitLog.java @@ -91,12 +91,8 @@ public void commitResponse() throws BaseNessieClientServerException { commit( response.getTargetBranch(), fromMessage("test"), - Put.of( - key1, IcebergTable.of("loc", 1, 2, 3, 4, contentIds.get(key1)), contents.get(key1)), - Put.of( - key2, - IcebergTable.of("blah", 1, 2, 3, 4, contentIds.get(key2)), - contents.get(key2))); + Put.of(key1, IcebergTable.of("loc", 1, 2, 3, 4, contentIds.get(key1))), + Put.of(key2, IcebergTable.of("blah", 1, 2, 3, 4, contentIds.get(key2)))); soft.assertThat(response.getAddedContents()).isNull(); } @@ -397,9 +393,7 @@ public void commitLogPaging() throws BaseNessieClientServerException { Put op; try { Content existing = contentApi().getContent(key, branch.getName(), currentHash).getContent(); - op = - Put.of( - key, IcebergTable.of("some-file-" + i, 42, 42, 42, 42, existing.getId()), existing); + op = Put.of(key, IcebergTable.of("some-file-" + i, 42, 42, 42, 42, existing.getId())); } catch (NessieNotFoundException notFound) { op = Put.of(key, IcebergTable.of("some-file-" + i, 42, 42, 42, 42)); } diff --git a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommits.java b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommits.java index 7e9c5597117..91f96bedbf3 100644 --- a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommits.java +++ b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestCommits.java @@ -50,7 +50,7 @@ public void renameTable() throws Exception { commit.getTargetBranch(), fromMessage("setup"), Delete.of(oldName), - Put.of(newName, table, table))) + Put.of(newName, table))) .doesNotThrowAnyException(); soft.assertThat(contents(main.getName(), null, oldName, newName)) diff --git a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestMergeTransplant.java b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestMergeTransplant.java index 89d099dfe3b..93c6a34dcb1 100644 --- a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestMergeTransplant.java +++ b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestMergeTransplant.java @@ -179,7 +179,7 @@ private void mergeTransplant( source.getName(), committed1.getHash(), fromMessage("test-branch2"), - Put.of(key1, table1, table1)) + Put.of(key1, table1)) .getTargetBranch(); soft.assertThat(committed2.getHash()).isNotNull(); @@ -546,8 +546,7 @@ public void mergeWithNamespaces(ReferenceMode refMode) throws BaseNessieClientSe contentApi().getContent(key1, committed1.getName(), committed1.getHash()).getContent(); Branch committed2 = - commit(committed1, fromMessage("test-branch2"), Put.of(key1, table1, table1)) - .getTargetBranch(); + commit(committed1, fromMessage("test-branch2"), Put.of(key1, table1)).getTargetBranch(); soft.assertThat(committed2.getHash()).isNotNull(); commit(base, fromMessage("test-main"), Put.of(key2, table2)); diff --git a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java index 5e5f7b4b2f5..75ffc1c0298 100644 --- a/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java +++ b/servers/services/src/testFixtures/java/org/projectnessie/services/impl/BaseTestServiceImpl.java @@ -491,11 +491,7 @@ protected String createCommits( try { Content existing = contentApi().getContent(key, branch.getName(), currentHash).getContent(); - op = - Put.of( - key, - IcebergTable.of("some-file-" + i, 42, 42, 42, 42, existing.getId()), - existing); + op = Put.of(key, IcebergTable.of("some-file-" + i, 42, 42, 42, 42, existing.getId())); } catch (NessieContentNotFoundException notFound) { op = Put.of(key, IcebergTable.of("some-file-" + i, 42, 42, 42, 42)); } @@ -554,12 +550,7 @@ protected static List operationsWithoutContentId(List oper protected static Operation operationWithoutContentId(Operation op) { if (op instanceof Put) { Put put = (Put) op; - return put.getExpectedContent() != null - ? Put.of( - put.getKey(), - contentWithoutId(put.getContent()), - contentWithoutId(put.getExpectedContent())) - : Put.of(put.getKey(), contentWithoutId(put.getContent())); + return Put.of(put.getKey(), contentWithoutId(put.getContent())); } return op; } diff --git a/tools/content-generator/src/main/java/org/projectnessie/tools/contentgenerator/cli/GenerateContent.java b/tools/content-generator/src/main/java/org/projectnessie/tools/contentgenerator/cli/GenerateContent.java index 8a2173e1bb1..46601a7d607 100644 --- a/tools/content-generator/src/main/java/org/projectnessie/tools/contentgenerator/cli/GenerateContent.java +++ b/tools/content-generator/src/main/java/org/projectnessie/tools/contentgenerator/cli/GenerateContent.java @@ -46,10 +46,7 @@ import org.projectnessie.model.CommitMeta; import org.projectnessie.model.Content; import org.projectnessie.model.ContentKey; -import org.projectnessie.model.DeltaLakeTable; import org.projectnessie.model.GetMultipleContentsResponse; -import org.projectnessie.model.IcebergTable; -import org.projectnessie.model.IcebergView; import org.projectnessie.model.ImmutableDeltaLakeTable; import org.projectnessie.model.ImmutableIcebergTable; import org.projectnessie.model.ImmutableIcebergView; @@ -258,13 +255,7 @@ public void execute() throws BaseNessieClientServerException { existingContent, random, existingContent != null ? existingContent.getId() : null); - if (existingContent instanceof IcebergTable - || existingContent instanceof IcebergView - || existingContent instanceof DeltaLakeTable) { - commit.operation(Put.of(key, newContents, existingContent)); - } else { - commit.operation(Put.of(key, newContents)); - } + commit.operation(Put.of(key, newContents)); } try { Branch newHead = commit.commit(); diff --git a/tools/content-generator/src/main/java/org/projectnessie/tools/contentgenerator/cli/RefreshContent.java b/tools/content-generator/src/main/java/org/projectnessie/tools/contentgenerator/cli/RefreshContent.java index 734b4d8a8c0..d1f8e89d6b8 100644 --- a/tools/content-generator/src/main/java/org/projectnessie/tools/contentgenerator/cli/RefreshContent.java +++ b/tools/content-generator/src/main/java/org/projectnessie/tools/contentgenerator/cli/RefreshContent.java @@ -192,7 +192,7 @@ private void commitSameContent( for (Map.Entry entry : contentMap.entrySet()) { Content content = entry.getValue(); - request.operation(Operation.Put.of(entry.getKey(), content, content)); + request.operation(Operation.Put.of(entry.getKey(), content)); } Branch head = request.commit(); diff --git a/versioned/persist/bench/src/main/java/org/projectnessie/versioned/persist/benchmarks/CommitBench.java b/versioned/persist/bench/src/main/java/org/projectnessie/versioned/persist/benchmarks/CommitBench.java index 8d52549637b..de97e43825a 100644 --- a/versioned/persist/bench/src/main/java/org/projectnessie/versioned/persist/benchmarks/CommitBench.java +++ b/versioned/persist/bench/src/main/java/org/projectnessie/versioned/persist/benchmarks/CommitBench.java @@ -249,8 +249,7 @@ private void doCommit( key, // Must add randomness here, otherwise concurrent threads will compute the same // hashes, because parent, "content", key are all the same. - onRef("commit value " + ThreadLocalRandom.current().nextLong(), contentId), - onRef("foo", contentId))); + onRef("commit value " + ThreadLocalRandom.current().nextLong(), contentId))); } bp.versionStore.commit( diff --git a/versioned/persist/store/src/main/java/org/projectnessie/versioned/persist/store/PersistVersionStore.java b/versioned/persist/store/src/main/java/org/projectnessie/versioned/persist/store/PersistVersionStore.java index 67bf139a376..6ac9f27578e 100644 --- a/versioned/persist/store/src/main/java/org/projectnessie/versioned/persist/store/PersistVersionStore.java +++ b/versioned/persist/store/src/main/java/org/projectnessie/versioned/persist/store/PersistVersionStore.java @@ -130,18 +130,10 @@ public Hash commit( if (operation instanceof Put) { Put op = (Put) operation; Content content = op.getValue(); - Content expected = op.getExpectedValue(); if (content.getId() == null) { // No content-ID --> New content - checkArgument( - expected == null, - "Expected content must not be set when creating new content. " - + "The put operation's content has no content ID and is considered as new. " - + "Key: '%s'", - op.getKey()); - // assign content-ID String cid = UUID.randomUUID().toString(); content = STORE_WORKER.applyId(content, cid); @@ -155,21 +147,6 @@ public Hash commit( contentId, payloadForContent(content), STORE_WORKER.toStoreOnReferenceState(content))); - - if (expected != null) { - String expectedId = expected.getId(); - checkArgument( - expectedId != null, - "Content id for expected content must not be null, key '%s'", - op.getKey()); - ContentId expectedContentId = ContentId.of(expectedId); - checkArgument( - contentId.equals(expectedContentId), - "Content ids for new ('%s') and expected ('%s') content differ for key '%s'", - contentId, - expectedContentId, - op.getKey()); - } } else if (operation instanceof Delete) { commitAttempt.addDeletes(operation.getKey()); } else if (operation instanceof Unchanged) { diff --git a/versioned/persist/tests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractCommitScenarios.java b/versioned/persist/tests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractCommitScenarios.java index 22b1490e66e..5b119d11790 100644 --- a/versioned/persist/tests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractCommitScenarios.java +++ b/versioned/persist/tests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractCommitScenarios.java @@ -101,7 +101,6 @@ static Stream commitRenameTableParams() { .boxed() .flatMap(i -> Stream.of(new RenameTable(i, i, i, i), new RenameTable(0, 0, 0, 0))); - // duplicate all params to use and not use global state return Stream.concat(zero, intervals); } diff --git a/versioned/spi/src/main/java/org/projectnessie/versioned/Delete.java b/versioned/spi/src/main/java/org/projectnessie/versioned/Delete.java index bd8a2fe16ba..d8e9806bf8c 100644 --- a/versioned/spi/src/main/java/org/projectnessie/versioned/Delete.java +++ b/versioned/spi/src/main/java/org/projectnessie/versioned/Delete.java @@ -26,6 +26,10 @@ public interface Delete extends Operation { /** * Creates a delete operation for the given key. * + *

If the key for a content shall change (aka a rename), then use a {@link Delete} operation + * using the current (old) key and a {@link Put} operation using the new key providing the {@code + * value} with the correct content ID. + * * @param key the key impacted by the operation * @return a delete operation for the key */ diff --git a/versioned/spi/src/main/java/org/projectnessie/versioned/Put.java b/versioned/spi/src/main/java/org/projectnessie/versioned/Put.java index b40e2c5e286..f4d678711d7 100644 --- a/versioned/spi/src/main/java/org/projectnessie/versioned/Put.java +++ b/versioned/spi/src/main/java/org/projectnessie/versioned/Put.java @@ -16,7 +16,6 @@ package org.projectnessie.versioned; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import org.immutables.value.Value; import org.projectnessie.model.Content; import org.projectnessie.model.ContentKey; @@ -32,48 +31,27 @@ public interface Put extends Operation { */ Content getValue(); - @Nullable - @jakarta.annotation.Nullable - Content getExpectedValue(); - /** - * Creates a put-operation for the given key and value without an expected-value so the returned - * put-operation is unconditional. + * Creates a put-operation for the given key and value. * - *

Unconditional put-operations must be used for content-types that do not support global-state - * and for those that do support global-state when a new content object is added. + *

{@code value} with a {@code null} content ID is required when creating/adding new + * content. * - * @param key the key impacted by the operation - * @param value the new value associated with the key - * @return a put operation for the key and value - */ - @Nonnull - @jakarta.annotation.Nonnull - static Put of( - @Nonnull @jakarta.annotation.Nonnull ContentKey key, - @Nonnull @jakarta.annotation.Nonnull Content value) { - return ImmutablePut.builder().key(key).value(value).build(); - } - - /** - * Creates a conditional put-operation for the given key and value with an expected-value so the - * returned put-operation will check whether the current state in Nessie matches the expected - * state in {@code expectedValue}. + *

{@code value} with a non-{@code null} content ID is required when updating existing + * content. * - *

Using a conditional put-operation for a content-type that does not support global-state - * results in an error. + *

A content object is considered to be the same using the {@link ContentKey content-key} and + * the {@link Content#getId() content-id}. * * @param key the key impacted by the operation * @param value the new value associated with the key - * @param expectedValue the expected value associated with the key * @return a put operation for the key and value */ @Nonnull @jakarta.annotation.Nonnull static Put of( @Nonnull @jakarta.annotation.Nonnull ContentKey key, - @Nonnull @jakarta.annotation.Nonnull Content value, - @Nonnull @jakarta.annotation.Nonnull Content expectedValue) { - return ImmutablePut.builder().key(key).value(value).expectedValue(expectedValue).build(); + @Nonnull @jakarta.annotation.Nonnull Content value) { + return ImmutablePut.builder().key(key).value(value).build(); } } diff --git a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/CommitImpl.java b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/CommitImpl.java index f2f7086f53b..0ca66ca00eb 100644 --- a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/CommitImpl.java +++ b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/CommitImpl.java @@ -269,7 +269,6 @@ private static void commitAddUnchanged( // TODO add much stricter handling of Delete against existing content, but that requires // changes to the model - // TODO require expectedContent for existing content // TODO validate content-ID in store-index against content-ID in operation if (expectedElement != null) { @@ -349,15 +348,12 @@ private void commitAddPut( UUID existingContentID; String expectedContentIDString; - Content expectedContent = put.getExpectedValue(); - ContentMapping contentMapping = new ContentMapping(persist); StoreIndexElement existing = expectedIndex.get(storeKey); - String expectedUUID = expectedContent != null ? expectedContent.getId() : null; - if (existing == null && expectedUUID != null) { + if (existing == null && putValueId != null) { // Check for a Delete-op in the same commit, representing a rename operation. - UUID expectedContentID = UUID.fromString(expectedUUID); + UUID expectedContentID = UUID.fromString(putValueId); StoreKey deletedKey = deleted.remove(expectedContentID); if (deletedKey != null) { existing = expectedIndex.get(deletedKey); @@ -387,23 +383,10 @@ private void commitAddPut( expectedContentIDString, putValueId); - checkArgument( - expectedContent != null, - "Key '%s' already exists, but Put-operation has no expectedValue", - putKey); - - checkArgument( - expectedUUID != null, "Expected value to update key '%s' has no content iD", putKey); - exists = true; } } if (!exists) { - checkArgument( - expectedContent == null, - "Key '%s' does not exist, but Put-operation has expectedValue", - putKey); - checkArgument(putValueId == null, "New value for new must not have a content iD"); newContent.accept(putKey, putValue); diff --git a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractCommitLog.java b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractCommitLog.java index b7715d2b17b..69104392ef1 100644 --- a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractCommitLog.java +++ b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractCommitLog.java @@ -74,10 +74,7 @@ public void commitLogPaging() throws Exception { .hashOnReference( branch, Optional.of(i == 0 ? createHash : commitHashes[i - 1])), key); - Put op = - value != null - ? Put.of(key, onRef(str, value.getId()), value) - : Put.of(key, newOnRef(str)); + Put op = value != null ? Put.of(key, onRef(str, value.getId())) : Put.of(key, newOnRef(str)); commitHashes[i] = store().commit(branch, Optional.of(parent), msg.build(), ImmutableList.of(op)); diff --git a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractCommits.java b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractCommits.java index 4589afcf3e2..607aa2a2430 100644 --- a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractCommits.java +++ b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractCommits.java @@ -148,7 +148,7 @@ public void commitSomeOperations() throws Exception { Hash secondCommit = commit("Second Commit") - .put("t1", V_1_2.withId(t1), t1) + .put("t1", V_1_2.withId(t1)) .delete("t2") .delete("t3") .put("t4", V_4_1) @@ -259,7 +259,7 @@ public void commitNonConflictingOperations() throws Exception { Hash t1Commit = commit("T1 Commit") .fromReference(initialCommit) - .put("t1", V_1_2.withId(t1), t1) + .put("t1", V_1_2.withId(t1)) .toBranch(branch); t1 = store().getValue(branch, ContentKey.of("t1")); @@ -269,8 +269,8 @@ public void commitNonConflictingOperations() throws Exception { Hash extraCommit = commit("Extra Commit") .fromReference(t1Commit) - .put("t1", V_1_3.withId(t1), t1) - .put("t3", V_3_2.withId(t3), t3) + .put("t1", V_1_3.withId(t1)) + .put("t3", V_3_2.withId(t3)) .toBranch(branch); Hash newT2Commit = commit("New T2 Commit").fromReference(t2Commit).put("t2", NEW_v2_1).toBranch(branch); @@ -391,7 +391,7 @@ public void commitConflictingOperationsLegacy() throws Exception { Hash secondCommit = commit("Second Commit") - .put("t1", V_1_2.withId(t1), t1) + .put("t1", V_1_2.withId(t1)) .delete("t2") .put("t3", V_3_1) .toBranch(branch); @@ -471,7 +471,7 @@ public void commitConflictingOperations() throws Exception { Hash secondCommit = commit("Second Commit") - .put("t1", V_1_2.withId(t1), t1) + .put("t1", V_1_2.withId(t1)) .delete("t2") .put("t3", V_3_1) .toBranch(branch); @@ -511,14 +511,6 @@ public void commitConflictingOperations() throws Exception { .fromReference(initialCommit) .put("t1", V_1_3.withId(t1)) .toBranch(branch)) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Key 't1' already exists, but Put-operation has no expectedValue"); - soft.assertThatThrownBy( - () -> - commit("Conflicting Commit") - .fromReference(initialCommit) - .put("t1", V_1_3.withId(t1), t1) - .toBranch(branch)) .isInstanceOf(ReferenceConflictException.class) .hasMessage( "There are conflicts that prevent committing the provided operations: values of existing and expected content for key 't1' are different."); @@ -526,7 +518,7 @@ public void commitConflictingOperations() throws Exception { () -> commit("Conflicting Commit") .fromReference(initialCommit) - .put("t2", V_2_2.withId(t2), t2) + .put("t2", V_2_2.withId(t2)) .toBranch(branch)) .isInstanceOf(ReferenceConflictException.class) .hasMessage( @@ -535,10 +527,10 @@ public void commitConflictingOperations() throws Exception { () -> commit("Conflicting Commit") .fromReference(initialCommit) - .put("t3", V_3_2.withId(t3), t3) + .put("t3", V_3_2.withId(t3)) .toBranch(branch)) .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Key 't3' does not exist, but Put-operation has expectedValue"); + .hasMessage("New value for new must not have a content iD"); soft.assertThatThrownBy( () -> commit("Conflicting Commit") @@ -602,7 +594,7 @@ public void forceCommitConflictingOperations() throws Exception { Content t1 = store().getValue(branch, ContentKey.of("t1")); commit("Second Commit") - .put("t1", V_1_2.withId(t1), t1) + .put("t1", V_1_2.withId(t1)) .delete("t2") .put("t3", V_3_1) .toBranch(branch); @@ -610,9 +602,9 @@ public void forceCommitConflictingOperations() throws Exception { Hash putCommit = forceCommit("Conflicting Commit") - .put("t1", V_1_3.withId(t1), t1) + .put("t1", V_1_3.withId(t1)) .put("t2", V_2_2) - .put("t3", V_3_2.withId(t3), t3) + .put("t3", V_3_2.withId(t3)) .toBranch(branch); soft.assertThat(store().hashOnReference(branch, Optional.empty())).isEqualTo(putCommit); diff --git a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractDiff.java b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractDiff.java index b84f4b6b7d9..3c3c763f295 100644 --- a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractDiff.java +++ b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractDiff.java @@ -60,7 +60,7 @@ protected void checkDiff() throws VersionStoreException { Content k2 = store().getValue(branch, ContentKey.of("k2")); Hash secondCommit = commit("Second Commit") - .put("k2", V_2_A.withId(k2), k2) + .put("k2", V_2_A.withId(k2)) .put("k3", V_3) .put("k1a", V_1_A) .toBranch(branch); diff --git a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractMerge.java b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractMerge.java index 78d9bc3c668..57fccc00b25 100644 --- a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractMerge.java +++ b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractMerge.java @@ -99,7 +99,7 @@ protected void setupCommits() throws VersionStoreException { Content t1 = store().getValue(branch, ContentKey.of("t1")); commit("Second Commit") - .put("t1", V_1_2.withId(t1), t1) + .put("t1", V_1_2.withId(t1)) .delete("t2") .delete("t3") .put("t4", V_4_1) @@ -501,7 +501,7 @@ protected void nonEmptyFastForwardMerge(boolean individualCommits) throws Versio etl, Optional.empty(), CommitMeta.fromMessage("commit 2"), - singletonList(Put.of(key, VALUE_2.withId(v), v))); + singletonList(Put.of(key, VALUE_2.withId(v)))); MergeResult mergeResult2 = store() .merge( diff --git a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractNestedVersionStore.java b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractNestedVersionStore.java index 15f5160a872..0d66ba0ec7c 100644 --- a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractNestedVersionStore.java +++ b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractNestedVersionStore.java @@ -172,12 +172,7 @@ protected static List operationsWithoutCo op -> { if (op instanceof Put) { Put put = (Put) op; - return put.getExpectedValue() != null - ? Put.of( - put.getKey(), - contentWithoutId(put.getValue()), - contentWithoutId(put.getExpectedValue())) - : Put.of(put.getKey(), contentWithoutId(put.getValue())); + return Put.of(put.getKey(), contentWithoutId(put.getValue())); } return op; }) diff --git a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractSingleBranch.java b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractSingleBranch.java index 6f097b53dd2..70a4bc66b61 100644 --- a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractSingleBranch.java +++ b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractSingleBranch.java @@ -150,14 +150,11 @@ private List singleBranchManyUsersOps( List ops; Content existing = store().getValue(store.hashOnReference(branch, Optional.of(hashKnownByUser)), key); - if (existing != null) { - Content value = - onRef(String.format("data_file_%03d_%03d", user, commitNum), existing.getId()); - ops = ImmutableList.of(Put.of(key, value, existing)); - } else { - Content value = newOnRef(String.format("data_file_%03d_%03d", user, commitNum)); - ops = ImmutableList.of(Put.of(key, value)); - } + Content value = + existing != null + ? onRef(String.format("data_file_%03d_%03d", user, commitNum), existing.getId()) + : newOnRef(String.format("data_file_%03d_%03d", user, commitNum)); + ops = ImmutableList.of(Put.of(key, value)); return ops; } } diff --git a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractTransplant.java b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractTransplant.java index 6bef7519089..9fc91e3cd76 100644 --- a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractTransplant.java +++ b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/AbstractTransplant.java @@ -105,7 +105,7 @@ protected void setupCommits() throws VersionStoreException { secondCommit = commit("Second Commit") - .put("t1", V_1_2.withId(t1), t1) + .put("t1", V_1_2.withId(t1)) .delete(T_2) .delete(T_3) .put(T_4, V_4_1) @@ -333,7 +333,7 @@ protected void checkTransplantWithNoExpectedHash(boolean individualCommits) store().create(newBranch, Optional.empty()); commit("Another commit").put(T_5, V_5_1).toBranch(newBranch); Content t5 = store().getValue(newBranch, ContentKey.of(T_5)); - commit("Another commit").put(T_5, V_1_4.withId(t5), t5).toBranch(newBranch); + commit("Another commit").put(T_5, V_1_4.withId(t5)).toBranch(newBranch); store() .transplant( diff --git a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/CommitBuilder.java b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/CommitBuilder.java index f61cae2c9a1..68338607ff7 100644 --- a/versioned/tests/src/main/java/org/projectnessie/versioned/tests/CommitBuilder.java +++ b/versioned/tests/src/main/java/org/projectnessie/versioned/tests/CommitBuilder.java @@ -55,10 +55,6 @@ public CommitBuilder put(String key, Content value) { return put(ContentKey.of(key), value); } - public CommitBuilder put(String key, Content value, Content expected) { - return put(ContentKey.of(key), value, expected); - } - /** * Adds a put operation to the current commit. * @@ -70,10 +66,6 @@ public CommitBuilder put(ContentKey key, Content value) { return add(Put.of(key, value)); } - public CommitBuilder put(ContentKey key, Content value, Content expected) { - return add(Put.of(key, value, expected)); - } - /** * Adds a delete operation to the current commit. *