Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Change default message for (squash) merges #6642

Merged
merged 1 commit into from
Apr 25, 2023
Merged

Change default message for (squash) merges #6642

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 18 additions & 12 deletions servers/services/src/main/java/org/projectnessie/services/impl/BaseApiImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.UUID;
import java.util.function.IntFunction;
import java.util.function.Supplier;
import javax.annotation.Nullable;
import org.projectnessie.error.NessieReferenceNotFoundException;
Expand Down Expand Up @@ -135,7 +136,8 @@ protected ServerAccessContext createAccessContext() {
}

protected MetadataRewriter<CommitMeta> commitMetaUpdate(
@Nullable @jakarta.annotation.Nullable CommitMeta commitMeta) {
@Nullable @jakarta.annotation.Nullable CommitMeta commitMeta,
IntFunction<String> squashMessage) {
return new MetadataRewriter<CommitMeta>() {
// Used for setting contextual commit properties during new and merge/transplant commits.
// WARNING: ONLY SET PROPERTIES, WHICH APPLY COMMONLY TO ALL COMMIT TYPES.
Expand Down Expand Up @@ -184,7 +186,9 @@ public CommitMeta rewriteSingle(CommitMeta metadata) {

@Override
public CommitMeta squash(List<CommitMeta> metadata) {
if (metadata.size() == 1) {
Optional<String> msg = Optional.ofNullable(squashMessage.apply(metadata.size()));

if (metadata.size() == 1 && !msg.isPresent()) {
return rewriteSingle(metadata.get(0));
}

Expand All @@ -195,16 +199,18 @@ public CommitMeta squash(List<CommitMeta> metadata) {

return buildCommitMeta(
CommitMeta.builder().properties(newProperties),
() -> {
StringBuilder newMessage = new StringBuilder();
for (CommitMeta commitMeta : metadata) {
if (newMessage.length() > 0) {
newMessage.append("\n---------------------------------------------\n");
}
newMessage.append(commitMeta.getMessage());
}
return newMessage.toString();
});
() ->
msg.orElseGet(
() -> {
StringBuilder newMessage = new StringBuilder();
for (CommitMeta commitMeta : metadata) {
if (newMessage.length() > 0) {
newMessage.append("\n---------------------------------------------\n");
}
newMessage.append(commitMeta.getMessage());
}
return newMessage.toString();
}));
}
};
}
Expand Down
3 changes: 2 additions & 1 deletion servers/services/src/main/java/org/projectnessie/services/impl/NamespaceApiImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,8 @@ private Hash commit(
.commit(
branch,
Optional.empty(),
commitMetaUpdate(null).rewriteSingle(CommitMeta.fromMessage(commitMsg)),
commitMetaUpdate(null, numCommits -> null)
.rewriteSingle(CommitMeta.fromMessage(commitMsg)),
Collections.singletonList(contentOperation),
validator,
(k, c) -> {});
Expand Down
39 changes: 30 additions & 9 deletions servers/services/src/main/java/org/projectnessie/services/impl/TreeApiImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,11 +568,9 @@ public MergeResponse transplantCommitsIntoBranch(
validateCommitMeta(commitMeta);

BranchName targetBranch = BranchName.of(branchName);
String lastHash = hashesToTransplant.get(hashesToTransplant.size() - 1);
startAccessCheck()
.canViewReference(
namedRefWithHashOrThrow(
fromRefName, hashesToTransplant.get(hashesToTransplant.size() - 1))
.getValue())
.canViewReference(namedRefWithHashOrThrow(fromRefName, lastHash).getValue())
.canCommitChangeAgainstReference(targetBranch)
.checkAndThrow();

Expand All @@ -587,13 +585,24 @@ public MergeResponse transplantCommitsIntoBranch(
commitMeta = null;
}

Optional<Hash> into = toHash(expectedHash, true);

MergeResult<Commit> result =
getStore()
.transplant(
targetBranch,
toHash(expectedHash, true),
into,
transplants,
commitMetaUpdate(commitMeta),
commitMetaUpdate(
commitMeta,
numCommits ->
String.format(
"Transplanted %d commits from %s at %s into %s%s",
numCommits,
fromRefName,
lastHash,
branchName,
into.map(h -> " at " + h.asString()).orElse(""))),
Boolean.TRUE.equals(keepIndividualCommits),
keyMergeBehaviors(keyMergeBehaviors),
defaultMergeBehavior(defaultMergeBehavior),
Expand Down Expand Up @@ -637,13 +646,25 @@ public MergeResponse mergeRefIntoBranch(
.canCommitChangeAgainstReference(targetBranch)
.checkAndThrow();

Hash from = toHash(fromRefName, fromHash);
Optional<Hash> into = toHash(expectedHash, true);

MergeResult<Commit> result =
getStore()
.merge(
toHash(fromRefName, fromHash),
targetBranch,
toHash(expectedHash, true),
commitMetaUpdate(commitMeta),
into,
commitMetaUpdate(
commitMeta,
numCommits ->
String.format(
"Merged %d commits from %s at %s into %s%s",
numCommits,
fromRefName,
from.asString(),
branchName,
into.map(h -> " at " + h.asString()).orElse(""))),
Boolean.TRUE.equals(keepIndividualCommits),
keyMergeBehaviors(keyMergeBehaviors),
defaultMergeBehavior(defaultMergeBehavior),
Expand Down Expand Up @@ -924,7 +945,7 @@ public CommitResponse commitMultipleOperations(
.commit(
BranchName.of(branch),
Optional.ofNullable(expectedHash).map(Hash::of),
commitMetaUpdate(null).rewriteSingle(commitMeta),
commitMetaUpdate(null, numCommits -> null).rewriteSingle(commitMeta),
ops,
() -> null,
(key, cid) -> {
Expand Down
102 changes: 77 additions & 25 deletions ...es/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestMergeTransplant.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.projectnessie.services.impl;

import static java.lang.String.format;
import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
import static java.util.Objects.requireNonNull;
Expand Down Expand Up @@ -100,7 +101,9 @@ private void testTransplant(boolean withDetachedCommit, boolean keepIndividualCo
NORMAL,
false,
false,
returnConflictAsResult));
returnConflictAsResult),
withDetachedCommit,
"Transplanted");
}

@ParameterizedTest
Expand Down Expand Up @@ -135,7 +138,9 @@ private void testMerge(ReferenceMode refMode, boolean keepIndividualCommits)
false,
false,
returnConflictAsResult);
});
},
refMode == ReferenceMode.DETACHED,
"Merged");
}

@FunctionalInterface
Expand All @@ -150,7 +155,11 @@ MergeResponse act(
}

private void mergeTransplant(
boolean verifyAdditionalParents, boolean keepIndividualCommits, MergeTransplantActor actor)
boolean verifyAdditionalParents,
boolean keepIndividualCommits,
MergeTransplantActor actor,
boolean detached,
String mergedTransplanted)
throws BaseNessieClientServerException {
Branch root = createBranch("root");

Expand Down Expand Up @@ -221,8 +230,14 @@ private void mergeTransplant(
soft.assertThat(log.stream().map(LogEntry::getCommitMeta).map(CommitMeta::getMessage))
.hasSize(3)
.first(InstanceOfAssertFactories.STRING)
.contains("test-branch2")
.contains("test-branch1");
.isEqualTo(
format(
"%s 2 commits from %s at %s into %s at %s",
mergedTransplanted,
detached ? "DETACHED" : source.getName(),
committed2.getHash(),
target.getName(),
target.getHash()));
}

// Verify that the commit-timestamp was updated
Expand Down Expand Up @@ -384,23 +399,58 @@ public void mergeMessage() throws BaseNessieClientServerException {

@Test
public void mergeMessageDefault() throws BaseNessieClientServerException {
testMergeTransplantMessage(
(target, source, committed1, committed2, returnConflictAsResult) ->
treeApi()
.mergeRefIntoBranch(
target.getName(),
target.getHash(),
source.getName(),
source.getHash(),
false,
null,
emptyList(),
NORMAL,
false,
false,
returnConflictAsResult),
ImmutableList.of(
"test-commit-1\n---------------------------------------------\ntest-commit-2"));
Branch target = createBranch("merge-transplant-msg-target");

// Common ancestor
target =
commit(
target,
fromMessage("test-root"),
Put.of(
ContentKey.of("irrelevant-to-this-test"),
IcebergTable.of("something", 42, 43, 44, 45)))
.getTargetBranch();

Branch source = createBranch("merge-transplant-msg-source", target);

ContentKey key1 = ContentKey.of("test-key1");
ContentKey key2 = ContentKey.of("test-key2");

source =
commit(
source,
fromMessage("test-commit-1"),
Put.of(key1, IcebergTable.of("table1", 42, 43, 44, 45)))
.getTargetBranch();

source =
commit(
source,
fromMessage("test-commit-2"),
Put.of(key2, IcebergTable.of("table2", 42, 43, 44, 45)))
.getTargetBranch();

treeApi()
.mergeRefIntoBranch(
target.getName(),
target.getHash(),
source.getName(),
source.getHash(),
false,
null,
emptyList(),
NORMAL,
false,
false,
false);

soft.assertThat(commitLog(target.getName()).stream().limit(1))
.isNotEmpty()
.extracting(e -> e.getCommitMeta().getMessage())
.containsExactly(
format(
"Merged 2 commits from %s at %s into %s at %s",
source.getName(), source.getHash(), target.getName(), target.getHash()));
}

@Test
Expand Down Expand Up @@ -577,9 +627,11 @@ public void mergeWithNamespaces(ReferenceMode refMode) throws BaseNessieClientSe
List<LogEntry> log = commitLog(base.getName(), MINIMAL, base.getHash(), null, null);
soft.assertThat(
log.stream().map(LogEntry::getCommitMeta).map(CommitMeta::getMessage).findFirst())
.isPresent()
.hasValueSatisfying(v -> assertThat(v).contains("test-branch1"))
.hasValueSatisfying(v -> assertThat(v).contains("test-branch2"));
.get()
.isEqualTo(
format(
"Merged 4 commits from %s at %s into %s at %s",
fromRef.getName(), fromRef.getHash(), base.getName(), base.getHash()));

soft.assertThat(
withoutNamespaces(entries(base.getName(), null)).stream()
Expand Down
24 changes: 15 additions & 9 deletions ...ervices/src/testFixtures/java/org/projectnessie/services/impl/AbstractTestPrincipals.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package org.projectnessie.services.impl;

import static java.lang.String.format;
import static java.util.Collections.emptyList;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.groups.Tuple.tuple;
Expand Down Expand Up @@ -98,18 +99,23 @@ public void committerAndAuthorMerge() throws Exception {
false,
false);

merge = (Branch) getReference(merge.getName());
Branch merged = (Branch) getReference(merge.getName());

assertThat(commitLog(merge.getName()).stream().limit(1).collect(Collectors.toList()))
.first()
.extracting(LogEntry::getCommitMeta)
.allSatisfy(
commitMeta -> {
assertThat(commitMeta.getCommitter()).isEqualTo("NessieHerself");
assertThat(commitMeta.getAuthor()).isEqualTo("NessieHerself");
assertThat(commitMeta.getMessage())
.contains("with security")
.contains("no security context");
});
.extracting(
CommitMeta::getCommitter,
CommitMeta::getAuthor,
CommitMeta::getMessage,
CommitMeta::getHash)
.containsExactly(
"NessieHerself",
"NessieHerself",
format(
"Merged 2 commits from %s at %s into %s at %s",
main.getName(), main.getHash(), merge.getName(), merge.getHash()),
merged.getHash());
}

@Test
Expand Down