You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "exceptionfactory (via GitHub)" <gi...@apache.org> on 2023/04/17 20:37:29 UTC

[GitHub] [nifi] exceptionfactory opened a new pull request, #7181: NIFI-11461 Improve User and Group Tenants Search

exceptionfactory opened a new pull request, #7181:
URL: https://github.com/apache/nifi/pull/7181

   # Summary
   
   [NIFI-11461](https://issues.apache.org/jira/browse/NIFI-11461) Improves the performance of the User and Group Tenants Search REST Resource method with the addition of a `searchTenants` method to the `NiFiServiceFacade`.
   
   The previous implementation called `getUsers()` and `getUserGroups()` in the Tenants Resource, which requires retrieving all users and all group members before evaluating the query.
   
   The new implementation passes the query string to the `NiFiServiceFacade.searchTenants()` method and applies the filtering criteria prior to additional object creation. This approach reduces the number of method calls significantly for large numbers of users and groups.
   
   Additional changes include adjusting the user interface autocomplete delay from the default 300 ms to 500 ms, minimizing the number of HTTP requests in some scenarios.
   
   # Tracking
   
   Please complete the following tracking steps prior to pull request creation.
   
   ### Issue Tracking
   
   - [X] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
   
   ### Pull Request Tracking
   
   - [X] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
   - [X] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
   
   ### Pull Request Formatting
   
   - [X] Pull Request based on current revision of the `main` branch
   - [X] Pull Request refers to a feature branch with one commit containing changes
   
   # Verification
   
   Please indicate the verification steps performed prior to pull request creation.
   
   ### Build
   
   - [X] Build completed using `mvn clean install -P contrib-check`
     - [X] JDK 11
     - [ ] JDK 17
   
   ### Licensing
   
   - [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
   - [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
   
   ### Documentation
   
   - [ ] Documentation formatting appears as expected in rendered files
   


-- 
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


[GitHub] [nifi] mcgilman commented on pull request #7181: NIFI-11461 Improve User and Group Tenants Search

Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on PR #7181:
URL: https://github.com/apache/nifi/pull/7181#issuecomment-1512070669

   Will review...


-- 
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


[GitHub] [nifi] exceptionfactory commented on a diff in pull request #7181: NIFI-11461 Improve User and Group Tenants Search

Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #7181:
URL: https://github.com/apache/nifi/pull/7181#discussion_r1170473571


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java:
##########
@@ -4461,6 +4465,62 @@ public Set<UserEntity> getUsers() {
             .collect(Collectors.toSet());
     }
 
+    /**
+     * Search for User and Group Tenants with optimized conversion from specific objects to Tenant objects
+     *
+     * @param query Search query where null or empty returns unfiltered results
+     * @return Tenants Entity containing zero or more matching Users and Groups
+     */
+    @Override
+    public TenantsEntity searchTenants(final String query) {
+        final PermissionsDTO permissions = dtoFactory.createPermissionsDto(authorizableLookup.getTenant());
+
+        final Set<TenantEntity> usersFound = userDAO.getUsers()
+                .stream()
+                .filter(user -> isMatched(user.getIdentity(), query))
+                .map(user -> createTenantEntity(user, permissions))
+                .collect(Collectors.toSet());
+
+        final Set<TenantEntity> userGroupsFound = userGroupDAO.getUserGroups()
+                .stream()
+                .filter(userGroup -> isMatched(userGroup.getName(), query))
+                .map(userGroup -> createTenantEntity(userGroup, permissions))
+                .collect(Collectors.toSet());
+
+        final TenantsEntity tenantsEntity = new TenantsEntity();
+        tenantsEntity.setUsers(usersFound);
+        tenantsEntity.setUserGroups(userGroupsFound);
+        return tenantsEntity;
+    }
+
+    private boolean isMatched(final String label, final String query) {
+        return StringUtils.isEmpty(query) || containsIgnoreCase(label, query);
+    }
+
+    private TenantEntity createTenantEntity(final User user, final PermissionsDTO permissions) {
+        final TenantDTO tenant = dtoFactory.createTenantDTO(user);
+        return createTenantEntity(tenant, permissions);
+    }
+
+    private TenantEntity createTenantEntity(final Group userGroup, final PermissionsDTO permissions) {

Review Comment:
   Thanks for pointer to `EntityFactory`, I pushed an update removing the private methods and adjusting the stream function to make use of `EntityFactory.createTenantEntity()`.



-- 
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


[GitHub] [nifi] mcgilman merged pull request #7181: NIFI-11461 Improve User and Group Tenants Search

Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman merged PR #7181:
URL: https://github.com/apache/nifi/pull/7181


-- 
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


[GitHub] [nifi] mcgilman commented on a diff in pull request #7181: NIFI-11461 Improve User and Group Tenants Search

Posted by "mcgilman (via GitHub)" <gi...@apache.org>.
mcgilman commented on code in PR #7181:
URL: https://github.com/apache/nifi/pull/7181#discussion_r1170313272


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/StandardNiFiServiceFacade.java:
##########
@@ -4461,6 +4465,62 @@ public Set<UserEntity> getUsers() {
             .collect(Collectors.toSet());
     }
 
+    /**
+     * Search for User and Group Tenants with optimized conversion from specific objects to Tenant objects
+     *
+     * @param query Search query where null or empty returns unfiltered results
+     * @return Tenants Entity containing zero or more matching Users and Groups
+     */
+    @Override
+    public TenantsEntity searchTenants(final String query) {
+        final PermissionsDTO permissions = dtoFactory.createPermissionsDto(authorizableLookup.getTenant());
+
+        final Set<TenantEntity> usersFound = userDAO.getUsers()
+                .stream()
+                .filter(user -> isMatched(user.getIdentity(), query))
+                .map(user -> createTenantEntity(user, permissions))
+                .collect(Collectors.toSet());
+
+        final Set<TenantEntity> userGroupsFound = userGroupDAO.getUserGroups()
+                .stream()
+                .filter(userGroup -> isMatched(userGroup.getName(), query))
+                .map(userGroup -> createTenantEntity(userGroup, permissions))
+                .collect(Collectors.toSet());
+
+        final TenantsEntity tenantsEntity = new TenantsEntity();
+        tenantsEntity.setUsers(usersFound);
+        tenantsEntity.setUserGroups(userGroupsFound);
+        return tenantsEntity;
+    }
+
+    private boolean isMatched(final String label, final String query) {
+        return StringUtils.isEmpty(query) || containsIgnoreCase(label, query);
+    }
+
+    private TenantEntity createTenantEntity(final User user, final PermissionsDTO permissions) {
+        final TenantDTO tenant = dtoFactory.createTenantDTO(user);
+        return createTenantEntity(tenant, permissions);
+    }
+
+    private TenantEntity createTenantEntity(final Group userGroup, final PermissionsDTO permissions) {

Review Comment:
   Our `EntityFactory` already contains methods for creating a `TenantEntity` given a `tenant`, `revision`, and `permissions`. Can we use that in favor of these new methods?



-- 
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