chore: added autocommit and protected branches methods (#38370)
## Description - Implementation for autocommit and protected branches Fixes #37445, #37446 , #37447, #37457, #37458 ## Automation /ok-to-test tags="@tag.Git" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!IMPORTANT] > 🟣 🟣 🟣 Your tests are running. > Tests running at: <https://github.com/appsmithorg/appsmith/actions/runs/12502337299> > Commit: d443eebced369bd1d225fff0e622ba8a6c547225 > Workflow: `PR Automation test suite` > Tags: `@tag.Git` > Spec: `` > <hr>Thu, 26 Dec 2024 09:40:10 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced methods for managing protected branches and auto-commit features. - Added analytics functionality for tracking changes in protected branches. - **Bug Fixes** - Enhanced error handling for unsupported operations and invalid parameters in new methods. - **Refactor** - Updated dependency management in several service classes to include auto-commit functionality. - **Documentation** - Improved method documentation for clarity on parameters and behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
092445e9e4
commit
0255bce037
|
|
@ -7,9 +7,12 @@ import com.appsmith.git.dto.CommitDTO;
|
|||
import com.appsmith.server.constants.ArtifactType;
|
||||
import com.appsmith.server.domains.Artifact;
|
||||
import com.appsmith.server.dtos.ArtifactImportDTO;
|
||||
import com.appsmith.server.dtos.AutoCommitResponseDTO;
|
||||
import com.appsmith.server.dtos.GitConnectDTO;
|
||||
import reactor.core.publisher.Mono;
|
||||
|
||||
import java.util.List;
|
||||
|
||||
public interface CentralGitServiceCE {
|
||||
|
||||
Mono<? extends ArtifactImportDTO> importArtifactFromGit(
|
||||
|
|
@ -52,4 +55,14 @@ public interface CentralGitServiceCE {
|
|||
|
||||
Mono<? extends Artifact> deleteGitReference(
|
||||
String baseArtifactId, GitRefDTO gitRefDTO, ArtifactType artifactType, GitType gitType);
|
||||
|
||||
Mono<List<String>> updateProtectedBranches(
|
||||
String baseArtifactId, List<String> branchNames, ArtifactType artifactType);
|
||||
|
||||
Mono<List<String>> getProtectedBranches(String baseArtifactId, ArtifactType artifactType);
|
||||
|
||||
Mono<Boolean> toggleAutoCommitEnabled(String baseArtifactId, ArtifactType artifactType);
|
||||
|
||||
Mono<AutoCommitResponseDTO> getAutoCommitProgress(
|
||||
String baseArtifactId, String branchName, ArtifactType artifactType);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ package com.appsmith.server.git.central;
|
|||
import com.appsmith.server.datasources.base.DatasourceService;
|
||||
import com.appsmith.server.exports.internal.ExportService;
|
||||
import com.appsmith.server.git.GitRedisUtils;
|
||||
import com.appsmith.server.git.autocommit.helpers.GitAutoCommitHelper;
|
||||
import com.appsmith.server.git.resolver.GitArtifactHelperResolver;
|
||||
import com.appsmith.server.git.resolver.GitHandlingServiceResolver;
|
||||
import com.appsmith.server.git.utils.GitAnalyticsUtils;
|
||||
|
|
@ -17,6 +18,7 @@ import com.appsmith.server.solutions.DatasourcePermission;
|
|||
import io.micrometer.observation.ObservationRegistry;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.springframework.stereotype.Service;
|
||||
import org.springframework.transaction.reactive.TransactionalOperator;
|
||||
|
||||
@Slf4j
|
||||
@Service
|
||||
|
|
@ -24,6 +26,7 @@ public class CentralGitServiceCECompatibleImpl extends CentralGitServiceCEImpl
|
|||
implements CentralGitServiceCECompatible {
|
||||
|
||||
public CentralGitServiceCECompatibleImpl(
|
||||
GitRedisUtils gitRedisUtils,
|
||||
GitProfileUtils gitProfileUtils,
|
||||
GitAnalyticsUtils gitAnalyticsUtils,
|
||||
UserDataService userDataService,
|
||||
|
|
@ -37,9 +40,11 @@ public class CentralGitServiceCECompatibleImpl extends CentralGitServiceCEImpl
|
|||
PluginService pluginService,
|
||||
ImportService importService,
|
||||
ExportService exportService,
|
||||
GitRedisUtils gitRedisUtils,
|
||||
GitAutoCommitHelper gitAutoCommitHelper,
|
||||
TransactionalOperator transactionalOperator,
|
||||
ObservationRegistry observationRegistry) {
|
||||
super(
|
||||
gitRedisUtils,
|
||||
gitProfileUtils,
|
||||
gitAnalyticsUtils,
|
||||
userDataService,
|
||||
|
|
@ -53,7 +58,8 @@ public class CentralGitServiceCECompatibleImpl extends CentralGitServiceCEImpl
|
|||
pluginService,
|
||||
importService,
|
||||
exportService,
|
||||
gitRedisUtils,
|
||||
gitAutoCommitHelper,
|
||||
transactionalOperator,
|
||||
observationRegistry);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,6 +16,7 @@ import com.appsmith.server.constants.FieldName;
|
|||
import com.appsmith.server.constants.GitDefaultCommitMessage;
|
||||
import com.appsmith.server.datasources.base.DatasourceService;
|
||||
import com.appsmith.server.domains.Artifact;
|
||||
import com.appsmith.server.domains.AutoCommitConfig;
|
||||
import com.appsmith.server.domains.GitArtifactMetadata;
|
||||
import com.appsmith.server.domains.GitAuth;
|
||||
import com.appsmith.server.domains.GitProfile;
|
||||
|
|
@ -25,11 +26,13 @@ import com.appsmith.server.domains.UserData;
|
|||
import com.appsmith.server.domains.Workspace;
|
||||
import com.appsmith.server.dtos.ArtifactExchangeJson;
|
||||
import com.appsmith.server.dtos.ArtifactImportDTO;
|
||||
import com.appsmith.server.dtos.AutoCommitResponseDTO;
|
||||
import com.appsmith.server.dtos.GitConnectDTO;
|
||||
import com.appsmith.server.exceptions.AppsmithError;
|
||||
import com.appsmith.server.exceptions.AppsmithException;
|
||||
import com.appsmith.server.exports.internal.ExportService;
|
||||
import com.appsmith.server.git.GitRedisUtils;
|
||||
import com.appsmith.server.git.autocommit.helpers.GitAutoCommitHelper;
|
||||
import com.appsmith.server.git.dtos.ArtifactJsonTransformationDTO;
|
||||
import com.appsmith.server.git.resolver.GitArtifactHelperResolver;
|
||||
import com.appsmith.server.git.resolver.GitHandlingServiceResolver;
|
||||
|
|
@ -50,6 +53,7 @@ import org.eclipse.jgit.api.errors.InvalidRemoteException;
|
|||
import org.eclipse.jgit.api.errors.TransportException;
|
||||
import org.eclipse.jgit.lib.BranchTrackingStatus;
|
||||
import org.springframework.stereotype.Service;
|
||||
import org.springframework.transaction.reactive.TransactionalOperator;
|
||||
import org.springframework.util.CollectionUtils;
|
||||
import org.springframework.util.StringUtils;
|
||||
import reactor.core.observability.micrometer.Micrometer;
|
||||
|
|
@ -60,6 +64,7 @@ import reactor.util.function.Tuple3;
|
|||
|
||||
import java.io.IOException;
|
||||
import java.time.Instant;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.Set;
|
||||
|
|
@ -85,6 +90,7 @@ import static org.springframework.util.StringUtils.hasText;
|
|||
@RequiredArgsConstructor
|
||||
public class CentralGitServiceCEImpl implements CentralGitServiceCE {
|
||||
|
||||
private final GitRedisUtils gitRedisUtils;
|
||||
private final GitProfileUtils gitProfileUtils;
|
||||
private final GitAnalyticsUtils gitAnalyticsUtils;
|
||||
private final UserDataService userDataService;
|
||||
|
|
@ -104,7 +110,8 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE {
|
|||
private final ImportService importService;
|
||||
private final ExportService exportService;
|
||||
|
||||
private final GitRedisUtils gitRedisUtils;
|
||||
private final GitAutoCommitHelper gitAutoCommitHelper;
|
||||
private final TransactionalOperator transactionalOperator;
|
||||
private final ObservationRegistry observationRegistry;
|
||||
|
||||
protected static final String ORIGIN = "origin/";
|
||||
|
|
@ -1765,4 +1772,127 @@ public class CentralGitServiceCEImpl implements CentralGitServiceCE {
|
|||
return Mono.create(sink ->
|
||||
recreatedArtifactFromLastCommit.subscribe(sink::success, sink::error, null, sink.currentContext()));
|
||||
}
|
||||
|
||||
@Override
|
||||
public Mono<List<String>> updateProtectedBranches(
|
||||
String baseArtifactId, List<String> branchNames, ArtifactType artifactType) {
|
||||
|
||||
GitArtifactHelper<?> gitArtifactHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType);
|
||||
AclPermission artifactManageProtectedBranchPermission =
|
||||
gitArtifactHelper.getArtifactManageProtectedBranchPermission();
|
||||
|
||||
Mono<? extends Artifact> baseArtifactMono =
|
||||
gitArtifactHelper.getArtifactById(baseArtifactId, artifactManageProtectedBranchPermission);
|
||||
|
||||
return baseArtifactMono
|
||||
.flatMap(baseArtifact -> {
|
||||
GitArtifactMetadata baseGitData = baseArtifact.getGitArtifactMetadata();
|
||||
final String defaultBranchName = baseGitData.getDefaultBranchName();
|
||||
final List<String> incomingProtectedBranches =
|
||||
CollectionUtils.isEmpty(branchNames) ? new ArrayList<>() : branchNames;
|
||||
|
||||
// user cannot protect multiple branches
|
||||
if (incomingProtectedBranches.size() > 1) {
|
||||
return Mono.error(new AppsmithException(AppsmithError.UNSUPPORTED_OPERATION));
|
||||
}
|
||||
|
||||
// user cannot protect a branch which is not default
|
||||
if (incomingProtectedBranches.size() == 1
|
||||
&& !defaultBranchName.equals(incomingProtectedBranches.get(0))) {
|
||||
return Mono.error(new AppsmithException(AppsmithError.UNSUPPORTED_OPERATION));
|
||||
}
|
||||
|
||||
return updateProtectedBranchesInArtifactAfterVerification(baseArtifact, incomingProtectedBranches);
|
||||
})
|
||||
.as(transactionalOperator::transactional);
|
||||
}
|
||||
|
||||
protected Mono<List<String>> updateProtectedBranchesInArtifactAfterVerification(
|
||||
Artifact baseArtifact, List<String> branchNames) {
|
||||
GitArtifactHelper<?> gitArtifactHelper =
|
||||
gitArtifactHelperResolver.getArtifactHelper(baseArtifact.getArtifactType());
|
||||
GitArtifactMetadata baseGitData = baseArtifact.getGitArtifactMetadata();
|
||||
|
||||
// keep a copy of old protected branches as it's required to send analytics event later
|
||||
List<String> oldProtectedBranches = baseGitData.getBranchProtectionRules() == null
|
||||
? new ArrayList<>()
|
||||
: baseGitData.getBranchProtectionRules();
|
||||
|
||||
baseGitData.setBranchProtectionRules(branchNames);
|
||||
return gitArtifactHelper
|
||||
.saveArtifact(baseArtifact)
|
||||
.then(gitArtifactHelper.updateArtifactWithProtectedBranches(baseArtifact.getId(), branchNames))
|
||||
.then(gitAnalyticsUtils.sendBranchProtectionAnalytics(baseArtifact, oldProtectedBranches, branchNames))
|
||||
.thenReturn(branchNames);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Mono<List<String>> getProtectedBranches(String baseArtifactId, ArtifactType artifactType) {
|
||||
|
||||
GitArtifactHelper<?> gitArtifactHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType);
|
||||
AclPermission artifactEditPermission = gitArtifactHelper.getArtifactEditPermission();
|
||||
|
||||
Mono<? extends Artifact> baseArtifactMono =
|
||||
gitArtifactHelper.getArtifactById(baseArtifactId, artifactEditPermission);
|
||||
|
||||
return baseArtifactMono.map(defaultArtifact -> {
|
||||
GitArtifactMetadata gitArtifactMetadata = defaultArtifact.getGitArtifactMetadata();
|
||||
/*
|
||||
user may have multiple branches as protected, but we only return the default branch
|
||||
as protected branch if it's present in the list of protected branches
|
||||
*/
|
||||
List<String> protectedBranches = gitArtifactMetadata.getBranchProtectionRules();
|
||||
String defaultBranchName = gitArtifactMetadata.getDefaultBranchName();
|
||||
|
||||
if (CollectionUtils.isEmpty(protectedBranches) || !protectedBranches.contains(defaultBranchName)) {
|
||||
return List.of();
|
||||
}
|
||||
|
||||
return List.of(defaultBranchName);
|
||||
});
|
||||
}
|
||||
|
||||
@Override
|
||||
public Mono<Boolean> toggleAutoCommitEnabled(String baseArtifactId, ArtifactType artifactType) {
|
||||
|
||||
GitArtifactHelper<?> gitArtifactHelper = gitArtifactHelperResolver.getArtifactHelper(artifactType);
|
||||
AclPermission artifactAutoCommitPermission = gitArtifactHelper.getArtifactAutoCommitPermission();
|
||||
|
||||
Mono<? extends Artifact> baseArtifactMono =
|
||||
gitArtifactHelper.getArtifactById(baseArtifactId, artifactAutoCommitPermission);
|
||||
// get base app
|
||||
|
||||
return baseArtifactMono
|
||||
.map(baseArtifact -> {
|
||||
GitArtifactMetadata baseGitMetadata = baseArtifact.getGitArtifactMetadata();
|
||||
if (!baseArtifact.getId().equals(baseGitMetadata.getDefaultArtifactId())) {
|
||||
log.error(
|
||||
"failed to toggle auto commit. reason: {} is not the id of the base Artifact",
|
||||
baseArtifactId);
|
||||
throw new AppsmithException(AppsmithError.INVALID_PARAMETER, "default baseArtifact id");
|
||||
}
|
||||
|
||||
AutoCommitConfig autoCommitConfig = baseGitMetadata.getAutoCommitConfig();
|
||||
if (autoCommitConfig.getEnabled()) {
|
||||
autoCommitConfig.setEnabled(FALSE);
|
||||
} else {
|
||||
autoCommitConfig.setEnabled(TRUE);
|
||||
}
|
||||
// need to call the setter because getter returns a default config if attribute is null
|
||||
baseArtifact.getGitArtifactMetadata().setAutoCommitConfig(autoCommitConfig);
|
||||
return baseArtifact;
|
||||
})
|
||||
.flatMap(baseArtifact -> gitArtifactHelper
|
||||
.saveArtifact(baseArtifact)
|
||||
.thenReturn(baseArtifact
|
||||
.getGitArtifactMetadata()
|
||||
.getAutoCommitConfig()
|
||||
.getEnabled()));
|
||||
}
|
||||
|
||||
@Override
|
||||
public Mono<AutoCommitResponseDTO> getAutoCommitProgress(
|
||||
String artifactId, String branchName, ArtifactType artifactType) {
|
||||
return gitAutoCommitHelper.getAutoCommitProgress(artifactId, branchName);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ package com.appsmith.server.git.central;
|
|||
import com.appsmith.server.datasources.base.DatasourceService;
|
||||
import com.appsmith.server.exports.internal.ExportService;
|
||||
import com.appsmith.server.git.GitRedisUtils;
|
||||
import com.appsmith.server.git.autocommit.helpers.GitAutoCommitHelper;
|
||||
import com.appsmith.server.git.resolver.GitArtifactHelperResolver;
|
||||
import com.appsmith.server.git.resolver.GitHandlingServiceResolver;
|
||||
import com.appsmith.server.git.utils.GitAnalyticsUtils;
|
||||
|
|
@ -17,12 +18,14 @@ import com.appsmith.server.solutions.DatasourcePermission;
|
|||
import io.micrometer.observation.ObservationRegistry;
|
||||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.springframework.stereotype.Service;
|
||||
import org.springframework.transaction.reactive.TransactionalOperator;
|
||||
|
||||
@Slf4j
|
||||
@Service
|
||||
public class CentralGitServiceImpl extends CentralGitServiceCECompatibleImpl implements CentralGitService {
|
||||
|
||||
public CentralGitServiceImpl(
|
||||
GitRedisUtils gitRedisUtils,
|
||||
GitProfileUtils gitProfileUtils,
|
||||
GitAnalyticsUtils gitAnalyticsUtils,
|
||||
UserDataService userDataService,
|
||||
|
|
@ -36,9 +39,11 @@ public class CentralGitServiceImpl extends CentralGitServiceCECompatibleImpl imp
|
|||
PluginService pluginService,
|
||||
ImportService importService,
|
||||
ExportService exportService,
|
||||
GitRedisUtils gitRedisUtils,
|
||||
GitAutoCommitHelper gitAutoCommitHelper,
|
||||
TransactionalOperator transactionalOperator,
|
||||
ObservationRegistry observationRegistry) {
|
||||
super(
|
||||
gitRedisUtils,
|
||||
gitProfileUtils,
|
||||
gitAnalyticsUtils,
|
||||
userDataService,
|
||||
|
|
@ -52,7 +57,8 @@ public class CentralGitServiceImpl extends CentralGitServiceCECompatibleImpl imp
|
|||
pluginService,
|
||||
importService,
|
||||
exportService,
|
||||
gitRedisUtils,
|
||||
gitAutoCommitHelper,
|
||||
transactionalOperator,
|
||||
observationRegistry);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,11 +13,16 @@ import lombok.RequiredArgsConstructor;
|
|||
import lombok.extern.slf4j.Slf4j;
|
||||
import org.springframework.stereotype.Component;
|
||||
import org.springframework.util.StringUtils;
|
||||
import reactor.core.publisher.Flux;
|
||||
import reactor.core.publisher.Mono;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
|
||||
import static com.appsmith.external.constants.AnalyticsEvents.GIT_ADD_PROTECTED_BRANCH;
|
||||
import static com.appsmith.external.constants.AnalyticsEvents.GIT_REMOVE_PROTECTED_BRANCH;
|
||||
import static org.apache.commons.lang.ObjectUtils.defaultIfNull;
|
||||
|
||||
@Slf4j
|
||||
|
|
@ -146,4 +151,40 @@ public class GitAnalyticsUtils {
|
|||
return analyticsService.sendEvent(
|
||||
AnalyticsEvents.UNIT_EXECUTION_TIME.getEventName(), currentUser.getUsername(), data);
|
||||
}
|
||||
|
||||
/**
|
||||
* Sends one or more analytics events when there's a change in protected branches.
|
||||
* If n number of branches are un-protected and m number of branches are protected, it'll send m+n number of
|
||||
* events. It receives the list of branches before and after the action.
|
||||
* For example, if user has "main" and "develop" branches as protected and wants to include "staging" branch as
|
||||
* protected as well, then oldProtectedBranches will be ["main", "develop"] and newProtectedBranches will be
|
||||
* ["main", "develop", "staging"]
|
||||
*
|
||||
* @param artifact Application object of the root artifact
|
||||
* @param oldProtectedBranches List of branches that were protected before this action.
|
||||
* @param newProtectedBranches List of branches that are going to be protected.
|
||||
* @return An empty Mono
|
||||
*/
|
||||
public Mono<Void> sendBranchProtectionAnalytics(
|
||||
Artifact artifact, List<String> oldProtectedBranches, List<String> newProtectedBranches) {
|
||||
List<String> itemsAdded = new ArrayList<>(newProtectedBranches); // add all new items
|
||||
itemsAdded.removeAll(oldProtectedBranches); // remove the items that were present earlier
|
||||
|
||||
List<String> itemsRemoved = new ArrayList<>(oldProtectedBranches); // add all old items
|
||||
itemsRemoved.removeAll(newProtectedBranches); // remove the items that are also present in new list
|
||||
|
||||
List<Mono<? extends Artifact>> eventSenderMonos = new ArrayList<>();
|
||||
|
||||
// send an analytics event for each removed branch
|
||||
for (String branchName : itemsRemoved) {
|
||||
eventSenderMonos.add(addAnalyticsForGitOperation(GIT_REMOVE_PROTECTED_BRANCH, branchName, artifact));
|
||||
}
|
||||
|
||||
// send an analytics event for each newly protected branch
|
||||
for (String branchName : itemsAdded) {
|
||||
eventSenderMonos.add(addAnalyticsForGitOperation(GIT_ADD_PROTECTED_BRANCH, branchName, artifact));
|
||||
}
|
||||
|
||||
return Flux.merge(eventSenderMonos).then();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user