rfellows commented on code in PR #11892: URL: https://github.com/apache/nifi/pull/11892#discussion_r4485971534
########## nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/service/resolver/connector-controller-service-advanced-ui-params.resolver.ts: ########## @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { ActivatedRouteSnapshot, ResolveFn } from '@angular/router'; +import { inject } from '@angular/core'; +import { catchError, EMPTY, map, of, switchMap, take } from 'rxjs'; +import { Store } from '@ngrx/store'; +import { HttpErrorResponse } from '@angular/common/http'; +import { NiFiState } from '../../../../state'; +import { AdvancedUiParams } from '../../../../state/shared'; +import { Client } from '../../../../service/client.service'; +import { ControllerServiceService } from '../../../flow-designer/service/controller-service.service'; +import { ClusterConnectionService } from '../../../../service/cluster-connection.service'; +import { ErrorHelper } from '../../../../service/error-helper.service'; +import { fullScreenError } from '../../../../state/error/error.actions'; +import { selectConnectorControllerServiceById } from '../../state/connector-controller-services/connector-controller-services.selectors'; + +export const connectorControllerServiceAdvancedUiParamsResolver: ResolveFn<AdvancedUiParams> = ( + route: ActivatedRouteSnapshot +) => { + const store: Store<NiFiState> = inject(Store); + const controllerServiceService: ControllerServiceService = inject(ControllerServiceService); + const client: Client = inject(Client); + const clusterConnectionService: ClusterConnectionService = inject(ClusterConnectionService); + const errorHelper: ErrorHelper = inject(ErrorHelper); + + const id: string | null = route.paramMap.get('serviceId'); + if (!id) { + return EMPTY; + } + + return store.select(selectConnectorControllerServiceById(id)).pipe( + switchMap((service) => { + if (service) { + return of(service); + } + return controllerServiceService.getControllerService(id).pipe( + catchError((errorResponse: HttpErrorResponse) => { + store.dispatch( + fullScreenError({ + errorDetail: { + title: 'Unable to Open Advanced UI', + message: errorHelper.getErrorString(errorResponse) + } + }) + ); + return EMPTY; + }) + ); + }), + map((entity) => { + const revision = client.getRevision(entity); + const editable = entity.status.runStatus === 'DISABLED' && entity.permissions.canWrite; + + return { + url: entity.component.customUiUrl, Review Comment: Same issue: `entity.component.customUiUrl` throws if the service isn't readable. You can reach this by deep-linking to the advanced route. Can we route unreadable or missing-`customUiUrl` cases to the existing `fullScreenError` path, matching how the fetch failure above is handled? ########## nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/state/connector-controller-services/connector-controller-services.effects.ts: ########## @@ -171,4 +231,811 @@ export class ConnectorControllerServicesEffects { ), { dispatch: false } ); + + /** + * Open the configure controller service dialog in editable mode (troubleshooting only). + * Fetches component history, wires property-table callbacks, and dispatches + * saveConnectorControllerService on submit. + */ + openConfigureConnectorControllerServiceDialog$ = createEffect( + () => + this.actions$.pipe( + ofType(ConnectorControllerServicesActions.openConfigureConnectorControllerServiceDialog), + map((action) => action.request), + filter((request) => request.controllerService.component != null), + concatLatestFrom(() => [ + this.store.select(selectConnectorIdFromRoute), + this.store.select(selectProcessGroupIdFromRoute) + ]), + switchMap(([request, connectorId, processGroupId]) => + from(this.propertyTableHelperService.getComponentHistory(request.id)).pipe( + map((history) => ({ + enrichedRequest: { ...request, history: history.componentHistory }, + connectorId, + processGroupId + })), + // Swallow the error so the outer switchMap stays alive and later + // configure requests still open the dialog. + catchError((err: HttpErrorResponse) => { + this.store.dispatch( + ErrorActions.addBannerError({ + errorContext: { + errors: [this.errorHelper.getErrorString(err)], + context: ErrorContextKey.CONTROLLER_SERVICES + } + }) + ); + return EMPTY; + }) + ) + ), + tap(({ enrichedRequest, connectorId, processGroupId }) => { + const serviceId = enrichedRequest.id; + + const dialogRef = this.dialog.open(EditControllerService, { + ...XL_DIALOG, + autoFocus: 'dialog', + data: enrichedRequest, + id: serviceId + }); + + const instance = dialogRef.componentInstance; + + instance.saving$ = this.store.select(selectConnectorControllerServicesSaving); + instance.convertToParameter = () => NEVER; + + instance.propertyVerificationResults$ = this.store.select(selectPropertyVerificationResults); + instance.propertyVerificationStatus$ = this.store.select(selectPropertyVerificationStatus); + instance.verify + .pipe(takeUntil(dialogRef.afterClosed())) + .subscribe((verificationRequest: VerifyPropertiesRequestContext) => { + this.store.dispatch(verifyProperties({ request: verificationRequest })); + }); + + instance.createNewProperty = this.propertyTableHelperService.createNewProperty( + serviceId, + this.controllerServiceService + ); + + instance.createNewService = this.propertyTableHelperService.createNewService( + serviceId, + this.controllerServiceService, + this.controllerServiceService, + processGroupId!, + (createdService) => { + this.store.dispatch( + ConnectorControllerServicesActions.inlineCreateConnectorControllerServiceSuccess({ + response: { controllerService: createdService } + }) + ); + } + ); + + instance.goToService = (targetServiceId: string) => { + this.controllerServiceService.getControllerService(targetServiceId).subscribe({ + next: (serviceEntity) => { + this.router.navigate([ + '/connectors', + connectorId, + 'canvas', + serviceEntity.component.parentGroupId, Review Comment: `ControllerServiceEntity.component` is now optional; the server omits it when the user can't read the target service. `goToService` dereferences it unconditionally here, so following a reference to a service the user can't read throws inside the subscribe instead of navigating or showing an error. `bindGoToService` already handles this by reading the entity-level `parentGroupId` and showing a banner when it's absent. Could this path do the same, for example `serviceEntity.component?.parentGroupId ?? serviceEntity.parentGroupId`, with a banner when neither is set? ########## nifi-frontend/src/main/frontend/libs/shared/src/types/entities/entities.spec.ts: ########## @@ -535,6 +536,51 @@ function createLabelEntity(componentValue: LabelDTO): LabelEntity { } describe('canonical response entity contracts', () => { + it('models readable and denied-read Controller Service response envelopes', () => { + const readable: ControllerServiceEntity = { + ...entityEnvelope('service'), + operatePermissions: permissions, + parentGroupId: 'root', + status: { runStatus: 'DISABLED', validationStatus: 'VALID' }, + bulletins: [], + component: { + id: 'service', + parentGroupId: 'root', + name: 'Service', + type: 'org.apache.nifi.Service', + bundle: { group: 'org.apache.nifi', artifact: 'service-nar', version: '1.0.0' }, + controllerServiceApis: [], + state: 'DISABLED', + bulletinLevel: 'WARN', + persistsState: false, + restricted: false, + deprecated: false, + extensionMissing: false, + multipleVersionsAvailable: false, + supportsSensitiveDynamicProperties: false, + properties: {}, + descriptors: {}, + validationStatus: 'VALID' + } + }; + const denied: ControllerServiceEntity = { + ...entityEnvelope('service'), + permissions: deniedPermissions, + operatePermissions: deniedPermissions, + parentGroupId: 'root', + status: { runStatus: 'DISABLED', validationStatus: 'VALID' } + }; + + expect(readable.component.name).toBe('Service'); Review Comment: Since `component` is now optional, `readable.component.name` doesn't compile under `tsc` (TS18048, possibly undefined). Vitest doesn't type-check, so the test still passes. `readable.component?.name` keeps the assertion and type-checks. ########## nifi-frontend/src/main/frontend/libs/shared/src/types/entities/controller-service-entity.ts: ########## @@ -16,9 +16,28 @@ */ import { Bundle, Permissions } from '../rest-api.types'; -import { ComponentDTO, EmbeddedComponentEntityBase } from './component-entity'; +import { ComponentDTO, ComponentEntityBase, EmbeddedComponentEntityBase } from './component-entity'; import { PropertyDescriptorDTO } from './property-descriptor-dto'; +/** + * @nifi-source: nifi-framework-bundle/nifi-framework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/entity/ControllerServiceEntity.java + * @nifi-revision: c31dc1f2d842 (2026-11-03) + * @vetted: 2026-11-04 + */ +export interface ControllerServiceEntity extends ComponentEntityBase<ControllerServiceDTO> { + operatePermissions: Permissions; + parentGroupId?: string; + status: ControllerServiceStatusDTO; +} + +/** + * @nifi-source: nifi-framework-bundle/nifi-framework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/dto/status/ControllerServiceStatusDTO.java + */ +export interface ControllerServiceStatusDTO { Review Comment: `ControllerServiceStatusDTO` extends `ComponentStatusDTO`, which also has `activeThreadCount` (an `Integer`, `ComponentStatusDTO.java:38`). Could we add `activeThreadCount?: number` so the type mirrors the cited DTO? Also, this block has `@nifi-source` but no `@nifi-revision` or `@vetted`, unlike the entity right above it. ########## nifi-frontend/src/main/frontend/apps/nifi/src/app/pages/connectors/state/connector-canvas/bind-go-to-service.ts: ########## @@ -66,9 +66,21 @@ export function bindGoToService( .pipe(takeUntil(dialogRef.afterClosed())) .subscribe({ next: (serviceEntity) => { + const processGroupId = serviceEntity.parentGroupId; Review Comment: I don't see a spec covering the branch where the fetched service has no `parentGroupId` and we post the banner. Could `connector-canvas.effects.spec.ts` add a case asserting `addBannerError` is dispatched and no navigation happens? -- 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]
