-
Notifications
You must be signed in to change notification settings - Fork 4.5k
chore: package refactors and left over changes for git autocommit pr #33768
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
Changes from all commits
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,3 @@ | ||
| package com.appsmith.server.git; | ||
|
|
||
| public interface AutoCommitEventHandler extends AutoCommitEventHandlerCE {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,11 @@ | ||
| package com.appsmith.server.helpers.ce.autocommit; | ||
| package com.appsmith.server.git.autocommit.helpers; | ||
|
|
||
| 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.AutoCommitTriggerDTO; | ||
| import com.appsmith.server.dtos.PageDTO; | ||
| import com.appsmith.server.git.GitRedisUtils; | ||
| import com.appsmith.server.helpers.CommonGitFileUtils; | ||
|
|
@@ -54,7 +55,7 @@ public Mono<Boolean> isServerAutoCommitRequired(String workspaceId, GitArtifactM | |
| .defaultIfEmpty(FALSE) | ||
| .cache(); | ||
|
|
||
| return Mono.defer(() -> gitRedisUtils.addFileLockWithoutRetry(defaultApplicationId)) | ||
| return Mono.defer(() -> gitRedisUtils.addFileLock(defaultApplicationId, FALSE)) | ||
|
Contributor
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. Tip Codebase Verification The verification process has revealed that the
These instances need to be updated to ensure consistency with the new method signature. Analysis chainThe updated usage of Scripts ExecutedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify usage of the updated addFileLock method across the project.
# Test: Search for the usage of addFileLock with the new parameter. Expect: Proper usage across the project.
rg --type java $'addFileLock'
Length of output: 10721 Script: #!/bin/bash
# Description: Verify the method signature and usage of addFileLock in the identified files.
# Check the method signature in GitRedisUtils.java
echo "Checking method signature in GitRedisUtils.java"
rg --type java --context 5 'public Mono<Boolean> addFileLock' app/server/appsmith-server/src/main/java/com/appsmith/server/git/GitRedisUtils.java
# Check the usage in AutoCommitEligibilityHelperImpl.java
echo "Checking usage in AutoCommitEligibilityHelperImpl.java"
rg --type java --context 5 'addFileLock' app/server/appsmith-server/src/main/java/com/appsmith/server/git/autocommit/helpers/AutoCommitEligibilityHelperImpl.java
# Check the usage in GitServiceCEImpl.java
echo "Checking usage in GitServiceCEImpl.java"
rg --type java --context 5 'addFileLock' app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/GitServiceCEImpl.java
# Check the usage in CommonGitServiceCEImpl.java
echo "Checking usage in CommonGitServiceCEImpl.java"
rg --type java --context 5 'addFileLock' app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/CommonGitServiceCEImpl.java
# Check the usage in AutoCommitEventHandlerCEImpl.java
echo "Checking usage in AutoCommitEventHandlerCEImpl.java"
rg --type java --context 5 'addFileLock' app/server/appsmith-server/src/main/java/com/appsmith/server/git/AutoCommitEventHandlerCEImpl.java
# Check the usage in RedisUtils.java
echo "Checking usage in RedisUtils.java"
rg --type java --context 5 'addFileLock' app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/RedisUtils.java
Length of output: 29273 |
||
| .then(Mono.defer(() -> isServerMigrationRequiredMonoCached)) | ||
| .then(Mono.defer(() -> gitRedisUtils.releaseFileLock(defaultApplicationId))) | ||
| .then(Mono.defer(() -> isServerMigrationRequiredMonoCached)) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,14 +43,14 @@ | |
| import com.appsmith.server.exceptions.AppsmithError; | ||
| import com.appsmith.server.exceptions.AppsmithException; | ||
| import com.appsmith.server.exports.internal.ExportService; | ||
| import com.appsmith.server.git.autocommit.helpers.GitAutoCommitHelper; | ||
|
Contributor
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. The method |
||
| import com.appsmith.server.helpers.CollectionUtils; | ||
| import com.appsmith.server.helpers.GitDeployKeyGenerator; | ||
| import com.appsmith.server.helpers.GitFileUtils; | ||
| import com.appsmith.server.helpers.GitPrivateRepoHelper; | ||
| import com.appsmith.server.helpers.GitUtils; | ||
| import com.appsmith.server.helpers.RedisUtils; | ||
| import com.appsmith.server.helpers.ResponseUtils; | ||
| import com.appsmith.server.helpers.ce.autocommit.GitAutoCommitHelper; | ||
| import com.appsmith.server.imports.internal.ImportService; | ||
| import com.appsmith.server.migrations.JsonSchemaVersions; | ||
| import com.appsmith.server.newactions.base.NewActionService; | ||
|
|
||
This file was deleted.
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.
Tip
Codebase Verification
The search results indicate that several instances of
addFileLockacross different files do not include theisRetryAllowedparameter. These instances need to be updated to use the new method signature.Files and Instances to Update:
AutoCommitEventHandlerCEImpl.java
private Mono<Boolean> addFileLock(String defaultApplicationId).addFileLock(defaultApplicationId)return addFileLock(autoCommitEvent.getApplicationId())return addFileLock(defaultApplicationId)CommonGitServiceCEImpl.java
return Mono.defer(() -> addFileLock(defaultArtifactId))private Mono<Boolean> addFileLock(String defaultArtifactId).addFileLock(defaultArtifactId)Mono<Boolean> fileLockMono = Mono.defer(() -> addFileLock(defaultArtifactId))addFileLockMono = addFileLock(defaultArtifactId)return addFileLock(tuple3.getT1().getDefaultArtifactId()).then(Mono.just(tuple3))return addFileLock(gitData.getDefaultArtifactId()).then(Mono.just(artifact))return addFileLock(artifact.getGitArtifactMetadata().getDefaultArtifactId())sourceAritfactMono = addFileLock(defaultArtifactId)Mono<? extends Artifact> checkoutRemoteBranchMono = addFileLock(defaultArtifactId)return addFileLock(srcBranchGitData.getDefaultArtifactId())return addFileLock(gitMetadata.getDefaultArtifactId()).flatMap(defaultArtifact -> addFileLock(defaultArtifactId).map(status -> defaultArtifact)).flatMap(branchedArtifact -> addFileLock(defaultArtifactId).thenReturn(branchedArtifact))return addFileLock(gitData.getDefaultArtifactId()).then(Mono.just(defaultArtifact))return addFileLock(defaultArtifactId)return addFileLock(metadata.getDefaultArtifactId())return addFileLock(defaultArtifact.getId())RedisUtils.java
public Mono<Boolean> addFileLock(String key)AutoCommitEligibilityHelperTest.java
Mockito.when(redisUtils.addFileLock(DEFAULT_APPLICATION_ID)).thenReturn(Mono.just(Boolean.TRUE))Action Required:
Update the above instances to include the
isRetryAllowedparameter in the method calls toaddFileLock.Analysis chain
Update to use the new
addFileLockmethod signature with retry control.Verify all instances of
addFileLockare updated to use the new method signature withisRetryAllowed.Verify specific instances of
addFileLockwithout theisRetryAllowedparameter.Scripts Executed
The following scripts were executed for the analysis:
Script:
Length of output: 10755
Script:
Length of output: 8737
Script:
Length of output: 3826