From 9e196f572435a1bb3157a078b3a0a185c0e48c6f Mon Sep 17 00:00:00 2001 From: Diljit Date: Tue, 1 Apr 2025 21:36:08 +0530 Subject: [PATCH] chore: ab test simple git reset in git status api (#39959) 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 #`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" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: a3833fe0e894bcb155455947856e7de93bbb0640 > Cypress dashboard. > Tags: `@tag.Git` > Spec: >
Tue, 01 Apr 2025 14:54:53 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Introduced an enhanced Git operation that enables reliable repository reset through a dedicated API endpoint. - Enabled advanced reset options controllable via a new feature flag, improving repository state management. - **Chores** - Upgraded Git-related dependencies and updated the container setup to include Git, ensuring a consistent and robust environment. --- Dockerfile | 6 +++ app/client/packages/rts/package.json | 3 +- app/client/packages/rts/src/ce/server.ts | 2 + .../packages/rts/src/controllers/git/index.ts | 33 +++++++++++++++ .../packages/rts/src/routes/git_routes.ts | 11 +++++ .../packages/rts/src/services/gitService.ts | 17 ++++++++ app/client/packages/rts/src/types/git.dto.ts | 3 ++ app/client/yarn.lock | 30 ++++++++++++- .../appsmith/git/files/FileUtilsCEImpl.java | 12 ++++-- .../appsmith/git/service/GitExecutorImpl.java | 6 ++- .../git/service/ce/GitExecutorCEImpl.java | 42 +++++++++++++++++-- .../git/helpers/FileUtilsImplTest.java | 4 +- .../external/enums/FeatureFlagEnum.java | 4 ++ .../appsmith/external/git/FileInterface.java | 5 ++- .../appsmith/external/git/GitExecutor.java | 3 +- .../git/constants/ce/GitConstantsCE.java | 1 + .../external/git/constants/ce/GitSpanCE.java | 1 + .../AutoCommitEventHandlerCEImpl.java | 2 +- .../git/common/CommonGitServiceCEImpl.java | 2 +- .../helpers/ce/CommonGitFileUtilsCE.java | 20 +++++---- .../server/git/CommonGitServiceCETest.java | 10 ++--- .../appsmith/server/git/GitExecutorTest.java | 2 +- .../AutoCommitEventHandlerImplTest.java | 20 ++++++--- .../server/helpers/GitFileUtilsTest.java | 5 ++- 24 files changed, 206 insertions(+), 38 deletions(-) create mode 100644 app/client/packages/rts/src/controllers/git/index.ts create mode 100644 app/client/packages/rts/src/routes/git_routes.ts create mode 100644 app/client/packages/rts/src/services/gitService.ts create mode 100644 app/client/packages/rts/src/types/git.dto.ts diff --git a/Dockerfile b/Dockerfile index b5374338cf..ca7dc3e9fe 100644 --- a/Dockerfile +++ b/Dockerfile @@ -11,6 +11,12 @@ ENV APPSMITH_SEGMENT_CE_KEY=${APPSMITH_SEGMENT_CE_KEY} COPY deploy/docker/fs / +# Install git +RUN apt-get update && \ + apt-get install -y git && \ + apt-get clean && \ + rm -rf /var/lib/apt/lists/* + RUN <&2 diff --git a/app/client/packages/rts/package.json b/app/client/packages/rts/package.json index 21c8d9eb40..85d37ef455 100644 --- a/app/client/packages/rts/package.json +++ b/app/client/packages/rts/package.json @@ -32,7 +32,8 @@ "loglevel": "^1.8.1", "mongodb": "^5.8.0", "nodemailer": "6.9.9", - "readline-sync": "1.4.10" + "readline-sync": "1.4.10", + "simple-git": "^3.27.0" }, "devDependencies": { "@types/express": "^4.17.14", diff --git a/app/client/packages/rts/src/ce/server.ts b/app/client/packages/rts/src/ce/server.ts index 33b14193ac..3db3ae0b89 100644 --- a/app/client/packages/rts/src/ce/server.ts +++ b/app/client/packages/rts/src/ce/server.ts @@ -7,6 +7,7 @@ import log from "loglevel"; import ast_routes from "../routes/ast_routes"; import dsl_routes from "../routes/dsl_routes"; import health_check_routes from "../routes/health_check_routes"; +import git_routes from "../routes/git_routes"; import { RTS_BASE_API_PATH } from "@constants/routes"; @@ -27,6 +28,7 @@ app.use(express.json({ limit: "5mb" })); app.use(`${RTS_BASE_API_PATH}/ast`, ast_routes); app.use(`${RTS_BASE_API_PATH}/dsl`, dsl_routes); +app.use(`${RTS_BASE_API_PATH}/git`, git_routes); app.use(`${RTS_BASE_API_PATH}`, health_check_routes); server.headersTimeout = 61000; diff --git a/app/client/packages/rts/src/controllers/git/index.ts b/app/client/packages/rts/src/controllers/git/index.ts new file mode 100644 index 0000000000..e12251f0dc --- /dev/null +++ b/app/client/packages/rts/src/controllers/git/index.ts @@ -0,0 +1,33 @@ +import type { Response, Request } from "express"; +import { StatusCodes } from "http-status-codes"; + +import BaseController from "@controllers/BaseController"; +import { reset } from "@services/gitService"; +import log from "loglevel"; +import type { GitResetRequestDTO } from "../../types/git.dto"; + +export default class GitController extends BaseController { + async reset(req: Request, res: Response) { + const request: GitResetRequestDTO = req.body; + + try { + log.info(`Resetting git repository for ${request.repoPath}`); + + const result = await reset(request); + + log.info(`Git repository reset for ${request.repoPath}`); + + return super.sendResponse(res, result); + } catch (err) { + log.info(`Error resetting git repository for ${request.repoPath}`); + log.error(err); + + return super.sendError( + res, + "Something went wrong", + [err.message], + StatusCodes.INTERNAL_SERVER_ERROR, + ); + } + } +} diff --git a/app/client/packages/rts/src/routes/git_routes.ts b/app/client/packages/rts/src/routes/git_routes.ts new file mode 100644 index 0000000000..7a2428896d --- /dev/null +++ b/app/client/packages/rts/src/routes/git_routes.ts @@ -0,0 +1,11 @@ +import express from "express"; +import GitController from "@controllers/git"; +import { Validator } from "@middlewares/Validator"; + +const router = express.Router(); +const gitController = new GitController(); +const validator = new Validator(); + +router.post("/reset", validator.validateRequest, gitController.reset); + +export default router; diff --git a/app/client/packages/rts/src/services/gitService.ts b/app/client/packages/rts/src/services/gitService.ts new file mode 100644 index 0000000000..8264c7be6e --- /dev/null +++ b/app/client/packages/rts/src/services/gitService.ts @@ -0,0 +1,17 @@ +import { type GitResetRequestDTO } from "src/types/git.dto"; +import { ResetMode, simpleGit } from "simple-git"; +import log from "loglevel"; + +export async function reset(request: GitResetRequestDTO) { + const { repoPath } = request; + + const git = simpleGit(repoPath); + + log.info("Resetting git repository: " + repoPath); + await git.reset(ResetMode.HARD); + log.info("Cleaning git repository: " + repoPath); + await git.clean("f", ["-d"]); + log.info("Git repository reset successfully: " + repoPath); + + return {}; +} diff --git a/app/client/packages/rts/src/types/git.dto.ts b/app/client/packages/rts/src/types/git.dto.ts new file mode 100644 index 0000000000..1f640f91d0 --- /dev/null +++ b/app/client/packages/rts/src/types/git.dto.ts @@ -0,0 +1,3 @@ +export interface GitResetRequestDTO { + repoPath: string; +} diff --git a/app/client/yarn.lock b/app/client/yarn.lock index 812fbc36f1..b3828f05df 100644 --- a/app/client/yarn.lock +++ b/app/client/yarn.lock @@ -4378,6 +4378,22 @@ __metadata: languageName: node linkType: hard +"@kwsites/file-exists@npm:^1.1.1": + version: 1.1.1 + resolution: "@kwsites/file-exists@npm:1.1.1" + dependencies: + debug: ^4.1.1 + checksum: 4ff945de7293285133aeae759caddc71e73c4a44a12fac710fdd4f574cce2671a3f89d8165fdb03d383cfc97f3f96f677d8de3c95133da3d0e12a123a23109fe + languageName: node + linkType: hard + +"@kwsites/promise-deferred@npm:^1.1.1": + version: 1.1.1 + resolution: "@kwsites/promise-deferred@npm:1.1.1" + checksum: 07455477a0123d9a38afb503739eeff2c5424afa8d3dbdcc7f9502f13604488a4b1d9742fc7288832a52a6422cf1e1c0a1d51f69a39052f14d27c9a0420b6629 + languageName: node + linkType: hard + "@leichtgewicht/ip-codec@npm:^2.0.1": version: 2.0.4 resolution: "@leichtgewicht/ip-codec@npm:2.0.4" @@ -13526,6 +13542,7 @@ __metadata: mongodb: ^5.8.0 nodemailer: 6.9.9 readline-sync: 1.4.10 + simple-git: ^3.27.0 supertest: ^6.3.3 ts-jest: 29.1.0 typescript: ^5.5.4 @@ -17208,7 +17225,7 @@ __metadata: languageName: node linkType: hard -"debug@npm:4, debug@npm:^4.0.0, debug@npm:^4.1.0, debug@npm:^4.1.1, debug@npm:^4.3.0, debug@npm:^4.3.1, debug@npm:^4.3.2, debug@npm:^4.3.3, debug@npm:^4.3.4, debug@npm:^4.4.0": +"debug@npm:4, debug@npm:^4.0.0, debug@npm:^4.1.0, debug@npm:^4.1.1, debug@npm:^4.3.0, debug@npm:^4.3.1, debug@npm:^4.3.2, debug@npm:^4.3.3, debug@npm:^4.3.4, debug@npm:^4.3.5, debug@npm:^4.4.0": version: 4.4.0 resolution: "debug@npm:4.4.0" dependencies: @@ -31859,6 +31876,17 @@ __metadata: languageName: node linkType: hard +"simple-git@npm:^3.27.0": + version: 3.27.0 + resolution: "simple-git@npm:3.27.0" + dependencies: + "@kwsites/file-exists": ^1.1.1 + "@kwsites/promise-deferred": ^1.1.1 + debug: ^4.3.5 + checksum: bc602d67317a5421363f4cbe446bc71336387a7ea9864b23993dcbbd7e4847e346a234aa5b46bf9d80130d2448cbaeb21cf8f7b62572dce093fb4643ff7ffafd + languageName: node + linkType: hard + "simplebar-react@npm:^2.4.3": version: 2.4.3 resolution: "simplebar-react@npm:2.4.3" 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 f6ccd3dbf3..e8aaf2898e 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 @@ -220,10 +220,14 @@ public class FileUtilsCEImpl implements FileInterface { * @param baseRepoSuffix path suffix used to create a repo path * @param artifactGitReference application reference object from which entire application can be rehydrated * @param branchName name of the branch for the current application + * @param isRtsResetEnabled flag to check if RTS reset is enabled * @return repo path where the application is stored */ public Mono saveApplicationToGitRepo( - Path baseRepoSuffix, ArtifactGitReference artifactGitReference, String branchName) + Path baseRepoSuffix, + ArtifactGitReference artifactGitReference, + String branchName, + Boolean isRtsResetEnabled) throws GitAPIException, IOException { ApplicationGitReference applicationGitReference = (ApplicationGitReference) artifactGitReference; @@ -233,7 +237,7 @@ public class FileUtilsCEImpl implements FileInterface { // Checkout to mentioned branch if not already checked-out Stopwatch processStopwatch = new Stopwatch("FS application save"); return gitExecutor - .resetToLastCommit(baseRepoSuffix, branchName) + .resetToLastCommit(baseRepoSuffix, branchName, isRtsResetEnabled) .flatMap(isSwitched -> { Path baseRepo = Paths.get(gitServiceConfig.getGitRootPath()).resolve(baseRepoSuffix); @@ -1226,7 +1230,7 @@ public class FileUtilsCEImpl implements FileInterface { if (Boolean.TRUE.equals(isResetToLastCommitRequired)) { // instead of checking out to last branch we are first cleaning the git repo, // then checking out to the desired branch - gitResetMono = gitExecutor.resetToLastCommit(baseRepoSuffix, branchName); + gitResetMono = gitExecutor.resetToLastCommit(baseRepoSuffix, branchName, false); } metadataMono = gitResetMono.map(isSwitched -> { @@ -1262,7 +1266,7 @@ public class FileUtilsCEImpl implements FileInterface { if (Boolean.TRUE.equals(resetToLastCommitRequired)) { // instead of checking out to last branch we are first cleaning the git repo, // then checking out to the desired branch - resetToLastCommit = gitExecutor.resetToLastCommit(baseRepoSuffixPath, branchName); + resetToLastCommit = gitExecutor.resetToLastCommit(baseRepoSuffixPath, branchName, false); } pageObjectMono = resetToLastCommit.map(isSwitched -> { diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/service/GitExecutorImpl.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/service/GitExecutorImpl.java index 16177dea96..05fc0975e8 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/service/GitExecutorImpl.java +++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/service/GitExecutorImpl.java @@ -3,6 +3,7 @@ package com.appsmith.git.service; import com.appsmith.external.configurations.git.GitConfig; import com.appsmith.external.git.GitExecutor; import com.appsmith.external.helpers.ObservationHelper; +import com.appsmith.external.services.RTSCaller; import com.appsmith.git.configurations.GitServiceConfig; import com.appsmith.git.service.ce.GitExecutorCEImpl; import io.micrometer.observation.ObservationRegistry; @@ -18,7 +19,8 @@ public class GitExecutorImpl extends GitExecutorCEImpl implements GitExecutor { GitServiceConfig gitServiceConfig, GitConfig gitConfig, ObservationRegistry observationRegistry, - ObservationHelper observationHelper) { - super(gitServiceConfig, gitConfig, observationRegistry, observationHelper); + ObservationHelper observationHelper, + RTSCaller rtsCaller) { + super(gitServiceConfig, gitConfig, observationRegistry, observationHelper, rtsCaller); } } diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java index 05c9d01f7d..47f8181958 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java +++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/service/ce/GitExecutorCEImpl.java @@ -11,6 +11,7 @@ import com.appsmith.external.git.GitExecutor; import com.appsmith.external.git.constants.GitSpan; import com.appsmith.external.helpers.ObservationHelper; import com.appsmith.external.helpers.Stopwatch; +import com.appsmith.external.services.RTSCaller; import com.appsmith.git.configurations.GitServiceConfig; import com.appsmith.git.constants.AppsmithBotAsset; import com.appsmith.git.constants.CommonConstants; @@ -61,6 +62,7 @@ import java.time.format.DateTimeFormatter; import java.util.ArrayList; import java.util.Arrays; import java.util.Date; +import java.util.HashMap; import java.util.List; import java.util.Optional; import java.util.Set; @@ -71,6 +73,7 @@ import java.util.stream.Stream; import static com.appsmith.external.git.constants.GitConstants.GitMetricConstants.CHECKOUT_REMOTE; import static com.appsmith.external.git.constants.GitConstants.GitMetricConstants.HARD_RESET; +import static com.appsmith.external.git.constants.GitConstants.GitMetricConstants.RTS_RESET; import static com.appsmith.git.constants.CommonConstants.FILE_MIGRATION_MESSAGE; import static java.lang.Boolean.FALSE; import static java.lang.Boolean.TRUE; @@ -94,6 +97,7 @@ public class GitExecutorCEImpl implements GitExecutor { private static final String SUCCESS_MERGE_STATUS = "This branch has no conflicts with the base branch."; private final ObservationHelper observationHelper; + private final RTSCaller rtsCaller; /** * This method will handle the git-commit functionality. Under the hood it checks if the repo has already been @@ -854,7 +858,7 @@ public class GitExecutorCEImpl implements GitExecutor { }) .onErrorResume(error -> { try { - return resetToLastCommit(repoSuffix, destinationBranch) + return resetToLastCommit(repoSuffix, destinationBranch, false) .thenReturn(error.getMessage()); } catch (GitAPIException | IOException e) { log.error("Error while hard resetting to latest commit {0}", e); @@ -1031,7 +1035,7 @@ public class GitExecutorCEImpl implements GitExecutor { .flatMap(status -> { try { // Revert uncommitted changes if any - return resetToLastCommit(repoSuffix, destinationBranch) + return resetToLastCommit(repoSuffix, destinationBranch, false) .map(ignore -> { processStopwatch.stopAndLogTimeInMillis(); return status; @@ -1121,7 +1125,39 @@ public class GitExecutorCEImpl implements GitExecutor { .subscribeOn(scheduler); } - public Mono resetToLastCommit(Path repoSuffix, String branchName) throws GitAPIException, IOException { + private Mono resetRts(Path repoSuffix, String branchName) { + Path repoPath = createRepoPath(repoSuffix); + HashMap requestBody = new HashMap<>(); + requestBody.put("repoPath", repoPath.toAbsolutePath().toString()); + log.debug( + "Getting git reset for repo: {}, branch: {}", + repoPath.toAbsolutePath().toString(), + branchName); + + return rtsCaller + .post("/rts-api/v1/git/reset", requestBody) + .flatMap(spec -> spec.retrieve().bodyToMono(Object.class)) + .thenReturn(true) + .tag(HARD_RESET, Boolean.FALSE.toString()) + .tag(RTS_RESET, "true") + .name(GitSpan.FS_RESET) + .tap(Micrometer.observation(observationRegistry)); + } + + public Mono resetToLastCommitRts(Path repoSuffix, String branchName) { + return resetRts(repoSuffix, branchName) + .flatMap(reset -> checkoutToBranch(repoSuffix, branchName)) + .flatMap(checkedOut -> resetRts(repoSuffix, branchName).thenReturn(true)) + .timeout(Duration.ofMillis(Constraint.TIMEOUT_MILLIS)); + } + + public Mono resetToLastCommit(Path repoSuffix, String branchName, Boolean isRtsResetEnabled) + throws GitAPIException, IOException { + if (isRtsResetEnabled) { + log.info("Resetting to last commit using RTS"); + return resetToLastCommitRts(repoSuffix, branchName).thenReturn(true); + } + return Mono.using( () -> Git.open(createRepoPath(repoSuffix).toFile()), git -> this.resetToLastCommit(git) 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 144fee83a0..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 @@ -67,7 +67,7 @@ public class FileUtilsImplTest { Files.createDirectories(actionDirectoryPath); Files.createDirectories(actionCollectionDirectoryPath); - Mockito.when(gitExecutor.resetToLastCommit(Mockito.any(Path.class), Mockito.any())) + Mockito.when(gitExecutor.resetToLastCommit(Mockito.any(Path.class), Mockito.any(), Mockito.anyBoolean())) .thenReturn(Mono.just(true)); ApplicationGitReference applicationGitReference = new ApplicationGitReference(); @@ -80,7 +80,7 @@ public class FileUtilsImplTest { applicationGitReference.setDatasources(new HashMap<>()); applicationGitReference.setJsLibraries(new HashMap<>()); fileUtils - .saveApplicationToGitRepo(Path.of(""), applicationGitReference, "branch") + .saveApplicationToGitRepo(Path.of(""), applicationGitReference, "branch", false) .block(); Assertions.assertFalse(actionDirectoryPath.toFile().exists()); diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java index a1579bdfd7..fcb8ab2c15 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/enums/FeatureFlagEnum.java @@ -20,6 +20,10 @@ public enum FeatureFlagEnum { * Feature flag to detect if the git reset optimization is enabled */ release_git_reset_optimization_enabled, + /** + * Feature flag to detect if the RTS git reset is enabled + */ + ab_rts_git_reset_enabled, // Deprecated CE flags over here release_git_autocommit_feature_enabled, 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 a379e15c04..3f48d2880d 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 @@ -32,7 +32,10 @@ public interface FileInterface { * --page2 */ Mono saveApplicationToGitRepo( - Path baseRepoSuffix, ArtifactGitReference artifactGitReference, String branchName) + Path baseRepoSuffix, + ArtifactGitReference artifactGitReference, + String branchName, + Boolean isRtsResetEnabled) throws IOException, GitAPIException; Mono saveArtifactToGitRepo( diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/GitExecutor.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/GitExecutor.java index e9337c11b8..7f6d9d2489 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/GitExecutor.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/GitExecutor.java @@ -174,7 +174,8 @@ public interface GitExecutor { * @throws GitAPIException * @throws IOException */ - Mono resetToLastCommit(Path repoSuffix, String branchName) throws GitAPIException, IOException; + Mono resetToLastCommit(Path repoSuffix, String branchName, Boolean isRtsResetEnabled) + throws GitAPIException, IOException; /** * diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/GitConstantsCE.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/GitConstantsCE.java index 186f01311f..b4eb2ff141 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/GitConstantsCE.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/GitConstantsCE.java @@ -31,6 +31,7 @@ public class GitConstantsCE { public class GitMetricConstantsCE { public static final String CHECKOUT_REMOTE = "checkout-remote"; public static final String HARD_RESET = "hard-reset"; + public static final String RTS_RESET = "rts-reset"; public static final String RESOURCE_TYPE = "resource-type"; public static final String METADATA = "Metadata"; public static final String WIDGETS = "Widgets"; diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/GitSpanCE.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/GitSpanCE.java index 304ea6eab1..e5671086ef 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/GitSpanCE.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/GitSpanCE.java @@ -26,6 +26,7 @@ public class GitSpanCE { public static final String JGIT_BRANCH_TRACK = APPSMITH_SPAN_PREFIX + GIT_SPAN_PREFIX + "jgit_branch_track"; public static final String FS_CREATE_REPO = APPSMITH_SPAN_PREFIX + GIT_SPAN_PREFIX + "fs_create_repo"; public static final String FS_RESET = APPSMITH_SPAN_PREFIX + GIT_SPAN_PREFIX + "fs_reset"; + public static final String SIMPLE_GIT_RESET = APPSMITH_SPAN_PREFIX + GIT_SPAN_PREFIX + "simple_git_reset"; public static final String JGIT_RESET_HARD = APPSMITH_SPAN_PREFIX + GIT_SPAN_PREFIX + "jgit_reset_hard"; public static final String JGIT_CLEAN = APPSMITH_SPAN_PREFIX + GIT_SPAN_PREFIX + "jgit_clean"; public static final String FS_MERGE = APPSMITH_SPAN_PREFIX + GIT_SPAN_PREFIX + "fs_merge"; diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/autocommit/AutoCommitEventHandlerCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/autocommit/AutoCommitEventHandlerCEImpl.java index 7f7675691f..b600407215 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/autocommit/AutoCommitEventHandlerCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/autocommit/AutoCommitEventHandlerCEImpl.java @@ -94,7 +94,7 @@ public class AutoCommitEventHandlerCEImpl implements AutoCommitEventHandlerCE { Path baseRepoSuffix = Paths.get( autoCommitEvent.getWorkspaceId(), autoCommitEvent.getApplicationId(), autoCommitEvent.getRepoName()); try { - return gitExecutor.resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName()); + return gitExecutor.resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName(), false); } catch (Exception e) { log.error( "failed to reset to last commit before auto commit. application {} branch {}", diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/common/CommonGitServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/common/CommonGitServiceCEImpl.java index f3402991e6..865103b449 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/common/CommonGitServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/common/CommonGitServiceCEImpl.java @@ -2678,7 +2678,7 @@ public class CommonGitServiceCEImpl implements CommonGitServiceCE { .onErrorResume(error -> { try { return gitExecutor - .resetToLastCommit(repoSuffix, destinationBranch) + .resetToLastCommit(repoSuffix, destinationBranch, false) .map(reset -> { MergeStatusDTO mergeStatus = new MergeStatusDTO(); mergeStatus.setMergeAble(false); 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 fa0efe3c30..33db75c8dc 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 @@ -101,7 +101,6 @@ public class CommonGitFileUtilsCE { private final NewActionService newActionService; private final ActionCollectionService actionCollectionService; - // Number of seconds after lock file is stale @Value("${appsmith.index.lock.file.time}") public final int INDEX_LOCK_FILE_STALE_TIME = 300; @@ -159,16 +158,19 @@ public class CommonGitFileUtilsCE { // this should come from the specific files ArtifactGitReference artifactGitReference = createArtifactReference(artifactExchangeJson); + Mono isRtsResetEnabledMono = featureFlagService.check(FeatureFlagEnum.ab_rts_git_reset_enabled); // Save application to git repo - try { - return fileUtils - .saveApplicationToGitRepo(baseRepoSuffix, artifactGitReference, branchName) - .subscribeOn(Schedulers.boundedElastic()); - } catch (IOException | GitAPIException e) { - log.error("Error occurred while saving files to local git repo: ", e); - throw Exceptions.propagate(e); - } + return isRtsResetEnabledMono + .flatMap(isRtsEnabled -> { + try { + return fileUtils.saveApplicationToGitRepo( + baseRepoSuffix, artifactGitReference, branchName, isRtsEnabled); + } catch (IOException | GitAPIException e) { + throw Exceptions.propagate(e); + } + }) + .subscribeOn(Schedulers.boundedElastic()); } public Mono saveArtifactToLocalRepoNew( diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/git/CommonGitServiceCETest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/git/CommonGitServiceCETest.java index ca81cd652b..9671e695a3 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/git/CommonGitServiceCETest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/git/CommonGitServiceCETest.java @@ -1608,7 +1608,7 @@ public class CommonGitServiceCETest { Mockito.anyString(), Mockito.anyBoolean())) .thenReturn(Mono.just("fetched")); - Mockito.when(gitExecutor.resetToLastCommit(any(Path.class), Mockito.anyString())) + Mockito.when(gitExecutor.resetToLastCommit(any(Path.class), Mockito.anyString(), Mockito.anyBoolean())) .thenReturn(Mono.just(true)); Mono applicationMono = @@ -1704,7 +1704,7 @@ public class CommonGitServiceCETest { Mockito.anyString(), Mockito.anyString())) .thenReturn(Mono.just(mergeStatusDTO)); - Mockito.when(gitExecutor.resetToLastCommit(any(Path.class), Mockito.anyString())) + Mockito.when(gitExecutor.resetToLastCommit(any(Path.class), Mockito.anyString(), Mockito.anyBoolean())) .thenReturn(Mono.just(true)); Mono applicationMono = @@ -1754,7 +1754,7 @@ public class CommonGitServiceCETest { Mockito.anyString(), Mockito.anyString())) .thenReturn(Mono.just("fetchResult")); - Mockito.when(gitExecutor.resetToLastCommit(any(Path.class), Mockito.anyString())) + Mockito.when(gitExecutor.resetToLastCommit(any(Path.class), Mockito.anyString(), Mockito.anyBoolean())) .thenReturn(Mono.just(TRUE)); Mockito.when(commonGitFileUtils.saveArtifactToLocalRepoWithAnalytics( any(Path.class), any(), Mockito.anyString())) @@ -1798,7 +1798,7 @@ public class CommonGitServiceCETest { Mockito.anyString(), Mockito.anyString())) .thenReturn(Mono.just("fetchResult")); - Mockito.when(gitExecutor.resetToLastCommit(any(Path.class), Mockito.anyString())) + Mockito.when(gitExecutor.resetToLastCommit(any(Path.class), Mockito.anyString(), Mockito.anyBoolean())) .thenReturn(Mono.just(Boolean.FALSE)); Mockito.when(commonGitFileUtils.saveArtifactToLocalRepoWithAnalytics( any(Path.class), any(), Mockito.anyString())) @@ -1848,7 +1848,7 @@ public class CommonGitServiceCETest { Mockito.anyString(), Mockito.anyString())) .thenReturn(Mono.just("fetchResult")); - Mockito.when(gitExecutor.resetToLastCommit(any(Path.class), Mockito.anyString())) + Mockito.when(gitExecutor.resetToLastCommit(any(Path.class), Mockito.anyString(), Mockito.anyBoolean())) .thenReturn(Mono.just(Boolean.FALSE)); Mockito.when(commonGitFileUtils.saveArtifactToLocalRepoWithAnalytics( any(Path.class), any(), Mockito.anyString())) diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/git/GitExecutorTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/git/GitExecutorTest.java index 6172258cec..13fe8a2ef1 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/git/GitExecutorTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/git/GitExecutorTest.java @@ -668,7 +668,7 @@ public class GitExecutorTest { public void resetToLastCommit_WithOutStaged_CleanStateForRepo() throws IOException, GitAPIException { createFileInThePath("testFile"); commitToRepo(); - Mono resetStatus = gitExecutor.resetToLastCommit(path, "master"); + Mono resetStatus = gitExecutor.resetToLastCommit(path, "master", false); StepVerifier.create(resetStatus) .assertNext(status -> { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/git/autocommit/AutoCommitEventHandlerImplTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/git/autocommit/AutoCommitEventHandlerImplTest.java index c437281adc..8057a74805 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/git/autocommit/AutoCommitEventHandlerImplTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/git/autocommit/AutoCommitEventHandlerImplTest.java @@ -191,7 +191,9 @@ public class AutoCommitEventHandlerImplTest { Path baseRepoSuffix = Paths.get( autoCommitEvent.getWorkspaceId(), autoCommitEvent.getApplicationId(), autoCommitEvent.getRepoName()); - doReturn(Mono.just(TRUE)).when(gitExecutor).resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName()); + doReturn(Mono.just(TRUE)) + .when(gitExecutor) + .resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName(), false); doReturn(Mono.just(applicationJson)) .when(commonGitFileUtils) @@ -261,7 +263,9 @@ public class AutoCommitEventHandlerImplTest { Path baseRepoSuffix = Paths.get( autoCommitEvent.getWorkspaceId(), autoCommitEvent.getApplicationId(), autoCommitEvent.getRepoName()); - doReturn(Mono.just(TRUE)).when(gitExecutor).resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName()); + doReturn(Mono.just(TRUE)) + .when(gitExecutor) + .resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName(), false); doReturn(Mono.just(applicationJson)) .when(commonGitFileUtils) @@ -292,7 +296,9 @@ public class AutoCommitEventHandlerImplTest { Path baseRepoSuffix = Paths.get( autoCommitEvent.getWorkspaceId(), autoCommitEvent.getApplicationId(), autoCommitEvent.getRepoName()); - doReturn(Mono.just(TRUE)).when(gitExecutor).resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName()); + doReturn(Mono.just(TRUE)) + .when(gitExecutor) + .resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName(), false); doReturn(Mono.just(applicationJson)) .when(commonGitFileUtils) @@ -367,7 +373,9 @@ public class AutoCommitEventHandlerImplTest { Path baseRepoSuffix = Paths.get( autoCommitEvent.getWorkspaceId(), autoCommitEvent.getApplicationId(), autoCommitEvent.getRepoName()); - doReturn(Mono.just(TRUE)).when(gitExecutor).resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName()); + doReturn(Mono.just(TRUE)) + .when(gitExecutor) + .resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName(), false); doReturn(Mono.just(applicationJson)) .when(commonGitFileUtils) @@ -411,7 +419,9 @@ public class AutoCommitEventHandlerImplTest { Path baseRepoSuffix = Paths.get( autoCommitEvent.getWorkspaceId(), autoCommitEvent.getApplicationId(), autoCommitEvent.getRepoName()); - doReturn(Mono.just(TRUE)).when(gitExecutor).resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName()); + doReturn(Mono.just(TRUE)) + .when(gitExecutor) + .resetToLastCommit(baseRepoSuffix, autoCommitEvent.getBranchName(), false); ApplicationGitReference appReference = (ApplicationGitReference) commonGitFileUtils.createArtifactReference(applicationJson); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitFileUtilsTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitFileUtilsTest.java index a92ce81d55..68a4668adb 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitFileUtilsTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/GitFileUtilsTest.java @@ -188,7 +188,10 @@ public class GitFileUtilsTest { ApplicationJson validAppJson = createAppJson(filePath).block(); Mockito.when(fileInterface.saveApplicationToGitRepo( - Mockito.any(Path.class), Mockito.any(ApplicationGitReference.class), Mockito.anyString())) + Mockito.any(Path.class), + Mockito.any(ApplicationGitReference.class), + Mockito.anyString(), + Mockito.anyBoolean())) .thenReturn(Mono.just(Path.of("workspaceId", "appId", "repoName"))); Mono resultMono = commonGitFileUtils.saveArtifactToLocalRepoWithAnalytics(