From a64c16d98acd2ceceea284de482a2001f810401a Mon Sep 17 00:00:00 2001 From: Manish Kumar <107841575+sondermanish@users.noreply.github.com> Date: Thu, 5 Dec 2024 10:35:44 +0530 Subject: [PATCH] chore: git detach (#37934) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description > [!TIP] > _Add a TL;DR when the description is longer than 500 words or extremely technical (helps the content, marketing, and DevRel team)._ > > _Please also include relevant motivation and context. List any dependencies that are required for this change. Add links to Notion, Figma or any other documents that might be relevant to the PR._ Fixes #37439 ## Automation /ok-to-test tags="@tag.Git" ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Introduced methods for committing artifacts and managing Git operations, including `commitArtifact`, `detachRemote`, and `publishArtifactPostCommit`. - Added functionality to list branches and check repository privacy. - Enhanced error handling for Git operations, improving reliability during commits and pushes. - **Bug Fixes** - Updated parameter naming for consistency across methods, enhancing clarity. - **Documentation** - Improved JavaDoc comments for better understanding of method purposes and parameters. > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: f058fbe37f866ecee2cfb8fde30e4d4b62e2fd7f > Cypress dashboard. > Tags: `@tag.Git` > Spec: >
Wed, 04 Dec 2024 11:42:39 UTC --- .../git/central/CentralGitServiceCE.java | 2 + .../git/central/CentralGitServiceCEImpl.java | 85 +++++++++++++++++-- .../git/central/GitHandlingServiceCE.java | 3 + .../server/git/fs/GitFSServiceCEImpl.java | 29 +++++++ 4 files changed, 112 insertions(+), 7 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCE.java index 38c19abeb8..27768442c0 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCE.java @@ -21,4 +21,6 @@ public interface CentralGitServiceCE { Mono commitArtifact( CommitDTO commitDTO, String branchedArtifactId, ArtifactType artifactType, GitType gitType); + + Mono detachRemote(String branchedArtifactId, ArtifactType artifactType, GitType gitType); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java index 097cf8e56a..9af8b9ce9e 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java @@ -2,6 +2,7 @@ package com.appsmith.server.git.central; import com.appsmith.external.constants.AnalyticsEvents; import com.appsmith.external.git.constants.GitConstants; +import com.appsmith.external.git.constants.GitSpan; import com.appsmith.external.models.Datasource; import com.appsmith.external.models.DatasourceStorage; import com.appsmith.git.dto.CommitDTO; @@ -47,8 +48,10 @@ import org.springframework.stereotype.Service; import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; import reactor.core.observability.micrometer.Micrometer; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.util.function.Tuple2; +import reactor.util.function.Tuple3; import java.io.IOException; import java.time.Instant; @@ -571,7 +574,7 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE { return this.commitArtifact(commitDTO, artifact.getId(), artifactType, gitType) .onErrorResume(error -> // If the push fails remove all the cloned files from local repo - this.detachRemote(baseArtifactId, artifactType) + this.detachRemote(baseArtifactId, artifactType, gitType) .flatMap(isDeleted -> { if (error instanceof TransportException) { return gitAnalyticsUtils @@ -852,13 +855,81 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE { } /** - * TODO: implementation quite similar to the disconnectGitRepo - * @param baseArtifactId - * @param artifactType - * @return + * Method to remove all the git metadata for the artifact and connected resources. This will remove: + * - local repo + * - all the branched applications present in DB except for default application + * + * @param branchedArtifactId : id of any branched artifact for the given repo + * @param artifactType : type of artifact + * @return : the base artifact after removal of git flow. */ - protected Mono detachRemote(String baseArtifactId, ArtifactType artifactType) { - return null; + public Mono detachRemote( + String branchedArtifactId, ArtifactType artifactType, GitType gitType) { + GitHandlingService gitHandlingService = gitHandlingServiceResolver.getGitHandlingService(gitType); + GitArtifactHelper gitArtifactHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType); + AclPermission gitConnectPermission = gitArtifactHelper.getArtifactGitConnectPermission(); + + Mono> baseAndBranchedArtifactMono = + getBaseAndBranchedArtifacts(branchedArtifactId, artifactType, gitConnectPermission); + + Mono disconnectMono = baseAndBranchedArtifactMono + .flatMap(artifactTuples -> { + Artifact baseArtifact = artifactTuples.getT1(); + + if (isBaseGitMetadataInvalid(baseArtifact.getGitArtifactMetadata(), gitType)) { + return Mono.error(new AppsmithException( + AppsmithError.INVALID_GIT_CONFIGURATION, + "Please reconfigure the artifact to connect to git repo")); + } + + GitArtifactMetadata gitArtifactMetadata = baseArtifact.getGitArtifactMetadata(); + ArtifactJsonTransformationDTO jsonTransformationDTO = new ArtifactJsonTransformationDTO(); + jsonTransformationDTO.setRefType(RefType.BRANCH); + jsonTransformationDTO.setWorkspaceId(baseArtifact.getWorkspaceId()); + jsonTransformationDTO.setBaseArtifactId(gitArtifactMetadata.getDefaultArtifactId()); + jsonTransformationDTO.setRepoName(gitArtifactMetadata.getRepoName()); + jsonTransformationDTO.setArtifactType(baseArtifact.getArtifactType()); + jsonTransformationDTO.setRefName(gitArtifactMetadata.getBranchName()); + + // Remove the git contents from file system + return Mono.zip(gitHandlingService.listBranches(jsonTransformationDTO), Mono.just(baseArtifact)); + }) + .flatMap(tuple -> { + List localBranches = tuple.getT1(); + Artifact baseArtifact = tuple.getT2(); + + baseArtifact.setGitArtifactMetadata(null); + gitArtifactHelper.resetAttributeInBaseArtifact(baseArtifact); + + GitArtifactMetadata gitArtifactMetadata = baseArtifact.getGitArtifactMetadata(); + ArtifactJsonTransformationDTO jsonTransformationDTO = new ArtifactJsonTransformationDTO(); + jsonTransformationDTO.setRefType(RefType.BRANCH); + jsonTransformationDTO.setWorkspaceId(baseArtifact.getWorkspaceId()); + jsonTransformationDTO.setBaseArtifactId(gitArtifactMetadata.getDefaultArtifactId()); + jsonTransformationDTO.setRepoName(gitArtifactMetadata.getRepoName()); + jsonTransformationDTO.setArtifactType(baseArtifact.getArtifactType()); + jsonTransformationDTO.setRefName(gitArtifactMetadata.getBranchName()); + + // Remove the parent application branch name from the list + Mono removeRepoMono = gitHandlingService.removeRepository(jsonTransformationDTO); + Mono updatedArtifactMono = gitArtifactHelper.saveArtifact(baseArtifact); + + Flux deleteAllBranchesFlux = + gitArtifactHelper.deleteAllBranches(branchedArtifactId, localBranches); + + return Mono.zip(updatedArtifactMono, removeRepoMono, deleteAllBranchesFlux.collectList()) + .map(Tuple3::getT1); + }) + .flatMap(updatedBaseArtifact -> { + return gitArtifactHelper + .disconnectEntitiesOfBaseArtifact(updatedBaseArtifact) + .then(gitAnalyticsUtils.addAnalyticsForGitOperation( + AnalyticsEvents.GIT_DISCONNECT, updatedBaseArtifact, false)); + }) + .name(GitSpan.OPS_DETACH_REMOTE) + .tap(Micrometer.observation(observationRegistry)); + + return Mono.create(sink -> disconnectMono.subscribe(sink::success, sink::error, null, sink.currentContext())); } private boolean isBaseGitMetadataInvalid(GitArtifactMetadata gitArtifactMetadata, GitType gitType) { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/GitHandlingServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/GitHandlingServiceCE.java index 747bddbd34..3b79a0f179 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/GitHandlingServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/GitHandlingServiceCE.java @@ -10,6 +10,7 @@ import com.appsmith.server.git.dtos.ArtifactJsonTransformationDTO; import reactor.core.publisher.Mono; import reactor.util.function.Tuple2; +import java.util.List; import java.util.Set; public interface GitHandlingServiceCE { @@ -38,6 +39,8 @@ public interface GitHandlingServiceCE { Mono removeRepository(ArtifactJsonTransformationDTO artifactJsonTransformationDTO); + Mono> listBranches(ArtifactJsonTransformationDTO artifactJsonTransformationDTO); + Mono validateEmptyRepository(ArtifactJsonTransformationDTO artifactJsonTransformationDTO); Mono initialiseReadMe( diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java index 5199170230..a65c46733c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java @@ -1,6 +1,7 @@ package com.appsmith.server.git.fs; import com.appsmith.external.constants.AnalyticsEvents; +import com.appsmith.external.dtos.GitBranchDTO; import com.appsmith.external.git.constants.GitConstants; import com.appsmith.external.git.constants.GitSpan; import com.appsmith.external.git.handler.FSGitHandler; @@ -49,12 +50,14 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.reactive.TransactionalOperator; import org.springframework.util.StringUtils; import reactor.core.observability.micrometer.Micrometer; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import reactor.util.function.Tuple2; import java.io.IOException; import java.nio.file.Path; import java.util.HashSet; +import java.util.List; import java.util.Set; import java.util.concurrent.TimeoutException; @@ -248,6 +251,32 @@ public class GitFSServiceCEImpl implements GitHandlingServiceCE { return commonGitFileUtils.deleteLocalRepo(repoSuffix); } + /** + * List all the local branches present in the file system + * @param artifactJsonTransformationDTO + * @return + */ + @Override + public Mono> listBranches(ArtifactJsonTransformationDTO artifactJsonTransformationDTO) { + GitArtifactHelper gitArtifactHelper = + gitArtifactHelperResolver.getArtifactHelper(artifactJsonTransformationDTO.getArtifactType()); + + Path repoSuffix = gitArtifactHelper.getRepoSuffixPath( + artifactJsonTransformationDTO.getWorkspaceId(), + artifactJsonTransformationDTO.getBaseArtifactId(), + artifactJsonTransformationDTO.getRepoName()); + + return fsGitHandler + .listBranches(repoSuffix) + .flatMapMany(Flux::fromIterable) + .filter(gitBranchDTO -> { + return StringUtils.hasText(gitBranchDTO.getBranchName()) + && !gitBranchDTO.getBranchName().startsWith("origin"); + }) + .map(GitBranchDTO::getBranchName) + .collectList(); + } + @Override public Mono validateEmptyRepository(ArtifactJsonTransformationDTO artifactJsonTransformationDTO) { GitArtifactHelper gitArtifactHelper =