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

fix: Stop updating action when datasource name is updated (#27668)

Browse Source 
## Description

#### PR fixes following issue(s)
Fixes #27666
This commit is contained in:
Nayan 2023-10-04 18:37:26 +06:00 committed by GitHub
parent 703048b7b5
commit 0c258e20bb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 145 additions and 149 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
app/server/appsmith-server/src/main/java/com/appsmith/server/datasources/base/DatasourceServiceCEImpl.java
View File

@ -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

12
app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/ImportExportUtils.java
View File

@ -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<String, Instant> 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);
}
}

3
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCE.java
View File

@ -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<NewActio
Mono<UpdateResult> archiveDeletedUnpublishedActions(String applicationId, AclPermission permission);
Mono<UpdateResult> updateDatasourceNameInActions(Datasource datasource);
Flux<PluginTypeAndCountDTO> countActionsByPluginType(String applicationId);
Flux<NewAction> findAllByApplicationIdsWithoutPermission(List<String> applicationIds, List<String> includeFields);

28
app/server/appsmith-server/src/main/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImpl.java
View File

@ -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<UpdateResult> 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);
}
}

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

@ -175,6 +175,7 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica
ApplicationJson applicationJson = new ApplicationJson();
Map<String, String> pluginMap = new HashMap<>();
Map<String, String> datasourceIdToNameMap = new HashMap<>();
Map<String, Instant> datasourceNameToUpdatedAtMap = new HashMap<>();
Map<String, String> pageIdToNameMap = new HashMap<>();
Map<String, String> actionIdToNameMap = new HashMap<>();
Map<String, String> collectionIdToNameMap = new HashMap<>();
@ -397,8 +398,11 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica
.flatMapMany(tuple2 -> {
List<Datasource> 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<DatasourceStorage> 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) {

35
app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/ImportExportUtilsTest.java
View File

@ -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<String, Instant> 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));
}
}

46
app/server/appsmith-server/src/test/java/com/appsmith/server/repositories/ce/CustomNewActionRepositoryCEImplTest.java
View File

@ -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<NewAction> 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<Map<String, Collection<NewAction>>> 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();
}
}

49
app/server/appsmith-server/src/test/java/com/appsmith/server/services/DatasourceServiceTest.java
View File

@ -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<NewAction> createActionMono = newActionRepository.save(newAction);
Datasource updateDto = new Datasource();
updateDto.setName(datasource.getName() + "#1");
Mono<Datasource> updateDatasourceMono =
datasourceService.updateDatasource(datasource.getId(), updateDto, defaultEnvironmentId, true);
Mono<Tuple2<NewAction, Datasource>> 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();
}
}

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

@ -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<ApplicationJson> 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<String, DatasourceStorageDTO> 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<String, Set<String>> updatedResources = applicationJson.getUpdatedResources();
assertThat(updatedResources).isNotNull();
Set<String> 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();
}
}