From c78c3b21ba37eb3119185b8e08ca56e09e7c9edc Mon Sep 17 00:00:00 2001 From: ThaminduDilshan Date: Fri, 3 Jan 2025 11:59:17 +0530 Subject: [PATCH 1/3] Update the group last modified date on group update --- .../identity/scim2/common/DAO/GroupDAO.java | 54 ++++++++++++--- .../identity/scim2/common/DAO/SQLQueries.java | 5 ++ .../scim2/common/group/SCIMGroupHandler.java | 13 ++++ .../scim2/common/impl/SCIMUserManager.java | 65 +++++++++++++++---- 4 files changed, 117 insertions(+), 20 deletions(-) diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAO.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAO.java index a97c6dcb0..8086c8fc6 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAO.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAO.java @@ -289,16 +289,55 @@ public void addSCIMGroupAttributesToSCIMDisabledHybridRoles(int tenantId, } } + /** + * @deprecated Use {@link GroupDAO#updateSCIMGroupAttributesWithUpdatedColumn(int, String, Map)} instead. + * Update SCIM group attributes. + * + * @param tenantId Tenant id. + * @param roleName Group name. + * @param attributes Attributes to be updated. + * @throws IdentitySCIMException If an error occurred while updating the attributes. + */ + @Deprecated public void updateSCIMGroupAttributes(int tenantId, String roleName, Map attributes) throws IdentitySCIMException { - Connection connection = IdentityDatabaseUtil.getDBConnection(); + doUpdateSCIMGroupAttributes(tenantId, roleName, attributes, SQLQueries.UPDATE_ATTRIBUTES_SQL); + } + + /** + * Update SCIM group attributes. + * + * @param tenantId Tenant id. + * @param roleName Group name. + * @param attributes Attributes to be updated. + * @throws IdentitySCIMException If an error occurred while updating the attributes. + */ + public void updateSCIMGroupAttributesWithUpdatedColumn(int tenantId, String roleName, + Map attributes) + throws IdentitySCIMException { + + doUpdateSCIMGroupAttributes(tenantId, roleName, attributes, SQLQueries.UPDATE_ATTRIBUTES_SQL_NEW); + } + + /** + * Do update SCIM group attributes. + * + * @param tenantId Tenant id. + * @param roleName Group name. + * @param attributes Attributes to be updated. + * @param sqlQuery SQL query to update the attributes. + * @throws IdentitySCIMException If an error occurred while updating the attributes. + */ + private void doUpdateSCIMGroupAttributes(int tenantId, String roleName, Map attributes, + String sqlQuery) throws IdentitySCIMException { + + Connection connection = IdentityDatabaseUtil.getDBConnection(true); PreparedStatement prepStmt = null; if (isExistingGroup(SCIMCommonUtils.getGroupNameWithDomain(roleName), tenantId)) { try { - prepStmt = connection.prepareStatement(SQLQueries.UPDATE_ATTRIBUTES_SQL); - + prepStmt = connection.prepareStatement(sqlQuery); prepStmt.setInt(2, tenantId); prepStmt.setString(3, roleName); @@ -308,19 +347,16 @@ public void updateSCIMGroupAttributes(int tenantId, String roleName, prepStmt.setString(4, entry.getKey()); prepStmt.setString(1, entry.getValue()); prepStmt.addBatch(); - } else { throw new IdentitySCIMException("Error when adding SCIM Attribute: " - + entry.getKey() - + " An attribute with the same name doesn't exists."); + + entry.getKey() + " An attribute with the same name doesn't exists."); } } - int[] return_count = prepStmt.executeBatch(); + int[] returnCount = prepStmt.executeBatch(); if (log.isDebugEnabled()) { - log.debug("No. of records updated for updating SCIM Group : " + return_count.length); + log.debug("No. of records updated for updating SCIM Group : " + returnCount.length); } connection.commit(); - } catch (SQLException e) { throw new IdentitySCIMException("Error updating the SCIM Group Attributes.", e); } finally { diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/SQLQueries.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/SQLQueries.java index d63eb6d41..614ce1013 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/SQLQueries.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/SQLQueries.java @@ -41,10 +41,15 @@ public class SQLQueries { public static final String CHECK_EXISTING_ATTRIBUTE_WITH_AUDIENCE_SQL = "SELECT TENANT_ID, ROLE_NAME, ATTR_NAME FROM IDN_SCIM_GROUP WHERE IDN_SCIM_GROUP.TENANT_ID=? AND " + "IDN_SCIM_GROUP.ROLE_NAME=? AND IDN_SCIM_GROUP.ATTR_NAME=? AND IDN_SCIM_GROUP.AUDIENCE_REF_ID=?"; + + @Deprecated public static final String UPDATE_ATTRIBUTES_SQL = "UPDATE IDN_SCIM_GROUP SET UM_ATTR_VALUE=? WHERE TENANT_ID=? AND ROLE_NAME=? AND ATTR_NAME=?"; + public static final String UPDATE_ATTRIBUTES_SQL_NEW = + "UPDATE IDN_SCIM_GROUP SET ATTR_VALUE=? WHERE TENANT_ID=? AND ROLE_NAME=? AND ATTR_NAME=?"; public static final String UPDATE_GROUP_NAME_SQL = "UPDATE IDN_SCIM_GROUP SET ROLE_NAME=? WHERE TENANT_ID=? AND ROLE_NAME=?"; + public static final String DELETE_GROUP_SQL = "DELETE FROM IDN_SCIM_GROUP WHERE TENANT_ID=? AND ROLE_NAME=?"; public static final String CHECK_EXISTING_ATTRIBUTE_SQL = diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandler.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandler.java index ad316edb8..2fedb3052 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandler.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandler.java @@ -335,4 +335,17 @@ public String[] getGroupListFromAttributeName(String attributeName, String searc GroupDAO groupDAO = new GroupDAO(); return groupDAO.getGroupNameList(attributeName, searchAttribute, this.tenantId, domainName); } + + /** + * Update SCIM attributes of the group. + * + * @param groupName The display name of the group. + * @param attributes The attributes to be updated. + * @throws IdentitySCIMException IdentitySCIMException when updating the SCIM Group information. + */ + public void updateSCIMAttributes(String groupName, Map attributes) throws IdentitySCIMException { + + GroupDAO groupDAO = new GroupDAO(); + groupDAO.updateSCIMGroupAttributesWithUpdatedColumn(tenantId, groupName, attributes); + } } 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 33b7c806c..2c05a8eae 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 @@ -508,7 +508,7 @@ public User getUser(String userId, Map requiredAttributes) thro throw resolveError(e, errMsg); } } catch (BadRequestException | NotImplementedException e) { - throw new CharonException("Error in getting user information from Carbon User Store", e); + throw new CharonException("Error in getting user information from Carbon User Store", e); } return scimUser; } @@ -601,7 +601,7 @@ public void deleteUser(String userId) throws NotFoundException, CharonException, @Override @Deprecated public UsersGetResponse listUsersWithGET(Node rootNode, int startIndex, int count, String sortBy, String sortOrder, - String domainName, Map requiredAttributes) + String domainName, Map requiredAttributes) throws CharonException, NotImplementedException, BadRequestException { if (sortBy != null || sortOrder != null) { @@ -615,7 +615,7 @@ public UsersGetResponse listUsersWithGET(Node rootNode, int startIndex, int coun @Override public UsersGetResponse listUsersWithGET(Node rootNode, Integer startIndex, Integer count, String sortBy, - String sortOrder, String domainName, Map requiredAttributes) + String sortOrder, String domainName, Map requiredAttributes) throws CharonException, NotImplementedException, BadRequestException { // Validate NULL value for startIndex. @@ -689,7 +689,7 @@ private Resource getResourceByTenantId(int tenantId) throws org.wso2.carbon.user * @throws BadRequestException */ private UsersGetResponse listUsers(Map requiredAttributes, int offset, Integer limit, - String sortBy, String sortOrder, String domainName) throws CharonException, + String sortBy, String sortOrder, String domainName) throws CharonException, BadRequestException { List scimUsers = new ArrayList<>(); @@ -980,7 +980,7 @@ private Set listUsernamesAcrossAllDomains * @throws CharonException Error while retrieving users */ private List getUserDetails(Set coreUsers, - Map requiredAttributes) + Map requiredAttributes) throws CharonException, BadRequestException { List users = new ArrayList<>(); @@ -1410,7 +1410,7 @@ private int handleLimitEqualsNULL(Integer limit) { * @throws BadRequestException */ private UsersGetResponse filterUsers(Node node, Map requiredAttributes, int offset, Integer limit, - String sortBy, String sortOrder, String domainName) throws CharonException, + String sortBy, String sortOrder, String domainName) throws CharonException, BadRequestException { // Handle limit equals NULL scenario. @@ -3503,6 +3503,9 @@ private void doPatchGroup(String groupId, String currentGroupName, Map displayNameOperations = new ArrayList<>(); List memberOperations = new ArrayList<>(); String newGroupName = currentGroupName; + Date groupLastUpdatedTime = null; + String groupNameForIDNScim = SCIMCommonUtils.getGroupNameWithDomain(currentGroupName); + for (List patchOperationList : patchOperations.values()) { for (PatchOperation patchOperation : patchOperationList) { if (StringUtils.equals(SCIMConstants.GroupSchemaConstants.DISPLAY_NAME, @@ -3519,7 +3522,14 @@ private void doPatchGroup(String groupId, String currentGroupName, Map attributes = new HashMap<>(); + attributes.put(SCIMConstants.CommonSchemaConstants.LAST_MODIFIED_URI, + AttributeUtil.formatDateTime(groupLastUpdatedTime.toInstant())); + SCIMGroupHandler groupHandler = new SCIMGroupHandler(carbonUM.getTenantId()); + groupHandler.updateSCIMAttributes(groupNameForIDNScim, attributes); + } } catch (UserStoreException e) { if (e instanceof org.wso2.carbon.user.core.UserStoreException && (StringUtils .equals(UserCoreErrorConstants.ErrorMessages.ERROR_CODE_DUPLICATE_WHILE_WRITING_TO_DATABASE @@ -3676,12 +3696,10 @@ private void prepareAddedRemovedMemberLists(Set addedMembers, Set re public boolean doUpdateGroup(Group oldGroup, Group newGroup) throws CharonException, IdentitySCIMException, BadRequestException, IdentityApplicationManagementException, org.wso2.carbon.user.core.UserStoreException { + Date groupLastUpdatedTime = null; + String groupNameForIDNScim = SCIMCommonUtils.getGroupNameWithDomain(oldGroup.getDisplayName()); + setGroupDisplayName(oldGroup, newGroup); if (log.isDebugEnabled()) { log.debug("Updating group: " + oldGroup.getDisplayName()); @@ -3806,6 +3836,8 @@ public boolean doUpdateGroup(Group oldGroup, Group newGroup) throws CharonExcept // Update group name in carbon UM carbonUM.renameGroup(oldGroup.getId(), newGroupDisplayName); updated = true; + groupLastUpdatedTime = new Date(); + groupNameForIDNScim = SCIMCommonUtils.getGroupNameWithDomain(newGroupDisplayName); } // Update the group with added members and deleted members. if (isNotEmpty(addedMembers) || isNotEmpty(deletedMembers)) { @@ -3813,7 +3845,18 @@ public boolean doUpdateGroup(Group oldGroup, Group newGroup) throws CharonExcept deletedMemberIdsFromUserstore.toArray(new String[0]), addedMemberIdsFromUserstore.toArray(new String[0])); updated = true; + groupLastUpdatedTime = new Date(); + } + + // Update the last modified time of the group. + if (!carbonUM.isUniqueGroupIdEnabled() && groupLastUpdatedTime != null) { + Map attributes = new HashMap<>(); + attributes.put(SCIMConstants.CommonSchemaConstants.LAST_MODIFIED_URI, + AttributeUtil.formatDateTime(groupLastUpdatedTime.toInstant())); + SCIMGroupHandler groupHandler = new SCIMGroupHandler(carbonUM.getTenantId()); + groupHandler.updateSCIMAttributes(groupNameForIDNScim, attributes); } + return updated; } From 9b28e2ae9dbe77105e162355b9db6cf3ebbe657b Mon Sep 17 00:00:00 2001 From: ThaminduDilshan Date: Fri, 3 Jan 2025 15:05:39 +0530 Subject: [PATCH 2/3] Add unit tests --- .../scim2/common/impl/SCIMUserManager.java | 12 +- .../scim2/common/DAO/GroupDAOTest.java | 192 ++++++++++++++++ .../common/group/SCIMGroupHandlerTest.java | 13 ++ .../common/impl/SCIMUserManagerTest.java | 215 ++++++++++++++++++ .../src/test/resources/testng.xml | 1 + 5 files changed, 427 insertions(+), 6 deletions(-) create mode 100644 components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAOTest.java 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 2c05a8eae..6df869c48 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 @@ -508,7 +508,7 @@ public User getUser(String userId, Map requiredAttributes) thro throw resolveError(e, errMsg); } } catch (BadRequestException | NotImplementedException e) { - throw new CharonException("Error in getting user information from Carbon User Store", e); + throw new CharonException("Error in getting user information from Carbon User Store", e); } return scimUser; } @@ -601,7 +601,7 @@ public void deleteUser(String userId) throws NotFoundException, CharonException, @Override @Deprecated public UsersGetResponse listUsersWithGET(Node rootNode, int startIndex, int count, String sortBy, String sortOrder, - String domainName, Map requiredAttributes) + String domainName, Map requiredAttributes) throws CharonException, NotImplementedException, BadRequestException { if (sortBy != null || sortOrder != null) { @@ -615,7 +615,7 @@ public UsersGetResponse listUsersWithGET(Node rootNode, int startIndex, int coun @Override public UsersGetResponse listUsersWithGET(Node rootNode, Integer startIndex, Integer count, String sortBy, - String sortOrder, String domainName, Map requiredAttributes) + String sortOrder, String domainName, Map requiredAttributes) throws CharonException, NotImplementedException, BadRequestException { // Validate NULL value for startIndex. @@ -689,7 +689,7 @@ private Resource getResourceByTenantId(int tenantId) throws org.wso2.carbon.user * @throws BadRequestException */ private UsersGetResponse listUsers(Map requiredAttributes, int offset, Integer limit, - String sortBy, String sortOrder, String domainName) throws CharonException, + String sortBy, String sortOrder, String domainName) throws CharonException, BadRequestException { List scimUsers = new ArrayList<>(); @@ -980,7 +980,7 @@ private Set listUsernamesAcrossAllDomains * @throws CharonException Error while retrieving users */ private List getUserDetails(Set coreUsers, - Map requiredAttributes) + Map requiredAttributes) throws CharonException, BadRequestException { List users = new ArrayList<>(); @@ -1410,7 +1410,7 @@ private int handleLimitEqualsNULL(Integer limit) { * @throws BadRequestException */ private UsersGetResponse filterUsers(Node node, Map requiredAttributes, int offset, Integer limit, - String sortBy, String sortOrder, String domainName) throws CharonException, + String sortBy, String sortOrder, String domainName) throws CharonException, BadRequestException { // Handle limit equals NULL scenario. diff --git a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAOTest.java b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAOTest.java new file mode 100644 index 000000000..4c6e2a332 --- /dev/null +++ b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAOTest.java @@ -0,0 +1,192 @@ +/* + * Copyright (c) 2025, WSO2 LLC. (https://www.wso2.com). + * + * WSO2 LLC. 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. + */ + +package org.wso2.carbon.identity.scim2.common.DAO; + +import org.mockito.Mock; +import org.mockito.MockedStatic; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; +import org.wso2.carbon.identity.core.util.IdentityDatabaseUtil; +import org.wso2.carbon.identity.scim2.common.exceptions.IdentitySCIMException; +import org.wso2.carbon.identity.scim2.common.utils.SCIMCommonUtils; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLException; +import java.util.HashMap; +import java.util.Map; + +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.*; +import static org.mockito.MockitoAnnotations.initMocks; + +public class GroupDAOTest { + + @Mock + private Connection connection; + + @Mock + private PreparedStatement mockedPreparedStatement; + + @Mock + private ResultSet resultSet; + + private MockedStatic identityDatabaseUtil; + private MockedStatic scimCommonUtils; + + @BeforeMethod + public void setUp() { + + initMocks(this); + identityDatabaseUtil = mockStatic(IdentityDatabaseUtil.class); + scimCommonUtils = mockStatic(SCIMCommonUtils.class); + } + + @AfterMethod + public void tearDown() { + + identityDatabaseUtil.close(); + scimCommonUtils.close(); + } + + @DataProvider(name = "updateSCIMGroupAttributesDataProvider") + public Object[][] updateSCIMGroupAttributesDataProvider() { + + return new Object[][]{ + {true}, + {false} + }; + } + + @Test(dataProvider = "updateSCIMGroupAttributesDataProvider") + public void testUpdateSCIMGroupAttributes(boolean isUpdatedColumnMethod) throws Exception { + + Map attributes = new HashMap<>(); + attributes.put("urn:ietf:params:scim:schemas:core:2.0:meta.created", "2017-10-10T10:10:10Z"); + attributes.put("urn:ietf:params:scim:schemas:core:2.0:meta.lastModified", "2017-10-10T10:10:10Z"); + + when(IdentityDatabaseUtil.getDBConnection(true)).thenReturn(connection); + when(connection.prepareStatement(anyString())).thenReturn(mockedPreparedStatement); + when(mockedPreparedStatement.executeBatch()).thenReturn(new int[]{1}); + when(resultSet.next()).thenReturn(true); + when(SCIMCommonUtils.getGroupNameWithDomain(anyString())).thenReturn("PRIMARY/GROUP_NAME"); + + Connection mockedConnection2 = mock(Connection.class); + PreparedStatement mockedPreparedStatement2 = mock(PreparedStatement.class); + ResultSet mockedResultSet2 = mock(ResultSet.class); + when(IdentityDatabaseUtil.getDBConnection()).thenReturn(mockedConnection2); + when(mockedConnection2.prepareStatement(anyString())).thenReturn(mockedPreparedStatement2); + when(mockedPreparedStatement2.executeQuery()).thenReturn(mockedResultSet2); + when(mockedResultSet2.next()).thenReturn(true); + + GroupDAO groupDAO = spy(new GroupDAO()); + doReturn(true).when(groupDAO).isExistingGroup(anyString(), anyInt()); + + if (isUpdatedColumnMethod) { + groupDAO.updateSCIMGroupAttributesWithUpdatedColumn(1, "GROUP_NAME", attributes); + } else { + groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); + } + verify(mockedPreparedStatement, times(1)).executeBatch(); + } + + @Test(dataProvider = "updateSCIMGroupAttributesDataProvider", + expectedExceptions = IdentitySCIMException.class, + expectedExceptionsMessageRegExp = "Error when updating SCIM Attributes for the group: GROUP_NAME " + + "A Group with the same name doesn't exists.") + public void testUpdateSCIMGroupAttributesWithNonExistingGroup(boolean isUpdatedColumnMethod) throws Exception { + + Map attributes = new HashMap<>(); + attributes.put("urn:ietf:params:scim:schemas:core:2.0:meta.created", "2017-10-10T10:10:10Z"); + attributes.put("urn:ietf:params:scim:schemas:core:2.0:meta.lastModified", "2017-10-10T10:10:10Z"); + + when(IdentityDatabaseUtil.getDBConnection(true)).thenReturn(connection); + when(SCIMCommonUtils.getGroupNameWithDomain(anyString())).thenReturn("PRIMARY/GROUP_NAME"); + + GroupDAO groupDAO = spy(new GroupDAO()); + doReturn(false).when(groupDAO).isExistingGroup(anyString(), anyInt()); + + if (isUpdatedColumnMethod) { + groupDAO.updateSCIMGroupAttributesWithUpdatedColumn(1, "GROUP_NAME", attributes); + } else { + groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); + } + } + + @Test(dataProvider = "updateSCIMGroupAttributesDataProvider", + expectedExceptions = IdentitySCIMException.class, + expectedExceptionsMessageRegExp = "Error when adding SCIM Attribute: nonExisting " + + "An attribute with the same name doesn't exists.") + public void testUpdateSCIMGroupAttributesWithNonExistingAttributes(boolean isUpdatedColumnMethod) + throws Exception { + + Map attributes = new HashMap<>(); + attributes.put("nonExisting", "test-value"); + + when(IdentityDatabaseUtil.getDBConnection(true)).thenReturn(connection); + when(connection.prepareStatement(anyString())).thenReturn(mockedPreparedStatement); + when(mockedPreparedStatement.executeBatch()).thenReturn(new int[]{1}); + when(resultSet.next()).thenReturn(true); + when(SCIMCommonUtils.getGroupNameWithDomain(anyString())).thenReturn("PRIMARY/GROUP_NAME"); + + Connection mockedConnection2 = mock(Connection.class); + PreparedStatement mockedPreparedStatement2 = mock(PreparedStatement.class); + ResultSet mockedResultSet2 = mock(ResultSet.class); + when(IdentityDatabaseUtil.getDBConnection()).thenReturn(mockedConnection2); + when(mockedConnection2.prepareStatement(anyString())).thenReturn(mockedPreparedStatement2); + when(mockedPreparedStatement2.executeQuery()).thenReturn(mockedResultSet2); + when(mockedResultSet2.next()).thenReturn(false); + + GroupDAO groupDAO = spy(new GroupDAO()); + doReturn(true).when(groupDAO).isExistingGroup(anyString(), anyInt()); + + if (isUpdatedColumnMethod) { + groupDAO.updateSCIMGroupAttributesWithUpdatedColumn(1, "GROUP_NAME", attributes); + } else { + groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); + } + } + + @Test(dataProvider = "updateSCIMGroupAttributesDataProvider", + expectedExceptions = IdentitySCIMException.class, + expectedExceptionsMessageRegExp = "Error updating the SCIM Group Attributes.") + public void testUpdateSCIMGroupAttributesWithSQLException(boolean isUpdatedColumnMethod) throws Exception { + + Map attributes = new HashMap<>(); + attributes.put("urn:ietf:params:scim:schemas:core:2.0:meta.created", "2017-10-10T10:10:10Z"); + attributes.put("urn:ietf:params:scim:schemas:core:2.0:meta.lastModified", "2017-10-10T10:10:10Z"); + + when(IdentityDatabaseUtil.getDBConnection(true)).thenReturn(connection); + when(connection.prepareStatement(anyString())).thenThrow(new SQLException()); + when(SCIMCommonUtils.getGroupNameWithDomain(anyString())).thenReturn("PRIMARY/GROUP_NAME"); + + GroupDAO groupDAO = spy(new GroupDAO()); + doReturn(true).when(groupDAO).isExistingGroup(anyString(), anyInt()); + + if (isUpdatedColumnMethod) { + groupDAO.updateSCIMGroupAttributesWithUpdatedColumn(1, "GROUP_NAME", attributes); + } else { + groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); + } + } +} diff --git a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandlerTest.java b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandlerTest.java index ec8cabee0..f17bbd2a8 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandlerTest.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandlerTest.java @@ -337,4 +337,17 @@ public void testListSCIMRoles() throws Exception { assertNotNull(new SCIMGroupHandler(1).listSCIMRoles()); } + @Test + public void testUpdateSCIMAttributes() throws IdentitySCIMException { + + Map attributes = new HashMap(); + attributes.put("urn:ietf:params:scim:schemas:core:2.0:meta.created", "2017-10-10T10:10:10Z"); + attributes.put("urn:ietf:params:scim:schemas:core:2.0:meta.lastModified", "2017-10-10T10:10:10Z"); + + try (MockedConstruction mockedConstruction = Mockito.mockConstruction(GroupDAO.class)) { + new SCIMGroupHandler(1).updateSCIMAttributes("GROUP_NAME", attributes); + verify(mockedConstruction.constructed().get(0)) + .updateSCIMGroupAttributesWithUpdatedColumn(1, "GROUP_NAME", attributes); + } + } } 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 5c6cd4c45..a6b0a8746 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 @@ -31,6 +31,7 @@ import org.testng.annotations.Test; import org.wso2.carbon.CarbonConstants; import org.wso2.carbon.base.MultitenantConstants; +import org.wso2.carbon.identity.application.common.IdentityApplicationManagementException; import org.wso2.carbon.identity.application.common.model.InboundProvisioningConfig; import org.wso2.carbon.identity.application.common.model.ServiceProvider; import org.wso2.carbon.identity.application.mgt.ApplicationManagementService; @@ -91,6 +92,7 @@ import org.wso2.charon3.core.utils.codeutils.ExpressionNode; import org.wso2.charon3.core.utils.codeutils.FilterTreeManager; import org.wso2.charon3.core.utils.codeutils.Node; +import org.wso2.charon3.core.utils.codeutils.PatchOperation; import org.wso2.charon3.core.utils.codeutils.SearchRequest; import org.wso2.carbon.identity.configuration.mgt.core.model.Resource; @@ -119,6 +121,8 @@ import static org.mockito.Mockito.mockConstruction; import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; import static org.testng.Assert.assertEquals; @@ -1694,6 +1698,217 @@ public void testCreateUserWithConflictingLoginIdentifier() throws Exception { // This method is for testing of throwing BadRequestException, hence no assertion. } + @DataProvider(name = "patchGroupDataProvider") + public Object[][] patchGroupDataProvider() { + + String currentGroupName = "CurrentGroupName"; + String updatedGroupName = "UpdatedGroupName"; + String userName = "user1"; + String userId = "1234"; + + Map> patchOperations1 = new HashMap<>(); + Map> patchOperations2 = new HashMap<>(); + Map> patchOperations3 = new HashMap<>(); + Map> patchOperations4 = new HashMap<>(); + + PatchOperation patchOperation1 = new PatchOperation(); + patchOperation1.setOperation("add"); + patchOperation1.setValues(updatedGroupName); + patchOperation1.setAttributeName("displayName"); + patchOperation1.setExecutionOrder(1); + patchOperations1.put("add", new ArrayList() {{ + add(patchOperation1); + }}); + patchOperations3.put("add", new ArrayList() {{ + add(patchOperation1); + }}); + + // Update with the same display name. + PatchOperation patchOperation2 = new PatchOperation(); + patchOperation2.setOperation("add"); + patchOperation2.setValues(currentGroupName); + patchOperation2.setAttributeName("displayName"); + patchOperation2.setExecutionOrder(1); + patchOperations2.put("add", new ArrayList() {{ + add(patchOperation2); + }}); + + PatchOperation patchOperation3 = new PatchOperation(); + patchOperation3.setOperation("remove"); + HashMap groupUsers = new HashMap<>(); + groupUsers.put("display", userName); + groupUsers.put("value", userId); + patchOperation3.setValues(groupUsers); + patchOperation3.setAttributeName("members"); + patchOperation3.setExecutionOrder(2); + patchOperation3.setPath("members[display eq \"" + userName + "\"]"); + patchOperations1.put("remove", new ArrayList() {{ + add(patchOperation3); + }}); + patchOperations2.put("remove", new ArrayList() {{ + add(patchOperation3); + }}); + patchOperations4.put("remove", new ArrayList() {{ + add(patchOperation3); + }}); + + return new Object[][]{ + {true, currentGroupName, updatedGroupName, patchOperations1}, + {false, currentGroupName, updatedGroupName, patchOperations1}, + {false, currentGroupName, currentGroupName, patchOperations2}, + {false, currentGroupName, updatedGroupName, patchOperations3}, + {false, currentGroupName, currentGroupName, patchOperations4}, + {true, currentGroupName, currentGroupName, new HashMap<>()}, + {false, currentGroupName, currentGroupName, new HashMap<>()}, + }; + } + + @Test(dataProvider = "patchGroupDataProvider") + public void testPatchGroup(boolean isUniqueGroupIdEnabled, String currentGroupName, String updatedGroupName, + Map> patchOperations) + throws NotImplementedException, BadRequestException, NotFoundException, CharonException, + IdentityApplicationManagementException, UserStoreException { + + String currentGroupNameWithDomain = "PRIMARY/" + currentGroupName; + String updatedGroupNameWithDomain = "PRIMARY/" + updatedGroupName; + + scimCommonUtils.when(() -> SCIMCommonUtils.getGroupNameWithDomain(currentGroupName)) + .thenReturn(currentGroupNameWithDomain); + scimCommonUtils.when(() -> SCIMCommonUtils.getGroupNameWithDomain(updatedGroupName)) + .thenReturn(updatedGroupNameWithDomain); + when(IdentityUtil.extractDomainFromName(anyString())).thenReturn("PRIMARY"); + + InboundProvisioningConfig inboundProvisioningConfig = new InboundProvisioningConfig(); + inboundProvisioningConfig.setProvisioningUserStore("PRIMARY"); + ServiceProvider serviceProvider = new ServiceProvider(); + serviceProvider.setInboundProvisioningConfig(inboundProvisioningConfig); + when(ApplicationManagementService.getInstance()).thenReturn(applicationManagementService); + when(applicationManagementService.getServiceProvider(anyString(), anyString())).thenReturn(serviceProvider); + + mockedUserStoreManager = mock(AbstractUserStoreManager.class); + when(mockedUserStoreManager.getSecondaryUserStoreManager(anyString())).thenReturn(secondaryUserStoreManager); + when(mockedUserStoreManager.isUniqueGroupIdEnabled()).thenReturn(isUniqueGroupIdEnabled); + when(secondaryUserStoreManager.getUserIDFromProperties(USERNAME_LOCAL_CLAIM, "user1", "default")) + .thenReturn("1234"); + + try (MockedConstruction mockedGroupHandler = mockConstruction(SCIMGroupHandler.class)) { + SCIMUserManager scimUserManager = new SCIMUserManager(mockedUserStoreManager, + mockClaimMetadataManagementService, MultitenantConstants.SUPER_TENANT_DOMAIN_NAME); + scimUserManager.patchGroup("123456", currentGroupName, patchOperations); + + // Assert if mocks are called as expected. + if (currentGroupName.equals(updatedGroupName) || !patchOperations.containsKey("add")) { + verify(mockedUserStoreManager, times(0)).renameGroup("123456", updatedGroupNameWithDomain); + } else { + verify(mockedUserStoreManager, times(1)).renameGroup("123456", updatedGroupNameWithDomain); + } + if (patchOperations.containsKey("remove")) { + verify(mockedUserStoreManager, times(1)).updateUserListOfRoleWithID(anyString(), any(), any()); + } + verify(mockedUserStoreManager, times(1)).updateGroupName(currentGroupName, updatedGroupName); + + if (isUniqueGroupIdEnabled || patchOperations.isEmpty()) { + assertEquals(mockedGroupHandler.constructed().size(), 0); + } else { + assertEquals(mockedGroupHandler.constructed().size(), 1); + } + } + } + + @DataProvider(name = "updateGroupDataProvider") + public Object[][] updateGroupDataProvider() throws BadRequestException, CharonException { + + String currentGroupName = "CurrentGroupName"; + String updatedGroupName = "UpdatedGroupName"; + + Group oldGroup = new Group(); + oldGroup.setDisplayName(currentGroupName); + oldGroup.setId("123456"); + oldGroup.setMember("1234", "user1", "https://wso2/scim2/Users/1234", "default"); + + Group newGroup1 = new Group(); + newGroup1.setDisplayName(updatedGroupName); + newGroup1.setId("123456"); + newGroup1.setMember("1234", "user1", "https://wso2/scim2/Users/1234", "default"); + + Group newGroup2 = new Group(); + newGroup2.setDisplayName(currentGroupName); + newGroup2.setId("123456"); + newGroup2.setMember("5678", "user2", "https://wso2/scim2/Users/5678", "default"); + + Group newGroup3 = new Group(); + newGroup3.setDisplayName(updatedGroupName); + newGroup3.setId("123456"); + newGroup3.setMember("5678", "user2", "https://wso2/scim2/Users/5678", "default"); + + Group newGroup4 = new Group(); + newGroup4.setDisplayName(currentGroupName); + newGroup4.setId("123456"); + newGroup4.setMember("1234", "user1", "https://wso2/scim2/Users/1234", "default"); + + return new Object[][]{ + {true, oldGroup, newGroup1}, + {false, oldGroup, newGroup1}, + {false, oldGroup, newGroup2}, + {false, oldGroup, newGroup3}, + {true, oldGroup, newGroup4}, + {false, oldGroup, newGroup4}, + }; + } + + @Test(dataProvider = "updateGroupDataProvider") + public void testUpdateGroup(boolean isUniqueGroupIdEnabled, Group oldGroup, Group newGroup) + throws CharonException, BadRequestException, IdentityApplicationManagementException, UserStoreException { + + String currentGroupName = oldGroup.getDisplayName(); + String updatedGroupName = newGroup.getDisplayName(); + String currentGroupNameWithDomain = "PRIMARY/" + currentGroupName; + String updatedGroupNameWithDomain = "PRIMARY/" + updatedGroupName; + + scimCommonUtils.when(() -> SCIMCommonUtils.getGroupNameWithDomain(currentGroupName)) + .thenReturn(currentGroupNameWithDomain); + scimCommonUtils.when(() -> SCIMCommonUtils.getGroupNameWithDomain(updatedGroupName)) + .thenReturn(updatedGroupNameWithDomain); + when(IdentityUtil.extractDomainFromName(anyString())).thenReturn("PRIMARY"); + when(IdentityUtil.addDomainToName(anyString(), anyString())).thenCallRealMethod(); + + InboundProvisioningConfig inboundProvisioningConfig = new InboundProvisioningConfig(); + inboundProvisioningConfig.setProvisioningUserStore("PRIMARY"); + ServiceProvider serviceProvider = new ServiceProvider(); + serviceProvider.setInboundProvisioningConfig(inboundProvisioningConfig); + when(ApplicationManagementService.getInstance()).thenReturn(applicationManagementService); + when(applicationManagementService.getServiceProvider(anyString(), anyString())).thenReturn(serviceProvider); + + mockedUserStoreManager = mock(AbstractUserStoreManager.class); + when(mockedUserStoreManager.getSecondaryUserStoreManager(anyString())).thenReturn(secondaryUserStoreManager); + when(mockedUserStoreManager.isUniqueGroupIdEnabled()).thenReturn(isUniqueGroupIdEnabled); + when(secondaryUserStoreManager.getUserIDFromProperties(USERNAME_LOCAL_CLAIM, "user1", "default")) + .thenReturn("1234"); + when(secondaryUserStoreManager.getUserIDFromProperties(USERNAME_LOCAL_CLAIM, "user2", "default")) + .thenReturn("5678"); + + try (MockedConstruction mockedGroupHandler = mockConstruction(SCIMGroupHandler.class)) { + SCIMUserManager scimUserManager = new SCIMUserManager(mockedUserStoreManager, + mockClaimMetadataManagementService, MultitenantConstants.SUPER_TENANT_DOMAIN_NAME); + scimUserManager.updateGroup(oldGroup, newGroup); + + // Assert if mocks are called as expected. + if (currentGroupName.equals(updatedGroupName)) { + verify(mockedUserStoreManager, times(0)).renameGroup( + "123456", updatedGroupNameWithDomain); + } else { + verify(mockedUserStoreManager, times(1)).renameGroup( + "123456", updatedGroupNameWithDomain); + } + + if (isUniqueGroupIdEnabled || oldGroup.equals(newGroup)) { + assertEquals(mockedGroupHandler.constructed().size(), 0); + } else { + assertEquals(mockedGroupHandler.constructed().size(), 1); + } + } + } + /** * Build a group object with the given params to mock the userstore response. * diff --git a/components/org.wso2.carbon.identity.scim2.common/src/test/resources/testng.xml b/components/org.wso2.carbon.identity.scim2.common/src/test/resources/testng.xml index e9b0cdcfd..7a75706d3 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/test/resources/testng.xml +++ b/components/org.wso2.carbon.identity.scim2.common/src/test/resources/testng.xml @@ -35,6 +35,7 @@ + From 71f1dbee55ed794a69b0e3a7d6f5b0b428489ca2 Mon Sep 17 00:00:00 2001 From: ThaminduDilshan Date: Tue, 21 Jan 2025 16:20:42 +0530 Subject: [PATCH 3/3] Address review comments --- .../identity/scim2/common/DAO/GroupDAO.java | 17 ------ .../identity/scim2/common/DAO/SQLQueries.java | 3 - .../scim2/common/group/SCIMGroupHandler.java | 2 +- .../scim2/common/DAO/GroupDAOTest.java | 57 ++++--------------- .../common/group/SCIMGroupHandlerTest.java | 2 +- 5 files changed, 14 insertions(+), 67 deletions(-) diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAO.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAO.java index 8086c8fc6..4f78b3d03 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAO.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAO.java @@ -290,7 +290,6 @@ public void addSCIMGroupAttributesToSCIMDisabledHybridRoles(int tenantId, } /** - * @deprecated Use {@link GroupDAO#updateSCIMGroupAttributesWithUpdatedColumn(int, String, Map)} instead. * Update SCIM group attributes. * * @param tenantId Tenant id. @@ -298,28 +297,12 @@ public void addSCIMGroupAttributesToSCIMDisabledHybridRoles(int tenantId, * @param attributes Attributes to be updated. * @throws IdentitySCIMException If an error occurred while updating the attributes. */ - @Deprecated public void updateSCIMGroupAttributes(int tenantId, String roleName, Map attributes) throws IdentitySCIMException { doUpdateSCIMGroupAttributes(tenantId, roleName, attributes, SQLQueries.UPDATE_ATTRIBUTES_SQL); } - /** - * Update SCIM group attributes. - * - * @param tenantId Tenant id. - * @param roleName Group name. - * @param attributes Attributes to be updated. - * @throws IdentitySCIMException If an error occurred while updating the attributes. - */ - public void updateSCIMGroupAttributesWithUpdatedColumn(int tenantId, String roleName, - Map attributes) - throws IdentitySCIMException { - - doUpdateSCIMGroupAttributes(tenantId, roleName, attributes, SQLQueries.UPDATE_ATTRIBUTES_SQL_NEW); - } - /** * Do update SCIM group attributes. * diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/SQLQueries.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/SQLQueries.java index 614ce1013..4f42ee067 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/SQLQueries.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/DAO/SQLQueries.java @@ -42,10 +42,7 @@ public class SQLQueries { "SELECT TENANT_ID, ROLE_NAME, ATTR_NAME FROM IDN_SCIM_GROUP WHERE IDN_SCIM_GROUP.TENANT_ID=? AND " + "IDN_SCIM_GROUP.ROLE_NAME=? AND IDN_SCIM_GROUP.ATTR_NAME=? AND IDN_SCIM_GROUP.AUDIENCE_REF_ID=?"; - @Deprecated public static final String UPDATE_ATTRIBUTES_SQL = - "UPDATE IDN_SCIM_GROUP SET UM_ATTR_VALUE=? WHERE TENANT_ID=? AND ROLE_NAME=? AND ATTR_NAME=?"; - public static final String UPDATE_ATTRIBUTES_SQL_NEW = "UPDATE IDN_SCIM_GROUP SET ATTR_VALUE=? WHERE TENANT_ID=? AND ROLE_NAME=? AND ATTR_NAME=?"; public static final String UPDATE_GROUP_NAME_SQL = "UPDATE IDN_SCIM_GROUP SET ROLE_NAME=? WHERE TENANT_ID=? AND ROLE_NAME=?"; diff --git a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandler.java b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandler.java index 2fedb3052..731d83805 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandler.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/main/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandler.java @@ -346,6 +346,6 @@ public String[] getGroupListFromAttributeName(String attributeName, String searc public void updateSCIMAttributes(String groupName, Map attributes) throws IdentitySCIMException { GroupDAO groupDAO = new GroupDAO(); - groupDAO.updateSCIMGroupAttributesWithUpdatedColumn(tenantId, groupName, attributes); + groupDAO.updateSCIMGroupAttributes(tenantId, groupName, attributes); } } diff --git a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAOTest.java b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAOTest.java index 4c6e2a332..615790751 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAOTest.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/DAO/GroupDAOTest.java @@ -22,7 +22,6 @@ import org.mockito.MockedStatic; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; -import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import org.wso2.carbon.identity.core.util.IdentityDatabaseUtil; import org.wso2.carbon.identity.scim2.common.exceptions.IdentitySCIMException; @@ -69,17 +68,8 @@ public void tearDown() { scimCommonUtils.close(); } - @DataProvider(name = "updateSCIMGroupAttributesDataProvider") - public Object[][] updateSCIMGroupAttributesDataProvider() { - - return new Object[][]{ - {true}, - {false} - }; - } - - @Test(dataProvider = "updateSCIMGroupAttributesDataProvider") - public void testUpdateSCIMGroupAttributes(boolean isUpdatedColumnMethod) throws Exception { + @Test + public void testUpdateSCIMGroupAttributes() throws Exception { Map attributes = new HashMap<>(); attributes.put("urn:ietf:params:scim:schemas:core:2.0:meta.created", "2017-10-10T10:10:10Z"); @@ -102,19 +92,14 @@ public void testUpdateSCIMGroupAttributes(boolean isUpdatedColumnMethod) throws GroupDAO groupDAO = spy(new GroupDAO()); doReturn(true).when(groupDAO).isExistingGroup(anyString(), anyInt()); - if (isUpdatedColumnMethod) { - groupDAO.updateSCIMGroupAttributesWithUpdatedColumn(1, "GROUP_NAME", attributes); - } else { - groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); - } + groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); verify(mockedPreparedStatement, times(1)).executeBatch(); } - @Test(dataProvider = "updateSCIMGroupAttributesDataProvider", - expectedExceptions = IdentitySCIMException.class, + @Test(expectedExceptions = IdentitySCIMException.class, expectedExceptionsMessageRegExp = "Error when updating SCIM Attributes for the group: GROUP_NAME " + "A Group with the same name doesn't exists.") - public void testUpdateSCIMGroupAttributesWithNonExistingGroup(boolean isUpdatedColumnMethod) throws Exception { + public void testUpdateSCIMGroupAttributesWithNonExistingGroup() throws Exception { Map attributes = new HashMap<>(); attributes.put("urn:ietf:params:scim:schemas:core:2.0:meta.created", "2017-10-10T10:10:10Z"); @@ -125,20 +110,13 @@ public void testUpdateSCIMGroupAttributesWithNonExistingGroup(boolean isUpdatedC GroupDAO groupDAO = spy(new GroupDAO()); doReturn(false).when(groupDAO).isExistingGroup(anyString(), anyInt()); - - if (isUpdatedColumnMethod) { - groupDAO.updateSCIMGroupAttributesWithUpdatedColumn(1, "GROUP_NAME", attributes); - } else { - groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); - } + groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); } - @Test(dataProvider = "updateSCIMGroupAttributesDataProvider", - expectedExceptions = IdentitySCIMException.class, + @Test(expectedExceptions = IdentitySCIMException.class, expectedExceptionsMessageRegExp = "Error when adding SCIM Attribute: nonExisting " + "An attribute with the same name doesn't exists.") - public void testUpdateSCIMGroupAttributesWithNonExistingAttributes(boolean isUpdatedColumnMethod) - throws Exception { + public void testUpdateSCIMGroupAttributesWithNonExistingAttributes() throws Exception { Map attributes = new HashMap<>(); attributes.put("nonExisting", "test-value"); @@ -159,18 +137,12 @@ public void testUpdateSCIMGroupAttributesWithNonExistingAttributes(boolean isUpd GroupDAO groupDAO = spy(new GroupDAO()); doReturn(true).when(groupDAO).isExistingGroup(anyString(), anyInt()); - - if (isUpdatedColumnMethod) { - groupDAO.updateSCIMGroupAttributesWithUpdatedColumn(1, "GROUP_NAME", attributes); - } else { - groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); - } + groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); } - @Test(dataProvider = "updateSCIMGroupAttributesDataProvider", - expectedExceptions = IdentitySCIMException.class, + @Test(expectedExceptions = IdentitySCIMException.class, expectedExceptionsMessageRegExp = "Error updating the SCIM Group Attributes.") - public void testUpdateSCIMGroupAttributesWithSQLException(boolean isUpdatedColumnMethod) throws Exception { + public void testUpdateSCIMGroupAttributesWithSQLException() throws Exception { Map attributes = new HashMap<>(); attributes.put("urn:ietf:params:scim:schemas:core:2.0:meta.created", "2017-10-10T10:10:10Z"); @@ -182,11 +154,6 @@ public void testUpdateSCIMGroupAttributesWithSQLException(boolean isUpdatedColum GroupDAO groupDAO = spy(new GroupDAO()); doReturn(true).when(groupDAO).isExistingGroup(anyString(), anyInt()); - - if (isUpdatedColumnMethod) { - groupDAO.updateSCIMGroupAttributesWithUpdatedColumn(1, "GROUP_NAME", attributes); - } else { - groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); - } + groupDAO.updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); } } diff --git a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandlerTest.java b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandlerTest.java index f17bbd2a8..ea0c86000 100644 --- a/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandlerTest.java +++ b/components/org.wso2.carbon.identity.scim2.common/src/test/java/org/wso2/carbon/identity/scim2/common/group/SCIMGroupHandlerTest.java @@ -347,7 +347,7 @@ public void testUpdateSCIMAttributes() throws IdentitySCIMException { try (MockedConstruction mockedConstruction = Mockito.mockConstruction(GroupDAO.class)) { new SCIMGroupHandler(1).updateSCIMAttributes("GROUP_NAME", attributes); verify(mockedConstruction.constructed().get(0)) - .updateSCIMGroupAttributesWithUpdatedColumn(1, "GROUP_NAME", attributes); + .updateSCIMGroupAttributes(1, "GROUP_NAME", attributes); } } }