From 2cb1aa3de1acb9a486a9a4453e77e8a098aa6bbe Mon Sep 17 00:00:00 2001
From: Nidhi <nidhi@appsmith.com>
Date: Wed, 28 Jul 2021 11:19:23 +0530
Subject: [PATCH] Added validation for refactor name (#6165)

---
 .../services/LayoutActionServiceImpl.java     |  8 +++-
 .../server/services/NewActionService.java     |  2 +
 .../server/services/NewActionServiceImpl.java |  5 +-
 .../services/LayoutActionServiceTest.java     | 46 +++++++++++++++++++
 4 files changed, 58 insertions(+), 3 deletions(-)

diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java
index 0e625ddf4d..d3275cf406 100644
--- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java
+++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/LayoutActionServiceImpl.java
@@ -166,7 +166,13 @@ public class LayoutActionServiceImpl implements LayoutActionService {
         String oldName = refactorActionNameDTO.getOldName();
         String newName = refactorActionNameDTO.getNewName();
         String actionId = refactorActionNameDTO.getActionId();
-        return isNameAllowed(pageId, layoutId, newName)
+        return Mono.just(newActionService.validateActionName(newName))
+                .flatMap(isValidName -> {
+                    if (!isValidName) {
+                        return Mono.error(new AppsmithException(AppsmithError.INVALID_ACTION_NAME));
+                    }
+                    return isNameAllowed(pageId, layoutId, newName);
+                })
                 .flatMap(allowed -> {
                     if (!allowed) {
                         return Mono.error(new AppsmithException(AppsmithError.NAME_CLASH_NOT_ALLOWED_IN_REFACTOR, oldName, newName));
diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewActionService.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewActionService.java
index 25bb224246..450a50fb72 100644
--- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewActionService.java
+++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewActionService.java
@@ -19,6 +19,8 @@ import java.util.Set;
 
 public interface NewActionService extends CrudService<NewAction, String> {
 
+    Boolean validateActionName(String name);
+
     void setCommonFieldsFromActionDTOIntoNewAction(ActionDTO action, NewAction newAction);
 
     Mono<ActionDTO> generateActionByViewMode(NewAction newAction, Boolean viewMode);
diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewActionServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewActionServiceImpl.java
index 9393b49c53..668a361313 100644
--- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewActionServiceImpl.java
+++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/NewActionServiceImpl.java
@@ -72,12 +72,12 @@ import java.util.stream.Collectors;
 
 import static com.appsmith.external.helpers.BeanCopyUtils.copyNewFieldValuesIntoOldObject;
 import static com.appsmith.external.helpers.DataTypeStringUtils.getDisplayDataTypes;
-import static com.appsmith.server.helpers.WidgetSuggestionHelper.getSuggestedWidgets;
 import static com.appsmith.server.acl.AclPermission.EXECUTE_ACTIONS;
 import static com.appsmith.server.acl.AclPermission.EXECUTE_DATASOURCES;
 import static com.appsmith.server.acl.AclPermission.MANAGE_ACTIONS;
 import static com.appsmith.server.acl.AclPermission.MANAGE_DATASOURCES;
 import static com.appsmith.server.acl.AclPermission.READ_ACTIONS;
+import static com.appsmith.server.helpers.WidgetSuggestionHelper.getSuggestedWidgets;
 import static java.lang.Boolean.FALSE;
 import static java.lang.Boolean.TRUE;
 
@@ -132,7 +132,8 @@ public class NewActionServiceImpl extends BaseService<NewActionRepository, NewAc
         this.objectMapper = new ObjectMapper();
     }
 
-    private Boolean validateActionName(String name) {
+    @Override
+    public Boolean validateActionName(String name) {
         boolean isValidName = SourceVersion.isName(name);
         String pattern = "^((?=[A-Za-z0-9_])(?![\\\\-]).)*$";
         boolean doesPatternMatch = name.matches(pattern);
diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java
index 7b0859760d..c05a9d691c 100644
--- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java
+++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/LayoutActionServiceTest.java
@@ -324,6 +324,52 @@ public class LayoutActionServiceTest {
                 .verifyComplete();
     }
 
+    @Test
+    @WithUserDetails(value = "api_user")
+    public void testRefactorActionName_withInvalidName_throwsError() {
+        Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(new MockPluginExecutor()));
+
+        ActionDTO action = new ActionDTO();
+        action.setName("beforeNameChange");
+        action.setPageId(testPage.getId());
+        ActionConfiguration actionConfiguration = new ActionConfiguration();
+        actionConfiguration.setHttpMethod(HttpMethod.GET);
+        action.setActionConfiguration(actionConfiguration);
+        action.setDatasource(datasource);
+
+        JSONObject dsl = new JSONObject();
+        dsl.put("widgetName", "firstWidget");
+        JSONArray temp = new JSONArray();
+        temp.addAll(List.of(new JSONObject(Map.of("key", "testField"))));
+        dsl.put("dynamicBindingPathList", temp);
+        dsl.put("testField", "{{ beforeNameChange.data }}");
+
+        Layout layout = testPage.getLayouts().get(0);
+        layout.setDsl(dsl);
+        layout.setPublishedDsl(dsl);
+
+        ActionDTO createdAction = layoutActionService.createAction(action).block();
+
+        LayoutDTO firstLayout = layoutActionService.updateLayout(testPage.getId(), layout.getId(), layout).block();
+
+        RefactorActionNameDTO refactorActionNameDTO = new RefactorActionNameDTO();
+        refactorActionNameDTO.setPageId(testPage.getId());
+        assert firstLayout != null;
+        refactorActionNameDTO.setLayoutId(firstLayout.getId());
+        refactorActionNameDTO.setOldName("beforeNameChange");
+        refactorActionNameDTO.setNewName("!PostNameChange");
+        assert createdAction != null;
+        refactorActionNameDTO.setActionId(createdAction.getId());
+
+        final Mono<LayoutDTO> layoutDTOMono = layoutActionService.refactorActionName(refactorActionNameDTO);
+
+        StepVerifier
+                .create(layoutDTOMono)
+                .expectErrorMatches(e -> e instanceof AppsmithException &&
+                        AppsmithError.INVALID_ACTION_NAME.getMessage().equalsIgnoreCase(e.getMessage()))
+                .verify();
+    }
+
     @Test
     @WithUserDetails(value = "api_user")
     public void actionExecuteOnLoadChangeOnUpdateLayout() {