Fix the handling of subscriptions with multiple topics and extra fields by abalarev · Pull Request #2425 · eclipse-ditto/ditto

abalarev · 2026-04-24T14:58:08Z

Summary
Resolves incorrect handling of subscriptions for multiple topics, containing at least one extra field.

Issue

multiple topics without extraFields works ok - sends to the matching ones
having multiple topics with extraFields works for only one of them (when it's matching), but not deterministic for which one
having a topic or topics with extraFields and some without extraFields works again only for one of the topics with extraFields (when it's matching and not for each topic), but for none of the topics without extraFields

What's changed
OutboundMaooingProcessorActor flow changed as follows:

retrieving of extra fields for all topics (per target) combined in a single request
the filter is now applied using already retrieved extra fields as it needs them to filter correctly. Only those extra fields defined in the matching filter are left in the outbound signal
splitting of targets with and without extra fields is now removed as it became redundant
the new flow is still optimized for targets with no extra fields in the topics.
outbound signal ordering is now changed, therefore the order of unit tests expectations is also changed
more unit tests are created to verify the handling of the problem cases described above

Added system tests: eclipse-ditto/ditto-testing#33

thjaeckle · 2026-04-29T13:44:55Z

@abalarev will you also provide a system-test which tests this fix - seems important enough to me to cover it with an integration-test

abalarev · 2026-04-29T14:06:21Z

@thjaeckle Sounds reasonable, I'll add some system test(s)

thjaeckle · 2026-05-04T08:36:17Z

@abalarev thanks for the added system tests 👍
I just ran them and seems there are some related test failures for MQTT, could you check them please?
https://github.com/eclipse-ditto/ditto/actions/runs/25307537926

abalarev · 2026-05-04T12:59:41Z

@thjaeckle Please run the system tests again, I've pushed a fix.

Signed-off-by: Andrey Balarev <andrey.balarev@bosch.com>

thjaeckle

Thanks for tackling this — the refactor direction looks right and the headline scenarios are well-covered. A few correctness concerns and polish items inline.

One overarching note on test coverage: the four correctness concerns flagged below (shared-root trimming, cross-streaming-type early return, non-deterministic findFirst over a Set, and catch-all / extras precedence) are not yet exercised by the new tests. Once the intended behavior for each is settled, please add regression tests that lock it in.

thjaeckle · 2026-05-06T12:27:34Z

+                    final var builder = extra.toBuilder();
+                    allExtraFields.getPointers().stream()
+                            .filter(pointer -> !neededFields.getPointers().contains(pointer))
+                            .forEach(pointer -> pointer.getRoot().ifPresent(builder::remove));


The trim removes by pointer.getRoot(), which deletes the whole top-level object. With allExtraFields = ["/features/f1", "/features/f2", "/attributes"] and neededFields = ["/features/f1", "/attributes"], the loop hits /features/f2, removes the features root, and also wipes /features/f1 that the matched topic actually requested.

The trimming should operate at full-pointer depth (remove the value at the exact pointer, or rebuild the JSON containing only neededFields) rather than at the root segment.

thjaeckle · 2026-05-06T12:27:34Z

+                .map(FilteredTopic::getExtraFields)
+                .flatMap(Optional::stream)
+                .toList();
+        boolean topicWithNoFilterNoExtraFieldsExists = topics.stream().anyMatch(topic -> topic.getFilter().isEmpty() && topic.getExtraFields().isEmpty());


topics is target.getTopics() unfiltered by streaming type — pairTargetsWithTopics now passes all of a target's topics whenever any one of them has extra fields. So a target with e.g. a TWIN_EVENTS extras-topic plus a LIVE_MESSAGES topic with no filter and no extras would short-circuit enrichment for a TWIN_EVENTS signal, even though the LIVE_MESSAGES topic is irrelevant to that signal.

This anyMatch should apply the same streamingType == StreamingType.fromTopic(topic.getTopic().getPubSubTopic()) guard used in pairTargetsWithTopics.

thjaeckle · 2026-05-06T12:27:34Z

+                .flatMap(Optional::stream)
+                .toList();
+        boolean topicWithNoFilterNoExtraFieldsExists = topics.stream().anyMatch(topic -> topic.getFilter().isEmpty() && topic.getExtraFields().isEmpty());
+        if (allExtraFields.isEmpty() || topicWithNoFilterNoExtraFieldsExists) {


Behavior change worth pinning down: a target that mixes { TWIN_EVENTS, no filter, no extras } with { TWIN_EVENTS, filter=X, extras=definition } will now be delivered without extras (the catch-all wins via this early return). Previously the same signal also produced an enriched copy.

Deduplication is good, but is it intentional that the catch-all topic wins over the topic that explicitly requested enrichment? Either way, please document the precedence rule in the javadoc of enrichAndFilterSignal so the semantics are unambiguous.

thjaeckle · 2026-05-06T12:27:34Z

+                                            .map(signal -> setTrimmedExtra(signal, topic, expressionResolver,
+                                                    extra, allExtraFieldsOptional.get()))
+                                            .stream())
+                                    .findFirst()


topics is a Set<FilteredTopic>, which has no defined iteration order. If a signal matches more than one topic on the target, findFirst() picks an arbitrary one, so the trimmed extras delivered downstream become non-deterministic — the very symptom this PR sets out to fix.

Consider iterating in a deterministic order: derive from target.getTopics() via a structure that preserves insertion order, or sort topics by a stable key, so the chosen match is reproducible.

thjaeckle · 2026-05-06T12:27:34Z


-
    // Called inside stream; must be thread-safe
    // precondition: whenever filteredTopic != null, it contains an extra fields


This precondition comment is now stale — the parameter is Set<FilteredTopic>, not a single nullable FilteredTopic. Please update or remove.

thjaeckle · 2026-05-06T12:27:34Z

+     *     As these targets have already passed pre-filtering in an early stage, no more filtering is needed.</li>
+     * <li>
+     *     For each target containing any extra field in its topics, it produces a pair of outbound signal and a set of its target topics.
+     *     As the filter could incLude extra fields, an additional filtering must be performed after extracting the extra fields.


Typo: incLude → include.

thjaeckle · 2026-05-06T12:27:34Z

-        if (extraFieldsOptional.isEmpty()) {
+        final Set<FilteredTopic> topics = outboundSignalWithExtraFields.second();
+
+        List<JsonFieldSelector> allExtraFields = topics.stream()


The rest of this file is consistently final on every local. Please add final to the new locals introduced in this method: allExtraFields (here), topicWithNoFilterNoExtraFieldsExists, topicWithNoFilterExists, allExtraFieldsOptional, and partialThingOptional.

thjaeckle · 2026-05-06T12:27:34Z

+    public void multipleTopicsWithoutExtraFieldsFastProcessed() {
+        new TestKit(actorSystemResource.getActorSystem()) {{
+            // Create a target with multiple topics all without extra fields.
+            // Must send outbound signal, skipping filtering as the pe-filtering has already done the job


Typo: pe-filtering → pre-filtering.

thjaeckle · 2026-05-06T12:27:34Z

+    public void multipleTopicsWithExtraFieldsFirstTopic() {
+        new TestKit(actorSystemResource.getActorSystem()) {{
+            // Create a target with multiple FilteredTopics for the same streaming type with different extraFields
+            List <Target> targets = List.of(createTestTargetMultiTopics(Set.of(topic4(), topic3(), topic5())));


Stray space between List and <Target> (also at lines 310, 341, 377, 402). Should be List<Target> to match the project style.

abalarev changed the title ~~Handling of subscriptions with multiple topics with some extraFields~~ [WIP] Handling of subscriptions with multiple topics with some extraFields Apr 27, 2026

abalarev force-pushed the bugfix/multple-topics-per-target branch from f26bbaf to 01fd8d8 Compare April 27, 2026 08:52

abalarev changed the title ~~[WIP] Handling of subscriptions with multiple topics with some extraFields~~ Handling of subscriptions with multiple topics with some extraFields Apr 27, 2026

abalarev changed the title ~~Handling of subscriptions with multiple topics with some extraFields~~ Handling of subscriptions with multiple topics with extraFields Apr 27, 2026

abalarev changed the title ~~Handling of subscriptions with multiple topics with extraFields~~ Handling of subscriptions with multiple topics with extra fields Apr 27, 2026

abalarev changed the title ~~Handling of subscriptions with multiple topics with extra fields~~ Fix the handling of subscriptions with multiple topics with extra fields Apr 27, 2026

alstanchev assigned abalarev Apr 28, 2026

thjaeckle added this to Ditto Planning Apr 29, 2026

thjaeckle added this to the 3.9.0 milestone Apr 29, 2026

thjaeckle added the bug label Apr 29, 2026

abalarev changed the title ~~Fix the handling of subscriptions with multiple topics with extra fields~~ Fix the handling of subscriptions with multiple topics and extra fields Apr 29, 2026

abalarev mentioned this pull request Apr 30, 2026

New tests for multiple topics with extra fields subscribers eclipse-ditto/ditto-testing#33

Open

Multiple topics with extraFields handling fix

340ea56

Signed-off-by: Andrey Balarev <andrey.balarev@bosch.com>

abalarev force-pushed the bugfix/multple-topics-per-target branch from 01fd8d8 to 340ea56 Compare May 5, 2026 08:55

thjaeckle reviewed May 6, 2026

View reviewed changes



		// Called inside stream; must be thread-safe
		// precondition: whenever filteredTopic != null, it contains an extra fields

Conversation

abalarev commented Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thjaeckle commented Apr 29, 2026

Uh oh!

abalarev commented Apr 29, 2026

Uh oh!

thjaeckle commented May 4, 2026

Uh oh!

abalarev commented May 4, 2026

Uh oh!

thjaeckle left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

abalarev commented Apr 24, 2026 •

edited

Loading