EDG-217 Houston: Bulk device tag browsing and import#1444
Open
EDG-217 Houston: Bulk device tag browsing and import#1444
Conversation
Test Results 514 files 514 suites 7m 23s ⏱️ Results for commit ca2e1a6. ♻️ This comment has been updated with latest results. |
Coverage Report
|
76a9caa to
04f7db8
Compare
codepitbull
approved these changes
Mar 18, 2026
04f7db8 to
71271e6
Compare
|
2b0a210 to
12be4a7
Compare
3 tasks
ff39191 to
2a5314a
Compare
Fixes SNYK-JAVA-COMFASTERXMLJACKSONCORE-15365924 (Allocation of Resources Without Limits or Throttling, CVSSv3 8.7) in jackson-core.
Two assertions in DeviceTagImporterTest compared ValidationError.Code enum values against String literals, causing AssertJ failures.
All assertions were comparing ValidationError.Code enum values against String literals, causing type mismatch failures in AssertJ.
- Thread safety: CopyOnWriteArrayList for concurrent variable collection - Back-pressure: Semaphore(32) gates concurrent browse requests - DataTypeTree for type resolution instead of hardcoded switch - AccessLevelType for access level decoding instead of bit masks - AttributeId enum instead of magic integer constants - Fix continuation point handling (single point, not list) - Eliminate redundant double toNodeId resolution - Batch attribute reads (DataType, AccessLevel, Description)
Switch import classification (edgeOnly/fileOnly/inBoth) from tagName to nodeId in DeviceTagImporter and DeviceTagValidator. Renames (same nodeId, different tagName) are now recognised as updates. Add tagName collision check for MERGE_SAFE/MERGE_OVERWRITE modes.
Add merge functions to Collectors.toMap() calls in OpcUaSubscriptionLifecycleHandler that previously threw IllegalStateException on duplicate node IDs. The first tag wins and a warning is logged. The import-time validation gap was already closed by the EDG-363 nodeId-based classification change.
Extend mappingsMatch() to also check includeTagNames, includeTimestamp, messageExpiryInterval, and mqttUserProperties. Previously only topic and maxQos were compared, so changes to the other fields were silently discarded even in OVERWRITE mode.
Move user-editable columns (tag_name, topics, QoS, etc.) before the protocol-specific informational columns (node_path, namespace_uri, etc.) so they appear first when opened in Excel. Import is unaffected since deserialization uses named column lookup.
Tag names are scoped to their adapter. Remove the EDGE_TAG_CONFLICT validation that rejected imported tags whose name existed on a different adapter. Replace the test with one asserting the same name is allowed.
Relax duplicate tag name and node ID checks in DeviceTagValidator to allow rows that share the same tag definition (nodeId + tagName + description) but differ in northbound mapping fields. Each row produces one northbound mapping; the tag is created once. Change DeviceTagImporter to group file rows by nodeId and build one NorthboundMappingEntity per row, one SouthboundMappingEntity per tag. Edge-side NB/SB maps are now list-valued (groupingBy). The identical check compares NB mapping sets rather than single entities.
Migrate DeviceTagBrowsingApi from Swagger v1 to OAS3 annotations (@tag, @operation, @parameter, @apiresponse) with operationIds for both browse and import endpoints. Add Redocly split YAML: path specs for browse and import, plus ImportResult and TagAction response schemas. Add annotation unit tests verifying OAS3 metadata is present.
Pre-sort BrowsedNode results in OpcUaNodeBrowser so the CSV serializer no longer re-sorts. Change streaming serialize overloads to accept Iterable<DeviceTagRow> and use JsonGenerator for element-at-a-time writing, avoiding intermediate List<RowDto>. In the REST endpoint, replace the materialized List<DeviceTagRow> with a lazy Iterable that maps BrowsedNode→DeviceTagRow on-the-fly during serialization. Peak memory at the HTTP response phase drops from ~3 full-size lists to ~1.
The method was named equals(Asset) which shadowed Object.equals and required a @SuppressWarnings. Callers and tests already expected the name matchesRemoteAsset. Also includes updated third-party licenses.
Align with SDK rename: rootNodeId → rootId in query parameter, OpenAPI spec, REST implementation, and OPC UA adapter/browser.
Replace the hand-crafted DeviceTagBrowsingApi with the interface generated by genJaxRs from the Redocly-bundled OpenAPI spec, aligning Houston's device-tag browsing with the standard API pattern used by all other REST endpoints in the codebase. - Delete hand-crafted DeviceTagBrowsingApi.java (128 lines) - Give device-tag endpoints their own "Device Tag Browsing" OpenAPI tag so genJaxRs generates a dedicated DeviceTagBrowsingApi interface instead of adding methods to ProtocolAdaptersApi - Adapt DeviceTagBrowsingResourceImpl to the generated method signatures: browse() -> browseDeviceTags(), Accept header via AbstractApi.headers importTags() -> importDeviceTags(), body as File, Content-Type via headers - Use PLACEHOLDER_HIVEMQ_VERSION in Redocly source for Gradle version stamping - Regenerate ext/hivemq-edge-openapi.yaml via Redocly bundle (71 paths)
Addresses Martin's review feedback on EDG-217: remove browse types from the adapter SDK and keep them internal until the right abstractions emerge from a second protocol implementation. - BulkTagBrowser, BrowsedNode, BrowseException moved to com.hivemq.edge.adapters.browse inside the OPC UA module (neutral package, visible to core via existing compileOnly) - BulkTagBrowser.browse() now returns Stream<BrowsedNode> - OpcUaNodeBrowser sorts DiscoveredVariable by path early, then lazily batch-reads attributes via a Spliterator — only one batch of BrowsedNode (100 nodes) is alive at any time - DeviceTagBrowsingResourceImpl consumes the stream directly, with UncheckedBrowseException handling for mid-stream failures
BulkTagBrowser, BrowsedNode, and BrowseException must live in core
because DeviceTagBrowsingResourceImpl references them at class-load
time, and modules are loaded after core initializes. Placing them
in the OPC UA module caused ClassNotFoundException at runtime.
- Move 3 files to hivemq-edge/src/main/java/.../browse/
- Remove compileOnly("com.hivemq:hivemq-edge-module-opcua") from core
- Add compileOnly("com.hivemq:hivemq-edge") to OPC UA module
- Package stays com.hivemq.edge.adapters.browse (no import changes)
Stress testing the device-tag browse/import APIs with concurrent workers uncovered several issues that are addressed here: - Browse dedup: track visited NodeIds during OPC UA address space traversal to eliminate duplicates from multi-path graph references - Import atomicity: synchronize the read-compute-write cycle in DeviceTagImporter on the ProtocolAdapterExtractor lock to prevent TOCTOU races between concurrent OVERWRITE imports - Debounced restart: coalesce rapid config notifications (500ms) so multiple sequential imports trigger a single adapter restart - Browse fallback: maintain a browseClient snapshot in OpcUaProtocol- Adapter so browse operations survive adapter restarts - Partial subscription tolerance: adapter stays CONNECTED when some monitored items fail (e.g. non-existent nodes), only goes to ERROR on total failure - DynamicEnumType converter: explicit JSON serialization for OPC UA dynamic enum types, eliminating fallback warnings - Skip empty payloads: guard against null/empty OPC UA values reaching the Jackson deserializer in NorthboundTagConsumer - Browse log level: downgrade transient browse failures during adapter restart from ERROR to WARN
The debounce on notifyConsumer() was applied to all config change paths (addAdapter, deleteAdapter, updateConfig, registerConsumer), causing adapter creation/deletion to be delayed by 500ms. Tests that create adapters and immediately list them saw 0 adapters. Split into notifyConsumer() (immediate, original behavior) and notifyConsumerDebounced() (500ms delay). Only updateAdapter() uses the debounced path — this is the import codepath where rapid sequential calls cause adapter restart storms.
The debounce on notifyConsumer/updateAdapter caused CI failures: - AdapterApiIT: adapter creation immediate but refresh delayed, listing saw 0 adapters - DeviceTagEndToEndIT: browse on adapter-b returned 409/500 because adapter wasn't restarted yet after import - AdapterApiIT test_list_adapter_update: getAdapter() reads from the running wrapper (not allConfigs), returned stale config The import atomicity (synchronized on adapterExtractor in doImport) already prevents the TOCTOU race. The restart storm from concurrent imports is an optimization to revisit separately — it requires the debounce to live in DeviceTagImporter with a quiet updateAdapter variant, plus test updates.
66ccfaa to
ca2e1a6
Compare
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
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.



Summary
Core implementation of the Houston feature (EDG-217) — bulk device tag browsing and import for OPC UA adapters. Enables customers with large OPC UA installations to browse the address space, export discovered nodes as CSV/JSON/YAML, edit in familiar tools (Excel, Python, curl), and import the result to atomically create tags and mappings.
REST endpoints
Two new endpoints under
/api/v1/management/protocol-adapters/adapters/{adapterId}/device-tags/:POST /browse— scans the OPC UA address space via a connected adapter, streams discovered variable nodes as a downloadable file (CSV, JSON, or YAML viaAcceptheader). Results pre-sorted bynodePath. Response streamed viaStreamingOutput— nobyte[]buffered in memory.POST /import— accepts an edited tag file, validates exhaustively, and atomically creates/updates/deletes tags and mappings. Five conflict-resolution modes:CREATE,DELETE,OVERWRITE,MERGE_SAFE(default),MERGE_OVERWRITE.Architecture (~5,400 lines production code)
DeviceTagBrowsingApi(OAS3 annotations),DeviceTagBrowsingResourceImplDeviceTagCsvSerializer,DeviceTagJsonSerializer,DeviceTagYamlSerializer— each with streamingserialize(Iterable, OutputStream)overloadsDeviceTagImporter,DeviceTagImporterExceptionDeviceTagValidator(15 error codes, exhaustive collection)DeviceTagRow(21-field),ImportMode,ImportResult,TagAction,FieldMappingInstruction,ValidationErrorOpcUaNodeBrowser(async recursive browse + batch attribute read, pre-sorted output)OpcUaProtocolAdapterimplementsBulkTagBrowserKey design decisions
BrowsedNode(SDK, 11 fields) →DeviceTagRow(Core, 21 fields)*→ default values) for templated importsBrowsedNode→DeviceTagRowmapping viaIterable, element-at-a-timeJsonGeneratorwriting. Peak memory at HTTP response:BrowsedNode[N]+ O(1) instead of 3 full listsOpcUaNodeBrowser— eliminates re-sort in CSV serializerPost-review fixes
mappingsMatchDependencies added
commons-csv:1.12.0,jackson-dataformat-yaml(existing Jackson version)Not included
Related PRs
Linear ticket
EDG-217 — Houston Implementation
Test plan
./gradlew :hivemq-edge:test— all unit tests pass (~3,020 lines, ~150 tests)./gradlew :hivemq-edge-module-opcua:test— OPC UA browser unit tests pass./gradlew buildfromhivemq-edge-composite