From f7293cfa05b1edf2a8fe1e65ca2a6f38440583ab Mon Sep 17 00:00:00 2001 From: Manish Kumar <107841575+sondermanish@users.noreply.github.com> Date: Thu, 18 Sep 2025 17:34:39 +0530 Subject: [PATCH] chore: prelim fixes for the in mem git (#41201) 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: d7e0d5646396a25ffc73c9444a57200993868926 > Cypress dashboard. > Tags: `@tag.Git` > Spec: >
Thu, 18 Sep 2025 11:50:06 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - New Features - Branch-aware Git clone/checkout with Redis-backed caching and automatic cleanup. - Operation-aware Git routing for endpoints. - Enhanced, timestamped logging for Git scripts. - Improvements - Faster, more reliable Git flows with lock-based FSM orchestration. - Consistent merge behavior that honors “keep working directory changes.” - Improved private key handling for SSH. - Error Handling - Clearer, granular Git error messages for metadata, FS ops, Redis download, and cleanup. - Documentation - Updated Git route flow documentation. - Tests - Extensive unit tests covering routing, metadata checks, cleanup gating, and key flows. --- .../com/appsmith/git/service/BashService.java | 4 +- .../appsmith-git/src/main/resources/git.sh | 188 ++++++- .../appsmith/server/annotations/GitRoute.java | 3 + .../artifacts/gitRoute/GitRouteArtifact.java | 6 +- .../gitRoute/GitRouteArtifactCE.java | 11 +- .../server/aspect/GitRouteAspect.java | 527 +++++++++++++++--- .../appsmith/server/aspect/GitRouteAspect.md | 79 +-- .../server/exceptions/AppsmithError.java | 32 ++ .../server/exceptions/AppsmithErrorCode.java | 4 + .../git/constants/GitRouteOperation.java | 75 +++ .../GitApplicationControllerCE.java | 106 +++- .../server/git/fs/GitFSServiceCEImpl.java | 22 +- .../server/aspect/GitRouteAspectTest.java | 448 +++++++++++++++ 13 files changed, 1355 insertions(+), 150 deletions(-) create mode 100644 app/server/appsmith-server/src/main/java/com/appsmith/server/git/constants/GitRouteOperation.java create mode 100644 app/server/appsmith-server/src/test/java/com/appsmith/server/aspect/GitRouteAspectTest.java diff --git a/app/server/appsmith-git/src/main/java/com/appsmith/git/service/BashService.java b/app/server/appsmith-git/src/main/java/com/appsmith/git/service/BashService.java index de1a3a41b2..f42519dbf9 100644 --- a/app/server/appsmith-git/src/main/java/com/appsmith/git/service/BashService.java +++ b/app/server/appsmith-git/src/main/java/com/appsmith/git/service/BashService.java @@ -63,9 +63,7 @@ public class BashService { "Bash execution failed: " + buildErrorDetails(output, error, exceptionError, exitCode)); } - log.info("Script: {}", fullScript); - log.info("Output: {}", output); - log.info("Error: {}", error); + log.info("Output: \n{}", output); log.info("Exit code: {}", exitCode); outputStream.close(); diff --git a/app/server/appsmith-git/src/main/resources/git.sh b/app/server/appsmith-git/src/main/resources/git.sh index 1c643c0b45..b0a123e06d 100644 --- a/app/server/appsmith-git/src/main/resources/git.sh +++ b/app/server/appsmith-git/src/main/resources/git.sh @@ -2,6 +2,23 @@ set -euo pipefail +# Simple logging helpers +log_ts() { + date '+%Y-%m-%dT%H:%M:%S%z' +} + +log_info() { + printf '[%s] [INFO] %s\n' "$(log_ts)" "$*" >&1 +} + +log_warn() { + printf '[%s] [WARN] %s\n' "$(log_ts)" "$*" >&1 +} + +log_error() { + printf '[%s] [ERROR] %s\n' "$(log_ts)" "$*" >&2 +} + # Time-to-live for git artifacts in Redis (24 hours in seconds) GIT_ARTIFACT_TTL=86400 @@ -25,7 +42,22 @@ git_clone() { git -C "$target_folder" init "$target_folder" --initial-branch=none git -C "$target_folder" remote add origin "$remote_url" - GIT_SSH_COMMAND="ssh -i $temp_private_key -o StrictHostKeyChecking=no" git -C "$target_folder" fetch origin --depth=1 + GIT_SSH_COMMAND="ssh -i $temp_private_key -o StrictHostKeyChecking=no" git -C "$target_folder" fetch origin +} + +git_clean_up() { + local redis_key="$1" + local redis_url="$2" + local target_folder="$3" + local key_value_pair_key="$4" + + trap 'rm -rf "'"$target_folder"'"' EXIT ERR + + ## delete the repository from redis + redis-cli -u "$redis_url" DEL "$redis_key" + + ## delete the repository branch_store + redis-cli -u "$redis_url" DEL "$key_value_pair_key" } # Uploads git repo to Redis as compressed archive @@ -33,14 +65,30 @@ git_upload() { local redis_key="$1" local redis_url="$2" local target_folder="$3" + local key_value_pair_key="$4" trap 'rm -rf "'"$target_folder"'"' EXIT ERR rm -f "$target_folder/.git/index.lock" + upload_branches_to_redis_hash "$target_folder" "$redis_url" "$key_value_pair_key" tar -cf - -C "$target_folder" . | zstd -q --threads=0 | base64 -w 0 | redis-cli -u "$redis_url" --raw -x SETEX "$redis_key" "$GIT_ARTIFACT_TTL" } +upload_branches_to_redis_hash() { + # get all local branches and their latest commit sha + local directory_path="$1" + local redis_url="$2" + local key_value_pair_key="$3" + + local branches=$(git -C "$directory_path" for-each-ref --format='"%(refname:short)" %(objectname:short)' refs/heads/) + + log_info "Preparing to upload branch store. Current branches: $branches" + + redis-cli -u "$redis_url" DEL "$key_value_pair_key" + redis-cli -u "$redis_url" HSET "$key_value_pair_key" $branches +} + # Downloads git repo from Redis or clones if not cached git_download() { local author_email="$1" @@ -50,6 +98,9 @@ git_download() { local redis_url="$5" local remote_url="$6" local target_folder="$7" + local key_value_pair_key="$8" + + log_info "Searching for repository: $target_folder with key: $redis_key in redis." rm -rf "$target_folder" mkdir -p "$target_folder" @@ -57,24 +108,141 @@ git_download() { if [ "$(redis-cli -u "$redis_url" --raw EXISTS "$redis_key")" = "1" ]; then redis-cli -u "$redis_url" --raw GET "$redis_key" | base64 -d | zstd -d --threads=0 | tar -xf - -C "$target_folder" else - git_clone "$private_key" "$remote_url" "$target_folder" + log_warn "Cache miss. Repository: $target_folder with key: $redis_key does not exist in redis." + return 1 fi rm -f "$target_folder/.git/index.lock" - + git -C "$target_folder" reset --hard git -C "$target_folder" config user.name "$author_name" git -C "$target_folder" config user.email "$author_email" - git -C "$target_folder" config fetch.parallel 4 +} - git -C "$target_folder" reset --hard +git_clone_and_checkout() { + local author_email="$1" + local author_name="$2" + local private_key="$3" + local remote_url="$4" + local target_folder="$5" + local redis_url="$6" + local key_value_pair_key="$7" - # Checkout all branches - for remote in $(git -C "$target_folder" branch -r | grep -vE 'origin/HEAD'); do - branch=${remote#origin/} - if ! git -C "$target_folder" show-ref --quiet "refs/heads/$branch"; then - git -C "$target_folder" checkout -b "$branch" "$remote" || true + ## branches are after argument 7 + + trap 'rm -rf "'"$target_folder"'"' ERR + + log_info "target_folder: $target_folder, remote_url: $remote_url" + ## remove the repository directory entirely + rm -rf "$target_folder" + + ## create the same directory + mkdir -p "$target_folder" + + git_clone "$private_key" "$remote_url" "$target_folder" + git -C "$target_folder" config user.name "$author_name" + git -C "$target_folder" config user.email "$author_email" + git -C "$target_folder" config fetch.parallel 4 + git -C "$target_folder" reflog expire --expire=now --all + git -C "$target_folder" gc --prune=now --aggressive + + # This provides all the arguments from arg 5 onwards to the function git_co_from_redis. + # This includes the target folder, redis url, key value pair key, and all branch names from db. + git_checkout_from_branch_store ${@:5} +} + +git_checkout_from_branch_store() { + local repository_path="$1" + local redis_url="$2" + local key_value_pair_key="$3" + + # 4th onwards args are branches names from db + # fetch raw value + log_info "Searching Redis branch-store for key : $key_value_pair_key" + local raw + raw=$(redis-cli -u "$redis_url" --raw HGETALL "$key_value_pair_key" | sed 's/\"//g') + + # error handling: empty or missing key + if [[ -z "$raw" ]]; then + log_warn "No value found for key '$key_value_pair_key'. Initiating checkouts from database branch names" + git_checkout_all_branches "$repository_path" "${@:4}" + return 0 + fi + + # Read into an array line by line (handles special chars, no word splitting) + arr=() + while IFS= read -r line; do + arr+=( "$line" ) + done <<< "$raw" + + # Iterate through pairs: arr[0]=branch, arr[1]=sha ... + local error_return_value=0 + for ((i=0; i<${#arr[@]}; i+=2)); do + local branch="${arr[i]}" + local commit="${arr[i+1]}" + + # Skip incomplete pair just in case + if [[ -z "$branch" || -z "$commit" ]]; then + continue fi + + # call the fallback function + git_checkout_at_commit "$repository_path" "$branch" "$commit" || { + log_warn "Git_checkout_at_commit failed for $branch $commit" + error_return_value=1 + } + done + + if [[ $error_return_value -eq 1 ]]; then + log_warn "git_checkout_at_commit failed for some branches. Initiating checkouts from database branch names" + git_checkout_all_branches "$repository_path" "${@:4}" + return 0 + fi + + return 0 +} + +# Function to checkout all local branches in a repository with args from db branch names +git_checkout_all_branches() { + local target_folder="$1" + + # From arg 2 onwards, all args are local branch names populated from db. + # Checkout all local branches + for arg in "${@:2}"; do + local local_branch=$arg + log_info "Checking out branch $local_branch from database" + git -C "$target_folder" checkout $local_branch || log_error "Git checkout failed for $local_branch" + done +} + +# Function to checkout a branch at a specific commit in a specified directory +# If branch exists, it will reset it to the given SHA +# If branch doesn't exist, it will create it at the given SHA +git_checkout_at_commit() { + # Check if we have exactly 3 arguments + if [ $# -ne 3 ]; then + log_info "Usage: git_checkout_at_commit " + log_info "Example: git_checkout_at_commit /path/to/repo feature-branch abc123def" + return 1 + fi + + local directory_path="$1" + local branch_name="$2" + local commit_sha="$3" + + # Check if directory exists + if [ ! -d "$directory_path" ]; then + log_error "Directory '$directory_path' does not exist" + return 1 + fi + + # Checkout the branch + git -C "$directory_path" checkout $branch_name + + # Hard reset to the specified commit + git -C "$directory_path" reset --hard "$commit_sha" + + log_info "✓ Branch '$branch_name' has been reset to commit $commit_sha in '$directory_path'" } git_merge_branch() { diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/annotations/GitRoute.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/annotations/GitRoute.java index 720963956e..05f57570d0 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/annotations/GitRoute.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/annotations/GitRoute.java @@ -1,6 +1,7 @@ package com.appsmith.server.annotations; import com.appsmith.server.constants.ArtifactType; +import com.appsmith.server.git.constants.GitRouteOperation; import java.lang.annotation.Documented; import java.lang.annotation.ElementType; @@ -15,4 +16,6 @@ public @interface GitRoute { String fieldName(); ArtifactType artifactType(); + + GitRouteOperation operation(); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/artifacts/gitRoute/GitRouteArtifact.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/artifacts/gitRoute/GitRouteArtifact.java index 2b11e85354..9c35a8a2d1 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/artifacts/gitRoute/GitRouteArtifact.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/artifacts/gitRoute/GitRouteArtifact.java @@ -1,12 +1,14 @@ package com.appsmith.server.artifacts.gitRoute; +import com.appsmith.server.git.resolver.GitArtifactHelperResolver; import com.appsmith.server.repositories.ApplicationRepository; import org.springframework.stereotype.Component; @Component public class GitRouteArtifact extends GitRouteArtifactCE { - public GitRouteArtifact(ApplicationRepository applicationRepository) { - super(applicationRepository); + public GitRouteArtifact( + ApplicationRepository applicationRepository, GitArtifactHelperResolver gitArtifactHelperResolver) { + super(applicationRepository, gitArtifactHelperResolver); } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/artifacts/gitRoute/GitRouteArtifactCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/artifacts/gitRoute/GitRouteArtifactCE.java index d81b513dd9..d15d348a73 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/artifacts/gitRoute/GitRouteArtifactCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/artifacts/gitRoute/GitRouteArtifactCE.java @@ -4,16 +4,21 @@ import com.appsmith.server.constants.ArtifactType; import com.appsmith.server.domains.Artifact; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.git.resolver.GitArtifactHelperResolver; import com.appsmith.server.repositories.ApplicationRepository; +import com.appsmith.server.services.GitArtifactHelper; import org.springframework.stereotype.Component; import reactor.core.publisher.Mono; @Component public abstract class GitRouteArtifactCE { + protected final GitArtifactHelperResolver gitArtifactHelperResolver; protected final ApplicationRepository applicationRepository; - public GitRouteArtifactCE(ApplicationRepository applicationRepository) { + public GitRouteArtifactCE( + ApplicationRepository applicationRepository, GitArtifactHelperResolver gitArtifactHelperResolver) { this.applicationRepository = applicationRepository; + this.gitArtifactHelperResolver = gitArtifactHelperResolver; } public Mono getArtifact(ArtifactType artifactType, String artifactId) { @@ -25,4 +30,8 @@ public abstract class GitRouteArtifactCE { default -> Mono.error(new AppsmithException(AppsmithError.GIT_ROUTE_HANDLER_NOT_FOUND, artifactType)); }; } + + public GitArtifactHelper getArtifactHelper(ArtifactType artifactType) { + return this.gitArtifactHelperResolver.getArtifactHelper(artifactType); + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/aspect/GitRouteAspect.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/aspect/GitRouteAspect.java index 56f520d00b..34370028c7 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/aspect/GitRouteAspect.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/aspect/GitRouteAspect.java @@ -1,5 +1,7 @@ package com.appsmith.server.aspect; +import com.appsmith.external.git.constants.GitSpan; +import com.appsmith.external.git.constants.ce.RefType; import com.appsmith.git.configurations.GitServiceConfig; import com.appsmith.git.service.BashService; import com.appsmith.server.annotations.GitRoute; @@ -10,8 +12,11 @@ import com.appsmith.server.domains.GitArtifactMetadata; import com.appsmith.server.domains.GitAuth; import com.appsmith.server.domains.GitProfile; import com.appsmith.server.exceptions.AppsmithError; +import com.appsmith.server.exceptions.AppsmithErrorCode; import com.appsmith.server.exceptions.AppsmithException; import com.appsmith.server.git.utils.GitProfileUtils; +import com.appsmith.server.services.GitArtifactHelper; +import io.micrometer.observation.ObservationRegistry; import lombok.AllArgsConstructor; import lombok.Data; import lombok.Getter; @@ -28,19 +33,29 @@ import org.bouncycastle.util.io.pem.PemReader; import org.springframework.beans.factory.annotation.Value; import org.springframework.data.redis.core.ReactiveRedisTemplate; import org.springframework.stereotype.Component; +import org.springframework.util.StringUtils; +import reactor.core.observability.micrometer.Micrometer; import reactor.core.publisher.Mono; +import reactor.util.retry.Retry; import java.io.StringReader; -import java.nio.file.Paths; +import java.nio.file.Path; import java.security.KeyFactory; import java.security.PrivateKey; import java.security.spec.PKCS8EncodedKeySpec; import java.time.Duration; +import java.util.ArrayList; import java.util.Base64; +import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.Function; import java.util.stream.IntStream; +import static com.appsmith.server.helpers.GitUtils.MAX_RETRIES; +import static com.appsmith.server.helpers.GitUtils.RETRY_DELAY; +import static org.springframework.util.StringUtils.hasText; + @Aspect @Component @RequiredArgsConstructor @@ -48,26 +63,37 @@ import java.util.stream.IntStream; public class GitRouteAspect { private static final Duration LOCK_TTL = Duration.ofSeconds(90); + private static final String REDIS_FILE_LOCK_VALUE = "inUse"; + + private static final String RUN_ERROR_MESSAGE_FORMAT = "An error occurred during state: %s of git. Error: %s"; private static final String REDIS_REPO_KEY_FORMAT = "purpose=repo/v=1/workspace=%s/artifact=%s/repository=%s/"; + private static final String REDIS_LOCK_KEY_FORMAT = "purpose=lock/%s"; + private static final String REDIS_BRANCH_STORE_FORMAT = "branchStore=%s"; + private static final String BASH_COMMAND_FILE = "git.sh"; + private static final String GIT_UPLOAD = "git_upload"; + private static final String GIT_DOWNLOAD = "git_download"; + private static final String GIT_CLONE = "git_clone_and_checkout"; + private static final String GIT_CLEAN_UP = "git_clean_up"; private final ReactiveRedisTemplate redis; private final GitProfileUtils gitProfileUtils; private final GitServiceConfig gitServiceConfig; private final GitRouteArtifact gitRouteArtifact; private final BashService bashService = new BashService(); + private final ObservationRegistry observationRegistry; @Value("${appsmith.redis.git.url}") private String redisUrl; - @Value("${appsmith.git.root}") - private String gitRootPath; - /* * FSM: Definitions */ private enum State { ARTIFACT, + ROUTE_FILTER, + METADATA_FILTER, + UNROUTED_EXECUTION, PARENT, GIT_META, REPO_KEY, @@ -78,7 +104,11 @@ public class GitRouteAspect { GIT_KEY, REPO_PATH, DOWNLOAD, + FETCH_BRANCHES, + CLONE, EXECUTE, + CLEAN_UP_FILTER, + CLEAN_UP, UPLOAD, UNLOCK, RESULT, @@ -115,6 +145,9 @@ public class GitRouteAspect { // Tasks private Artifact artifact; + private Boolean routeFilter; + private Boolean metadataFilter; + private Boolean cleanUpFilter; private Artifact parent; private GitArtifactMetadata gitMeta; private String repoKey; @@ -124,19 +157,31 @@ public class GitRouteAspect { private GitAuth gitAuth; private String gitKey; private String repoPath; + private String branchStoreKey; private Object download; private Object execute; private Object upload; + private Object cleanUp; private Boolean unlock; private Object result; + private List localBranches; + private Object clone; // Errors - private Throwable error; + private AppsmithException error; } // Refer to GitRouteAspect.md#gitroute-fsm-execution-flow for the FSM diagram. private final Map FSM = Map.ofEntries( - Map.entry(State.ARTIFACT, new StateConfig(State.PARENT, State.RESULT, "artifact", this::artifact)), + Map.entry( + State.ROUTE_FILTER, + new StateConfig(State.ARTIFACT, State.UNROUTED_EXECUTION, "routeFilter", this::routeFilter)), + Map.entry(State.UNROUTED_EXECUTION, new StateConfig(State.RESULT, State.RESULT, "execute", this::execute)), + Map.entry(State.RESULT, new StateConfig(State.DONE, State.DONE, "result", this::result)), + Map.entry(State.ARTIFACT, new StateConfig(State.METADATA_FILTER, State.RESULT, "artifact", this::artifact)), + Map.entry( + State.METADATA_FILTER, + new StateConfig(State.PARENT, State.UNROUTED_EXECUTION, "metadataFilter", this::metadataFilter)), Map.entry(State.PARENT, new StateConfig(State.GIT_META, State.RESULT, "parent", this::parent)), Map.entry(State.GIT_META, new StateConfig(State.REPO_KEY, State.RESULT, "gitMeta", this::gitMeta)), Map.entry(State.REPO_KEY, new StateConfig(State.LOCK_KEY, State.RESULT, "repoKey", this::repoKey)), @@ -146,17 +191,35 @@ public class GitRouteAspect { Map.entry(State.GIT_AUTH, new StateConfig(State.GIT_KEY, State.UNLOCK, "gitAuth", this::gitAuth)), Map.entry(State.GIT_KEY, new StateConfig(State.REPO_PATH, State.UNLOCK, "gitKey", this::gitKey)), Map.entry(State.REPO_PATH, new StateConfig(State.DOWNLOAD, State.UNLOCK, "repoPath", this::repoPath)), - Map.entry(State.DOWNLOAD, new StateConfig(State.EXECUTE, State.UNLOCK, "download", this::download)), - Map.entry(State.EXECUTE, new StateConfig(State.UPLOAD, State.UPLOAD, "execute", this::execute)), + Map.entry( + State.DOWNLOAD, + new StateConfig(State.EXECUTE, State.FETCH_BRANCHES, "download", this::downloadFromRedis)), + Map.entry( + State.EXECUTE, + new StateConfig(State.CLEAN_UP_FILTER, State.CLEAN_UP_FILTER, "execute", this::execute)), + Map.entry( + State.CLEAN_UP_FILTER, + new StateConfig(State.UPLOAD, State.CLEAN_UP, "cleanUpFilter", this::cleanUpFilter)), Map.entry(State.UPLOAD, new StateConfig(State.UNLOCK, State.UNLOCK, "upload", this::upload)), + Map.entry(State.CLEAN_UP, new StateConfig(State.UNLOCK, State.UNLOCK, "cleanUp", this::cleanUp)), Map.entry(State.UNLOCK, new StateConfig(State.RESULT, State.RESULT, "unlock", this::unlock)), - Map.entry(State.RESULT, new StateConfig(State.DONE, State.DONE, "result", this::result))); + Map.entry( + State.FETCH_BRANCHES, + new StateConfig(State.CLONE, State.UNLOCK, "localBranches", this::getLocalBranches)), + Map.entry(State.CLONE, new StateConfig(State.EXECUTE, State.UNLOCK, "clone", this::clone))); /* * FSM: Runners */ - // Entry point for Git operations + /** + * Entry point advice for methods annotated with {@link GitRoute}. When Git is configured to operate in-memory, + * this initializes the FSM context and executes the Git-aware flow; otherwise, proceeds directly. + * + * @param joinPoint the intercepted join point + * @param gitRoute the {@link GitRoute} annotation from the intercepted method + * @return the result of the intercepted method, possibly wrapped via FSM flow + */ @Around("@annotation(gitRoute)") public Object handleGitRoute(ProceedingJoinPoint joinPoint, GitRoute gitRoute) { Context ctx = new Context().setJoinPoint(joinPoint).setGitRoute(gitRoute); @@ -167,14 +230,20 @@ public class GitRouteAspect { } String fieldValue = extractFieldValue(joinPoint, gitRoute.fieldName()); - ctx.setFieldValue(fieldValue); - - return run(ctx, State.ARTIFACT) - .flatMap(unused -> ctx.getError() != null ? Mono.error(ctx.getError()) : Mono.just(ctx.getResult())); + return run(ctx, State.ROUTE_FILTER).flatMap(unused -> { + return this.result(ctx); + }); } - // State machine executor + /** + * Executes the Git FSM by evaluating the function associated with the current state and transitioning based + * on the outcome until reaching the DONE state. + * + * @param ctx the FSM execution context + * @param current the current {@link State} + * @return a Mono that completes when the FSM reaches the DONE state + */ private Mono run(Context ctx, State current) { if (current == State.DONE) { return Mono.just(true); @@ -188,52 +257,182 @@ public class GitRouteAspect { .flatMap(result -> { setContextField(ctx, config.getContextField(), result); long duration = System.currentTimeMillis() - startTime; - log.info("State: {}, SUCCESS: {}, Time: {}ms", current, result, duration); + + // selective logging of information + if (State.REPO_KEY.equals(current) || State.LOCK_KEY.equals(current)) { + log.info( + "Operation : {}, State {} : {}, Data : {}, Time : {}ms", + ctx.getGitRoute().operation(), + current, + Outcome.SUCCESS.name(), + result, + duration); + } + + log.info( + "Operation : {}, State {} : {}, Time: {}ms", + ctx.getGitRoute().operation(), + current, + Outcome.SUCCESS.name(), + duration); return run(ctx, config.next(Outcome.SUCCESS)); }) .onErrorResume(e -> { - ctx.setError(e); + if (e instanceof AppsmithException appsmithException) { + ctx.setError(appsmithException); + } else { + ctx.setError(new AppsmithException( + AppsmithError.GIT_ACTION_FAILED, + ctx.getGitRoute().operation().toString().toLowerCase(), + String.format(RUN_ERROR_MESSAGE_FORMAT, current.name(), e.getMessage()))); + } + long duration = System.currentTimeMillis() - startTime; - log.info("State: {}, FAIL: {}, Time: {}ms", current, e.getMessage(), duration); + log.error( + "Operation : {}, State {} : {}, Error : {}, Time: {}ms", + ctx.getGitRoute().operation(), + current, + Outcome.FAIL.name(), + e.getMessage(), + duration); return run(ctx, config.next(Outcome.FAIL)); }); } - /* - * FSM: Tasks + /** + * Attempts to acquire a Redis-based lock for the given key, storing the git command as value. + * + * @param key the redis lock key + * @param gitCommand the git command being executed (used for diagnostics) + * @return Mono emitting true if lock acquired, or error if already locked */ + private Mono setLock(String key, String gitCommand) { + String command = hasText(gitCommand) ? gitCommand : REDIS_FILE_LOCK_VALUE; - // Acquires Redis lock - private Mono lock(Context ctx) { - return redis.opsForValue() - .setIfAbsent(ctx.getLockKey(), "1", LOCK_TTL) - .flatMap(locked -> locked - ? Mono.just(true) - : Mono.error(new AppsmithException(AppsmithError.GIT_FILE_IN_USE, ctx.getLockKey()))); + return redis.opsForValue().setIfAbsent(key, command, LOCK_TTL).flatMap(locked -> { + if (Boolean.TRUE.equals(locked)) { + return Mono.just(Boolean.TRUE); + } + + return redis.opsForValue() + .get(key) + .flatMap(commandName -> + Mono.error(new AppsmithException(AppsmithError.GIT_FILE_IN_USE, command, commandName))); + }); } - // Finds artifact + /** + * Acquires a Redis lock with retry semantics for transient contention scenarios. + * + * @param key the redis lock key + * @param gitCommand the git command associated with the lock + * @return Mono emitting true on successful lock acquisition, or error after retries exhaust + */ + private Mono addLockWithRetry(String key, String gitCommand) { + return this.setLock(key, gitCommand) + .retryWhen(Retry.fixedDelay(MAX_RETRIES, RETRY_DELAY) + .onRetryExhaustedThrow((retryBackoffSpec, retrySignal) -> { + if (retrySignal.failure() instanceof AppsmithException) { + throw (AppsmithException) retrySignal.failure(); + } + + throw new AppsmithException(AppsmithError.GIT_FILE_IN_USE, gitCommand); + })) + .name(GitSpan.ADD_FILE_LOCK) + .tap(Micrometer.observation(observationRegistry)); + } + + /** + * FSM state: acquire the Redis lock for the current repository operation. + * + * @param ctx the FSM execution context containing lock key and operation + * @return Mono emitting true when lock is acquired + */ + private Mono lock(Context ctx) { + String key = ctx.getLockKey(); + String command = ctx.getGitRoute().operation().name(); + + return this.addLockWithRetry(key, command); + } + + /** + * FSM state: resolve the target {@link Artifact} for the operation based on annotation metadata. + * + * @param ctx the FSM execution context + * @return Mono emitting the resolved Artifact + */ private Mono artifact(Context ctx) { ArtifactType artifactType = ctx.getGitRoute().artifactType(); String artifactId = ctx.getFieldValue(); return gitRouteArtifact.getArtifact(artifactType, artifactId); } - // Finds parent artifact + /** + * This method finds out if the current operation requires git operation. + * @param ctx : context + * @return: A mono which emits a boolean, else errors out. + */ + private Mono routeFilter(Context ctx) { + if (ctx.getGitRoute().operation().requiresGitOperation()) { + return Mono.just(Boolean.TRUE); + } + + return Mono.error(new AppsmithException( + AppsmithError.GIT_ROUTE_FS_OPS_NOT_REQUIRED, ctx.getGitRoute().operation())); + } + + /** + * FSM state: validate that the artifact has sufficient Git metadata to require filesystem operations. + * + * @param ctx the FSM execution context + * @return Mono emitting true if metadata is present and valid, or error otherwise + */ + private Mono metadataFilter(Context ctx) { + // if the metadata is null, default artifact id, remote url, or reponame is not present, + // then that means that at this point, either this artifact is not git connected. + Artifact artifact = ctx.getArtifact(); + GitArtifactMetadata metadata = artifact.getGitArtifactMetadata(); + + if (metadata == null + || !StringUtils.hasText(metadata.getDefaultArtifactId()) + || !StringUtils.hasText(metadata.getRemoteUrl()) + || !StringUtils.hasText(metadata.getRepoName())) { + return Mono.error(new AppsmithException(AppsmithError.GIT_ROUTE_METADATA_NOT_FOUND)); + } + + return Mono.just(Boolean.TRUE); + } + + /** + * FSM state: resolve the parent artifact (by default artifact id) for repository-scoped operations. + * + * @param ctx the FSM execution context + * @return Mono emitting the parent Artifact + */ private Mono parent(Context ctx) { ArtifactType artifactType = ctx.getGitRoute().artifactType(); String parentArtifactId = ctx.getArtifact().getGitArtifactMetadata().getDefaultArtifactId(); return gitRouteArtifact.getArtifact(artifactType, parentArtifactId); } - // Validates Git metadata + /** + * FSM state: validate that Git metadata exists on the parent artifact. + * + * @param ctx the FSM execution context + * @return Mono emitting the {@link GitArtifactMetadata} or an error if missing + */ private Mono gitMeta(Context ctx) { return Mono.justOrEmpty(ctx.getParent().getGitArtifactMetadata()) .switchIfEmpty(Mono.error(new AppsmithException( AppsmithError.INVALID_GIT_CONFIGURATION, "Git metadata is not configured"))); } - // Generates Redis repo key + /** + * FSM state: generate the canonical Redis repository key for the operation. + * + * @param ctx the FSM execution context + * @return Mono emitting the repository key string + */ private Mono repoKey(Context ctx) { String key = String.format( REDIS_REPO_KEY_FORMAT, @@ -243,13 +442,23 @@ public class GitRouteAspect { return Mono.just(key); } - // Generates Redis lock key + /** + * FSM state: generate the lock key from the repository key. + * + * @param ctx the FSM execution context + * @return Mono emitting the lock key string + */ private Mono lockKey(Context ctx) { - String key = String.format("purpose=lock/%s", ctx.getRepoKey()); + String key = String.format(REDIS_LOCK_KEY_FORMAT, ctx.getRepoKey()); return Mono.just(key); } - // Gets Git user profile + /** + * FSM state: resolve the current user's {@link GitProfile}. + * + * @param ctx the FSM execution context + * @return Mono emitting the Git profile, or error if not configured + */ private Mono gitProfile(Context ctx) { return gitProfileUtils .getGitProfileForUser(ctx.getFieldValue()) @@ -257,49 +466,130 @@ public class GitRouteAspect { AppsmithError.INVALID_GIT_CONFIGURATION, "Git profile is not configured"))); } - // Validates Git auth + /** + * FSM state: validate presence of {@link GitAuth} on the artifact metadata. + * + * @param ctx the FSM execution context + * @return Mono emitting the GitAuth or error if missing + */ private Mono gitAuth(Context ctx) { return Mono.justOrEmpty(ctx.getGitMeta().getGitAuth()) .switchIfEmpty(Mono.error(new AppsmithException( AppsmithError.INVALID_GIT_CONFIGURATION, "Git authentication is not configured"))); } - // Processes Git SSH key + /** + * FSM state: process and normalize the SSH private key for Git operations. + * + * @param ctx the FSM execution context + * @return Mono emitting a normalized private key string or error if processing fails + */ private Mono gitKey(Context ctx) { try { return Mono.just(processPrivateKey( ctx.getGitAuth().getPrivateKey(), ctx.getGitAuth().getPublicKey())); } catch (Exception e) { return Mono.error(new AppsmithException( - AppsmithError.INVALID_GIT_CONFIGURATION, "Failed to process private key: " + e.getMessage())); + AppsmithError.GIT_ROUTE_INVALID_PRIVATE_KEY, "Failed to process private key: " + e.getMessage())); } } - // Gets local repo path + /** + * FSM state: compute the local repository path for the artifact and set the branch store key. + * + * @param ctx the FSM execution context + * @return Mono emitting the absolute repository path as string + */ private Mono repoPath(Context ctx) { - var path = Paths.get( - gitRootPath, - ctx.getArtifact().getWorkspaceId(), - ctx.getGitMeta().getDefaultArtifactId(), - ctx.getGitMeta().getRepoName()); - return Mono.just(path.toString()); + // this needs to be changed based on artifact as well. + Path repositorySuffixPath = gitRouteArtifact + .getArtifactHelper(ctx.getGitRoute().artifactType()) + .getRepoSuffixPath( + ctx.getArtifact().getWorkspaceId(), + ctx.getGitMeta().getDefaultArtifactId(), + ctx.getGitMeta().getRepoName()); + + ctx.setBranchStoreKey(String.format(REDIS_BRANCH_STORE_FORMAT, repositorySuffixPath)); + Path repositoryPath = Path.of(gitServiceConfig.getGitRootPath()).resolve(repositorySuffixPath); + return Mono.just(repositoryPath.toAbsolutePath().toString()); } - // Downloads Git repo - private Mono download(Context ctx) { - return bashService.callFunction( - "git.sh", - "git_download", - ctx.getGitProfile().getAuthorEmail(), + /** + * FSM state: download repository content from Redis-backed storage into the working directory. + * + * @param ctx the FSM execution context + * @return Mono signaling completion of download script execution + */ + private Mono downloadFromRedis(Context ctx) { + return bashService + .callFunction( + BASH_COMMAND_FILE, + GIT_DOWNLOAD, + ctx.getGitProfile().getAuthorEmail(), + ctx.getGitProfile().getAuthorName(), + ctx.getGitKey(), + ctx.getRepoKey(), + redisUrl, + ctx.getGitMeta().getRemoteUrl(), + ctx.getRepoPath(), + ctx.getBranchStoreKey()) + .onErrorResume(error -> Mono.error( + new AppsmithException(AppsmithError.GIT_ROUTE_REDIS_DOWNLOAD_FAILED, error.getMessage()))); + } + + /** + * FSM state: fetch all local branch ref names for the parent artifact base id. + * + * @param ctx the FSM execution context + * @return Mono emitting a list of local branch names + */ + private Mono getLocalBranches(Context ctx) { + GitArtifactHelper gitArtifactHelper = + gitRouteArtifact.getArtifactHelper(ctx.getGitRoute().artifactType()); + return gitArtifactHelper + .getAllArtifactByBaseId(ctx.getParent().getGitArtifactMetadata().getDefaultArtifactId(), null) + .filter(artifact -> { + if (artifact.getGitArtifactMetadata() == null + || RefType.tag.equals( + artifact.getGitArtifactMetadata().getRefType())) { + return Boolean.FALSE; + } + return Boolean.TRUE; + }) + .map(artifact -> artifact.getGitArtifactMetadata().getRefName()) + .collectList(); + } + + /** + * FSM state: fallback clone and checkout flow when download fails; clones remote and checks out known branches. + * + * @param ctx the FSM execution context + * @return Mono signaling completion of clone script execution + */ + private Mono clone(Context ctx) { + List metaArgs = List.of( + ctx.getGitProfile().getAuthorName(), ctx.getGitProfile().getAuthorName(), ctx.getGitKey(), - ctx.getRepoKey(), - redisUrl, ctx.getGitMeta().getRemoteUrl(), - ctx.getRepoPath()); + ctx.getRepoPath(), + redisUrl, + ctx.getBranchStoreKey()); + + List completeArgs = new ArrayList<>(metaArgs); + completeArgs.addAll(ctx.localBranches); + + String[] varArgs = completeArgs.toArray(new String[0]); + + return bashService.callFunction(BASH_COMMAND_FILE, GIT_CLONE, varArgs); } - // Executes Git operation + /** + * FSM state: proceed the intercepted join point (business logic) within the Git-managed flow. + * + * @param ctx the FSM execution context + * @return Mono emitting the intercepted method's result or error + */ private Mono execute(Context ctx) { try { return (Mono) ctx.getJoinPoint().proceed(); @@ -308,26 +598,90 @@ public class GitRouteAspect { } } - // Uploads Git changes - private Mono upload(Context ctx) { - return bashService.callFunction("git.sh", "git_upload", ctx.getRepoKey(), redisUrl, ctx.getRepoPath()); + /** + * FSM state: This method finds out if redis and FS cleanup is required + * + * @param ctx the FSM execution context + * @return Mono emitting the intercepted method's result or error + */ + private Mono cleanUpFilter(Context ctx) { + // if clean up is not required then proceed + if (!ctx.getGitRoute().operation().gitCleanUp()) { + return Mono.just(Boolean.TRUE); + } + + return Mono.error(new AppsmithException( + AppsmithError.GIT_ROUTE_FS_CLEAN_UP_REQUIRED, ctx.getGitRoute().operation())); } - // Releases Redis lock + /** + * FSM state: upload local repository changes to Redis-backed storage. + * + * @param ctx the FSM execution context + * @return Mono signaling completion of upload script execution + */ + private Mono cleanUp(Context ctx) { + return bashService.callFunction( + BASH_COMMAND_FILE, + GIT_CLEAN_UP, + ctx.getRepoKey(), + redisUrl, + ctx.getRepoPath(), + ctx.getBranchStoreKey()); + } + + /** + * FSM state: upload local repository changes to Redis-backed storage. + * + * @param ctx the FSM execution context + * @return Mono signaling completion of upload script execution + */ + private Mono upload(Context ctx) { + return bashService.callFunction( + BASH_COMMAND_FILE, GIT_UPLOAD, ctx.getRepoKey(), redisUrl, ctx.getRepoPath(), ctx.getBranchStoreKey()); + } + + /** + * FSM state: release the Redis lock acquired for the repository. + * + * @param ctx the FSM execution context + * @return Mono emitting true if the lock was released + */ private Mono unlock(Context ctx) { return redis.delete(ctx.getLockKey()).map(count -> count > 0); } - // Returns operation result + /** + * FSM state: finalize by returning the intercepted method execution result, or propagate an error when + * appropriate based on the error state. + * + * @param ctx the FSM execution context + * @return Mono emitting the business method result or an error + */ private Mono result(Context ctx) { - return ctx.getError() != null ? Mono.error(ctx.getError()) : Mono.just(ctx.getExecute()); + Set errorStates = Set.of( + AppsmithErrorCode.GIT_ROUTE_REDIS_DOWNLOAD_FAILED.getCode(), + AppsmithErrorCode.GIT_ROUTE_FS_CLEAN_UP_REQUIRED.getCode(), + AppsmithErrorCode.GIT_ROUTE_FS_OPS_NOT_REQUIRED.getCode(), + AppsmithErrorCode.GIT_ROUTE_METADATA_NOT_FOUND.getCode()); + if (ctx.getError() == null || errorStates.contains(ctx.getError().getAppErrorCode())) { + return Mono.just(ctx.getExecute()); + } + + return Mono.error(ctx.getError()); } /* * Helpers: Git Route */ - // Extracts field from join point + /** + * Extracts a named parameter value from a {@link ProceedingJoinPoint}. + * + * @param jp the join point + * @param target the target parameter name to extract + * @return stringified value of the parameter if found, otherwise null + */ private static String extractFieldValue(ProceedingJoinPoint jp, String target) { String[] names = ((CodeSignature) jp.getSignature()).getParameterNames(); Object[] values = jp.getArgs(); @@ -338,7 +692,13 @@ public class GitRouteAspect { .orElse(null); } - // Sets context field value + /** + * Uses reflection to set a field on the {@link Context} object by name. + * + * @param ctx the FSM context + * @param fieldName the field name to set + * @param value the value to assign + */ private static void setContextField(Context ctx, String fieldName, Object value) { try { var field = ctx.getClass().getDeclaredField(fieldName); @@ -354,7 +714,15 @@ public class GitRouteAspect { * Reference: SshTransportConfigCallback.java */ - // Processes SSH private key + /** + * Process an SSH private key that may be in PEM or Base64 PKCS8 form and return a normalized encoded key + * string suitable for downstream usage. + * + * @param privateKey the private key content + * @param publicKey the corresponding public key (used to determine algorithm) + * @return normalized and encoded private key string + * @throws Exception if parsing or key generation fails + */ private static String processPrivateKey(String privateKey, String publicKey) throws Exception { String[] splitKeys = privateKey.split("-----.*-----\n"); return splitKeys.length > 1 @@ -362,7 +730,14 @@ public class GitRouteAspect { : handleBase64Format(privateKey, publicKey); } - // Handles PEM format key + /** + * Handle an OpenSSH PEM-formatted private key and return a Base64-encoded PKCS8 representation. + * + * @param privateKey the PEM-formatted private key + * @param publicKey the public key to infer algorithm + * @return Base64-encoded PKCS8 private key + * @throws Exception if parsing or key generation fails + */ private static String handlePemFormat(String privateKey, String publicKey) throws Exception { byte[] content = new PemReader(new StringReader(privateKey)).readPemObject().getContent(); @@ -372,7 +747,14 @@ public class GitRouteAspect { return Base64.getEncoder().encodeToString(generatedPrivateKey.getEncoded()); } - // Handles Base64 format key + /** + * Handle a Base64-encoded PKCS8 private key blob and return a normalized, formatted key string. + * + * @param privateKey the Base64-encoded PKCS8 private key + * @param publicKey the public key to infer algorithm + * @return normalized, formatted private key string + * @throws Exception if decoding or key generation fails + */ private static String handleBase64Format(String privateKey, String publicKey) throws Exception { PKCS8EncodedKeySpec privateKeySpec = new PKCS8EncodedKeySpec(Base64.getDecoder().decode(privateKey)); @@ -380,13 +762,24 @@ public class GitRouteAspect { return formatPrivateKey(Base64.getEncoder().encodeToString(generatedPrivateKey.getEncoded())); } - // Gets key factory for algorithm + /** + * Get a {@link KeyFactory} instance for the algorithm implied by the provided public key. + * + * @param publicKey the public key whose prefix indicates algorithm + * @return a {@link KeyFactory} for RSA or ECDSA + * @throws Exception if the algorithm or provider is unavailable + */ private static KeyFactory getKeyFactory(String publicKey) throws Exception { String algo = publicKey.startsWith("ssh-rsa") ? "RSA" : "ECDSA"; return KeyFactory.getInstance(algo, new BouncyCastleProvider()); } - // Formats private key string + /** + * Format a Base64-encoded PKCS8 private key into a standard PEM wrapper string. + * + * @param privateKey the Base64-encoded PKCS8 private key + * @return a PEM-wrapped private key string + */ private static String formatPrivateKey(String privateKey) { return "-----BEGIN PRIVATE KEY-----\n" + privateKey + "\n-----END PRIVATE KEY-----"; } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/aspect/GitRouteAspect.md b/app/server/appsmith-server/src/main/java/com/appsmith/server/aspect/GitRouteAspect.md index 063b17ba85..85fe15ca36 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/aspect/GitRouteAspect.md +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/aspect/GitRouteAspect.md @@ -2,43 +2,44 @@ ```mermaid flowchart TD - START([START]) --> ARTIFACT - - ARTIFACT -->|SUCCESS| PARENT - ARTIFACT -->|FAIL| RESULT - - PARENT -->|SUCCESS| GIT_META - PARENT -->|FAIL| RESULT - - GIT_META -->|SUCCESS| REPO_KEY - GIT_META -->|FAIL| RESULT - - REPO_KEY -->|SUCCESS| LOCK_KEY - REPO_KEY -->|FAIL| RESULT - - LOCK_KEY -->|SUCCESS| LOCK - LOCK_KEY -->|FAIL| RESULT - - LOCK -->|SUCCESS| GIT_PROFILE - LOCK -->|FAIL| RESULT - - GIT_PROFILE -->|SUCCESS| GIT_AUTH - GIT_PROFILE -->|FAIL| UNLOCK - - GIT_AUTH -->|SUCCESS| GIT_KEY - GIT_AUTH -->|FAIL| UNLOCK - - GIT_KEY -->|SUCCESS| REPO_PATH - GIT_KEY -->|FAIL| UNLOCK - - REPO_PATH -->|SUCCESS| DOWNLOAD - REPO_PATH -->|FAIL| UNLOCK - - DOWNLOAD -->|SUCCESS| EXECUTE - DOWNLOAD -->|FAIL| UNLOCK - - EXECUTE -->|SUCCESS or FAIL| UPLOAD - UPLOAD -->|SUCCESS or FAIL| UNLOCK - UNLOCK -->|SUCCESS or FAIL| RESULT - RESULT --> DONE([DONE]) + flowchart TD + START(["START"]) --> ROUTE_FILTER["ROUTE_FILTER"] + ROUTE_FILTER -- SUCCESS --> ARTIFACT["ARTIFACT"] + ROUTE_FILTER -- FAIL --> UNROUTED_EXECUTION["UNROUTED_EXECUTION"] + ARTIFACT -- SUCCESS --> METADATA_FILTER["METADATA_FILTER"] + ARTIFACT -- FAIL --> RESULT["RESULT"] + UNROUTED_EXECUTION -- SUCCESS or FAIL --> RESULT + METADATA_FILTER -- SUCCESS --> PARENT["PARENT"] + METADATA_FILTER -- FAIL --> UNROUTED_EXECUTION + PARENT -- SUCCESS --> GIT_META["GIT_META"] + PARENT -- FAIL --> RESULT + GIT_META -- SUCCESS --> REPO_KEY["REPO_KEY"] + GIT_META -- FAIL --> RESULT + REPO_KEY -- SUCCESS --> LOCK_KEY["LOCK_KEY"] + REPO_KEY -- FAIL --> RESULT + LOCK_KEY -- SUCCESS --> LOCK["LOCK"] + LOCK_KEY -- FAIL --> RESULT + LOCK -- SUCCESS --> GIT_PROFILE["GIT_PROFILE"] + LOCK -- FAIL --> RESULT + GIT_PROFILE -- SUCCESS --> GIT_AUTH["GIT_AUTH"] + GIT_PROFILE -- FAIL --> UNLOCK["UNLOCK"] + GIT_AUTH -- SUCCESS --> GIT_KEY["GIT_KEY"] + GIT_AUTH -- FAIL --> UNLOCK + GIT_KEY -- SUCCESS --> REPO_PATH["REPO_PATH"] + GIT_KEY -- FAIL --> UNLOCK + REPO_PATH -- SUCCESS --> DOWNLOAD["DOWNLOAD"] + REPO_PATH -- FAIL --> UNLOCK + DOWNLOAD -- SUCCESS --> EXECUTE["EXECUTE"] + DOWNLOAD -- FAIL --> FETCH_BRANCHES["FETCH_BRANCHES"] + FETCH_BRANCHES -- SUCCESS --> CLONE["CLONE"] + FETCH_BRANCHES -- FAIL --> UNLOCK + CLONE -- SUCCESS --> EXECUTE + CLONE -- FAIL --> UNLOCK + EXECUTE -- SUCCESS or FAIL --> CLEAN_UP_FILTER["CLEAN_UP_FILTER"] + CLEAN_UP_FILTER -- SUCCESS --> UPLOAD["UPLOAD"] + CLEAN_UP_FILTER -- FAIL --> CLEAN_UP["CLEAN_UP"] + UPLOAD -- SUCCESS or FAIL --> UNLOCK + CLEAN_UP -- SUCCESS or FAIL --> UNLOCK + UNLOCK -- SUCCESS or FAIL --> RESULT + RESULT --> DONE(["DONE"]) ``` diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java index 2877b7fcd2..ab6e454a9c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithError.java @@ -1020,6 +1020,38 @@ public enum AppsmithError { "Insufficient password strength", ErrorType.ARGUMENT_ERROR, null), + GIT_ROUTE_METADATA_NOT_FOUND( + 404, + AppsmithErrorCode.GIT_ROUTE_METADATA_NOT_FOUND.getCode(), + "Artifact metadata not found for Git route: {0}", + AppsmithErrorAction.DEFAULT, + "Git route metadata not found", + ErrorType.GIT_CONFIGURATION_ERROR, + null), + GIT_ROUTE_FS_OPS_NOT_REQUIRED( + 404, + AppsmithErrorCode.GIT_ROUTE_FS_OPS_NOT_REQUIRED.getCode(), + "Endpoint : {0}, does not require File system operation", + AppsmithErrorAction.DEFAULT, + "Git FS operation not required", + ErrorType.GIT_CONFIGURATION_ERROR, + null), + GIT_ROUTE_FS_CLEAN_UP_REQUIRED( + 404, + AppsmithErrorCode.GIT_ROUTE_FS_CLEAN_UP_REQUIRED.getCode(), + "Endpoint : {0}, requires FS and redis cleanup", + AppsmithErrorAction.DEFAULT, + "Git FS operation not required", + ErrorType.INTERNAL_ERROR, + null), + GIT_ROUTE_REDIS_DOWNLOAD_FAILED( + 500, + AppsmithErrorCode.GIT_ROUTE_REDIS_DOWNLOAD_FAILED.getCode(), + "Download of git repository from redis failed. Error: {0}", + AppsmithErrorAction.DEFAULT, + "Git repository download failed", + ErrorType.GIT_CONFIGURATION_ERROR, + null), GIT_ROUTE_HANDLER_NOT_FOUND( 500, AppsmithErrorCode.GIT_ROUTE_HANDLER_NOT_FOUND.getCode(), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java index e19a9f0353..5f11cbf93b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/exceptions/AppsmithErrorCode.java @@ -138,6 +138,10 @@ public enum AppsmithErrorCode { GIT_ROUTE_CONTEXT_BUILD_ERROR("AE-GIT-5007", "Git route context build error"), GIT_ROUTE_ARTIFACT_NOT_FOUND("AE-GIT-5008", "Git route artifact not found"), GIT_ROUTE_INVALID_PRIVATE_KEY("AE-GIT-5009", "Git route invalid private key"), + GIT_ROUTE_METADATA_NOT_FOUND("AE-GIT-5010", "Git route metadata not found"), + GIT_ROUTE_FS_OPS_NOT_REQUIRED("AE-GIT-5011", "Git FS operation not required"), + GIT_ROUTE_REDIS_DOWNLOAD_FAILED("AE-GIT-5012", "Git redis download failed"), + GIT_ROUTE_FS_CLEAN_UP_REQUIRED("AE-GIT-5015", "Git FS clean up required"), DATASOURCE_CONNECTION_RATE_LIMIT_BLOCKING_FAILED( "AE-TMR-4031", "Rate limit exhausted, blocking the host name failed"), diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/constants/GitRouteOperation.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/constants/GitRouteOperation.java new file mode 100644 index 0000000000..867ba44f72 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/constants/GitRouteOperation.java @@ -0,0 +1,75 @@ +package com.appsmith.server.git.constants; + +/** + * Common Git route operations for Application and Package controllers. + * + * Grouping: + * - Common operations are listed first. + * - Controller-specific operations are separated by comment delimiters. + */ +public enum GitRouteOperation { + + // Common operations + COMMIT(true), + CREATE_REF(true), + CHECKOUT_REF(true), + DISCONNECT(true, true), + PULL(true), + STATUS(true), + FETCH_REMOTE_CHANGES(true), + MERGE(true), + MERGE_STATUS(true), + DELETE_REF(true), + DISCARD_CHANGES(true), + LIST_REFS(true), + AUTO_COMMIT(true), + + // whitelisted ones + + METADATA(false), + CONNECT(false), + GET_PROTECTED_BRANCHES(false), + UPDATE_PROTECTED_BRANCHES(false), + GET_SSH_KEY(false), + GENERATE_SSH_KEYPAIR(false), + GET_AUTO_COMMIT_PROGRESS(false), + TOGGLE_AUTO_COMMIT(false), + + // EE specific features + + SET_DEFAULT_BRANCH(true), + DEPLOY(true), + + TOGGLE_AUTO_DEPLOYMENT(false), + GENERATE_GIT_TOKEN(false), + + // Package specific + PRE_TAG(true), + PUBLISH(true), + ; + + private final boolean requiresGitOperation; + private final boolean gitCleanUp; + + GitRouteOperation() { + this(true, false); + } + + GitRouteOperation(boolean requiresGitOperation) { + this.requiresGitOperation = requiresGitOperation; + this.gitCleanUp = false; + } + + GitRouteOperation(boolean requiresGitOperation, boolean gitCleanUp) { + this.requiresGitOperation = requiresGitOperation; + this.gitCleanUp = gitCleanUp; + } + + public boolean requiresGitOperation() { + return requiresGitOperation; + } + + public boolean gitCleanUp() { + return gitCleanUp; + } +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/controllers/GitApplicationControllerCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/controllers/GitApplicationControllerCE.java index 8d509911a4..91b611d8b9 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/git/controllers/GitApplicationControllerCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/git/controllers/GitApplicationControllerCE.java @@ -24,6 +24,7 @@ import com.appsmith.server.dtos.ResponseDTO; import com.appsmith.server.git.autocommit.AutoCommitService; import com.appsmith.server.git.central.CentralGitService; import com.appsmith.server.git.central.GitType; +import com.appsmith.server.git.constants.GitRouteOperation; import com.fasterxml.jackson.annotation.JsonView; import jakarta.validation.Valid; import lombok.RequiredArgsConstructor; @@ -60,7 +61,10 @@ public class GitApplicationControllerCE { @JsonView({Views.Metadata.class}) @GetMapping("/{baseApplicationId}/metadata") - @GitRoute(fieldName = "baseApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "baseApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.METADATA) public Mono> getGitMetadata(@PathVariable String baseApplicationId) { return centralGitService .getGitArtifactMetadata(baseApplicationId, ARTIFACT_TYPE) @@ -69,7 +73,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PostMapping("/{applicationId}/connect") - @GitRoute(fieldName = "applicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "applicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.CONNECT) public Mono> connectApplicationToRemoteRepo( @PathVariable String applicationId, @RequestBody GitConnectDTO gitConnectDTO, @@ -82,7 +89,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PostMapping("/{branchedApplicationId}/commit") @ResponseStatus(HttpStatus.CREATED) - @GitRoute(fieldName = "branchedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "branchedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.COMMIT) public Mono> commit( @RequestBody CommitDTO commitDTO, @PathVariable String branchedApplicationId) { log.info("Going to commit branchedApplicationId {}", branchedApplicationId); @@ -94,7 +104,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PostMapping("/{referencedApplicationId}/create-ref") @ResponseStatus(HttpStatus.CREATED) - @GitRoute(fieldName = "referencedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "referencedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.CREATE_REF) public Mono> createReference( @PathVariable String referencedApplicationId, @RequestHeader(name = FieldName.BRANCH_NAME, required = false) String srcBranch, @@ -110,7 +123,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PostMapping("/{referencedApplicationId}/checkout-ref") - @GitRoute(fieldName = "referencedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "referencedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.CHECKOUT_REF) public Mono> checkoutReference( @PathVariable String referencedApplicationId, @RequestBody GitRefDTO gitRefDTO) { return centralGitService @@ -120,7 +136,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PostMapping("/{branchedApplicationId}/disconnect") - @GitRoute(fieldName = "branchedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "branchedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.DISCONNECT) public Mono> disconnectFromRemote(@PathVariable String branchedApplicationId) { log.info("Going to remove the remoteUrl for application {}", branchedApplicationId); return centralGitService @@ -130,7 +149,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @GetMapping("/{branchedApplicationId}/pull") - @GitRoute(fieldName = "branchedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "branchedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.PULL) public Mono> pull(@PathVariable String branchedApplicationId) { log.info("Going to pull the latest for branchedApplicationId {}", branchedApplicationId); return centralGitService @@ -140,7 +162,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @GetMapping("/{branchedApplicationId}/status") - @GitRoute(fieldName = "branchedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "branchedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.STATUS) public Mono> getStatus( @PathVariable String branchedApplicationId, @RequestParam(required = false, defaultValue = "true") Boolean compareRemote) { @@ -152,7 +177,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @GetMapping("/{referencedApplicationId}/fetch/remote") - @GitRoute(fieldName = "referencedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "referencedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.FETCH_REMOTE_CHANGES) public Mono> fetchRemoteChanges( @PathVariable String referencedApplicationId, @RequestHeader(required = false, defaultValue = "branch") RefType refType) { @@ -164,7 +192,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PostMapping("/{branchedApplicationId}/merge") - @GitRoute(fieldName = "branchedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "branchedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.MERGE) public Mono> merge( @PathVariable String branchedApplicationId, @RequestBody GitMergeDTO gitMergeDTO) { log.debug( @@ -179,7 +210,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PostMapping("/{branchedApplicationId}/merge/status") - @GitRoute(fieldName = "branchedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "branchedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.MERGE_STATUS) public Mono> mergeStatus( @PathVariable String branchedApplicationId, @RequestBody GitMergeDTO gitMergeDTO) { log.info( @@ -194,7 +228,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @DeleteMapping("/{baseArtifactId}/ref") - @GitRoute(fieldName = "baseArtifactId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "baseArtifactId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.DELETE_REF) public Mono> deleteBranch( @PathVariable String baseArtifactId, @RequestParam String refName, @@ -207,7 +244,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PutMapping("/{branchedApplicationId}/discard") - @GitRoute(fieldName = "branchedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "branchedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.DISCARD_CHANGES) public Mono> discardChanges(@PathVariable String branchedApplicationId) { log.info("Going to discard changes for branchedApplicationId {}", branchedApplicationId); return centralGitService @@ -217,7 +257,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PostMapping("/{baseArtifactId}/protected-branches") - @GitRoute(fieldName = "baseArtifactId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "baseArtifactId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.UPDATE_PROTECTED_BRANCHES) public Mono>> updateProtectedBranches( @PathVariable String baseArtifactId, @RequestBody @Valid BranchProtectionRequestDTO branchProtectionRequestDTO) { @@ -228,7 +271,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @GetMapping("/{baseArtifactId}/protected-branches") - @GitRoute(fieldName = "baseArtifactId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "baseArtifactId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.GET_PROTECTED_BRANCHES) public Mono>> getProtectedBranches(@PathVariable String baseArtifactId) { return centralGitService .getProtectedBranches(baseArtifactId, ARTIFACT_TYPE) @@ -237,7 +283,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PostMapping("/{branchedApplicationId}/auto-commit") - @GitRoute(fieldName = "branchedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "branchedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.AUTO_COMMIT) public Mono> autoCommitApplication(@PathVariable String branchedApplicationId) { return autoCommitService .autoCommitApplication(branchedApplicationId) @@ -246,7 +295,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @GetMapping("/{baseApplicationId}/auto-commit/progress") - @GitRoute(fieldName = "baseApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "baseApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.GET_AUTO_COMMIT_PROGRESS) public Mono> getAutoCommitProgress( @PathVariable String baseApplicationId, @RequestHeader(name = FieldName.BRANCH_NAME, required = false) String branchName) { @@ -257,7 +309,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PatchMapping("/{baseArtifactId}/auto-commit/toggle") - @GitRoute(fieldName = "baseArtifactId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "baseArtifactId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.TOGGLE_AUTO_COMMIT) public Mono> toggleAutoCommitEnabled(@PathVariable String baseArtifactId) { return centralGitService .toggleAutoCommitEnabled(baseArtifactId, ARTIFACT_TYPE) @@ -266,7 +321,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @GetMapping("/{branchedApplicationId}/refs") - @GitRoute(fieldName = "branchedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "branchedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.LIST_REFS) public Mono>> getReferences( @PathVariable String branchedApplicationId, @RequestParam(required = false, defaultValue = "branch") RefType refType, @@ -279,7 +337,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @GetMapping("/{branchedApplicationId}/ssh-keypair") - @GitRoute(fieldName = "branchedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "branchedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.GET_SSH_KEY) public Mono> getSSHKey(@PathVariable String branchedApplicationId) { return artifactService .getSshKey(ARTIFACT_TYPE, branchedApplicationId) @@ -288,7 +349,10 @@ public class GitApplicationControllerCE { @JsonView(Views.Public.class) @PostMapping("/{branchedApplicationId}/ssh-keypair") - @GitRoute(fieldName = "branchedApplicationId", artifactType = ArtifactType.APPLICATION) + @GitRoute( + fieldName = "branchedApplicationId", + artifactType = ArtifactType.APPLICATION, + operation = GitRouteOperation.GENERATE_SSH_KEYPAIR) public Mono> generateSSHKeyPair( @PathVariable String branchedApplicationId, @RequestParam(required = false) String keyType) { return artifactService 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 69cbce3e56..1652a992ec 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 @@ -714,17 +714,25 @@ public class GitFSServiceCEImpl implements GitHandlingServiceCE { GitArtifactHelper gitArtifactHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType); Path repoSuffix = gitArtifactHelper.getRepoSuffixPath(workspaceId, baseArtifactId, repoName); - if (gitServiceConfig.isGitInMemory()) { - return fsGitHandler.mergeBranch( - repoSuffix, gitMergeDTO.getSourceBranch(), gitMergeDTO.getDestinationBranch()); - } - Mono keepWorkingDirChangesMono = featureFlagService.check(FeatureFlagEnum.release_git_reset_optimization_enabled); // At this point the assumption is that the repository has already checked out the destination branch - return keepWorkingDirChangesMono.flatMap(keepWorkingDirChanges -> fsGitHandler.mergeBranch( - repoSuffix, gitMergeDTO.getSourceBranch(), gitMergeDTO.getDestinationBranch(), keepWorkingDirChanges)); + return keepWorkingDirChangesMono.flatMap(keepWorkingDirChanges -> { + if (gitServiceConfig.isGitInMemory()) { + return fsGitHandler.mergeBranch( + repoSuffix, + gitMergeDTO.getSourceBranch(), + gitMergeDTO.getDestinationBranch(), + keepWorkingDirChanges); + } + + return fsGitHandler.mergeBranch( + repoSuffix, + gitMergeDTO.getSourceBranch(), + gitMergeDTO.getDestinationBranch(), + keepWorkingDirChanges); + }); } @Override diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/aspect/GitRouteAspectTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/aspect/GitRouteAspectTest.java new file mode 100644 index 0000000000..60bbc42360 --- /dev/null +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/aspect/GitRouteAspectTest.java @@ -0,0 +1,448 @@ +package com.appsmith.server.aspect; + +import com.appsmith.external.git.constants.ce.RefType; +import com.appsmith.server.annotations.GitRoute; +import com.appsmith.server.artifacts.gitRoute.GitRouteArtifact; +import com.appsmith.server.constants.ArtifactType; +import com.appsmith.server.domains.Application; +import com.appsmith.server.domains.Artifact; +import com.appsmith.server.domains.GitArtifactMetadata; +import com.appsmith.server.domains.GitAuth; +import com.appsmith.server.domains.GitProfile; +import com.appsmith.server.exceptions.AppsmithError; +import com.appsmith.server.exceptions.AppsmithException; +import com.appsmith.server.git.constants.GitRouteOperation; +import com.appsmith.server.services.GitArtifactHelper; +import io.micrometer.observation.ObservationRegistry; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.stubbing.Answer; +import org.springframework.data.redis.core.ReactiveRedisTemplate; +import reactor.core.publisher.Flux; +import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; + +import java.lang.reflect.Constructor; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class GitRouteAspectTest { + + @Mock + private ReactiveRedisTemplate redis; + + @Mock + private com.appsmith.server.git.utils.GitProfileUtils gitProfileUtils; + + @Mock + private com.appsmith.git.configurations.GitServiceConfig gitServiceConfig; + + @Mock + private GitRouteArtifact gitRouteArtifact; + + @Mock + private GitArtifactHelper gitArtifactHelper; + + private ObservationRegistry observationRegistry; + + private GitRouteAspect aspect; + + @BeforeEach + void setUp() { + observationRegistry = ObservationRegistry.create(); + aspect = new GitRouteAspect(redis, gitProfileUtils, gitServiceConfig, gitRouteArtifact, observationRegistry); + } + + // ---------------------- Helpers ---------------------- + + private static Class contextClass() throws Exception { + return Class.forName("com.appsmith.server.aspect.GitRouteAspect$Context"); + } + + private static Object newContext() throws Exception { + Class ctxClz = contextClass(); + Constructor ctor = ctxClz.getDeclaredConstructor(); + ctor.setAccessible(true); + return ctor.newInstance(); + } + + private static void setCtx(Object ctx, String field, Object value) throws Exception { + Field f = ctx.getClass().getDeclaredField(field); + f.setAccessible(true); + f.set(ctx, value); + } + + private static Object getCtx(Object ctx, String field) throws Exception { + Field f = ctx.getClass().getDeclaredField(field); + f.setAccessible(true); + return f.get(ctx); + } + + @SuppressWarnings("unchecked") + private Mono invokeMono(String methodName, Class[] paramTypes, Object... args) throws Exception { + Method m = GitRouteAspect.class.getDeclaredMethod(methodName, paramTypes); + m.setAccessible(true); + return (Mono) m.invoke(aspect, args); + } + + private Mono invokeSetLock(String key, String command) throws Exception { + return invokeMono("setLock", new Class[] {String.class, String.class}, key, command); + } + + private Mono invokeAddLockWithRetry(String key, String command) throws Exception { + return invokeMono("addLockWithRetry", new Class[] {String.class, String.class}, key, command); + } + + private Mono invokeRouteFilter(Object ctx) throws Exception { + return invokeMono("routeFilter", new Class[] {contextClass()}, ctx); + } + + private Mono invokeCleanUpFilter(Object ctx) throws Exception { + return invokeMono("cleanUpFilter", new Class[] {contextClass()}, ctx); + } + + private Mono invokeMetadataFilter(Object ctx) throws Exception { + return invokeMono("metadataFilter", new Class[] {contextClass()}, ctx); + } + + private Mono invokeGitProfile(Object ctx) throws Exception { + return invokeMono("gitProfile", new Class[] {contextClass()}, ctx); + } + + private Mono invokeGitAuth(Object ctx) throws Exception { + return invokeMono("gitAuth", new Class[] {contextClass()}, ctx); + } + + private Mono invokeRepoKey(Object ctx) throws Exception { + return invokeMono("repoKey", new Class[] {contextClass()}, ctx); + } + + private Mono invokeLockKey(Object ctx) throws Exception { + return invokeMono("lockKey", new Class[] {contextClass()}, ctx); + } + + private Mono invokeGetLocalBranches(Object ctx) throws Exception { + return invokeMono("getLocalBranches", new Class[] {contextClass()}, ctx); + } + + private Mono invokeExecute(Object ctx) throws Exception { + return invokeMono("execute", new Class[] {contextClass()}, ctx); + } + + private Mono invokeUnlock(Object ctx) throws Exception { + return invokeMono("unlock", new Class[] {contextClass()}, ctx); + } + + private Mono invokeResult(Object ctx) throws Exception { + return invokeMono("result", new Class[] {contextClass()}, ctx); + } + + private static GitRoute mockRoute(GitRouteOperation op, ArtifactType type) { + GitRoute route = Mockito.mock(GitRoute.class); + when(route.artifactType()).thenReturn(ArtifactType.APPLICATION); + return route; + } + + private static GitRoute mockRoute(GitRouteOperation op, ArtifactType type, String fieldName) { + GitRoute route = Mockito.mock(GitRoute.class); + when(route.operation()).thenReturn(op); + return route; + } + + private static Artifact newArtifact(String workspaceId, GitArtifactMetadata meta) { + Application a = new Application(); + a.setWorkspaceId(workspaceId); + a.setGitArtifactMetadata(meta); + return a; + } + + // ---------------------- getLocalBranches ---------------------- + + @Test + @DisplayName("getLocalBranches: returns only branch ref names, excluding tags") + void getLocalBranches_positive_filtersTags() throws Exception { + Object ctx = newContext(); + + // route uses APPLICATION to select corresponding helper + setCtx(ctx, "gitRoute", mockRoute(GitRouteOperation.STATUS, ArtifactType.APPLICATION)); + + // parent with base id + GitArtifactMetadata parentMeta = newMeta("base-123", "git@github.com:org/repo.git", "repo"); + Application parent = new Application(); + parent.setGitArtifactMetadata(parentMeta); + setCtx(ctx, "parent", parent); + + // five applications: 3 branches, 2 tags + Application a1 = new Application(); + GitArtifactMetadata m1 = new GitArtifactMetadata(); + m1.setRefType(RefType.branch); + m1.setRefName("b1"); + a1.setGitArtifactMetadata(m1); + + Application a2 = new Application(); + GitArtifactMetadata m2 = new GitArtifactMetadata(); + m2.setRefType(RefType.tag); + m2.setRefName("t1"); + a2.setGitArtifactMetadata(m2); + + Application a3 = new Application(); + GitArtifactMetadata m3 = new GitArtifactMetadata(); + m3.setRefType(RefType.branch); + m3.setRefName("b2"); + a3.setGitArtifactMetadata(m3); + + Application a4 = new Application(); + GitArtifactMetadata m4 = new GitArtifactMetadata(); + m4.setRefType(RefType.tag); + m4.setRefName("t2"); + a4.setGitArtifactMetadata(m4); + + Application a5 = new Application(); + GitArtifactMetadata m5 = new GitArtifactMetadata(); + m5.setRefType(RefType.branch); + m5.setRefName("b3"); + a5.setGitArtifactMetadata(m5); + + // stubs + when(gitRouteArtifact.getArtifactHelper(ArtifactType.APPLICATION)) + .thenReturn((GitArtifactHelper) gitArtifactHelper); + when(gitArtifactHelper.getAllArtifactByBaseId("base-123", null)) + .thenAnswer(getArtifactAnswer(List.of(a1, a2, a3, a4, a5))); + + StepVerifier.create(invokeGetLocalBranches(ctx)) + .assertNext(result -> { + @SuppressWarnings("unchecked") + java.util.List branches = (java.util.List) result; + assertThat(branches).hasSize(3); + assertThat(branches).containsExactly("b1", "b2", "b3"); + }) + .verifyComplete(); + } + + private static Answer> getArtifactAnswer(List appList) { + return invocationOnMock -> Flux.fromIterable(appList); + } + + private static GitArtifactMetadata newMeta(String baseId, String remoteUrl, String repoName) { + GitArtifactMetadata m = new GitArtifactMetadata(); + m.setDefaultArtifactId(baseId); + m.setRemoteUrl(remoteUrl); + m.setRepoName(repoName); + return m; + } + + // ---------------------- routeFilter ---------------------- + + @Test + @DisplayName("routeFilter: emits TRUE when operation requires git FS ops") + void routeFilter_positive() throws Exception { + Object ctx = newContext(); + setCtx(ctx, "gitRoute", mockRoute(GitRouteOperation.COMMIT, ArtifactType.APPLICATION, "id")); + + StepVerifier.create(invokeRouteFilter(ctx)) + .assertNext(nodeResult -> { + Boolean isFsOp = (Boolean) nodeResult; + assertThat(isFsOp).isTrue(); + }) + .verifyComplete(); + } + + @Test + @DisplayName("routeFilter: errors when operation does not require git FS ops") + void routeFilter_negative() throws Exception { + Object ctx = newContext(); + setCtx(ctx, "gitRoute", mockRoute(GitRouteOperation.METADATA, ArtifactType.APPLICATION, "id")); + + StepVerifier.create(invokeRouteFilter(ctx)) + .expectErrorSatisfies(err -> { + assertThat(err).isInstanceOf(AppsmithException.class); + assertThat(((AppsmithException) err).getAppErrorCode()) + .isEqualTo(AppsmithError.GIT_ROUTE_FS_OPS_NOT_REQUIRED.getAppErrorCode()); + }) + .verify(); + } + + // ---------------------- clean-up-filter ---------------------- + + @Test + @DisplayName("cleanUpFilter: emits TRUE when operation does not require cleanup") + void cleanUpFilter_positive() throws Exception { + Object ctx = newContext(); + setCtx(ctx, "gitRoute", mockRoute(GitRouteOperation.COMMIT, ArtifactType.APPLICATION, "id")); + + StepVerifier.create(invokeCleanUpFilter(ctx)) + .assertNext(nodeResult -> { + Boolean notRequiresFilter = (Boolean) nodeResult; + assertThat(notRequiresFilter).isTrue(); + }) + .verifyComplete(); + } + + @Test + @DisplayName("cleanUpFilter: errors when operation require cleanup") + void cleanUp_negative() throws Exception { + Object ctx = newContext(); + setCtx(ctx, "gitRoute", mockRoute(GitRouteOperation.DISCONNECT, ArtifactType.APPLICATION, "id")); + + StepVerifier.create(invokeCleanUpFilter(ctx)) + .expectErrorSatisfies(err -> { + assertThat(err).isInstanceOf(AppsmithException.class); + assertThat(((AppsmithException) err).getAppErrorCode()) + .isEqualTo(AppsmithError.GIT_ROUTE_FS_CLEAN_UP_REQUIRED.getAppErrorCode()); + }) + .verify(); + } + + // ---------------------- metadataFilter ---------------------- + + @Test + @DisplayName("metadataFilter: emits TRUE when metadata is complete") + void metadataFilter_positive() throws Exception { + Object ctx = newContext(); + GitArtifactMetadata meta = newMeta("baseId", "git@github.com:repo.git", "repo"); + setCtx(ctx, "artifact", newArtifact("ws1", meta)); + + StepVerifier.create(invokeMetadataFilter(ctx)) + .assertNext(result -> { + assertThat(result).isEqualTo(Boolean.TRUE); + }) + .verifyComplete(); + } + + @Test + @DisplayName("metadataFilter: errors when metadata is missing/blank") + void metadataFilter_negative() throws Exception { + Object ctx = newContext(); + GitArtifactMetadata meta = newMeta(null, null, null); + setCtx(ctx, "artifact", newArtifact("ws1", meta)); + + StepVerifier.create(invokeMetadataFilter(ctx)) + .expectErrorSatisfies(err -> { + assertThat(err).isInstanceOf(AppsmithException.class); + assertThat(((AppsmithException) err).getAppErrorCode()) + .isEqualTo(AppsmithError.GIT_ROUTE_METADATA_NOT_FOUND.getAppErrorCode()); + }) + .verify(); + } + + // ---------------------- gitProfile ---------------------- + + @Test + @DisplayName("gitProfile: emits profile when present") + void gitProfile_positive() throws Exception { + Object ctx = newContext(); + setCtx(ctx, "fieldValue", "user-id"); + GitProfile profile = new GitProfile(); + profile.setAuthorEmail("u@example.com"); + profile.setAuthorName("User"); + when(gitProfileUtils.getGitProfileForUser("user-id")).thenReturn(Mono.just(profile)); + + StepVerifier.create(invokeGitProfile(ctx)) + .assertNext(gitProfile -> { + GitProfile testProfile = (GitProfile) gitProfile; + assertThat(testProfile).isNotNull(); + assertThat(testProfile.getAuthorName()).isEqualTo("User"); + assertThat(testProfile.getAuthorEmail()).isEqualTo("u@example.com"); + }) + .verifyComplete(); + } + + @Test + @DisplayName("gitProfile: errors when profile not configured") + void gitProfile_negative() throws Exception { + Object ctx = newContext(); + setCtx(ctx, "fieldValue", "user-id"); + when(gitProfileUtils.getGitProfileForUser("user-id")).thenReturn(Mono.empty()); + StepVerifier.create(invokeGitProfile(ctx)) + .expectErrorSatisfies(err -> { + assertThat(err).isInstanceOf(AppsmithException.class); + assertThat(((AppsmithException) err).getAppErrorCode()) + .isEqualTo(AppsmithError.INVALID_GIT_CONFIGURATION.getAppErrorCode()); + }) + .verify(); + } + + // ---------------------- gitAuth ---------------------- + + @Test + @DisplayName("gitAuth: emits auth when present") + void gitAuth_positive() throws Exception { + Object ctx = newContext(); + GitArtifactMetadata meta = newMeta("baseId", "git@github.com:repo.git", "repo"); + GitAuth auth = new GitAuth(); + auth.setPrivateKey( + "-----BEGIN OPENSSH PRIVATE KEY-----\nAAAAB3NzaC1yc2EAAAADAQABAAABAQ==\n-----END OPENSSH PRIVATE KEY-----\n"); + auth.setPublicKey("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQ test"); + meta.setGitAuth(auth); + setCtx(ctx, "gitMeta", meta); + + StepVerifier.create(invokeGitAuth(ctx)) + .assertNext(gitAuth -> { + GitAuth gitAuth1 = (GitAuth) gitAuth; + assertThat(gitAuth1).isNotNull(); + assertThat(gitAuth1.getPublicKey()).isEqualTo("ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQ test"); + assertThat(gitAuth1.getPrivateKey()) + .isEqualTo( + "-----BEGIN OPENSSH PRIVATE KEY-----\nAAAAB3NzaC1yc2EAAAADAQABAAABAQ==\n-----END OPENSSH PRIVATE KEY-----\n"); + }) + .verifyComplete(); + } + + @Test + @DisplayName("gitAuth: errors when missing") + void gitAuth_negative() throws Exception { + Object ctx = newContext(); + GitArtifactMetadata meta = newMeta("baseId", "git@github.com:repo.git", "repo"); + meta.setGitAuth(null); + setCtx(ctx, "gitMeta", meta); + + StepVerifier.create(invokeGitAuth(ctx)) + .expectErrorSatisfies(err -> { + assertThat(err).isInstanceOf(AppsmithException.class); + assertThat(((AppsmithException) err).getAppErrorCode()) + .isEqualTo(AppsmithError.INVALID_GIT_CONFIGURATION.getAppErrorCode()); + }) + .verify(); + } + + // ---------------------- keys (repoKey/lockKey) ---------------------- + + @Test + @DisplayName("repoKey: emits formatted repo key from context") + void repoKey_positive() throws Exception { + Object ctx = newContext(); + GitArtifactMetadata meta = newMeta("base-app", "git@github.com:org/repo.git", "repo"); + Artifact artifact = newArtifact("ws-123", meta); + setCtx(ctx, "artifact", artifact); + setCtx(ctx, "gitMeta", meta); + + StepVerifier.create(invokeRepoKey(ctx)) + .assertNext(key -> { + assertThat(key).isInstanceOf(String.class); + assertThat((String) key) + .isEqualTo("purpose=repo/v=1/workspace=ws-123/artifact=base-app/repository=repo/"); + }) + .verifyComplete(); + } + + @Test + @DisplayName("lockKey: formats lock key even when repoKey is null") + void lockKey_nullRepoKey_yieldsString() throws Exception { + Object ctx = newContext(); + setCtx(ctx, "repoKey", null); + + StepVerifier.create(invokeLockKey(ctx)) + .assertNext(key -> assertThat((String) key).isEqualTo("purpose=lock/null")) + .verifyComplete(); + } +}