Skip to content
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package com.appsmith.server.git;

import com.appsmith.server.exceptions.AppsmithError;
import com.appsmith.server.exceptions.AppsmithException;
import com.appsmith.server.helpers.RedisUtils;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;
import reactor.util.retry.Retry;

import static com.appsmith.server.helpers.GitUtils.MAX_RETRIES;
import static com.appsmith.server.helpers.GitUtils.RETRY_DELAY;

@Component
@RequiredArgsConstructor
public class GitRedisUtils {

private final RedisUtils redisUtils;

public Mono<Boolean> addFileLock(String defaultApplicationId) {
return redisUtils
.addFileLock(defaultApplicationId)
.retryWhen(Retry.fixedDelay(MAX_RETRIES, RETRY_DELAY)
.onRetryExhaustedThrow((retryBackoffSpec, retrySignal) -> {
throw new AppsmithException(AppsmithError.GIT_FILE_IN_USE);
}));
}

public Mono<Boolean> addFileLockWithoutRetry(String defaultApplicationId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Approving the PR due to urgency, but please refactor this call to make retry a parameter instead of a separate method.

return redisUtils.addFileLock(defaultApplicationId);
}

public Mono<Boolean> releaseFileLock(String defaultApplicationId) {
return redisUtils.releaseFileLock(defaultApplicationId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -312,18 +312,18 @@ public Mono<Map<String, Integer>> reconstructMetadataFromRepo(
/**
* Provides the server schema version in the application json for the given branch
*
* @param workspaceId : workspaceId of the artifact
* @param workspaceId : workspaceId of the artifact
* @param defaultArtifactId : default branch id of the artifact
* @param branchName : current branch name of the artifact
* @param repoName : repository name
* @param artifactType : artifact type of this operation
* @param repoName : repository name
* @param branchName : current branch name of the artifact
* @param artifactType : artifact type of this operation
* @return the server schema migration version number
*/
public Mono<Integer> getMetadataServerSchemaMigrationVersion(
String workspaceId,
String defaultArtifactId,
String branchName,
String repoName,
String branchName,
ArtifactType artifactType) {

if (!hasText(workspaceId)) {
Expand All @@ -342,7 +342,7 @@ public Mono<Integer> getMetadataServerSchemaMigrationVersion(
return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.REPO_NAME));
}

return reconstructMetadataFromRepo(workspaceId, defaultArtifactId, branchName, repoName, artifactType)
return reconstructMetadataFromRepo(workspaceId, defaultArtifactId, repoName, branchName, artifactType)
.map(metadataMap -> {
return metadataMap.getOrDefault(
CommonConstants.SERVER_SCHEMA_VERSION, JsonSchemaVersions.serverVersion);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import com.appsmith.server.dtos.PageDTO;
import reactor.core.publisher.Mono;

public interface AutoCommitEligibiltyHelper {
public interface AutoCommitEligibilityHelper {

Mono<Boolean> isServerAutoCommitRequired(String workspaceId, GitArtifactMetadata gitMetadata);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package com.appsmith.server.helpers.ce.autocommit;

import com.appsmith.server.domains.GitArtifactMetadata;
import com.appsmith.server.dtos.PageDTO;
import lombok.extern.slf4j.Slf4j;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;

import static java.lang.Boolean.FALSE;

@Slf4j
@Component
public class AutoCommitEligibilityHelperFallbackImpl implements AutoCommitEligibilityHelper {

@Override
public Mono<Boolean> isServerAutoCommitRequired(String workspaceId, GitArtifactMetadata gitMetadata) {
return Mono.just(FALSE);
}

@Override
public Mono<Boolean> isClientMigrationRequired(PageDTO pageDTO) {
return Mono.just(FALSE);
}

@Override
public Mono<AutoCommitTriggerDTO> isAutoCommitRequired(
String workspaceId, GitArtifactMetadata gitArtifactMetadata, PageDTO pageDTO) {
return Mono.just(new AutoCommitTriggerDTO(FALSE, FALSE, FALSE));
}
}
Original file line number Diff line number Diff line change
@@ -1,57 +1,75 @@
package com.appsmith.server.helpers.ce.autocommit;

import com.appsmith.external.annotations.FeatureFlagged;
import com.appsmith.external.enums.FeatureFlagEnum;
import com.appsmith.server.constants.ArtifactType;
import com.appsmith.server.domains.GitArtifactMetadata;
import com.appsmith.server.domains.Layout;
import com.appsmith.server.dtos.PageDTO;
import com.appsmith.server.git.GitRedisUtils;
import com.appsmith.server.helpers.CommonGitFileUtils;
import com.appsmith.server.helpers.DSLMigrationUtils;
import com.appsmith.server.helpers.GitUtils;
import com.appsmith.server.migrations.JsonSchemaVersions;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import net.minidev.json.JSONObject;
import org.springframework.context.annotation.Primary;
import org.springframework.stereotype.Component;
import reactor.core.publisher.Mono;

import static java.lang.Boolean.FALSE;
import static java.lang.Boolean.TRUE;

@Slf4j
@Primary
@Component
@RequiredArgsConstructor
public class AutoCommitEligibilityHelperImpl implements AutoCommitEligibiltyHelper {
public class AutoCommitEligibilityHelperImpl extends AutoCommitEligibilityHelperFallbackImpl
implements AutoCommitEligibilityHelper {

private final CommonGitFileUtils commonGitFileUtils;
private final DSLMigrationUtils dslMigrationUtils;
private final GitRedisUtils gitRedisUtils;

@Override
@FeatureFlagged(featureFlagName = FeatureFlagEnum.release_git_server_autocommit_feature_enabled)
public Mono<Boolean> isServerAutoCommitRequired(String workspaceId, GitArtifactMetadata gitMetadata) {

String defaultApplicationId = gitMetadata.getDefaultArtifactId();
String branchName = gitMetadata.getBranchName();
String repoName = gitMetadata.getRepoName();

return commonGitFileUtils
Mono<Boolean> isServerMigrationRequiredMonoCached = commonGitFileUtils
.getMetadataServerSchemaMigrationVersion(
workspaceId, defaultApplicationId, branchName, repoName, ArtifactType.APPLICATION)
workspaceId, defaultApplicationId, repoName, branchName, ArtifactType.APPLICATION)
.map(serverSchemaVersion -> {
if (JsonSchemaVersions.serverVersion > serverSchemaVersion) {
return TRUE;
}

return FALSE;
log.info(
"server schema for application id : {} and branch name : {} is : {}",
defaultApplicationId,
branchName,
serverSchemaVersion);
return JsonSchemaVersions.serverVersion > serverSchemaVersion ? TRUE : FALSE;
})
.defaultIfEmpty(FALSE)
.cache();

return Mono.defer(() -> gitRedisUtils.addFileLockWithoutRetry(defaultApplicationId))
.then(Mono.defer(() -> isServerMigrationRequiredMonoCached))
.then(Mono.defer(() -> gitRedisUtils.releaseFileLock(defaultApplicationId)))
.then(Mono.defer(() -> isServerMigrationRequiredMonoCached))
.onErrorResume(error -> {
log.debug(
"error while retrieving the metadata for defaultApplicationId : {}, branchName : {}",
"error while retrieving the metadata for defaultApplicationId : {}, branchName : {} error : {}",
defaultApplicationId,
branchName);
return Mono.just(FALSE);
branchName,
error.getMessage());
return gitRedisUtils.releaseFileLock(defaultApplicationId).then(Mono.just(FALSE));
});
}

@Override
@FeatureFlagged(featureFlagName = FeatureFlagEnum.release_git_autocommit_feature_enabled)
public Mono<Boolean> isClientMigrationRequired(PageDTO pageDTO) {
return dslMigrationUtils
.getLatestDslVersion()
Expand All @@ -63,6 +81,7 @@ public Mono<Boolean> isClientMigrationRequired(PageDTO pageDTO) {
JSONObject layoutDsl = layout.getDsl();
return GitUtils.isMigrationRequired(layoutDsl, latestDslVersion);
})
.defaultIfEmpty(FALSE)
.onErrorResume(error -> {
log.debug("Error fetching latest DSL version");
return Mono.just(Boolean.FALSE);
Expand All @@ -72,21 +91,11 @@ public Mono<Boolean> isClientMigrationRequired(PageDTO pageDTO) {
@Override
public Mono<AutoCommitTriggerDTO> isAutoCommitRequired(
String workspaceId, GitArtifactMetadata gitArtifactMetadata, PageDTO pageDTO) {
String defaultApplicationId = gitArtifactMetadata.getDefaultArtifactId();
String branchName = gitArtifactMetadata.getBranchName();

Mono<Boolean> isClientAutocommitRequiredMono =
isClientMigrationRequired(pageDTO).defaultIfEmpty(FALSE);

Mono<Boolean> isServerAutocommitRequiredMono = isServerAutoCommitRequired(workspaceId, gitArtifactMetadata)
.defaultIfEmpty(FALSE)
.onErrorResume(error -> {
log.debug(
"Error in checking server migration for application id : {} branch name : {}",
defaultApplicationId,
branchName);
return Mono.just(Boolean.FALSE);
});
Mono<Boolean> isServerAutocommitRequiredMono = isServerAutoCommitRequired(workspaceId, gitArtifactMetadata);

return isServerAutocommitRequiredMono
.zipWith(isClientAutocommitRequiredMono)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package com.appsmith.server.helpers.ce.autocommit;

import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.RequiredArgsConstructor;

@Data
@AllArgsConstructor
@RequiredArgsConstructor
public class AutoCommitTriggerDTO {

private Boolean isAutoCommitRequired;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import com.appsmith.server.helpers.DSLMigrationUtils;
import com.appsmith.server.helpers.GitFileUtils;
import com.appsmith.server.helpers.ResponseUtils;
import com.appsmith.server.helpers.ce.autocommit.AutoCommitEligibiltyHelper;
import com.appsmith.server.helpers.ce.autocommit.AutoCommitEligibilityHelper;
import com.appsmith.server.helpers.ce.autocommit.GitAutoCommitHelper;
import com.appsmith.server.layouts.UpdateLayoutService;
import com.appsmith.server.newactions.base.NewActionService;
Expand Down Expand Up @@ -64,7 +64,7 @@ public ApplicationPageServiceImpl(
DatasourcePermission datasourcePermission,
DSLMigrationUtils dslMigrationUtils,
GitAutoCommitHelper gitAutoCommitHelper,
AutoCommitEligibiltyHelper autoCommitEligibiltyHelper,
AutoCommitEligibilityHelper autoCommitEligibilityHelper,
ClonePageService<NewAction> actionClonePageService,
ClonePageService<ActionCollection> actionCollectionClonePageService) {
super(
Expand Down Expand Up @@ -96,7 +96,7 @@ public ApplicationPageServiceImpl(
datasourcePermission,
dslMigrationUtils,
gitAutoCommitHelper,
autoCommitEligibiltyHelper,
autoCommitEligibilityHelper,
actionClonePageService,
actionCollectionClonePageService);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
import com.appsmith.server.helpers.GitUtils;
import com.appsmith.server.helpers.ResponseUtils;
import com.appsmith.server.helpers.UserPermissionUtils;
import com.appsmith.server.helpers.ce.autocommit.AutoCommitEligibiltyHelper;
import com.appsmith.server.helpers.ce.autocommit.AutoCommitEligibilityHelper;
import com.appsmith.server.helpers.ce.autocommit.GitAutoCommitHelper;
import com.appsmith.server.layouts.UpdateLayoutService;
import com.appsmith.server.migrations.ApplicationVersion;
Expand Down Expand Up @@ -128,7 +128,7 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE {
private final DatasourcePermission datasourcePermission;
private final DSLMigrationUtils dslMigrationUtils;
private final GitAutoCommitHelper gitAutoCommitHelper;
private final AutoCommitEligibiltyHelper autoCommitEligibiltyHelper;
private final AutoCommitEligibilityHelper autoCommitEligibilityHelper;
private final ClonePageService<NewAction> actionClonePageService;
private final ClonePageService<ActionCollection> actionCollectionClonePageService;

Expand Down Expand Up @@ -358,7 +358,7 @@ private Mono<Boolean> migrateSchemasForGitConnectedApps(Application application,
Mono<PageDTO> pageDTOMono = getPage(newPages.get(0), false);

return pageDTOMono.flatMap(pageDTO -> {
return autoCommitEligibiltyHelper
return autoCommitEligibilityHelper
.isAutoCommitRequired(workspaceId, gitMetadata, pageDTO)
.flatMap(autoCommitTriggerDTO -> {
if (Boolean.TRUE.equals(autoCommitTriggerDTO.getIsAutoCommitRequired())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
import com.appsmith.server.dtos.PageDTO;
import com.appsmith.server.helpers.DSLMigrationUtils;
import com.appsmith.server.helpers.GitPrivateRepoHelper;
import com.appsmith.server.helpers.ce.autocommit.AutoCommitEligibiltyHelper;
import com.appsmith.server.helpers.ce.autocommit.AutoCommitEligibilityHelper;
import com.appsmith.server.helpers.ce.autocommit.AutoCommitTriggerDTO;
import com.appsmith.server.migrations.JsonSchemaMigration;
import com.appsmith.server.migrations.JsonSchemaVersions;
import com.appsmith.server.newpages.base.NewPageService;
Expand Down Expand Up @@ -96,7 +97,7 @@ public class ApplicationPageServiceAutoCommitTest {
GitPrivateRepoHelper gitPrivateRepoHelper;

@SpyBean
AutoCommitEligibiltyHelper autoCommitEligibiltyHelper;
AutoCommitEligibilityHelper autoCommitEligibilityHelper;

@MockBean
BranchTrackingStatus branchTrackingStatus;
Expand Down Expand Up @@ -228,16 +229,15 @@ public void afterTest() {
}

@Test
public void testAutoCommit_WhenOnlyServerIsEligibleForMigration_CommitSuccess()
public void testAutoCommit_whenOnlyServerIsEligibleForMigration_commitSuccess()
throws URISyntaxException, IOException, GitAPIException {

ApplicationJson applicationJson =
gitFileSystemTestHelper.getApplicationJson(this.getClass().getResource(APP_JSON_NAME));

// server migration as true
doReturn(Mono.just(TRUE)).when(autoCommitEligibiltyHelper).isServerAutoCommitRequired(any(), any());
// client migration as false
doReturn(Mono.just(FALSE)).when(autoCommitEligibiltyHelper).isClientMigrationRequired(any());
doReturn(Mono.just(new AutoCommitTriggerDTO(TRUE, FALSE, TRUE)))
.when(autoCommitEligibilityHelper)
.isAutoCommitRequired(anyString(), any(GitArtifactMetadata.class), any(PageDTO.class));

ApplicationJson applicationJson1 = new ApplicationJson();
AppsmithBeanUtils.copyNewFieldValuesIntoOldObject(applicationJson, applicationJson1);
Expand Down Expand Up @@ -282,7 +282,7 @@ public void testAutoCommit_WhenOnlyServerIsEligibleForMigration_CommitSuccess()
}

@Test
public void testAutoCommit_WhenOnlyClientIsEligibleForMigration_CommitSuccess()
public void testAutoCommit_whenOnlyClientIsEligibleForMigration_commitSuccess()
throws GitAPIException, IOException, URISyntaxException {
ApplicationJson applicationJson =
gitFileSystemTestHelper.getApplicationJson(this.getClass().getResource(APP_JSON_NAME));
Expand All @@ -297,8 +297,10 @@ public void testAutoCommit_WhenOnlyClientIsEligibleForMigration_CommitSuccess()
.getAsNumber("version")
.intValue();

// server migration as false
doReturn(Mono.just(FALSE)).when(autoCommitEligibiltyHelper).isServerAutoCommitRequired(any(), any());
doReturn(Mono.just(new AutoCommitTriggerDTO(TRUE, TRUE, FALSE)))
.when(autoCommitEligibilityHelper)
.isAutoCommitRequired(anyString(), any(GitArtifactMetadata.class), any(PageDTO.class));

Mockito.when(dslMigrationUtils.getLatestDslVersion()).thenReturn(Mono.just(pageDSLNumber + 1));

JSONObject dslAfterMigration = new JSONObject();
Expand Down Expand Up @@ -342,16 +344,15 @@ public void testAutoCommit_WhenOnlyClientIsEligibleForMigration_CommitSuccess()
}

@Test
public void testAutoCommit_WhenAutoCommitNotEligible_ReturnsFalse()
public void testAutoCommit_whenAutoCommitNotEligible_returnsFalse()
throws URISyntaxException, IOException, GitAPIException {

ApplicationJson applicationJson =
gitFileSystemTestHelper.getApplicationJson(this.getClass().getResource(APP_JSON_NAME));

// server migration as false
doReturn(Mono.just(FALSE)).when(autoCommitEligibiltyHelper).isServerAutoCommitRequired(any(), any());
// client migration as false
doReturn(Mono.just(FALSE)).when(autoCommitEligibiltyHelper).isClientMigrationRequired(any());
doReturn(Mono.just(new AutoCommitTriggerDTO(FALSE, FALSE, FALSE)))
.when(autoCommitEligibilityHelper)
.isAutoCommitRequired(anyString(), any(GitArtifactMetadata.class), any(PageDTO.class));

gitFileSystemTestHelper.setupGitRepository(
WORKSPACE_ID, DEFAULT_APP_ID, BRANCH_NAME, REPO_NAME, applicationJson);
Expand Down
Loading