Skip to content

Commit f6d146f

Browse files
pvillard31pvillard31
authored
NIFI-15381 - Improve UX for the view Show/Revert Local Changes to account for environmental changes (#10681)
* NIFI-15381 - Improve UX for the view Show/Revert Local Changes to account for environmental changes Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com> * review --------- Signed-off-by: Pierre Villard <pierre.villard.fr@gmail.com>
1 parent 4d6eb98 commit f6d146f

9 files changed

Lines changed: 353 additions & 43 deletions

File tree

‎nifi-framework-bundle/nifi-framework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/dto/DifferenceDTO.java‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
public class DifferenceDTO {
2727
private String differenceType;
2828
private String difference;
29+
private Boolean isEnvironmental;
2930

3031
@Schema(description = "The type of difference")
3132
public String getDifferenceType() {
@@ -45,6 +46,16 @@ public void setDifference(String difference) {
4546
this.difference = difference;
4647
}
4748

49+
@Schema(description = "Whether this difference is environmental (e.g., bundle version change due to NiFi upgrade) " +
50+
"rather than a user-initiated change. Environmental changes are typically not reverted when reverting local changes.")
51+
public Boolean getEnvironmental() {
52+
return isEnvironmental;
53+
}
54+
55+
public void setEnvironmental(Boolean environmental) {
56+
isEnvironmental = environmental;
57+
}
58+
4859
@Override
4960
public boolean equals(final Object o) {
5061
if (this == o) {
@@ -54,11 +65,11 @@ public boolean equals(final Object o) {
5465
return false;
5566
}
5667
final DifferenceDTO that = (DifferenceDTO) o;
57-
return Objects.equals(differenceType, that.differenceType) && Objects.equals(difference, that.difference);
68+
return Objects.equals(differenceType, that.differenceType) && Objects.equals(difference, that.difference) && Objects.equals(isEnvironmental, that.isEnvironmental);
5869
}
5970

6071
@Override
6172
public int hashCode() {
62-
return Objects.hash(differenceType, difference);
73+
return Objects.hash(differenceType, difference, isEnvironmental);
6374
}
6475
}

‎nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5615,18 +5615,20 @@ public FlowComparisonEntity getLocalModifications(final String processGroupId) {
56155615
+ " but cannot find a Flow Registry with that identifier");
56165616
}
56175617

5618-
VersionedProcessGroup registryGroup = versionControlInfo.getFlowSnapshot();
5619-
if (registryGroup == null) {
5620-
try {
5621-
final FlowVersionLocation flowVersionLocation = new FlowVersionLocation(versionControlInfo.getBranch(), versionControlInfo.getBucketIdentifier(),
5622-
versionControlInfo.getFlowIdentifier(), versionControlInfo.getVersion());
5623-
final FlowSnapshotContainer flowSnapshotContainer = flowRegistry.getFlowContents(FlowRegistryClientContextFactory.getContextForUser(NiFiUserUtils.getNiFiUser()),
5624-
flowVersionLocation, true);
5625-
final RegisteredFlowSnapshot versionedFlowSnapshot = flowSnapshotContainer.getFlowSnapshot();
5626-
registryGroup = versionedFlowSnapshot.getFlowContents();
5627-
} catch (final IOException | FlowRegistryException e) {
5628-
throw new NiFiCoreException("Failed to retrieve flow with Flow Registry in order to calculate local differences due to " + e.getMessage(), e);
5629-
}
5618+
// Always fetch the flow from the registry to get the original bundle versions.
5619+
// The cached snapshot (versionControlInfo.getFlowSnapshot()) may have been mutated
5620+
// during import/revert operations by discoverCompatibleBundles(), which would cause
5621+
// bundle version differences to not be detected.
5622+
final VersionedProcessGroup registryGroup;
5623+
try {
5624+
final FlowVersionLocation flowVersionLocation = new FlowVersionLocation(versionControlInfo.getBranch(), versionControlInfo.getBucketIdentifier(),
5625+
versionControlInfo.getFlowIdentifier(), versionControlInfo.getVersion());
5626+
final FlowSnapshotContainer flowSnapshotContainer = flowRegistry.getFlowContents(FlowRegistryClientContextFactory.getContextForUser(NiFiUserUtils.getNiFiUser()),
5627+
flowVersionLocation, true);
5628+
final RegisteredFlowSnapshot versionedFlowSnapshot = flowSnapshotContainer.getFlowSnapshot();
5629+
registryGroup = versionedFlowSnapshot.getFlowContents();
5630+
} catch (final IOException | FlowRegistryException e) {
5631+
throw new NiFiCoreException("Failed to retrieve flow with Flow Registry in order to calculate local differences due to " + e.getMessage(), e);
56305632
}
56315633

56325634
final NiFiRegistryFlowMapper mapper = makeNiFiRegistryFlowMapper(controllerFacade.getExtensionManager());

‎nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/dto/DtoFactory.java‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2810,7 +2810,7 @@ public Set<ComponentDifferenceDTO> createComponentDifferenceDtosForLocalModifica
28102810
if (FlowDifferenceFilters.isBundleChange(difference)) {
28112811
final ComponentDifferenceDTO componentDiff = createBundleDifference(difference);
28122812
final Set<DifferenceDTO> differences = bundleDifferencesByComponent.computeIfAbsent(componentDiff, key -> new HashSet<>());
2813-
differences.add(createDifferenceDto(difference));
2813+
differences.add(createBundleDifferenceDto(difference));
28142814
}
28152815

28162816
// Ignore any environment-specific change
@@ -2829,17 +2829,13 @@ public Set<ComponentDifferenceDTO> createComponentDifferenceDtosForLocalModifica
28292829
differences.add(createDifferenceDto(difference));
28302830
}
28312831

2832-
if (!differencesByComponent.isEmpty()) {
2833-
// differences were found, so now let's add back in any BUNDLE_CHANGED differences
2834-
// since they were initially filtered out as an environment-specific change
2835-
bundleDifferencesByComponent.forEach((key, value) -> {
2836-
List<DifferenceDTO> values = value.stream().toList();
2837-
differencesByComponent.merge(key, values, (v1, v2) -> {
2838-
v1.addAll(v2);
2839-
return v1;
2840-
});
2832+
bundleDifferencesByComponent.forEach((key, value) -> {
2833+
final List<DifferenceDTO> values = value.stream().toList();
2834+
differencesByComponent.merge(key, values, (v1, v2) -> {
2835+
v1.addAll(v2);
2836+
return v1;
28412837
});
2842-
}
2838+
});
28432839

28442840
for (final Map.Entry<ComponentDifferenceDTO, List<DifferenceDTO>> entry : differencesByComponent.entrySet()) {
28452841
entry.getKey().setDifferences(entry.getValue());
@@ -2858,6 +2854,30 @@ DifferenceDTO createDifferenceDto(final FlowDifference difference) {
28582854
return dto;
28592855
}
28602856

2857+
/**
2858+
* Creates a DifferenceDTO for bundle changes, determining whether the change is environmental
2859+
* (due to NiFi upgrade where the original bundle version is not available) or user-initiated
2860+
* (user manually changed the bundle version when multiple versions are available).
2861+
*/
2862+
DifferenceDTO createBundleDifferenceDto(final FlowDifference difference) {
2863+
final DifferenceDTO dto = createDifferenceDto(difference);
2864+
2865+
final Object valueA = difference.getValueA();
2866+
if (valueA instanceof org.apache.nifi.flow.Bundle registryBundle) {
2867+
final BundleCoordinate registryCoordinate = new BundleCoordinate(
2868+
registryBundle.getGroup(),
2869+
registryBundle.getArtifact(),
2870+
registryBundle.getVersion()
2871+
);
2872+
2873+
dto.setEnvironmental(extensionManager.getBundle(registryCoordinate) == null);
2874+
} else {
2875+
dto.setEnvironmental(true);
2876+
}
2877+
2878+
return dto;
2879+
}
2880+
28612881
private Map<String, VersionedProcessGroup> flattenProcessGroups(final VersionedProcessGroup group) {
28622882
final Map<String, VersionedProcessGroup> flattened = new HashMap<>();
28632883
flattenProcessGroups(group, flattened);

‎nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/web/api/dto/DtoFactoryTest.java‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
import org.apache.nifi.nar.SystemBundle;
4444
import org.apache.nifi.processor.Relationship;
4545
import org.apache.nifi.registry.flow.FlowRegistryClientNode;
46+
import org.apache.nifi.registry.flow.diff.DifferenceType;
47+
import org.apache.nifi.registry.flow.diff.FlowDifference;
4648
import org.apache.nifi.web.api.entity.AllowableValueEntity;
4749
import org.junit.jupiter.api.Test;
4850
import org.slf4j.Logger;
@@ -659,4 +661,61 @@ void testConnectionDtoCopy() {
659661
assertNotSame(original.getAvailableRelationships(), copy.getAvailableRelationships());
660662
assertNotSame(original.getRetriedRelationships(), copy.getRetriedRelationships());
661663
}
664+
665+
@Test
666+
void testCreateBundleDifferenceDtoWhenRegistryBundleAvailable() {
667+
final org.apache.nifi.flow.Bundle registryBundle = new org.apache.nifi.flow.Bundle("com.example", "my-nar", "1.0.0");
668+
final BundleCoordinate expectedCoordinate = new BundleCoordinate("com.example", "my-nar", "1.0.0");
669+
670+
final FlowDifference difference = mock(FlowDifference.class);
671+
when(difference.getDifferenceType()).thenReturn(DifferenceType.BUNDLE_CHANGED);
672+
when(difference.getDescription()).thenReturn("Bundle changed from 1.0.0 to 2.0.0");
673+
when(difference.getValueA()).thenReturn(registryBundle);
674+
675+
final ExtensionManager extensionManager = mock(ExtensionManager.class);
676+
when(extensionManager.getBundle(eq(expectedCoordinate))).thenReturn(createBundle("com.example", "my-nar", "1.0.0"));
677+
678+
final DtoFactory dtoFactory = new DtoFactory();
679+
dtoFactory.setExtensionManager(extensionManager);
680+
681+
final DifferenceDTO dto = dtoFactory.createBundleDifferenceDto(difference);
682+
assertEquals(DifferenceType.BUNDLE_CHANGED.getDescription(), dto.getDifferenceType());
683+
assertFalse(dto.getEnvironmental());
684+
}
685+
686+
@Test
687+
void testCreateBundleDifferenceDtoWhenRegistryBundleNotAvailable() {
688+
final org.apache.nifi.flow.Bundle registryBundle = new org.apache.nifi.flow.Bundle("com.example", "my-nar", "1.0.0");
689+
690+
final FlowDifference difference = mock(FlowDifference.class);
691+
when(difference.getDifferenceType()).thenReturn(DifferenceType.BUNDLE_CHANGED);
692+
when(difference.getDescription()).thenReturn("Bundle changed from 1.0.0 to 2.0.0");
693+
when(difference.getValueA()).thenReturn(registryBundle);
694+
695+
final ExtensionManager extensionManager = mock(ExtensionManager.class);
696+
when(extensionManager.getBundle(any(BundleCoordinate.class))).thenReturn(null);
697+
698+
final DtoFactory dtoFactory = new DtoFactory();
699+
dtoFactory.setExtensionManager(extensionManager);
700+
701+
final DifferenceDTO dto = dtoFactory.createBundleDifferenceDto(difference);
702+
assertEquals(DifferenceType.BUNDLE_CHANGED.getDescription(), dto.getDifferenceType());
703+
assertTrue(dto.getEnvironmental());
704+
}
705+
706+
@Test
707+
void testCreateBundleDifferenceDtoWhenValueIsNotBundle() {
708+
final FlowDifference difference = mock(FlowDifference.class);
709+
when(difference.getDifferenceType()).thenReturn(DifferenceType.BUNDLE_CHANGED);
710+
when(difference.getDescription()).thenReturn("Bundle changed");
711+
when(difference.getValueA()).thenReturn("not-a-bundle");
712+
713+
final ExtensionManager extensionManager = mock(ExtensionManager.class);
714+
715+
final DtoFactory dtoFactory = new DtoFactory();
716+
dtoFactory.setExtensionManager(extensionManager);
717+
718+
final DifferenceDTO dto = dtoFactory.createBundleDifferenceDto(difference);
719+
assertTrue(dto.getEnvironmental());
720+
}
662721
}

‎nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/state/flow/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -865,6 +865,7 @@ export interface FlowUpdateRequestEntity {
865865
export interface Difference {
866866
differenceType: string;
867867
difference: string;
868+
environmental?: boolean;
868869
}
869870

870871
export interface ComponentDifference {

‎nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/local-changes-dialog/local-changes-dialog.html‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ <h2 mat-dialog-title>
3434
</div>
3535
<div class="flex-1">
3636
<local-changes-table
37+
[mode]="mode"
3738
[differences]="localModifications.componentDifferences"
3839
(goToChange)="goToChange.next($event)"></local-changes-table>
3940
</div>

‎nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/flow-designer/ui/canvas/items/flow/local-changes-dialog/local-changes-table/local-changes-table.html‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,21 @@
1919
<div>
2020
<div>
2121
<form [formGroup]="filterForm" class="my-2">
22-
<div class="flex pt-2">
22+
<div class="flex pt-2 items-center">
2323
<div class="mr-2">
2424
<mat-form-field subscriptSizing="dynamic">
2525
<mat-label>Filter</mat-label>
2626
<input matInput type="text" class="small" formControlName="filterTerm" />
2727
</mat-form-field>
2828
</div>
29+
@if (mode === 'SHOW') {
30+
<mat-checkbox
31+
[checked]="showEnvironmentalChanges"
32+
(change)="toggleEnvironmentalChanges()"
33+
class="ml-4">
34+
Show environmental changes ({{ environmentalCount }})
35+
</mat-checkbox>
36+
}
2937
</div>
3038
</form>
3139
<div class="my-2 tertiary-color leading-none font-medium">
@@ -55,6 +63,9 @@
5563
<ng-container matColumnDef="changeType">
5664
<th mat-header-cell *matHeaderCellDef mat-sort-header>Change Type</th>
5765
<td mat-cell *matCellDef="let item" [title]="formatChangeType(item)">
66+
@if (isEnvironmental(item)) {
67+
<i class="fa fa-info-circle neutral-color mr-2" title="Environmental change"></i>
68+
}
5869
{{ formatChangeType(item) }}
5970
</td>
6071
</ng-container>

0 commit comments

Comments
 (0)