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

chore: merge and merge status #38495

Merged
merged 11 commits into from
Jan 8, 2025
Merged

chore: merge and merge status #38495

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
927fc5d
added git resource map
sondermanish Jan 3, 2025
41f5faa
added reviewed comments
sondermanish Jan 3, 2025
f464a95
added changes
sondermanish Jan 3, 2025
7a3406e
review comments
sondermanish Jan 3, 2025
70ad2a1
modified mocking for test failures
sondermanish Jan 3, 2025
8877a5b
more mocking
sondermanish Jan 3, 2025
1283574
added merge
sondermanish Jan 4, 2025
2976a4c
merge with origin/release
sondermanish Jan 6, 2025
3a72c16
Merge remote-tracking branch 'origin/release' into chore/status
sondermanish Jan 7, 2025
9500e83
review comments
sondermanish Jan 8, 2025
ab5e336
further modification
sondermanish Jan 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 50 additions & 37 deletions app/server/appsmith-git/src/main/java/com/appsmith/git/handler/ce/FSGitHandlerCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.appsmith.external.dtos.MergeStatusDTO;
import com.appsmith.external.git.constants.GitSpan;
import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.external.git.dtos.FetchRemoteDTO;
import com.appsmith.external.git.handler.FSGitHandler;
import com.appsmith.external.helpers.Stopwatch;
import com.appsmith.git.configurations.GitServiceConfig;
Expand All @@ -33,7 +34,6 @@
import org.eclipse.jgit.api.ResetCommand;
import org.eclipse.jgit.api.Status;
import org.eclipse.jgit.api.TransportConfigCallback;
import org.eclipse.jgit.api.errors.CheckoutConflictException;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.BranchTrackingStatus;
import org.eclipse.jgit.lib.PersonIdent;
Expand All @@ -42,6 +42,7 @@
import org.eclipse.jgit.merge.MergeStrategy;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.TagOpt;
import org.eclipse.jgit.util.StringUtils;
import org.springframework.stereotype.Component;
import org.springframework.util.FileSystemUtils;
Expand Down Expand Up @@ -92,6 +93,11 @@ public class FSGitHandlerCEImpl implements FSGitHandler {

private final Scheduler scheduler = Schedulers.boundedElastic();

private static final String BRANCH_REF_REMOTE_SRC = "refs/heads/";
private static final String BRANCH_REF_LOCAL_DST = "refs/remotes/origin/";
private static final String SRC_DST_DELIMITER = ":";
private static final String TAG_REF = "refs/tags/";

private static final String SUCCESS_MERGE_STATUS = "This branch has no conflicts with the base branch.";

/**
Expand Down Expand Up @@ -846,15 +852,14 @@ public Mono<String> mergeBranch(Path repoSuffix, String sourceBranch, String des
git -> Mono.fromCallable(() -> {
Stopwatch processStopwatch = StopwatchHelpers.startStopwatch(
repoSuffix, AnalyticsEvents.GIT_MERGE.getEventName());
log.debug(Thread.currentThread().getName() + ": Merge branch " + sourceBranch
+ " on " + destinationBranch);
try {
// checkout the branch on which the merge command is run
git.checkout()
.setName(destinationBranch)
.setCreateBranch(false)
.call();

log.info(
"{}: Merge branch {} on {}",
Thread.currentThread().getName(),
sourceBranch,
destinationBranch);

try {
MergeResult mergeResult = git.merge()
.include(git.getRepository().findRef(sourceBranch))
.setStrategy(MergeStrategy.RECURSIVE)
Expand Down Expand Up @@ -934,9 +939,7 @@ public Mono<String> fetchRemote(

@Override
public Mono<String> fetchRemote(
Path repoSuffix, String publicKey, String privateKey, boolean isRepoPath, String... branchNames) {
Stopwatch processStopwatch =
StopwatchHelpers.startStopwatch(repoSuffix, AnalyticsEvents.GIT_FETCH.getEventName());
Path repoSuffix, boolean isRepoPath, FetchRemoteDTO fetchRemoteDTO, String publicKey, String privateKey) {
Path repoPath = TRUE.equals(isRepoPath) ? repoSuffix : createRepoPath(repoSuffix);
return Mono.using(
() -> Git.open(repoPath.toFile()),
Expand All @@ -945,21 +948,35 @@ public Mono<String> fetchRemote(
new SshTransportConfigCallback(privateKey, publicKey);
String fetchMessages;

List<String> refNames = fetchRemoteDTO.getRefNames();
RefType refType = fetchRemoteDTO.getRefType();

List<RefSpec> refSpecs = new ArrayList<>();
for (String branchName : branchNames) {
RefSpec ref = new RefSpec(
"refs/heads/" + branchName + ":refs/remotes/origin/" + branchName);
refSpecs.add(ref);
if (RefType.tag.equals(refType)) {
for (String tagName : refNames) {
RefSpec refSpec = new RefSpec(TAG_REF + tagName + ":" + TAG_REF + tagName);
refSpecs.add(refSpec);
}
} else {
for (String refName : refNames) {
RefSpec ref = new RefSpec(BRANCH_REF_REMOTE_SRC
+ refName
+ SRC_DST_DELIMITER
+ BRANCH_REF_LOCAL_DST
+ refName);
refSpecs.add(ref);
}
}

fetchMessages = git.fetch()
.setRefSpecs(refSpecs.toArray(new RefSpec[0]))
.setRemoveDeletedRefs(true)
.setTagOpt(TagOpt.NO_TAGS) // no tags would mean that tags are fetched
// explicitly
.setTransportConfigCallback(config)
.call()
.getMessages();

processStopwatch.stopAndLogTimeInMillis();
return fetchMessages;
})
.onErrorResume(error -> {
Expand All @@ -980,30 +997,13 @@ public Mono<MergeStatusDTO> isMergeBranch(Path repoSuffix, String sourceBranch,
return Mono.using(
() -> Git.open(createRepoPath(repoSuffix).toFile()),
git -> Mono.fromCallable(() -> {
log.debug(
Thread.currentThread().getName()
+ ": Check mergeability for repo {} with src: {}, dest: {}",
log.info(
"{}: Check merge-ability for repo {} with source: {}, destination: {}",
Thread.currentThread().getName(),
repoSuffix,
sourceBranch,
destinationBranch);

// checkout the branch on which the merge command is run
try {
git.checkout()
.setName(destinationBranch)
.setCreateBranch(false)
.call();
} catch (GitAPIException e) {
if (e instanceof CheckoutConflictException) {
MergeStatusDTO mergeStatus = new MergeStatusDTO();
mergeStatus.setMergeAble(false);
mergeStatus.setConflictingFiles(
((CheckoutConflictException) e).getConflictingPaths());
processStopwatch.stopAndLogTimeInMillis();
return mergeStatus;
}
}

MergeResult mergeResult = git.merge()
.include(git.getRepository().findRef(sourceBranch))
.setFastForward(MergeCommand.FastForwardMode.NO_FF)
Expand Down Expand Up @@ -1054,6 +1054,19 @@ public Mono<MergeStatusDTO> isMergeBranch(Path repoSuffix, String sourceBranch,
return Mono.error(e);
}
})
.onErrorResume(error -> {
MergeStatusDTO mergeStatusDTO = new MergeStatusDTO();
mergeStatusDTO.setMergeAble(false);
mergeStatusDTO.setMessage(error.getMessage());
mergeStatusDTO.setReferenceDoc(ErrorReferenceDocUrl.GIT_MERGE_CONFLICT.getDocUrl());
try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a try catch here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case of errors we would need to reset it to last commit, and the method reset to last commit is throwing checked exceptions which needs to be handled.

return resetToLastCommit(repoSuffix, destinationBranch)
.thenReturn(mergeStatusDTO);
} catch (GitAPIException | IOException e) {
log.error("Error while hard resetting to latest commit {0}", e);
return Mono.error(e);
}
})
.timeout(Duration.ofMillis(Constraint.TIMEOUT_MILLIS)),
Git::close)
.subscribeOn(scheduler);
Expand Down
29 changes: 29 additions & 0 deletions ...rver/appsmith-interfaces/src/main/java/com/appsmith/external/git/dtos/FetchRemoteDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package com.appsmith.external.git.dtos;

import com.appsmith.external.git.constants.ce.RefType;
import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.NoArgsConstructor;

import java.util.List;

@Data
@NoArgsConstructor
@AllArgsConstructor
public class FetchRemoteDTO {

/**
* List of references which is to be fetched from remote.
*/
List<String> refNames;

/**
* Assumption is that we fetch only one type of refs at once.
*/
RefType refType;

/**
* fetch all the remotes
*/
Boolean isFetchAll;
}
5 changes: 4 additions & 1 deletion ...ver/appsmith-interfaces/src/main/java/com/appsmith/external/git/handler/FSGitHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.appsmith.external.dtos.GitRefDTO;
import com.appsmith.external.dtos.GitStatusDTO;
import com.appsmith.external.dtos.MergeStatusDTO;
import com.appsmith.external.git.dtos.FetchRemoteDTO;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.BranchTrackingStatus;
import reactor.core.publisher.Mono;
Expand Down Expand Up @@ -135,6 +136,8 @@ Mono<MergeStatusDTO> pullApplication(
Mono<GitStatusDTO> getStatus(Path repoPath, String branchName);

/**
* This method merges source branch into destination branch for a git repository which is present on the partial
* path provided. <B> This assumes that the branch on which the merge will happen is already checked out </B>
* @param repoSuffix suffixedPath used to generate the base repo path this includes orgId, defaultAppId, repoName
* @param sourceBranch name of the branch whose commits will be referred amd merged to destinationBranch
* @param destinationBranch Merge operation is performed on this branch
Expand All @@ -158,7 +161,7 @@ Mono<String> fetchRemote(
boolean isFetchAll);

Mono<String> fetchRemote(
Path repoSuffix, String publicKey, String privateKey, boolean isRepoPath, String... branchNames);
Path repoSuffix, boolean isRepoPath, FetchRemoteDTO fetchRemoteDTO, String publicKey, String privateKey);

/**
*
Expand Down
8 changes: 8 additions & 0 deletions ...er/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.appsmith.external.dtos.GitBranchDTO;
import com.appsmith.external.dtos.GitRefDTO;
import com.appsmith.external.dtos.GitStatusDTO;
import com.appsmith.external.dtos.MergeStatusDTO;
import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.git.dto.CommitDTO;
import com.appsmith.server.constants.ArtifactType;
Expand All @@ -13,6 +14,7 @@
import com.appsmith.server.dtos.AutoCommitResponseDTO;
import com.appsmith.server.dtos.GitConnectDTO;
import com.appsmith.server.dtos.GitDocsDTO;
import com.appsmith.server.dtos.GitMergeDTO;
import com.appsmith.server.dtos.GitPullDTO;
import reactor.core.publisher.Mono;

Expand Down Expand Up @@ -45,6 +47,12 @@ Mono<String> fetchRemoteChanges(
GitType gitType,
RefType refType);

Mono<MergeStatusDTO> mergeBranch(
String branchedArtifactId, ArtifactType artifactType, GitMergeDTO gitMergeDTO, GitType gitType);

Mono<MergeStatusDTO> isBranchMergable(
String branchedArtifactId, ArtifactType artifactType, GitMergeDTO gitMergeDTO, GitType gitType);

Mono<? extends Artifact> discardChanges(String branchedArtifactId, ArtifactType artifactType, GitType gitType);

Mono<GitStatusDTO> getStatus(
Expand Down
Loading
Loading