[IS]Remove b2b org count. #13
Conversation
WalkthroughThe pull request removes B2B organization counting functionality from the identity usage data collector component. Changes include deleting the OrganizationCounter class, refactoring UserCounter to operate independently of OrganizationManager, removing the organizationManager dependency throughout the stack, and eliminating B2B organization-related fields from data models. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (8)
💤 Files with no reviewable changes (5)
🧰 Additional context used🧠 Learnings (4)📚 Learning: 2025-12-07T18:41:22.612ZApplied to files:
📚 Learning: 2025-11-21T04:35:45.575ZApplied to files:
📚 Learning: 2025-12-19T11:41:50.935ZApplied to files:
📚 Learning: 2025-12-03T03:41:25.406ZApplied to files:
🔇 Additional comments (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
DO NOT MERGE we need to create different branch for this |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/UsageDataCollector.java (1)
47-47: Unused constantTOTAL_B2B_ORGSshould be removed.This constant is no longer referenced after removing B2B org metric publishing from
publishUsageMetrics. It should be removed as part of this cleanup.🔎 Proposed fix
public static final String SUPER_TENANT = "carbon.super"; public static final String TOTAL_USERS = "TOTAL_USERS"; - public static final String TOTAL_B2B_ORGS = "TOTAL_B2B_ORGS"; public static final String TOTAL_ROOT_ORGS = "TOTAL_ROOT_ORGS";collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/counter/UserCounter.java (2)
44-48: Remove unused constants.
LDAP_PAGE_SIZEandMAX_LDAP_ITERATIONSare not used anywhere after LDAP counting logic was removed. The remaining constants (SLEEP_BETWEEN_REQUESTS_MS,SLEEP_AFTER_MAX_REQUESTS_MS,MAX_REQUESTS_PER_MINUTE) are only used byapplyRateLimiting, which itself appears to be dead code (see below).🔎 Proposed fix
public class UserCounter { private static final Log LOG = LogFactory.getLog(UserCounter.class); - // Configuration - private static final int LDAP_PAGE_SIZE = 100; - private static final int MAX_LDAP_ITERATIONS = 1000; - private static final long SLEEP_BETWEEN_REQUESTS_MS = 100; // 100ms - private static final long SLEEP_AFTER_MAX_REQUESTS_MS = 5_000; // 5 seconds - private static final int MAX_REQUESTS_PER_MINUTE = 2; - private final RealmService realmService;
162-185: Remove unusedapplyRateLimitingmethod and associated constants.The
applyRateLimitingmethod is never invoked anywhere in the codebase. Additionally, the constantsSLEEP_BETWEEN_REQUESTS_MS,SLEEP_AFTER_MAX_REQUESTS_MS, andMAX_REQUESTS_PER_MINUTEare only referenced within this dead method and should be removed along with it.🔎 Proposed fix
private boolean isJDBCUserStore(UserStoreManager userStoreManager, String domain) { try { AbstractUserStoreManager abstractUSM = (AbstractUserStoreManager) userStoreManager.getSecondaryUserStoreManager(domain); return abstractUSM instanceof JDBCUserStoreManager; } catch (Exception e) { return false; } } - - /** - * Apply rate limiting to protect database - * - Sleeps 100ms between every request - * - Sleeps 6 seconds after every 10 requests - */ - private void applyRateLimiting(int iteration) throws InterruptedException { - - // Always sleep a bit between requests - Thread.sleep(SLEEP_BETWEEN_REQUESTS_MS); - - // After take a longer break after certain request count. - if (iteration % MAX_REQUESTS_PER_MINUTE == 0) { - LOG.debug(String.format("Rate limit checkpoint reached (%d requests). Sleeping for %dms", - iteration, SLEEP_AFTER_MAX_REQUESTS_MS)); - Thread.sleep(SLEEP_AFTER_MAX_REQUESTS_MS); - LOG.debug("Resumed after rate limit sleep"); - } - - // After 5000 users enforce another sleep. - if (iteration % 50 == 0) { - Thread.sleep(SLEEP_AFTER_MAX_REQUESTS_MS); - LOG.debug(String.format("Progress: %d iterations completed", iteration)); - } - } }Also remove the unused constants at lines 46-48:
- private static final long SLEEP_BETWEEN_REQUESTS_MS = 100; // 100ms - private static final long SLEEP_AFTER_MAX_REQUESTS_MS = 5_000; // 5 seconds - private static final int MAX_REQUESTS_PER_MINUTE = 2;
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
collectors/org.wso2.carbon.usage.data.collector.identity/pom.xmlcollectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/UsageDataCollector.javacollectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/counter/OrganizationCounter.javacollectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/counter/UserCounter.javacollectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/internal/UsageDataCollectorDataHolder.javacollectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/internal/UsageDataCollectorServiceComponent.javacollectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/model/SystemUsage.javacollectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/model/TenantUsage.java
💤 Files with no reviewable changes (5)
- collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/internal/UsageDataCollectorDataHolder.java
- collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/internal/UsageDataCollectorServiceComponent.java
- collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/model/SystemUsage.java
- collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/counter/OrganizationCounter.java
- collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/model/TenantUsage.java
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-12-07T18:41:22.612Z
Learnt from: ashanhr
Repo: wso2/usage-data-collector PR: 8
File: collectors/org.wso2.carbon.usage.data.collector.common/src/main/java/org/wso2/carbon/usage/data/collector/common/internal/UsageDataCollectorServiceComponent.java:93-108
Timestamp: 2025-12-07T18:41:22.612Z
Learning: In the wso2/usage-data-collector repository, for the common collector module (org.wso2.carbon.usage.data.collector.common), errors in UsageDataCollectorServiceComponent should be silenced (gated behind debug flags) to avoid exposing internal issues in customer-facing scenarios, particularly for non-fatal errors like meta information publishing failures at startup.
Applied to files:
collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/UsageDataCollector.javacollectors/org.wso2.carbon.usage.data.collector.identity/pom.xml
📚 Learning: 2025-11-21T04:35:45.575Z
Learnt from: inthirakumaaran
Repo: wso2/usage-data-collector PR: 4
File: collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/counter/UserCounter.java:170-227
Timestamp: 2025-11-21T04:35:45.575Z
Learning: In the WSO2 usage data collector (collectors/org.wso2.carbon.usage.data.collector.identity), errors should not be logged at ERROR level in production for customer-facing scenarios. Error logging should be gated behind debug flags to avoid exposing internal issues to customers unless debug logging is explicitly enabled.
Applied to files:
collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/UsageDataCollector.java
📚 Learning: 2025-12-19T11:41:50.935Z
Learnt from: inthirakumaaran
Repo: wso2/usage-data-collector PR: 12
File: collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/UsageDataCollectorScheduler.java:43-43
Timestamp: 2025-12-19T11:41:50.935Z
Learning: In the wso2/usage-data-collector repository, for the identity collector module (org.wso2.carbon.usage.data.collector.identity), the default scheduled day for UsageDataCollectorScheduler is WEDNESDAY (DayOfWeek.WEDNESDAY), not SUNDAY, as per the latest decision.
Applied to files:
collectors/org.wso2.carbon.usage.data.collector.identity/pom.xml
📚 Learning: 2025-12-03T03:41:25.406Z
Learnt from: inthirakumaaran
Repo: wso2/usage-data-collector PR: 9
File: collectors/org.wso2.carbon.usage.data.collector.identity/pom.xml:74-93
Timestamp: 2025-12-03T03:41:25.406Z
Learning: In WSO2 Carbon projects, dependency versions (such as httpclient5, httpcore5, securevault) are often constrained by what's available in the Carbon platform. Since libraries are imported as OSGi packages with version ranges, components will automatically use updated/secure versions when the Carbon platform is upgraded, without requiring changes to the component's pom.xml.
Applied to files:
collectors/org.wso2.carbon.usage.data.collector.identity/pom.xml
🔇 Additional comments (6)
collectors/org.wso2.carbon.usage.data.collector.identity/pom.xml (2)
32-234: Dependency removal for B2B organization counting is complete and verified.The removal of the
org.wso2.carbon.identity.organization.management.servicedependency, along with related imports and properties, is correctly implemented. No lingering references to organization management APIs remain in the codebase, confirming the changes enable backward compatibility with WSO2 IS 6.1.0 and below as intended.
220-220: No backward compatibility issues detected with Identity Framework version range.The version range has been widened from
[7.0.0,8.0.0)to[5.0.0,8.0.0)to support older Identity Framework versions. Verification confirms the collector only uses stable core utilities (IdentityDatabaseUtil,IdentityUtil) fromorg.wso2.carbon.identity.core, which are fundamental components available across all supported versions. The organization management dependency and all its references have been completely removed, eliminating any version-specific API concerns. The change is safe and aligns with the PR objective to support IS 6.1.0 and below.collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/UsageDataCollector.java (2)
54-59: LGTM!The constructor change correctly aligns with the simplified
UserCounterthat now only requiresRealmService.
129-136: LGTM!The tenant processing is cleanly simplified. The user count is now retrieved directly via
countUsersInOrganization, which aligns with the removal of B2B organization counting.collectors/org.wso2.carbon.usage.data.collector.identity/src/main/java/org/wso2/carbon/usage/data/collector/identity/counter/UserCounter.java (2)
52-55: LGTM!Constructor correctly simplified to accept only
RealmServiceafter removingOrganizationManagerdependency.
60-81: LGTM!The refactored method:
- Correctly resolves tenant ID from domain
- Properly manages the privileged tenant flow with cleanup in
finally- Error handling appropriately gates logging behind debug flag, consistent with project conventions
Based on learnings, error logging gated behind debug flags is the expected pattern for this module.
1 participant
Purpose
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.