-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
NIFI-13976: Adding endpoints to support copying and pasting flow cont… #9535
Conversation
…ents. - Including the revision for the current Process Group in the flow response. - Introduce a method propose built for adding new versioned components to an existing Process Group. - Updating the paste response payload to only include the entities for the components that were just added. - Always generating new IDs when copying components. - Creating an ID for the copy action. - Code clean up. - Mapping IDs on paste to ensure connections are created correctly. - Authorizing any components whose instance IDs are specified. - Copying any sensitive properties from copied instances when pasting. - Fixing mapping of copy response to ensure that nested Controller Services can be referenced. - Including external Controller Services in copy response to they can be resolved on paste. - Excluding unreferenced services from copy response. - Including property descriptors to allow external services to resolve when pasting. - Including Parameters and Parameter Providers in copy response. - Fixing issue returning unreferenced services in the current process group. - Reverting changes building up external service references. - Only including external services that are references by the copy selection. - Adding some additional test coverage. - Adding consideration for the current ids in the paste id mapping. - Adding an endpoint response merger for the paste endpoint. - Including version control information in copy response. - Being more lenient when pasting unknown registry clients. - Ensuring groups IDs are mapped correctly when and when not versioned. - Including registry client id in vci of versioned process groups. - Updating copy/paste logic in existing system tests to leverage new endpoints. - Restoring originally proposed port name if possible. - Allow ports to be pasted into the root group. - Fixing authorization issue when pasting a flow containing Parameter Providers. - Fixing issue when authorizing exporting a Flow that contains a Process Group bound to a Parameter Context. - Introducing some Copy/Paste system tests. - Adding system tests for verify various copy paste functionality. - Fix checkstyle issue. - Fixing checkstyle issues. - Ensuring appropriate access around Parameter Providers following resolution based on locally available Parameter Providers. - Ensuring user has access to controller services directly referenced. - Fixing NPE in log message of Parameter Context. - Adding more copy/paste system tests. - Moving copy/paste version tests into the CopyPasteIT. - Fixing unit test mocking. - Include service authorization based on versioned component id since it may resolve during synchronization. - Skipping properties with no values when resolving service identifiers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the detailed work on the support for copy-paste @mcgilman. I ran through an initial code review and noted a few minor questions and comments.
...amework/nifi-client-dto/src/main/java/org/apache/nifi/web/api/entity/CopyResponseEntity.java
Outdated
Show resolved
Hide resolved
...i-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/ProcessGroupAuthorizable.java
Show resolved
Hide resolved
...i-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/ProcessGroupAuthorizable.java
Outdated
Show resolved
Hide resolved
...i-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/ProcessGroupAuthorizable.java
Show resolved
Hide resolved
...mework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessGroupResource.java
Outdated
Show resolved
Hide resolved
...mework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessGroupResource.java
Show resolved
Hide resolved
...mework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ProcessGroupResource.java
Outdated
Show resolved
Hide resolved
.findFirst() | ||
.ifPresent(serviceEntry -> serviceEntry.setValue(serviceIdMapping.get(serviceEntry.getValue()))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want to use .forEach
here instead of .findFirst().ifPresent()
because we want to do this for all properties, right?
.findFirst() | ||
.ifPresent(serviceEntry -> serviceEntry.setValue(serviceIdMapping.get(serviceEntry.getValue()))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And a .forEach
here as well
Thanks @mcgilman I ran a bunch of tests, copying & pasting locally as well as copying from 1 instance and pasting to another. Tested simple processors/connections. Tested child groups. Tested with Controller Services. Tested with Parameter Contexts and references. Tested sensitive properties were removed, but not those that reference parameters. Tested Controller Service A referencing Controller Service B. Tested Processors with multiple Controller Services. Tested references to external Controller Services when (a) the external services doesn't exist in the destination instance (b) the external service does exist in the destination instance. Everything worked flawlessly. Code review looks good to me as well. I'm a +1! Many thanks for adding this feature, I'm very excited to be able to do this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…ents.
This PR can be tested with the corresponding front end changes available in this PR [1].
[1] #9536