From 49bed912a25aa07f0280522820648522214f268e Mon Sep 17 00:00:00 2001 From: Manish Kumar <107841575+sondermanish@users.noreply.github.com> Date: Tue, 10 Dec 2024 01:38:55 +0530 Subject: [PATCH] chore: fetch remote reference in local (#38009) ## Description - Added fetch remote CGS Impl Fixes # > [!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" ### :mag: Cypress test results > [!WARNING] > Tests have not run on the HEAD d964d59e0347f1ad8592edbdf3158818c7f26a17 yet >
Mon, 09 Dec 2024 07:03:35 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit ## Release Notes - **New Features** - Introduced new methods for fetching remote changes in various Git services, enhancing synchronization capabilities. - Added new fields for internal metadata management. - Implemented user session management improvements for better handling of user-related data. - Added analytics tracking for unit execution time to improve performance insights. - **Bug Fixes** - Enhanced error handling in new fetch methods to ensure robustness. --- .../domains/ce/GitArtifactMetadataCE.java | 11 ++ .../git/central/CentralGitServiceCE.java | 8 ++ .../CentralGitServiceCECompatibleImpl.java | 3 + .../git/central/CentralGitServiceCEImpl.java | 107 ++++++++++++++++++ .../git/central/CentralGitServiceImpl.java | 3 + .../git/central/GitHandlingServiceCE.java | 2 + .../server/git/fs/GitFSServiceCEImpl.java | 27 +++++ .../server/git/utils/GitAnalyticsUtils.java | 24 ++++ 8 files changed, 185 insertions(+) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/GitArtifactMetadataCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/GitArtifactMetadataCE.java index 1c3b700c56..f5b038489c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/GitArtifactMetadataCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/domains/ce/GitArtifactMetadataCE.java @@ -2,6 +2,7 @@ package com.appsmith.server.domains.ce; import com.appsmith.external.models.AppsmithDomain; import com.appsmith.external.views.Views; +import com.appsmith.server.constants.ce.RefType; import com.appsmith.server.domains.AutoCommitConfig; import com.appsmith.server.domains.GitAuth; import com.appsmith.server.domains.GitProfile; @@ -24,6 +25,16 @@ public class GitArtifactMetadataCE implements AppsmithDomain { @JsonView(Views.Public.class) String branchName; + // TODO: make this public view and remove transient annotation once implmentation completes + @Transient + @JsonView(Views.Internal.class) + String refName; + + // TODO: make this public view and remove transient annotation once implementation completes + @Transient + @JsonView(Views.Internal.class) + RefType refType; + // Git default branch corresponding to the remote git repo to which the application is connected to @JsonView(Views.Public.class) String defaultBranchName; 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 27768442c0..576a7fb99c 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 @@ -2,6 +2,7 @@ package com.appsmith.server.git.central; import com.appsmith.git.dto.CommitDTO; import com.appsmith.server.constants.ArtifactType; +import com.appsmith.server.constants.ce.RefType; import com.appsmith.server.domains.Artifact; import com.appsmith.server.dtos.ArtifactImportDTO; import com.appsmith.server.dtos.GitConnectDTO; @@ -23,4 +24,11 @@ public interface CentralGitServiceCE { CommitDTO commitDTO, String branchedArtifactId, ArtifactType artifactType, GitType gitType); Mono detachRemote(String branchedArtifactId, ArtifactType artifactType, GitType gitType); + + Mono fetchRemoteChanges( + String referenceArtifactId, + boolean isFileLock, + ArtifactType artifactType, + GitType gitType, + RefType refType); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCECompatibleImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCECompatibleImpl.java index b996e58491..f33fa75b12 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCECompatibleImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCECompatibleImpl.java @@ -10,6 +10,7 @@ import com.appsmith.server.git.utils.GitProfileUtils; import com.appsmith.server.helpers.GitPrivateRepoHelper; import com.appsmith.server.imports.internal.ImportService; import com.appsmith.server.plugins.base.PluginService; +import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.UserDataService; import com.appsmith.server.services.WorkspaceService; import com.appsmith.server.solutions.DatasourcePermission; @@ -26,6 +27,7 @@ public class CentralGitServiceCECompatibleImpl extends CentralGitServiceCEImpl GitProfileUtils gitProfileUtils, GitAnalyticsUtils gitAnalyticsUtils, UserDataService userDataService, + SessionUserService sessionUserService, GitArtifactHelperResolver gitArtifactHelperResolver, GitHandlingServiceResolver gitHandlingServiceResolver, GitPrivateRepoHelper gitPrivateRepoHelper, @@ -41,6 +43,7 @@ public class CentralGitServiceCECompatibleImpl extends CentralGitServiceCEImpl gitProfileUtils, gitAnalyticsUtils, userDataService, + sessionUserService, gitArtifactHelperResolver, gitHandlingServiceResolver, gitPrivateRepoHelper, 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 9af8b9ce9e..64bf970ce9 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 @@ -18,6 +18,7 @@ import com.appsmith.server.domains.GitArtifactMetadata; import com.appsmith.server.domains.GitAuth; import com.appsmith.server.domains.GitProfile; import com.appsmith.server.domains.Plugin; +import com.appsmith.server.domains.User; import com.appsmith.server.domains.UserData; import com.appsmith.server.domains.Workspace; import com.appsmith.server.dtos.ArtifactExchangeJson; @@ -36,6 +37,7 @@ import com.appsmith.server.helpers.GitPrivateRepoHelper; import com.appsmith.server.imports.internal.ImportService; import com.appsmith.server.plugins.base.PluginService; import com.appsmith.server.services.GitArtifactHelper; +import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.UserDataService; import com.appsmith.server.services.WorkspaceService; import com.appsmith.server.solutions.DatasourcePermission; @@ -44,6 +46,7 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.eclipse.jgit.api.errors.InvalidRemoteException; import org.eclipse.jgit.api.errors.TransportException; +import org.eclipse.jgit.lib.BranchTrackingStatus; import org.springframework.stereotype.Service; import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; @@ -65,6 +68,7 @@ import static com.appsmith.external.git.constants.ce.GitConstantsCE.GIT_CONFIG_E import static com.appsmith.external.git.constants.ce.GitConstantsCE.GIT_PROFILE_ERROR; import static com.appsmith.external.git.constants.ce.GitSpanCE.OPS_COMMIT; import static com.appsmith.external.helpers.AppsmithBeanUtils.copyNestedNonNullProperties; +import static com.appsmith.server.constants.FieldName.BRANCH_NAME; import static com.appsmith.server.constants.FieldName.DEFAULT; import static com.appsmith.server.constants.SerialiseArtifactObjective.VERSION_CONTROL; import static java.lang.Boolean.FALSE; @@ -79,6 +83,7 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE { private final GitProfileUtils gitProfileUtils; private final GitAnalyticsUtils gitAnalyticsUtils; private final UserDataService userDataService; + private final SessionUserService sessionUserService; protected final GitArtifactHelperResolver gitArtifactHelperResolver; protected final GitHandlingServiceResolver gitHandlingServiceResolver; @@ -940,6 +945,108 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE { .isGitAuthInvalid(gitArtifactMetadata.getGitAuth()); } + public Mono fetchRemoteChanges( + Artifact baseArtifact, Artifact refArtifact, boolean isFileLock, GitType gitType, RefType refType) { + + if (refArtifact == null + || baseArtifact == null + || isBaseGitMetadataInvalid(baseArtifact.getGitArtifactMetadata(), gitType)) { + return Mono.error(new AppsmithException(AppsmithError.GIT_GENERIC_ERROR)); + } + + GitArtifactMetadata baseArtifactGitData = baseArtifact.getGitArtifactMetadata(); + GitArtifactMetadata refArtifactGitData = refArtifact.getGitArtifactMetadata(); + + String baseArtifactId = baseArtifactGitData.getDefaultArtifactId(); + + // TODO add gitType in all error messages. + if (refArtifactGitData == null || !hasText(refArtifactGitData.getRefName())) { + return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, BRANCH_NAME)); + } + + Mono currUserMono = sessionUserService.getCurrentUser().cache(); // will be used to send analytics event + Mono acquireGitLockMono = + gitRedisUtils.acquireGitLock(baseArtifactId, GitConstants.GitCommandConstants.FETCH_REMOTE, isFileLock); + + ArtifactJsonTransformationDTO jsonTransformationDTO = new ArtifactJsonTransformationDTO(); + jsonTransformationDTO.setWorkspaceId(baseArtifact.getWorkspaceId()); + jsonTransformationDTO.setBaseArtifactId(baseArtifactGitData.getDefaultArtifactId()); + jsonTransformationDTO.setRepoName(baseArtifactGitData.getRepoName()); + jsonTransformationDTO.setArtifactType(baseArtifact.getArtifactType()); + jsonTransformationDTO.setRefName(refArtifactGitData.getRefName()); + jsonTransformationDTO.setRefType(refType); + + GitHandlingService gitHandlingService = gitHandlingServiceResolver.getGitHandlingService(gitType); + + // current user mono has been zipped just to run in parallel. + Mono fetchRemoteMono = acquireGitLockMono + .then(Mono.defer(() -> + gitHandlingService.fetchRemoteChanges(jsonTransformationDTO, baseArtifactGitData.getGitAuth()))) + .flatMap(fetchedRemoteStatusString -> { + return gitRedisUtils.releaseFileLock(baseArtifactId).thenReturn(fetchedRemoteStatusString); + }) + .onErrorResume(throwable -> { + /* + in case of any error, the global exception handler will release the lock + hence we don't need to do that manually + */ + log.error( + "Error to fetch from remote for application: {}, branch: {}, git type {}", + baseArtifactId, + refArtifactGitData.getRefName(), + gitType, + throwable); + return Mono.error( + new AppsmithException(AppsmithError.GIT_ACTION_FAILED, "fetch", throwable.getMessage())); + }) + .elapsed() + .zipWith(currUserMono) + .flatMap(objects -> { + Long elapsedTime = objects.getT1().getT1(); + String fetchRemote = objects.getT1().getT2(); + User currentUser = objects.getT2(); + return gitAnalyticsUtils + .sendUnitExecutionTimeAnalyticsEvent( + AnalyticsEvents.GIT_FETCH.getEventName(), elapsedTime, currentUser, refArtifact) + .thenReturn(fetchRemote); + }) + .name(GitSpan.OPS_FETCH_REMOTE) + .tap(Micrometer.observation(observationRegistry)); + + return Mono.create(sink -> { + fetchRemoteMono.subscribe(sink::success, sink::error, null, sink.currentContext()); + }); + } + + /** + * This method is responsible to compare the current branch with the remote branch. + * Comparing means finding two numbers - how many commits ahead and behind the local branch is. + * It'll do the following things - + * 1. Checkout (if required) to the branch to make sure we are comparing the right branch + * 2. Run a git fetch command to fetch the latest changes from the remote + * + * @param refArtifactId id of the reference + * @param isFileLock whether to add file lock or not + * @param artifactType + * @return Mono of {@link BranchTrackingStatus} + */ + @Override + public Mono fetchRemoteChanges( + String refArtifactId, boolean isFileLock, ArtifactType artifactType, GitType gitType, RefType refType) { + GitArtifactHelper artifactGitHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType); + AclPermission artifactEditPermission = artifactGitHelper.getArtifactEditPermission(); + + Mono> baseAndBranchedArtifactMono = + getBaseAndBranchedArtifacts(refArtifactId, artifactType, artifactEditPermission); + + return baseAndBranchedArtifactMono.flatMap(artifactTuples -> { + Artifact baseArtifact = artifactTuples.getT1(); + Artifact refArtifact = artifactTuples.getT2(); + + return fetchRemoteChanges(baseArtifact, refArtifact, isFileLock, gitType, refType); + }); + } + /** * Returns baseArtifact and branchedArtifact * This operation is quite frequently used, hence providing the right set diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceImpl.java index 7c642d832d..0a85050f1f 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceImpl.java @@ -10,6 +10,7 @@ import com.appsmith.server.git.utils.GitProfileUtils; import com.appsmith.server.helpers.GitPrivateRepoHelper; import com.appsmith.server.imports.internal.ImportService; import com.appsmith.server.plugins.base.PluginService; +import com.appsmith.server.services.SessionUserService; import com.appsmith.server.services.UserDataService; import com.appsmith.server.services.WorkspaceService; import com.appsmith.server.solutions.DatasourcePermission; @@ -25,6 +26,7 @@ public class CentralGitServiceImpl extends CentralGitServiceCECompatibleImpl imp GitProfileUtils gitProfileUtils, GitAnalyticsUtils gitAnalyticsUtils, UserDataService userDataService, + SessionUserService sessionUserService, GitArtifactHelperResolver gitArtifactHelperResolver, GitHandlingServiceResolver gitHandlingServiceResolver, GitPrivateRepoHelper gitPrivateRepoHelper, @@ -40,6 +42,7 @@ public class CentralGitServiceImpl extends CentralGitServiceCECompatibleImpl imp gitProfileUtils, gitAnalyticsUtils, userDataService, + sessionUserService, gitArtifactHelperResolver, gitHandlingServiceResolver, gitPrivateRepoHelper, 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 3b79a0f179..5b9551ffa6 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 @@ -56,4 +56,6 @@ public interface GitHandlingServiceCE { Mono> commitArtifact( Artifact branchedArtifact, CommitDTO commitDTO, ArtifactJsonTransformationDTO jsonTransformationDTO); + + Mono fetchRemoteChanges(ArtifactJsonTransformationDTO jsonTransformationDTO, GitAuth gitAuth); } 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 a65c46733c..ffd8c26e4f 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 @@ -568,4 +568,31 @@ public class GitFSServiceCEImpl implements GitHandlingServiceCE { } return Mono.just(pushResult); } + + /** + * File system implementation of fetching remote changes. equivalent to git fetch + * @param jsonTransformationDTO : DTO to create path and other ref related details + * @param gitAuth : authentication holder + * @return : returns string for remote fetch + */ + @Override + public Mono fetchRemoteChanges(ArtifactJsonTransformationDTO jsonTransformationDTO, GitAuth gitAuth) { + + 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); + + Path repoPath = fsGitHandler.createRepoPath(repoSuffix); + Mono checkoutBranchMono = fsGitHandler.checkoutToBranch(repoSuffix, refName); + + Mono fetchRemoteMono = fsGitHandler.fetchRemote( + repoPath, gitAuth.getPublicKey(), gitAuth.getPrivateKey(), true, refName, false); + + return checkoutBranchMono.then(Mono.defer(() -> fetchRemoteMono)); + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/utils/GitAnalyticsUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/utils/GitAnalyticsUtils.java index add712ac1c..1cc0f71d61 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/utils/GitAnalyticsUtils.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/utils/GitAnalyticsUtils.java @@ -5,6 +5,7 @@ import com.appsmith.server.constants.FieldName; import com.appsmith.server.domains.ApplicationMode; import com.appsmith.server.domains.Artifact; import com.appsmith.server.domains.GitArtifactMetadata; +import com.appsmith.server.domains.User; import com.appsmith.server.helpers.GitUtils; import com.appsmith.server.services.AnalyticsService; import com.appsmith.server.services.SessionUserService; @@ -122,4 +123,27 @@ public class GitAnalyticsUtils { .sendEvent(event.getEventName(), user.getUsername(), analyticsProps) .thenReturn(artifact)); } + + public Mono sendUnitExecutionTimeAnalyticsEvent( + String flowName, Long elapsedTime, User currentUser, Artifact artifact) { + GitArtifactMetadata gitArtifactMetadata = artifact.getGitArtifactMetadata(); + + final Map data = Map.of( + FieldName.FLOW_NAME, + flowName, + FieldName.APPLICATION_ID, + gitArtifactMetadata.getDefaultArtifactId(), + "appId", + gitArtifactMetadata.getDefaultArtifactId(), + FieldName.BRANCH_NAME, + gitArtifactMetadata.getBranchName(), + "organizationId", + artifact.getWorkspaceId(), + "repoUrl", + gitArtifactMetadata.getRemoteUrl(), + "executionTime", + elapsedTime); + return analyticsService.sendEvent( + AnalyticsEvents.UNIT_EXECUTION_TIME.getEventName(), currentUser.getUsername(), data); + } }