From 93f4a85b6b2e8a342d7b6ff0d6359a8a2bb13f72 Mon Sep 17 00:00:00 2001 From: Diljit Date: Wed, 18 Jun 2025 21:00:05 +0530 Subject: [PATCH] chore: revert in-memory git status (CE) (#40971) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This reverts commit dea1c030da221cc4dd614e03615287fb4538184a. There are duplicate entries for the same js object in the DB for git connected application. In these cases the map generated from the DB git resource map fails as the map doesn't allow duplicate entries. @manish will fix the root cause of the duplicate entries of JS object and then restore this PR. [Relevant thread ](https://theappsmith.slack.com/archives/C04HERDNZPA/p1750063381844219) 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.Sanity" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: e2585053fc35703705fac2e276f0e025a811a1fa > Cypress dashboard. > Tags: `@tag.Sanity` > Spec: >
Wed, 18 Jun 2025 13:53:23 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **Refactor** - Removed certain Git status computation features and related methods from the application. - Internal instrumentation and observation logic have been eliminated. - Adjusted method visibility for improved encapsulation. - **Chores** - Updated internal interfaces and constructors to reflect the removal of deprecated methods and parameters. - Cleaned up unused imports and code references. --- .../appsmith/git/files/FileUtilsCEImpl.java | 74 +------------------ .../com/appsmith/git/files/FileUtilsImpl.java | 13 +--- .../git/handler/ce/FSGitHandlerCEImpl.java | 3 +- .../git/helpers/FileUtilsImplTest.java | 4 +- .../appsmith/external/git/FileInterface.java | 5 -- .../external/git/handler/FSGitHandler.java | 2 - .../git/central/CentralGitServiceCEImpl.java | 33 +-------- .../git/central/GitHandlingServiceCE.java | 3 - .../server/git/fs/GitFSServiceCEImpl.java | 13 ---- .../helpers/ce/CommonGitFileUtilsCE.java | 13 ---- 10 files changed, 11 insertions(+), 152 deletions(-) diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java index 5cd5eef146..862a8b9596 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java +++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsCEImpl.java @@ -1,6 +1,5 @@ package com.appsmith.git.files; -import com.appsmith.external.dtos.GitStatusDTO; import com.appsmith.external.dtos.ModifiedResources; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginError; import com.appsmith.external.exceptions.pluginExceptions.AppsmithPluginException; @@ -20,7 +19,6 @@ import com.appsmith.git.configurations.GitServiceConfig; import com.appsmith.git.constants.CommonConstants; import com.appsmith.git.helpers.DSLTransformerHelper; import com.fasterxml.jackson.databind.ObjectMapper; -import io.micrometer.observation.ObservationRegistry; import io.micrometer.tracing.Span; import lombok.Getter; import lombok.extern.slf4j.Slf4j; @@ -33,7 +31,6 @@ import org.springframework.context.annotation.Import; import org.springframework.stereotype.Component; import org.springframework.util.FileSystemUtils; import org.springframework.util.StringUtils; -import reactor.core.observability.micrometer.Micrometer; import reactor.core.publisher.Mono; import reactor.core.scheduler.Scheduler; import reactor.core.scheduler.Schedulers; @@ -93,7 +90,7 @@ public class FileUtilsCEImpl implements FileInterface { protected final FileOperations fileOperations; private final ObservationHelper observationHelper; protected final ObjectMapper objectMapper; - private final ObservationRegistry observationRegistry; + private static final String EDIT_MODE_URL_TEMPLATE = "{{editModeUrl}}"; private static final String VIEW_MODE_URL_TEMPLATE = "{{viewModeUrl}}"; @@ -111,15 +108,13 @@ public class FileUtilsCEImpl implements FileInterface { GitExecutor gitExecutor, FileOperations fileOperations, ObservationHelper observationHelper, - ObjectMapper objectMapper, - ObservationRegistry observationRegistry) { + ObjectMapper objectMapper) { this.gitServiceConfig = gitServiceConfig; this.fsGitHandler = fsGitHandler; this.gitExecutor = gitExecutor; this.fileOperations = fileOperations; this.observationHelper = observationHelper; this.objectMapper = objectMapper; - this.observationRegistry = observationRegistry; } protected Map getModifiedResourcesTypes() { @@ -295,66 +290,6 @@ public class FileUtilsCEImpl implements FileInterface { .subscribeOn(scheduler); } - public Mono computeGitStatus( - Path baseRepoSuffix, GitResourceMap gitResourceMapFromDB, String branchName, boolean keepWorkingDirChanges) - throws GitAPIException, IOException { - return fsGitHandler - .resetToLastCommit(baseRepoSuffix, branchName, keepWorkingDirChanges) - .flatMap(__ -> constructGitResourceMapFromGitRepo(baseRepoSuffix, branchName)) - .flatMap(gitResourceMapFromFS -> { - Map resourceMapFromDB = gitResourceMapFromDB.getGitResourceMap(); - Map resourceMapFromFS = gitResourceMapFromFS.getGitResourceMap(); - - Map filePathObjectsMapFromFS = resourceMapFromFS.entrySet().parallelStream() - .collect( - Collectors.toMap(entry -> entry.getKey().getFilePath(), entry -> entry.getValue())); - - Map filePathToObjectsFromDB = resourceMapFromDB.entrySet().parallelStream() - .collect( - Collectors.toMap(entry -> entry.getKey().getFilePath(), entry -> entry.getValue())); - - Set filePathsInDb = new HashSet<>(filePathToObjectsFromDB.keySet()); - Set filePathsInFS = new HashSet<>(filePathObjectsMapFromFS.keySet()); - - // added files - Set addedFiles = new HashSet<>(filePathsInDb); - addedFiles.removeAll(filePathsInFS); - - // removed files - Set removedFiles = new HashSet<>(filePathsInFS); - removedFiles.removeAll(filePathsInDb); - removedFiles.remove(README_FILE_NAME); - - // common files - Set commonFiles = new HashSet<>(filePathsInDb); - commonFiles.retainAll(filePathsInFS); - - // modified files - Set modifiedFiles = commonFiles.stream() - .filter(filePath -> { - Object fileInDB = filePathToObjectsFromDB.get(filePath); - Object fileInFS = filePathObjectsMapFromFS.get(filePath); - try { - return fileOperations.hasFileChanged(fileInDB, fileInFS); - } catch (IOException e) { - log.error("Error while checking if file has changed", e); - return false; - } - }) - .collect(Collectors.toSet()); - - GitStatusDTO localRepoStatus = new GitStatusDTO(); - localRepoStatus.setAdded(addedFiles); - localRepoStatus.setModified(modifiedFiles); - localRepoStatus.setRemoved(removedFiles); - boolean isClean = addedFiles.isEmpty() && modifiedFiles.isEmpty() && removedFiles.isEmpty(); - localRepoStatus.setIsClean(isClean); - - fsGitHandler.populateModifiedEntities(localRepoStatus); - return Mono.just(localRepoStatus); - }); - } - protected Set getWhitelistedPaths() { String pages = PAGE_DIRECTORY + DELIMITER_PATH; String datasources = DATASOURCE_DIRECTORY + DELIMITER_PATH; @@ -867,10 +802,7 @@ public class FileUtilsCEImpl implements FileInterface { @Override public Mono constructGitResourceMapFromGitRepo(Path repositorySuffix, String refName) { Path repositoryPath = Paths.get(gitServiceConfig.getGitRootPath()).resolve(repositorySuffix); - return Mono.fromCallable(() -> fetchGitResourceMap(repositoryPath)) - .subscribeOn(scheduler) - .name("construct-git-resource-map") - .tap(Micrometer.observation(observationRegistry)); + return Mono.fromCallable(() -> fetchGitResourceMap(repositoryPath)).subscribeOn(scheduler); } /** diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsImpl.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsImpl.java index 532101f3fe..9122b5d7c3 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsImpl.java +++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/files/FileUtilsImpl.java @@ -7,7 +7,6 @@ import com.appsmith.external.git.operations.FileOperations; import com.appsmith.external.helpers.ObservationHelper; import com.appsmith.git.configurations.GitServiceConfig; import com.fasterxml.jackson.databind.ObjectMapper; -import io.micrometer.observation.ObservationRegistry; import lombok.Getter; import lombok.extern.slf4j.Slf4j; import org.springframework.context.annotation.Import; @@ -27,15 +26,7 @@ public class FileUtilsImpl extends FileUtilsCEImpl implements FileInterface { GitExecutor gitExecutor, FileOperations fileOperations, ObservationHelper observationHelper, - ObjectMapper objectMapper, - ObservationRegistry observationRegistry) { - super( - gitServiceConfig, - fsGitHandler, - gitExecutor, - fileOperations, - observationHelper, - objectMapper, - observationRegistry); + ObjectMapper objectMapper) { + super(gitServiceConfig, fsGitHandler, gitExecutor, fileOperations, observationHelper, objectMapper); } } diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/handler/ce/FSGitHandlerCEImpl.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/handler/ce/FSGitHandlerCEImpl.java index 8900ff542d..14f08fb935 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/handler/ce/FSGitHandlerCEImpl.java +++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/handler/ce/FSGitHandlerCEImpl.java @@ -935,8 +935,7 @@ public class FSGitHandlerCEImpl implements FSGitHandler { .subscribeOn(scheduler); } - @Override - public void populateModifiedEntities(GitStatusDTO response) { + protected void populateModifiedEntities(GitStatusDTO response) { populatePageChanges(response); populateQueryChanges(response); populateJsObjectChanges(response); diff --git a/app/server/appsmith-git/src/test/java/com/appsmith/git/helpers/FileUtilsImplTest.java b/app/server/appsmith-git/src/test/java/com/appsmith/git/helpers/FileUtilsImplTest.java index 74ea5e1412..c3779473dc 100644 --- a/app/server/appsmith-git/src/test/java/com/appsmith/git/helpers/FileUtilsImplTest.java +++ b/app/server/appsmith-git/src/test/java/com/appsmith/git/helpers/FileUtilsImplTest.java @@ -9,7 +9,6 @@ import com.appsmith.git.files.FileUtilsImpl; import com.appsmith.git.files.operations.FileOperationsImpl; import com.appsmith.git.service.GitExecutorImpl; import com.fasterxml.jackson.databind.ObjectMapper; -import io.micrometer.observation.ObservationRegistry; import org.apache.commons.io.FileUtils; import org.eclipse.jgit.api.errors.GitAPIException; import org.junit.jupiter.api.AfterEach; @@ -52,8 +51,7 @@ public class FileUtilsImplTest { gitExecutor, fileOperations, ObservationHelper.NOOP, - new ObjectMapper(), - ObservationRegistry.NOOP); + new ObjectMapper()); } @AfterEach diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/FileInterface.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/FileInterface.java index 234f23ddc4..389feea2fc 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/FileInterface.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/FileInterface.java @@ -1,6 +1,5 @@ package com.appsmith.external.git; -import com.appsmith.external.dtos.GitStatusDTO; import com.appsmith.external.git.models.GitResourceMap; import com.appsmith.external.models.ApplicationGitReference; import com.appsmith.external.models.ArtifactGitReference; @@ -43,10 +42,6 @@ public interface FileInterface { Path baseRepoSuffix, GitResourceMap gitResourceMap, String branchName, boolean keepWorkingDirChanges) throws GitAPIException, IOException; - Mono computeGitStatus( - Path baseRepoSuffix, GitResourceMap gitResourceMap, String branchName, boolean keepWorkingDirChanges) - throws GitAPIException, IOException; - /** * This method will reconstruct the application from the repo * diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/handler/FSGitHandler.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/handler/FSGitHandler.java index ffdcf87b10..dd86807c08 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/handler/FSGitHandler.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/handler/FSGitHandler.java @@ -160,8 +160,6 @@ public interface FSGitHandler { */ Mono getStatus(Path repoPath, String branchName, boolean keepWorkingDirChanges); - void populateModifiedEntities(GitStatusDTO response); - /** * This method merges source branch into destination branch for a git repository which is present on the partial * path provided. This assumes that the branch on which the merge will happen is already checked out 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 168c3c2c21..0751d73f55 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 @@ -1676,10 +1676,8 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE { Mono lockHandledStatusMono = Mono.usingWhen( exportedArtifactJsonMono, artifactExchangeJson -> { - Mono statusMono = gitHandlingService - .computeGitStatus(jsonTransformationDTO, artifactExchangeJson) - .name("in-memory-status-computation") - .tap(Micrometer.observation(observationRegistry)); + Mono prepareForStatus = + gitHandlingService.prepareChangesToBeCommitted(jsonTransformationDTO, artifactExchangeJson); Mono fetchRemoteMono = Mono.just("ignored"); @@ -1697,31 +1695,8 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE { error.getMessage())))); } - return Mono.zip(statusMono, fetchRemoteMono) - .flatMap(tuple -> { - return gitHandlingService - .getBranchTrackingStatus(jsonTransformationDTO) - .map(branchTrackingStatus -> { - GitStatusDTO status = tuple.getT1(); - - if (branchTrackingStatus != null) { - status.setAheadCount(branchTrackingStatus.getAheadCount()); - status.setBehindCount(branchTrackingStatus.getBehindCount()); - status.setRemoteBranch(branchTrackingStatus.getRemoteTrackingBranch()); - - } else { - log.debug( - "Remote tracking details not present for branch: {}, repo: {}", - finalBranchName, - repoName); - status.setAheadCount(0); - status.setBehindCount(0); - status.setRemoteBranch("untracked"); - } - - return status; - }); - }) + return Mono.zip(prepareForStatus, fetchRemoteMono) + .then(Mono.defer(() -> gitHandlingService.getStatus(jsonTransformationDTO))) .onErrorResume(throwable -> { /* in case of any error, the global exception handler will release the lock 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 f7df326090..f3f155e400 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 @@ -86,9 +86,6 @@ public interface GitHandlingServiceCE { Mono prepareChangesToBeCommitted( ArtifactJsonTransformationDTO jsonTransformationDTO, ArtifactExchangeJson artifactExchangeJson); - Mono computeGitStatus( - ArtifactJsonTransformationDTO jsonTransformationDTO, ArtifactExchangeJson artifactExchangeJson); - Mono> commitArtifact( Artifact branchedArtifact, CommitDTO commitDTO, ArtifactJsonTransformationDTO jsonTransformationDTO); 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 eccdcf1442..4ecd79d268 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 @@ -764,19 +764,6 @@ public class GitFSServiceCEImpl implements GitHandlingServiceCE { keepWorkingDirChanges -> fsGitHandler.getStatus(repoPath, refName, keepWorkingDirChanges)); } - public Mono computeGitStatus( - ArtifactJsonTransformationDTO jsonTransformationDTO, ArtifactExchangeJson artifactExchangeJson) { - String workspaceId = jsonTransformationDTO.getWorkspaceId(); - String baseArtifactId = jsonTransformationDTO.getBaseArtifactId(); - String repoName = jsonTransformationDTO.getRepoName(); - String branchName = jsonTransformationDTO.getRefName(); - - ArtifactType artifactType = jsonTransformationDTO.getArtifactType(); - GitArtifactHelper gitArtifactHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType); - Path repoSuffix = gitArtifactHelper.getRepoSuffixPath(workspaceId, baseArtifactId, repoName); - return commonGitFileUtils.computeGitStatus(repoSuffix, artifactExchangeJson, branchName); - } - @Override public Mono createGitReference( ArtifactJsonTransformationDTO baseRefJsonTransformationDTO, diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/CommonGitFileUtilsCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/CommonGitFileUtilsCE.java index e5e3fc29bc..c339613ccb 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/CommonGitFileUtilsCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ce/CommonGitFileUtilsCE.java @@ -1,7 +1,6 @@ package com.appsmith.server.helpers.ce; import com.appsmith.external.constants.AnalyticsEvents; -import com.appsmith.external.dtos.GitStatusDTO; import com.appsmith.external.enums.FeatureFlagEnum; import com.appsmith.external.git.FileInterface; import com.appsmith.external.git.models.GitResourceIdentity; @@ -196,18 +195,6 @@ public class CommonGitFileUtilsCE { }); } - public Mono computeGitStatus( - Path baseRepoSuffix, ArtifactExchangeJson artifactExchangeJson, String branchName) { - GitResourceMap gitResourceMapFromDB = createGitResourceMap(artifactExchangeJson); - try { - return fileUtils - .computeGitStatus(baseRepoSuffix, gitResourceMapFromDB, branchName, true) - .subscribeOn(Schedulers.boundedElastic()); - } catch (IOException | GitAPIException exception) { - return Mono.error(exception); - } - } - public Mono saveArtifactToLocalRepoWithAnalytics( Path baseRepoSuffix, ArtifactExchangeJson artifactExchangeJson, String branchName) {