From ca71a310059fa15c2c4653780bfbfdac605d3ea7 Mon Sep 17 00:00:00 2001 From: sadilchamishka Date: Thu, 7 Nov 2024 07:26:04 +0530 Subject: [PATCH 1/3] Optimize user count retrieval logic in user filter operation to reduce extensive DB calls --- .../scim2/common/impl/SCIMUserManager.java | 186 +++++++++++++++++- .../common/impl/SCIMUserManagerTest.java | 9 + pom.xml | 2 +- 3 files changed, 192 insertions(+), 5 deletions(-) diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java index 275c7b7f..7690e5b1 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java @@ -1445,7 +1445,7 @@ private UsersGetResponse filterUsersBySingleAttribute(ExpressionNode node, Map 1) { @@ -6451,4 +6451,182 @@ private String maskIfRequired(String value) { return LoggerUtils.isLogMaskingEnable ? LoggerUtils.getMaskedContent(value) : value; } + + /** + * This method retrieves the count of users based on the filter query without depending on pagination params. + * + * @param node Filter condition tree. + * @param offset Starting index of the count. + * @param maxLimit The maximum number of users to be counted. + * @param domainName Domain that the filter should perform. If empty, filtering is applied to all available domains. + * @return The count of users that match the filtering conditions. + * @throws CharonException Error while filtering the users. + * @throws BadRequestException Domain miss match in domain parameter and attribute value. + */ + private int getFilteredUsersCount(Node node, int offset, int maxLimit, String domainName) + throws CharonException, BadRequestException { + + if (SCIMConstants.UserSchemaConstants.GROUP_URI.equals(((ExpressionNode) node).getAttributeValue())) { + return filterUsersByGroup(node, offset, maxLimit, domainName).size(); + } + if (StringUtils.isNotEmpty(domainName)) { + Condition condition = createConditionForSingleAttributeFilter(domainName, node); + return getFilteredUserCountFromSingleDomain(condition, offset, maxLimit, domainName); + } else { + return getFilteredUserCountFromMultipleDomains(node, offset, maxLimit, null); + } + } + + /** + * Retrieves the count of users from multiple domains based on the specified filtering conditions, limit, and offset. + * + * @param node Expression or Operation node. + * @param offset The starting index for counting users. + * @param limit The maximum number of users to consider. + * @param conditionForListingUsers Condition for listing users when the function is used to list users + * except for filtering. For filtering this value should be set to NULL. + * @return The total count of users that match the filtering conditions across all specified domains. + * @throws CharonException Error while filtering the users. + * @throws BadRequestException Domain miss match in domain parameter and attribute value. + */ + private int getFilteredUserCountFromMultipleDomains(Node node, int offset, int limit, + Condition conditionForListingUsers) + throws CharonException, BadRequestException { + + // Filter users when the domain is not set in the request. Then filter through multiple domains. + String[] userStoreDomainNames = getDomainNames(); + int filteredUsersCount = 0; + + Condition condition; + for (String userStoreDomainName : userStoreDomainNames) { + + // Check for a user listing scenario. (For filtering this value will be set to NULL) + if (conditionForListingUsers == null) { + + if (isLoginIdentifiersEnabled() && SCIMConstants.UserSchemaConstants.USER_NAME_URI + .equals(((ExpressionNode) node).getAttributeValue())) { + try { + ((ExpressionNode) node).setAttributeValue(getScimUriForPrimaryLoginIdentifier(node)); + } catch (org.wso2.carbon.user.core.UserStoreException e) { + throw new CharonException("Error in retrieving scim to local mappings.", e); + } + } + // Create filter condition for each domain for single attribute filter. + condition = createConditionForSingleAttributeFilter(userStoreDomainName, node); + } else { + condition = conditionForListingUsers; + } + + // Filter users for given condition and domain. + try { + filteredUsersCount += + getFilteredUserCountFromSingleDomain(condition, offset, limit, userStoreDomainName); + } catch (CharonException e) { + log.error("Error occurred while getting the filtered users count for domain: " + userStoreDomainName, + e); + } + } + return filteredUsersCount; + } + + /** + * Retrieves the count of users from a single domain based on the specified filtering condition, limit, and offset. + * + * @param condition The filtering condition to be applied. + * @param offset The starting index of the count. + * @param limit The maximum number of users to consider. + * @param domainName The domain in which to search for users. + * @return The count of users that match the filtering conditions within the specified domain. + * @throws CharonException Error while filtering the users. + * @throws BadRequestException Domain miss match in domain parameter and attribute value. + */ + private int getFilteredUserCountFromSingleDomain(Condition condition, int offset, int limit, String domainName) + throws CharonException, BadRequestException { + + if (log.isDebugEnabled()) { + log.debug(String.format("Getting the filtered users count in domain : %s with limit: %d and offset: %d.", + domainName, limit, offset)); + } + try { + return carbonUM.getUsersCount(condition, domainName, UserCoreConstants.DEFAULT_PROFILE, limit, offset, + removeDuplicateUsersInUsersResponseEnabled); + } catch (UserStoreException e) { + // Sometimes client exceptions are wrapped in the super class. + // Therefore checking for possible client exception. + Throwable rootCause = ExceptionUtils.getRootCause(e); + String errorMessage = String.format( + "Error while retrieving filtered users count for the domain: %s with limit: %d and offset: %d. %s", + domainName, limit, offset, rootCause != null ? rootCause.getMessage() : e.getMessage()); + if (log.isDebugEnabled()) { + log.debug(errorMessage, e); + } + if (e instanceof UserStoreClientException || rootCause instanceof UserStoreClientException) { + throw new BadRequestException(errorMessage, ResponseCodeConstants.INVALID_VALUE); + } else { + throw new CharonException(errorMessage, e); + } + } + } + + /** + * This method retrieves the count of users based on multi-attribute filtering. + * + * @param node Filter condition tree. + * @param offset Starting index of the count. + * @param domainName Domain that the filter should perform. If empty, filtering is applied to all available domains. + * @param maxLimit The maximum number of users to be counted. + * @return The count of users that match the filtering conditions. + * @throws CharonException Error while filtering the users. + * @throws BadRequestException Domain miss match in domain parameter and attribute value. + */ + private int getMultiAttributeFilteredUsersCount(Node node, int offset, String domainName, int maxLimit) + throws CharonException, BadRequestException { + + int filteredUsersCount = 0; + if (StringUtils.isNotEmpty(domainName)) { + filteredUsersCount += + getMultiAttributeFilteredUsersCountFromSingleDomain(node, offset, domainName, maxLimit); + } else { + AbstractUserStoreManager tempCarbonUM = carbonUM; + // If domain name are not given, then perform filtering on all available user stores. + while (tempCarbonUM != null && maxLimit > 0) { + // If carbonUM is not an instance of Abstract User Store Manger we can't get the domain name. + if (tempCarbonUM instanceof AbstractUserStoreManager) { + domainName = + tempCarbonUM.getRealmConfiguration().getUserStoreProperty(UserCoreConstants.RealmConfig. + PROPERTY_DOMAIN_NAME); + filteredUsersCount += + getMultiAttributeFilteredUsersCountFromSingleDomain(node, offset, domainName, maxLimit); + } + // If secondary user store manager assigned to carbonUM then global variable carbonUM will contains the + // secondary user store manager. + tempCarbonUM = (AbstractUserStoreManager) tempCarbonUM.getSecondaryUserStoreManager(); + } + } + return filteredUsersCount; + } + + /** + * This method retrieves the count of users based on multi-attribute filtering. + * + * @param node Filter condition tree. + * @param offset Starting index of the count. + * @param domainName Domain that the filter should perform. + * @param limit The number of users to be counted. + * @return The count of users that match the filtering conditions. + * @throws CharonException Error while filtering the users. + * @throws BadRequestException Domain miss match in domain parameter and attribute value. + */ + private int getMultiAttributeFilteredUsersCountFromSingleDomain(Node node, int offset, String domainName, int limit) + throws CharonException, BadRequestException { + + if (log.isDebugEnabled()) { + log.debug(String.format( + "Getting users count filtered by multi attributes in domain : %s with limit: %d and offset: %d.", + domainName, limit, offset)); + } + Map attributes = getAllAttributes(domainName); + Condition condition = getCondition(node, attributes); + return getFilteredUserCountFromSingleDomain(condition, offset, limit, domainName); + } } diff --git a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java index 3dd0fb46..c939c7c8 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java @@ -675,6 +675,15 @@ public void testFilteringUsersWithGETWithPagination(List()); if(domain.equals("PRIMARY")){ diff --git a/pom.xml b/pom.xml index 471edf2c..d1f85100 100644 --- a/pom.xml +++ b/pom.xml @@ -278,7 +278,7 @@ 3.3.7 6.5.3 3.2.0.wso2v1 - 4.10.16 + 4.10.17 7.5.109 4.13.1 20030203.000129 From bb8fa39aafa5236957dd6879bb232f6af89a22df Mon Sep 17 00:00:00 2001 From: sadilchamishka Date: Thu, 7 Nov 2024 07:39:42 +0530 Subject: [PATCH 2/3] Fix multi-attribute user filter with count provided. --- .../scim2/common/impl/SCIMUserManager.java | 68 ++++++++++++++++--- 1 file changed, 58 insertions(+), 10 deletions(-) diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java index 7690e5b1..d4021373 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java @@ -2115,14 +2115,15 @@ private UsersGetResponse getMultiAttributeFilteredUsers(Node node, Map 0) { - users = getFilteredUsersFromMultiAttributeFiltering(node, offset, limit, sortBy, sortOrder, domainName, + users = getMultiAttributeFilteredUsersWithMaxLimit(node, offset, sortBy, sortOrder, domainName, limit, true); - filteredUsers.addAll(getFilteredUserDetails(users, requiredAttributes)); } else { int maxLimit = getMaxLimit(domainName); - users = getMultiAttributeFilteredUsersWithMaxLimit(node, offset, sortBy, sortOrder, domainName, maxLimit); - filteredUsers.addAll(getFilteredUserDetails(users, requiredAttributes)); + users = getMultiAttributeFilteredUsersWithMaxLimit(node, offset, sortBy, sortOrder, domainName, maxLimit, + false); } + filteredUsers.addAll(getFilteredUserDetails(users, requiredAttributes)); + // Check that total user count matching the client query needs to be calculated. if (isJDBCUSerStore(domainName) || isAllConfiguredUserStoresJDBC() || SCIMCommonUtils.isConsiderTotalRecordsForTotalResultOfLDAPEnabled()) { @@ -2144,27 +2145,41 @@ private UsersGetResponse getMultiAttributeFilteredUsers(Node node, Map getMultiAttributeFilteredUsersWithMaxLimit(Node node, int offset, - String sortBy, String sortOrder, String domainName, int maxLimit) + String sortBy, String sortOrder, String domainName, int maxLimit, boolean paginationRequested) throws CharonException, BadRequestException { - Set users = null; + Set users = new LinkedHashSet<>(); if (StringUtils.isNotEmpty(domainName)) { users = getFilteredUsersFromMultiAttributeFiltering(node, offset, maxLimit, sortBy, sortOrder, domainName, false); return users; } else { AbstractUserStoreManager tempCarbonUM = carbonUM; - // If pagination and domain name are not given, then perform filtering on all available user stores. - while (tempCarbonUM != null) { + // If domain name are not given, then perform filtering on all available user stores. + while (tempCarbonUM != null && maxLimit > 0) { // If carbonUM is not an instance of Abstract User Store Manger we can't get the domain name. if (tempCarbonUM instanceof AbstractUserStoreManager) { domainName = tempCarbonUM.getRealmConfiguration().getUserStoreProperty("DomainName"); - users = getFilteredUsersFromMultiAttributeFiltering(node, offset, maxLimit, sortBy, sortOrder, - domainName, false); + users.addAll(getFilteredUsersFromMultiAttributeFiltering(node, offset, maxLimit, sortBy, sortOrder, + domainName, paginationRequested)); } // If secondary user store manager assigned to carbonUM then global variable carbonUM will contains the // secondary user store manager. tempCarbonUM = (AbstractUserStoreManager) tempCarbonUM.getSecondaryUserStoreManager(); + + // Calculating new offset and limit parameters. + int numberOfFilteredUsers = users.size(); + if (numberOfFilteredUsers <= 0 && offset > 1) { + if (log.isDebugEnabled()) { + log.debug(String.format("Filter returned no results for original offset: %d.", offset)); + } + offset = calculateOffsetForMultiAttributeFilter(node, offset, sortBy, sortOrder, domainName); + } else { + // Returned usernames size > 0 implies there are users in that domain which is larger than + // the offset. + offset = 1; + maxLimit = calculateLimit(maxLimit, numberOfFilteredUsers); + } } return users; } @@ -6629,4 +6644,37 @@ private int getMultiAttributeFilteredUsersCountFromSingleDomain(Node node, int o Condition condition = getCondition(node, attributes); return getFilteredUserCountFromSingleDomain(condition, offset, limit, domainName); } + + /** + * Method to update the offset when iterating a filter across all domains with multi attribute filter. + * + * @param node Condition node of the multi attribute filter + * @param offset Starting index + * @param sortBy Sort by + * @param sortOrder Sort order + * @param domainName Domain to be filtered + * @return New calculated offset + * @throws CharonException Error while filtering the domain from index 1 to offset + */ + private int calculateOffsetForMultiAttributeFilter(Node node, int offset, String sortBy, String sortOrder, + String domainName) throws CharonException, BadRequestException { + + if (log.isDebugEnabled()) { + log.debug(String.format("Checking for number of matches from the beginning to the original offset: %d for " + + "the same filter and updating the new offset.", offset)); + } + // Starting index of the filter + int initialOffset = 1; + + // Checking the number of matches till the original offset. + boolean paginationRequest = false; + Set skippedUsers = + getFilteredUsersFromMultiAttributeFiltering(node, initialOffset, offset, sortBy, sortOrder, domainName, + paginationRequest); + + int skippedUserCount = skippedUsers.size(); + + // Calculate the new offset and return + return offset - skippedUserCount; + } } From 7282944609c9deb0b54a3eac655d366bcf7aca7a Mon Sep 17 00:00:00 2001 From: sadilchamishka Date: Thu, 7 Nov 2024 08:19:41 +0530 Subject: [PATCH 3/3] Improve identity and non-identity claim filtering --- .../scim2/common/impl/SCIMUserManager.java | 117 +++++++++++------- .../common/impl/SCIMUserManagerTest.java | 88 +++++++------ pom.xml | 2 +- 3 files changed, 119 insertions(+), 88 deletions(-) diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java index d4021373..09b5155e 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManager.java @@ -66,6 +66,7 @@ import org.wso2.carbon.user.core.UserStoreManager; import org.wso2.carbon.user.core.claim.ClaimManager; import org.wso2.carbon.user.core.common.AbstractUserStoreManager; +import org.wso2.carbon.user.core.common.PaginatedUserResponse; import org.wso2.carbon.user.core.constants.UserCoreClaimConstants; import org.wso2.carbon.user.core.constants.UserCoreErrorConstants; import org.wso2.carbon.user.core.jdbc.JDBCUserStoreManager; @@ -2110,66 +2111,78 @@ private UsersGetResponse getMultiAttributeFilteredUsers(Node node, Map filteredUsers = new ArrayList<>(); - Set users; + List filteredUserDetails; + int maxLimit = getMaxLimitForTotalResults(domainName); + int totalResults = 0; + PaginatedUserResponse paginatedUserResult; + // Handle pagination. if (limit > 0) { - users = getMultiAttributeFilteredUsersWithMaxLimit(node, offset, sortBy, sortOrder, domainName, limit, + paginatedUserResult = getMultiAttributeFilteredUsersWithMaxLimit(node, offset, sortBy, sortOrder, domainName, limit, true); } else { - int maxLimit = getMaxLimit(domainName); - users = getMultiAttributeFilteredUsersWithMaxLimit(node, offset, sortBy, sortOrder, domainName, maxLimit, + paginatedUserResult = getMultiAttributeFilteredUsersWithMaxLimit(node, offset, sortBy, sortOrder, domainName, maxLimit, false); } - filteredUsers.addAll(getFilteredUserDetails(users, requiredAttributes)); + filteredUserDetails = getFilteredUserDetails(new LinkedHashSet<>(paginatedUserResult.getFilteredUsers()), + requiredAttributes); // Check that total user count matching the client query needs to be calculated. - if (isJDBCUSerStore(domainName) || isAllConfiguredUserStoresJDBC() || - SCIMCommonUtils.isConsiderTotalRecordsForTotalResultOfLDAPEnabled()) { - int maxLimit = getMaxLimitForTotalResults(domainName); - if (!SCIMCommonUtils.isConsiderMaxLimitForTotalResultEnabled()) { - maxLimit = Math.max(maxLimit, limit); + if (paginatedUserResult.getTotalResults() > 0) { + /* If the total result is provided with the paginated user result, give the priority instead of calculating + user total count. */ + totalResults = paginatedUserResult.getTotalResults(); + if (totalResults > maxLimit && SCIMCommonUtils.isConsiderMaxLimitForTotalResultEnabled()) { + totalResults = maxLimit; } - // Get total users based on the filter query. - totalResults += getMultiAttributeFilteredUsersCount(node, 1, domainName, maxLimit); - } else { - totalResults += filteredUsers.size(); - if (totalResults == 0 && filteredUsers.size() > 1) { - totalResults = filteredUsers.size() - 1; + if (isJDBCUSerStore(domainName) || isAllConfiguredUserStoresJDBC() || + SCIMCommonUtils.isConsiderTotalRecordsForTotalResultOfLDAPEnabled()) { + // Get total users based on the filter query. + totalResults += getMultiAttributeFilteredUsersCount(node, 1, domainName, maxLimit); + + } else { + totalResults += paginatedUserResult.getFilteredUsers().size(); + if (totalResults == 0 && paginatedUserResult.getFilteredUsers().size() > 1) { + totalResults = paginatedUserResult.getFilteredUsers().size() - 1; + } } } - return getDetailedUsers(filteredUsers, totalResults); + + return getDetailedUsers(filteredUserDetails, totalResults); } - private Set getMultiAttributeFilteredUsersWithMaxLimit(Node node, int offset, + private PaginatedUserResponse getMultiAttributeFilteredUsersWithMaxLimit(Node node, int offset, String sortBy, String sortOrder, String domainName, int maxLimit, boolean paginationRequested) throws CharonException, BadRequestException { - Set users = new LinkedHashSet<>(); if (StringUtils.isNotEmpty(domainName)) { - users = getFilteredUsersFromMultiAttributeFiltering(node, offset, maxLimit, sortBy, sortOrder, domainName, + return getFilteredUsersFromMultiAttributeFiltering(node, offset, maxLimit, sortBy, sortOrder, domainName, false); - return users; } else { + PaginatedUserResponse combinedPaginatedUserResult = new PaginatedUserResponse(); AbstractUserStoreManager tempCarbonUM = carbonUM; // If domain name are not given, then perform filtering on all available user stores. - while (tempCarbonUM != null && maxLimit > 0) { - // If carbonUM is not an instance of Abstract User Store Manger we can't get the domain name. - if (tempCarbonUM instanceof AbstractUserStoreManager) { - domainName = tempCarbonUM.getRealmConfiguration().getUserStoreProperty("DomainName"); - users.addAll(getFilteredUsersFromMultiAttributeFiltering(node, offset, maxLimit, sortBy, sortOrder, - domainName, paginationRequested)); - } + while (tempCarbonUM != null && checkIterateOverDomainRequired(maxLimit, combinedPaginatedUserResult)) { + domainName = tempCarbonUM.getRealmConfiguration().getUserStoreProperty("DomainName"); + PaginatedUserResponse paginatedUserResult = getFilteredUsersFromMultiAttributeFiltering(node, offset, + maxLimit, sortBy, sortOrder, domainName, paginationRequested); + // Accumulate the filtered users. + combinedPaginatedUserResult.getFilteredUsers().addAll(paginatedUserResult.getFilteredUsers()); + + // Accumulate the total user counts. + combinedPaginatedUserResult.setTotalResults(combinedPaginatedUserResult.getTotalResults() + + paginatedUserResult.getTotalResults()); + // If secondary user store manager assigned to carbonUM then global variable carbonUM will contains the // secondary user store manager. tempCarbonUM = (AbstractUserStoreManager) tempCarbonUM.getSecondaryUserStoreManager(); // Calculating new offset and limit parameters. - int numberOfFilteredUsers = users.size(); - if (numberOfFilteredUsers <= 0 && offset > 1) { + int numberOfFilteredUsers = combinedPaginatedUserResult.getFilteredUsers().size(); + if (numberOfFilteredUsers == 0 && offset > 1) { if (log.isDebugEnabled()) { log.debug(String.format("Filter returned no results for original offset: %d.", offset)); } @@ -2181,7 +2194,7 @@ private Set getMultiAttributeFilteredUser maxLimit = calculateLimit(maxLimit, numberOfFilteredUsers); } } - return users; + return combinedPaginatedUserResult; } } /** @@ -2386,13 +2399,10 @@ private Map getMappedAttributes(String extClaimDialectName, Stri * @return * @throws CharonException */ - private Set getFilteredUsersFromMultiAttributeFiltering(Node node, - int offset, - int limit, - String sortBy, - String sortOrder, - String domainName, - Boolean paginationRequested) + private PaginatedUserResponse getFilteredUsersFromMultiAttributeFiltering(Node node, int offset, int limit, + String sortBy, String sortOrder, + String domainName, + Boolean paginationRequested) throws CharonException, BadRequestException { Set coreUsers; @@ -2422,9 +2432,13 @@ private Set getFilteredUsersFromMultiAttr if (paginationRequested) { checkForPaginationSupport(condition); } - coreUsers.addAll(carbonUM.getUserListWithID(condition, domainName, - UserCoreConstants.DEFAULT_PROFILE, limit, offset, sortBy, sortOrder)); - return coreUsers; + PaginatedUserResponse paginatedUserResult = carbonUM.getPaginatedUserListWithID(condition, domainName, + UserCoreConstants.DEFAULT_PROFILE, limit, offset, sortBy, sortOrder); + + // Remove duplicates. + coreUsers.addAll(paginatedUserResult.getFilteredUsers()); + paginatedUserResult.setFilteredUsers(new ArrayList<>(coreUsers)); + return paginatedUserResult; } catch (UserStoreException e) { throw resolveError(e, "Error in filtering users by multi attributes in domain: " + domainName); } @@ -6668,13 +6682,22 @@ private int calculateOffsetForMultiAttributeFilter(Node node, int offset, String // Checking the number of matches till the original offset. boolean paginationRequest = false; - Set skippedUsers = - getFilteredUsersFromMultiAttributeFiltering(node, initialOffset, offset, sortBy, sortOrder, domainName, - paginationRequest); - - int skippedUserCount = skippedUsers.size(); + int skippedUserCount = getFilteredUsersFromMultiAttributeFiltering(node, initialOffset, offset, sortBy, + sortOrder, domainName, paginationRequest).getFilteredUsers().size(); // Calculate the new offset and return return offset - skippedUserCount; } + + private boolean checkIterateOverDomainRequired(int limit, PaginatedUserResponse paginatedUserResponse) { + + /* When paginated user response contains total result, need to iterate over all the user stores and accumulate + total matching user count as total count is not calculated afterward. */ + if (paginatedUserResponse.getTotalResults() > 0) { + return true; + } + + // When required users are retrieved and limit is set to zero, skip iterating the user stores. + return limit > 0; + } } diff --git a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java index c939c7c8..26c66e95 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/impl/SCIMUserManagerTest.java @@ -47,6 +47,7 @@ import org.wso2.carbon.identity.scim2.common.group.SCIMGroupHandler; import org.wso2.carbon.identity.scim2.common.internal.SCIMCommonComponentHolder; import org.wso2.carbon.user.core.UserStoreClientException; +import org.wso2.carbon.user.core.common.PaginatedUserResponse; import org.wso2.charon3.core.exceptions.NotImplementedException; import org.wso2.charon3.core.extensions.UserManager; import org.wso2.charon3.core.objects.plainobjects.UsersGetResponse; @@ -573,6 +574,9 @@ public void testFilteringUsersWithGET(List()); when(mockedUserStoreManager.getSecondaryUserStoreManager("PRIMARY")).thenReturn(mockedUserStoreManager); when(mockedUserStoreManager.isSCIMEnabled()).thenReturn(true); @@ -643,8 +647,8 @@ public Object[][] userInfoForFiltering() { @Test(dataProvider = "getDataForFilterUsersWithPagination") public void testFilteringUsersWithGETWithPagination(List users, String filter, - List filteredUsersWithPagination, - List filteredUsersWithoutPagination, + PaginatedUserResponse filteredUsersWithPagination, + PaginatedUserResponse filteredUsersWithoutPagination, boolean isConsiderTotalRecordsForTotalResultOfLDAPEnabled, boolean isConsiderMaxLimitForTotalResultEnabled, String domain, int count, int configuredMaxLimit, int expectedResultCount, @@ -666,23 +670,27 @@ public void testFilteringUsersWithGETWithPagination(List()); @@ -836,87 +844,87 @@ public Object[][] getDataForFilterUsersWithPagination() { // value of the 'totalResult'. {users, "name.givenName eq testUser", - new ArrayList() {{ + new PaginatedUserResponse(new ArrayList() {{ add(testUser1); add(testUser2); - }}, - new ArrayList() {{ + }}), + new PaginatedUserResponse(new ArrayList() {{ add(testUser1); add(testUser2); add(testUser3); - }}, + }}), true, false, "PRIMARY", 2, 4, 2, 3}, {users, "name.givenName eq testUser", - new ArrayList() {{ + new PaginatedUserResponse(new ArrayList() {{ add(testUser1); add(testUser2); - }}, - new ArrayList() {{ + }}), + new PaginatedUserResponse(new ArrayList() {{ add(testUser1); add(testUser2); add(testUser3); - }}, + }}), false, false, "PRIMARY", 2, 4, 2, 2}, {users, "name.givenName eq testUser", - new ArrayList() {{ + new PaginatedUserResponse(new ArrayList() {{ add(testUser1); add(testUser2); - }}, - new ArrayList() {{ + }}), + new PaginatedUserResponse(new ArrayList() {{ add(testUser1); add(testUser2); add(testUser3); - }}, + }}), true, false, "SECONDARY", 2, 4, 2, 3}, {users, "name.givenName eq testUser", - new ArrayList() {{ + new PaginatedUserResponse(new ArrayList() {{ add(testUser1); add(testUser2); - }}, - new ArrayList() {{ + }}), + new PaginatedUserResponse(new ArrayList() {{ add(testUser1); add(testUser2); add(testUser3); - }}, + }}), true, true, "SECONDARY", 2, 4, 2, 3}, {users, "name.givenName sw testUser and name.givenName co New", - new ArrayList() {{ + new PaginatedUserResponse(new ArrayList() {{ add(testUser4); - }}, - new ArrayList() {{ + }}), + new PaginatedUserResponse(new ArrayList() {{ add(testUser4); add(testUser5); - }}, + }}), true, false, "PRIMARY", 1, 4, 1, 2}, {users, "name.givenName sw testUser and name.givenName co New", - new ArrayList() {{ + new PaginatedUserResponse(new ArrayList() {{ add(testUser4); - }}, - new ArrayList() {{ + }}), + new PaginatedUserResponse(new ArrayList() {{ add(testUser4); add(testUser5); - }}, + }}), false, false, "PRIMARY", 1, 4, 1, 1}, {users, "name.givenName sw testUser and name.givenName co New", - new ArrayList() {{ + new PaginatedUserResponse(new ArrayList() {{ add(testUser4); - }}, - new ArrayList() {{ + }}), + new PaginatedUserResponse(new ArrayList() {{ add(testUser4); add(testUser5); - }}, + }}), true, false, "SECONDARY", 1, 4, 1, 2}, {users, "name.givenName sw testUser and name.givenName co New", - new ArrayList() {{ + new PaginatedUserResponse(new ArrayList() {{ add(testUser4); - }}, - new ArrayList() {{ + }}), + new PaginatedUserResponse(new ArrayList() {{ add(testUser4); add(testUser5); - }}, + }}), false, false, "SECONDARY", 1, 4, 1, 2}, }; diff --git a/pom.xml b/pom.xml index d1f85100..0a853d7b 100644 --- a/pom.xml +++ b/pom.xml @@ -278,7 +278,7 @@ 3.3.7 6.5.3 3.2.0.wso2v1 - 4.10.17 + 4.10.24 7.5.109 4.13.1 20030203.000129