[GitHub] [nifi] tpalfy commented on a diff in pull request #5958: NIFI-9895 Allow parameters to reference controller services

[GitBox <gi...@apache.org>]

tpalfy commented on code in PR #5958:
URL: https://github.com/apache/nifi/pull/5958#discussion_r850517804


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ParameterContextResource.java:
##########
@@ -390,12 +397,62 @@ public Response submitParameterContextUpdate(
             requestRevision,
             lookup -> {
                 // Verify READ and WRITE permissions for user, for the Parameter Context itself
-                final Authorizable parameterContext = lookup.getParameterContext(contextId);
+                final ParameterContext parameterContext = lookup.getParameterContext(contextId);
                 parameterContext.authorize(authorizer, RequestAction.READ, user);
                 parameterContext.authorize(authorizer, RequestAction.WRITE, user);
 
                 // Verify READ and WRITE permissions for user, for every component that is affected
                 affectedComponents.forEach(component -> authorizeAffectedComponent(component, lookup, user, true, true));
+
+                requestEntity.getComponent().getParameters().forEach(parameterEntity -> {
+                    String parameterName = parameterEntity.getParameter().getName();
+                    List<ParameterReferencedControllerServiceData> referencedControllerServiceDataSet = parameterContext
+                        .getParameterReferenceManager()
+                        .getReferencedControllerServiceData(parameterContext, parameterName);
+
+                    Set<? extends Class<? extends ControllerService>> referencedControllerServiceTypes = referencedControllerServiceDataSet
+                        .stream()
+                        .map(ParameterReferencedControllerServiceData::getReferencedControllerServiceType)
+                        .collect(Collectors.toSet());
+
+                    if (referencedControllerServiceTypes.size() > 1) {
+                        throw new IllegalStateException("Parameter is used by multiple different types of controller service references");
+                    } else if (!referencedControllerServiceTypes.isEmpty()) {
+                        Optional.ofNullable(parameterEntity)
+                            .map(ParameterEntity::getParameter)
+                            .map(ParameterDTO::getName)
+                            .flatMap(parameterContext::getParameter)
+                            .map(Parameter::getValue)
+                            .map(lookup::getControllerService)
+                            .map(ComponentAuthorizable::getAuthorizable)
+                            .ifPresent(controllerServiceAuthorizable -> {
+                                controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
+                                controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
+                            });
+
+                        Optional.ofNullable(parameterEntity)
+                            .map(ParameterEntity::getParameter)
+                            .map(ParameterDTO::getValue)
+                            .map(lookup::getControllerService)
+                            .map(ComponentAuthorizable::getAuthorizable)
+                            .ifPresent(controllerServiceAuthorizable -> {
+                                controllerServiceAuthorizable.authorize(authorizer, RequestAction.READ, user);
+                                controllerServiceAuthorizable.authorize(authorizer, RequestAction.WRITE, user);
+
+                                if (
+                                    !referencedControllerServiceTypes
+                                        .stream()
+                                        .findFirst()
+                                        .get()
+                                        .isAssignableFrom(
+                                            ((StandardControllerServiceNode) controllerServiceAuthorizable).getComponent().getClass()
+                                        )
+                                ) {
+                                    throw new IllegalArgumentException("New Parameter value attempts to reference an incompatible controller service");
+                                }
+                            });
+                    }
+                });

Review Comment:
   The Guava's recommendations are for Guava's own functional API _specifically_.
   The very reason is the lack of language-level support. This makes the use of Guava functional API bloated with boilerplate code. So the advantages of the functional approach is counteracted with the disadvantages of the cumbersome API.
   
   That is not the case for the java 8 API. It was deliberately designed to be much more user(dev)-friendly and is very readable. This point is supported by the fact that in the Guava documentation the provided examples showing bad readability in certain circumstances would not hold when used the java 8 API.
   
   Effective Java mostly focuses on streams when addressing this issue. My cases are very different as it concerns optionals. And while I do agree that the overuse of stream (and stream-like) API can be to the detriment of understandability in many cases, I'd argue it isn't the case _every time_.
   
   In my cases each transformation logic is a one-step value lookup and the steps follow a direct outer-scope -> inner-scope flow. Can't go much simpler than that in my opinion.
   Also using stream-like api for optionals saves on the multiple annoying and not at all reader-friendly null-checks. The alternative would be several layered if-else statements with half the code being boilerplate.



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org