diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/CreateActionMetaDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/CreateActionMetaDTO.java new file mode 100644 index 000000000000..667cd81e22b3 --- /dev/null +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/CreateActionMetaDTO.java @@ -0,0 +1,18 @@ +package com.appsmith.server.dtos; + +import com.appsmith.external.helpers.AppsmithEventContext; +import com.appsmith.server.domains.NewPage; +import lombok.AllArgsConstructor; +import lombok.Builder; +import lombok.Data; +import lombok.NoArgsConstructor; + +@Data +@AllArgsConstructor +@NoArgsConstructor +@Builder(toBuilder = true) +public class CreateActionMetaDTO { + Boolean isJsAction; + AppsmithEventContext eventContext; + NewPage newPage; +} diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/fork/internal/ApplicationForkingServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/fork/internal/ApplicationForkingServiceCEImpl.java index c05299753f87..48b66a44e1b8 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/fork/internal/ApplicationForkingServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/fork/internal/ApplicationForkingServiceCEImpl.java @@ -23,6 +23,7 @@ import com.appsmith.server.domains.Workspace; import com.appsmith.server.dtos.ActionCollectionDTO; import com.appsmith.server.dtos.ApplicationImportDTO; +import com.appsmith.server.dtos.CreateActionMetaDTO; import com.appsmith.server.dtos.ForkingMetaDTO; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.exceptions.AppsmithError; @@ -309,12 +310,16 @@ public Mono> forkApplications( actionDTO.setSource( ActionCreationSourceTypeEnum .FORK_APPLICATION); - return layoutActionService.createAction( - actionDTO, + + CreateActionMetaDTO createActionMetaDTO = + new CreateActionMetaDTO(); + createActionMetaDTO.setIsJsAction(Boolean.FALSE); + createActionMetaDTO.setEventContext( new AppsmithEventContext( AppsmithEventContextType - .CLONE_PAGE), - Boolean.FALSE); + .CLONE_PAGE)); + return layoutActionService.createAction( + actionDTO, createActionMetaDTO); }) .map(ActionDTO::getId), Mono.justOrEmpty(originalActionId)); diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/clonepage/ActionClonePageServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/clonepage/ActionClonePageServiceCEImpl.java index e8b71a426425..91990d715288 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/clonepage/ActionClonePageServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/newactions/clonepage/ActionClonePageServiceCEImpl.java @@ -7,6 +7,7 @@ import com.appsmith.server.clonepage.ClonePageServiceCE; import com.appsmith.server.domains.NewAction; import com.appsmith.server.dtos.ClonePageMetaDTO; +import com.appsmith.server.dtos.CreateActionMetaDTO; import com.appsmith.server.newactions.base.NewActionService; import com.appsmith.server.services.LayoutActionService; import com.appsmith.server.solutions.ActionPermission; @@ -54,7 +55,10 @@ public Mono cloneEntities(ClonePageMetaDTO clonePageMetaDTO) { // Indicates that source of action creation is clone page action cloneActionDTO.setSource(ActionCreationSourceTypeEnum.CLONE_PAGE); copyNestedNonNullProperties(actionDTO, cloneActionDTO); - return layoutActionService.createAction(cloneActionDTO, eventContext, isJsAction); + CreateActionMetaDTO createActionMetaDTO = new CreateActionMetaDTO(); + createActionMetaDTO.setIsJsAction(isJsAction); + createActionMetaDTO.setEventContext(eventContext); + return layoutActionService.createAction(cloneActionDTO, createActionMetaDTO); }) .then(); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java index c46abdc38a98..264c910545dd 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCE.java @@ -1,8 +1,8 @@ package com.appsmith.server.services.ce; -import com.appsmith.external.helpers.AppsmithEventContext; import com.appsmith.external.models.ActionDTO; import com.appsmith.server.dtos.ActionMoveDTO; +import com.appsmith.server.dtos.CreateActionMetaDTO; import reactor.core.publisher.Mono; public interface LayoutActionServiceCE { @@ -23,7 +23,7 @@ public interface LayoutActionServiceCE { Mono createSingleAction(ActionDTO actionDTO, Boolean isJsAction); - Mono createAction(ActionDTO actionDTO, AppsmithEventContext eventContext, Boolean isJsAction); + Mono createAction(ActionDTO actionDTO, CreateActionMetaDTO actionMetaDTO); Mono deleteUnpublishedAction(String id); } diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java index 9ddaad682f2a..6eef236d4def 100755 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/LayoutActionServiceCEImpl.java @@ -13,6 +13,7 @@ import com.appsmith.server.domains.NewAction; import com.appsmith.server.domains.NewPage; import com.appsmith.server.dtos.ActionMoveDTO; +import com.appsmith.server.dtos.CreateActionMetaDTO; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.exceptions.AppsmithError; import com.appsmith.server.exceptions.AppsmithException; @@ -337,25 +338,29 @@ public Mono createSingleAction(ActionDTO actionDTO, Boolean isJsActio if (!StringUtils.hasLength(actionDTO.getPageId())) { return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.PAGE_ID)); } + + AclPermission aclPermission = + isJsAction ? pagePermission.getReadPermission() : pagePermission.getActionCreatePermission(); + return newPageService - .findById(actionDTO.getPageId(), pagePermission.getActionCreatePermission()) + .findById(actionDTO.getPageId(), aclPermission) .switchIfEmpty(Mono.error( new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.PAGE, actionDTO.getPageId()))) .flatMap(newPage -> { actionDTO.setBranchName(newPage.getBranchName()); - return createAction(actionDTO, isJsAction); + AppsmithEventContext eventContext = new AppsmithEventContext(AppsmithEventContextType.DEFAULT); + CreateActionMetaDTO createActionMetaDTO = new CreateActionMetaDTO(); + createActionMetaDTO.setIsJsAction(isJsAction); + createActionMetaDTO.setNewPage(newPage); + createActionMetaDTO.setEventContext(eventContext); + return createAction(actionDTO, createActionMetaDTO); }); } - protected Mono createAction(ActionDTO actionDTO, Boolean isJsAction) { - AppsmithEventContext eventContext = new AppsmithEventContext(AppsmithEventContextType.DEFAULT); - return createAction(actionDTO, eventContext, isJsAction); - } - @Override - public Mono createAction(ActionDTO actionDTO, AppsmithEventContext eventContext, Boolean isJsAction) { - - return validateAndGenerateActionDomainBasedOnContext(actionDTO, isJsAction) + public Mono createAction(ActionDTO actionDTO, CreateActionMetaDTO actionMetaDTO) { + AppsmithEventContext eventContext = actionMetaDTO.getEventContext(); + return validateAndGenerateActionDomainBasedOnContext(actionDTO, actionMetaDTO) .name(VALIDATE_AND_GENERATE_ACTION_DOMAIN_BASED_ON_CONTEXT) .tap(Micrometer.observation(observationRegistry)) .flatMap(newAction -> { @@ -412,7 +417,10 @@ public Mono createAction(ActionDTO actionDTO, AppsmithEventContext ev }); } - protected Mono validateAndGenerateActionDomainBasedOnContext(ActionDTO action, boolean isJsAction) { + protected Mono validateAndGenerateActionDomainBasedOnContext( + ActionDTO action, CreateActionMetaDTO actionMetaDTO) { + Boolean isJsAction = actionMetaDTO.getIsJsAction(); + NewPage newPage = actionMetaDTO.getNewPage(); if (!StringUtils.hasLength(action.getPageId())) { return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, FieldName.PAGE_ID)); } @@ -421,13 +429,15 @@ protected Mono validateAndGenerateActionDomainBasedOnContext(ActionDT AclPermission aclPermission = isJsAction ? pagePermission.getReadPermission() : pagePermission.getActionCreatePermission(); - Mono pageMono = newPageService - .findById(action.getPageId(), aclPermission) - .name(GET_PAGE_BY_ID) - .tap(Micrometer.observation(observationRegistry)) - .switchIfEmpty(Mono.error( - new AppsmithException(AppsmithError.NO_RESOURCE_FOUND, FieldName.PAGE, action.getPageId()))) - .cache(); + Mono pageMono = newPage != null + ? Mono.just(newPage) + : newPageService + .findById(action.getPageId(), aclPermission) + .name(GET_PAGE_BY_ID) + .tap(Micrometer.observation(observationRegistry)) + .switchIfEmpty(Mono.error(new AppsmithException( + AppsmithError.NO_RESOURCE_FOUND, FieldName.PAGE, action.getPageId()))) + .cache(); final NewAction newAction = newActionService.generateActionDomain(action); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java index 3feeac7facf7..b31038346352 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionCollectionServiceTest.java @@ -94,7 +94,7 @@ public class ActionCollectionServiceTest { @Autowired ApplicationPageService applicationPageService; - @Autowired + @SpyBean LayoutActionService layoutActionService; @Autowired @@ -967,4 +967,53 @@ public void testLayoutOnLoadActions_withTwoWidgetsAndSameBinding_callsCorrectAct }) .verifyComplete(); } + + @Test + @WithUserDetails(value = "api_user") + public void testUpdateUnpublishedActionCollection_createSingleAction_fetchesPageFromDBOnlyOnce() { + Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(pluginExecutor)); + Mockito.when(pluginExecutor.getHintMessages(Mockito.any(), Mockito.any())) + .thenReturn(Mono.zip(Mono.just(new HashSet<>()), Mono.just(new HashSet<>()))); + + ActionCollectionDTO actionCollectionDTO = new ActionCollectionDTO(); + actionCollectionDTO.setName("testCollection1"); + actionCollectionDTO.setPageId(testPage.getId()); + actionCollectionDTO.setApplicationId(testApp.getId()); + actionCollectionDTO.setWorkspaceId(workspaceId); + actionCollectionDTO.setPluginId(datasource.getPluginId()); + actionCollectionDTO.setVariables(List.of(new JSValue("test", "String", "test", true))); + actionCollectionDTO.setBody("export default {\n\t\n}"); + actionCollectionDTO.setPluginType(PluginType.JS); + + // Create Js object + ActionCollectionDTO createdActionCollectionDTO = + layoutCollectionService.createCollection(actionCollectionDTO).block(); + assert createdActionCollectionDTO != null; + assert createdActionCollectionDTO.getId() != null; + String createdActionCollectionId = createdActionCollectionDTO.getId(); + + // Update JS object to create an action with same name as previously created action + ActionDTO action1 = new ActionDTO(); + action1.setName("testAction1"); + action1.setActionConfiguration(new ActionConfiguration()); + action1.getActionConfiguration().setBody("mockBody"); + action1.getActionConfiguration().setIsValid(false); + + actionCollectionDTO.setActions(List.of(action1)); + + final Mono updatedActionCollectionDTO = + layoutCollectionService.updateUnpublishedActionCollection( + createdActionCollectionId, actionCollectionDTO); + + StepVerifier.create(updatedActionCollectionDTO) + .assertNext(actionCollectionDTO1 -> { + assertEquals(createdActionCollectionId, actionCollectionDTO1.getId()); + Mockito.verify(layoutActionService, Mockito.times(1)) + .createAction( + Mockito.any(), + Mockito.argThat(createActionMetaDTO -> + createActionMetaDTO != null && createActionMetaDTO.getNewPage() != null)); + }) + .verifyComplete(); + } } diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ActionServiceCE_Test.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ActionServiceCE_Test.java index 86d94f1b5f10..9b496524358c 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ActionServiceCE_Test.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ce/ActionServiceCE_Test.java @@ -29,6 +29,7 @@ import com.appsmith.server.dtos.ActionViewDTO; import com.appsmith.server.dtos.ApplicationAccessDTO; import com.appsmith.server.dtos.ApplicationJson; +import com.appsmith.server.dtos.CreateActionMetaDTO; import com.appsmith.server.dtos.LayoutDTO; import com.appsmith.server.dtos.PageDTO; import com.appsmith.server.exceptions.AppsmithError; @@ -1180,7 +1181,10 @@ public void testExecuteOnLoadParamOnActionCreateWithClonePageContext() { action.setDatasource(datasource); AppsmithEventContext eventContext = new AppsmithEventContext(AppsmithEventContextType.CLONE_PAGE); - Mono actionMono = layoutActionService.createAction(action, eventContext, Boolean.FALSE); + CreateActionMetaDTO createActionMetaDTO = new CreateActionMetaDTO(); + createActionMetaDTO.setEventContext(eventContext); + createActionMetaDTO.setIsJsAction(Boolean.FALSE); + Mono actionMono = layoutActionService.createAction(action, createActionMetaDTO); StepVerifier.create(actionMono) .assertNext(createdAction -> { // executeOnLoad is expected to be set to false in case of default context