Merge pull request #2344 from plotly/1519-callbacks-waiting-fix

1519 callbacks waiting fix

Author: alexcjohnson

CHANGELOG.md

@@ -6,6 +6,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 
+- [#2344](https://github.com/plotly/dash/pull/2344) Fix [#1519](https://github.com/plotly/dash/issues/1519), a case where dependent callbacks can be called too many times and with inconsistent inputs
 - [#2332](https://github.com/plotly/dash/pull/2332) Add key to wrapped children props in list.
 - [#2336](https://github.com/plotly/dash/pull/2336) Fix inserted dynamic ids in component as props.
 

dash/dash-renderer/src/actions/dependencies_ts.ts

@@ -145,10 +145,52 @@ export function getPriority(
     return map(i => Math.min(i, 35).toString(36), priority).join('');
 }
 
+export function getAllSubsequentOutputsForCallback(
+    graphs: any,
+    paths: any,
+    callback: ICallback
+) {
+    let callbacks: ICallback[] = [callback];
+    let touchedOutputs: {[key: string]: boolean} = {};
+
+    // this traverses the graph all the way to the end
+    while (callbacks.length) {
+        // don't add it if it already exists based on id and props
+        const outputs = filter(
+            o => !touchedOutputs[combineIdAndProp(o)],
+            flatten(map(cb => flatten(cb.getOutputs(paths)), callbacks))
+        );
+
+        touchedOutputs = reduce(
+            (touched, o) => assoc(combineIdAndProp(o), true, touched),
+            touchedOutputs,
+            outputs
+        );
+
+        callbacks = flatten(
+            map(
+                ({id, property}: any) =>
+                    getCallbacksByInput(
+                        graphs,
+                        paths,
+                        id,
+                        property,
+                        INDIRECT,
+                        false
+                    ),
+                outputs
+            )
+        );
+    }
+
+    return touchedOutputs;
+}
+
 export const getReadyCallbacks = (
     paths: any,
     candidates: ICallback[],
-    callbacks: ICallback[] = candidates
+    callbacks: ICallback[] = candidates,
+    graphs: any = {}
 ): ICallback[] => {
     // Skip if there's no candidates
     if (!candidates.length) {
@@ -166,9 +208,31 @@ export const getReadyCallbacks = (
     );
 
     // Make `outputs` hash table for faster access
-    const outputsMap: {[key: string]: boolean} = {};
+    let outputsMap: {[key: string]: boolean} = {};
     forEach(output => (outputsMap[output] = true), outputs);
 
+    // find all the outputs touched by activeCallbacks
+    // remove this check if graph is accessible all the time
+
+    if (Object.keys(graphs).length) {
+        //not sure if graph will be accessible all the time
+        const allTouchedOutputs: {[key: string]: boolean}[] = flatten(
+            map(
+                cb => getAllSubsequentOutputsForCallback(graphs, paths, cb),
+                callbacks
+            )
+        );
+
+        // overrrides the outputsMap, will duplicate callbacks filtered
+        // this is only done to silence typescript errors
+        if (allTouchedOutputs.length > 0) {
+            outputsMap = Object.assign(
+                allTouchedOutputs[0],
+                ...allTouchedOutputs
+            );
+        }
+    }
+
     // Find `requested` callbacks that do not depend on a outstanding output (as either input or state)
     // Outputs which overlap an input do not count as an outstanding output
     return filter(

dash/dash-renderer/src/observers/requestedCallbacks.ts

@@ -62,7 +62,8 @@ const observer: IStoreObserverDefinition<IStoreState> = {
         const {
             callbacks,
             callbacks: {prioritized, blocked, executing, watched, stored},
-            paths
+            paths,
+            graphs
         } = getState();
         let {
             callbacks: {requested}
@@ -234,7 +235,8 @@ const observer: IStoreObserverDefinition<IStoreState> = {
         let readyCallbacks = getReadyCallbacks(
             paths,
             requested,
-            pendingCallbacks
+            pendingCallbacks,
+            graphs
         );
 
         let oldBlocked: ICallback[] = [];

tests/integration/callbacks/test_multiple_callbacks.py

@@ -1,11 +1,13 @@
 import time
-from multiprocessing import Value
+from multiprocessing import Value, Lock
 
 import pytest
 
 from dash import Dash, Input, Output, State, callback_context, html, dcc, dash_table
 from dash.exceptions import PreventUpdate
 
+import dash.testing.wait as wait
+
 
 def test_cbmt001_called_multiple_times_and_out_of_order(dash_duo):
     app = Dash(__name__)
@@ -17,7 +19,7 @@ def test_cbmt001_called_multiple_times_and_out_of_order(dash_duo):
 
     @app.callback(Output("output", "children"), [Input("input", "n_clicks")])
     def update_output(n_clicks):
-        call_count.value = call_count.value + 1
+        call_count.value += 1
         if n_clicks == 1:
             time.sleep(1)
         return n_clicks
@@ -578,3 +580,91 @@ def callback(*args):
         assert call_counts[outputid].value == 1
 
     assert call_counts["container"].value == (1 if generate else 0)
+
+
+def test_cbmt013_chained_callback_should_be_blocked(dash_duo):
+    all_options = {
+        "America": ["New York City", "San Francisco", "Cincinnati"],
+        "Canada": ["Montreal", "Toronto", "Ottawa"],
+    }
+
+    app = Dash(__name__)
+    app.layout = html.Div(
+        [
+            dcc.RadioItems(
+                id="countries-radio",
+                options=[{"label": k, "value": k} for k in all_options.keys()],
+                value="America",
+            ),
+            html.Hr(),
+            dcc.RadioItems(id="cities-radio"),
+            html.Hr(),
+            html.Div(id="display-selected-values"),
+        ]
+    )
+
+    opts_call_count = Value("i", 0)
+    city_call_count = Value("i", 0)
+    out_call_count = Value("i", 0)
+    out_lock = Lock()
+
+    @app.callback(Output("cities-radio", "options"), Input("countries-radio", "value"))
+    def set_cities_options(selected_country):
+        opts_call_count.value += 1
+        return [{"label": i, "value": i} for i in all_options[selected_country]]
+
+    @app.callback(Output("cities-radio", "value"), Input("cities-radio", "options"))
+    def set_cities_value(available_options):
+        city_call_count.value += 1
+        return available_options[0]["value"]
+
+    @app.callback(
+        Output("display-selected-values", "children"),
+        Input("countries-radio", "value"),
+        Input("cities-radio", "value"),
+    )
+    def set_display_children(selected_country, selected_city):
+        # this may actually be the key to this whole test:
+        # these inputs should never be out of sync.
+        assert selected_city in all_options[selected_country]
+
+        out_call_count.value += 1
+        with out_lock:
+            return "{} is a city in {}".format(
+                selected_city,
+                selected_country,
+            )
+
+    dash_duo.start_server(app)
+
+    new_york_text = "New York City is a city in America"
+    canada_text = "Montreal is a city in Canada"
+
+    # If we get to the correct initial state with only one call of each callback,
+    # then there mustn't have been any intermediate changes to the output text
+    dash_duo.wait_for_text_to_equal("#display-selected-values", new_york_text)
+    assert opts_call_count.value == 1
+    assert city_call_count.value == 1
+    assert out_call_count.value == 1
+
+    all_labels = dash_duo.find_elements("label")
+    canada_opt = next(
+        i for i in all_labels if i.text == "Canada"
+    ).find_element_by_tag_name("input")
+
+    with out_lock:
+        canada_opt.click()
+
+        # all three callbacks have fired once more, but since we haven't allowed the
+        # last one to execute, the output hasn't been changed
+        wait.until(lambda: out_call_count.value == 2, timeout=3)
+        assert opts_call_count.value == 2
+        assert city_call_count.value == 2
+        assert dash_duo.find_element("#display-selected-values").text == new_york_text
+
+    dash_duo.wait_for_text_to_equal("#display-selected-values", canada_text)
+    assert opts_call_count.value == 2
+    assert city_call_count.value == 2
+    assert out_call_count.value == 2
+
+    assert dash_duo.get_logs() == []