Skip to content

chore: Convert non-reactive methods consuming @FeatureFlagged to reactive #39594

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
abhvsn merged 2 commits into from
Mar 7, 2025
Merged

chore: Convert non-reactive methods consuming @FeatureFlagged to reactive #39594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
220d5d9
chore: Convert non-reactive methods consuming @FeatureFlagged to reac…
abhvsn Mar 6, 2025
d8ade4d
fix: Concurrency issue in import flow
abhvsn Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions ...c/main/java/com/appsmith/server/actioncollections/base/ActionCollectionServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,13 +280,16 @@ public Mono<ActionCollectionDTO> update(String id, ActionCollectionDTO actionCol

@Override
public Mono<ActionCollectionDTO> deleteWithoutPermissionUnpublishedActionCollection(String id) {
return deleteUnpublishedActionCollection(id, null, actionPermission.getDeletePermission());
return actionPermission
.getDeletePermission()
.flatMap(permission -> deleteUnpublishedActionCollection(id, null, permission));
}

@Override
public Mono<ActionCollectionDTO> deleteUnpublishedActionCollection(String id) {
return deleteUnpublishedActionCollection(
id, actionPermission.getDeletePermission(), actionPermission.getDeletePermission());
return actionPermission
.getDeletePermission()
.flatMap(permission -> deleteUnpublishedActionCollection(id, permission, permission));
}

@Override
Expand Down Expand Up @@ -418,10 +421,12 @@ public Mono<ActionCollection> archiveById(String id) {
}

protected Mono<ActionCollection> archiveGivenActionCollection(ActionCollection actionCollection) {
Flux<NewAction> unpublishedJsActionsFlux = newActionService.findByCollectionIdAndViewMode(
actionCollection.getId(), false, actionPermission.getDeletePermission());
Flux<NewAction> publishedJsActionsFlux = newActionService.findByCollectionIdAndViewMode(
actionCollection.getId(), true, actionPermission.getDeletePermission());
Mono<AclPermission> deleteActionPermissionMono =
actionPermission.getDeletePermission().cache();
Flux<NewAction> unpublishedJsActionsFlux = deleteActionPermissionMono.flatMapMany(permission ->
newActionService.findByCollectionIdAndViewMode(actionCollection.getId(), false, permission));
Flux<NewAction> publishedJsActionsFlux = deleteActionPermissionMono.flatMapMany(permission ->
newActionService.findByCollectionIdAndViewMode(actionCollection.getId(), true, permission));
return unpublishedJsActionsFlux
.mergeWith(publishedJsActionsFlux)
.flatMap(toArchive -> newActionService
Expand Down
175 changes: 97 additions & 78 deletions ...appsmith/server/actioncollections/importable/ActionCollectionImportableServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.apache.commons.collections.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.springframework.stereotype.Service;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

import java.util.ArrayList;
Expand Down Expand Up @@ -172,84 +173,102 @@ private Mono<ImportActionCollectionResultDTO> importActionCollections(
// collections
resultDTO.setExistingActionCollections(actionsCollectionsInCurrentArtifact.values());

List<ActionCollection> newActionCollections = new ArrayList<>();
List<ActionCollection> existingActionCollections = new ArrayList<>();

for (ActionCollection actionCollection : importedActionCollectionList) {
final ActionCollectionDTO unpublishedCollection =
actionCollection.getUnpublishedCollection();
if (unpublishedCollection == null
|| StringUtils.isEmpty(unpublishedCollection.calculateContextId())) {
continue; // invalid action collection, skip it
}

String idFromJsonFile = actionCollection.getId();

ActionCollection branchedActionCollection = null;

if (actionsCollectionsInBranches.containsKey(actionCollection.getGitSyncId())) {
branchedActionCollection =
artifactBasedImportableService
.getExistingEntityInOtherBranchForImportedEntity(
mappedImportableResourcesDTO,
actionsCollectionsInBranches,
actionCollection);
}

Context baseContext = populateIdReferencesAndReturnBaseContext(
importingMetaDTO,
mappedImportableResourcesDTO,
artifact,
branchedActionCollection,
actionCollection);

// Check if the action has gitSyncId and if it's already in DB
if (existingArtifactContainsCollection(
actionsCollectionsInCurrentArtifact, actionCollection)) {

// Since the resource is already present in DB, just update resource
ActionCollection existingActionCollection =
artifactBasedImportableService
.getExistingEntityInCurrentBranchForImportedEntity(
mappedImportableResourcesDTO,
actionsCollectionsInCurrentArtifact,
actionCollection);

updateExistingCollection(
importingMetaDTO,
mappedImportableResourcesDTO,
actionCollection,
existingActionCollection);

existingActionCollections.add(existingActionCollection);
resultDTO.getSavedActionCollectionIds().add(existingActionCollection.getId());
resultDTO
.getSavedActionCollectionMap()
.put(idFromJsonFile, existingActionCollection);
} else {
artifactBasedImportableService.createNewResource(
importingMetaDTO, actionCollection, baseContext);

populateDomainMappedReferences(mappedImportableResourcesDTO, actionCollection);

// it's new actionCollection
newActionCollections.add(actionCollection);
resultDTO.getSavedActionCollectionIds().add(actionCollection.getId());
resultDTO.getSavedActionCollectionMap().put(idFromJsonFile, actionCollection);
}
}
log.info(
"Saving action collections in bulk. New: {}, Updated: {}",
newActionCollections.size(),
existingActionCollections.size());
return Mono.when(
actionCollectionService
.bulkValidateAndInsertActionCollectionInRepository(
newActionCollections),
actionCollectionService
.bulkValidateAndUpdateActionCollectionInRepository(
existingActionCollections))
.thenReturn(resultDTO);
// Use a synchronised list to avoid concurrent modification exception
List<ActionCollection> newActionCollections =
Collections.synchronizedList(new ArrayList<>());
List<ActionCollection> existingActionCollections =
Collections.synchronizedList(new ArrayList<>());

return Flux.fromIterable(importedActionCollectionList)
.flatMap(actionCollection -> {
final ActionCollectionDTO unpublishedCollection =
actionCollection.getUnpublishedCollection();
if (unpublishedCollection == null
|| StringUtils.isEmpty(
unpublishedCollection.calculateContextId())) {
return Mono.empty(); // invalid action collection, skip it
}

String idFromJsonFile = actionCollection.getId();

ActionCollection branchedActionCollection = null;

if (actionsCollectionsInBranches.containsKey(
actionCollection.getGitSyncId())) {
branchedActionCollection =
artifactBasedImportableService
.getExistingEntityInOtherBranchForImportedEntity(
mappedImportableResourcesDTO,
actionsCollectionsInBranches,
actionCollection);
}

Context baseContext = populateIdReferencesAndReturnBaseContext(
importingMetaDTO,
mappedImportableResourcesDTO,
artifact,
branchedActionCollection,
actionCollection);

// Check if the action has gitSyncId and if it's already in DB
if (existingArtifactContainsCollection(
actionsCollectionsInCurrentArtifact, actionCollection)) {

// Since the resource is already present in DB, just update resource
ActionCollection existingActionCollection =
artifactBasedImportableService
.getExistingEntityInCurrentBranchForImportedEntity(
mappedImportableResourcesDTO,
actionsCollectionsInCurrentArtifact,
actionCollection);

updateExistingCollection(
importingMetaDTO,
mappedImportableResourcesDTO,
actionCollection,
existingActionCollection);

existingActionCollections.add(existingActionCollection);
resultDTO
.getSavedActionCollectionIds()
.add(existingActionCollection.getId());
resultDTO
.getSavedActionCollectionMap()
.put(idFromJsonFile, existingActionCollection);
return Mono.just(existingActionCollection);
}
return artifactBasedImportableService
.createNewResource(importingMetaDTO, actionCollection, baseContext)
.flatMap(updatedActionCollection -> {
// populate the domain mapped references
populateDomainMappedReferences(
mappedImportableResourcesDTO, updatedActionCollection);

// it's new actionCollection
newActionCollections.add(updatedActionCollection);
resultDTO
.getSavedActionCollectionIds()
.add(updatedActionCollection.getId());
resultDTO
.getSavedActionCollectionMap()
.put(idFromJsonFile, updatedActionCollection);
return Mono.just(updatedActionCollection);
});
})
.then(Mono.defer(() -> {
log.info(
"Saving action collections in bulk. New: {}, Updated: {}",
newActionCollections.size(),
existingActionCollections.size());
return Mono.when(
actionCollectionService
.bulkValidateAndInsertActionCollectionInRepository(
newActionCollections),
actionCollectionService
.bulkValidateAndUpdateActionCollectionInRepository(
existingActionCollections))
.thenReturn(resultDTO);
}));
});
})
.onErrorResume(e -> {
Expand Down
40 changes: 23 additions & 17 deletions ...llections/importable/applications/ActionCollectionApplicationImportableServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.apache.commons.lang3.StringUtils;
import org.springframework.stereotype.Service;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;

import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -88,26 +89,31 @@ public Context updateContextInResource(
}

@Override
public void createNewResource(
public Mono<ActionCollection> createNewResource(
ImportingMetaDTO importingMetaDTO, ActionCollection actionCollection, Context baseContext) {
if (!importingMetaDTO.getPermissionProvider().canCreateAction((NewPage) baseContext)) {
throw new AppsmithException(
AppsmithError.ACL_NO_RESOURCE_FOUND, FieldName.PAGE, ((NewPage) baseContext).getId());
}
Mono<Boolean> canCreateAction = importingMetaDTO.getPermissionProvider().canCreateAction((NewPage) baseContext);
return canCreateAction.flatMap(canCreate -> {
if (!canCreate) {
return Mono.error(new AppsmithException(
AppsmithError.ACL_NO_RESOURCE_FOUND, FieldName.PAGE, ((NewPage) baseContext).getId()));
}

// this will generate the id and other auto generated fields e.g. createdAt
actionCollection.updateForBulkWriteOperation();
actionCollectionService.generateAndSetPolicies((NewPage) baseContext, actionCollection);
// this will generate the id and other auto generated fields e.g. createdAt
actionCollection.updateForBulkWriteOperation();
actionCollectionService.generateAndSetPolicies((NewPage) baseContext, actionCollection);

// create or update base id for the action
// values already set to base id are kept unchanged
actionCollection.setBaseId(actionCollection.getBaseIdOrFallback());
actionCollection.setRefType(importingMetaDTO.getRefType());
actionCollection.setRefName(importingMetaDTO.getRefName());
// create or update base id for the action
// values already set to base id are kept unchanged
actionCollection.setBaseId(actionCollection.getBaseIdOrFallback());
actionCollection.setRefType(importingMetaDTO.getRefType());
actionCollection.setRefName(importingMetaDTO.getRefName());

// generate gitSyncId if it's not present
if (actionCollection.getGitSyncId() == null) {
actionCollection.setGitSyncId(actionCollection.getApplicationId() + "_" + UUID.randomUUID());
}
// generate gitSyncId if it's not present
if (actionCollection.getGitSyncId() == null) {
actionCollection.setGitSyncId(actionCollection.getApplicationId() + "_" + UUID.randomUUID());
}

return Mono.just(actionCollection);
});
}
}
7 changes: 3 additions & 4 deletions ...-server/src/main/java/com/appsmith/server/applications/base/ApplicationServiceCEImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,10 +593,9 @@ protected List<Mono<Void>> updatePoliciesForIndependentDomains(
// the same level in the hierarchy. A user may have permission to change view on application, but
// may not have explicit permissions on the datasource.
Mono<Void> updatedDatasourcesMono = datasourceIdsMono
.flatMapMany(datasourceIds -> {
return policySolution.updateWithNewPoliciesToDatasourcesByDatasourceIdsWithoutPermission(
datasourceIds, datasourcePolicyMap, addViewAccess);
})
.flatMapMany(datasourceIds ->
policySolution.updateWithNewPoliciesToDatasourcesByDatasourceIdsWithoutPermission(
datasourceIds, datasourcePolicyMap, addViewAccess))
.then();

list.add(updatedDatasourcesMono);
Expand Down
Loading