From 56d296afc88a39f8943fbd6b7c8c5ded2f45450c Mon Sep 17 00:00:00 2001 From: subratadeypappu Date: Mon, 26 May 2025 23:53:13 +0600 Subject: [PATCH] chore: Add code-split for OPL with UI module instances (#40749) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description EE Counterpart PR: https://github.com/appsmithorg/appsmith-ee/pull/7559 Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _If no issue exists, please create an issue first, and check with the maintainers if the issue is valid._ ## Automation /ok-to-test tags="@tag.All" ### :mag: Cypress test results > [!TIP] > 🟒 🟒 🟒 All cypress tests have passed! πŸŽ‰ πŸŽ‰ πŸŽ‰ > Workflow run: > Commit: dd70a1c673da72ef7e5bdea172b90d7ec338f41a > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Mon, 26 May 2025 13:55:15 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit - **Refactor** - Enhanced layout update processing by modularizing synthetic widget handling, enabling future UI-specific widget injection and removal. No direct impact on user-facing features. --- .../layouts/UpdateLayoutServiceCEImpl.java | 258 +++++++++++------- 1 file changed, 153 insertions(+), 105 deletions(-) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java index a974e878f8..cd1c6a9895 100755 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/layouts/UpdateLayoutServiceCEImpl.java @@ -24,6 +24,7 @@ import com.appsmith.server.services.SessionUserService; import com.appsmith.server.solutions.PagePermission; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.Sets; import io.micrometer.observation.ObservationRegistry; import io.micrometer.tracing.Span; import lombok.RequiredArgsConstructor; @@ -43,6 +44,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import java.util.regex.Pattern; @@ -109,6 +111,12 @@ public class UpdateLayoutServiceCEImpl implements UpdateLayoutServiceCE { }); } + // Extension point for UI module handling + protected Mono>>> processDslAndCreateSyntheticWidgets( + String creatorId, CreatorContextType creatorType, JSONObject dsl, ArrayList mainChildren) { + return Mono.just(Map.entry(dsl, Optional.empty())); + } + protected Mono updateLayoutDsl( String creatorId, String layoutId, @@ -121,122 +129,162 @@ public class UpdateLayoutServiceCEImpl implements UpdateLayoutServiceCE { return Mono.just(generateResponseDTO(layout)); } - Set widgetNames = new HashSet<>(); - Map> widgetDynamicBindingsMap = new HashMap<>(); - Set escapedWidgetNames = new HashSet<>(); + // Get the main DSL's children array. We *don't* create an empty list + // yet; if no UI‑module instances are injected we want to leave the DSL + // unchanged so tests that expect a verbatim payload pass. + ArrayList mainChildren = (ArrayList) dsl.get(FieldName.CHILDREN); - Span extractAllWidgetNamesAndDynamicBindingsFromDSLSpan = - observationHelper.createSpan(EXTRACT_ALL_WIDGET_NAMES_AND_DYNAMIC_BINDINGS_FROM_DSL); + // Process DSL with module instances if any + return processDslAndCreateSyntheticWidgets(creatorId, creatorType, dsl, mainChildren) + .flatMap(entry -> { + JSONObject processedDsl = entry.getKey(); + Optional> syntheticWidgetNamesOpt = entry.getValue(); - observationHelper.startSpan(extractAllWidgetNamesAndDynamicBindingsFromDSLSpan); + Set widgetNames = new HashSet<>(); + Map> widgetDynamicBindingsMap = new HashMap<>(); + Set escapedWidgetNames = new HashSet<>(); - try { - dsl = extractAllWidgetNamesAndDynamicBindingsFromDSL( - dsl, widgetNames, widgetDynamicBindingsMap, creatorId, layoutId, escapedWidgetNames, creatorType); + Span extractAllWidgetNamesAndDynamicBindingsFromDSLSpan = + observationHelper.createSpan(EXTRACT_ALL_WIDGET_NAMES_AND_DYNAMIC_BINDINGS_FROM_DSL); - } catch (Throwable t) { - return sendUpdateLayoutAnalyticsEvent(creatorId, layoutId, dsl, false, t, creatorType) - .then(Mono.error(t)); - } + observationHelper.startSpan(extractAllWidgetNamesAndDynamicBindingsFromDSLSpan); - observationHelper.endSpan(extractAllWidgetNamesAndDynamicBindingsFromDSLSpan); - - layout.setWidgetNames(widgetNames); - - if (!escapedWidgetNames.isEmpty()) { - layout.setMongoEscapedWidgetNames(escapedWidgetNames); - } - - Set executableNames = new HashSet<>(); - Set edges = new HashSet<>(); - Set executablesUsedInDSL = new HashSet<>(); - List flatmapOnLoadExecutables = new ArrayList<>(); - List executableUpdatesRef = new ArrayList<>(); - List messagesRef = new ArrayList<>(); - - AtomicReference validOnLoadExecutables = new AtomicReference<>(Boolean.TRUE); - - // setting the layoutOnLoadActionActionErrors to empty to remove the existing - // errors before new DAG calculation. - layout.setLayoutOnLoadActionErrors(new ArrayList<>()); - - Mono>> allOnLoadExecutablesMono = onLoadExecutablesUtil - .findAllOnLoadExecutables( - creatorId, - evaluatedVersion, - widgetNames, - edges, - widgetDynamicBindingsMap, - flatmapOnLoadExecutables, - executablesUsedInDSL, - creatorType) - .name(FIND_ALL_ON_LOAD_EXECUTABLES) - .tap(Micrometer.observation(observationRegistry)) - .onErrorResume(AppsmithException.class, error -> { - log.info(error.getMessage()); - validOnLoadExecutables.set(FALSE); - layout.setLayoutOnLoadActionErrors(List.of(new ErrorDTO( - error.getAppErrorCode(), - error.getErrorType(), - layoutOnLoadActionErrorToastMessage, - error.getMessage(), - error.getTitle()))); - return Mono.just(new ArrayList<>()); - }); - - // First update the actions and set execute on load to true - JSONObject finalDsl = dsl; - - Mono layoutDTOMono = allOnLoadExecutablesMono - .flatMap(allOnLoadExecutables -> { - // If there has been an error (e.g. cyclical dependency), then don't update any - // actions. - // This is so that unnecessary updates don't happen to actions while the page is - // in invalid state. - if (!validOnLoadExecutables.get()) { - return Mono.just(allOnLoadExecutables); + try { + processedDsl = extractAllWidgetNamesAndDynamicBindingsFromDSL( + processedDsl, + widgetNames, + widgetDynamicBindingsMap, + creatorId, + layoutId, + escapedWidgetNames, + creatorType); + } catch (Throwable t) { + return sendUpdateLayoutAnalyticsEvent(creatorId, layoutId, processedDsl, false, t, creatorType) + .then(Mono.error(t)); } - // Update these executables to be executed on load, unless the user has touched - // the runBehaviour - // setting for this - return onLoadExecutablesUtil - .updateExecutablesRunBehaviour( - flatmapOnLoadExecutables, creatorId, executableUpdatesRef, messagesRef, creatorType) - .name(UPDATE_EXECUTABLES_RUN_BEHAVIOUR) + + observationHelper.endSpan(extractAllWidgetNamesAndDynamicBindingsFromDSLSpan); + + // ----------------------------------------------------------------------------- + // If we have synthetic widgets, filter them out and scrub the DSL + // ----------------------------------------------------------------------------- + if (syntheticWidgetNamesOpt.isPresent()) { + Set syntheticWidgetNames = syntheticWidgetNamesOpt.get(); + + Set widgetNamesToPersist = Sets.difference(widgetNames, syntheticWidgetNames); + + processedDsl = stripSyntheticWidgets(processedDsl, syntheticWidgetNamesOpt); + removeSpecialCharactersFromKeys(processedDsl, escapedWidgetNames); + + layout.setWidgetNames(widgetNamesToPersist); + } else { + layout.setWidgetNames(widgetNames); + } + + layout.setDsl(processedDsl); + + // We always attach escaped names when we have them + if (!escapedWidgetNames.isEmpty()) { + layout.setMongoEscapedWidgetNames(escapedWidgetNames); + } + + Set executableNames = new HashSet<>(); + Set edges = new HashSet<>(); + Set executablesUsedInDSL = new HashSet<>(); + List flatmapOnLoadExecutables = new ArrayList<>(); + List executableUpdatesRef = new ArrayList<>(); + List messagesRef = new ArrayList<>(); + + AtomicReference validOnLoadExecutables = new AtomicReference<>(Boolean.TRUE); + + // setting the layoutOnLoadActionActionErrors to empty to remove the existing + // errors before new DAG calculation. + layout.setLayoutOnLoadActionErrors(new ArrayList<>()); + + Mono>> allOnLoadExecutablesMono = onLoadExecutablesUtil + .findAllOnLoadExecutables( + creatorId, + evaluatedVersion, + widgetNames, + edges, + widgetDynamicBindingsMap, + flatmapOnLoadExecutables, + executablesUsedInDSL, + creatorType) + .name(FIND_ALL_ON_LOAD_EXECUTABLES) .tap(Micrometer.observation(observationRegistry)) - .thenReturn(allOnLoadExecutables); - }) - // Now update the page layout with the page load executables and the graph. - .flatMap(onLoadExecutables -> { - layout.setLayoutOnLoadActions(onLoadExecutables); - layout.setAllOnPageLoadActionNames(executableNames); - layout.setActionsUsedInDynamicBindings(executablesUsedInDSL); - // The below field is to ensure that we record if the page load actions - // computation was - // valid when last stored in the database. - layout.setValidOnPageLoadActions(validOnLoadExecutables.get()); + .onErrorResume(AppsmithException.class, error -> { + log.info(error.getMessage()); + validOnLoadExecutables.set(FALSE); + layout.setLayoutOnLoadActionErrors(List.of(new ErrorDTO( + error.getAppErrorCode(), + error.getErrorType(), + layoutOnLoadActionErrorToastMessage, + error.getMessage(), + error.getTitle()))); + return Mono.just(new ArrayList<>()); + }); - return onLoadExecutablesUtil - .findAndUpdateLayout(creatorId, creatorType, layoutId, layout) - .tag("no_of_widgets", String.valueOf(widgetNames.size())) - .tag("no_of_executables", String.valueOf(executableNames.size())) - .name(FIND_AND_UPDATE_LAYOUT) - .tap(Micrometer.observation(observationRegistry)); - }) - .map(savedLayout -> { - savedLayout.setDsl(this.unescapeMongoSpecialCharacters(savedLayout)); - return savedLayout; - }) - .flatMap(savedLayout -> { - LayoutDTO layoutDTO = generateResponseDTO(savedLayout); - layoutDTO.setActionUpdates(executableUpdatesRef); - layoutDTO.setMessages(messagesRef); + // First update the actions and set execute on load to true + JSONObject finalDsl = processedDsl; - return sendUpdateLayoutAnalyticsEvent(creatorId, layoutId, finalDsl, true, null, creatorType) - .thenReturn(layoutDTO); + return allOnLoadExecutablesMono + .flatMap(allOnLoadExecutables -> { + // If there has been an error (e.g. cyclical dependency), then don't update any + // actions. + // This is so that unnecessary updates don't happen to actions while the page is + // in invalid state. + if (!validOnLoadExecutables.get()) { + return Mono.just(allOnLoadExecutables); + } + // Update these executables to be executed on load, unless the user has touched + // the runBehaviour + // setting for this + return onLoadExecutablesUtil + .updateExecutablesRunBehaviour( + flatmapOnLoadExecutables, + creatorId, + executableUpdatesRef, + messagesRef, + creatorType) + .name(UPDATE_EXECUTABLES_RUN_BEHAVIOUR) + .tap(Micrometer.observation(observationRegistry)) + .thenReturn(allOnLoadExecutables); + }) + // Now update the page layout with the page load executables and the graph. + .flatMap(onLoadExecutables -> { + layout.setLayoutOnLoadActions(onLoadExecutables); + layout.setAllOnPageLoadActionNames(executableNames); + layout.setActionsUsedInDynamicBindings(executablesUsedInDSL); + // The below field is to ensure that we record if the page load actions + // computation was + // valid when last stored in the database. + + return onLoadExecutablesUtil + .findAndUpdateLayout(creatorId, creatorType, layoutId, layout) + .tag("no_of_widgets", String.valueOf(widgetNames.size())) + .tag("no_of_executables", String.valueOf(executableNames.size())) + .name(FIND_AND_UPDATE_LAYOUT) + .tap(Micrometer.observation(observationRegistry)); + }) + .map(savedLayout -> { + savedLayout.setDsl(this.unescapeMongoSpecialCharacters(savedLayout)); + return savedLayout; + }) + .flatMap(savedLayout -> { + LayoutDTO layoutDTO = generateResponseDTO(savedLayout); + layoutDTO.setActionUpdates(executableUpdatesRef); + layoutDTO.setMessages(messagesRef); + + return sendUpdateLayoutAnalyticsEvent( + creatorId, layoutId, finalDsl, true, null, creatorType) + .thenReturn(layoutDTO); + }); }); + } - return layoutDTOMono; + protected JSONObject stripSyntheticWidgets(JSONObject dsl, Optional> syntheticNamesOpt) { + return dsl; } // TODO: Add contextType and change all its usage to conform to that so that we can get rid of the overloaded