Merge pull request #2876 from plotly/fix/pmc-side-updates

T4rk1n · web-flow · commit efb9d32aa42c · 2024-06-12T09:04:14.000-04:00
Fix pattern matching in side updates (running/progress/set_props)
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 - [#2859](https://github.com/plotly/dash/pull/2859) Fix base patch operators. fixes [#2855](https://github.com/plotly/dash/issues/2855)
 - [#2856](https://github.com/plotly/dash/pull/2856) Fix multiple consecutive calls with same id to set_props only keeping the last props. Fixes [#2852](https://github.com/plotly/dash/issues/2852)
 - [#2867](https://github.com/plotly/dash/pull/2867) Fix clientside no output callback. Fixes [#2866](https://github.com/plotly/dash/issues/2866)
+- [#2876](https://github.com/plotly/dash/pull/2876) Fix pattern matching in callback running argument. Fixes [#2863](https://github.com/plotly/dash/issues/2863)
 
 ## [2.17.0] - 2024-05-03
 
diff --git a/dash/dash-renderer/src/actions/callbacks.ts b/dash/dash-renderer/src/actions/callbacks.ts
@@ -31,7 +31,8 @@ import {
     IPrioritizedCallback,
     LongCallbackInfo,
     CallbackResponse,
-    CallbackResponseData
+    CallbackResponseData,
+    SideUpdateOutput
 } from '../types/callbacks';
 import {isMultiValued, stringifyId, isMultiOutputProp} from './dependencies';
 import {urlBase} from './utils';
@@ -44,6 +45,8 @@ import {handlePatch, isPatch} from './patch';
 import {getPath} from './paths';
 
 import {requestDependencies} from './requestDependencies';
+import {parsePMCId} from './patternMatching';
+import {replacePMC} from './patternMatching';
 
 export const addBlockedCallbacks = createAction<IBlockedCallback[]>(
     CallbackActionType.AddBlocked
@@ -340,33 +343,53 @@ function updateComponent(component_id: any, props: any) {
     };
 }
 
-function sideUpdate(outputs: any, dispatch: any) {
-    toPairs(outputs).forEach(([id, value]) => {
-        let componentId = id,
-            propName;
+/**
+ * Update a component props with `running`/`progress`/`set_props` calls.
+ *
+ * @param outputs Props to update.
+ * @param cb The originating callback info.
+ * @returns
+ */
+function sideUpdate(outputs: SideUpdateOutput, cb: ICallbackPayload) {
+    return function (dispatch: any, getState: any) {
+        toPairs(outputs)
+            .reduce((acc, [id, value], i) => {
+                let componentId = id,
+                    propName,
+                    replacedIds = [];
+
+                if (id.startsWith('{')) {
+                    [componentId, propName] = parsePMCId(id);
+                    replacedIds = replacePMC(componentId, cb, i, getState);
+                } else if (id.includes('.')) {
+                    [componentId, propName] = id.split('.');
+                }
 
-        if (id.startsWith('{')) {
-            const index = id.lastIndexOf('}');
-            if (index + 2 < id.length) {
-                propName = id.substring(index + 2);
-                componentId = JSON.parse(id.substring(0, index + 1));
-            } else {
-                componentId = JSON.parse(id);
-            }
-        } else if (id.includes('.')) {
-            [componentId, propName] = id.split('.');
-        }
+                const props = propName ? {[propName]: value} : value;
 
-        const props = propName ? {[propName]: value} : value;
-        dispatch(updateComponent(componentId, props));
-    });
+                if (replacedIds.length === 0) {
+                    acc.push([componentId, props]);
+                } else if (replacedIds.length === 1) {
+                    acc.push([replacedIds[0], props]);
+                } else {
+                    replacedIds.forEach((rep: any) => {
+                        acc.push([rep, props]);
+                    });
+                }
+
+                return acc;
+            }, [] as any[])
+            .forEach(([id, idProps]) => {
+                dispatch(updateComponent(id, idProps));
+            });
+    };
 }
 
 function handleServerside(
     dispatch: any,
     hooks: any,
     config: any,
-    payload: any,
+    payload: ICallbackPayload,
     long: LongCallbackInfo | undefined,
     additionalArgs: [string, string, boolean?][] | undefined,
     getState: any,
@@ -386,7 +409,7 @@ function handleServerside(
     let moreArgs = additionalArgs;
 
     if (running) {
-        sideUpdate(running.running, dispatch);
+        dispatch(sideUpdate(running.running, payload));
         runningOff = running.runningOff;
     }
 
@@ -496,10 +519,10 @@ function handleServerside(
                     dispatch(removeCallbackJob({jobId: job}));
                 }
                 if (runningOff) {
-                    sideUpdate(runningOff, dispatch);
+                    dispatch(sideUpdate(runningOff, payload));
                 }
                 if (progressDefault) {
-                    sideUpdate(progressDefault, dispatch);
+                    dispatch(sideUpdate(progressDefault, payload));
                 }
             };
 
@@ -522,11 +545,11 @@ function handleServerside(
                     }
 
                     if (data.sideUpdate) {
-                        sideUpdate(data.sideUpdate, dispatch);
+                        dispatch(sideUpdate(data.sideUpdate, payload));
                     }
 
                     if (data.progress) {
-                        sideUpdate(data.progress, dispatch);
+                        dispatch(sideUpdate(data.progress, payload));
                     }
                     if (!progressDefault && data.progressDefault) {
                         progressDefault = data.progressDefault;
@@ -671,11 +694,19 @@ export function executeCallback(
 
         const __execute = async (): Promise<CallbackResult> => {
             try {
+                const changedPropIds = keys<string>(cb.changedPropIds);
+                const parsedChangedPropsIds = changedPropIds.map(propId => {
+                    if (propId.startsWith('{')) {
+                        return parsePMCId(propId)[0];
+                    }
+                    return propId;
+                });
                 const payload: ICallbackPayload = {
                     output,
                     outputs: isMultiOutputProp(output) ? outputs : outputs[0],
                     inputs: inVals,
-                    changedPropIds: keys(cb.changedPropIds),
+                    changedPropIds,
+                    parsedChangedPropsIds,
                     state: cb.callback.state.length
                         ? fillVals(paths, layout, cb, state, 'State')
                         : undefined
@@ -721,7 +752,9 @@ export function executeCallback(
                         if (inter.length) {
                             additionalArgs.push(['cancelJob', job.jobId]);
                             if (job.progressDefault) {
-                                sideUpdate(job.progressDefault, dispatch);
+                                dispatch(
+                                    sideUpdate(job.progressDefault, payload)
+                                );
                             }
                         }
                     }
diff --git a/dash/dash-renderer/src/actions/patternMatching.ts b/dash/dash-renderer/src/actions/patternMatching.ts
@@ -0,0 +1,91 @@
+import {keys, equals, dissoc, toPairs} from 'ramda';
+import {ICallbackPayload} from '../types/callbacks';
+
+/**
+ * Deserialize pattern matching ids that come in one of the form:
+ * - '{"type":"component","index":["MATCH"]}.children'
+ * - '{"type":"component","index":["MATCH"]}'
+ *
+ * @param id The raw object as a string id.
+ * @returns The id object.
+ */
+export function parsePMCId(id: string): [any, string | undefined] {
+    let componentId, propName;
+    const index = id.lastIndexOf('}');
+    if (index + 2 < id.length) {
+        propName = id.substring(index + 2);
+        componentId = JSON.parse(id.substring(0, index + 1));
+    } else {
+        componentId = JSON.parse(id);
+    }
+    return [componentId, propName];
+}
+
+/**
+ * Get all the associated ids for an id.
+ *
+ * @param id Id to get all the pmc ids from.
+ * @param state State of the store.
+ * @param triggerKey Key to remove from the equality comparison.
+ * @returns
+ */
+export function getAllPMCIds(id: any, state: any, triggerKey: string) {
+    const keysOfIds = keys(id);
+    const idKey = keysOfIds.join(',');
+    return state.paths.objs[idKey]
+        .map((obj: any) =>
+            keysOfIds.reduce((acc, key, i) => {
+                acc[key] = obj.values[i];
+                return acc;
+            }, {} as any)
+        )
+        .filter((obj: any) =>
+            equals(dissoc(triggerKey, obj), dissoc(triggerKey, id))
+        );
+}
+
+/**
+ * Replace the pattern matching ids with the actual trigger value
+ * for MATCH, all the ids for ALL and smaller than the trigger value
+ * for ALLSMALLER.
+ *
+ * @param id The parsed id in dictionary format.
+ * @param cb Original callback info.
+ * @param index Index of the dependency in case there is more than one changed id.
+ * @param getState Function to get the state of the redux store.
+ * @returns List of replaced ids.
+ */
+export function replacePMC(
+    id: any,
+    cb: ICallbackPayload,
+    index: number,
+    getState: any
+): any[] {
+    let extras: any = [];
+    const replaced: any = {};
+    toPairs(id).forEach(([key, value]) => {
+        if (extras.length) {
+            // All done.
+            return;
+        }
+        if (Array.isArray(value)) {
+            const triggerValue = (cb.parsedChangedPropsIds[index] ||
+                cb.parsedChangedPropsIds[0])[key];
+            if (value.includes('MATCH')) {
+                replaced[key] = triggerValue;
+            } else if (value.includes('ALL')) {
+                extras = getAllPMCIds(id, getState(), key);
+            } else if (value.includes('ALLSMALLER')) {
+                extras = getAllPMCIds(id, getState(), key).filter(
+                    (obj: any) => obj[key] < triggerValue
+                );
+            }
+        } else {
+            replaced[key] = value;
+        }
+    });
+    if (extras.length) {
+        return extras;
+    }
+    return [replaced];
+}
diff --git a/dash/dash-renderer/src/types/callbacks.ts b/dash/dash-renderer/src/types/callbacks.ts
@@ -72,6 +72,7 @@ export interface IStoredCallback extends IExecutedCallback {
 
 export interface ICallbackPayload {
     changedPropIds: any[];
+    parsedChangedPropsIds: any[];
     inputs: any[];
     output: string;
     outputs: any[];
@@ -106,3 +107,7 @@ export type CallbackResponseData = {
     cancel?: ICallbackProperty[];
     sideUpdate?: any;
 };
+
+export type SideUpdateOutput = {
+    [key: string]: any;
+};
diff --git a/tests/integration/callbacks/test_wildcards.py b/tests/integration/callbacks/test_wildcards.py
@@ -2,6 +2,7 @@
 import re
 from selenium.webdriver.common.keys import Keys
 import json
+from multiprocessing import Lock
 
 from dash.testing import wait
 import dash
@@ -552,3 +553,69 @@ def update_selected_values(values):
     dash_duo.wait_for_text_to_equal(
         "#selected-values", "['option0-2', 'option1-2', 'option2-2']"
     )
+
+
+def test_cbwc008_running_match(dash_duo):
+    lock = Lock()
+    app = dash.Dash()
+
+    app.layout = [
+        html.Div(
+            [
+                html.Button(
+                    "Test1",
+                    id={"component": "button", "index": "1"},
+                ),
+                html.Button(
+                    "Test2",
+                    id={"component": "button", "index": "2"},
+                ),
+            ],
+            id="buttons",
+        ),
+        html.Div(html.Div(id={"component": "output", "index": "1"}), id="output1"),
+        html.Div(html.Div(id={"component": "output", "index": "2"}), id="output2"),
+    ]
+
+    @app.callback(
+        Output({"component": "output", "index": MATCH}, "children"),
+        Input({"component": "button", "index": MATCH}, "n_clicks"),
+        running=[
+            (
+                Output({"component": "button", "index": MATCH}, "children"),
+                "running",
+                "finished",
+            ),
+            (Output({"component": "button", "index": ALL}, "disabled"), True, False),
+        ],
+        prevent_initial_call=True,
+    )
+    def on_click(_) -> str:
+        with lock:
+            return "done"
+
+    dash_duo.start_server(app)
+
+    for i in range(1, 3):
+        with lock:
+            dash_duo.find_element(f"#buttons button:nth-child({i})").click()
+            dash_duo.wait_for_text_to_equal(
+                f"#buttons button:nth-child({i})", "running"
+            )
+            # verify all the buttons were disabled.
+            assert dash_duo.find_element("#buttons button:nth-child(1)").get_attribute(
+                "disabled"
+            )
+            assert dash_duo.find_element("#buttons button:nth-child(2)").get_attribute(
+                "disabled"
+            )
+
+        dash_duo.wait_for_text_to_equal(f"#output{i}", "done")
+        dash_duo.wait_for_text_to_equal(f"#buttons button:nth-child({i})", "finished")
+
+        assert not dash_duo.find_element("#buttons button:nth-child(1)").get_attribute(
+            "disabled"
+        )
+        assert not dash_duo.find_element("#buttons button:nth-child(2)").get_attribute(
+            "disabled"
+        )
diff --git a/tests/integration/renderer/test_request_hooks.py b/tests/integration/renderer/test_request_hooks.py
@@ -95,13 +95,15 @@ def update_output(value):
         "output": "output-1.children",
         "outputs": {"id": "output-1", "property": "children"},
         "changedPropIds": ["input.value"],
+        "parsedChangedPropsIds": ["input.value"],
         "inputs": [{"id": "input", "property": "value", "value": "fire request hooks"}],
     }
 
     assert json.loads(dash_duo.find_element("#output-post-payload").text) == {
         "output": "output-1.children",
         "outputs": {"id": "output-1", "property": "children"},
         "changedPropIds": ["input.value"],
+        "parsedChangedPropsIds": ["input.value"],
         "inputs": [{"id": "input", "property": "value", "value": "fire request hooks"}],
     }
 
diff --git a/tests/integration/test_patch.py b/tests/integration/test_patch.py
@@ -1,10 +1,13 @@
 import json
 
+import flaky
+
 from selenium.webdriver.common.keys import Keys
 
 from dash import Dash, html, dcc, Input, Output, State, ALL, Patch
 
 
+@flaky.flaky(max_runs=3)
 def test_pch001_patch_operations(dash_duo):
 
     app = Dash(__name__)

Original file line number	Diff line number	Diff line change
`@@ -95,13 +95,15 @@ def update_output(value):`
`95`	`95`	`"output": "output-1.children",`
`96`	`96`	`"outputs": {"id": "output-1", "property": "children"},`
`97`	`97`	`"changedPropIds": ["input.value"],`
	`98`	`+ "parsedChangedPropsIds": ["input.value"],`
`98`	`99`	`"inputs": [{"id": "input", "property": "value", "value": "fire request hooks"}],`
`99`	`100`	`}`
`100`	`101`
`101`	`102`	`assert json.loads(dash_duo.find_element("#output-post-payload").text) == {`
`102`	`103`	`"output": "output-1.children",`
`103`	`104`	`"outputs": {"id": "output-1", "property": "children"},`
`104`	`105`	`"changedPropIds": ["input.value"],`
	`106`	`+ "parsedChangedPropsIds": ["input.value"],`
`105`	`107`	`"inputs": [{"id": "input", "property": "value", "value": "fire request hooks"}],`
`106`	`108`	`}`
`107`	`109`