fix: Fix importing datasource with same name but different plugin issue (#12512)

Co-authored-by: Anagh Hegde <anagh@appsmith.com>
This commit is contained in:
Abhijeet 2022-04-04 13:23:00 +05:30 committed by GitHub
parent f201ec7bcd
commit ffd0595330
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 126 additions and 24 deletions

View File

@ -591,9 +591,9 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica
/** /**
* This function will take the application reference object to hydrate the application in mongoDB * This function will take the application reference object to hydrate the application in mongoDB
* *
* @param organizationId organization to which application is going to be stored * @param organizationId organization to which application is going to be stored
* @param applicationJson application resource which contains necessary information to save the application * @param applicationJson application resource which contains necessary information to import the application
* @param applicationId application which needs to be saved with the updated resources * @param applicationId application which needs to be saved with the updated resources
* @return Updated application * @return Updated application
*/ */
public Mono<Application> importApplicationInOrganization(String organizationId, public Mono<Application> importApplicationInOrganization(String organizationId,
@ -689,6 +689,7 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica
// Check for duplicate datasources to avoid duplicates in target organization // Check for duplicate datasources to avoid duplicates in target organization
.flatMap(datasource -> { .flatMap(datasource -> {
final String importedDatasourceName = datasource.getName();
// Check if the datasource has gitSyncId and if it's already in DB // Check if the datasource has gitSyncId and if it's already in DB
if (datasource.getGitSyncId() != null if (datasource.getGitSyncId() != null
&& savedDatasourcesGitIdToDatasourceMap.containsKey(datasource.getGitSyncId())) { && savedDatasourcesGitIdToDatasourceMap.containsKey(datasource.getGitSyncId())) {
@ -702,7 +703,11 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica
datasource.setPluginId(null); datasource.setPluginId(null);
AppsmithBeanUtils.copyNestedNonNullProperties(datasource, existingDatasource); AppsmithBeanUtils.copyNestedNonNullProperties(datasource, existingDatasource);
existingDatasource.setStructure(null); existingDatasource.setStructure(null);
return datasourceService.update(existingDatasource.getId(), existingDatasource); return datasourceService.update(existingDatasource.getId(), existingDatasource)
.map(datasource1 -> {
datasourceMap.put(importedDatasourceName, datasource1.getId());
return datasource1;
});
} }
// This is explicitly copied over from the map we created before // This is explicitly copied over from the map we created before
@ -719,11 +724,11 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica
updateAuthenticationDTO(datasource, decryptedFields); updateAuthenticationDTO(datasource, decryptedFields);
} }
return createUniqueDatasourceIfNotPresent(existingDatasourceFlux, datasource, organizationId, applicationId); return createUniqueDatasourceIfNotPresent(existingDatasourceFlux, datasource, organizationId)
}) .map(datasource1 -> {
.map(datasource -> { datasourceMap.put(importedDatasourceName, datasource1.getId());
datasourceMap.put(datasource.getName(), datasource.getId()); return datasource1;
return datasource; });
}) })
.collectList(); .collectList();
}) })
@ -1726,9 +1731,7 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica
*/ */
private Mono<Datasource> createUniqueDatasourceIfNotPresent(Flux<Datasource> existingDatasourceFlux, private Mono<Datasource> createUniqueDatasourceIfNotPresent(Flux<Datasource> existingDatasourceFlux,
Datasource datasource, Datasource datasource,
String organizationId, String organizationId) {
String applicationId) {
/* /*
1. If same datasource is present return 1. If same datasource is present return
2. If unable to find the datasource create a new datasource with unique name and return 2. If unable to find the datasource create a new datasource with unique name and return
@ -1743,16 +1746,8 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica
} }
return existingDatasourceFlux return existingDatasourceFlux
.map(ds -> {
final DatasourceConfiguration dsAuthConfig = ds.getDatasourceConfiguration();
if (dsAuthConfig != null && dsAuthConfig.getAuthentication() != null) {
dsAuthConfig.getAuthentication().setAuthenticationResponse(null);
dsAuthConfig.getAuthentication().setAuthenticationType(null);
}
return ds;
})
// For git import exclude datasource configuration // For git import exclude datasource configuration
.filter(ds -> applicationId != null ? ds.getName().equals(datasource.getName()) : ds.softEquals(datasource)) .filter(ds -> ds.getName().equals(datasource.getName()) && datasource.getPluginId().equals(ds.getPluginId()))
.next() // Get the first matching datasource, we don't need more than one here. .next() // Get the first matching datasource, we don't need more than one here.
.switchIfEmpty(Mono.defer(() -> { .switchIfEmpty(Mono.defer(() -> {
if (datasourceConfig != null && datasourceConfig.getAuthentication() != null) { if (datasourceConfig != null && datasourceConfig.getAuthentication() != null) {

View File

@ -1702,7 +1702,7 @@ public class GitServiceTest {
StepVerifier StepVerifier
.create(applicationMono) .create(applicationMono)
.expectErrorMatches(throwable -> throwable instanceof AppsmithException .expectErrorMatches(throwable -> throwable instanceof AppsmithException
&& throwable.getMessage().equals(AppsmithError.GIT_ACTION_FAILED.getMessage("checkout", "origin/branchInLocal already exists in remote"))) && throwable.getMessage().equals(AppsmithError.GIT_ACTION_FAILED.getMessage("checkout", "origin/branchInLocal already exists in local - branchInLocal")))
.verify(); .verify();
} }
@ -2486,7 +2486,7 @@ public class GitServiceTest {
@Test @Test
@WithUserDetails(value = "api_user") @WithUserDetails(value = "api_user")
public void importApplicationFromGit_validRequestWithDuplicateDatasourceOfSameTypeCancelledMidway_Success() throws GitAPIException, IOException { public void importApplicationFromGit_validRequestWithDuplicateDatasourceOfSameTypeCancelledMidway_Success() {
Organization organization = new Organization(); Organization organization = new Organization();
organization.setName("gitImportOrgCancelledMidway"); organization.setName("gitImportOrgCancelledMidway");
final String testOrgId = organizationService.create(organization) final String testOrgId = organizationService.create(organization)
@ -2497,6 +2497,7 @@ public class GitServiceTest {
GitAuth gitAuth = gitService.generateSSHKey().block(); GitAuth gitAuth = gitService.generateSSHKey().block();
ApplicationJson applicationJson = createAppJson(filePath).block(); ApplicationJson applicationJson = createAppJson(filePath).block();
applicationJson.getExportedApplication().setName(null);
applicationJson.getDatasourceList().get(0).setName("db-auth-testGitImportRepo"); applicationJson.getDatasourceList().get(0).setName("db-auth-testGitImportRepo");
String pluginId = pluginRepository.findByPackageName("mongo-plugin").block().getId(); String pluginId = pluginRepository.findByPackageName("mongo-plugin").block().getId();
@ -2521,7 +2522,7 @@ public class GitServiceTest {
// Wait for git clone to complete // Wait for git clone to complete
Mono<Application> gitConnectedAppFromDbMono = Mono.just(testOrgId) Mono<Application> gitConnectedAppFromDbMono = Mono.just(testOrgId)
.flatMap(application -> { .flatMap(ignore -> {
try { try {
// Before fetching the git connected application, sleep for 5 seconds to ensure that the clone // Before fetching the git connected application, sleep for 5 seconds to ensure that the clone
// completes // completes

View File

@ -2182,5 +2182,111 @@ public class ImportExportApplicationServiceTests {
}) })
.verifyComplete(); .verifyComplete();
} }
@Test
@WithUserDetails(value = "api_user")
public void importApplication_datasourceWithSameNameAndDifferentPlugin_importedWithValidActionsAndSuffixedDatasource() {
ApplicationJson applicationJson = createAppJson("test_assets/ImportExportServiceTest/valid-application.json").block();
Organization testOrganization = new Organization();
testOrganization.setName("Duplicate datasource with different plugin org");
testOrganization = organizationService.create(testOrganization).block();
Datasource testDatasource = new Datasource();
// Chose any plugin except for mongo, as json static file has mongo plugin for datasource
Plugin postgreSQLPlugin = pluginRepository.findByName("PostgreSQL").block();
testDatasource.setPluginId(postgreSQLPlugin.getId());
testDatasource.setOrganizationId(testOrganization.getId());
final String datasourceName = applicationJson.getDatasourceList().get(0).getName();
testDatasource.setName(datasourceName);
datasourceService.create(testDatasource).block();
final Mono<Application> resultMono = importExportApplicationService.importApplicationInOrganization(testOrganization.getId(), applicationJson);
StepVerifier
.create(resultMono
.flatMap(application -> Mono.zip(
Mono.just(application),
datasourceService.findAllByOrganizationId(application.getOrganizationId(), MANAGE_DATASOURCES).collectList(),
newActionService.findAllByApplicationIdAndViewMode(application.getId(), false, READ_ACTIONS, null).collectList()
)))
.assertNext(tuple -> {
final Application application = tuple.getT1();
final List<Datasource> datasourceList = tuple.getT2();
final List<NewAction> actionList = tuple.getT3();
assertThat(application.getName()).isEqualTo("valid_application");
List<String> datasourceNameList = new ArrayList<>();
assertThat(datasourceList).isNotEmpty();
datasourceList.forEach(datasource -> {
assertThat(datasource.getOrganizationId()).isEqualTo(application.getOrganizationId());
datasourceNameList.add(datasource.getName());
});
// Check if both suffixed and newly imported datasource are present
assertThat(datasourceNameList).contains(datasourceName, datasourceName + " #1");
assertThat(actionList).isNotEmpty();
actionList.forEach(newAction -> {
ActionDTO actionDTO = newAction.getUnpublishedAction();
assertThat(actionDTO.getDatasource()).isNotNull();
});
})
.verifyComplete();
}
@Test
@WithUserDetails(value = "api_user")
public void importApplication_datasourceWithSameNameAndPlugin_importedWithValidActionsWithoutSuffixedDatasource() {
ApplicationJson applicationJson = createAppJson("test_assets/ImportExportServiceTest/valid-application.json").block();
Organization testOrganization = new Organization();
testOrganization.setName("Duplicate datasource with same plugin org");
testOrganization = organizationService.create(testOrganization).block();
Datasource testDatasource = new Datasource();
// Chose plugin same as mongo, as json static file has mongo plugin for datasource
Plugin postgreSQLPlugin = pluginRepository.findByName("MongoDB").block();
testDatasource.setPluginId(postgreSQLPlugin.getId());
testDatasource.setOrganizationId(testOrganization.getId());
final String datasourceName = applicationJson.getDatasourceList().get(0).getName();
testDatasource.setName(datasourceName);
datasourceService.create(testDatasource).block();
final Mono<Application> resultMono = importExportApplicationService.importApplicationInOrganization(testOrganization.getId(), applicationJson);
StepVerifier
.create(resultMono
.flatMap(application -> Mono.zip(
Mono.just(application),
datasourceService.findAllByOrganizationId(application.getOrganizationId(), MANAGE_DATASOURCES).collectList(),
newActionService.findAllByApplicationIdAndViewMode(application.getId(), false, READ_ACTIONS, null).collectList()
)))
.assertNext(tuple -> {
final Application application = tuple.getT1();
final List<Datasource> datasourceList = tuple.getT2();
final List<NewAction> actionList = tuple.getT3();
assertThat(application.getName()).isEqualTo("valid_application");
List<String> datasourceNameList = new ArrayList<>();
assertThat(datasourceList).isNotEmpty();
datasourceList.forEach(datasource -> {
assertThat(datasource.getOrganizationId()).isEqualTo(application.getOrganizationId());
datasourceNameList.add(datasource.getName());
});
// Check that there are no datasources are created with suffix names as datasource's are of same plugin
assertThat(datasourceNameList).contains(datasourceName);
assertThat(actionList).isNotEmpty();
actionList.forEach(newAction -> {
ActionDTO actionDTO = newAction.getUnpublishedAction();
assertThat(actionDTO.getDatasource()).isNotNull();
});
})
.verifyComplete();
}
} }