From 4c0e1019db0bcb0753a5ce7887ec29021aa44c46 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:15:42 +0200 Subject: [PATCH 1/3] Add failing tests for repeated asset targets in batch dispatch --- manager/build.gradle | 2 +- .../rules/facade/AssetsFacadeTest.groovy | 82 +++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) create mode 100644 manager/src/test/groovy/org/openremote/manager/rules/facade/AssetsFacadeTest.groovy diff --git a/manager/build.gradle b/manager/build.gradle index 4c3d6d3d23..7ae6cc236c 100644 --- a/manager/build.gradle +++ b/manager/build.gradle @@ -79,7 +79,7 @@ dependencies { sourceSets { test { - groovy.srcDirs = ['test/src/groovy'] + groovy.srcDirs = ['src/test/groovy', 'test/src/groovy'] } } diff --git a/manager/src/test/groovy/org/openremote/manager/rules/facade/AssetsFacadeTest.groovy b/manager/src/test/groovy/org/openremote/manager/rules/facade/AssetsFacadeTest.groovy new file mode 100644 index 0000000000..df818120af --- /dev/null +++ b/manager/src/test/groovy/org/openremote/manager/rules/facade/AssetsFacadeTest.groovy @@ -0,0 +1,82 @@ +/* + * Copyright 2026, OpenRemote Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + * SPDX-License-Identifier: AGPL-3.0-or-later + */ +package org.openremote.manager.rules.facade + +import org.openremote.manager.asset.AssetStorageService +import org.openremote.manager.rules.RulesEngineId +import org.openremote.model.asset.impl.ThingAsset +import org.openremote.model.attribute.AttributeEvent +import org.openremote.model.query.AssetQuery +import spock.lang.Specification + +class AssetsFacadeTest extends Specification { + + def "#scope scope dispatches every event in order for #batchCase"() { + given: "two assets accessible to the rule engine" + def root = Stub(ThingAsset) { + getId() >> "root" + getPath() >> (["root"] as String[]) + } + def child = Stub(ThingAsset) { + getId() >> "child" + } + def availableAssets = [root, child] + List queries = [] + def storage = Stub(AssetStorageService) { + find("root", true) >> root + findAll(_ as AssetQuery) >> { AssetQuery query -> + queries.add(query) + // An asset-ID query returns each matching asset once, even for repeated IDs. + availableAssets.findAll { query.ids.contains(it.id) } + } + } + def engineId = scope == "global" ? new RulesEngineId() : + scope == "realm" ? new RulesEngineId("building") : new RulesEngineId("building", "root") + List forwarded = [] + def facade = new AssetsFacade(engineId, storage, { AttributeEvent event -> + forwarded.add(event) + }) + def events = targetIds.withIndex().collect { String id, int index -> + new AttributeEvent(id, "attribute${index}", index) + } + + when: "one call dispatches updates to different attributes on the permitted targets" + facade.dispatch(events as AttributeEvent[]) + + then: "target lookup retains the engine scope" + queries.size() == 1 + queries[0].ids.toSet() == targetIds.toSet() + queries[0].realm?.name == (scope == "global" ? null : "building") + queries[0].paths?.collect { it.path.toList() } == (scope == "asset" ? [["root"]] : null) + + and: "every original event is forwarded exactly once, in the supplied order" + forwarded.size() == events.size() + events.indices.every { forwarded[it].is(events[it]) } + + where: + [scope, batchCase, targetIds] << ["global", "realm", "asset"].collectMany { scope -> + [ + ["a single event", ["root"]], + ["distinct targets", ["root", "child"]], + ["two events for one target", ["root", "root"]], + ["mixed repeated and distinct targets", ["root", "child", "root"]] + ].collect { row -> [scope] + row } + } + } +} From af63bcac23149f676617ba0420bb4ceabe9712d0 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:17:18 +0200 Subject: [PATCH 2/3] Fix batch dispatch validation for repeated asset targets --- .../org/openremote/manager/rules/facade/AssetsFacade.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/manager/src/main/java/org/openremote/manager/rules/facade/AssetsFacade.java b/manager/src/main/java/org/openremote/manager/rules/facade/AssetsFacade.java index fbf894d627..8254435aa0 100644 --- a/manager/src/main/java/org/openremote/manager/rules/facade/AssetsFacade.java +++ b/manager/src/main/java/org/openremote/manager/rules/facade/AssetsFacade.java @@ -102,8 +102,9 @@ public Stream> getResults(AssetQuery assetQuery) { public AssetsFacade dispatch(AttributeEvent... events) { if (events == null || events.length == 0) return this; - // Check if the asset ID of every event can be found with the default security of this facade - String[] ids = Arrays.stream(events).map(AttributeEvent::getId).toArray(String[]::new); + // Validate each distinct target asset while retaining every event for dispatch below. + String[] ids = + Arrays.stream(events).map(AttributeEvent::getId).distinct().toArray(String[]::new); AssetQuery query = new AssetQuery().ids(ids); long count = this.getResults(query).count(); From 0672acf64e1f23637fdb655777869592a8f63b02 Mon Sep 17 00:00:00 2001 From: Eric Bariaux <375613+ebariaux@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:22:58 +0200 Subject: [PATCH 3/3] Add tests for rejecting batches with missing or out-of-scope assets --- .../rules/facade/AssetsFacadeTest.groovy | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/manager/src/test/groovy/org/openremote/manager/rules/facade/AssetsFacadeTest.groovy b/manager/src/test/groovy/org/openremote/manager/rules/facade/AssetsFacadeTest.groovy index df818120af..7f7db3f2ed 100644 --- a/manager/src/test/groovy/org/openremote/manager/rules/facade/AssetsFacadeTest.groovy +++ b/manager/src/test/groovy/org/openremote/manager/rules/facade/AssetsFacadeTest.groovy @@ -79,4 +79,56 @@ class AssetsFacadeTest extends Specification { ].collect { row -> [scope] + row } } } + + def "#scope scope rejects the whole batch for #rejectedTarget with repeated permitted targets: #repeated"() { + given: "the scoped asset lookup excludes a missing or inaccessible target" + def root = Stub(ThingAsset) { + getId() >> "root" + getPath() >> (["root"] as String[]) + } + def child = Stub(ThingAsset) { + getId() >> "child" + } + List queries = [] + def storage = Stub(AssetStorageService) { + find("root", true) >> root + findAll(_ as AssetQuery) >> { AssetQuery query -> + queries.add(query) + // Model a partial result from the scoped lookup: only permitted assets were found. + [root, child].findAll { query.ids.contains(it.id) } + } + } + def engineId = scope == "global" ? new RulesEngineId() : + scope == "realm" ? new RulesEngineId("building") : new RulesEngineId("building", "root") + List forwarded = [] + def facade = new AssetsFacade(engineId, storage, { AttributeEvent event -> + forwarded.add(event) + }) + def targetIds = (repeated ? ["root", "child", "root"] : ["root", "child"]) + [rejectedTarget] + def events = targetIds.withIndex().collect { String id, int index -> + new AttributeEvent(id, "attribute${index}", index) + } + + when: "the rejected target is last, after events for permitted assets" + facade.dispatch(events as AttributeEvent[]) + + then: "the lookup includes every target and retains the engine scope" + queries.size() == 1 + queries[0].ids.toSet() == targetIds.toSet() + queries[0].realm?.name == (scope == "global" ? null : "building") + queries[0].paths?.collect { it.path.toList() } == (scope == "asset" ? [["root"]] : null) + + and: "no events are forwarded, including those for permitted assets" + forwarded.empty + + where: + [scope, rejectedTarget, repeated] << [ + ["global", "missing"], + ["realm", "missing"], + ["realm", "foreign-realm"], + ["asset", "missing"], + ["asset", "foreign-realm"], + ["asset", "outside-subtree"] + ].collectMany { row -> [false, true].collect { repeated -> row + [repeated] } } + } }