diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java index daa93bea45..f9b9d21ac7 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java @@ -277,29 +277,10 @@ public class DatasourceServiceCEImpl implements DatasourceServiceCE { // This is meant to be an update for just the datasource - like a renamed return datasourceMono .map(datasourceInDb -> { - // check whether name is going to be updated - boolean isRenamed = StringUtils.hasLength(datasource.getName()) - && !datasource.getName().equals(datasourceInDb.getName()); copyNestedNonNullProperties(datasource, datasourceInDb); - return Tuples.of(datasourceInDb, isRenamed); - }) - .flatMap(tuples -> { - Datasource datasourceInDb = tuples.getT1(); - boolean isRenamed = tuples.getT2(); - return validateAndSaveDatasourceToRepository(datasourceInDb).flatMap(savedDatasource -> { - if (isRenamed) { - /* - if name updated, we've to set the updatedAt date for the related actions. - Because in git file system, actions are mapped with a datasource by the name. - If updatedAt changed, git will remap the updated actions with new datasource name. - */ - return newActionRepository - .updateDatasourceNameInActions(savedDatasource) - .thenReturn(savedDatasource); - } - return Mono.just(savedDatasource); - }); + return datasourceInDb; }) + .flatMap(this::validateAndSaveDatasourceToRepository) .map(savedDatasource -> { // not required by client side in order to avoid updating it to a null storage, // one alternative is that we find and send datasourceStorages along, but that is an expensive call diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java index d3d2807556..b1493ac79a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java @@ -11,6 +11,7 @@ import org.springframework.data.mongodb.MongoTransactionException; import org.springframework.transaction.TransactionException; import org.springframework.util.CollectionUtils; +import java.time.Instant; import java.util.Map; import java.util.Set; @@ -131,4 +132,15 @@ public class ImportExportUtils { } return pageName != null && updatedPageNames.contains(pageName); } + + public static boolean isDatasourceUpdatedSinceLastCommit( + Map datasourceNameToUpdatedAtMap, + ActionDTO actionDTO, + Instant applicationLastCommittedAt) { + String datasourceName = actionDTO.getDatasource().getId(); + Instant datasourceUpdatedAt = datasourceName != null ? datasourceNameToUpdatedAtMap.get(datasourceName) : null; + return datasourceUpdatedAt != null + && applicationLastCommittedAt != null + && datasourceUpdatedAt.isAfter(applicationLastCommittedAt); + } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java index ee1a842fa5..c3bda9cc5a 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java @@ -1,6 +1,5 @@ package com.appsmith.server.repositories.ce; -import com.appsmith.external.models.Datasource; import com.appsmith.server.acl.AclPermission; import com.appsmith.server.domains.NewAction; import com.appsmith.server.dtos.PluginTypeAndCountDTO; @@ -82,8 +81,6 @@ public interface CustomNewActionRepositoryCE extends AppsmithRepository archiveDeletedUnpublishedActions(String applicationId, AclPermission permission); - Mono updateDatasourceNameInActions(Datasource datasource); - Flux countActionsByPluginType(String applicationId); Flux findAllByApplicationIdsWithoutPermission(List applicationIds, List includeFields); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java index f516eb219a..840c0dfe6b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java @@ -1,6 +1,5 @@ package com.appsmith.server.repositories.ce; -import com.appsmith.external.models.Datasource; import com.appsmith.external.models.PluginType; import com.appsmith.external.models.QActionConfiguration; import com.appsmith.external.models.QBranchAwareDomain; @@ -644,31 +643,4 @@ public class CustomNewActionRepositoryCEImpl extends BaseAppsmithRepositoryImpl< Criteria applicationCriteria = Criteria.where(FieldName.APPLICATION_ID).in(applicationIds); return queryAll(List.of(applicationCriteria), includeFields, null, null, NO_RECORD_LIMIT); } - - /** - * Sets the datasource.name and updatedAt fields of newAction as per the provided datasource. The actions which - * have unpublishedAction.datasource.id same as the provided datasource.id will be updated. - * @param datasource The datasource object - * @return result of the multi update query - */ - @Override - public Mono updateDatasourceNameInActions(Datasource datasource) { - String unpublishedActionDatasourceIdFieldPath = String.format( - "%s.%s.%s", - fieldName(QNewAction.newAction.unpublishedAction), - fieldName(QNewAction.newAction.unpublishedAction.datasource), - fieldName(QNewAction.newAction.unpublishedAction.datasource.id)); - - String unpublishedActionDatasourceNameFieldPath = String.format( - "%s.%s.%s", - fieldName(QNewAction.newAction.unpublishedAction), - fieldName(QNewAction.newAction.unpublishedAction.datasource), - fieldName(QNewAction.newAction.unpublishedAction.datasource.name)); - - Criteria criteria = where(unpublishedActionDatasourceIdFieldPath).is(datasource.getId()); - Update update = new Update(); - update.set(FieldName.UPDATED_AT, datasource.getUpdatedAt()); - update.set(unpublishedActionDatasourceNameFieldPath, datasource.getName()); - return updateByCriteria(List.of(criteria), update, null); - } } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ImportExportApplicationServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ImportExportApplicationServiceCEImpl.java index 0d4b79f4a2..00d0e816b9 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ImportExportApplicationServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/solutions/ce/ImportExportApplicationServiceCEImpl.java @@ -175,6 +175,7 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica ApplicationJson applicationJson = new ApplicationJson(); Map pluginMap = new HashMap<>(); Map datasourceIdToNameMap = new HashMap<>(); + Map datasourceNameToUpdatedAtMap = new HashMap<>(); Map pageIdToNameMap = new HashMap<>(); Map actionIdToNameMap = new HashMap<>(); Map collectionIdToNameMap = new HashMap<>(); @@ -397,8 +398,11 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica .flatMapMany(tuple2 -> { List datasourceList = tuple2.getT1(); String environmentId = tuple2.getT2(); - datasourceList.forEach(datasource -> - datasourceIdToNameMap.put(datasource.getId(), datasource.getName())); + datasourceList.forEach(datasource -> { + datasourceIdToNameMap.put(datasource.getId(), datasource.getName()); + datasourceNameToUpdatedAtMap.put(datasource.getName(), datasource.getUpdatedAt()); + }); + List storageList = datasourceList.stream() .map(datasource -> { DatasourceStorage storage = @@ -578,6 +582,10 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica : null; // TODO: check whether resource updated after last commit - move to a function String pageName = actionDTO.getPageId(); + // we've replaced the datasource id with datasource name in previous step + boolean isDatasourceUpdated = ImportExportUtils.isDatasourceUpdatedSinceLastCommit( + datasourceNameToUpdatedAtMap, actionDTO, applicationLastCommittedAt); + boolean isPageUpdated = ImportExportUtils.isPageNameInUpdatedList(applicationJson, pageName); Instant newActionUpdatedAt = newAction.getUpdatedAt(); @@ -585,6 +593,7 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica || isServerSchemaMigrated || applicationLastCommittedAt == null || isPageUpdated + || isDatasourceUpdated || newActionUpdatedAt == null || applicationLastCommittedAt.isBefore(newActionUpdatedAt); if (isNewActionUpdated && newActionName != null) { diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ImportExportUtilsTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ImportExportUtilsTest.java index b2a7924917..cd0e40bb55 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ImportExportUtilsTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ImportExportUtilsTest.java @@ -1,5 +1,7 @@ package com.appsmith.server.helpers; +import com.appsmith.external.models.ActionDTO; +import com.appsmith.external.models.Datasource; import com.appsmith.server.constants.FieldName; import com.appsmith.server.dtos.ApplicationJson; import com.appsmith.server.exceptions.AppsmithError; @@ -8,6 +10,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.springframework.data.mongodb.MongoTransactionException; +import java.time.Instant; import java.util.Map; import java.util.Set; @@ -47,4 +50,36 @@ class ImportExportUtilsTest { Assertions.assertFalse(ImportExportUtils.isPageNameInUpdatedList(applicationJson, "")); Assertions.assertFalse(ImportExportUtils.isPageNameInUpdatedList(applicationJson, null)); } + + @Test + public void isDatasourceUpdatedSinceLastCommit() { + Map map = Map.of("Datasource1", Instant.now()); + ActionDTO actionDTO = new ActionDTO(); + actionDTO.setDatasource(new Datasource()); + // should return true if datasource has no id set + Assertions.assertFalse(ImportExportUtils.isDatasourceUpdatedSinceLastCommit( + map, actionDTO, Instant.now().minusSeconds(10))); + + actionDTO.getDatasource().setName("Datasource1"); + // should return false if datasource has name set but no id + Assertions.assertFalse(ImportExportUtils.isDatasourceUpdatedSinceLastCommit( + map, actionDTO, Instant.now().minusSeconds(10))); + + actionDTO.getDatasource().setId("Datasource2"); + // should return false if datasource id does not exist in the map + Assertions.assertFalse(ImportExportUtils.isDatasourceUpdatedSinceLastCommit( + map, actionDTO, Instant.now().minusSeconds(10))); + + actionDTO.getDatasource().setId("Datasource1"); + // should return true if datasource has name set but no id + Assertions.assertTrue(ImportExportUtils.isDatasourceUpdatedSinceLastCommit( + map, actionDTO, Instant.now().minusSeconds(10))); + + // should return false if datasource was modified before last commit + Assertions.assertFalse(ImportExportUtils.isDatasourceUpdatedSinceLastCommit( + map, actionDTO, Instant.now().plusSeconds(10))); + + // should return false if last commit date is null + Assertions.assertFalse(ImportExportUtils.isDatasourceUpdatedSinceLastCommit(map, actionDTO, null)); + } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImplTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImplTest.java index 07e63cc47d..e8382a337a 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImplTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImplTest.java @@ -19,11 +19,8 @@ import reactor.test.StepVerifier; import reactor.util.function.Tuple2; import java.time.Instant; -import java.time.temporal.ChronoUnit; import java.util.ArrayList; -import java.util.Collection; import java.util.List; -import java.util.Map; import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; @@ -293,47 +290,4 @@ public class CustomNewActionRepositoryCEImplTest { }) .verifyComplete(); } - - @Test - public void updateDatasourceNameInActions_WhenDatasourceIdMatched_MatchedObjectsUpdated() { - String uniqueString = UUID.randomUUID().toString(); - String datasourceOne = "ds-1-" + uniqueString, datasourceTwo = "ds-2-" + uniqueString; - String applicationId = "app-" + uniqueString; - - List newActions = List.of( - createActionWithDatasource(applicationId, datasourceOne), - createActionWithDatasource(applicationId, datasourceOne), - createActionWithDatasource(applicationId, datasourceTwo)); - - Datasource datasource = new Datasource(); - datasource.setId(datasourceOne); - datasource.setUpdatedAt(Instant.now().minusSeconds(60 * 60)); // 1 hour ago - - Mono>> newActionsByDatasourceIdMapMono = newActionRepository - .saveAll(newActions) - .then(newActionRepository.updateDatasourceNameInActions(datasource)) - .thenMany(newActionRepository.findByApplicationId(applicationId)) - .collectMultimap((newAction -> - newAction.getUnpublishedAction().getDatasource().getId())); - - StepVerifier.create(newActionsByDatasourceIdMapMono) - .assertNext(multiMap -> { - assertThat(multiMap.size()).isEqualTo(2); - assertThat(multiMap.get(datasourceOne).size()).isEqualTo(2); - assertThat(multiMap.get(datasourceTwo).size()).isEqualTo(1); - - multiMap.get(datasourceOne).forEach(newAction -> { - long diffInMinutes = - datasource.getUpdatedAt().until(newAction.getUpdatedAt(), ChronoUnit.MINUTES); - assertThat(diffInMinutes).isEqualTo(0); - }); - - multiMap.get(datasourceTwo).forEach(newAction -> { - long diffInMinutes = - datasource.getUpdatedAt().until(newAction.getUpdatedAt(), ChronoUnit.MINUTES); - assertThat(diffInMinutes).isGreaterThan(50); // should be at least 50 minutes ago - }); - }) - .verifyComplete(); - } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java index 17b4d160a6..3ac17236f2 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java @@ -21,7 +21,6 @@ import com.appsmith.server.constants.FieldName; import com.appsmith.server.datasources.base.DatasourceService; import com.appsmith.server.datasourcestorages.base.DatasourceStorageService; import com.appsmith.server.domains.Application; -import com.appsmith.server.domains.NewAction; import com.appsmith.server.domains.PermissionGroup; import com.appsmith.server.domains.Plugin; import com.appsmith.server.domains.User; @@ -57,13 +56,11 @@ import reactor.core.publisher.Mono; import reactor.test.StepVerifier; import reactor.util.function.Tuple2; -import java.time.Duration; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Set; -import java.util.UUID; import static com.appsmith.server.acl.AclPermission.DELETE_DATASOURCES; import static com.appsmith.server.acl.AclPermission.EXECUTE_DATASOURCES; @@ -2028,50 +2025,4 @@ public class DatasourceServiceTest { }) .verifyComplete(); } - - @Test - @WithUserDetails(value = "api_user") - public void updateDatasource_WhenNameUpdated_ActionUpdatedDateModified() { - Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())) - .thenReturn(Mono.just(new MockPluginExecutor())); - - String uniqueString = UUID.randomUUID().toString(); - String applicationId = "app_" + uniqueString; - - Workspace toCreate = new Workspace(); - toCreate.setName("workspace_" + uniqueString); - - Workspace workspace = workspaceService.create(toCreate).block(); - String workspaceId = workspace.getId(); - - Datasource datasource = createDatasource("D", workspaceId); - - // create a new action for this datasource - ActionDTO actionDTO = new ActionDTO(); - actionDTO.setDatasource(datasource); - NewAction newAction = new NewAction(); - newAction.setApplicationId(applicationId); - newAction.setUnpublishedAction(actionDTO); - - Mono createActionMono = newActionRepository.save(newAction); - Datasource updateDto = new Datasource(); - updateDto.setName(datasource.getName() + "#1"); - Mono updateDatasourceMono = - datasourceService.updateDatasource(datasource.getId(), updateDto, defaultEnvironmentId, true); - - Mono> tuple2Mono = createActionMono - .delayElement(Duration.ofSeconds(1)) - .then(updateDatasourceMono) - .then(newActionRepository.findByApplicationId(applicationId).last()) - .zipWhen(action -> datasourceService.findById( - action.getUnpublishedAction().getDatasource().getId())); - - StepVerifier.create(tuple2Mono) - .assertNext(tuples -> { - NewAction action = tuples.getT1(); - Datasource updatedDatasource = tuples.getT2(); - assertThat(action.getUpdatedAt()).isEqualTo(updatedDatasource.getUpdatedAt()); - }) - .verifyComplete(); - } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportExportApplicationServiceTests.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportExportApplicationServiceTests.java index 9c33e45d88..0f09a9dfbf 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportExportApplicationServiceTests.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/solutions/ImportExportApplicationServiceTests.java @@ -4864,4 +4864,89 @@ public class ImportExportApplicationServiceTests { }) .verifyComplete(); } + + @Test + @WithUserDetails("api_user") + public void exportApplicationByWhen_WhenGitConnectedAndDatasourceRenamed_QueriesAreInUpdatedResources() { + // create an application + Application testApplication = new Application(); + final String appName = UUID.randomUUID().toString(); + testApplication.setName(appName); + testApplication.setWorkspaceId(workspaceId); + testApplication.setUpdatedAt(Instant.now()); + testApplication.setLastDeployedAt(Instant.now()); + testApplication.setClientSchemaVersion(JsonSchemaVersions.clientVersion); + testApplication.setServerSchemaVersion(JsonSchemaVersions.serverVersion); + + Mono applicationJsonMono = applicationPageService + .createApplication(testApplication, workspaceId) + .flatMap(application -> { + // add a datasource to the workspace + Datasource ds1 = new Datasource(); + ds1.setName("DS_FOR_RENAME_TEST"); + ds1.setWorkspaceId(workspaceId); + ds1.setPluginId(installedPlugin.getId()); + final DatasourceConfiguration datasourceConfiguration = new DatasourceConfiguration(); + datasourceConfiguration.setUrl("http://example.org/get"); + datasourceConfiguration.setHeaders(List.of(new Property("X-Answer", "42"))); + + HashMap storages1 = new HashMap<>(); + storages1.put( + defaultEnvironmentId, + new DatasourceStorageDTO(null, defaultEnvironmentId, datasourceConfiguration)); + ds1.setDatasourceStorages(storages1); + return datasourceService.create(ds1).zipWith(Mono.just(application)); + }) + .flatMap(objects -> { + Datasource datasource = objects.getT1(); + Application application = objects.getT2(); + + // create an action with the datasource + ActionDTO action = new ActionDTO(); + ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + action.setActionConfiguration(actionConfiguration); + action.setDatasource(datasource); + action.setName("MyAction"); + action.setPageId(application.getPages().get(0).getId()); + return layoutActionService.createAction(action).thenReturn(objects); + }) + .flatMap(objects -> { + Application application = objects.getT2(); + // set git meta data for the application and set a last commit date + GitApplicationMetadata gitApplicationMetadata = new GitApplicationMetadata(); + // add buffer of 5 seconds so that the last commit date is definitely after the last updated date + gitApplicationMetadata.setLastCommittedAt(Instant.now()); + application.setGitApplicationMetadata(gitApplicationMetadata); + return applicationRepository.save(application).thenReturn(objects); + }) + .delayElement(Duration.ofMillis( + 100)) // to make sure the last commit date is definitely after the last updated date + .flatMap(objects -> { + // rename the datasource + Datasource datasource = objects.getT1(); + Application application = objects.getT2(); + datasource.setName("DS_FOR_RENAME_TEST_RENAMED"); + return datasourceService + .save(datasource) + .then(importExportApplicationService.exportApplicationById( + application.getId(), SerialiseApplicationObjective.VERSION_CONTROL)); + }); + + // verify that the exported json has the updated page name, and the queries are in the updated resources + StepVerifier.create(applicationJsonMono) + .assertNext(applicationJson -> { + Map> updatedResources = applicationJson.getUpdatedResources(); + assertThat(updatedResources).isNotNull(); + Set updatedActionNames = updatedResources.get(FieldName.ACTION_LIST); + assertThat(updatedActionNames).isNotNull(); + + // action should be present in the updated resources although action not updated but datasource is + assertThat(updatedActionNames.size()).isEqualTo(1); + updatedActionNames.forEach(actionName -> { + assertThat(actionName).contains("MyAction"); + }); + }) + .verifyComplete(); + } }