feat(transport): unified attribute request — fetch-all per scope + scope-separated gateway response#15865
Open
ShvaykaD wants to merge 22 commits into
Open
Conversation
GetAttributeRequestMsg: allClientAttributes, allSharedAttributes, separateScopesResponse. GetAttributeResponseMsg: separateScopesResponse; deprecate isMultipleAttributesRequest. GatewayAttributesRequestMsg: clientKeys, sharedKeys, allClientKeys, allSharedKeys. onlyShared left untouched (CoAP attribute-observe initial shared state).
Actor getAttributesKvEntries: explicit per-scope all/specific/none with all-wins;
both-absent keeps the 'fetch everything' backward-compat. Echo separateScopesResponse
request->response. JsonConverter.getJsonObjectForGateway emits {client,shared} when
separateScopesResponse, else legacy value/values. Unit test for the converter.
clientKeys/sharedKeys: absent => exclude scope; present+empty => all in scope; present+list => those keys. Fixes the previous empty-string dead-end.
New clientKeys/sharedKeys format (empty=all) sets separateScopesResponse -> new
{client,shared} response; legacy client+key/keys keeps value/values. Neither marker
=> fetch all with new response (also fixes the omit-keys session crash).
Additive boolean params for per-scope fetch-all; existing clientKeys/sharedKeys unchanged; all-wins over specific.
Additive per-scope fetch-all; existing clientKeys/sharedKeys and onlyShared unchanged.
Device: empty-value all-shared/all-client, empty-object all-both.
Gateway: new clientKeys/sharedKeys format -> scope-separated {client,shared} response
(all-both and all-shared). Existing legacy tests untouched.
Gateway: sharedKeys empty -> scope-separated {shared} response.
Device: sharedKeys empty -> all shared, no client. Legacy value/values test kept.
…quest - transport.proto: deprecate legacy GatewayAttributesRequestMsg client/keys - scope @SuppressWarnings("deprecation") to legacy-only helpers instead of the dual-mode methods (onDeviceAttributesRequestProto, handleGetAttributesRequest, getJsonObjectForGateway) - dedupe per-scope JSON parsing into JsonMqttAdaptor.parseAttributeScope, reused by the gateway handler; add JsonMqttAdaptorTest covering all branches - CoapAdaptorUtils: route toKeys through getQueryValue - JsonConverter: separated gateway response delegates to toJson() - DeviceActorMessageProcessor: extract resolveScopeFuture helper - tests: proto gateway all-client/all-shared separated coverage via one parameterized helper, hoist shared key-list constants + buildClientAndSharedEntityKeys helper, replace fixed sleep with Awaitility polling derived from the seeded attributes, fix JUnit5 import in JsonConverterGatewayResponseTest
Saving the shared attribute also triggers an attribute-update push on v1/gateway/attributes (gateways auto-receive updates for connected devices). That push can reach the listener queue before the request response, so the blind poll occasionally parsed the update message (no 'shared' field) instead of the response, failing on CI. Poll until an event arrives on v1/gateway/attributes/response.
Spend the 10*timeoutMultiplier budget across messages instead of resetting it per message, so a stream of non-matching events can't extend the wait indefinitely.
…git commit -am Restores getProjectVersion() to read build properties; the hardcoded "4.2.2.3" was a local-only change unrelated to this PR.
… to CSV The unified request mirrors the device MQTT API's comma-separated clientKeys/sharedKeys format (with empty value = all in scope). Drop the extra JSON-array input branch from parseAttributeScope so the accepted format stays exactly the long-standing device contract; remove the two array-specific unit tests accordingly.
… docs, expand tests - move parseAttributeScope to JsonConverter; add applyClientScope/applySharedScope shared by HTTP/CoAP - CoAP: skip key parsing when allClientKeys/allSharedKeys set (precedence structural) - HTTP: correct @parameter required flag on clientKeys/sharedKeys - relocate parseAttributeScope unit tests to common/proto; add apply*Scope coverage - gateway JSON tests: all-client-only + specific shared keys; unique device names + subscription await - migrate attribute-key literals to shared constants (MQTT + CoAP test bases) - HTTP: add allSharedKeys integration test
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
ShvaykaD
commented
Jun 30, 2026
ShvaykaD
left a comment
ShvaykaD
left a comment
Contributor
Author
There was a problem hiding this comment.
Review summary
Reviewed 18 changed files in feat(transport): unified attribute request — fetch-all per scope + scope-separated gateway response. Left 8 comments inline. The change is additive and backward-compatible; the correctness/security pass found no blocking issues (the scope-separated gateway response reuses toJson, which emits only client/shared, and the actor's resolveScopeFuture applies all-wins precedence correctly). The inline notes are quality/design nits plus one latent NPE on the legacy gateway path.
Additional findings
- application/src/test/java/org/thingsboard/server/transport/coap/attributes/request/CoapAttributesRequestProtoIntegrationTest.java — The CoAP proto variant has no tests for the new
allClientKeys/allSharedKeysparams. The CoAP proto adaptor delegates to the sameCoapAdaptorUtils.toGetAttributeRequestMsg()as JSON, so the behavior holds, but it's untested under proto config — a test mirroring the JSON variant'stestRequestAllClientAttributeswould close the gap.
This review was auto-generated. Findings may contain errors — please verify before applying changes.
- collapse applyClientScope/applySharedScope into a shared applyScope helper - hoist CLIENT_ATTRIBUTE_KEYS/SHARED_ATTRIBUTE_KEYS to AbstractTransportIntegrationTest - extract shared CoAP attribute-request setup into postAttributesAndAwaitWsUpdate
ShvaykaD
commented
Jun 30, 2026
Contributor
Author
There was a problem hiding this comment.
Review summary
Reviewed 19 changed files in feat(transport): unified attribute request — fetch-all per scope + scope-separated gateway response.
The change is additive and backward-compatible, and all findings from the previous review round are resolved. The one actionable item from this pass — the stale Swagger @Schema(defaultValue="state"/"configuration") on the now-optional clientKeys/sharedKeys HTTP params — has been addressed in 852026ff3f (defaults dropped, descriptions point to allClientKeys/allSharedKeys for fetching a whole scope). No other issues.
This review was auto-generated. Findings may contain errors — please verify before applying changes.
…edKeys The @Schema(defaultValue="state"/"configuration") advertised default keys that are no longer applied (params now default to empty). Replace with a note pointing to allClientKeys/allSharedKeys for fetching a whole scope.
…tributeScope - Drop the always-false isMultipleAttributesRequest flag from the onlyShared path - Split the GetAttribute response into explicit legacy vs. separate-scopes branches - Return a typed ClientSharedAttributes record from getAttributesKvEntries instead of List<List<>>, collapsing the empty-request backward-compat branch into a single fetchAll flag and removing the get(0)/get(1) NPE surface - Rename/reorder parseAttributeScope params (selectAllKeys before selectKeys) to match processing order and the sibling applyScope; clarify its Javadoc
ShvaykaD
commented
Jul 1, 2026
ShvaykaD
left a comment
ShvaykaD
left a comment
Contributor
Author
There was a problem hiding this comment.
Review summary
Reviewed 5 changed files in commit 14f9230e58 (refactor: tidy GetAttribute response handling and parseAttributeScope). No issues found — the refactor is behavior-preserving across every consumer of the affected response fields, and the two quality nits from the prior pass (redundant legacyRequest local; duplicated scope-precedence rule) have been addressed. Everything is clear.
This review was auto-generated. Findings may contain errors — please verify before applying changes.
17 tasks
…t-api/get-attributes-request
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request description
Unifies the device & gateway attribute request API across all three transports (MQTT, HTTP, CoAP) so a client can request all keys in a scope (client and/or shared), not just an explicit key list. Adds a scope-separated gateway response and fixes several backward-compat dead-ends in the existing API.
Motivation
The previous attribute-request API only supported explicit key lists. An empty value was a dead-end (empty string → no result), and there was no way to say "give me everything in this scope". The gateway response also flattened client and shared attributes into a single object, so callers couldn't tell which scope a value came from.
This delivers the capability requested in #5906 (request all shared attributes from the MQTT Gateway API).
Scope of changes
Proto (
transport.proto,queue.proto)GetAttributeRequestMsg: addallClientAttributes,allSharedAttributes,separateScopesResponse.GetAttributeResponseMsg: addseparateScopesResponse; deprecateisMultipleAttributesRequest.GatewayAttributesRequestMsg: addclientKeys,sharedKeys,allClientKeys,allSharedKeys; deprecate legacyclient/keys.onlyShared(CoAP attribute-observe initial shared state) left untouched.Per-scope semantics (client / shared)
Transports
clientKeys/sharedKeysformat (empty = all) returns a scope-separated{client, shared}response; legacyclient+key/keysstill returnsvalue/values. Also fixes an omit-keys session crash.allClientKeys/allSharedKeysboolean params; existingclientKeys/sharedKeysunchanged.allClientKeys/allSharedKeysquery params; existing params andonlySharedunchanged.Server
DeviceActorMessageProcessor: explicit per-scope all/specific/none resolution with all-wins; echoesseparateScopesResponsefrom request to response.JsonConverter.getJsonObjectForGateway: emits{client, shared}whenseparateScopesResponse, else legacyvalue/values.Documentation notes
The transport API docs (MQTT/HTTP/CoAP attribute request sections) should be updated to document the new
allClientKeys/allSharedKeysparams, the empty-value = "all in scope" behavior, and the scope-separated gateway response shape.Documentation PR: thingsboard/thingsboard.io#529
General checklist
Front-End feature checklist
Not applicable — no front-end changes.
Back-End feature checklist
Downstream consumers
This change is additive and backward-compatible — no consumer breaks. Follow-ups (full analysis tracked internally):
thingsboard-python-rest-client): regenerate the device API client so the newallClientKeys/allSharedKeysparams are exposed (CE + PE).tb-mqtt-client/thingsboard-gateway): optionally migrate gateway attribute requests from the now-deprecatedclient/key/keysto the newclientKeys/sharedKeys(they remain supported).allClientKeys/allSharedKeys.