Add review suggestions

wso2-extensions · Oct 24, 2024 · 706acfb · 706acfb
1 parent 36b13b1
commit 706acfb
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 16 deletions.
diff --git a/...y/organization/discovery/service/listener/OrganizationDiscoveryUserOperationListener.java b/...y/organization/discovery/service/listener/OrganizationDiscoveryUserOperationListener.java
@@ -81,7 +81,8 @@ public boolean doPreAddUserWithID(String userName, Object credential, String[] r
             if (!OrganizationManagementUtil.isOrganization(tenantDomain)) {
                 return true;
             }
-            if (claims.containsKey(CLAIM_MANAGED_ORGANIZATION)) {
+            if (claims != null && claims.containsKey(CLAIM_MANAGED_ORGANIZATION)
+                    && StringUtils.isNotBlank(claims.get(CLAIM_MANAGED_ORGANIZATION))) {
                 return true;
             }
             String organizationId = PrivilegedCarbonContext.getThreadLocalCarbonContext().getOrganizationId();

diff --git a/...ganization.discovery.service/listener/OrganizationDiscoveryUserOperationListenerTest.java b/...ganization.discovery.service/listener/OrganizationDiscoveryUserOperationListenerTest.java
@@ -18,15 +18,18 @@
 
 package org.wso2.carbon.identity.organization.discovery.service.listener;
 
+import org.apache.commons.lang3.StringUtils;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
-import org.testng.annotations.AfterMethod;
 import org.testng.annotations.BeforeMethod;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 import org.wso2.carbon.context.PrivilegedCarbonContext;
+import org.wso2.carbon.identity.organization.discovery.service.internal.OrganizationDiscoveryServiceHolder;
 import org.wso2.carbon.identity.organization.discovery.service.util.TestUtils;
 import org.wso2.carbon.identity.organization.management.service.OrganizationManager;
+import org.wso2.carbon.identity.organization.management.service.exception.OrganizationManagementException;
 import org.wso2.carbon.identity.organization.management.service.internal.OrganizationManagementDataHolder;
 import org.wso2.carbon.user.core.UserStoreException;
 import org.wso2.carbon.user.core.UserStoreManager;
@@ -38,10 +41,14 @@
 import java.util.Map;
 
 import static org.mockito.Mockito.when;
-import static org.testng.AssertJUnit.assertTrue;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertTrue;
 import static org.wso2.carbon.identity.organization.management.organization.user.sharing.constant.UserSharingConstants.CLAIM_MANAGED_ORGANIZATION;
 import static org.wso2.carbon.user.core.UserCoreConstants.DEFAULT_PROFILE;
 
+/**
+ * This class contains unit tests to verify the behavior of the OrganizationDiscoveryUserOperationListener class.
+ */
 public class OrganizationDiscoveryUserOperationListenerTest {
 
     private static final String TEST_ORG_ID = "10084a8d-113f-4211-a0d5-efe36b082211";
@@ -73,12 +80,11 @@ public void setUp() throws Exception {
 
         MockitoAnnotations.openMocks(this);
 
-        TestUtils.initiateH2Base();
         TestUtils.mockDataSource();
-
         PrivilegedCarbonContext.getThreadLocalCarbonContext().setTenantDomain(TEST_TENANT_DOMAIN);
         OrganizationManagementDataHolder.getInstance().setRealmService(realmService);
         OrganizationManagementDataHolder.getInstance().setOrganizationManager(organizationManager);
+        OrganizationDiscoveryServiceHolder.getInstance().setOrganizationManager(organizationManager);
 
         when(realmService.getTenantManager()).thenReturn(tenantManager);
         when(tenantManager.getTenantId(TEST_TENANT_DOMAIN)).thenReturn(TEST_TENANT_ID);
@@ -87,21 +93,31 @@ public void setUp() throws Exception {
         when(organizationManager.getOrganizationDepthInHierarchy(TEST_ORG_ID)).thenReturn(1);
     }
 
-    @Test
-    public void testSkipEmailDomainValidationForSharedUserCreation() throws UserStoreException {
+    @DataProvider(name = "skipEmailDomainValidationTestDataProvider")
+    public Object[][] claimsProvider() {
 
-        Map<String, String> claims = new HashMap<>();
-        claims.put(CLAIM_MANAGED_ORGANIZATION, TEST_ORG_ID);
+        return new Object[][]{
+                {null, false},
+                {new HashMap<String, String>(), false},
+                {new HashMap<String, String>() {{ put(CLAIM_MANAGED_ORGANIZATION, StringUtils.EMPTY); }}, false},
+                {new HashMap<String, String>() {{ put(CLAIM_MANAGED_ORGANIZATION, TEST_ORG_ID); }}, true}
+        };
+    }
 
-        boolean result = organizationDiscoveryUserOperationListener.doPreAddUserWithID(TEST_USER, TEST_CREDENTIALS,
-                new String[]{}, claims, DEFAULT_PROFILE, userStoreManager);
+    @Test(dataProvider = "skipEmailDomainValidationTestDataProvider")
+    public void testSkipEmailDomainValidationForSharedUserCreation(Map<String, String> claims, boolean expectedResult)
+            throws UserStoreException, OrganizationManagementException {
 
-        assertTrue("Email domain validation should be skipped for shared user creation.", result);
-    }
+        when(organizationManager.resolveOrganizationId(TEST_TENANT_DOMAIN))
+                .thenThrow(new OrganizationManagementException("Validation not skipped."));
 
-    @AfterMethod
-    public void tearDown() {
+        boolean result = organizationDiscoveryUserOperationListener.doPreAddUserWithID(TEST_USER, TEST_CREDENTIALS,
+                new String[]{}, claims, DEFAULT_PROFILE, userStoreManager);
 
-        PrivilegedCarbonContext.endTenantFlow();
+        if (expectedResult) {
+            assertTrue(result, "Email domain validation should be skipped for shared user creation.");
+        } else {
+            assertFalse(result, "Expected false when CLAIM_MANAGED_ORGANIZATION claim is missing.");
+        }
     }
 }
diff --git a/...nts/org.wso2.carbon.identity.organization.discovery.service/src/test/resources/testng.xml b/...nts/org.wso2.carbon.identity.organization.discovery.service/src/test/resources/testng.xml
@@ -21,6 +21,7 @@
 
     <test name="organization-discovery-tests" preserve-order="true" parallel="false">
         <classes>
+            <class name="org.wso2.carbon.identity.organization.discovery.service.listener.OrganizationDiscoveryUserOperationListenerTest"/>
             <class name="org.wso2.carbon.identity.organization.discovery.service.OrganizationDiscoveryManagerImplTest"/>
         </classes>
     </test>