rfellows commented on code in PR #11773:
URL: https://github.com/apache/nifi/pull/11773#discussion_r4211590756
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/canvas/canvas.component.ts:
##########
@@ -566,14 +692,33 @@ export class CanvasComponent implements OnInit,
AfterViewInit, OnDestroy {
if (connectionsChanged) {
// Connections changed (added/removed) - create new array
+ const previous = this._internalConnections;
this._internalConnections = this.mapConnections(inputConnections);
+ for (const connection of this._internalConnections) {
+ const existing = previous.find((item) => item.entity.id ===
connection.entity.id);
+ if (!existing) continue;
+ connection.ui.dragStartEntity = existing.ui.dragStartEntity;
+ connection.ui.dragStartRevision =
existing.ui.dragStartRevision;
+ connection.ui.dragStartBends = existing.ui.dragStartBends;
+ connection.ui.dragging = existing.ui.dragging;
+ connection.ui.bends = existing.ui.bends;
Review Comment:
This changes the pre-PR reconstruction behavior for surviving connections.
When any connection is added or removed, the rebuilt wrapper now retains its
previous `ui.bends` even when idle. Previously, rebuilding left `ui.bends`
unset so rendering initialized it from the latest server entity.
A polling response that changes both the connection set and a surviving
connection's geometry can therefore display stale bends. Because
[NIFI-16397](https://issues.apache.org/jira/browse/NIFI-16397) requires
bend-point behavior to remain unchanged, could we preserve the previous bends
only while that connection is actively dragging or saving? Otherwise, the
rebuilt wrapper should use the latest entity geometry.
A regression test where a sibling connection is added or removed while an
idle surviving connection receives new bends would cover this path.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/canvas/canvas.component.ts:
##########
@@ -2267,6 +2479,160 @@ export class CanvasComponent implements OnInit,
AfterViewInit, OnDestroy {
this.componentDoubleClick.emit({ entity: processor.entity,
componentType: ComponentType.Processor });
}
+ onDragEnd({ delta, movingIds }: { delta: Position; movingIds: Set<string>
}): void {
+ const targetGroupId = DragUtils.readDropTarget(this.canvasGroup) ??
undefined;
+ if (delta.x === 0 && delta.y === 0 && !targetGroupId) {
+ this.clearNoOpDragState(movingIds);
+ return;
+ }
+
+ const items: DragEndBaselineItem[] = [];
+ const includedIds = new Set<string>();
+ for (const id of movingIds) {
+ const type = this.componentTypeMap().get(id);
+ if (type === undefined) {
+ continue;
+ }
+ const wrapper = this.findWrapperByType(id, type);
+ if (!wrapper) {
+ continue;
+ }
+ const source = wrapper.ui.dragStartEntity ?? wrapper.entity;
+ items.push(this.projectBaselineItem(id, type as
DragEndBaselineItem['type'], source));
+ includedIds.add(id);
+ delete wrapper.ui.dragStartEntity;
+ }
+ for (const connection of this.internalConnections()) {
+ const entity = connection.entity;
+ if (
+ entity.sourceId === entity.destinationId &&
+ movingIds.has(entity.sourceId) &&
+ !includedIds.has(entity.id)
+ ) {
+ const source = connection.ui.dragStartEntity ?? entity;
+ items.push(this.projectBaselineItem(entity.id,
ComponentType.Connection, source));
+ delete connection.ui.dragStartEntity;
+ }
+ }
+
+ if (items.length === 0) {
+ return;
+ }
+ this.markItemsSaving(items);
Review Comment:
Nonblocking planning note for the future editable workflow: every emitted
item is marked as saving, but there is no generic way for a consumer to release
items it decides not to persist. Also, after drag emission removes
`dragStartEntity`, a same-type sibling reconstruction no longer preserves
`currentPosition`, so an optimistic position can snap back before confirmation.
These are beyond NIFI-16397's typing scope, but they should be addressed
when bulk position persistence and failure recovery are wired.
[NIFI-16403](https://issues.apache.org/jira/browse/NIFI-16403) is the closest
planned owner. If that issue is intended to cover alignment only, we may need a
separate child Jira for manual drag persistence and saving-state cleanup.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/canvas/canvas.component.ts:
##########
@@ -277,6 +337,46 @@ export class CanvasComponent implements OnInit,
AfterViewInit, OnDestroy {
private savingPorts = signal<Set<string>>(new Set());
disabledPortIds = computed(() => this.savingPorts());
+ connectableBehavior = computed<ConnectableBehaviorHelper | null>(() => {
+ const policy = this.connectable();
+ if (!policy) {
+ return null;
+ }
+ return new ConnectableBehaviorHelper(
+ policy,
+ {
+ onSelectSource: (source) =>
this.selectComponents.emit([source]),
+ onConnectionRequested: (payload) =>
this.createConnectionRequested.emit(payload),
+ isDisabled: (id) =>
+ this.disabledProcessorIds().has(id) ||
+ this.disabledFunnelIds().has(id) ||
+ this.disabledPortIds().has(id) ||
+ this.disabledProcessGroupIds().has(id) ||
+ this.disabledRemoteProcessGroupIds().has(id)
+ },
+ this.canvasRootResolver
+ );
+ });
+
+ connectionReconnect = computed<ConnectionReconnectContext | null>(() => {
+ const policy = this.connectable();
+ if (!policy) {
+ return null;
+ }
+ const geometry = { ...DEFAULT_GEOMETRY, ...policy.geometry };
+ return {
+ isValidConnectionDestination: (selection) =>
policy.isValidConnectionDestination(selection),
+ getPerimeterPoint: geometry.getPerimeterPoint,
+ selfLoopXOffset: geometry.selfLoopXOffset,
+ selfLoopYOffset: geometry.selfLoopYOffset
+ };
+ });
+
+ readonly canvasRootResolver: CanvasRootResolver = () =>
+ this.canvasGroup ?? d3.select<SVGGElement, unknown>(null as unknown as
SVGGElement);
+
+ readonly isDropAllowedFn = (): boolean => true;
Review Comment:
Nonblocking planning note: once editable troubleshooting is enabled, this
unconditional result can expose the child-process-group drop affordance for
connected selections that should not be movable as a disconnected unit.
I do not think this interaction-semantic change belongs in NIFI-16397. Could
it be tracked under
[NIFI-16400](https://issues.apache.org/jira/browse/NIFI-16400), where
troubleshooting state begins enabling canvas editing, or under a dedicated
follow-up if that ticket is not intended to own drag/drop eligibility?
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/canvas/canvas.component.spec.ts:
##########
@@ -1605,4 +1690,335 @@ describe('CanvasComponent', () => {
});
});
});
+
+ describe('typed reusable-canvas contracts', () => {
+ it('keeps connectable behavior inert when no policy is supplied',
async () => {
+ const { component } = await setup();
+
+ expect(component.connectable()).toBeNull();
+ expect(component.connectableBehavior()).toBeNull();
+ expect(component.connectionReconnect()).toBeNull();
+ });
+
+ it('replaces connectable listeners and removes them when policy
changes to null', async () => {
+ const { fixture } = await setup({
+ processors: [createMockProcessor({ id: 'processor-1' })],
+ dataReady: true
+ });
+ const firstPolicy: ConnectablePolicy = {
+ isValidConnectionSource: vi.fn(() => true),
+ isValidConnectionDestination: vi.fn(() => true)
+ };
+ const secondPolicy: ConnectablePolicy = {
+ isValidConnectionSource: vi.fn(() => true),
+ isValidConnectionDestination: vi.fn(() => true)
+ };
+ fixture.componentRef.setInput('connectable', firstPolicy);
+ fixture.detectChanges();
+ const processor =
fixture.nativeElement.querySelector('g.processor') as SVGGElement;
+
+ processor.dispatchEvent(new MouseEvent('mouseenter'));
+ expect(processor.querySelector('text.add-connect')).not.toBeNull();
+
+ fixture.componentRef.setInput('connectable', secondPolicy);
+ fixture.detectChanges();
+ expect(processor.querySelector('text.add-connect')).toBeNull();
+ processor.dispatchEvent(new MouseEvent('mouseenter'));
+
expect(firstPolicy.isValidConnectionSource).toHaveBeenCalledTimes(1);
+
expect(secondPolicy.isValidConnectionSource).toHaveBeenCalledTimes(1);
+
+ fixture.componentRef.setInput('connectable', null);
+ fixture.detectChanges();
+ expect(processor.querySelector('text.add-connect')).toBeNull();
+ const sourceChecks =
vi.mocked(secondPolicy.isValidConnectionSource).mock.calls.length;
+
+ processor.dispatchEvent(new MouseEvent('mouseenter'));
+
expect(secondPolicy.isValidConnectionSource).toHaveBeenCalledTimes(sourceChecks);
+ expect(processor.querySelector('text.add-connect')).toBeNull();
+ });
+
+ it('deactivates an active create-connection gesture when the canvas is
destroyed', async () => {
+ const { component, fixture } = await setup({
+ processors: [createMockProcessor({ id: 'processor-1' })],
+ dataReady: true
+ });
+ const policy: ConnectablePolicy = {
+ isValidConnectionSource: vi.fn(() => true),
+ isValidConnectionDestination: vi.fn(() => true)
+ };
+ fixture.componentRef.setInput('connectable', policy);
+ fixture.detectChanges();
+ const helper = component.connectableBehavior()!;
+ const deactivate = vi.spyOn(helper, 'deactivate');
+ const processor =
fixture.nativeElement.querySelector('g.processor') as SVGGElement;
+ processor.dispatchEvent(new MouseEvent('mouseenter'));
+ const canvasRoot = asPrivate(component).canvasGroup;
+ const handle = processor.querySelector('text.add-connect')!;
+ d3.select(handle).classed('dragging', true);
+ canvasRoot.node()!.appendChild(handle);
+ canvasRoot.append('path').attr('class', 'connector');
+
+ fixture.destroy();
+
+ expect(deactivate).toHaveBeenCalled();
+ expect(canvasRoot.select('text.add-connect').empty()).toBe(true);
+ expect(canvasRoot.select('path.connector').empty()).toBe(true);
+ });
+
+ it('clears component and self-loop drag state without emitting for a
zero-delta drop', async () => {
+ const processor = createMockProcessor({ id: 'processor-1' });
+ processor.revision = { version: 4 };
+ const connection = {
+ id: 'connection-1',
+ uri: 'https://localhost/nifi-api/connections/connection-1',
+ revision: { version: 7 },
+ permissions: { canRead: true, canWrite: true },
+ sourceId: 'processor-1',
+ destinationId: 'processor-1',
+ bends: [{ x: 25, y: 30 }],
+ component: { id: 'connection-1', source: {}, destination: {} }
+ } as unknown as ConnectionEntity;
+ const { component } = await setup({ processors: [processor],
connections: [connection] });
+ const processorDatum = component.internalProcessors()[0];
+ const connectionDatum = component.internalConnections()[0];
+ processorDatum.ui.dragStartEntity = processor;
+ processorDatum.ui.dragStartPosition = { ...processor.position };
+ processorDatum.ui.currentPosition = { ...processor.position };
+ processorDatum.ui.dragDelta = { x: 0, y: 0 };
+ processorDatum.ui.dragMovingIds = new Set(['processor-1']);
+ connectionDatum.ui.dragStartEntity = connection;
+ connectionDatum.ui.dragStartBends = [{ x: 25, y: 30 }];
+ connectionDatum.ui.bends = [{ x: 25, y: 30 }];
+ const emitted: unknown[] = [];
+ outputToObservable(component.componentsDragEnd).subscribe((event)
=> emitted.push(event));
+
+ component.onDragEnd({ delta: { x: 0, y: 0 }, movingIds: new
Set(['processor-1']) });
+
+ expect(emitted).toEqual([]);
+ expect(processorDatum.ui.dragStartEntity).toBeUndefined();
+ expect(processorDatum.ui.dragStartPosition).toBeUndefined();
+ expect(processorDatum.ui.currentPosition).toBeUndefined();
+ expect(processorDatum.ui.dragDelta).toBeUndefined();
+ expect(processorDatum.ui.dragMovingIds).toBeUndefined();
+ expect(connectionDatum.ui.dragStartEntity).toBeUndefined();
+ expect(connectionDatum.ui.dragStartBends).toBeUndefined();
+ expect(connectionDatum.ui.bends).toEqual(connection.bends);
+ });
+
+ it('refreshes idle connection bends from a same-id entity update',
async () => {
+ const connection = {
+ id: 'connection-1',
+ uri: 'https://localhost/nifi-api/connections/connection-1',
+ revision: { version: 1 },
+ permissions: { canRead: true, canWrite: true },
+ sourceId: 'source-1',
+ destinationId: 'dest-1',
+ bends: [{ x: 10, y: 20 }],
+ component: { id: 'connection-1', source: {}, destination: {} }
+ } as unknown as ConnectionEntity;
+ const { fixture, component } = await setup({ connections:
[connection] });
+ const wrapper = component.internalConnections()[0];
+ wrapper.ui.bends = [{ x: 10, y: 20 }];
+ const updatedConnection = {
+ ...connection,
+ revision: { version: 2 },
+ bends: [{ x: 30, y: 40 }]
+ };
+
+ fixture.componentRef.setInput('connections', [updatedConnection]);
+ fixture.detectChanges();
+
+
expect(component.internalConnections()[0].entity).toBe(updatedConnection);
+ expect(component.internalConnections()[0].ui.bends).toEqual([{ x:
30, y: 40 }]);
+
expect(component.internalConnections()[0].ui.bends).not.toBe(updatedConnection.bends);
+ });
+
+ it('preserves optimistic connection bends during an active drag',
async () => {
+ const connection = {
+ id: 'connection-1',
+ uri: 'https://localhost/nifi-api/connections/connection-1',
+ revision: { version: 1 },
+ permissions: { canRead: true, canWrite: true },
+ sourceId: 'source-1',
+ destinationId: 'dest-1',
+ bends: [{ x: 10, y: 20 }],
+ component: { id: 'connection-1', source: {}, destination: {} }
+ } as unknown as ConnectionEntity;
+ const { fixture, component } = await setup({ connections:
[connection] });
+ const wrapper = component.internalConnections()[0];
+ wrapper.ui.dragging = true;
+ wrapper.ui.bends = [{ x: 50, y: 60 }];
+ const updatedConnection = {
+ ...connection,
+ revision: { version: 2 },
+ bends: [{ x: 30, y: 40 }]
+ };
+
+ fixture.componentRef.setInput('connections', [updatedConnection]);
+ fixture.detectChanges();
+
+
expect(component.internalConnections()[0].entity).toBe(updatedConnection);
+ expect(component.internalConnections()[0].ui.bends).toEqual([{ x:
50, y: 60 }]);
+ });
+
+ it('preserves optimistic connection bends while a save is pending',
async () => {
+ const connection = {
+ id: 'connection-1',
+ uri: 'https://localhost/nifi-api/connections/connection-1',
+ revision: { version: 1 },
+ permissions: { canRead: true, canWrite: true },
+ sourceId: 'source-1',
+ destinationId: 'dest-1',
+ bends: [{ x: 10, y: 20 }],
+ component: { id: 'connection-1', source: {}, destination: {} }
+ } as unknown as ConnectionEntity;
+ const { fixture, component } = await setup({ connections:
[connection] });
+ const wrapper = component.internalConnections()[0];
+ wrapper.ui.bends = [{ x: 50, y: 60 }];
+ asPrivate(component).savingConnections.set(new
Set(['connection-1']));
+ const updatedConnection = {
+ ...connection,
+ revision: { version: 2 },
+ bends: [{ x: 30, y: 40 }]
+ };
+
+ fixture.componentRef.setInput('connections', [updatedConnection]);
+ fixture.detectChanges();
+
+
expect(component.internalConnections()[0].entity).toBe(updatedConnection);
+ expect(component.internalConnections()[0].ui.bends).toEqual([{ x:
50, y: 60 }]);
+ });
+
+ it('preserves optimistic reconnect state across wrapper rebuilds and
confirms it', async () => {
+ const connection = {
+ id: 'connection-1',
+ uri: 'https://localhost/nifi-api/connections/connection-1',
+ revision: { version: 7 },
+ permissions: { canRead: true, canWrite: true },
+ sourceId: 'source-1',
+ destinationId: 'dest-1',
+ bends: [],
+ component: { id: 'connection-1', source: {}, destination: {} }
+ } as unknown as ConnectionEntity;
+ const addedConnection = {
+ ...connection,
+ id: 'connection-2',
+ uri: 'https://localhost/nifi-api/connections/connection-2',
+ component: { ...connection.component, id: 'connection-2' }
+ } as ConnectionEntity;
+ const destination = createMockProcessor({ id: 'dest-2' });
+ const { fixture, component } = await setup({ connections:
[connection] });
+ const wrapper = component.internalConnections()[0];
+ wrapper.ui.dragStartRevision = { version: 3 };
+ const emitted: Array<{ id: string; bends?: Array<{ x: number; y:
number }> }> = [];
+
outputToObservable(component.connectionDestinationChangeRequested).subscribe((event)
=>
+ emitted.push(event)
+ );
+
+ component.onConnectionEndpointReconnect({
+ connection: wrapper,
+ newDestination: {
+ id: 'dest-2',
+ componentType: ComponentType.Processor,
+ entity: destination
+ },
+ bends: [{ x: 10, y: 20 }]
+ });
+
+ expect(wrapper.ui.reconnectDestinationId).toBe('dest-2');
+ expect(wrapper.ui.bends).toEqual([{ x: 10, y: 20 }]);
+ expect(wrapper.ui.dragStartRevision).toBeUndefined();
+ expect(emitted).toEqual([
+ {
+ id: 'connection-1',
+ revision: { version: 3 },
+ newDestination: expect.any(Object),
+ bends: [{ x: 10, y: 20 }]
+ }
+ ]);
+
+ fixture.componentRef.setInput('connections', [connection,
addedConnection]);
+ fixture.detectChanges();
+ const rebuilt = component.internalConnections().find((item) =>
item.entity.id === 'connection-1')!;
+ expect(rebuilt.ui.reconnectDestinationId).toBe('dest-2');
+ expect(rebuilt.ui.bends).toEqual([{ x: 10, y: 20 }]);
+
+ component.confirmConnectionDestination('connection-1');
+
expect(component.disabledConnectionIds().has('connection-1')).toBe(false);
+ expect(rebuilt.ui.reconnectDestinationId).toBeUndefined();
+ expect(rebuilt.ui.bends).toBeUndefined();
+ });
+
+ it('reverts optimistic reconnect destination and bends', async () => {
+ const connection = {
+ id: 'connection-1',
+ uri: 'https://localhost/nifi-api/connections/connection-1',
+ revision: { version: 7 },
+ permissions: { canRead: true, canWrite: true },
+ sourceId: 'source-1',
+ destinationId: 'dest-1',
+ bends: [],
+ component: { id: 'connection-1', source: {}, destination: {} }
+ } as unknown as ConnectionEntity;
+ const destination = createMockProcessor({ id: 'dest-2' });
+ const { component } = await setup({ connections: [connection] });
+ const wrapper = component.internalConnections()[0];
+ component.onConnectionEndpointReconnect({
+ connection: wrapper,
+ newDestination: {
+ id: 'dest-2',
+ componentType: ComponentType.Processor,
+ entity: destination
+ },
+ bends: [{ x: 10, y: 20 }]
+ });
+ wrapper.ui.endPointDragging = true;
+
+ component.revertConnectionDestination('connection-1');
+
+
expect(component.disabledConnectionIds().has('connection-1')).toBe(false);
+ expect(wrapper.ui.reconnectDestinationId).toBeUndefined();
+ expect(wrapper.ui.endPointDragging).toBeUndefined();
+ expect(wrapper.ui.bends).toBeUndefined();
+ });
+
+ it('emits the label resize gesture baseline revision', async () => {
+ const { component } = await setup();
+ const emitted: Array<{ revision: { version: number } }> = [];
Review Comment:
These two new event tests require `revision.version`, but the canonical
`RevisionRequest.version` property is optional. A direct spec TypeScript check
reports both subscriptions because the test's collection type is narrower than
the output contract.
Since NIFI-16397 explicitly covers canonical gesture payloads and
compile-time validation, could these use `RevisionRequest` or infer the event
type from the output? The same adjustment is needed in the connection-label
test below.
##########
nifi-frontend/src/main/frontend/apps/nifi/src/app/ui/common/canvas/canvas.component.spec.ts:
##########
@@ -643,6 +682,52 @@ describe('CanvasComponent', () => {
const data = component.getBirdseyeComponentData();
expect(data.length).toBe(1);
});
+
+ it('should preserve the viewport when polling toggles dataReady for
the same process group', async () => {
Review Comment:
The production change looks correct, but this test never places the canvas
in its initialized state because `internalCanvasReady` remains false. As
written, the previous implementation could also pass without invoking either
viewport path.
This is nonblocking for NIFI-16397 because polling behavior predates its
scope. If we retain the fix here, it would still be valuable to make the test
exercise an initialized canvas; otherwise, this coverage could follow under
[NIFI-16400](https://issues.apache.org/jira/browse/NIFI-16400), which owns
troubleshooting polling and canvas-state transitions.
--
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]
