fix: partial import resource deleted from original page if imported in same app on a different page (#29379)

This commit is contained in:
Anagh Hegde 2023-12-07 18:59:42 +05:30 committed by GitHub
parent d1eaeffe76
commit e26f8dece2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 67 additions and 37 deletions

View File

@ -53,7 +53,8 @@ public class ActionCollectionImportableServiceCEImpl implements ImportableServic
MappedImportableResourcesDTO mappedImportableResourcesDTO, MappedImportableResourcesDTO mappedImportableResourcesDTO,
Mono<Workspace> workspaceMono, Mono<Workspace> workspaceMono,
Mono<Application> applicationMono, Mono<Application> applicationMono,
ApplicationJson applicationJson) { ApplicationJson applicationJson,
boolean isPartialImport) {
List<ActionCollection> importedActionCollectionList = List<ActionCollection> importedActionCollectionList =
CollectionUtils.isEmpty(applicationJson.getActionCollectionList()) CollectionUtils.isEmpty(applicationJson.getActionCollectionList())
? new ArrayList<>() ? new ArrayList<>()

View File

@ -61,7 +61,8 @@ public class DatasourceImportableServiceCEImpl implements ImportableServiceCE<Da
MappedImportableResourcesDTO mappedImportableResourcesDTO, MappedImportableResourcesDTO mappedImportableResourcesDTO,
Mono<Workspace> workspaceMono, Mono<Workspace> workspaceMono,
Mono<Application> applicationMono, Mono<Application> applicationMono,
ApplicationJson applicationJson) { ApplicationJson applicationJson,
boolean isPartialImport) {
return workspaceMono.flatMap(workspace -> { return workspaceMono.flatMap(workspace -> {
final Flux<Datasource> existingDatasourceFlux = datasourceService final Flux<Datasource> existingDatasourceFlux = datasourceService
.getAllByWorkspaceIdWithStorages(workspace.getId(), Optional.empty()) .getAllByWorkspaceIdWithStorages(workspace.getId(), Optional.empty())

View File

@ -15,12 +15,14 @@ public interface ImportableServiceCE<T extends BaseDomain> {
MappedImportableResourcesDTO mappedImportableResourcesDTO, MappedImportableResourcesDTO mappedImportableResourcesDTO,
Mono<Workspace> workspaceMono, Mono<Workspace> workspaceMono,
Mono<Application> applicationMono, Mono<Application> applicationMono,
ApplicationJson applicationJson); ApplicationJson applicationJson,
boolean isPartialImport);
default Mono<Void> updateImportedEntities( default Mono<Void> updateImportedEntities(
Application application, Application application,
ImportingMetaDTO importingMetaDTO, ImportingMetaDTO importingMetaDTO,
MappedImportableResourcesDTO mappedImportableResourcesDTO) { MappedImportableResourcesDTO mappedImportableResourcesDTO,
boolean isPartialImport) {
return null; return null;
} }
} }

View File

@ -554,9 +554,9 @@ public class ImportApplicationServiceCEImpl implements ImportApplicationServiceC
.then(importedApplicationMono) .then(importedApplicationMono)
.flatMap(application -> { .flatMap(application -> {
return newActionImportableService return newActionImportableService
.updateImportedEntities(application, importingMetaDTO, mappedImportableResourcesDTO) .updateImportedEntities(application, importingMetaDTO, mappedImportableResourcesDTO, false)
.then(newPageImportableService.updateImportedEntities( .then(newPageImportableService.updateImportedEntities(
application, importingMetaDTO, mappedImportableResourcesDTO)) application, importingMetaDTO, mappedImportableResourcesDTO, false))
.thenReturn(application); .thenReturn(application);
}) })
.flatMap(application -> { .flatMap(application -> {
@ -663,7 +663,8 @@ public class ImportApplicationServiceCEImpl implements ImportApplicationServiceC
mappedImportableResourcesDTO, mappedImportableResourcesDTO,
workspaceMono, workspaceMono,
importedApplicationMono, importedApplicationMono,
applicationJson); applicationJson,
false);
// Directly updates required theme information in DB // Directly updates required theme information in DB
Mono<Void> importedThemesMono = themeImportableService.importEntities( Mono<Void> importedThemesMono = themeImportableService.importEntities(
@ -671,7 +672,8 @@ public class ImportApplicationServiceCEImpl implements ImportApplicationServiceC
mappedImportableResourcesDTO, mappedImportableResourcesDTO,
workspaceMono, workspaceMono,
importedApplicationMono, importedApplicationMono,
applicationJson); applicationJson,
false);
// Updates pageNametoIdMap and pageNameMap in importable resources. // Updates pageNametoIdMap and pageNameMap in importable resources.
// Also directly updates required information in DB // Also directly updates required information in DB
@ -680,7 +682,8 @@ public class ImportApplicationServiceCEImpl implements ImportApplicationServiceC
mappedImportableResourcesDTO, mappedImportableResourcesDTO,
workspaceMono, workspaceMono,
importedApplicationMono, importedApplicationMono,
applicationJson); applicationJson,
false);
// Requires pluginMap to be present in importable resources. // Requires pluginMap to be present in importable resources.
// Updates datasourceNameToIdMap in importable resources. // Updates datasourceNameToIdMap in importable resources.
@ -690,7 +693,8 @@ public class ImportApplicationServiceCEImpl implements ImportApplicationServiceC
mappedImportableResourcesDTO, mappedImportableResourcesDTO,
workspaceMono, workspaceMono,
importedApplicationMono, importedApplicationMono,
applicationJson)); applicationJson,
false));
return List.of(importedDatasourcesMono, importedPagesMono, importedThemesMono); return List.of(importedDatasourcesMono, importedPagesMono, importedThemesMono);
} }
@ -711,7 +715,8 @@ public class ImportApplicationServiceCEImpl implements ImportApplicationServiceC
mappedImportableResourcesDTO, mappedImportableResourcesDTO,
workspaceMono, workspaceMono,
importedApplicationMono, importedApplicationMono,
applicationJson); applicationJson,
false);
// Requires pageNameMap, pageNameToOldNameMap, pluginMap and actionResultDTO to be present in importable // Requires pageNameMap, pageNameToOldNameMap, pluginMap and actionResultDTO to be present in importable
// resources. // resources.
@ -722,7 +727,8 @@ public class ImportApplicationServiceCEImpl implements ImportApplicationServiceC
mappedImportableResourcesDTO, mappedImportableResourcesDTO,
workspaceMono, workspaceMono,
importedApplicationMono, importedApplicationMono,
applicationJson); applicationJson,
false);
Mono<Void> combinedActionExportablesMono = importedNewActionsMono.then(importedActionCollectionsMono); Mono<Void> combinedActionExportablesMono = importedNewActionsMono.then(importedActionCollectionsMono);
return List.of(combinedActionExportablesMono); return List.of(combinedActionExportablesMono);
@ -734,7 +740,7 @@ public class ImportApplicationServiceCEImpl implements ImportApplicationServiceC
MappedImportableResourcesDTO mappedImportableResourcesDTO) { MappedImportableResourcesDTO mappedImportableResourcesDTO) {
// Persists relevant information and updates mapped resources // Persists relevant information and updates mapped resources
Mono<Void> installedJsLibsMono = customJSLibImportableService.importEntities( Mono<Void> installedJsLibsMono = customJSLibImportableService.importEntities(
importingMetaDTO, mappedImportableResourcesDTO, null, null, applicationJson); importingMetaDTO, mappedImportableResourcesDTO, null, null, applicationJson, false);
return installedJsLibsMono; return installedJsLibsMono;
} }

View File

@ -147,9 +147,9 @@ public class PartialImportServiceCEImpl implements PartialImportServiceCE {
} }
return newActionImportableService return newActionImportableService
.updateImportedEntities( .updateImportedEntities(
application, importingMetaDTO, mappedImportableResourcesDTO) application, importingMetaDTO, mappedImportableResourcesDTO, true)
.then(newPageImportableService.updateImportedEntities( .then(newPageImportableService.updateImportedEntities(
application, importingMetaDTO, mappedImportableResourcesDTO)) application, importingMetaDTO, mappedImportableResourcesDTO, true))
.thenReturn(application); .thenReturn(application);
}); });
}) })
@ -209,17 +209,19 @@ public class PartialImportServiceCEImpl implements PartialImportServiceCE {
mappedImportableResourcesDTO, mappedImportableResourcesDTO,
workspaceMono, workspaceMono,
importedApplicationMono, importedApplicationMono,
applicationJson); applicationJson,
true);
Mono<Void> datasourceMono = datasourceImportableService.importEntities( Mono<Void> datasourceMono = datasourceImportableService.importEntities(
importingMetaDTO, importingMetaDTO,
mappedImportableResourcesDTO, mappedImportableResourcesDTO,
workspaceMono, workspaceMono,
importedApplicationMono, importedApplicationMono,
applicationJson); applicationJson,
true);
Mono<Void> customJsLibMono = customJSLibImportableService.importEntities( Mono<Void> customJsLibMono = customJSLibImportableService.importEntities(
importingMetaDTO, mappedImportableResourcesDTO, null, null, applicationJson); importingMetaDTO, mappedImportableResourcesDTO, null, null, applicationJson, true);
return pluginMono.then(datasourceMono).then(customJsLibMono).then(); return pluginMono.then(datasourceMono).then(customJsLibMono).then();
} }
@ -235,14 +237,16 @@ public class PartialImportServiceCEImpl implements PartialImportServiceCE {
mappedImportableResourcesDTO, mappedImportableResourcesDTO,
workspaceMono, workspaceMono,
importedApplicationMono, importedApplicationMono,
applicationJson); applicationJson,
true);
Mono<Void> actionCollectionMono = actionCollectionImportableService.importEntities( Mono<Void> actionCollectionMono = actionCollectionImportableService.importEntities(
importingMetaDTO, importingMetaDTO,
mappedImportableResourcesDTO, mappedImportableResourcesDTO,
workspaceMono, workspaceMono,
importedApplicationMono, importedApplicationMono,
applicationJson); applicationJson,
true);
return actionMono.then(actionCollectionMono).then(); return actionMono.then(actionCollectionMono).then();
} }
@ -283,9 +287,10 @@ public class PartialImportServiceCEImpl implements PartialImportServiceCE {
return Mono.just(pageName); return Mono.just(pageName);
} }
applicationJson.getActionCollectionList().forEach(actionCollection -> { applicationJson.getActionCollectionList().forEach(actionCollection -> {
actionCollection.getPublishedCollection().setPageId(pageName);
actionCollection.getUnpublishedCollection().setPageId(pageName); actionCollection.getUnpublishedCollection().setPageId(pageName);
if (actionCollection.getPublishedCollection() != null) {
actionCollection.getPublishedCollection().setPageId(pageName);
}
String collectionName = actionCollection.getId().split("_")[1]; String collectionName = actionCollection.getId().split("_")[1];
actionCollection.setId(pageName + "_" + collectionName); actionCollection.setId(pageName + "_" + collectionName);
actionCollection.setGitSyncId(null); actionCollection.setGitSyncId(null);

View File

@ -30,7 +30,8 @@ public class CustomJSLibImportableServiceCEImpl implements ImportableServiceCE<C
MappedImportableResourcesDTO mappedImportableResourcesDTO, MappedImportableResourcesDTO mappedImportableResourcesDTO,
Mono<Workspace> workspaceMono, Mono<Workspace> workspaceMono,
Mono<Application> applicationMono, Mono<Application> applicationMono,
ApplicationJson applicationJson) { ApplicationJson applicationJson,
boolean isPartialImport) {
List<CustomJSLib> customJSLibs = applicationJson.getCustomJSLibList(); List<CustomJSLib> customJSLibs = applicationJson.getCustomJSLibList();
if (customJSLibs == null) { if (customJSLibs == null) {
customJSLibs = new ArrayList<>(); customJSLibs = new ArrayList<>();

View File

@ -66,7 +66,8 @@ public class NewActionImportableServiceCEImpl implements ImportableServiceCE<New
MappedImportableResourcesDTO mappedImportableResourcesDTO, MappedImportableResourcesDTO mappedImportableResourcesDTO,
Mono<Workspace> workspaceMono, Mono<Workspace> workspaceMono,
Mono<Application> applicationMono, Mono<Application> applicationMono,
ApplicationJson applicationJson) { ApplicationJson applicationJson,
boolean isPartialImport) {
List<NewAction> importedNewActionList = applicationJson.getActionList(); List<NewAction> importedNewActionList = applicationJson.getActionList();
@ -105,9 +106,13 @@ public class NewActionImportableServiceCEImpl implements ImportableServiceCE<New
&& CollectionUtils.isNotEmpty(importActionResultDTO.getExistingActions())) { && CollectionUtils.isNotEmpty(importActionResultDTO.getExistingActions())) {
// Remove unwanted actions // Remove unwanted actions
Set<String> invalidActionIds = new HashSet<>(); Set<String> invalidActionIds = new HashSet<>();
for (NewAction action : importActionResultDTO.getExistingActions()) { if (Boolean.FALSE.equals(isPartialImport)) {
if (!importActionResultDTO.getImportedActionIds().contains(action.getId())) { for (NewAction action : importActionResultDTO.getExistingActions()) {
invalidActionIds.add(action.getId()); if (!importActionResultDTO
.getImportedActionIds()
.contains(action.getId())) {
invalidActionIds.add(action.getId());
}
} }
} }
log.info("Deleting {} actions which are no more used", invalidActionIds.size()); log.info("Deleting {} actions which are no more used", invalidActionIds.size());
@ -136,7 +141,8 @@ public class NewActionImportableServiceCEImpl implements ImportableServiceCE<New
public Mono<Void> updateImportedEntities( public Mono<Void> updateImportedEntities(
Application application, Application application,
ImportingMetaDTO importingMetaDTO, ImportingMetaDTO importingMetaDTO,
MappedImportableResourcesDTO mappedImportableResourcesDTO) { MappedImportableResourcesDTO mappedImportableResourcesDTO,
boolean isPartialImport) {
ImportActionResultDTO importActionResultDTO = mappedImportableResourcesDTO.getActionResultDTO(); ImportActionResultDTO importActionResultDTO = mappedImportableResourcesDTO.getActionResultDTO();
ImportActionCollectionResultDTO importActionCollectionResultDTO = ImportActionCollectionResultDTO importActionCollectionResultDTO =
@ -157,7 +163,8 @@ public class NewActionImportableServiceCEImpl implements ImportableServiceCE<New
// Delete the invalid resources (which are not the part of applicationJsonDTO) in // Delete the invalid resources (which are not the part of applicationJsonDTO) in
// the git flow only // the git flow only
if (!StringUtils.isEmpty(importingMetaDTO.getApplicationId()) if (!StringUtils.isEmpty(importingMetaDTO.getApplicationId())
&& !importingMetaDTO.getAppendToApp()) { && !importingMetaDTO.getAppendToApp()
&& Boolean.FALSE.equals(isPartialImport)) {
// Remove unwanted action collections // Remove unwanted action collections
Set<String> invalidCollectionIds = new HashSet<>(); Set<String> invalidCollectionIds = new HashSet<>();
for (ActionCollection collection : for (ActionCollection collection :

View File

@ -68,7 +68,8 @@ public class NewPageImportableServiceCEImpl implements ImportableServiceCE<NewPa
MappedImportableResourcesDTO mappedImportableResourcesDTO, MappedImportableResourcesDTO mappedImportableResourcesDTO,
Mono<Workspace> workspaceMono, Mono<Workspace> workspaceMono,
Mono<Application> applicationMono, Mono<Application> applicationMono,
ApplicationJson applicationJson) { ApplicationJson applicationJson,
boolean isPartialImport) {
List<NewPage> importedNewPageList = applicationJson.getPageList(); List<NewPage> importedNewPageList = applicationJson.getPageList();
@ -113,7 +114,8 @@ public class NewPageImportableServiceCEImpl implements ImportableServiceCE<NewPa
public Mono<Void> updateImportedEntities( public Mono<Void> updateImportedEntities(
Application application, Application application,
ImportingMetaDTO importingMetaDTO, ImportingMetaDTO importingMetaDTO,
MappedImportableResourcesDTO mappedImportableResourcesDTO) { MappedImportableResourcesDTO mappedImportableResourcesDTO,
boolean isPartialImport) {
ImportedActionAndCollectionMapsDTO actionAndCollectionMapsDTO = ImportedActionAndCollectionMapsDTO actionAndCollectionMapsDTO =
mappedImportableResourcesDTO.getActionAndCollectionMapsDTO(); mappedImportableResourcesDTO.getActionAndCollectionMapsDTO();

View File

@ -33,7 +33,8 @@ public class PluginImportableServiceCEImpl implements ImportableServiceCE<Plugin
MappedImportableResourcesDTO mappedImportableResourcesDTO, MappedImportableResourcesDTO mappedImportableResourcesDTO,
Mono<Workspace> workspaceMono, Mono<Workspace> workspaceMono,
Mono<Application> applicationMono, Mono<Application> applicationMono,
ApplicationJson applicationJson) { ApplicationJson applicationJson,
boolean isPartialImport) {
return workspaceMono return workspaceMono
.map(workspace -> workspace.getPlugins().stream() .map(workspace -> workspace.getPlugins().stream()
.map(WorkspacePlugin::getPluginId) .map(WorkspacePlugin::getPluginId)

View File

@ -53,7 +53,8 @@ public class ThemeImportableServiceCEImpl implements ImportableServiceCE<Theme>
MappedImportableResourcesDTO mappedImportableResourcesDTO, MappedImportableResourcesDTO mappedImportableResourcesDTO,
Mono<Workspace> workspaceMono, Mono<Workspace> workspaceMono,
Mono<Application> applicationMono, Mono<Application> applicationMono,
ApplicationJson applicationJson) { ApplicationJson applicationJson,
boolean isPartialImport) {
if (Boolean.TRUE.equals(importingMetaDTO.getAppendToApp())) { if (Boolean.TRUE.equals(importingMetaDTO.getAppendToApp())) {
// appending to existing app, theme should not change // appending to existing app, theme should not change
return Mono.empty().then(); return Mono.empty().then();

View File

@ -180,7 +180,8 @@ public class ThemeImportableServiceCETest {
new MappedImportableResourcesDTO(), new MappedImportableResourcesDTO(),
null, null,
Mono.just(application), Mono.just(application),
applicationJson) applicationJson,
false)
.thenReturn(savedApplication.getId())) .thenReturn(savedApplication.getId()))
.flatMap(applicationId -> applicationRepository.findById(applicationId, MANAGE_APPLICATIONS)); .flatMap(applicationId -> applicationRepository.findById(applicationId, MANAGE_APPLICATIONS));
@ -224,7 +225,8 @@ public class ThemeImportableServiceCETest {
new MappedImportableResourcesDTO(), new MappedImportableResourcesDTO(),
null, null,
Mono.just(savedApplication), Mono.just(savedApplication),
applicationJson) applicationJson,
false)
.thenReturn(savedApplication.getId()); .thenReturn(savedApplication.getId());
}) })
.flatMap(applicationId -> applicationRepository.findById(applicationId, MANAGE_APPLICATIONS)); .flatMap(applicationId -> applicationRepository.findById(applicationId, MANAGE_APPLICATIONS));

View File

@ -45,6 +45,7 @@ import reactor.test.StepVerifier;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyBoolean;
// All the test case are for failure or exception. Test cases for valid json file is already present in // All the test case are for failure or exception. Test cases for valid json file is already present in
// ImportExportApplicationServiceTest class // ImportExportApplicationServiceTest class
@ -135,7 +136,7 @@ public class ImportApplicationTransactionServiceTest {
Workspace newWorkspace = new Workspace(); Workspace newWorkspace = new Workspace();
newWorkspace.setName("Template Workspace"); newWorkspace.setName("Template Workspace");
Mockito.when(newActionImportableService.importEntities(any(), any(), any(), any(), any())) Mockito.when(newActionImportableService.importEntities(any(), any(), any(), any(), any(), anyBoolean()))
.thenReturn(Mono.error(new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST))); .thenReturn(Mono.error(new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST)));
Workspace createdWorkspace = workspaceService.create(newWorkspace).block(); Workspace createdWorkspace = workspaceService.create(newWorkspace).block();
@ -166,7 +167,7 @@ public class ImportApplicationTransactionServiceTest {
Workspace newWorkspace = new Workspace(); Workspace newWorkspace = new Workspace();
newWorkspace.setName("Template Workspace"); newWorkspace.setName("Template Workspace");
Mockito.when(newActionImportableService.importEntities(any(), any(), any(), any(), any())) Mockito.when(newActionImportableService.importEntities(any(), any(), any(), any(), any(), anyBoolean()))
.thenReturn(Mono.error(new MongoTransactionException( .thenReturn(Mono.error(new MongoTransactionException(
"Command failed with error 251 (NoSuchTransaction): 'Transaction 1 has been aborted.'"))); "Command failed with error 251 (NoSuchTransaction): 'Transaction 1 has been aborted.'")));