Skip to content
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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Dec 17, 2024
Merged

chore: reference lifecycle #38174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9e090e7
added status changes
sondermanish Dec 14, 2024
7d556ed
added spotless
sondermanish Dec 15, 2024
e74219c
added checkout branch
sondermanish Dec 16, 2024
21933b8
reverted config file commit
sondermanish Dec 16, 2024
4cd584f
reverted config changes
sondermanish Dec 16, 2024
a7b8606
reverted config changes
sondermanish Dec 16, 2024
9f2f3bc
reverted config changes
sondermanish Dec 16, 2024
592833e
changes to EOF for commit file
sondermanish Dec 16, 2024
f5dcc3f
added changes
sondermanish Dec 16, 2024
73a48ae
maded EOF change to config file
sondermanish Dec 16, 2024
376d3ba
Merge branch 'chore/status' into chore/references
sondermanish Dec 16, 2024
804ed27
added changes for checkout
sondermanish Dec 16, 2024
fe1648a
merge with release
sondermanish Dec 17, 2024
5cf5026
merge with release
sondermanish Dec 17, 2024
03a3843
added changes
sondermanish Dec 17, 2024
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
20 changes: 20 additions & 0 deletions app/server/appsmith-interfaces/src/main/java/com/appsmith/external/dtos/GitRefDTO.java
View file
Open in desktop
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
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.

}
6 changes: 6 additions & 0 deletions ...ver/appsmith-interfaces/src/main/java/com/appsmith/external/git/constants/ce/RefType.java
View file
Open in desktop
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
}
2 changes: 2 additions & 0 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/FieldNameCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ 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_TYPE = "refType";
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
6 changes: 0 additions & 6 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/constants/ce/RefType.java
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion ...r/appsmith-server/src/main/java/com/appsmith/server/domains/ce/GitArtifactMetadataCE.java
View file
Open in desktop
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
14 changes: 13 additions & 1 deletion ...er/appsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCE.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package com.appsmith.server.git.central;

import com.appsmith.external.dtos.GitRefDTO;
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 +38,15 @@ Mono<String> fetchRemoteChanges(

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

Mono<? extends Artifact> checkoutReference(
Copy link
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
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
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);

Mono<? extends Artifact> createReference(
String referencedArtifactId, GitRefDTO refDTO, ArtifactType artifactType, GitType gitType);
}
245 changes: 242 additions & 3 deletions ...ppsmith-server/src/main/java/com/appsmith/server/git/central/CentralGitServiceCEImpl.java
View file
Open in desktop

Large diffs are not rendered by default.

14 changes: 13 additions & 1 deletion ...r/appsmith-server/src/main/java/com/appsmith/server/git/central/GitHandlingServiceCE.java
View file
Open in desktop
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);
}
2 changes: 1 addition & 1 deletion ...mith-server/src/main/java/com/appsmith/server/git/dtos/ArtifactJsonTransformationDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package com.appsmith.server.git.dtos;

import com.appsmith.external.git.constants.ce.RefType;
import com.appsmith.server.constants.ArtifactType;
import com.appsmith.server.constants.ce.RefType;
import lombok.Data;

// TODO: Find a better name for this DTO
Expand Down
53 changes: 46 additions & 7 deletions app/server/appsmith-server/src/main/java/com/appsmith/server/git/fs/GitFSServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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.git.handler.FSGitHandler;
import com.appsmith.git.dto.CommitDTO;
import com.appsmith.server.acl.AclPermission;
Expand Down Expand Up @@ -262,22 +263,46 @@ public Mono<List<String>> listBranches(ArtifactJsonTransformationDTO artifactJso
GitArtifactHelper<?> gitArtifactHelper =
gitArtifactHelperResolver.getArtifactHelper(artifactJsonTransformationDTO.getArtifactType());

return listBranches(artifactJsonTransformationDTO, Boolean.FALSE);
}

@Override
public Mono<List<String>> listBranches(
ArtifactJsonTransformationDTO jsonTransformationDTO, Boolean listRemoteBranches) {
GitArtifactHelper<?> gitArtifactHelper =
gitArtifactHelperResolver.getArtifactHelper(jsonTransformationDTO.getArtifactType());

Path repoSuffix = gitArtifactHelper.getRepoSuffixPath(
artifactJsonTransformationDTO.getWorkspaceId(),
artifactJsonTransformationDTO.getBaseArtifactId(),
artifactJsonTransformationDTO.getRepoName());
jsonTransformationDTO.getWorkspaceId(),
jsonTransformationDTO.getBaseArtifactId(),
jsonTransformationDTO.getRepoName());

return fsGitHandler
.listBranches(repoSuffix)
.flatMapMany(Flux::fromIterable)
.filter(gitBranchDTO -> {
return StringUtils.hasText(gitBranchDTO.getBranchName())
&& !gitBranchDTO.getBranchName().startsWith("origin");
boolean branchToBeListed = !gitBranchDTO.getBranchName().startsWith("origin")
|| Boolean.TRUE.equals(listRemoteBranches);

return StringUtils.hasText(gitBranchDTO.getBranchName()) && branchToBeListed;
})
.map(GitBranchDTO::getBranchName)
.collectList();
}

@Override
public Mono<List<String>> listReferences(
ArtifactJsonTransformationDTO artifactJsonTransformationDTO,
Boolean checkRemoteReferences,
RefType refType) {
if (RefType.BRANCH.equals(refType)) {
listBranches(artifactJsonTransformationDTO, checkRemoteReferences);
}

// TODO: include ref type for tags in fsGit Handler
return Mono.just(List.of());
}

@Override
public Mono<Boolean> validateEmptyRepository(ArtifactJsonTransformationDTO artifactJsonTransformationDTO) {
GitArtifactHelper<?> gitArtifactHelper =
Expand Down Expand Up @@ -580,7 +605,8 @@ private Mono<String> pushArtifactErrorRecovery(String pushResult, Artifact artif
* @return : returns string for remote fetch
*/
@Override
public Mono<String> fetchRemoteChanges(ArtifactJsonTransformationDTO jsonTransformationDTO, GitAuth gitAuth) {
public Mono<String> fetchRemoteChanges(
ArtifactJsonTransformationDTO jsonTransformationDTO, GitAuth gitAuth, Boolean isFetchAll) {

String workspaceId = jsonTransformationDTO.getWorkspaceId();
String baseArtifactId = jsonTransformationDTO.getBaseArtifactId();
Expand All @@ -595,7 +621,7 @@ public Mono<String> fetchRemoteChanges(ArtifactJsonTransformationDTO jsonTransfo
Mono<Boolean> checkoutBranchMono = fsGitHandler.checkoutToBranch(repoSuffix, refName);

Mono<String> fetchRemoteMono = fsGitHandler.fetchRemote(
repoPath, gitAuth.getPublicKey(), gitAuth.getPrivateKey(), true, refName, false);
repoPath, gitAuth.getPublicKey(), gitAuth.getPrivateKey(), true, refName, isFetchAll);

return checkoutBranchMono.then(Mono.defer(() -> fetchRemoteMono));
}
Expand Down Expand Up @@ -633,4 +659,17 @@ public Mono<GitStatusDTO> getStatus(ArtifactJsonTransformationDTO jsonTransforma
Path repoPath = fsGitHandler.createRepoPath(repoSuffix);
return fsGitHandler.getStatus(repoPath, refName);
}

@Override
public Mono<String> prepareForNewRefCreation(ArtifactJsonTransformationDTO jsonTransformationDTO) {
GitArtifactHelper<?> gitArtifactHelper =
gitArtifactHelperResolver.getArtifactHelper(jsonTransformationDTO.getArtifactType());

Path repoSuffix = gitArtifactHelper.getRepoSuffixPath(
jsonTransformationDTO.getWorkspaceId(),
jsonTransformationDTO.getBaseArtifactId(),
jsonTransformationDTO.getRepoName());

return fsGitHandler.createAndCheckoutToBranch(repoSuffix, jsonTransformationDTO.getRefName());
}
}
Loading