diff --git a/api/model/src/main/java/org/projectnessie/model/MergeResponse.java b/api/model/src/main/java/org/projectnessie/model/MergeResponse.java index 120fd771643..94e9423dc0f 100644 --- a/api/model/src/main/java/org/projectnessie/model/MergeResponse.java +++ b/api/model/src/main/java/org/projectnessie/model/MergeResponse.java @@ -73,12 +73,14 @@ default boolean wasSuccessful() { @Deprecated // for removal and replaced with something else @Schema(deprecated = true, hidden = true) + @JsonView(Views.V1.class) List getSourceCommits(); @Nullable @jakarta.annotation.Nullable @Deprecated // for removal and replaced with something else @Schema(deprecated = true, hidden = true) + @JsonView(Views.V1.class) List getTargetCommits(); /** Details of all keys encountered during the merge or transplant operation. */ @@ -93,18 +95,23 @@ interface ContentKeyDetails { MergeBehavior getMergeBehavior(); + @Deprecated // for removal, #getConflict() is a proper replacement @Value.Default @JsonDeserialize(using = ContentKeyConflict.Deserializer.class) + @Schema(deprecated = true, hidden = true) + @JsonView(Views.V1.class) default ContentKeyConflict getConflictType() { return ContentKeyConflict.NONE; } @Deprecated // for removal and replaced with something else @Schema(deprecated = true, hidden = true) + @JsonView(Views.V1.class) List getSourceCommits(); @Deprecated // for removal and replaced with something else @Schema(deprecated = true, hidden = true) + @JsonView(Views.V1.class) List getTargetCommits(); /** {@link Conflict} details, if available. */ @@ -115,6 +122,7 @@ default ContentKeyConflict getConflictType() { Conflict getConflict(); } + @Deprecated // for removal enum ContentKeyConflict { NONE, UNRESOLVABLE; diff --git a/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java b/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java index d2191638101..66f3fdb38a8 100644 --- a/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java +++ b/servers/jax-rs-tests/src/main/java/org/projectnessie/jaxrs/tests/BaseTestNessieApi.java @@ -44,6 +44,7 @@ import java.util.stream.Stream; import javax.validation.constraints.NotNull; import org.assertj.core.api.AbstractThrowableAssert; +import org.assertj.core.api.ListAssert; import org.assertj.core.api.SoftAssertions; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -87,7 +88,8 @@ import org.projectnessie.model.ImmutableReferenceMetadata; import org.projectnessie.model.LogResponse; import org.projectnessie.model.LogResponse.LogEntry; -import org.projectnessie.model.MergeResponse; +import org.projectnessie.model.MergeResponse.ContentKeyConflict; +import org.projectnessie.model.MergeResponse.ContentKeyDetails; import org.projectnessie.model.Namespace; import org.projectnessie.model.NessieConfiguration; import org.projectnessie.model.Operation; @@ -572,7 +574,8 @@ public void mergeTransplantDryRunWithConflictInResult() throws Exception { Branch main = prepCommit(main0, "main", dummyPut("conflictingKey1"), dummyPut("mainKey")).commit(); - soft.assertThat( + ListAssert mergeAssert = + soft.assertThat( api() .mergeRefIntoBranch() .fromRef(branch) @@ -580,18 +583,27 @@ public void mergeTransplantDryRunWithConflictInResult() throws Exception { .returnConflictAsResult(true) // adds coverage on top of AbstractMerge tests .dryRun(true) .merge() - .getDetails()) - .extracting( - MergeResponse.ContentKeyDetails::getKey, - MergeResponse.ContentKeyDetails::getConflictType) - .contains( - tuple(ContentKey.of("branchKey"), MergeResponse.ContentKeyConflict.NONE), - tuple(ContentKey.of("conflictingKey1"), MergeResponse.ContentKeyConflict.UNRESOLVABLE)); + .getDetails()); + if (isV2()) { + mergeAssert + // old model returns "UNKNOWN" for Conflict.conflictType(), new model returns KEY_EXISTS + .extracting(ContentKeyDetails::getKey, d -> d.getConflict() != null) + .contains( + tuple(ContentKey.of("branchKey"), false), + tuple(ContentKey.of("conflictingKey1"), true)); + } else { + mergeAssert + .extracting(ContentKeyDetails::getKey, ContentKeyDetails::getConflictType) + .contains( + tuple(ContentKey.of("branchKey"), ContentKeyConflict.NONE), + tuple(ContentKey.of("conflictingKey1"), ContentKeyConflict.UNRESOLVABLE)); + } // Assert no change to the ref HEAD soft.assertThat(api().getReference().refName(main.getName()).get()).isEqualTo(main); - soft.assertThat( + mergeAssert = + soft.assertThat( api() .transplantCommitsIntoBranch() .fromRefName(branch.getName()) @@ -600,14 +612,21 @@ public void mergeTransplantDryRunWithConflictInResult() throws Exception { .returnConflictAsResult(true) // adds coverage on top of AbstractTransplant tests .dryRun(true) .transplant() - .getDetails()) - .extracting( - MergeResponse.ContentKeyDetails::getKey, - MergeResponse.ContentKeyDetails::getConflictType) - .contains( - tuple(ContentKey.of("branchKey"), MergeResponse.ContentKeyConflict.NONE), - tuple(ContentKey.of("conflictingKey1"), MergeResponse.ContentKeyConflict.UNRESOLVABLE)); - + .getDetails()); + if (isV2()) { + mergeAssert + // old model returns "UNKNOWN" for Conflict.conflictType(), new model returns KEY_EXISTS + .extracting(ContentKeyDetails::getKey, d -> d.getConflict() != null) + .contains( + tuple(ContentKey.of("branchKey"), false), + tuple(ContentKey.of("conflictingKey1"), true)); + } else { + mergeAssert + .extracting(ContentKeyDetails::getKey, ContentKeyDetails::getConflictType) + .contains( + tuple(ContentKey.of("branchKey"), ContentKeyConflict.NONE), + tuple(ContentKey.of("conflictingKey1"), ContentKeyConflict.UNRESOLVABLE)); + } // Assert no change to the ref HEAD soft.assertThat(api().getReference().refName(main.getName()).get()).isEqualTo(main); } diff --git a/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java b/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java index b4602ff0f3a..a888ed375ea 100644 --- a/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java +++ b/versioned/persist/adapter/src/main/java/org/projectnessie/versioned/persist/adapter/spi/AbstractDatabaseAdapter.java @@ -2046,7 +2046,12 @@ protected boolean hasKeyCollisions( removeKeyCollisionsForNamespaces( ctx, refHead, commitsChronological.get(0).getHash(), keyCollisions); if (!keyCollisions.isEmpty()) { - keyCollisions.forEach(key -> keyDetails.apply(key).conflictType(ConflictType.UNRESOLVABLE)); + keyCollisions.forEach( + key -> + keyDetails + .apply(key) + .conflict(Conflict.conflict(Conflict.ConflictType.UNKNOWN, key, "UNRESOLVABLE")) + .conflictType(ConflictType.UNRESOLVABLE)); return true; } } diff --git a/versioned/persist/tests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractMergeTransplant.java b/versioned/persist/tests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractMergeTransplant.java index bce231e873d..20c750a1c28 100644 --- a/versioned/persist/tests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractMergeTransplant.java +++ b/versioned/persist/tests/src/main/java/org/projectnessie/versioned/persist/tests/AbstractMergeTransplant.java @@ -33,6 +33,7 @@ import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.CsvSource; +import org.projectnessie.model.Conflict; import org.projectnessie.model.ContentKey; import org.projectnessie.model.MergeBehavior; import org.projectnessie.nessie.relocated.protobuf.ByteString; @@ -458,7 +459,10 @@ private MergeResult conflictExpectedMergeResult( .collect(Collectors.toList())); if (k < 2) { - details.conflictType(ConflictType.UNRESOLVABLE).addTargetCommits(conflictHead); + details + .conflict(Conflict.conflict(Conflict.ConflictType.UNKNOWN, key, "UNRESOLVABLE")) + .conflictType(ConflictType.UNRESOLVABLE) + .addTargetCommits(conflictHead); } expectedMergeResult.putDetails(key, details.build()); diff --git a/versioned/spi/src/main/java/org/projectnessie/versioned/MergeConflictException.java b/versioned/spi/src/main/java/org/projectnessie/versioned/MergeConflictException.java index 4cac148159c..b7edc45718b 100644 --- a/versioned/spi/src/main/java/org/projectnessie/versioned/MergeConflictException.java +++ b/versioned/spi/src/main/java/org/projectnessie/versioned/MergeConflictException.java @@ -44,8 +44,7 @@ private static List asReferenceConflicts(MergeResult mergeResult) { Conflict conflict = keyDetails.getConflict(); return conflict != null ? conflict - : Conflict.conflict( - ConflictType.KEY_CONFLICT, e.getKey(), e.getValue().getConflictType().name()); + : Conflict.conflict(ConflictType.KEY_CONFLICT, e.getKey(), "UNKNOWN"); }) .collect(Collectors.toList()); } diff --git a/versioned/spi/src/main/java/org/projectnessie/versioned/MergeResult.java b/versioned/spi/src/main/java/org/projectnessie/versioned/MergeResult.java index f0e5b364633..d1e98b27696 100644 --- a/versioned/spi/src/main/java/org/projectnessie/versioned/MergeResult.java +++ b/versioned/spi/src/main/java/org/projectnessie/versioned/MergeResult.java @@ -80,6 +80,7 @@ interface KeyDetails { @Value.Parameter(order = 1) MergeBehavior getMergeBehavior(); + @Deprecated // for removal, #getConflict() is a proper replacement @Value.Default @Value.Parameter(order = 2) default ConflictType getConflictType() { @@ -102,16 +103,15 @@ static ImmutableKeyDetails.Builder builder() { return ImmutableKeyDetails.builder(); } - static KeyDetails keyDetails(MergeBehavior mergeBehavior, ConflictType conflictType) { - return keyDetails(mergeBehavior, conflictType, null); - } - - static KeyDetails keyDetails( - MergeBehavior mergeBehavior, ConflictType conflictType, Conflict conflict) { - return ImmutableKeyDetails.of(mergeBehavior, conflictType, conflict); + static KeyDetails keyDetails(MergeBehavior mergeBehavior, Conflict conflict) { + return ImmutableKeyDetails.of( + mergeBehavior, + conflict != null ? ConflictType.UNRESOLVABLE : ConflictType.NONE, + conflict); } } + @Deprecated // for removal enum ConflictType { NONE, UNRESOLVABLE diff --git a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/BaseCommitHelper.java b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/BaseCommitHelper.java index f93b3541ad5..31ef9da7819 100644 --- a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/BaseCommitHelper.java +++ b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/BaseCommitHelper.java @@ -65,7 +65,6 @@ import org.projectnessie.versioned.Hash; import org.projectnessie.versioned.ImmutableMergeResult; import org.projectnessie.versioned.MergeResult; -import org.projectnessie.versioned.MergeResult.ConflictType; import org.projectnessie.versioned.MergeResult.KeyDetails; import org.projectnessie.versioned.ReferenceConflictException; import org.projectnessie.versioned.ReferenceNotFoundException; @@ -530,7 +529,7 @@ ImmutableMergeResult mergeSquashFastForward( ContentKey key = storeKeyToKey(k); // Note: key==null, if not the "main universe" or not a "content" discriminator if (key != null) { - result.putDetails(key, keyDetails(mergeBehaviors.mergeBehavior(key), ConflictType.NONE)); + result.putDetails(key, keyDetails(mergeBehaviors.mergeBehavior(key), null)); } } @@ -606,7 +605,7 @@ && contentTypeForPayload((byte) op.payload()) == NAMESPACE) { // Do not plain ignore (due to FORCE) or drop (DROP), when the caller provided an // expectedTargetContent. if (mergeKeyBe.getExpectedTargetContent() == null) { - keyDetailsMap.put(key, keyDetails(mergeBehavior, ConflictType.NONE)); + keyDetailsMap.put(key, keyDetails(mergeBehavior, null)); return mergeBehavior == MergeBehavior.FORCE ? ConflictResolution.ADD : ConflictResolution.DROP; @@ -614,11 +613,7 @@ && contentTypeForPayload((byte) op.payload()) == NAMESPACE) { // fall through case NORMAL: keyDetailsMap.put( - key, - keyDetails( - mergeBehavior, - ConflictType.UNRESOLVABLE, - commitConflictToConflict(conflict))); + key, keyDetails(mergeBehavior, commitConflictToConflict(conflict))); return ConflictResolution.ADD; default: throw new IllegalStateException("Unknown merge behavior " + mergeBehavior); @@ -633,8 +628,7 @@ && contentTypeForPayload((byte) op.payload()) == NAMESPACE) { ContentKey key = storeKeyToKey(storeKey); // Note: key==null, if not the "main universe" or not a "content" discriminator if (key != null) { - keyDetailsMap.putIfAbsent( - key, keyDetails(mergeBehaviors.mergeBehavior(key), ConflictType.NONE)); + keyDetailsMap.putIfAbsent(key, keyDetails(mergeBehaviors.mergeBehavior(key), null)); } }, /* @@ -709,9 +703,7 @@ boolean recordKeyDetailsAndCheckConflicts( boolean hasConflicts = false; for (Entry keyDetail : keyDetailsMap.entrySet()) { KeyDetails details = keyDetail.getValue(); - if (details.getConflictType() == ConflictType.UNRESOLVABLE) { - hasConflicts = true; - } + hasConflicts |= details.getConflict() != null; mergeResult.putDetails(keyDetail.getKey(), details); } return hasConflicts; diff --git a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/VersionStoreImpl.java b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/VersionStoreImpl.java index 8094a962bda..08185394aab 100644 --- a/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/VersionStoreImpl.java +++ b/versioned/storage/store/src/main/java/org/projectnessie/versioned/storage/versionstore/VersionStoreImpl.java @@ -81,7 +81,6 @@ import org.projectnessie.versioned.KeyEntry; import org.projectnessie.versioned.MergeConflictException; import org.projectnessie.versioned.MergeResult; -import org.projectnessie.versioned.MergeResult.ConflictType; import org.projectnessie.versioned.MetadataRewriter; import org.projectnessie.versioned.NamedRef; import org.projectnessie.versioned.Operation; @@ -687,7 +686,7 @@ private MergeResult mergeTransplantResponse(MergeResult mergeRes String.format( "The following keys have been changed in conflict: %s", mergeResult.getDetails().entrySet().stream() - .filter(e -> e.getValue().getConflictType() != ConflictType.NONE) + .filter(e -> e.getValue().getConflict() != null) .map(Map.Entry::getKey) .sorted() .map(key -> String.format("'%s'", key)) 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 e75ed07db15..514fd987e99 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 @@ -228,7 +228,6 @@ protected void mergeKeyBehaviorValidation(boolean dryRun) throws Exception { false, KeyDetails.keyDetails( mergeBehavior, - MergeResult.ConflictType.UNRESOLVABLE, Conflict.conflict( ConflictType.VALUE_DIFFERS, keyT3,