Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.appsmith.external.dtos;

import com.appsmith.external.git.constants.ce.RefType;
import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.NoArgsConstructor;

@Data
@AllArgsConstructor
@NoArgsConstructor
public class GitRefDTO {

String refName;

RefType refType;

boolean isDefault;

boolean createdFromLocal;
Comment on lines +13 to +19

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add field validation and documentation.

The fields would benefit from:

  1. Validation annotations for refName
  2. JavaDoc documentation explaining each field's purpose
  3. Making fields final for immutability
+import javax.validation.constraints.NotNull;
+
 @Data
 @AllArgsConstructor
 @NoArgsConstructor
 public class GitRefDTO {
 
+    /** The name of the Git reference (e.g., branch name or tag name) */
+    @NotNull
-    String refName;
+    private final String refName;
 
+    /** The type of the Git reference (BRANCH or TAG) */
+    @NotNull
-    RefType refType;
+    private final RefType refType;
 
+    /** Indicates if this is the default reference (e.g., default branch) */
-    boolean isDefault;
+    private final boolean isDefault;
 
+    /** Indicates if the reference was created in the local repository */
-    boolean createdFromLocal;
+    private final boolean createdFromLocal;

Committable suggestion skipped: line range outside the PR's diff.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package com.appsmith.external.git.constants.ce;

public enum RefType {
BRANCH,
TAG
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ public class FieldNameCE {
public static final String IS_REQUIRED = "isRequired";
public static final String UNUSED_DATASOURCE = "UNUSED_DATASOURCE";
public static final String BRANCH_NAME = "branchName";
public static final String REF_NAME = "refName";
public static final String SOURCE_BRANCH = "sourceBranch";
public static final String DESTINATION_BRANCH = "destinationBranch";
public static final String DEFAULT = "default";
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package com.appsmith.server.domains.ce;

import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.external.models.AppsmithDomain;
import com.appsmith.external.views.Views;
import com.appsmith.server.constants.ce.RefType;
import com.appsmith.server.domains.AutoCommitConfig;
import com.appsmith.server.domains.GitAuth;
import com.appsmith.server.domains.GitProfile;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
package com.appsmith.server.git.central;

import com.appsmith.external.dtos.GitStatusDTO;
import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.git.dto.CommitDTO;
import com.appsmith.server.constants.ArtifactType;
import com.appsmith.server.constants.ce.RefType;
import com.appsmith.server.domains.Artifact;
import com.appsmith.server.dtos.ArtifactImportDTO;
import com.appsmith.server.dtos.GitConnectDTO;
Expand Down Expand Up @@ -37,4 +37,12 @@ Mono<String> fetchRemoteChanges(

Mono<GitStatusDTO> getStatus(
String branchedArtifactId, boolean compareRemote, ArtifactType artifactType, GitType gitType);

Mono<? extends Artifact> checkoutReference(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit, should we not be able to make a DTO out of these params?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will you be replacing usage in a separate PR?

String referenceArtifactId,
String referenceToBeCheckedOut,
boolean addFileLock,
ArtifactType artifactType,
GitType gitType,
RefType refType);
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package com.appsmith.server.git.central;

import com.appsmith.external.constants.AnalyticsEvents;
import com.appsmith.external.dtos.GitRefDTO;
import com.appsmith.external.dtos.GitStatusDTO;
import com.appsmith.external.git.constants.GitConstants;
import com.appsmith.external.git.constants.GitSpan;
import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.external.models.Datasource;
import com.appsmith.external.models.DatasourceStorage;
import com.appsmith.git.dto.CommitDTO;
Expand All @@ -12,7 +14,6 @@
import com.appsmith.server.constants.ArtifactType;
import com.appsmith.server.constants.FieldName;
import com.appsmith.server.constants.GitDefaultCommitMessage;
import com.appsmith.server.constants.ce.RefType;
import com.appsmith.server.datasources.base.DatasourceService;
import com.appsmith.server.domains.Artifact;
import com.appsmith.server.domains.GitArtifactMetadata;
Expand Down Expand Up @@ -104,6 +105,9 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE {
private final GitRedisUtils gitRedisUtils;
private final ObservationRegistry observationRegistry;

protected static final String ORIGIN = "origin/";
protected static final String REMOTE_NAME_REPLACEMENT = "";

protected Mono<Boolean> isRepositoryLimitReachedForWorkspace(String workspaceId, Boolean isRepositoryPrivate) {
if (!isRepositoryPrivate) {
return Mono.just(FALSE);
Expand Down Expand Up @@ -333,6 +337,236 @@ private boolean checkIsDatasourceNameConflict(
return false;
}

@Override
public Mono<? extends Artifact> checkoutReference(
String referenceArtifactId,
String referenceToBeCheckedOut,
boolean addFileLock,
ArtifactType artifactType,
GitType gitType,
RefType refType) {

if (!hasText(referenceToBeCheckedOut)) {
return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.REF_NAME));
}

Mono<Tuple2<? extends Artifact, ? extends Artifact>> baseAndBranchedArtifactMono =
getBaseAndBranchedArtifacts(referenceArtifactId, artifactType);

return baseAndBranchedArtifactMono.flatMap(artifactTuples -> {
Artifact sourceArtifact = artifactTuples.getT1();
return checkoutReference(sourceArtifact, referenceToBeCheckedOut, addFileLock, gitType, refType);
});
}

protected Mono<? extends Artifact> checkoutReference(
Artifact baseArtifact,
String referenceToBeCheckedOut,
boolean addFileLock,
GitType gitType,
RefType refType) {

if (!hasText(referenceToBeCheckedOut)) {
return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.REF_NAME));
}

GitArtifactMetadata baseGitMetadata = baseArtifact.getGitArtifactMetadata();

if (isBaseGitMetadataInvalid(baseGitMetadata, gitType)) {
return Mono.error(new AppsmithException(AppsmithError.INVALID_GIT_SSH_CONFIGURATION));
}

String baseArtifactId = baseGitMetadata.getDefaultArtifactId();
final String finalRefName = referenceToBeCheckedOut.replaceFirst(ORIGIN, REMOTE_NAME_REPLACEMENT);

GitArtifactHelper<?> gitArtifactHelper =
gitArtifactHelperResolver.getArtifactHelper(baseArtifact.getArtifactType());
GitHandlingService gitHandlingService = gitHandlingServiceResolver.getGitHandlingService(gitType);

Mono<Boolean> acquireFileLock = gitRedisUtils.acquireGitLock(
baseArtifactId, GitConstants.GitCommandConstants.CHECKOUT_BRANCH, addFileLock);

Mono<? extends Artifact> checkedOutArtifactMono;
// If the user is trying to check out remote reference, create a new reference if it does not exist already

ArtifactJsonTransformationDTO jsonTransformationDTO = new ArtifactJsonTransformationDTO();
jsonTransformationDTO.setWorkspaceId(baseArtifact.getWorkspaceId());
jsonTransformationDTO.setBaseArtifactId(baseGitMetadata.getDefaultArtifactId());
jsonTransformationDTO.setRefType(refType);
jsonTransformationDTO.setArtifactType(baseArtifact.getArtifactType());
jsonTransformationDTO.setRepoName(baseGitMetadata.getRepoName());

if (referenceToBeCheckedOut.startsWith(ORIGIN)) {

// checking for local present references first
checkedOutArtifactMono = gitHandlingService
.listReferences(jsonTransformationDTO, FALSE, refType)
.flatMap(gitRefs -> {
long branchMatchCount = gitRefs.stream()
.filter(gitRef -> gitRef.equals(finalRefName))
.count();

if (branchMatchCount == 0) {
return checkoutRemoteBranch(baseArtifact, finalRefName);
}

return Mono.error(new AppsmithException(
AppsmithError.GIT_ACTION_FAILED,
"checkout",
referenceToBeCheckedOut + " already exists in local - " + finalRefName));
});
} else {
// TODO refactor method to account for RefName as well
checkedOutArtifactMono = Mono.defer(() -> gitArtifactHelper
.getArtifactByBaseIdAndBranchName(
baseArtifactId, finalRefName, gitArtifactHelper.getArtifactReadPermission())
.flatMap(artifact -> gitAnalyticsUtils.addAnalyticsForGitOperation(
AnalyticsEvents.GIT_CHECKOUT_BRANCH,
artifact,
artifact.getGitArtifactMetadata().getIsRepoPrivate())));
}

return checkedOutArtifactMono
.doFinally(signalType -> gitRedisUtils.releaseFileLock(baseArtifactId, addFileLock))
.tag(GitConstants.GitMetricConstants.CHECKOUT_REMOTE, FALSE.toString())
.name(GitSpan.OPS_CHECKOUT_BRANCH)
.tap(Micrometer.observation(observationRegistry))
.onErrorResume(Mono::error);
}

// TODO @Manish: add checkout Remote Branch
protected Mono<? extends Artifact> checkoutRemoteBranch(Artifact baseArtifact, String finalRefName) {
return null;
}
Comment on lines +439 to +442

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Implement the checkoutRemoteBranch method.

The method is currently returning null which could lead to NullPointerException when checking out remote branches.

Would you like me to help implement this method or create a GitHub issue to track this task?


public Mono<? extends Artifact> checkoutReference(
String referencedArtifactId,
GitRefDTO refDTO,
ArtifactType artifactType,
GitType gitType,
RefType refType) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we sending ref type separately here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can put it in the DTO itself, right.


/*
1. Check if the src artifact is available and user have sufficient permissions
2. Create and checkout to requested branch
3. Rehydrate the artifact from source artifact reference
*/

if (!hasText(refDTO.getRefName()) || refDTO.getRefName().startsWith(ORIGIN)) {
return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, BRANCH_NAME));
}

GitArtifactHelper<?> gitArtifactHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType);
GitHandlingService gitHandlingService = gitHandlingServiceResolver.getGitHandlingService(gitType);
AclPermission artifactEditPermission = gitArtifactHelper.getArtifactEditPermission();

Mono<Tuple2<? extends Artifact, ? extends Artifact>> baseAndBranchedArtifactMono =
getBaseAndBranchedArtifacts(referencedArtifactId, artifactType, artifactEditPermission);

Mono<? extends Artifact> createBranchMono = baseAndBranchedArtifactMono
.flatMap(artifactTuples -> {
Artifact baseArtifact = artifactTuples.getT1();
Artifact parentArtifact = artifactTuples.getT2();

GitArtifactMetadata baseGitMetadata = baseArtifact.getGitArtifactMetadata();
GitAuth baseGitAuth = baseGitMetadata.getGitAuth();
GitArtifactMetadata parentGitMetadata = parentArtifact.getGitArtifactMetadata();

ArtifactJsonTransformationDTO jsonTransformationDTO = new ArtifactJsonTransformationDTO();
jsonTransformationDTO.setWorkspaceId(baseArtifact.getWorkspaceId());
jsonTransformationDTO.setBaseArtifactId(baseGitMetadata.getDefaultArtifactId());
jsonTransformationDTO.setRepoName(baseGitMetadata.getRepoName());
jsonTransformationDTO.setArtifactType(baseArtifact.getArtifactType());
jsonTransformationDTO.setRefType(refType);
jsonTransformationDTO.setRefName(refDTO.getRefName());

if (parentGitMetadata == null
|| !hasText(parentGitMetadata.getDefaultArtifactId())
|| !hasText(parentGitMetadata.getRepoName())) {
// TODO: add refType in error
return Mono.error(
new AppsmithException(
AppsmithError.INVALID_GIT_CONFIGURATION,
"Unable to find the parent reference. Please create a reference from other available references"));
}

Mono<Boolean> acquireGitLockMono = gitRedisUtils.acquireGitLock(
baseGitMetadata.getDefaultArtifactId(),
GitConstants.GitCommandConstants.CREATE_BRANCH,
FALSE);
Mono<String> fetchRemoteMono =
gitHandlingService.fetchRemoteChanges(jsonTransformationDTO, baseGitAuth, TRUE);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Update fetchRemoteChanges calls to match new method signature

The fetchRemoteChanges method signature has changed. Update the calls at lines 498 and 1334 to match the new parameters, preventing compilation errors.

Example correction:

- gitHandlingService.fetchRemoteChanges(jsonTransformationDTO, baseGitAuth, TRUE);
+ gitHandlingService.fetchRemoteChanges(baseArtifact, refArtifact, TRUE, gitType, refType);

Also applies to: 1334-1335


return acquireGitLockMono
.flatMap(ignoreLockAcquisition -> fetchRemoteMono.onErrorResume(error ->
Mono.error(new AppsmithException(AppsmithError.GIT_ACTION_FAILED, "fetch", error))))
.flatMap(ignoreFetchString -> gitHandlingService
.listReferences(jsonTransformationDTO, TRUE, refType)
.flatMap(refList -> {
boolean isDuplicateName = refList.stream()
// We are only supporting origin as the remote name so this is safe
// but needs to be altered if we start supporting user defined remote
// names
.anyMatch(ref -> ref.replaceFirst(ORIGIN, REMOTE_NAME_REPLACEMENT)
.equals(refDTO.getRefName()));

if (isDuplicateName) {
return Mono.error(new AppsmithException(
AppsmithError.DUPLICATE_KEY_USER_ERROR,
"remotes/origin/" + refDTO.getRefName(),
FieldName.BRANCH_NAME));
}

Mono<? extends ArtifactExchangeJson> artifactExchangeJsonMono =
exportService.exportByArtifactId(
parentArtifact.getId(), VERSION_CONTROL, artifactType);

Mono<? extends Artifact> newArtifactFromSourceMono =
// TODO: add refType support over here
gitArtifactHelper.createNewArtifactForCheckout(
parentArtifact, refDTO.getRefName());

return Mono.zip(newArtifactFromSourceMono, artifactExchangeJsonMono);
}))
.flatMap(tuple -> {
ArtifactExchangeJson exportedJson = tuple.getT2();
Artifact newRefArtifact = tuple.getT1();

Mono<String> refCreationMono = gitHandlingService
.prepareForNewRefCreation(jsonTransformationDTO)
// TODO: ths error could be shipped to handling layer as well?
.onErrorResume(error -> Mono.error(new AppsmithException(
AppsmithError.GIT_ACTION_FAILED,
"ref creation preparation",
error.getMessage())));

return refCreationMono
.flatMap(ignoredString -> {
return importService.importArtifactInWorkspaceFromGit(
newRefArtifact.getWorkspaceId(),
newRefArtifact.getId(),
exportedJson,
refDTO.getRefName());
})
// after the branch is created, we need to reset the older branch to initial
// commit
.doOnSuccess(newImportedArtifact ->
discardChanges(parentArtifact.getId(), artifactType, gitType));
});
})
.flatMap(newImportedArtifact -> gitAnalyticsUtils
.addAnalyticsForGitOperation(
AnalyticsEvents.GIT_CREATE_BRANCH,
newImportedArtifact,
newImportedArtifact.getGitArtifactMetadata().getIsRepoPrivate())
.doFinally(signalType -> gitRedisUtils.releaseFileLock(
newImportedArtifact.getGitArtifactMetadata().getDefaultArtifactId(), TRUE)))
.name(GitSpan.OPS_CREATE_BRANCH)
.tap(Micrometer.observation(observationRegistry));

return Mono.create(sink -> createBranchMono.subscribe(sink::success, sink::error, null, sink.currentContext()));
}

/**
* Connect the artifact from Appsmith to a git repo
* This is the prerequisite step needed to perform all the git operation for an artifact
Expand Down Expand Up @@ -1097,8 +1331,8 @@ public Mono<String> fetchRemoteChanges(

// current user mono has been zipped just to run in parallel.
Mono<String> fetchRemoteMono = acquireGitLockMono
.then(Mono.defer(() ->
gitHandlingService.fetchRemoteChanges(jsonTransformationDTO, baseArtifactGitData.getGitAuth())))
.then(Mono.defer(() -> gitHandlingService.fetchRemoteChanges(
jsonTransformationDTO, baseArtifactGitData.getGitAuth(), FALSE)))
.flatMap(fetchedRemoteStatusString -> {
return gitRedisUtils
.releaseFileLock(baseArtifactId, isFileLock)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.appsmith.server.git.central;

import com.appsmith.external.dtos.GitStatusDTO;
import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.git.dto.CommitDTO;
import com.appsmith.server.domains.Artifact;
import com.appsmith.server.domains.GitArtifactMetadata;
Expand Down Expand Up @@ -40,8 +41,16 @@ void setRepositoryDetailsInGitArtifactMetadata(

Mono<Boolean> removeRepository(ArtifactJsonTransformationDTO artifactJsonTransformationDTO);

Mono<List<String>> listBranches(
ArtifactJsonTransformationDTO artifactJsonTransformationDTO, Boolean checkRemoteBranches);

Mono<List<String>> listBranches(ArtifactJsonTransformationDTO artifactJsonTransformationDTO);

Mono<List<String>> listReferences(
ArtifactJsonTransformationDTO artifactJsonTransformationDTO,
Boolean checkRemoteReferences,
RefType refType);

Mono<Boolean> validateEmptyRepository(ArtifactJsonTransformationDTO artifactJsonTransformationDTO);

Mono<Boolean> initialiseReadMe(
Expand All @@ -59,10 +68,13 @@ Mono<Boolean> prepareChangesToBeCommitted(
Mono<Tuple2<? extends Artifact, String>> commitArtifact(
Artifact branchedArtifact, CommitDTO commitDTO, ArtifactJsonTransformationDTO jsonTransformationDTO);

Mono<String> fetchRemoteChanges(ArtifactJsonTransformationDTO jsonTransformationDTO, GitAuth gitAuth);
Mono<String> fetchRemoteChanges(
ArtifactJsonTransformationDTO jsonTransformationDTO, GitAuth gitAuth, Boolean isFetchAll);

Mono<? extends ArtifactExchangeJson> recreateArtifactJsonFromLastCommit(
ArtifactJsonTransformationDTO jsonTransformationDTO);

Mono<GitStatusDTO> getStatus(ArtifactJsonTransformationDTO jsonTransformationDTO);

Mono<String> prepareForNewRefCreation(ArtifactJsonTransformationDTO artifactJsonTransformationDTO);
}
Loading