added comments

Zeral-Zhang · Dec 4, 2024 · 6dc1f5a · 6dc1f5a
1 parent f0c7fd1
commit 6dc1f5a
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 19 deletions.
diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/GitRedisUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/GitRedisUtils.java
@@ -23,15 +23,20 @@ public class GitRedisUtils {
     private final RedisUtils redisUtils;
     private final ObservationRegistry observationRegistry;
 
-    public Mono<Boolean> addFileLock(String defaultApplicationId, String commandName, Boolean isRetryAllowed) {
+    /**
+     * Adds a baseArtifact id as a key in redis, the presence of this key represents a symbolic lock, essentially meaning that no new operations
+     * should be performed till this key remains present.
+     * @param baseArtifactId : base id of the artifact for which the key is getting added.
+     * @param commandName : Name of the operation which is trying to acquire the lock, this value will be added against the key
+     * @param isRetryAllowed : Boolean for whether retries for adding the value is allowed
+     * @return a boolean publisher for the added file locks
+     */
+    public Mono<Boolean> addFileLock(String baseArtifactId, String commandName, Boolean isRetryAllowed) {
         long numberOfRetries = Boolean.TRUE.equals(isRetryAllowed) ? MAX_RETRIES : 0L;
 
-        log.info(
-                "Git command {} is trying to acquire the lock for application id {}",
-                commandName,
-                defaultApplicationId);
+        log.info("Git command {} is trying to acquire the lock for application id {}", commandName, baseArtifactId);
         return redisUtils
-                .addFileLock(defaultApplicationId, commandName)
+                .addFileLock(baseArtifactId, commandName)
                 .retryWhen(Retry.fixedDelay(numberOfRetries, RETRY_DELAY)
                         .onRetryExhaustedThrow((retryBackoffSpec, retrySignal) -> {
                             if (retrySignal.failure() instanceof AppsmithException) {
@@ -55,6 +60,15 @@ public Mono<Boolean> releaseFileLock(String defaultApplicationId) {
                 .tap(Micrometer.observation(observationRegistry));
     }
 
+    /**
+     * This is a wrapper method for acquiring git lock, since multiple ops are used in sequence
+     * for a complete composite operation not all ops require to acquire the lock hence a dummy flag is sent back for
+     * operations in that is getting executed in between
+     * @param baseArtifactId : id of the base artifact for which ops would be locked
+     * @param isLockRequired : is lock really required or is it a proxy function
+     * @return : Boolean for whether the lock is acquired
+     */
+    // TODO @Manish add artifactType reference in incoming prs.
     public Mono<Boolean> acquireGitLock(String baseArtifactId, String commandName, boolean isLockRequired) {
         if (!Boolean.TRUE.equals(isLockRequired)) {
             return Mono.just(Boolean.TRUE);
@@ -63,6 +77,15 @@ public Mono<Boolean> acquireGitLock(String baseArtifactId, String commandName, b
         return addFileLock(baseArtifactId, commandName);
     }
 
+    /**
+     * This is a wrapper method for releasing git lock, since multiple ops are used in sequence
+     * for a complete composite operation not all ops require to acquire the lock hence a dummy flag is sent back for
+     * operations in that is getting executed in between
+     * @param baseArtifactId : id of the base artifact for which ops would be locked
+     * @param isLockRequired : is lock really required or is it a proxy function
+     * @return : Boolean for whether the lock is released
+     */
+    // TODO @Manish add artifactType reference in incoming prs
     public Mono<Boolean> releaseFileLock(String baseArtifactId, boolean isLockRequired) {
         if (!Boolean.TRUE.equals(isLockRequired)) {
             return Mono.just(Boolean.TRUE);

diff --git a/...ppsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCECompatibleImpl.java b/...ppsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCECompatibleImpl.java
@@ -1,6 +1,5 @@
 package com.appsmith.server.git.fs;
 
-import com.appsmith.external.git.GitExecutor;
 import com.appsmith.external.git.handler.FSGitHandler;
 import com.appsmith.server.configurations.EmailConfig;
 import com.appsmith.server.datasources.base.DatasourceService;
@@ -43,7 +42,6 @@ public GitFSServiceCECompatibleImpl(
             TransactionalOperator transactionalOperator,
             AnalyticsService analyticsService,
             ObservationRegistry observationRegistry,
-            GitExecutor gitExecutor,
             WorkspaceService workspaceService,
             DatasourceService datasourceService,
             DatasourcePermission datasourcePermission,
@@ -67,7 +65,6 @@ public GitFSServiceCECompatibleImpl(
                 transactionalOperator,
                 analyticsService,
                 observationRegistry,
-                gitExecutor,
                 workspaceService,
                 datasourceService,
                 datasourcePermission,

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
@@ -1,7 +1,6 @@
 package com.appsmith.server.git.fs;
 
 import com.appsmith.external.constants.AnalyticsEvents;
-import com.appsmith.external.git.GitExecutor;
 import com.appsmith.external.git.constants.GitConstants;
 import com.appsmith.external.git.constants.GitSpan;
 import com.appsmith.external.git.handler.FSGitHandler;
@@ -80,8 +79,6 @@ public class GitFSServiceCEImpl implements GitHandlingServiceCE {
     protected final AnalyticsService analyticsService;
     private final ObservationRegistry observationRegistry;
 
-    protected final GitExecutor gitExecutor;
-
     private final WorkspaceService workspaceService;
     private final DatasourceService datasourceService;
     private final DatasourcePermission datasourcePermission;
@@ -348,7 +345,7 @@ public Mono<Tuple2<? extends Artifact, String>> commitArtifact(
         StringBuilder result = new StringBuilder();
         result.append("Commit Result : ");
 
-        Mono<String> gitCommitMono = gitExecutor
+        Mono<String> gitCommitMono = fsGitHandler
                 .commitArtifact(
                         repoSuffix,
                         commitDTO.getMessage(),
@@ -377,6 +374,12 @@ public Mono<Tuple2<? extends Artifact, String>> commitArtifact(
                 });
     }
 
+    /**
+     * Used for pushing commits present in the given branched artifact.
+     * @param branchedArtifactId : id of the branched artifact.
+     * @param artifactType : type of the artifact
+     * @return : returns a string which has details of operations
+     */
     public Mono<String> pushArtifact(String branchedArtifactId, ArtifactType artifactType) {
         GitArtifactHelper<?> gitArtifactHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType);
         AclPermission artifactEditPermission = gitArtifactHelper.getArtifactEditPermission();
@@ -437,11 +440,11 @@ protected Mono<String> pushArtifact(Artifact branchedArtifact, boolean isFileLoc
                             artifact.getWorkspaceId(), gitData.getDefaultArtifactId(), gitData.getRepoName());
                     GitAuth gitAuth = gitData.getGitAuth();
 
-                    return gitExecutor
+                    return fsGitHandler
                             .checkoutToBranch(
                                     baseRepoSuffix,
                                     artifact.getGitArtifactMetadata().getBranchName())
-                            .then(Mono.defer(() -> gitExecutor
+                            .then(Mono.defer(() -> fsGitHandler
                                     .pushApplication(
                                             baseRepoSuffix,
                                             gitData.getRemoteUrl(),
@@ -526,7 +529,7 @@ private Mono<String> pushArtifactErrorRecovery(String pushResult, Artifact artif
             Path path = gitArtifactHelper.getRepoSuffixPath(
                     artifact.getWorkspaceId(), gitMetadata.getDefaultArtifactId(), gitMetadata.getRepoName());
 
-            return gitExecutor
+            return fsGitHandler
                     .resetHard(path, gitMetadata.getBranchName())
                     .then(Mono.error(new AppsmithException(
                             AppsmithError.GIT_ACTION_FAILED,

diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceImpl.java
@@ -1,6 +1,5 @@
 package com.appsmith.server.git.fs;
 
-import com.appsmith.external.git.GitExecutor;
 import com.appsmith.external.git.handler.FSGitHandler;
 import com.appsmith.server.configurations.EmailConfig;
 import com.appsmith.server.datasources.base.DatasourceService;
@@ -43,7 +42,6 @@ public GitFSServiceImpl(
             TransactionalOperator transactionalOperator,
             AnalyticsService analyticsService,
             ObservationRegistry observationRegistry,
-            GitExecutor gitExecutor,
             WorkspaceService workspaceService,
             DatasourceService datasourceService,
             DatasourcePermission datasourcePermission,
@@ -67,7 +65,6 @@ public GitFSServiceImpl(
                 transactionalOperator,
                 analyticsService,
                 observationRegistry,
-                gitExecutor,
                 workspaceService,
                 datasourceService,
                 datasourcePermission,