NMS-19877: Topology (Preview) graph-service provider for the path outage hierarchy#8565
Open
marshallmassengill wants to merge 4 commits into
Open
NMS-19877: Topology (Preview) graph-service provider for the path outage hierarchy#8565marshallmassengill wants to merge 4 commits into
marshallmassengill wants to merge 4 commits into
Conversation
The node parent / critical-path hierarchy (OnmsNode.getParent(), the nodeParentID column) was only reachable through the legacy Vaadin topology's PathOutageProvider; the new graph service had no equivalent, so non-Vaadin clients couldn't read it. Add features/graph/provider/pathoutage with a native GraphProvider that builds the parent->child forest from NodeDao inside a read-only transaction (getParent() is a lazy many-to-one). Vertices carry the label and nodeID properties, with node and alarm-status enrichment enabled, matching the other node-backed providers. Nodes that take part in no parent-child relationship are omitted -- isolated dots convey nothing in this graph -- so installs without node parents serve an empty graph. Registered as a bare GraphProvider, the service wraps it in a single-graph container served at /api/v2/graphs/pathoutage/pathoutage, with cacheInvalidateInterval set so the graph follows provisioning changes instead of caching the first load forever.
The mockito unit tests can't cover the one risky part of the provider: OnmsNode.getParent() is a lazy many-to-one, so building the graph has to happen inside the provider's own read-only transaction. Exercise loadGraph() against the real Hibernate NodeDao on a temporary database, deliberately without a test-managed transaction -- the way the Graph REST service calls it -- so a missing transaction wrapper would fail with a LazyInitializationException.
…outage-graph-r36 Adapts the pathoutage provider module's parent reference to the 36.0.2-SNAPSHOT version bump that came with the 36.0.1 release.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new Graph Service provider that exposes the legacy “Path Outage” node-parent hierarchy as a native graph (served under /api/v2/graphs/pathoutage/pathoutage) so the Vue map and other Graph Service consumers can render it without relying on the old Vaadin-era topology provider.
Changes:
- Introduces
PathOutageGraphProvider(OSGi bundle + blueprint) that builds a parent→child forest fromOnmsNode.getParent()inside a read-only transaction. - Adds unit + integration tests to validate graph construction and to exercise the lazy parent association under real Hibernate without a caller transaction.
- Registers and wires the new provider via Maven module inclusion and Karaf feature definitions.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| features/graph/provider/pom.xml | Adds the new pathoutage provider module to the build. |
| features/graph/provider/pathoutage/pom.xml | New OSGi bundle module for the Path Outage graph provider (+ test dependencies for IT). |
| features/graph/provider/pathoutage/src/main/java/org/opennms/netmgt/graph/provider/pathoutage/PathOutageGraphProvider.java | Implements graph generation from node parent relationships within a read-only transaction. |
| features/graph/provider/pathoutage/src/main/resources/OSGI-INF/blueprint/blueprint.xml | Registers the provider as an OSGi GraphProvider with cache invalidation. |
| features/graph/provider/pathoutage/src/test/java/org/opennms/netmgt/graph/provider/pathoutage/PathOutageGraphProviderTest.java | Unit tests for graph namespace, vertex/edge construction, and isolated-node omission. |
| features/graph/provider/pathoutage/src/test/java/org/opennms/netmgt/graph/provider/pathoutage/PathOutageGraphProviderIT.java | Integration test validating lazy parent traversal without a caller transaction. |
| container/features/src/main/resources/features.xml | Adds a new Karaf feature for the provider and includes it in the graph providers set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Collect parent->child pairs in a child->parent map instead of a Set<int[]>: arrays compare by identity, so the set never actually de-duplicated; a map is value-based, and a child having exactly one parent makes duplicates structurally impossible. - Drop the manual incrementing edge id and let GenericEdge derive its deterministic source->target id, stable regardless of NodeDao.findAll() ordering.
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.
Exposes the node-parent / critical-path hierarchy (nodeParentID) as a native graph-service provider — GET /api/v2/graphs/pathoutage/pathoutage — making the legacy Path Outage topology available to the new Vue map and any other Graph Service consumer, without bridging the Vaadin-era provider.
The provider (new features/graph/provider/pathoutage module) builds the parent→child forest from NodeDao inside a read-only transaction, since the parent association is lazy and the REST call path supplies no session.
Vertices carry label and nodeID so standard node-info/status enrichment applies; nodes with neither parent nor children are omitted to keep the graph focused on the configured hierarchy. Registers with cacheInvalidateInterval=300 so parent changes appear within five minutes (without it, the graph service caches the first load indefinitely). Covered by unit tests and an integration test exercising the lazy parent under real Hibernate with no caller transaction — the exact REST-path condition.
Assisted-by: Claude Code:Opus 4.8/Fable 5
Jira: https://opennms.atlassian.net/browse/NMS-19877