chore: Modifying git fetch remote method signature (#40896)
## Description - Modified the git fetch remote method signature. Changed it from String to BranchTrackingStatus, which tells about the numbers of commit local is ahead and behind of remote - This will have an EE counterpart as well Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.Git" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/15539521248> > Commit: 0793974c3dc6daf18c1496a1c026be546454aa2c > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=15539521248&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Git` > Spec: > <hr>Mon, 09 Jun 2025 19:23:06 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Remote fetch operations now provide detailed branch tracking status, showing how many commits your local branch is ahead or behind the remote branch. - **API Changes** - Endpoints and methods that previously returned a simple fetch result now return comprehensive branch tracking information. - Method signatures have been updated to reflect these changes for improved clarity and consistency. - **Bug Fixes** - Improved error handling to ensure file locks are properly released during remote fetch failures, preventing potential deadlocks. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
f4e0551c4a
commit
b6fc1de100
|
|
@ -1563,9 +1563,9 @@ public class FSGitHandlerCEImpl implements FSGitHandler {
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Mono<BranchTrackingStatus> getBranchTrackingStatus(Path repoPath, String branchName) {
|
public Mono<BranchTrackingStatus> getBranchTrackingStatus(Path repoSuffix, String branchName) {
|
||||||
return Mono.using(
|
return Mono.using(
|
||||||
() -> Git.open(repoPath.toFile()),
|
() -> Git.open(createRepoPath(repoSuffix).toFile()),
|
||||||
git -> Mono.fromCallable(() -> {
|
git -> Mono.fromCallable(() -> {
|
||||||
Span jgitBranchTrackingSpan =
|
Span jgitBranchTrackingSpan =
|
||||||
observationHelper.createSpan(GitSpan.JGIT_BRANCH_TRACK);
|
observationHelper.createSpan(GitSpan.JGIT_BRANCH_TRACK);
|
||||||
|
|
|
||||||
|
|
@ -249,5 +249,5 @@ public interface FSGitHandler {
|
||||||
|
|
||||||
Path createRepoPath(Path suffix);
|
Path createRepoPath(Path suffix);
|
||||||
|
|
||||||
Mono<BranchTrackingStatus> getBranchTrackingStatus(Path repoPath, String branchName);
|
Mono<BranchTrackingStatus> getBranchTrackingStatus(Path repoSuffix, String branchName);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -15,6 +15,7 @@ import com.appsmith.server.dtos.GitConnectDTO;
|
||||||
import com.appsmith.server.dtos.GitDocsDTO;
|
import com.appsmith.server.dtos.GitDocsDTO;
|
||||||
import com.appsmith.server.dtos.GitMergeDTO;
|
import com.appsmith.server.dtos.GitMergeDTO;
|
||||||
import com.appsmith.server.dtos.GitPullDTO;
|
import com.appsmith.server.dtos.GitPullDTO;
|
||||||
|
import org.eclipse.jgit.lib.BranchTrackingStatus;
|
||||||
import reactor.core.publisher.Mono;
|
import reactor.core.publisher.Mono;
|
||||||
|
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
|
@ -39,13 +40,26 @@ public interface CentralGitServiceCE {
|
||||||
Mono<List<GitRefDTO>> listBranchForArtifact(
|
Mono<List<GitRefDTO>> listBranchForArtifact(
|
||||||
String branchedArtifactId, ArtifactType artifactType, Boolean pruneBranches, GitType gitType);
|
String branchedArtifactId, ArtifactType artifactType, Boolean pruneBranches, GitType gitType);
|
||||||
|
|
||||||
Mono<String> fetchRemoteChanges(
|
Mono<BranchTrackingStatus> fetchRemoteChanges(
|
||||||
String referenceArtifactId,
|
String referenceArtifactId,
|
||||||
ArtifactType artifactType,
|
ArtifactType artifactType,
|
||||||
boolean isFileLock,
|
boolean isFileLock,
|
||||||
GitType gitType,
|
GitType gitType,
|
||||||
RefType refType);
|
RefType refType);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Fetches remote changes from remote git repository.
|
||||||
|
* This overloaded method is directly used for autocommit purpose
|
||||||
|
* @param baseArtifact : base artifact on which the repository was connected
|
||||||
|
* @param refArtifact : the reference/branch artifact for which remote changes are to be fetched
|
||||||
|
* @param isFileLock : would this require a redis file lock
|
||||||
|
* @param gitType : GitType of this operation
|
||||||
|
* @param refType : RefType for this operation
|
||||||
|
* @return : branchTrackingStatus, i.e., How many commits is local ahead and behind of remote
|
||||||
|
*/
|
||||||
|
Mono<BranchTrackingStatus> fetchRemoteChanges(
|
||||||
|
Artifact baseArtifact, Artifact refArtifact, boolean isFileLock, GitType gitType, RefType refType);
|
||||||
|
|
||||||
Mono<MergeStatusDTO> mergeBranch(
|
Mono<MergeStatusDTO> mergeBranch(
|
||||||
String branchedArtifactId, ArtifactType artifactType, GitMergeDTO gitMergeDTO, GitType gitType);
|
String branchedArtifactId, ArtifactType artifactType, GitMergeDTO gitMergeDTO, GitType gitType);
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1906,7 +1906,8 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
public Mono<String> fetchRemoteChanges(
|
@Override
|
||||||
|
public Mono<BranchTrackingStatus> fetchRemoteChanges(
|
||||||
Artifact baseArtifact, Artifact refArtifact, boolean isFileLock, GitType gitType, RefType refType) {
|
Artifact baseArtifact, Artifact refArtifact, boolean isFileLock, GitType gitType, RefType refType) {
|
||||||
|
|
||||||
if (refArtifact == null
|
if (refArtifact == null
|
||||||
|
|
@ -1941,9 +1942,12 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE {
|
||||||
GitHandlingService gitHandlingService = gitHandlingServiceResolver.getGitHandlingService(gitType);
|
GitHandlingService gitHandlingService = gitHandlingServiceResolver.getGitHandlingService(gitType);
|
||||||
|
|
||||||
// current user mono has been zipped just to run in parallel.
|
// current user mono has been zipped just to run in parallel.
|
||||||
Mono<String> fetchRemoteMono = acquireGitLockMono
|
Mono<BranchTrackingStatus> fetchRemoteMono = acquireGitLockMono
|
||||||
.then(Mono.defer(() -> gitHandlingService.fetchRemoteReferences(
|
.then(Mono.defer(() -> gitHandlingService.fetchRemoteReferences(
|
||||||
jsonTransformationDTO, baseArtifactGitData.getGitAuth(), FALSE)))
|
jsonTransformationDTO, baseArtifactGitData.getGitAuth(), FALSE)))
|
||||||
|
.flatMap(fetchedRemoteString -> {
|
||||||
|
return gitHandlingService.getBranchTrackingStatus(jsonTransformationDTO);
|
||||||
|
})
|
||||||
.flatMap(fetchedRemoteStatusString -> {
|
.flatMap(fetchedRemoteStatusString -> {
|
||||||
return gitRedisUtils
|
return gitRedisUtils
|
||||||
.releaseFileLock(artifactType, baseArtifactId, isFileLock)
|
.releaseFileLock(artifactType, baseArtifactId, isFileLock)
|
||||||
|
|
@ -1960,14 +1964,17 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE {
|
||||||
refArtifactGitData.getRefName(),
|
refArtifactGitData.getRefName(),
|
||||||
gitType,
|
gitType,
|
||||||
throwable);
|
throwable);
|
||||||
return Mono.error(
|
|
||||||
new AppsmithException(AppsmithError.GIT_ACTION_FAILED, "fetch", throwable.getMessage()));
|
return gitRedisUtils
|
||||||
|
.releaseFileLock(artifactType, baseArtifactId, isFileLock)
|
||||||
|
.then(Mono.error(new AppsmithException(
|
||||||
|
AppsmithError.GIT_ACTION_FAILED, "fetch", throwable.getMessage())));
|
||||||
})
|
})
|
||||||
.elapsed()
|
.elapsed()
|
||||||
.zipWith(currUserMono)
|
.zipWith(currUserMono)
|
||||||
.flatMap(objects -> {
|
.flatMap(objects -> {
|
||||||
Long elapsedTime = objects.getT1().getT1();
|
Long elapsedTime = objects.getT1().getT1();
|
||||||
String fetchRemote = objects.getT1().getT2();
|
BranchTrackingStatus fetchRemote = objects.getT1().getT2();
|
||||||
User currentUser = objects.getT2();
|
User currentUser = objects.getT2();
|
||||||
return gitAnalyticsUtils
|
return gitAnalyticsUtils
|
||||||
.sendUnitExecutionTimeAnalyticsEvent(
|
.sendUnitExecutionTimeAnalyticsEvent(
|
||||||
|
|
@ -1995,7 +2002,7 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE {
|
||||||
* @return Mono of {@link BranchTrackingStatus}
|
* @return Mono of {@link BranchTrackingStatus}
|
||||||
*/
|
*/
|
||||||
@Override
|
@Override
|
||||||
public Mono<String> fetchRemoteChanges(
|
public Mono<BranchTrackingStatus> fetchRemoteChanges(
|
||||||
String refArtifactId, ArtifactType artifactType, boolean isFileLock, GitType gitType, RefType refType) {
|
String refArtifactId, ArtifactType artifactType, boolean isFileLock, GitType gitType, RefType refType) {
|
||||||
GitArtifactHelper<?> artifactGitHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType);
|
GitArtifactHelper<?> artifactGitHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType);
|
||||||
AclPermission artifactEditPermission = artifactGitHelper.getArtifactEditPermission();
|
AclPermission artifactEditPermission = artifactGitHelper.getArtifactEditPermission();
|
||||||
|
|
|
||||||
|
|
@ -13,6 +13,7 @@ import com.appsmith.server.dtos.ArtifactExchangeJson;
|
||||||
import com.appsmith.server.dtos.GitConnectDTO;
|
import com.appsmith.server.dtos.GitConnectDTO;
|
||||||
import com.appsmith.server.dtos.GitMergeDTO;
|
import com.appsmith.server.dtos.GitMergeDTO;
|
||||||
import com.appsmith.server.git.dtos.ArtifactJsonTransformationDTO;
|
import com.appsmith.server.git.dtos.ArtifactJsonTransformationDTO;
|
||||||
|
import org.eclipse.jgit.lib.BranchTrackingStatus;
|
||||||
import reactor.core.publisher.Mono;
|
import reactor.core.publisher.Mono;
|
||||||
import reactor.util.function.Tuple2;
|
import reactor.util.function.Tuple2;
|
||||||
|
|
||||||
|
|
@ -94,6 +95,8 @@ public interface GitHandlingServiceCE {
|
||||||
Mono<String> fetchRemoteReferences(
|
Mono<String> fetchRemoteReferences(
|
||||||
ArtifactJsonTransformationDTO jsonTransformationDTO, FetchRemoteDTO fetchRemoteDTO, GitAuth gitAuth);
|
ArtifactJsonTransformationDTO jsonTransformationDTO, FetchRemoteDTO fetchRemoteDTO, GitAuth gitAuth);
|
||||||
|
|
||||||
|
Mono<BranchTrackingStatus> getBranchTrackingStatus(ArtifactJsonTransformationDTO artifactJsonTransformationDTO);
|
||||||
|
|
||||||
Mono<String> mergeBranches(ArtifactJsonTransformationDTO jsonTransformationDTO, GitMergeDTO gitMergeDTO);
|
Mono<String> mergeBranches(ArtifactJsonTransformationDTO jsonTransformationDTO, GitMergeDTO gitMergeDTO);
|
||||||
|
|
||||||
Mono<MergeStatusDTO> isBranchMergable(ArtifactJsonTransformationDTO JsonTransformationDTO, GitMergeDTO gitMergeDTO);
|
Mono<MergeStatusDTO> isBranchMergable(ArtifactJsonTransformationDTO JsonTransformationDTO, GitMergeDTO gitMergeDTO);
|
||||||
|
|
|
||||||
|
|
@ -450,8 +450,9 @@ public class CommonGitServiceCEImpl implements CommonGitServiceCE {
|
||||||
baseArtifactId,
|
baseArtifactId,
|
||||||
finalBranchName,
|
finalBranchName,
|
||||||
throwable);
|
throwable);
|
||||||
return Mono.error(new AppsmithException(
|
return releaseFileLock(baseArtifactId, isFileLock)
|
||||||
AppsmithError.GIT_ACTION_FAILED, "fetch", throwable.getMessage()));
|
.then(Mono.error(new AppsmithException(
|
||||||
|
AppsmithError.GIT_ACTION_FAILED, "fetch", throwable.getMessage())));
|
||||||
});
|
});
|
||||||
})
|
})
|
||||||
.elapsed()
|
.elapsed()
|
||||||
|
|
|
||||||
|
|
@ -28,6 +28,7 @@ import jakarta.validation.Valid;
|
||||||
import lombok.RequiredArgsConstructor;
|
import lombok.RequiredArgsConstructor;
|
||||||
import lombok.extern.slf4j.Slf4j;
|
import lombok.extern.slf4j.Slf4j;
|
||||||
import org.apache.commons.lang3.BooleanUtils;
|
import org.apache.commons.lang3.BooleanUtils;
|
||||||
|
import org.eclipse.jgit.lib.BranchTrackingStatus;
|
||||||
import org.springframework.http.HttpStatus;
|
import org.springframework.http.HttpStatus;
|
||||||
import org.springframework.web.bind.annotation.DeleteMapping;
|
import org.springframework.web.bind.annotation.DeleteMapping;
|
||||||
import org.springframework.web.bind.annotation.GetMapping;
|
import org.springframework.web.bind.annotation.GetMapping;
|
||||||
|
|
@ -142,7 +143,7 @@ public class GitApplicationControllerCE {
|
||||||
|
|
||||||
@JsonView(Views.Public.class)
|
@JsonView(Views.Public.class)
|
||||||
@GetMapping("/{referencedApplicationId}/fetch/remote")
|
@GetMapping("/{referencedApplicationId}/fetch/remote")
|
||||||
public Mono<ResponseDTO<String>> fetchRemoteChanges(
|
public Mono<ResponseDTO<BranchTrackingStatus>> fetchRemoteChanges(
|
||||||
@PathVariable String referencedApplicationId,
|
@PathVariable String referencedApplicationId,
|
||||||
@RequestHeader(required = false, defaultValue = "branch") RefType refType) {
|
@RequestHeader(required = false, defaultValue = "branch") RefType refType) {
|
||||||
log.info("Going to compare with remote for default referencedApplicationId {}", referencedApplicationId);
|
log.info("Going to compare with remote for default referencedApplicationId {}", referencedApplicationId);
|
||||||
|
|
|
||||||
|
|
@ -42,6 +42,7 @@ import org.eclipse.jgit.api.errors.EmptyCommitException;
|
||||||
import org.eclipse.jgit.api.errors.InvalidRemoteException;
|
import org.eclipse.jgit.api.errors.InvalidRemoteException;
|
||||||
import org.eclipse.jgit.api.errors.TransportException;
|
import org.eclipse.jgit.api.errors.TransportException;
|
||||||
import org.eclipse.jgit.errors.RepositoryNotFoundException;
|
import org.eclipse.jgit.errors.RepositoryNotFoundException;
|
||||||
|
import org.eclipse.jgit.lib.BranchTrackingStatus;
|
||||||
import org.springframework.stereotype.Service;
|
import org.springframework.stereotype.Service;
|
||||||
import org.springframework.util.StringUtils;
|
import org.springframework.util.StringUtils;
|
||||||
import reactor.core.observability.micrometer.Micrometer;
|
import reactor.core.observability.micrometer.Micrometer;
|
||||||
|
|
@ -674,6 +675,20 @@ public class GitFSServiceCEImpl implements GitHandlingServiceCE {
|
||||||
return fsGitHandler.fetchRemote(repoSuffix, false, fetchRemoteDTO, publicKey, privateKey);
|
return fsGitHandler.fetchRemote(repoSuffix, false, fetchRemoteDTO, publicKey, privateKey);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Override
|
||||||
|
public Mono<BranchTrackingStatus> getBranchTrackingStatus(ArtifactJsonTransformationDTO jsonTransformationDTO) {
|
||||||
|
String workspaceId = jsonTransformationDTO.getWorkspaceId();
|
||||||
|
String baseArtifactId = jsonTransformationDTO.getBaseArtifactId();
|
||||||
|
String repoName = jsonTransformationDTO.getRepoName();
|
||||||
|
String refName = jsonTransformationDTO.getRefName();
|
||||||
|
|
||||||
|
ArtifactType artifactType = jsonTransformationDTO.getArtifactType();
|
||||||
|
GitArtifactHelper<?> gitArtifactHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType);
|
||||||
|
Path repoSuffix = gitArtifactHelper.getRepoSuffixPath(workspaceId, baseArtifactId, repoName);
|
||||||
|
|
||||||
|
return fsGitHandler.getBranchTrackingStatus(repoSuffix, refName);
|
||||||
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
public Mono<String> mergeBranches(ArtifactJsonTransformationDTO jsonTransformationDTO, GitMergeDTO gitMergeDTO) {
|
public Mono<String> mergeBranches(ArtifactJsonTransformationDTO jsonTransformationDTO, GitMergeDTO gitMergeDTO) {
|
||||||
String workspaceId = jsonTransformationDTO.getWorkspaceId();
|
String workspaceId = jsonTransformationDTO.getWorkspaceId();
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user