From 475f0dbd74ff5c468fd2075120069d4b7b95cfc6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Csneha122=E2=80=9D?= <“sneha@appsmith.com”> Date: Wed, 21 Aug 2024 20:56:03 +0530 Subject: [PATCH 1/2] fix: moved js filter to mongoDB --- .../newactions/base/NewActionServiceCE.java | 3 ++ .../base/NewActionServiceCEImpl.java | 39 +++++++++++++++++-- .../ce/CustomNewActionRepositoryCE.java | 3 ++ .../ce/CustomNewActionRepositoryCEImpl.java | 25 +++++++++--- 4 files changed, 61 insertions(+), 9 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCE.java index 7b2c4b71384..b52362f4268 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCE.java @@ -78,6 +78,9 @@ Flux findAllByApplicationIdAndViewMode( Flux findAllByApplicationIdAndViewMode( String applicationId, Boolean viewMode, Optional permission, Optional sort); + Flux findAllByApplicationIdAndPluginType( + String applicationId, Boolean viewMode, AclPermission permission, Sort sort, List pluginTypes); + Flux getActionsForViewMode(String applicationId); ActionViewDTO generateActionViewDTO(NewAction action, ActionDTO actionDTO, boolean viewMode); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java index 952a7605d80..bbc8d30cd28 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java @@ -734,10 +734,10 @@ public Flux findByPageIdAndViewMode(String pageId, Boolean viewMode, } @Override - public Flux findAllByApplicationIdAndViewMode( - String applicationId, Boolean viewMode, AclPermission permission, Sort sort) { + public Flux findAllByApplicationIdAndPluginType( + String applicationId, Boolean viewMode, AclPermission permission, Sort sort, List pluginTypes) { return repository - .findByApplicationId(applicationId, permission, sort) + .findByApplicationIdAndPluginType(applicationId, pluginTypes, permission, sort) .name(VIEW_MODE_FETCH_ACTIONS_FROM_DB) .tap(Micrometer.observation(observationRegistry)) // In case of view mode being true, filter out all the actions which haven't been published @@ -761,6 +761,28 @@ public Flux findAllByApplicationIdAndViewMode( .tap(Micrometer.observation(observationRegistry)); } + @Override + public Flux findAllByApplicationIdAndViewMode( + String applicationId, Boolean viewMode, AclPermission permission, Sort sort) { + return repository + .findByApplicationId(applicationId, permission, sort) + // In case of view mode being true, filter out all the actions which haven't been published + .flatMap(action -> { + if (Boolean.TRUE.equals(viewMode)) { + // In case we are trying to fetch published actions but this action has not been published, do + // not return + if (action.getPublishedAction() == null) { + return Mono.empty(); + } + } + // No need to handle the edge case of unpublished action not being present. This is not possible + // because every created action starts from an unpublishedAction state. + + return Mono.just(action); + }) + .flatMap(this::sanitizeAction); + } + @Override public Flux findAllByApplicationIdAndViewMode( String applicationId, Boolean viewMode, Optional permission, Optional sort) { @@ -792,9 +814,18 @@ public Flux getActionsForViewMode(String applicationId) { return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.APPLICATION_ID)); } + List pluginTypes = List.of( + PluginType.DB.toString(), + PluginType.API.toString(), + PluginType.SAAS.toString(), + PluginType.REMOTE.toString(), + PluginType.AI.toString(), + PluginType.INTERNAL.toString()); + // fetch the published actions by applicationId // No need to sort the results - return findAllByApplicationIdAndViewMode(applicationId, true, actionPermission.getExecutePermission(), null) + return findAllByApplicationIdAndPluginType( + applicationId, true, actionPermission.getExecutePermission(), null, pluginTypes) .name(VIEW_MODE_INITIAL_ACTION) .tap(Micrometer.observation(observationRegistry)) .filter(newAction -> !PluginType.JS.equals(newAction.getPluginType())) 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 47957edb548..6026e79bc62 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 @@ -37,6 +37,9 @@ Flux findAllActionsByNameAndPageIdsAndViewMode( Flux findByApplicationId(String applicationId, AclPermission aclPermission, Sort sort); + Flux findByApplicationIdAndPluginType( + String applicationId, List pluginTypes, AclPermission aclPermission, Sort sort); + Flux findByApplicationId( String applicationId, Optional aclPermission, Optional sort); 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 37de19d0f3d..54ea14d9eef 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 @@ -22,7 +22,6 @@ import org.springframework.data.mongodb.core.aggregation.MatchOperation; import org.springframework.data.mongodb.core.aggregation.ProjectionOperation; import org.springframework.data.mongodb.core.query.Criteria; -import reactor.core.observability.micrometer.Micrometer; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; @@ -31,7 +30,6 @@ import java.util.Optional; import java.util.Set; -import static com.appsmith.external.constants.spans.ce.ActionSpanCE.VIEW_MODE_FETCH_ACTIONS_FROM_DB_QUERY; import static org.springframework.data.mongodb.core.aggregation.Aggregation.group; import static org.springframework.data.mongodb.core.aggregation.Aggregation.match; import static org.springframework.data.mongodb.core.aggregation.Aggregation.newAggregation; @@ -52,9 +50,7 @@ public Flux findByApplicationId(String applicationId, AclPermission a return queryBuilder() .criteria(getCriterionForFindByApplicationId(applicationId)) .permission(aclPermission) - .all() - .name(VIEW_MODE_FETCH_ACTIONS_FROM_DB_QUERY) - .tap(Micrometer.observation(observationRegistry)); + .all(); } @Override @@ -206,6 +202,25 @@ protected BridgeQuery getCriterionForFindByApplicationId(String appli return Bridge.equal(NewAction.Fields.applicationId, applicationId); } + @Override + public Flux findByApplicationIdAndPluginType( + String applicationId, List pluginTypes, AclPermission aclPermission, Sort sort) { + return queryBuilder() + .criteria(getCriterionForFindByApplicationIdAndPluginType(applicationId, pluginTypes)) + .permission(aclPermission) + .sort(sort) + .all(); + } + + protected BridgeQuery getCriterionForFindByApplicationIdAndPluginType( + String applicationId, List pluginTypes) { + final BridgeQuery q = getCriterionForFindByApplicationId(applicationId); + q.and(Bridge.or( + Bridge.in(NewAction.Fields.pluginType, pluginTypes), Bridge.isNull(NewAction.Fields.pluginType))); + + return q; + } + @Override public Flux findByApplicationIdAndViewMode( String applicationId, Boolean viewMode, AclPermission aclPermission) { From f0cf9b23c1ca80eb652c5e3a5e3502fdb4dcab59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Csneha122=E2=80=9D?= <“sneha@appsmith.com”> Date: Fri, 23 Aug 2024 13:31:41 +0530 Subject: [PATCH 2/2] fix: code review changes done --- .../base/NewActionServiceCEImpl.java | 60 ++++++++----------- .../ce/CustomNewActionRepositoryCEImpl.java | 9 +-- 2 files changed, 29 insertions(+), 40 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java index bbc8d30cd28..127254e2600 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/base/NewActionServiceCEImpl.java @@ -735,25 +735,17 @@ public Flux findByPageIdAndViewMode(String pageId, Boolean viewMode, @Override public Flux findAllByApplicationIdAndPluginType( - String applicationId, Boolean viewMode, AclPermission permission, Sort sort, List pluginTypes) { + String applicationId, + Boolean viewMode, + AclPermission permission, + Sort sort, + List excludedPluginTypes) { return repository - .findByApplicationIdAndPluginType(applicationId, pluginTypes, permission, sort) + .findByApplicationIdAndPluginType(applicationId, excludedPluginTypes, permission, sort) .name(VIEW_MODE_FETCH_ACTIONS_FROM_DB) .tap(Micrometer.observation(observationRegistry)) // In case of view mode being true, filter out all the actions which haven't been published - .flatMap(action -> { - if (Boolean.TRUE.equals(viewMode)) { - // In case we are trying to fetch published actions but this action has not been published, do - // not return - if (action.getPublishedAction() == null) { - return Mono.empty(); - } - } - // No need to handle the edge case of unpublished action not being present. This is not possible - // because every created action starts from an unpublishedAction state. - - return Mono.just(action); - }) + .flatMap(action -> this.filterAction(action, viewMode)) .name(VIEW_MODE_FILTER_ACTION) .tap(Micrometer.observation(observationRegistry)) .flatMap(this::sanitizeAction) @@ -767,19 +759,7 @@ public Flux findAllByApplicationIdAndViewMode( return repository .findByApplicationId(applicationId, permission, sort) // In case of view mode being true, filter out all the actions which haven't been published - .flatMap(action -> { - if (Boolean.TRUE.equals(viewMode)) { - // In case we are trying to fetch published actions but this action has not been published, do - // not return - if (action.getPublishedAction() == null) { - return Mono.empty(); - } - } - // No need to handle the edge case of unpublished action not being present. This is not possible - // because every created action starts from an unpublishedAction state. - - return Mono.just(action); - }) + .flatMap(action -> this.filterAction(action, viewMode)) .flatMap(this::sanitizeAction); } @@ -814,18 +794,12 @@ public Flux getActionsForViewMode(String applicationId) { return Flux.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.APPLICATION_ID)); } - List pluginTypes = List.of( - PluginType.DB.toString(), - PluginType.API.toString(), - PluginType.SAAS.toString(), - PluginType.REMOTE.toString(), - PluginType.AI.toString(), - PluginType.INTERNAL.toString()); + List excludedPluginTypes = List.of(PluginType.JS.toString()); // fetch the published actions by applicationId // No need to sort the results return findAllByApplicationIdAndPluginType( - applicationId, true, actionPermission.getExecutePermission(), null, pluginTypes) + applicationId, true, actionPermission.getExecutePermission(), null, excludedPluginTypes) .name(VIEW_MODE_INITIAL_ACTION) .tap(Micrometer.observation(observationRegistry)) .filter(newAction -> !PluginType.JS.equals(newAction.getPluginType())) @@ -1114,6 +1088,20 @@ public Mono sanitizeAction(NewAction action) { return actionMono; } + public Mono filterAction(NewAction action, Boolean viewMode) { + if (Boolean.TRUE.equals(viewMode)) { + // In case we are trying to fetch published actions but this action has not been published, do + // not return + if (action.getPublishedAction() == null) { + return Mono.empty(); + } + } + // No need to handle the edge case of unpublished action not being present. This is not possible + // because every created action starts from an unpublishedAction state. + + return Mono.just(action); + } + public Flux addMissingPluginDetailsIntoAllActions(List actionList) { Mono> pluginMapMono = Mono.just(defaultPluginMap); 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 54ea14d9eef..5913e10ced0 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 @@ -204,19 +204,20 @@ protected BridgeQuery getCriterionForFindByApplicationId(String appli @Override public Flux findByApplicationIdAndPluginType( - String applicationId, List pluginTypes, AclPermission aclPermission, Sort sort) { + String applicationId, List excludedPluginTypes, AclPermission aclPermission, Sort sort) { return queryBuilder() - .criteria(getCriterionForFindByApplicationIdAndPluginType(applicationId, pluginTypes)) + .criteria(getCriterionForFindByApplicationIdAndPluginType(applicationId, excludedPluginTypes)) .permission(aclPermission) .sort(sort) .all(); } protected BridgeQuery getCriterionForFindByApplicationIdAndPluginType( - String applicationId, List pluginTypes) { + String applicationId, List excludedPluginTypes) { final BridgeQuery q = getCriterionForFindByApplicationId(applicationId); q.and(Bridge.or( - Bridge.in(NewAction.Fields.pluginType, pluginTypes), Bridge.isNull(NewAction.Fields.pluginType))); + Bridge.notIn(NewAction.Fields.pluginType, excludedPluginTypes), + Bridge.isNull(NewAction.Fields.pluginType))); return q; }