diff --git a/documentation/api/change_log.rst b/documentation/api/change_log.rst index d4e66fa650..ac947cc677 100644 --- a/documentation/api/change_log.rst +++ b/documentation/api/change_log.rst @@ -5,6 +5,9 @@ API change log .. note:: The FlexMeasures API follows its own versioning scheme. This is also reflected in the URL (e.g. `/api/v3_0`), allowing developers to upgrade at their own pace. +v3.0-39 | September 17, 2026 +"""""""""""""""""""""""""""" +- ``POST /api/v3_0/assets//automations`` now names the part of the request each validation error came from. An error in the data generator's configuration is reported under ``config``, where every error used to be reported under ``parameters``, and a report automation that names no reporter is reported under ``data-generator`` rather than as a bare message. A ``scheduling`` automation now rejects ``config`` and ``data-generator`` with a ``422`` naming the field, where it used to answer ``201 Created`` and ignore both: a schedule automation's scheduler and the flex config it runs under follow from the asset and its flex context. This is not a breaking change, as the automation endpoints have not been part of a release. v3.0-38 | September 16, 2026 """""""""""""""""""""""""""" - ``GET /api/v3_0/assets//automations`` now lists the automations of the assets below the asset as well, at any depth, so that a site asset reports everything that runs below it, and each entry names the asset it is defined on in ``asset`` and ``asset-name``. ``GET /api/v3_0/assets//jobs`` lists the jobs of the assets below the asset in the same way. Pass ``include-child-assets=false`` to either one to list only what belongs to the asset itself. Only the assets below it which the caller may read are included, as a child asset can belong to another organisation than its parent. diff --git a/documentation/changelog.rst b/documentation/changelog.rst index b73682eece..4663523e9a 100644 --- a/documentation/changelog.rst +++ b/documentation/changelog.rst @@ -88,6 +88,7 @@ Automations arrived over several pull requests. This is what each of them contri * Every automation times its runs the same way: a fixed ``start``, ``end`` or ``prior`` in its parameters is refused, as every run would share that moment, and two of ``start-offset``, ``end-offset`` and ``duration`` describe the period each run covers instead, with the offsets applied to the time the run was due on the automation's own clock, so that, for instance, a schedule automation can plan the whole of the next day [see `PR #2551 `_] * Look up automations from the command line with ``flexmeasures show automations``, which lists them all (inactive ones included) with the IDs that the edit, delete and run commands expect, and, with ``--id``, shows a single automation's recurrence, cursor, parameters and the sensors it reads from and writes to [see `PR #2533 `_] * The *Automations* page lists the automations of the assets below an asset too, says when each automation runs next, keeps itself up to date, and shows the configuration of an automation's data source, which the *New automation* form now also lets you set [see `PR #2554 `_] +* An automation that is refused now names the part of the request at fault: a mistake in a data generator's configuration is reported against ``config`` rather than against ``parameters``, and a report automation that names no reporter is reported against ``data-generator``. A schedule automation now refuses a ``config`` or a ``data-generator`` by name, where it used to accept and silently ignore both, since its scheduler and the flex config it runs under follow from the asset and its flex context [see `PR #2563 `_] v1.0.1 | September 9, 2026 diff --git a/flexmeasures/api/v3_0/assets.py b/flexmeasures/api/v3_0/assets.py index 77e0c53487..c5bce14e9d 100644 --- a/flexmeasures/api/v3_0/assets.py +++ b/flexmeasures/api/v3_0/assets.py @@ -1753,7 +1753,9 @@ def post_automation(self, automation_data: dict, id: int, asset: GenericAsset): asset, origin="API", check_permissions=True, **automation_data ) except ValidationError as e: - return unprocessable_entity({"parameters": e.messages}) + # The service names the part of the request each error came from, + # so that an error in the config is not reported against the parameters. + return unprocessable_entity(e.messages) except AutomationSensorsUnknown as e: return unprocessable_entity(str(e)) except ValueError as e: diff --git a/flexmeasures/api/v3_0/tests/test_automations_api.py b/flexmeasures/api/v3_0/tests/test_automations_api.py index 3e63b8bb87..96b67672d6 100644 --- a/flexmeasures/api/v3_0/tests/test_automations_api.py +++ b/flexmeasures/api/v3_0/tests/test_automations_api.py @@ -2,11 +2,12 @@ from __future__ import annotations +import json from datetime import datetime, timedelta, timezone import pytest from flask import url_for -from sqlalchemy import select +from sqlalchemy import func, select from flexmeasures.data.models.automations import Automation from flexmeasures.data.models.data_sources import DataSource @@ -14,6 +15,14 @@ from flexmeasures.data.models.time_series import Sensor +def _with_sensor(parameters: dict, sensor_id: int) -> dict: + """Fill in the sensor id that a parametrised payload leaves as "SENSOR". + + The id only exists once the fixtures have run, which is after the parameters are written. + """ + return json.loads(json.dumps(parameters).replace('"SENSOR"', str(sensor_id))) + + @pytest.fixture(scope="function") def add_automations(fresh_db, add_battery_assets_fresh_db): battery = add_battery_assets_fresh_db["Test battery"] @@ -933,6 +942,247 @@ def test_a_forecast_automation_names_the_data_generator_it_runs( fresh_db.session.flush() +@pytest.mark.parametrize( + "automation_type, data_generator, config, parameters", + [ + ( + "forecasting", + "TrainPredictPipeline", + {"not-a-config-field": 1}, + {"sensor": "SENSOR"}, + ), + ( + "reporting", + "PandasReporter", + { + "required_input": [{"name": "flow"}], + "required_output": [{"name": "flow"}], + "transformations": [], + "not-a-config-field": 1, + }, + { + "input": [{"name": "flow", "sensor": "SENSOR"}], + "output": [{"name": "flow", "sensor": "SENSOR"}], + }, + ), + ], +) +@pytest.mark.parametrize( + "requesting_user", ["test_prosumer_user_2@seita.nl"], indirect=True +) +def test_post_automation_reports_a_config_error_against_the_config( + app, + fresh_db, + add_battery_assets_fresh_db, + requesting_user, + automation_type, + data_generator, + config, + parameters, +): + """A fault in the data generator's config is reported against `config`, not against `parameters`. + + Both are validated by schemas of the data generator's choosing, so naming the wrong one + sends the caller looking for a mistake in a part of the request that is fine. + """ + battery = add_battery_assets_fresh_db["Test battery"] + sensor_id = battery.sensors[0].id + parameters = _with_sensor(parameters, sensor_id) + + with app.test_client() as client: + response = client.post( + url_for("AssetAPI:post_automation", id=battery.id), + json={ + "name": "Bad config", + "cron": "0 6 * * *", + "type": automation_type, + "data-generator": data_generator, + "config": config, + "parameters": parameters, + }, + ) + + assert response.status_code == 422, response.json + messages = response.json["message"]["json"] + assert "not-a-config-field" in str(messages["config"]) + assert "parameters" not in messages + + +@pytest.mark.parametrize( + "automation_type, data_generator, config, parameters", + [ + ( + "forecasting", + "TrainPredictPipeline", + {}, + {"sensor": "SENSOR", "not-a-parameter": 1}, + ), + ( + "reporting", + "PandasReporter", + { + "required_input": [{"name": "flow"}], + "required_output": [{"name": "flow"}], + "transformations": [], + }, + { + "input": [{"name": "flow", "sensor": "SENSOR"}], + "output": [{"name": "flow", "sensor": "SENSOR"}], + "not-a-parameter": 1, + }, + ), + ("scheduling", None, {}, {"duration": "PT12H", "not-a-parameter": 1}), + ], +) +@pytest.mark.parametrize( + "requesting_user", ["test_prosumer_user_2@seita.nl"], indirect=True +) +def test_post_automation_reports_a_parameter_error_against_the_parameters( + app, + fresh_db, + add_battery_assets_fresh_db, + requesting_user, + automation_type, + data_generator, + config, + parameters, +): + """A fault in the parameters is reported against `parameters`, for every automation type.""" + battery = add_battery_assets_fresh_db["Test battery"] + parameters = _with_sensor(parameters, battery.sensors[0].id) + payload = { + "name": "Bad parameters", + "cron": "0 6 * * *", + "type": automation_type, + "parameters": parameters, + } + if data_generator is not None: + payload["data-generator"] = data_generator + payload["config"] = config + + with app.test_client() as client: + response = client.post( + url_for("AssetAPI:post_automation", id=battery.id), json=payload + ) + + assert response.status_code == 422, response.json + messages = response.json["message"]["json"] + assert "not-a-parameter" in str(messages["parameters"]) + assert "config" not in messages + + +@pytest.mark.parametrize( + "field, value", + [ + ("config", {"model": "CustomLGBM"}), + ("data-generator", "TrainPredictPipeline"), + ], +) +@pytest.mark.parametrize( + "requesting_user", ["test_prosumer_user_2@seita.nl"], indirect=True +) +def test_post_schedule_automation_rejects_a_data_generator_and_its_config( + app, fresh_db, add_battery_assets_fresh_db, requesting_user, field, value +): + """A schedule automation resolves its own scheduler and flex config from the asset. + + Taking either field here would record a choice that nothing goes on to read, + so each is refused by name rather than silently ignored. + """ + battery = add_battery_assets_fresh_db["Test battery"] + with app.test_client() as client: + response = client.post( + url_for("AssetAPI:post_automation", id=battery.id), + json={ + "name": "Schedules with an unusable field", + "cron": "0 6 * * *", + "type": "scheduling", + "parameters": {"duration": "PT12H"}, + field: value, + }, + ) + + assert response.status_code == 422, response.json + assert field in response.json["message"]["json"] + assert ( + fresh_db.session.execute( + select(Automation).filter_by(name="Schedules with an unusable field") + ).scalar_one_or_none() + is None + ) + + +@pytest.mark.parametrize( + "requesting_user", ["test_prosumer_user_2@seita.nl"], indirect=True +) +def test_post_report_automation_without_a_reporter_names_the_field_to_fill_in( + app, fresh_db, add_battery_assets_fresh_db, requesting_user +): + """A report automation has to name its reporter, and the error says which field is missing.""" + battery = add_battery_assets_fresh_db["Test battery"] + with app.test_client() as client: + response = client.post( + url_for("AssetAPI:post_automation", id=battery.id), + json={ + "name": "Reporter-less report", + "cron": "0 1 * * *", + "type": "reporting", + "parameters": {"input": [{"sensor": battery.sensors[0].id}]}, + }, + ) + + assert response.status_code == 422, response.json + assert "A reporter is required" in str( + response.json["message"]["json"]["data-generator"] + ) + + +@pytest.mark.parametrize( + "requesting_user", ["test_prosumer_user_2@seita.nl"], indirect=True +) +def test_a_refused_automation_leaves_nothing_behind( + app, fresh_db, add_battery_assets_fresh_db, requesting_user +): + """A rejected request records neither the automation, nor a data source for its generator, nor an audit log entry.""" + from flexmeasures.data.models.audit_log import AssetAuditLog + + battery = add_battery_assets_fresh_db["Test battery"] + sensor_id = battery.sensors[0].id + before = { + model: fresh_db.session.scalar(select(func.count()).select_from(model)) + for model in (Automation, DataSource, AssetAuditLog) + } + refused = [ + { + "type": "forecasting", + "config": {"not-a-config-field": 1}, + "parameters": {"sensor": sensor_id}, + }, + { + "type": "forecasting", + "parameters": {"sensor": sensor_id, "not-a-parameter": 1}, + }, + { + "type": "scheduling", + "data-generator": "TrainPredictPipeline", + "parameters": {"duration": "PT12H"}, + }, + ] + with app.test_client() as client: + for index, payload in enumerate(refused): + response = client.post( + url_for("AssetAPI:post_automation", id=battery.id), + json={"name": f"Refused {index}", "cron": "0 6 * * *", **payload}, + ) + assert response.status_code == 422, response.json + + after = { + model: fresh_db.session.scalar(select(func.count()).select_from(model)) + for model in (Automation, DataSource, AssetAuditLog) + } + assert after == before + + @pytest.fixture(scope="function") def add_automation_on_a_child_asset(fresh_db, add_battery_assets_fresh_db): """Put an automation on a sub-asset of the battery, where automations usually live.""" diff --git a/flexmeasures/cli/data_add.py b/flexmeasures/cli/data_add.py index 84bd333831..726df1f9b8 100755 --- a/flexmeasures/cli/data_add.py +++ b/flexmeasures/cli/data_add.py @@ -1966,8 +1966,9 @@ def add_automation( origin="CLI", ) except ValidationError as e: + # The messages name the part of the request at fault, which is not always the parameters. click.secho( - f"Invalid {Automation.RESULT_NOUNS[automation_type]} parameters: {e.messages}", + f"Invalid {Automation.RESULT_NOUNS[automation_type]} automation: {e.messages}", **MsgStyle.ERROR, ) raise click.Abort() diff --git a/flexmeasures/cli/tests/test_automations.py b/flexmeasures/cli/tests/test_automations.py index de6b05d706..bd5ddea0ba 100644 --- a/flexmeasures/cli/tests/test_automations.py +++ b/flexmeasures/cli/tests/test_automations.py @@ -914,7 +914,11 @@ def test_add_schedule_automation(app, fresh_db, setup_dummy_data, tmp_path): ], ) # fmt: skip assert result.exit_code != 0 - assert "Invalid schedule parameters" in result.output + # The error names the part of the request at fault, which for a schedule automation is always the parameters. + assert "Invalid schedule automation" in result.output + assert ( + "{'parameters': {'not-a-trigger-field': ['Unknown field.']}}" in result.output + ) # minimal valid parameters (flex config can live on the asset) parameters_file.write_text('duration: "PT12H"\n') @@ -1029,7 +1033,8 @@ def test_add_schedule_automation_rejects_unsupported_durations( ) assert result.exit_code != 0 - assert "Invalid schedule parameters" in result.output + assert "Invalid schedule automation" in result.output + assert "'parameters'" in result.output def test_add_schedule_automation_rejects_forecast_config( @@ -1103,7 +1108,45 @@ def test_add_forecast_automation_still_requires_sensor(app, fresh_db, setup_dumm ) assert result.exit_code != 0 - assert "Invalid forecast parameters" in result.output + assert "Invalid forecast automation" in result.output + assert ( + "{'parameters': {'sensor': ['Missing data for required field.']}}" + in result.output + ) + + +def test_add_forecast_automation_reports_a_config_error_against_the_config( + app, fresh_db, setup_dummy_data, tmp_path +): + """A fault in the forecaster's config is reported against the config, rather than against the parameters. + + Both are validated by schemas of the data generator's choosing, so naming the wrong one + sends the user looking for a mistake in a part of the command that is fine. + """ + from flexmeasures.cli.data_add import add_automation + + config_file = tmp_path / "config.yml" + config_file.write_text("not-a-config-field: 1\n") + result = app.test_cli_runner().invoke( + add_automation, + [ + "--asset", "1", + "--name", "Bad forecaster config", + "--cron", "0 6 * * *", + "--sensor", str(setup_dummy_data[0]), + "--config", str(config_file), + ], + ) # fmt: skip + + assert result.exit_code != 0 + assert "Invalid forecast automation" in result.output + assert "{'config': {'not-a-config-field': ['Unknown field.']}}" in result.output + assert ( + fresh_db.session.execute( + select(Automation).filter_by(name="Bad forecaster config") + ).scalar_one_or_none() + is None + ) @pytest.mark.parametrize("is_dst", (True, False)) diff --git a/flexmeasures/data/services/automations.py b/flexmeasures/data/services/automations.py index 120e2fcf5d..84a3065203 100644 --- a/flexmeasures/data/services/automations.py +++ b/flexmeasures/data/services/automations.py @@ -4,6 +4,7 @@ from __future__ import annotations +from contextlib import contextmanager from copy import copy, deepcopy from dataclasses import dataclass from datetime import datetime, timedelta, timezone @@ -1116,6 +1117,45 @@ def get_automation_job_stats(automation: Automation) -> dict[str, int]: return counts +def refuse_fields_a_schedule_automation_cannot_use( + config: dict | None, generator_class: str | None +) -> None: + """Refuse a schedule automation's config or data generator, which it has no use for. + + A schedule automation's scheduler, and the flex config it runs under, follow from the asset and its flex context. + Accepting either field here would record a choice that nothing goes on to read, + leaving the caller to believe it applied. + + :raises marshmallow.ValidationError: keyed by the field(s) given, if either was. + """ + unsupported = {} + if config: + unsupported["config"] = [ + "A schedule automation configures no data generator of its own:" + " its scheduler and flex config follow from the asset and its flex context." + ] + if generator_class: + unsupported["data-generator"] = [ + "A schedule automation does not choose a data generator:" + " its scheduler follows from the asset." + ] + if unsupported: + raise ValidationError(unsupported) + + +@contextmanager +def errors_reported_for(section: str): + """Name the part of the request a validation error came from, so the caller can say which one to fix. + + Both `config` and `parameters` are validated against schemas of the data generator's choosing, + and either can raise. Without this, one is indistinguishable from the other by the time it surfaces. + """ + try: + yield + except ValidationError as error: + raise ValidationError({section: error.messages}) from error + + def _stored_sensor_id(sensor_reference: Any) -> int | None: """Return the sensor ID from a stored automation parameter naming a sensor. @@ -1143,7 +1183,8 @@ def _prepare_forecast_automation( from flexmeasures.data.services.data_sources import get_data_generator warnings = [] - deserialized_parameters = ForecasterParametersSchema().load(parameters) + with errors_reported_for("parameters"): + deserialized_parameters = ForecasterParametersSchema().load(parameters) sensor = deserialized_parameters.get("sensor") # A target may be given as a source-filtered reference, whose filters say which beliefs to train on, and not which sensor is meant. if isinstance(sensor, SensorReference): @@ -1153,13 +1194,14 @@ def _prepare_forecast_automation( f"The sensor to forecast ({sensor.id}) does not belong to asset {asset.id}." ) model = generator_class or "TrainPredictPipeline" - forecaster = get_data_generator( - source=source, - model=model, - config=config or {}, - save_config=True, - data_generator_type=Forecaster, - ) + with errors_reported_for("config"): + forecaster = get_data_generator( + source=source, + model=model, + config=config or {}, + save_config=True, + data_generator_type=Forecaster, + ) if forecaster is None: # With a source, the class and its config come from the source, so the source is what failed. if source is not None: @@ -1177,22 +1219,25 @@ def _prepare_report_automation( source, ) -> tuple[Reporter, dict, list[str]]: """Validate report automation parameters without creating a data source.""" - from marshmallow import ValidationError - from flexmeasures.data.services.data_sources import get_data_generator warnings: list[str] = [] if generator_class is None and source is None: raise ValidationError( - "A reporter is required for report automations (e.g. PandasReporter)." + { + "data-generator": [ + "A reporter is required for report automations (e.g. PandasReporter)." + ] + } + ) + with errors_reported_for("config"): + reporter = get_data_generator( + source=source, + model=generator_class, + config=config or {}, + save_config=True, + data_generator_type=Reporter, ) - reporter = get_data_generator( - source=source, - model=generator_class, - config=config or {}, - save_config=True, - data_generator_type=Reporter, - ) if reporter is None: # With a source, the class and its config come from the source, so the source is what failed. if source is not None: @@ -1200,9 +1245,10 @@ def _prepare_report_automation( raise ValueError(f"Could not set up reporter '{generator_class}'.") # Validate with the chosen reporter's own parameters schema, # which may extend the base ReporterParametersSchema. - deserialized_parameters = reporter._parameters_schema.load( - prepare_report_parameters(parameters, cronstr, automation_timezone) - ) + with errors_reported_for("parameters"): + deserialized_parameters = reporter._parameters_schema.load( + prepare_report_parameters(parameters, cronstr, automation_timezone) + ) return reporter, deserialized_parameters, warnings @@ -1234,8 +1280,6 @@ def create_automation( :raises werkzeug.exceptions.Forbidden: if a sensor is not accessible to the user. :returns: the automation and a list of warnings. """ - from marshmallow import ValidationError - from flexmeasures.data.models.audit_log import AssetAuditLog parameters = parameters or {} @@ -1248,12 +1292,17 @@ def create_automation( if automation_type in Automation.SUPPORTED_TYPES: # An automation runs again and again, so a moment fixed in its parameters would be shared by every run. refuse_fixed_moments(parameters, automation_type) - validate_automation_window(parameters, automation_type) + with errors_reported_for("parameters"): + validate_automation_window(parameters, automation_type) if automation_type == "forecasting": + with errors_reported_for("parameters"): + forecast_window = resolve_automation_window( + parameters, automation_type, timezone + ) forecaster, deserialized_parameters, forecast_warnings = ( _prepare_forecast_automation( asset, - resolve_automation_window(parameters, automation_type, timezone), + forecast_window, generator_class, config, source, @@ -1276,12 +1325,16 @@ def create_automation( find_momentary_flex_config_fields, ) + refuse_fields_a_schedule_automation_cannot_use(config, generator_class) + # The flex config has to describe the site and its devices, rather than one moment: # the automation computes a fresh schedule on every run, # so a value tied to a fixed moment would be stale on the next one. - momentary_fields = find_momentary_flex_config_fields( - prepare_schedule_trigger_message(dict(parameters), asset.id, timezone) - ) + with errors_reported_for("parameters"): + trigger_message = prepare_schedule_trigger_message( + dict(parameters), asset.id, timezone + ) + momentary_fields = find_momentary_flex_config_fields(trigger_message) if momentary_fields: raise RecurringAutomationFixesAMoment( f"{flexmeasures_inflection.join_words_into_a_list(momentary_fields)} fixes a moment in time," @@ -1292,9 +1345,10 @@ def create_automation( # A schedule is recorded on the sensors that the scheduler returns its results for, # and reads whatever other sensors the flex-model and flex-context refer to, # such as price sensors and the sensors of inflexible devices. - schedule_sensors = resolve_schedule_automation_sensors( - parameters, asset.id, timezone - ) + with errors_reported_for("parameters"): + schedule_sensors = resolve_schedule_automation_sensors( + parameters, asset.id, timezone + ) input_sensors = schedule_sensors["input_sensors"] output_sensors = schedule_sensors["output_sensors"] elif automation_type == "reporting": @@ -1310,7 +1364,11 @@ def create_automation( output_sensors = report_sensors["output_sensors"] else: raise ValidationError( - f"Automation type '{automation_type}' is not supported (supported types: {Automation.SUPPORTED_TYPES})." + { + "type": [ + f"Automation type '{automation_type}' is not supported (supported types: {Automation.SUPPORTED_TYPES})." + ] + } ) if check_permissions: