Protei/PromucFlow_constructor
2
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

fix: App visibility of git connected application is reset after git merge (#11890)

Browse Source 
* Fix issue with policies being overridden during hydration from file system to db

* Fix tests

* Update the policies for the branched application

* Add test cases for the visibility changes for git connected applications

* Remove the visibility flag from application object while committing to git repo
This commit is contained in:
Anagh Hegde 2022-03-24 12:31:28 +05:30 committed by GitHub
parent 6a503b4d57
commit fae7fc6e7c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 167 additions and 45 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/GitFileUtils.java
View File

@ -258,6 +258,7 @@ public class GitFileUtils {
// Don't commit application name as while importing we are using the repoName as application name
application.setName(null);
application.setPublishedPages(null);
application.setIsPublic(null);
application.setSlug(null);
}

2
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCE.java
View File

@ -34,7 +34,7 @@ public interface CustomApplicationRepositoryCE extends AppsmithRepository<Applic
Mono<Application> getApplicationByGitBranchAndDefaultApplicationId(String defaultApplicationId, String branchName, AclPermission aclPermission);
Flux<Application> getApplicationByGitDefaultApplicationId(String defaultApplicationId);
Flux<Application> getApplicationByGitDefaultApplicationId(String defaultApplicationId, AclPermission permission);
Mono<List<String>> getAllApplicationId(String organizationId);

4
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomApplicationRepositoryCEImpl.java
View File

@ -139,12 +139,12 @@ public class CustomApplicationRepositoryCEImpl extends BaseAppsmithRepositoryImp
}
@Override
public Flux<Application> getApplicationByGitDefaultApplicationId(String defaultApplicationId) {
public Flux<Application> getApplicationByGitDefaultApplicationId(String defaultApplicationId, AclPermission permission) {
String gitApplicationMetadata = fieldName(QApplication.application.gitApplicationMetadata);
Criteria applicationIdCriteria = where(gitApplicationMetadata + "." + fieldName(QApplication.application.gitApplicationMetadata.defaultApplicationId)).is(defaultApplicationId);
Criteria deletionCriteria = where(fieldName(QApplication.application.deleted)).ne(true);
return queryAll(List.of(applicationIdCriteria, deletionCriteria), AclPermission.MANAGE_APPLICATIONS);
return queryAll(List.of(applicationIdCriteria, deletionCriteria), permission);
}
/**

2
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java
View File

@ -400,7 +400,7 @@ public class ApplicationPageServiceCEImpl implements ApplicationPageServiceCE {
// Delete git repo from local and delete the applications from DB
return gitFileUtils.detachRemote(repoPath)
.flatMapMany(isCleared -> applicationService
.findAllApplicationsByDefaultApplicationId(gitData.getDefaultApplicationId()));
.findAllApplicationsByDefaultApplicationId(gitData.getDefaultApplicationId(), MANAGE_APPLICATIONS));
}
return Flux.fromIterable(List.of(application));
})

2
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationServiceCE.java
View File

@ -55,7 +55,7 @@ public interface ApplicationServiceCE extends CrudService<Application, String> {
Mono<String> findBranchedApplicationId(String branchName, String defaultApplicationId, AclPermission permission);
Flux<Application> findAllApplicationsByDefaultApplicationId(String defaultApplicationId);
Flux<Application> findAllApplicationsByDefaultApplicationId(String defaultApplicationId, AclPermission permission);
Mono<Long> getGitConnectedApplicationsCountWithPrivateRepoByOrgId(String organizationId);

11
app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationServiceCEImpl.java
View File

@ -266,9 +266,12 @@ public class ApplicationServiceCEImpl extends BaseService<ApplicationRepository,
public Mono<Application> changeViewAccess(String defaultApplicationId,
String branchName,
ApplicationAccessDTO applicationAccessDTO) {
return this.findByBranchNameAndDefaultApplicationId(branchName, defaultApplicationId, MAKE_PUBLIC_APPLICATIONS)
// For git connected application update the policy for all the branch's
return findAllApplicationsByDefaultApplicationId(defaultApplicationId, MAKE_PUBLIC_APPLICATIONS)
.switchIfEmpty(this.findByBranchNameAndDefaultApplicationId(branchName, defaultApplicationId, MAKE_PUBLIC_APPLICATIONS))
.flatMap(branchedApplication -> changeViewAccess(branchedApplication.getId(), applicationAccessDTO))
.map(responseUtils::updateApplicationWithDefaultResources);
.then(repository.findById(defaultApplicationId, MAKE_PUBLIC_APPLICATIONS)
.map(responseUtils::updateApplicationWithDefaultResources));
}
@Override
@ -535,8 +538,8 @@ public class ApplicationServiceCEImpl extends BaseService<ApplicationRepository,
* @return Application flux which match the condition
*/
@Override
public Flux<Application> findAllApplicationsByDefaultApplicationId(String defaultApplicationId) {
return repository.getApplicationByGitDefaultApplicationId(defaultApplicationId);
public Flux<Application> findAllApplicationsByDefaultApplicationId(String defaultApplicationId, AclPermission permission) {
return repository.getApplicationByGitDefaultApplicationId(defaultApplicationId, permission);
}
@Override

40
app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ImportExportApplicationServiceCEImpl.java
View File

@ -672,7 +672,8 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica
// 1. Assign the policies for the imported application
// 2. Check for possible duplicate names,
// 3. Save the updated application
applicationPageService.setApplicationPolicies(currUserMono, organizationId, importedApplication)
Mono.just(importedApplication)
.zipWith(currUserMono)
.map(objects -> {
Application application = objects.getT1();
@ -698,28 +699,37 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica
// the changes from remote
// We are using the save instead of update as we are using @Encrypted
// for GitAuth
return applicationService.save(existingApplication)
.onErrorResume(DuplicateKeyException.class, error -> {
if (error.getMessage() != null) {
return applicationPageService
.createOrUpdateSuffixedApplication(
existingApplication,
existingApplication.getName(),
0
);
}
throw error;
return applicationService.findById(existingApplication.getGitApplicationMetadata().getDefaultApplicationId())
.flatMap(application1 -> {
// Set the policies from the defaultApplication
existingApplication.setPolicies(application1.getPolicies());
importedApplication.setPolicies(application1.getPolicies());
return applicationService.save(existingApplication)
.onErrorResume(DuplicateKeyException.class, error -> {
if (error.getMessage() != null) {
return applicationPageService
.createOrUpdateSuffixedApplication(
existingApplication,
existingApplication.getName(),
0
);
}
throw error;
});
});
});
}
Mono<Application> applicationMono = applicationPageService.setApplicationPolicies(currUserMono, organizationId, importedApplication);
return applicationService
.findByOrganizationId(organizationId, MANAGE_APPLICATIONS)
.collectList()
.flatMap(applicationList -> {
.zipWith(applicationMono)
.flatMap(objects -> {
Application application1 = objects.getT2();
List<Application> applicationList = objects.getT1();
Application duplicateNameApp = applicationList
.stream()
.filter(application1 -> StringUtils.equals(application1.getName(), application.getName()))
.filter(application2 -> StringUtils.equals(application2.getName(), application1.getName()))
.findAny()
.orElse(null);

45
app/server/appsmith-server/src/test/java/com/appsmith/server/services/ApplicationServiceTest.java
View File

@ -792,6 +792,16 @@ public class ApplicationServiceTest {
ApplicationAccessDTO applicationAccessDTO = new ApplicationAccessDTO();
applicationAccessDTO.setPublicAccess(true);
// Create a branch
Application testApplication = new Application();
testApplication.setName("branch1");
testApplication.setOrganizationId(orgId);
GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata();
gitApplicationMetadata.setDefaultApplicationId(gitConnectedApp.getId());
gitApplicationMetadata.setBranchName("test");
testApplication.setGitApplicationMetadata(gitApplicationMetadata);
Application application = applicationPageService.createApplication(testApplication).block();
Mono<Application> publicAppMono = applicationService
.changeViewAccess(gitConnectedApp.getId(), "testBranch", applicationAccessDTO)
.cache();
@ -815,6 +825,15 @@ public class ApplicationServiceTest {
assertThat(page.getPolicies()).containsAll(Set.of(managePagePolicy, readPagePolicy));
})
.verifyComplete();
// Get branch application
Mono<Application> branchApplicationMono = applicationService.findById(application.getId());
StepVerifier
.create(branchApplicationMono)
.assertNext(branchApplication -> {
assertThat(branchApplication.getPolicies()).containsAll(Set.of(manageAppPolicy, readAppPolicy));
})
.verifyComplete();
}
@Test
@ -834,13 +853,23 @@ public class ApplicationServiceTest {
.users(Set.of("api_user"))
.build();
// Create a branch
Application testApplication = new Application();
testApplication.setName("branch2");
testApplication.setOrganizationId(orgId);
GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata();
gitApplicationMetadata.setDefaultApplicationId(gitConnectedApp.getId());
gitApplicationMetadata.setBranchName("test2");
testApplication.setGitApplicationMetadata(gitApplicationMetadata);
Application application = applicationPageService.createApplication(testApplication).block();
ApplicationAccessDTO applicationAccessDTO = new ApplicationAccessDTO();
applicationAccessDTO.setPublicAccess(true);
Mono<Application> privateAppMono = applicationService.changeViewAccess(gitConnectedApp.getId(), "testBranch", applicationAccessDTO)
.flatMap(application1 -> {
applicationAccessDTO.setPublicAccess(false);
return applicationService.changeViewAccess(application1.getId(), applicationAccessDTO);
return applicationService.changeViewAccess(application1.getId(), "testBranch", applicationAccessDTO);
})
.cache();
@ -863,6 +892,16 @@ public class ApplicationServiceTest {
assertThat(page.getPolicies()).containsAll(Set.of(managePagePolicy, readPagePolicy));
})
.verifyComplete();
// Get branch application
Mono<Application> branchApplicationMono = applicationService.findById(application.getId());
StepVerifier
.create(branchApplicationMono)
.assertNext(branchApplication -> {
assertThat(branchApplication.getIsPublic()).isFalse();
assertThat(branchApplication.getPolicies()).containsAll(Set.of(readAppPolicy, manageAppPolicy));
})
.verifyComplete();
}
@Test
@ -1027,7 +1066,7 @@ public class ApplicationServiceTest {
assertThat(clonedPageList).isNotEmpty();
for (PageDTO page : clonedPageList) {
assertThat(page.getPolicies()).containsAll(Set.of(managePagePolicy, readPagePolicy));
assertThat(page.getPolicies()).containsAll(Set.of(readPagePolicy, managePagePolicy));
assertThat(page.getApplicationId()).isEqualTo(clonedApplication.getId());
}
})
@ -1176,7 +1215,7 @@ public class ApplicationServiceTest {
assertThat(clonedActionList).isNotEmpty();
assertThat(defaultClonedActionIdsFromDb).isNotEmpty();
for (NewAction newAction : clonedActionList) {
assertThat(newAction.getPolicies()).containsAll(Set.of(manageActionPolicy, readActionPolicy, executeActionPolicy));
assertThat(newAction.getPolicies()).containsAll(Set.of(readActionPolicy, executeActionPolicy, manageActionPolicy));
assertThat(newAction.getApplicationId()).isEqualTo(clonedApplication.getId());
assertThat(newAction.getUnpublishedAction().getPageId()).isEqualTo(newAction.getUnpublishedAction().getDefaultResources().getPageId());
}

105
app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportExportApplicationServiceTests.java
View File

@ -38,6 +38,7 @@ import com.appsmith.server.repositories.PluginRepository;
import com.appsmith.server.repositories.ThemeRepository;
import com.appsmith.server.services.ActionCollectionService;
import com.appsmith.server.services.ApplicationPageService;
import com.appsmith.server.services.ApplicationService;
import com.appsmith.server.services.DatasourceService;
import com.appsmith.server.services.LayoutActionService;
import com.appsmith.server.services.LayoutCollectionService;
@ -159,6 +160,9 @@ public class ImportExportApplicationServiceTests {
@Autowired
ThemeService themeService;
@Autowired
ApplicationService applicationService;
private static final String INVALID_JSON_FILE = "invalid json file";
private static Plugin installedPlugin;
private static String orgId;
@ -1101,7 +1105,11 @@ public class ImportExportApplicationServiceTests {
gitData.setBranchName("testBranch");
testApplication.setGitApplicationMetadata(gitData);
Application savedApplication = applicationPageService.createApplication(testApplication, orgId).block();
Application savedApplication = applicationPageService.createApplication(testApplication, orgId)
.flatMap(application1 -> {
application1.getGitApplicationMetadata().setDefaultApplicationId(application1.getId());
return applicationService.save(application1);
}).block();
Mono<Application> result = newPageService.findNewPagesByApplicationId(savedApplication.getId(), READ_PAGES).collectList()
.flatMap(newPages -> {
@ -1259,7 +1267,11 @@ public class ImportExportApplicationServiceTests {
gitData.setBranchName("master");
testApplication.setGitApplicationMetadata(gitData);
Application application = applicationPageService.createApplication(testApplication, orgId).block();
Application application = applicationPageService.createApplication(testApplication, orgId)
.flatMap(application1 -> {
application1.getGitApplicationMetadata().setDefaultApplicationId(application1.getId());
return applicationService.save(application1);
}).block();
String gitSyncIdBeforeImport = newPageService.findById(application.getPages().get(0).getId(), MANAGE_PAGES).block().getGitSyncId();
PageDTO page = new PageDTO();
@ -1319,7 +1331,11 @@ public class ImportExportApplicationServiceTests {
gitData.setBranchName("master");
testApplication.setGitApplicationMetadata(gitData);
Application application = applicationPageService.createApplication(testApplication, orgId).block();
Application application = applicationPageService.createApplication(testApplication, orgId)
.flatMap(application1 -> {
application1.getGitApplicationMetadata().setDefaultApplicationId(application1.getId());
return applicationService.save(application1);
}).block();
String gitSyncIdBeforeImport = newPageService.findById(application.getPages().get(0).getId(), MANAGE_PAGES).block().getGitSyncId();
@ -1434,10 +1450,19 @@ public class ImportExportApplicationServiceTests {
final Mono<Application> resultMonoWithDiscardOperation = resultMonoWithoutDiscardOperation
.flatMap(importedApplication ->
applicationJsonMono
.flatMap(applicationJson ->
importExportApplicationService
.importApplicationInOrganization(importedApplication.getOrganizationId(), applicationJson, importedApplication.getId(), "main")
)
.flatMap(applicationJson ->
{
importedApplication.setGitApplicationMetadata(new GitApplicationMetadata());
importedApplication.getGitApplicationMetadata().setDefaultApplicationId(importedApplication.getId());
return applicationService.save(importedApplication)
.then(importExportApplicationService.importApplicationInOrganization(
importedApplication.getOrganizationId(),
applicationJson,
importedApplication.getId(),
"main")
);
}
)
);
StepVerifier
@ -1519,9 +1544,17 @@ public class ImportExportApplicationServiceTests {
final Mono<Application> resultMonoWithDiscardOperation = resultMonoWithoutDiscardOperation
.flatMap(importedApplication ->
applicationJsonMono
.flatMap(applicationJson ->
importExportApplicationService
.importApplicationInOrganization(importedApplication.getOrganizationId(), applicationJson, importedApplication.getId(), "main")
.flatMap(applicationJson -> {
importedApplication.setGitApplicationMetadata(new GitApplicationMetadata());
importedApplication.getGitApplicationMetadata().setDefaultApplicationId(importedApplication.getId());
return applicationService.save(importedApplication)
.then(importExportApplicationService.importApplicationInOrganization(
importedApplication.getOrganizationId(),
applicationJson,
importedApplication.getId(),
"main")
);
}
)
);
@ -1612,8 +1645,17 @@ public class ImportExportApplicationServiceTests {
.flatMap(importedApplication ->
applicationJsonMono
.flatMap(applicationJson ->
importExportApplicationService
.importApplicationInOrganization(importedApplication.getOrganizationId(), applicationJson, importedApplication.getId(), "main")
{
importedApplication.setGitApplicationMetadata(new GitApplicationMetadata());
importedApplication.getGitApplicationMetadata().setDefaultApplicationId(importedApplication.getId());
return applicationService.save(importedApplication)
.then(importExportApplicationService.importApplicationInOrganization(
importedApplication.getOrganizationId(),
applicationJson,
importedApplication.getId(),
"main")
);
}
)
);
@ -1694,8 +1736,17 @@ public class ImportExportApplicationServiceTests {
.flatMap(importedApplication ->
applicationJsonMono
.flatMap(applicationJson ->
importExportApplicationService
.importApplicationInOrganization(importedApplication.getOrganizationId(), applicationJson, importedApplication.getId(), "main")
{
importedApplication.setGitApplicationMetadata(new GitApplicationMetadata());
importedApplication.getGitApplicationMetadata().setDefaultApplicationId(importedApplication.getId());
return applicationService.save(importedApplication)
.then(importExportApplicationService.importApplicationInOrganization(
importedApplication.getOrganizationId(),
applicationJson,
importedApplication.getId(),
"main")
);
}
)
);
@ -1771,8 +1822,17 @@ public class ImportExportApplicationServiceTests {
.flatMap(importedApplication ->
applicationJsonMono
.flatMap(applicationJson ->
importExportApplicationService
.importApplicationInOrganization(importedApplication.getOrganizationId(), applicationJson, importedApplication.getId(), "main")
{
importedApplication.setGitApplicationMetadata(new GitApplicationMetadata());
importedApplication.getGitApplicationMetadata().setDefaultApplicationId(importedApplication.getId());
return applicationService.save(importedApplication)
.then(importExportApplicationService.importApplicationInOrganization(
importedApplication.getOrganizationId(),
applicationJson,
importedApplication.getId(),
"main")
);
}
)
);
@ -1849,8 +1909,17 @@ public class ImportExportApplicationServiceTests {
.flatMap(importedApplication ->
applicationJsonMono
.flatMap(applicationJson ->
importExportApplicationService
.importApplicationInOrganization(importedApplication.getOrganizationId(), applicationJson, importedApplication.getId(), "main")
{
importedApplication.setGitApplicationMetadata(new GitApplicationMetadata());
importedApplication.getGitApplicationMetadata().setDefaultApplicationId(importedApplication.getId());
return applicationService.save(importedApplication)
.then(importExportApplicationService.importApplicationInOrganization(
importedApplication.getOrganizationId(),
applicationJson,
importedApplication.getId(),
"main")
);
}
)
);