diff --git a/CHANGELOG.md b/CHANGELOG.md index 96587693ea..25049d7211 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/dash/dash-renderer/src/actions/dependencies_ts.ts b/dash/dash-renderer/src/actions/dependencies_ts.ts index 43f7902e28..50c342f5c7 100644 --- a/dash/dash-renderer/src/actions/dependencies_ts.ts +++ b/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( diff --git a/dash/dash-renderer/src/observers/requestedCallbacks.ts b/dash/dash-renderer/src/observers/requestedCallbacks.ts index 855b5f141b..e6179495d5 100644 --- a/dash/dash-renderer/src/observers/requestedCallbacks.ts +++ b/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[] = []; diff --git a/tests/integration/callbacks/test_multiple_callbacks.py b/tests/integration/callbacks/test_multiple_callbacks.py index 4442e0aac5..f78d029386 100644 --- a/tests/integration/callbacks/test_multiple_callbacks.py +++ b/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() == []