-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: reference lifecycle #38174
chore: reference lifecycle #38174
Changes from 3 commits
9e090e7
7d556ed
e74219c
21933b8
4cd584f
a7b8606
9f2f3bc
592833e
f5dcc3f
73a48ae
376d3ba
804ed27
fe1648a
5cf5026
03a3843
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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; | ||
} |
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 | ||
} |
This file was deleted.
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; | ||
|
@@ -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; | ||
|
@@ -397,8 +398,9 @@ protected Mono<? extends Artifact> checkoutReference( | |
|
||
if (referenceToBeCheckedOut.startsWith(ORIGIN)) { | ||
|
||
// checking for local present references first | ||
checkedOutArtifactMono = gitHandlingService | ||
.listReferences(jsonTransformationDTO, refType) | ||
.listReferences(jsonTransformationDTO, FALSE, refType) | ||
.flatMap(gitRefs -> { | ||
long branchMatchCount = gitRefs.stream() | ||
.filter(gitRef -> gitRef.equals(finalRefName)) | ||
|
@@ -437,6 +439,134 @@ protected Mono<? extends Artifact> checkoutRemoteBranch(Artifact baseArtifact, S | |
return null; | ||
} | ||
|
||
public Mono<? extends Artifact> checkoutReference( | ||
String referencedArtifactId, | ||
GitRefDTO refDTO, | ||
ArtifactType artifactType, | ||
GitType gitType, | ||
RefType refType) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we sending ref type separately here? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Update The 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 | ||
|
@@ -1201,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) | ||
|
There was a problem hiding this comment.
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: