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.

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: added autocommit and protected branches methods #38370

Merged
merged 4 commits into from
Dec 27, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
Expand Up @@ -7,9 +7,12 @@
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(
Expand Down Expand Up @@ -52,4 +55,14 @@ Mono<? extends Artifact> createReference(

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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
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;
Expand All @@ -17,13 +18,15 @@
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 CentralGitServiceCECompatibleImpl extends CentralGitServiceCEImpl
implements CentralGitServiceCECompatible {

public CentralGitServiceCECompatibleImpl(
GitRedisUtils gitRedisUtils,
GitProfileUtils gitProfileUtils,
GitAnalyticsUtils gitAnalyticsUtils,
UserDataService userDataService,
Expand All @@ -37,9 +40,11 @@ public CentralGitServiceCECompatibleImpl(
PluginService pluginService,
ImportService importService,
ExportService exportService,
GitRedisUtils gitRedisUtils,
GitAutoCommitHelper gitAutoCommitHelper,
TransactionalOperator transactionalOperator,
ObservationRegistry observationRegistry) {
super(
gitRedisUtils,
gitProfileUtils,
gitAnalyticsUtils,
userDataService,
Expand All @@ -53,7 +58,8 @@ public CentralGitServiceCECompatibleImpl(
pluginService,
importService,
exportService,
gitRedisUtils,
gitAutoCommitHelper,
transactionalOperator,
observationRegistry);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
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;
Expand All @@ -25,11 +26,13 @@
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;
Expand All @@ -50,6 +53,7 @@
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;
Expand All @@ -60,6 +64,7 @@

import java.io.IOException;
import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand All @@ -85,6 +90,7 @@
@RequiredArgsConstructor
public class CentralGitServiceCEImpl implements CentralGitServiceCE {

private final GitRedisUtils gitRedisUtils;
private final GitProfileUtils gitProfileUtils;
private final GitAnalyticsUtils gitAnalyticsUtils;
private final UserDataService userDataService;
Expand All @@ -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/";
Expand Down Expand Up @@ -1765,4 +1772,127 @@ protected Mono<? extends Artifact> discardChanges(Artifact branchedArtifact, Git
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) {
Copy link
Contributor

Choose a reason for hiding this comment

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

QQ: Since user cannot protect multiple branches what's the reason behind being it a list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we select the list from the UI, we are essentially sending multiple protected branches, hence this is take as a list. This api request is stateless, what this essentially means is that your request is the absolute value which is going to be saved.

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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
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;
Expand All @@ -17,12 +18,14 @@
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,
Expand All @@ -36,9 +39,11 @@ public CentralGitServiceImpl(
PluginService pluginService,
ImportService importService,
ExportService exportService,
GitRedisUtils gitRedisUtils,
GitAutoCommitHelper gitAutoCommitHelper,
TransactionalOperator transactionalOperator,
ObservationRegistry observationRegistry) {
super(
gitRedisUtils,
gitProfileUtils,
gitAnalyticsUtils,
userDataService,
Expand All @@ -52,7 +57,8 @@ public CentralGitServiceImpl(
pluginService,
importService,
exportService,
gitRedisUtils,
gitAutoCommitHelper,
transactionalOperator,
observationRegistry);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,16 @@
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
Expand Down Expand Up @@ -146,4 +151,40 @@ public Mono<Void> sendUnitExecutionTimeAnalyticsEvent(
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