fix: generate unique widgetIds when cloning page

app/server/appsmith-server/src/main/java/com/appsmith/server/helpers/DslUtils.java

@@ -2,22 +2,141 @@
 
 import com.appsmith.external.helpers.MustacheHelper;
 import com.appsmith.external.models.MustacheBindingToken;
+import com.appsmith.server.constants.FieldName;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.node.ArrayNode;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.fasterxml.jackson.databind.node.TextNode;
 import lombok.AllArgsConstructor;
+import lombok.extern.slf4j.Slf4j;
+import net.minidev.json.JSONObject;
+import net.minidev.json.JSONValue;
+import org.apache.commons.lang3.RandomStringUtils;
 
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.regex.Pattern;
 
+@Slf4j
 public class DslUtils {
 
+    // The client-side widget id generator (see app/client/src/utils/generators.tsx) uses nanoid
+    // with this alphabet and length. Matching the format here keeps cloned widget ids
+    // indistinguishable from widgets created by the client.
+    private static final char[] WIDGET_ID_ALPHABET = "1234567890abcdefghijklmnopqrstuvwxyz".toCharArray();
+    private static final int WIDGET_ID_LENGTH = 10;
+
+    // The root MainContainer widget has a fixed, well-known id of "0" on every page. It is
+    // intentionally shared across pages (child widgets reference it via parentId = "0"), so we
+    // leave it untouched when regenerating ids.
+    private static final String MAIN_CONTAINER_WIDGET_ID = "0";
+
+    /**
+     * Generates a new widget id in the same format the client uses (10 char lowercase alphanumeric),
+     * backed by a cryptographically strong random source.
+     */
+    public static String generateWidgetId() {
+        return RandomStringUtils.secure().next(WIDGET_ID_LENGTH, WIDGET_ID_ALPHABET);
+    }
+
+    /**
+     * Walks the given layout DSL tree and returns a deep copy with every widget id replaced by a
+     * freshly generated one. Every string occurrence of an old id (e.g. parentId, widget-specific
+     * id references) is rewritten to the new id, preserving the internal relationships of the DSL.
+     * The root MainContainer id ("0") is preserved so that children continue to reference their
+     * canvas correctly.
+     *
+     * <p>The input DSL is not mutated; a new tree is returned. If the DSL is null or empty, the
+     * input is returned as-is.
+     */
+    public static JSONObject regenerateWidgetIds(JSONObject sourceDsl) {
+        if (sourceDsl == null || sourceDsl.isEmpty()) {
+            return sourceDsl;
+        }
+
+        JSONObject dsl = deepCopy(sourceDsl);
+        Map<String, String> oldToNewWidgetIdMap = new HashMap<>();
+        collectWidgetIds(dsl, oldToNewWidgetIdMap);
+
+        if (oldToNewWidgetIdMap.isEmpty()) {
+            return dsl;
+        }
+
+        rewriteWidgetIdReferences(dsl, oldToNewWidgetIdMap);
+        return dsl;
+    }
+
+    private static void collectWidgetIds(JSONObject widget, Map<String, String> oldToNewWidgetIdMap) {
+        Object widgetIdValue = widget.get(FieldName.WIDGET_ID);
+        if (widgetIdValue instanceof String widgetId
+                && !widgetId.isEmpty()
+                && !MAIN_CONTAINER_WIDGET_ID.equals(widgetId)
+                && !oldToNewWidgetIdMap.containsKey(widgetId)) {
+            oldToNewWidgetIdMap.put(widgetId, generateWidgetId());
+        }
+
+        Object children = widget.get(FieldName.CHILDREN);
+        if (children instanceof List<?> childrenList) {
+            for (Object child : childrenList) {
+                if (child instanceof JSONObject childObject) {
+                    collectWidgetIds(childObject, oldToNewWidgetIdMap);
+                }
+            }
+        }
+    }
+
+    @SuppressWarnings("unchecked")
+    private static void rewriteWidgetIdReferences(Object node, Map<String, String> oldToNewWidgetIdMap) {
+        if (node instanceof Map<?, ?> mapNode) {
+            Map<String, Object> typedMap = (Map<String, Object>) mapNode;
+            for (Map.Entry<String, Object> entry : typedMap.entrySet()) {
+                Object value = entry.getValue();
+                if (value instanceof String stringValue && isWidgetIdReferenceKey(entry.getKey())) {
+                    // Only rewrite string values at keys that denote a widget id reference
+                    // (e.g. widgetId, parentId, mainCanvasId, selectedTabWidgetId). This avoids
+                    // clobbering fields like widgetName or user content strings that might
+                    // coincidentally match a widget id.
+                    String replacement = oldToNewWidgetIdMap.get(stringValue);
+                    if (replacement != null) {
+                        entry.setValue(replacement);
+                    }
+                } else if (!(value instanceof String)) {
+                    rewriteWidgetIdReferences(value, oldToNewWidgetIdMap);
+                }
+            }
+        } else if (node instanceof List<?> listNode) {
+            for (Object element : listNode) {
+                if (!(element instanceof String)) {
+                    rewriteWidgetIdReferences(element, oldToNewWidgetIdMap);
+                }
+            }
+        }
+    }
+
+    private static boolean isWidgetIdReferenceKey(String key) {
+        // DSL fields that hold widget id references follow the camelCase convention of ending
+        // with "Id" (widgetId, parentId, mainCanvasId, referencedWidgetId, selectedTabWidgetId,
+        // prefixMetaWidgetId, ...). Fields that happen to end with "Id" but hold non-widget-id
+        // values (e.g. tabId -> "tab1") are safe because the value lookup in the oldToNew map
+        // still acts as a second filter.
+        return key != null && key.endsWith("Id");
+    }
+
+    private static JSONObject deepCopy(JSONObject source) {
+        Object parsed = JSONValue.parse(source.toJSONString());
+        if (parsed instanceof JSONObject copied) {
+            return copied;
+        }
+        // Should never happen for a well-formed DSL, but fall back to the source rather than crash.
+        log.warn("Failed to deep copy DSL, parsed type was {}", parsed == null ? "null" : parsed.getClass());
+        return source;
+    }
+
     public static Set<MustacheBindingToken> getMustacheValueSetFromSpecificDynamicBindingPath(
             JsonNode dsl, String fieldPath) {
 

app/server/appsmith-server/src/main/java/com/appsmith/server/services/ce/ApplicationPageServiceCEImpl.java

@@ -35,6 +35,7 @@
 import com.appsmith.server.exceptions.AppsmithException;
 import com.appsmith.server.helpers.CommonGitFileUtils;
 import com.appsmith.server.helpers.DSLMigrationUtils;
+import com.appsmith.server.helpers.DslUtils;
 import com.appsmith.server.helpers.GitUtils;
 import com.appsmith.server.helpers.LoadShifter;
 import com.appsmith.server.helpers.UserPermissionUtils;
@@ -641,7 +642,11 @@ protected Mono<PageDTO> clonePageGivenApplicationId(
                             String id = new ObjectId().toString();
                             newLayout.setId(id);
                             newLayout.setMongoEscapedWidgetNames(layout.getMongoEscapedWidgetNames());
-                            newLayout.setDsl(layout.getDsl());
+                            // Regenerate widget ids so the cloned page does not collide with the
+                            // source page at application level. The helper preserves referential
+                            // integrity inside the DSL (e.g. parentId), and leaves the root
+                            // MainContainer id ("0") untouched.
+                            newLayout.setDsl(DslUtils.regenerateWidgetIds(layout.getDsl()));
                             return newLayout;
                         })
                         .collectList()

app/server/appsmith-server/src/test/java/com/appsmith/server/helpers/DslUtilsTest.java

@@ -6,10 +6,15 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 import com.fasterxml.jackson.databind.node.TextNode;
+import net.minidev.json.JSONArray;
+import net.minidev.json.JSONObject;
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 
 class DslUtilsTest {
@@ -96,4 +101,215 @@ void replaceValuesInSpecificDynamicBindingPath_withSuccessfulMultipleReplacement
         Assertions.assertThat(replacedDsl.get("existingPath").asText())
                 .isEqualTo("newishFieldValue1 newerFieldValue2 newishFieldValue1 newerFieldValue2");
     }
+
+    @Test
+    void generateWidgetId_producesTenCharLowercaseAlphanumericIds() {
+        String widgetId = DslUtils.generateWidgetId();
+        Assertions.assertThat(widgetId).hasSize(10).matches("[0-9a-z]{10}");
+    }
+
+    @Test
+    void generateWidgetId_producesDistinctIdsAcrossCalls() {
+        Set<String> generatedIds = new HashSet<>();
+        for (int i = 0; i < 1000; i++) {
+            generatedIds.add(DslUtils.generateWidgetId());
+        }
+        // With a 36^10 space, 1000 calls should never collide in practice.
+        Assertions.assertThat(generatedIds).hasSize(1000);
+    }
+
+    @Test
+    void regenerateWidgetIds_withNullOrEmptyDsl_returnsInputAsIs() {
+        Assertions.assertThat(DslUtils.regenerateWidgetIds(null)).isNull();
+        JSONObject empty = new JSONObject();
+        Assertions.assertThat(DslUtils.regenerateWidgetIds(empty)).isSameAs(empty);
+    }
+
+    @Test
+    void regenerateWidgetIds_preservesMainContainerIdAndRegeneratesChildIds() {
+        JSONObject child = new JSONObject();
+        child.put("widgetId", "childWidgetId1");
+        child.put("widgetName", "Button1");
+        child.put("parentId", "0");
+
+        JSONArray children = new JSONArray();
+        children.add(child);
+
+        JSONObject dsl = new JSONObject();
+        dsl.put("widgetId", "0");
+        dsl.put("widgetName", "MainContainer");
+        dsl.put("children", children);
+
+        JSONObject regenerated = DslUtils.regenerateWidgetIds(dsl);
+
+        // MainContainer id is preserved
+        Assertions.assertThat(regenerated.get("widgetId")).isEqualTo("0");
+        Assertions.assertThat(regenerated.get("widgetName")).isEqualTo("MainContainer");
+
+        // Child widget id is regenerated, but the parentId reference to MainContainer is preserved
+        List<?> regeneratedChildren = (List<?>) regenerated.get("children");
+        Assertions.assertThat(regeneratedChildren).hasSize(1);
+        JSONObject regeneratedChild = (JSONObject) regeneratedChildren.get(0);
+        Assertions.assertThat(regeneratedChild.get("widgetId"))
+                .isNotEqualTo("childWidgetId1")
+                .asString()
+                .matches("[0-9a-z]{10}");
+        Assertions.assertThat(regeneratedChild.get("widgetName")).isEqualTo("Button1");
+        Assertions.assertThat(regeneratedChild.get("parentId")).isEqualTo("0");
+    }
+
+    @Test
+    void regenerateWidgetIds_rewritesAllInternalReferencesConsistently() {
+        // Build: MainContainer -> Container (parent) -> Button (child of Container).
+        JSONObject button = new JSONObject();
+        button.put("widgetId", "buttonOldId");
+        button.put("widgetName", "Button1");
+        button.put("parentId", "containerOldId");
+
+        JSONArray containerChildren = new JSONArray();
+        containerChildren.add(button);
+
+        JSONObject container = new JSONObject();
+        container.put("widgetId", "containerOldId");
+        container.put("widgetName", "Container1");
+        container.put("parentId", "0");
+        container.put("children", containerChildren);
+        // Widget-specific id reference (e.g. List widget's mainCanvasId) must also be rewritten.
+        container.put("mainCanvasId", "buttonOldId");
+
+        JSONArray rootChildren = new JSONArray();
+        rootChildren.add(container);
+
+        JSONObject dsl = new JSONObject();
+        dsl.put("widgetId", "0");
+        dsl.put("widgetName", "MainContainer");
+        dsl.put("children", rootChildren);
+
+        JSONObject regenerated = DslUtils.regenerateWidgetIds(dsl);
+
+        JSONObject regeneratedContainer = (JSONObject) ((List<?>) regenerated.get("children")).get(0);
+        JSONObject regeneratedButton = (JSONObject) ((List<?>) regeneratedContainer.get("children")).get(0);
+
+        String newContainerId = (String) regeneratedContainer.get("widgetId");
+        String newButtonId = (String) regeneratedButton.get("widgetId");
+
+        Assertions.assertThat(newContainerId).isNotEqualTo("containerOldId").matches("[0-9a-z]{10}");
+        Assertions.assertThat(newButtonId).isNotEqualTo("buttonOldId").matches("[0-9a-z]{10}");
+        // Container keeps its parent (MainContainer) reference.
+        Assertions.assertThat(regeneratedContainer.get("parentId")).isEqualTo("0");
+        // Button's parent reference has been rewritten to the new container id.
+        Assertions.assertThat(regeneratedButton.get("parentId")).isEqualTo(newContainerId);
+        // Arbitrary widget-specific references are rewritten to the new button id.
+        Assertions.assertThat(regeneratedContainer.get("mainCanvasId")).isEqualTo(newButtonId);
+    }
+
+    @Test
+    void regenerateWidgetIds_doesNotMutateSourceDsl() {
+        JSONObject child = new JSONObject();
+        child.put("widgetId", "originalChildId");
+        child.put("widgetName", "Button1");
+
+        JSONArray children = new JSONArray();
+        children.add(child);
+
+        JSONObject dsl = new JSONObject();
+        dsl.put("widgetId", "originalRootId");
+        dsl.put("widgetName", "Canvas1");
+        dsl.put("children", children);
+
+        JSONObject regenerated = DslUtils.regenerateWidgetIds(dsl);
+
+        Assertions.assertThat(regenerated).isNotSameAs(dsl);
+        // Source DSL widget ids are untouched.
+        Assertions.assertThat(dsl.get("widgetId")).isEqualTo("originalRootId");
+        JSONObject originalChild = (JSONObject) ((List<?>) dsl.get("children")).get(0);
+        Assertions.assertThat(originalChild.get("widgetId")).isEqualTo("originalChildId");
+        // Regenerated DSL has different widget ids.
+        Assertions.assertThat(regenerated.get("widgetId")).isNotEqualTo("originalRootId");
+    }
+
+    @Test
+    void regenerateWidgetIds_producesNoDuplicateIdsAcrossTree() {
+        JSONObject dsl = new JSONObject();
+        dsl.put("widgetId", "0");
+        dsl.put("widgetName", "MainContainer");
+
+        JSONArray children = new JSONArray();
+        for (int i = 0; i < 20; i++) {
+            JSONObject child = new JSONObject();
+            child.put("widgetId", "child" + i);
+            child.put("widgetName", "Widget" + i);
+            child.put("parentId", "0");
+            children.add(child);
+        }
+        dsl.put("children", children);
+
+        JSONObject regenerated = DslUtils.regenerateWidgetIds(dsl);
+
+        Set<String> ids = new HashSet<>();
+        ids.add((String) regenerated.get("widgetId"));
+        for (Object child : (List<?>) regenerated.get("children")) {
+            ids.add((String) ((JSONObject) child).get("widgetId"));
+        }
+        // 1 MainContainer + 20 distinct child ids.
+        Assertions.assertThat(ids).hasSize(21);
+    }
+
+    @Test
+    void regenerateWidgetIds_doesNotRewriteNonIdReferenceFieldsThatMatchAWidgetId() {
+        // Guard against false-positive rewrites: if a non-id field (e.g. widgetName, user text
+        // content, dynamic binding paths) happens to contain the same string as a widget id,
+        // the rewriter must leave it alone.
+        JSONObject child = new JSONObject();
+        child.put("widgetId", "Table1");
+        // widgetName deliberately shares the same value as the widgetId (as Appsmith's
+        // PageServiceTest fixtures do). It must survive the regeneration unchanged.
+        child.put("widgetName", "Table1");
+        child.put("parentId", "0");
+        // A plain text field whose content happens to equal the widget id must also survive.
+        child.put("text", "Table1");
+        // Dynamic binding path lists reference property names (e.g. "primaryColumns._id"),
+        // never widget ids. They must survive untouched even if a path fragment matches.
+        JSONArray bindingPaths = new JSONArray();
+        JSONObject pathEntry = new JSONObject();
+        pathEntry.put("key", "Table1");
+        bindingPaths.add(pathEntry);
+        child.put("dynamicBindingPathList", bindingPaths);
+
+        JSONArray children = new JSONArray();
+        children.add(child);
+
+        JSONObject dsl = new JSONObject();
+        dsl.put("widgetId", "0");
+        dsl.put("widgetName", "MainContainer");
+        dsl.put("children", children);
+
+        JSONObject regenerated = DslUtils.regenerateWidgetIds(dsl);
+
+        JSONObject regeneratedChild = (JSONObject) ((List<?>) regenerated.get("children")).get(0);
+        String newWidgetId = (String) regeneratedChild.get("widgetId");
+
+        Assertions.assertThat(newWidgetId).isNotEqualTo("Table1").matches("[0-9a-z]{10}");
+        Assertions.assertThat(regeneratedChild.get("widgetName")).isEqualTo("Table1");
+        Assertions.assertThat(regeneratedChild.get("text")).isEqualTo("Table1");
+        Assertions.assertThat(regeneratedChild.get("parentId")).isEqualTo("0");
+        JSONObject regeneratedPathEntry =
+                (JSONObject) ((List<?>) regeneratedChild.get("dynamicBindingPathList")).get(0);
+        Assertions.assertThat(regeneratedPathEntry.get("key")).isEqualTo("Table1");
+    }
+
+    @Test
+    void regenerateWidgetIds_emptyChildrenArray_regeneratesRootId() {
+        JSONObject dsl = new JSONObject();
+        dsl.put("widgetId", "rootOldId");
+        dsl.put("widgetName", "Canvas1");
+        dsl.put("children", new ArrayList<>());
+
+        JSONObject regenerated = DslUtils.regenerateWidgetIds(dsl);
+
+        Assertions.assertThat(regenerated.get("widgetId"))
+                .isNotEqualTo("rootOldId")
+                .asString()
+                .matches("[0-9a-z]{10}");
+    }
 }