rfellows commented on code in PR #11263:
URL: https://github.com/apache/nifi/pull/11263#discussion_r3284016409
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java:
##########
@@ -3950,6 +3950,26 @@ public Set<ControllerServiceEntity>
getConnectorControllerServices(final String
.collect(Collectors.toSet());
}
+ @Override
+ public ParameterContextEntity getConnectorParameterContext(final String
connectorId, final String processGroupId) {
+ final ConnectorNode connectorNode =
connectorDAO.getConnector(connectorId, ConnectorSyncMode.LOCAL_ONLY);
+ final ProcessGroup managedProcessGroup =
connectorNode.getActiveFlowContext().getManagedProcessGroup();
+ final ProcessGroup targetProcessGroup =
managedProcessGroup.findProcessGroup(processGroupId);
+ if (targetProcessGroup == null) {
+ throw new ResourceNotFoundException("Process Group with ID " +
processGroupId + " was not found within Connector " + connectorId);
+ }
+
+ final ParameterContext parameterContext =
targetProcessGroup.getParameterContext();
+ if (parameterContext == null) {
+ return null;
+ }
+
+ // Connector-managed parameter contexts (and any contexts they inherit
from) are not registered with the
+ // global flow's ParameterContextManager. The DTO factory now resolves
a parameter's source context by
+ // walking the in-memory inheritance graph on the supplied context, so
an empty lookup is sufficient here.
Review Comment:
Nit: "now resolves" is patch-relative — it describes the code relative to
what it used to do rather than its permanent contract. Comments should explain
the invariant, not the change (those belong in the commit message).
Suggested rewrite:
```java
// Connector-managed parameter contexts are not registered with the global
ParameterContextManager,
// so the DAO-backed lookup would fail for any inherited parameter. The
in-memory inheritance graph
// reachable from the supplied context is sufficient to resolve parameter
source contexts for
// connector-managed flows, making an empty lookup safe here.
```
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java:
##########
@@ -1631,6 +1630,51 @@ public ParameterDTO createParameterDto(final
ParameterContext parameterContext,
return dto;
}
+ /**
+ * Resolves the {@link ParameterContext} where the given parameter was
originally defined.
+ *
+ * <p>For parameters declared directly on the current context (or whose
source id matches the current
+ * context's identifier), the current context is returned without
consulting any external lookup. For
+ * inherited parameters, the source context is found by walking the
in-memory inheritance graph reachable
+ * from the current context via {@link
ParameterContext#getInheritedParameterContexts()}. Only if the
+ * source context is not reachable on that graph (which is expected only
for legacy callers that pass a
+ * registry-backed lookup) does the provided {@link
ParameterContextLookup} get consulted as a fallback.</p>
+ */
+ private ParameterContext resolveContainingParameterContext(final
ParameterContext parameterContext, final Parameter parameter,
+ final
ParameterContextLookup parameterContextLookup) {
+ final String sourceId = parameter.getParameterContextId();
+ if (sourceId == null ||
sourceId.equals(parameterContext.getIdentifier())) {
+ return parameterContext;
+ }
+
+ final ParameterContext fromGraph =
findInheritedParameterContext(parameterContext, sourceId, new HashSet<>());
+ if (fromGraph != null) {
+ return fromGraph;
+ }
+
+ return parameterContextLookup.getParameterContext(sourceId);
Review Comment:
`resolveContainingParameterContext` can return `null` here when both the
in-memory graph walk and the lookup come up empty. The connector path in
`StandardNiFiServiceFacade` passes `ParameterContextLookup.EMPTY`, whose
`getParameterContext` unconditionally returns `null`. If a parameter carries a
`parameterContextId` that is not reachable in the in-memory graph (e.g. data
inconsistency after a reconciliation), this method returns `null`, and the
caller on line 1625 immediately dereferences it with `.getIdentifier()`,
producing an NPE.
Suggested fix — guard the return value:
```java
final ParameterContext result =
parameterContextLookup.getParameterContext(sourceId);
return result != null ? result : parameterContext;
```
Falling back to `parameterContext` (treating the parameter as locally
defined) is conservative and safe. Alternatively, throw an
`IllegalStateException` with a descriptive message so the inconsistency
surfaces clearly rather than crashing silently.
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/api/dto/DtoFactoryTest.java:
##########
@@ -752,4 +761,199 @@ void testCreateAssetReferenceDtoWhenContentFileMissing() {
assertNotNull(dto.getMissingContent());
assertTrue(dto.getMissingContent());
}
+
+ @Test
+ void
testCreateParameterDtoResolvesSourceContextWhenParameterContextIdIsNull() {
+ final String contextId = "context-1";
+ final ParameterContext parameterContext =
createMockParameterContext(contextId, "context-1-name",
Collections.emptyList());
+
+ final Parameter parameter = new Parameter.Builder()
+ .name("param-name")
+ .value("param-value")
+ .build();
+
+ final ParameterContextLookup lookup =
mock(ParameterContextLookup.class);
+
+ final DtoFactory dtoFactory = newDtoFactoryForParameters();
+ final ParameterDTO dto =
dtoFactory.createParameterDto(parameterContext, parameter,
mock(RevisionManager.class), lookup);
+
+ assertEquals("param-name", dto.getName());
+ assertEquals("param-value", dto.getValue());
+ assertFalse(dto.getInherited());
+
+ final ParameterContextReferenceEntity reference =
dto.getParameterContext();
+ assertNotNull(reference);
+ assertEquals(contextId, reference.getId());
+
+ verify(lookup, never()).getParameterContext(anyString());
+ }
+
+ @Test
+ void
testCreateParameterDtoResolvesSourceContextWhenParameterContextIdMatchesCurrent()
{
+ final String contextId = "context-1";
+ final ParameterContext parameterContext =
createMockParameterContext(contextId, "context-1-name",
Collections.emptyList());
+
+ final Parameter parameter = new Parameter.Builder()
+ .name("param-name")
+ .value("param-value")
+ .parameterContextId(contextId)
+ .build();
+
+ final ParameterContextLookup lookup =
mock(ParameterContextLookup.class);
+
+ final DtoFactory dtoFactory = newDtoFactoryForParameters();
+ final ParameterDTO dto =
dtoFactory.createParameterDto(parameterContext, parameter,
mock(RevisionManager.class), lookup);
+
+ assertFalse(dto.getInherited());
+ assertEquals(contextId, dto.getParameterContext().getId());
+
+ verify(lookup, never()).getParameterContext(anyString());
+ }
+
+ @Test
+ void testCreateParameterDtoResolvesSourceContextFromInheritedGraph() {
+ final String childId = "context-child";
+ final String parentId = "context-parent";
+
+ final ParameterContext parentContext =
createMockParameterContext(parentId, "parent", Collections.emptyList());
+ final ParameterContext childContext =
createMockParameterContext(childId, "child", List.of(parentContext));
+
+ final Parameter parameter = new Parameter.Builder()
+ .name("param-name")
+ .value("param-value")
+ .parameterContextId(parentId)
+ .build();
+
+ final ParameterContextLookup lookup =
mock(ParameterContextLookup.class);
+
+ final DtoFactory dtoFactory = newDtoFactoryForParameters();
+ final ParameterDTO dto = dtoFactory.createParameterDto(childContext,
parameter, mock(RevisionManager.class), lookup);
+
+ assertTrue(dto.getInherited());
+ assertEquals(parentId, dto.getParameterContext().getId());
+
+ verify(lookup, never()).getParameterContext(anyString());
+ }
+
+ @Test
+ void
testCreateParameterDtoResolvesSourceContextFromTransitiveInheritedGraph() {
+ final String childId = "context-child";
+ final String parentId = "context-parent";
+ final String grandparentId = "context-grandparent";
+
+ final ParameterContext grandparentContext =
createMockParameterContext(grandparentId, "grandparent",
Collections.emptyList());
+ final ParameterContext parentContext =
createMockParameterContext(parentId, "parent", List.of(grandparentContext));
+ final ParameterContext childContext =
createMockParameterContext(childId, "child", List.of(parentContext));
+
+ final Parameter parameter = new Parameter.Builder()
+ .name("param-name")
+ .value("param-value")
+ .parameterContextId(grandparentId)
+ .build();
+
+ final ParameterContextLookup lookup =
mock(ParameterContextLookup.class);
+
+ final DtoFactory dtoFactory = newDtoFactoryForParameters();
+ final ParameterDTO dto = dtoFactory.createParameterDto(childContext,
parameter, mock(RevisionManager.class), lookup);
+
+ assertTrue(dto.getInherited());
+ assertEquals(grandparentId, dto.getParameterContext().getId());
+
+ verify(lookup, never()).getParameterContext(anyString());
+ }
+
+ @Test
+ void
testCreateParameterDtoFallsBackToLookupWhenSourceNotReachableInGraph() {
+ final String contextId = "context-1";
+ final String externalId = "context-external";
+
+ final ParameterContext parameterContext =
createMockParameterContext(contextId, "context-1-name",
Collections.emptyList());
+ final ParameterContext externalContext =
createMockParameterContext(externalId, "context-external-name",
Collections.emptyList());
+
+ final Parameter parameter = new Parameter.Builder()
+ .name("param-name")
+ .value("param-value")
+ .parameterContextId(externalId)
+ .build();
+
+ final ParameterContextLookup lookup =
mock(ParameterContextLookup.class);
+
when(lookup.getParameterContext(externalId)).thenReturn(externalContext);
+
+ final DtoFactory dtoFactory = newDtoFactoryForParameters();
+ final ParameterDTO dto =
dtoFactory.createParameterDto(parameterContext, parameter,
mock(RevisionManager.class), lookup);
+
+ assertTrue(dto.getInherited());
+ assertEquals(externalId, dto.getParameterContext().getId());
+
+ verify(lookup).getParameterContext(externalId);
+ }
Review Comment:
This test covers the fallback path when the lookup returns a valid context,
but there is no companion test for the case where both the graph walk **and**
the lookup return `null` — i.e. `ParameterContextLookup.EMPTY`, which is
exactly what the connector path passes. Without it, the behavior of
`resolveContainingParameterContext` when both paths come up empty is
unspecified by the test suite, and the NPE at the call site (line 1625) would
go undetected.
Suggested addition alongside the null-guard fix:
```java
@Test
void
testCreateParameterDtoFallsBackToCurrentContextWhenSourceNotReachableInGraphAndLookupIsEmpty()
{
final String contextId = "context-1";
final String externalId = "context-external";
final ParameterContext parameterContext =
createMockParameterContext(contextId, "context-1-name",
Collections.emptyList());
final Parameter parameter = new Parameter.Builder()
.name("param-name")
.value("param-value")
.parameterContextId(externalId)
.build();
final DtoFactory dtoFactory = newDtoFactoryForParameters();
final ParameterDTO dto = dtoFactory.createParameterDto(
parameterContext, parameter, mock(RevisionManager.class),
ParameterContextLookup.EMPTY);
// Source context not found in graph or lookup; falls back to treating
as local
assertFalse(dto.getInherited());
assertEquals(contextId, dto.getParameterContext().getId());
}
```
(Exact assertion depends on the chosen fallback behavior, but the test is
needed either way to pin the contract.)
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/state/connector-canvas/connector-canvas.effects.spec.ts:
##########
@@ -1080,4 +1159,244 @@ describe('ConnectorCanvasEffects', () => {
expect(emitted).toEqual([reloadConnectorFlow()]);
});
});
+
+ describe('loadConnectorParameterContextOnLoadSuccess$', () => {
+ function flowSuccessAction(connectorId: string, processGroupId: string
| null) {
+ return loadConnectorFlowSuccess({
+ connectorId,
+ processGroupId,
+ parentProcessGroupId: null,
+ breadcrumb: null,
+ labels: [],
+ funnels: [],
+ inputPorts: [],
+ outputPorts: [],
+ remoteProcessGroups: [],
+ processGroups: [],
+ processors: [],
+ connections: []
+ });
+ }
+
+ function controllerServicesSuccessAction(connectorId: string,
processGroupId: string) {
+ return loadConnectorControllerServicesSuccess({
+ response: {
+ connectorId,
+ processGroupId,
+ controllerServices: [],
+ breadcrumb: null,
+ loadedTimestamp: '2024-01-01T00:00:00.000Z'
+ }
+ });
+ }
+
+ it('should dispatch loadConnectorParameterContext scoped to
CONNECTOR_CANVAS after flow load success', async () => {
+ const { effects, actions$ } = await setup();
+ actions$(of(flowSuccessAction('connector-123', 'pg-abc')));
+
+ const result = await
firstValueFrom(effects.loadConnectorParameterContextOnLoadSuccess$);
+ expect(result).toEqual(
+ loadConnectorParameterContext({
+ connectorId: 'connector-123',
+ processGroupId: 'pg-abc',
+ errorContext: ErrorContextKey.CONNECTOR_CANVAS
+ })
+ );
+ });
+
+ it('should dispatch loadConnectorParameterContext scoped to
CONTROLLER_SERVICES after controller services load success (deep link)', async
() => {
+ const { effects, actions$ } = await setup();
+ actions$(of(controllerServicesSuccessAction('connector-123',
'pg-abc')));
+
+ const result = await
firstValueFrom(effects.loadConnectorParameterContextOnLoadSuccess$);
+ expect(result).toEqual(
+ loadConnectorParameterContext({
+ connectorId: 'connector-123',
+ processGroupId: 'pg-abc',
+ errorContext: ErrorContextKey.CONTROLLER_SERVICES
+ })
+ );
+ });
+
+ it('should not dispatch when processGroupId is null', async () => {
+ const { effects, actions$ } = await setup();
+ actions$(of(flowSuccessAction('connector-123', null)));
+
+ const emitted = await
firstValueFrom(effects.loadConnectorParameterContextOnLoadSuccess$.pipe(toArray()));
+
+ expect(emitted).toEqual([]);
+ });
+
+ it('should dispatch only once when polling refires
loadConnectorFlowSuccess for the same process group', async () => {
+ const { effects, actions$ } = await setup();
+ actions$(
+ of(
+ flowSuccessAction('connector-123', 'pg-abc'),
+ flowSuccessAction('connector-123', 'pg-abc'),
+ flowSuccessAction('connector-123', 'pg-abc')
+ )
+ );
+
+ const emitted = await
firstValueFrom(effects.loadConnectorParameterContextOnLoadSuccess$.pipe(toArray()));
+
+ expect(emitted).toEqual([
+ loadConnectorParameterContext({
+ connectorId: 'connector-123',
+ processGroupId: 'pg-abc',
+ errorContext: ErrorContextKey.CONNECTOR_CANVAS
+ })
+ ]);
+ });
+
+ it('should dedupe canvas → controller-services transition within the
same process group', async () => {
+ const { effects, actions$ } = await setup();
+ actions$(
+ of(
+ flowSuccessAction('connector-123', 'pg-abc'),
+ controllerServicesSuccessAction('connector-123', 'pg-abc')
+ )
+ );
+
+ const emitted = await
firstValueFrom(effects.loadConnectorParameterContextOnLoadSuccess$.pipe(toArray()));
+
+ expect(emitted).toEqual([
+ loadConnectorParameterContext({
+ connectorId: 'connector-123',
+ processGroupId: 'pg-abc',
+ errorContext: ErrorContextKey.CONNECTOR_CANVAS
+ })
+ ]);
+ });
+
+ it('should dispatch again when the process group changes', async () =>
{
+ const { effects, actions$ } = await setup();
+ actions$(
+ of(
+ flowSuccessAction('connector-123', 'pg-abc'),
+ flowSuccessAction('connector-123', 'pg-abc'),
+ flowSuccessAction('connector-123', 'pg-xyz')
+ )
+ );
+
+ const emitted = await
firstValueFrom(effects.loadConnectorParameterContextOnLoadSuccess$.pipe(toArray()));
+
+ expect(emitted).toEqual([
+ loadConnectorParameterContext({
+ connectorId: 'connector-123',
+ processGroupId: 'pg-abc',
+ errorContext: ErrorContextKey.CONNECTOR_CANVAS
+ }),
+ loadConnectorParameterContext({
+ connectorId: 'connector-123',
+ processGroupId: 'pg-xyz',
+ errorContext: ErrorContextKey.CONNECTOR_CANVAS
+ })
+ ]);
+ });
Review Comment:
The dedup key in `loadConnectorParameterContextOnLoadSuccess$` is
`(connectorId, processGroupId)` — both fields must change (or one must change)
to pass the `distinctUntilChanged` predicate. This test covers the PG-changes
case, but the symmetric case — same PG id, different connector — is missing.
Without it, someone could simplify the predicate to check only `processGroupId`
and no test would fail, even though navigating between two connectors with the
same root PG id would silently skip refetching the parameter context.
Suggested addition:
```typescript
it('should dispatch again when the connector changes even if the process
group id is the same', async () => {
const { effects, actions$ } = await setup();
actions$(
of(
flowSuccessAction('connector-123', 'pg-abc'),
flowSuccessAction('connector-456', 'pg-abc')
)
);
const emitted = await
firstValueFrom(effects.loadConnectorParameterContextOnLoadSuccess$.pipe(toArray()));
expect(emitted).toEqual([
loadConnectorParameterContext({
connectorId: 'connector-123',
processGroupId: 'pg-abc',
errorContext: ErrorContextKey.CONNECTOR_CANVAS
}),
loadConnectorParameterContext({
connectorId: 'connector-456',
processGroupId: 'pg-abc',
errorContext: ErrorContextKey.CONNECTOR_CANVAS
})
]);
});
```
##########
nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/api/dto/DtoFactoryTest.java:
##########
@@ -752,4 +761,199 @@ void testCreateAssetReferenceDtoWhenContentFileMissing() {
assertNotNull(dto.getMissingContent());
assertTrue(dto.getMissingContent());
}
+
+ @Test
+ void
testCreateParameterDtoResolvesSourceContextWhenParameterContextIdIsNull() {
+ final String contextId = "context-1";
+ final ParameterContext parameterContext =
createMockParameterContext(contextId, "context-1-name",
Collections.emptyList());
+
+ final Parameter parameter = new Parameter.Builder()
+ .name("param-name")
+ .value("param-value")
+ .build();
+
+ final ParameterContextLookup lookup =
mock(ParameterContextLookup.class);
+
+ final DtoFactory dtoFactory = newDtoFactoryForParameters();
+ final ParameterDTO dto =
dtoFactory.createParameterDto(parameterContext, parameter,
mock(RevisionManager.class), lookup);
+
+ assertEquals("param-name", dto.getName());
+ assertEquals("param-value", dto.getValue());
+ assertFalse(dto.getInherited());
+
+ final ParameterContextReferenceEntity reference =
dto.getParameterContext();
+ assertNotNull(reference);
+ assertEquals(contextId, reference.getId());
+
+ verify(lookup, never()).getParameterContext(anyString());
+ }
+
+ @Test
+ void
testCreateParameterDtoResolvesSourceContextWhenParameterContextIdMatchesCurrent()
{
+ final String contextId = "context-1";
+ final ParameterContext parameterContext =
createMockParameterContext(contextId, "context-1-name",
Collections.emptyList());
+
+ final Parameter parameter = new Parameter.Builder()
+ .name("param-name")
+ .value("param-value")
+ .parameterContextId(contextId)
+ .build();
+
+ final ParameterContextLookup lookup =
mock(ParameterContextLookup.class);
+
+ final DtoFactory dtoFactory = newDtoFactoryForParameters();
+ final ParameterDTO dto =
dtoFactory.createParameterDto(parameterContext, parameter,
mock(RevisionManager.class), lookup);
+
+ assertFalse(dto.getInherited());
+ assertEquals(contextId, dto.getParameterContext().getId());
+
+ verify(lookup, never()).getParameterContext(anyString());
+ }
+
+ @Test
+ void testCreateParameterDtoResolvesSourceContextFromInheritedGraph() {
+ final String childId = "context-child";
+ final String parentId = "context-parent";
+
+ final ParameterContext parentContext =
createMockParameterContext(parentId, "parent", Collections.emptyList());
+ final ParameterContext childContext =
createMockParameterContext(childId, "child", List.of(parentContext));
+
+ final Parameter parameter = new Parameter.Builder()
+ .name("param-name")
+ .value("param-value")
+ .parameterContextId(parentId)
+ .build();
+
+ final ParameterContextLookup lookup =
mock(ParameterContextLookup.class);
+
+ final DtoFactory dtoFactory = newDtoFactoryForParameters();
+ final ParameterDTO dto = dtoFactory.createParameterDto(childContext,
parameter, mock(RevisionManager.class), lookup);
+
+ assertTrue(dto.getInherited());
+ assertEquals(parentId, dto.getParameterContext().getId());
+
+ verify(lookup, never()).getParameterContext(anyString());
+ }
+
+ @Test
+ void
testCreateParameterDtoResolvesSourceContextFromTransitiveInheritedGraph() {
+ final String childId = "context-child";
+ final String parentId = "context-parent";
+ final String grandparentId = "context-grandparent";
+
+ final ParameterContext grandparentContext =
createMockParameterContext(grandparentId, "grandparent",
Collections.emptyList());
+ final ParameterContext parentContext =
createMockParameterContext(parentId, "parent", List.of(grandparentContext));
+ final ParameterContext childContext =
createMockParameterContext(childId, "child", List.of(parentContext));
+
+ final Parameter parameter = new Parameter.Builder()
+ .name("param-name")
+ .value("param-value")
+ .parameterContextId(grandparentId)
+ .build();
+
+ final ParameterContextLookup lookup =
mock(ParameterContextLookup.class);
+
+ final DtoFactory dtoFactory = newDtoFactoryForParameters();
+ final ParameterDTO dto = dtoFactory.createParameterDto(childContext,
parameter, mock(RevisionManager.class), lookup);
+
+ assertTrue(dto.getInherited());
+ assertEquals(grandparentId, dto.getParameterContext().getId());
+
+ verify(lookup, never()).getParameterContext(anyString());
+ }
+
+ @Test
+ void
testCreateParameterDtoFallsBackToLookupWhenSourceNotReachableInGraph() {
+ final String contextId = "context-1";
+ final String externalId = "context-external";
+
+ final ParameterContext parameterContext =
createMockParameterContext(contextId, "context-1-name",
Collections.emptyList());
+ final ParameterContext externalContext =
createMockParameterContext(externalId, "context-external-name",
Collections.emptyList());
+
+ final Parameter parameter = new Parameter.Builder()
+ .name("param-name")
+ .value("param-value")
+ .parameterContextId(externalId)
+ .build();
+
+ final ParameterContextLookup lookup =
mock(ParameterContextLookup.class);
+
when(lookup.getParameterContext(externalId)).thenReturn(externalContext);
+
+ final DtoFactory dtoFactory = newDtoFactoryForParameters();
+ final ParameterDTO dto =
dtoFactory.createParameterDto(parameterContext, parameter,
mock(RevisionManager.class), lookup);
+
+ assertTrue(dto.getInherited());
+ assertEquals(externalId, dto.getParameterContext().getId());
+
+ verify(lookup).getParameterContext(externalId);
+ }
+
+ @Test
+ void testCreateParameterDtoInheritanceGraphHandlesCycles() {
+ final String childId = "context-child";
+ final String parentId = "context-parent";
+ final String missingId = "context-missing";
+
+ final ParameterContext parentContext = mock(ParameterContext.class);
+ final ParameterContext childContext = mock(ParameterContext.class);
+ configureBaseParameterContext(childContext, childId, "child");
+ configureBaseParameterContext(parentContext, parentId, "parent");
+
when(childContext.getInheritedParameterContexts()).thenReturn(List.of(parentContext));
+
when(parentContext.getInheritedParameterContexts()).thenReturn(List.of(childContext));
+
+ final Parameter parameter = new Parameter.Builder()
+ .name("param-name")
+ .value("param-value")
+ .parameterContextId(missingId)
+ .build();
+
+ final ParameterContext fallbackContext =
createMockParameterContext(missingId, "missing", Collections.emptyList());
+ final ParameterContextLookup lookup =
mock(ParameterContextLookup.class);
+
when(lookup.getParameterContext(missingId)).thenReturn(fallbackContext);
+
+ final DtoFactory dtoFactory = newDtoFactoryForParameters();
+ final ParameterDTO dto = dtoFactory.createParameterDto(childContext,
parameter, mock(RevisionManager.class), lookup);
+
+ assertTrue(dto.getInherited());
+ assertEquals(missingId, dto.getParameterContext().getId());
+
+ verify(lookup).getParameterContext(missingId);
+ }
Review Comment:
The cycle test covers mutual recursion (child → parent → child), but the
`visited` set in `findInheritedParameterContext` also guards against redundant
traversal in diamond inheritance (A inherits B and C, both inherit D). That
second behavior is untested. Without it, a future change that accidentally
removes or weakens the visited-set guard would not be caught.
Suggested addition:
```java
@Test
void
testCreateParameterDtoResolvesSourceContextFromDiamondInheritanceGraph() {
final String contextAId = "context-a";
final String contextBId = "context-b";
final String contextCId = "context-c";
final String contextDId = "context-d";
final ParameterContext contextD = createMockParameterContext(contextDId,
"context-d-name", Collections.emptyList());
final ParameterContext contextB = createMockParameterContext(contextBId,
"context-b-name", List.of(contextD));
final ParameterContext contextC = createMockParameterContext(contextCId,
"context-c-name", List.of(contextD));
final ParameterContext contextA = createMockParameterContext(contextAId,
"context-a-name", List.of(contextB, contextC));
final Parameter parameter = new Parameter.Builder()
.name("param-name")
.value("param-value")
.parameterContextId(contextDId)
.build();
final DtoFactory dtoFactory = newDtoFactoryForParameters();
final ParameterDTO dto = dtoFactory.createParameterDto(
contextA, parameter, mock(RevisionManager.class),
ParameterContextLookup.EMPTY);
assertTrue(dto.getInherited());
assertEquals(contextDId, dto.getParameterContext().getId());
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
