WIP: feat: add user operation quota#103
Conversation
📝 WalkthroughWalkthroughIntroduces operation-type-based quota tracking, adds persistence and API support for current quota usage, and migrates profile quota data to ChangesQuota tracking refactor
Sequence Diagram(s)sequenceDiagram
participant Client
participant UserQuotaController
participant UserQuotaService
participant UserInfosRepository
Client->>UserQuotaController: POST /users/{sub}/quota/{operation}/{operationId}/start
UserQuotaController->>UserQuotaService: startUserOperation(sub, operation, operationId)
UserQuotaService->>UserInfosRepository: findById(sub)
UserInfosRepository-->>UserQuotaService: UserInfosEntity
UserQuotaService->>UserInfosRepository: save(entity with new UserOperationEntity)
UserQuotaController-->>Client: 200 OK
Client->>UserQuotaController: GET /users/{sub}/quota/current
UserQuotaController->>UserQuotaService: getUserCurrentQuotaUsage(sub)
UserQuotaService->>UserInfosRepository: findById(sub)
UserInfosRepository-->>UserQuotaService: UserInfosEntity
UserQuotaService-->>UserQuotaController: Map<OperationType, Integer>
UserQuotaController-->>Client: 200 OK with current usage
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/java/org/gridsuite/useradmin/server/service/UserQuotaService.java (1)
96-99: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueTODO: consider using a
UserOperationEntityrepository for removal.The in-memory
removeIfon the loaded collection works but loads/re-persists the wholeuserOperationslist. A targeted repository delete (bysub+operationId+operationType) would be more efficient. I can open a follow-up issue or draft the repository method — want me to?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/useradmin/server/service/UserQuotaService.java` around lines 96 - 99, The removal logic in UserQuotaService currently loads the whole UserInfosEntity and prunes userOperations in memory via removeIf, which is inefficient. Update the delete path to use a targeted UserOperationEntity repository method (or equivalent delete query) keyed by sub, operationId, and operationType, and keep the existing UserInfosEntity lookup only for the not-found check if needed.src/main/resources/db/changelog/changesets/changelog_20260703T132737Z.xml (1)
1-22: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd an index on
sub(and optionallyoperation_type) foruser_quotas.PostgreSQL doesn't automatically index foreign-key columns.
UserInfosEntity.getQuotaUsageMap()/userOperationswill query this table bysubon every quota check; without an index this becomes a full table scan as usage rows accumulate.♻️ Proposed addition
<changeSet author="abdellahghi (generated)" id="1783085266479-3"> <addForeignKeyConstraint baseColumnNames="sub" baseTableName="user_quotas" constraintName="userOperation_userInfo_sub_fk_constraint" deferrable="false" initiallyDeferred="false" referencedColumnNames="sub" referencedTableName="user_infos" validate="true"/> </changeSet> + <changeSet author="abdellahghi (generated)" id="1783085266479-4"> + <createIndex indexName="user_quotas_sub_operation_type_idx" tableName="user_quotas"> + <column name="sub"/> + <column name="operation_type"/> + </createIndex> + </changeSet>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/db/changelog/changesets/changelog_20260703T132737Z.xml` around lines 1 - 22, The `user_quotas` table is created without an index on the foreign-key column used by quota lookups. Update the changelog changeSet that creates `user_quotas` to add an index for `sub` (and `operation_type` if needed for the query pattern), so `UserInfosEntity.getQuotaUsageMap()`/`userOperations` can avoid full table scans during quota checks.src/main/java/org/gridsuite/useradmin/server/entity/UserInfosEntity.java (1)
91-94: 🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoffConsider computing quota usage via a repository count query instead of loading the full collection.
getQuotaUsageMap()lazily loads the entireuserOperationscollection into memory just to group/count by type. As theuser_quotastable grows per-user, this becomes an increasingly expensive in-memory aggregation versus aGROUP BY operation_typequery at the DB layer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/org/gridsuite/useradmin/server/entity/UserInfosEntity.java` around lines 91 - 94, `UserInfosEntity.getQuotaUsageMap()` currently aggregates `userOperations` in memory, which forces lazy-loading the full collection; replace this with a repository-backed count query that groups by `OperationType` at the database layer. Add a query method on the relevant repository for the `user_quotas`/`UserOperationEntity` data and have `getQuotaUsageMap()` (or its caller) use that result instead of streaming `userOperations`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/org/gridsuite/useradmin/server/entity/UserInfosEntity.java`:
- Around line 41-48: The `UserInfosEntity(String sub)` constructor is nulling
out `userOperations` by passing `null` into the delegating constructor, which
can later break `getQuotaUsageMap()`. Update `UserInfosEntity(String)` to
delegate with an empty list for the `userOperations` argument so the
`userOperations` collection stays initialized for all instances.
In
`@src/main/java/org/gridsuite/useradmin/server/entity/UserOperationEntity.java`:
- Around line 25-42: Add protection against duplicate user operation rows in
UserOperationEntity: the current append-only inserts from startUserOperation()
can create multiple entries for the same userInfos/sub and operationId, which
inflates getQuotaUsageMap() counts. Update the entity mapping to enforce
uniqueness on the (sub, operation_id) pair via a constraint/index, or otherwise
prevent duplicate inserts before persisting, using UserOperationEntity and its
userInfos/operationId fields as the key points to fix.
In `@src/main/java/org/gridsuite/useradmin/server/service/UserQuotaService.java`:
- Around line 70-73: `getUserProfileMaxAllowedBuilds` is reading the wrong quota
key, so it can return the cases limit instead of the builds limit. Update the
lookup in `UserQuotaService#getUserProfileMaxAllowedBuilds` to use the BUILD
quota symbol instead of CASES, while keeping the fallback to
`applicationProps.getDefaultMaxAllowedBuilds()`.
---
Nitpick comments:
In `@src/main/java/org/gridsuite/useradmin/server/entity/UserInfosEntity.java`:
- Around line 91-94: `UserInfosEntity.getQuotaUsageMap()` currently aggregates
`userOperations` in memory, which forces lazy-loading the full collection;
replace this with a repository-backed count query that groups by `OperationType`
at the database layer. Add a query method on the relevant repository for the
`user_quotas`/`UserOperationEntity` data and have `getQuotaUsageMap()` (or its
caller) use that result instead of streaming `userOperations`.
In `@src/main/java/org/gridsuite/useradmin/server/service/UserQuotaService.java`:
- Around line 96-99: The removal logic in UserQuotaService currently loads the
whole UserInfosEntity and prunes userOperations in memory via removeIf, which is
inefficient. Update the delete path to use a targeted UserOperationEntity
repository method (or equivalent delete query) keyed by sub, operationId, and
operationType, and keep the existing UserInfosEntity lookup only for the
not-found check if needed.
In `@src/main/resources/db/changelog/changesets/changelog_20260703T132737Z.xml`:
- Around line 1-22: The `user_quotas` table is created without an index on the
foreign-key column used by quota lookups. Update the changelog changeSet that
creates `user_quotas` to add an index for `sub` (and `operation_type` if needed
for the query pattern), so `UserInfosEntity.getQuotaUsageMap()`/`userOperations`
can avoid full table scans during quota checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 05a02330-342a-47fd-a23a-a8d0c76f608a
📒 Files selected for processing (21)
src/main/java/org/gridsuite/useradmin/server/UserAdminApplicationProps.javasrc/main/java/org/gridsuite/useradmin/server/controller/UserAdminController.javasrc/main/java/org/gridsuite/useradmin/server/controller/UserQuotaController.javasrc/main/java/org/gridsuite/useradmin/server/dto/OperationType.javasrc/main/java/org/gridsuite/useradmin/server/dto/UserProfile.javasrc/main/java/org/gridsuite/useradmin/server/entity/UserInfosEntity.javasrc/main/java/org/gridsuite/useradmin/server/entity/UserOperationEntity.javasrc/main/java/org/gridsuite/useradmin/server/service/UserAdminService.javasrc/main/java/org/gridsuite/useradmin/server/service/UserProfileService.javasrc/main/java/org/gridsuite/useradmin/server/service/UserQuotaService.javasrc/main/resources/db/changelog/changesets/changelog_20260703T132737Z.xmlsrc/main/resources/db/changelog/db.changelog-master.yamlsrc/test/java/org/gridsuite/useradmin/server/DtoConverterTest.javasrc/test/java/org/gridsuite/useradmin/server/NoQuotaTest.javasrc/test/java/org/gridsuite/useradmin/server/UserAdminTest.javasrc/test/java/org/gridsuite/useradmin/server/UserIdentityIntegrationTest.javasrc/test/java/org/gridsuite/useradmin/server/UserProfileTest.javasrc/test/java/org/gridsuite/useradmin/server/controller/UserGroupControllerTest.javasrc/test/java/org/gridsuite/useradmin/server/controller/UserInfosControllerTest.javasrc/test/java/org/gridsuite/useradmin/server/service/UserGroupServiceTest.javasrc/test/java/org/gridsuite/useradmin/server/service/UserInfosServiceTest.java
| @OneToMany(orphanRemoval = true, mappedBy = "userInfos", cascade = CascadeType.ALL) | ||
| private List<UserOperationEntity> userOperations = new ArrayList<>(); | ||
| @ManyToMany(mappedBy = "users") | ||
| private Set<GroupInfosEntity> groups; | ||
|
|
||
| public UserInfosEntity(String sub) { | ||
| this(UUID.randomUUID(), sub, null, null, null); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for call sites that might invoke getQuotaUsageMap()/use userOperations right after constructing via UserInfosEntity(String sub) without a DB round-trip
rg -n -B3 -A10 'new UserInfosEntity\(' --type=java
rg -n 'getQuotaUsageMap' --type=javaRepository: gridsuite/user-admin-server
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the entity and nearby methods
file='src/main/java/org/gridsuite/useradmin/server/entity/UserInfosEntity.java'
wc -l "$file"
ast-grep outline "$file" --view expanded || true
sed -n '1,220p' "$file" | cat -n
# Find all uses of the convenience constructor and quota map method
rg -n -B2 -A6 'new UserInfosEntity\(' src/main/java src/test/java || true
rg -n -B2 -A6 'getQuotaUsageMap\(' src/main/java src/test/java || trueRepository: gridsuite/user-admin-server
Length of output: 31091
Initialize userOperations in UserInfosEntity(String)
this(UUID.randomUUID(), sub, null, null, null) overwrites the field initializer with null, so instances created through this constructor can NPE in getQuotaUsageMap(). Pass an empty list here instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/org/gridsuite/useradmin/server/entity/UserInfosEntity.java`
around lines 41 - 48, The `UserInfosEntity(String sub)` constructor is nulling
out `userOperations` by passing `null` into the delegating constructor, which
can later break `getQuotaUsageMap()`. Update `UserInfosEntity(String)` to
delegate with an empty list for the `userOperations` argument so the
`userOperations` collection stays initialized for all instances.
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name = "sub", | ||
| referencedColumnName = "sub", nullable = false, | ||
| foreignKey = @ForeignKey(name = "userOperation_userInfo_sub_fk_constraint")) | ||
| private UserInfosEntity userInfos; | ||
|
|
||
| @Column(name = "operation_id", nullable = false) | ||
| private UUID operationId; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| @Column(name = "operation_type", nullable = false) | ||
| private OperationType operationType; | ||
|
|
||
| public UserOperationEntity(UserInfosEntity userInfos, UUID operationId, OperationType operationType) { | ||
| this.userInfos = userInfos; | ||
| this.operationId = operationId; | ||
| this.operationType = operationType; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether UserQuotaService guards against duplicate operationId inserts
rg -n -A15 'startOperation' src/main/java/org/gridsuite/useradmin/server/service/UserQuotaService.javaRepository: gridsuite/user-admin-server
Length of output: 165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate quota-related code paths and duplicate-insert handling
git ls-files 'src/main/java/**/*.java' | rg 'UserQuota|Quota|operationId|operation_type|operation_id|UserOperationEntity|UserInfosEntity'
printf '\n--- matches for operationId usage ---\n'
rg -n -A4 -B4 'operationId|operation_id|operation_type' src/main/java
printf '\n--- quota-related service/repository/entity definitions ---\n'
fd -a 'UserQuota|UserOperation|UserInfo|UserInfos' src/main/javaRepository: gridsuite/user-admin-server
Length of output: 9709
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- UserInfosEntity ---\n'
sed -n '1,240p' src/main/java/org/gridsuite/useradmin/server/entity/UserInfosEntity.java
printf '\n--- UserOperationEntity ---\n'
sed -n '1,220p' src/main/java/org/gridsuite/useradmin/server/entity/UserOperationEntity.java
printf '\n--- schema/migration references for user_operation or unique constraints ---\n'
rg -n -A3 -B3 'user_operation|operation_id|operation_type|unique.*sub|unique.*operation' src/main/resources src/main/javaRepository: gridsuite/user-admin-server
Length of output: 12438
Guard user_quotas against duplicate (sub, operation_id) rows
UserQuotaService.startUserOperation() always appends a new UserOperationEntity, and UserInfosEntity.getQuotaUsageMap() counts every row. A retry or re-submit of the same request will inflate quota usage. Add a unique constraint on (sub, operation_id) or deduplicate before insert.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/java/org/gridsuite/useradmin/server/entity/UserOperationEntity.java`
around lines 25 - 42, Add protection against duplicate user operation rows in
UserOperationEntity: the current append-only inserts from startUserOperation()
can create multiple entries for the same userInfos/sub and operationId, which
inflates getQuotaUsageMap() counts. Update the entity mapping to enforce
uniqueness on the (sub, operation_id) pair via a constraint/index, or otherwise
prevent duplicate inserts before persisting, using UserOperationEntity and its
userInfos/operationId fields as the key points to fix.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/test/java/org/gridsuite/useradmin/server/controller/UserQuotaControllerTest.java (2)
66-232: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing test for quota-exceeded rejection.
This suite exercises max-quota lookup, current-usage aggregation, start/end lifecycle, and reset, but no test verifies that
startis rejected once a user's usage for an operation type reaches its max quota. Since the PR's stated purpose is to enforce a quota check before operations, this is the central behavior and currently has no coverage in this controller test.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/org/gridsuite/useradmin/server/controller/UserQuotaControllerTest.java` around lines 66 - 232, Add a controller test in UserQuotaControllerTest that verifies `startUserOperation` is rejected once the user reaches the max quota for an `OperationType`. Use the existing helpers like `startOperation` and `getCurrentQuota`, and first create a user plus enough existing operations to hit the limit for a type such as `BUILD` or `CASES`. Then call the `start` endpoint again and assert the response is not accepted (for example a conflict/forbidden-style status) and that the quota usage does not increase.
68-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRepeated entity setup could be extracted.
new UserInfosEntity(UUID.randomUUID(), USER_A, null, null, null)is duplicated across 8 tests. Consider a@BeforeEachhelper to reduce duplication.Also applies to: 99-99, 117-117, 152-152, 174-174, 186-186, 209-209, 225-225
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/org/gridsuite/useradmin/server/controller/UserQuotaControllerTest.java` at line 68, Extract the repeated UserInfosEntity setup used in UserQuotaControllerTest into a shared `@BeforeEach` helper or factory method so the same user record is created once per test instead of duplicating new UserInfosEntity(UUID.randomUUID(), USER_A, null, null, null) across multiple test cases. Keep the helper near the test fixture setup and update the affected tests to reuse it, using userInfosRepository as the common insertion point.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/test/java/org/gridsuite/useradmin/server/controller/UserQuotaControllerTest.java`:
- Around line 66-232: Add a controller test in UserQuotaControllerTest that
verifies `startUserOperation` is rejected once the user reaches the max quota
for an `OperationType`. Use the existing helpers like `startOperation` and
`getCurrentQuota`, and first create a user plus enough existing operations to
hit the limit for a type such as `BUILD` or `CASES`. Then call the `start`
endpoint again and assert the response is not accepted (for example a
conflict/forbidden-style status) and that the quota usage does not increase.
- Line 68: Extract the repeated UserInfosEntity setup used in
UserQuotaControllerTest into a shared `@BeforeEach` helper or factory method so
the same user record is created once per test instead of duplicating new
UserInfosEntity(UUID.randomUUID(), USER_A, null, null, null) across multiple
test cases. Keep the helper near the test fixture setup and update the affected
tests to reuse it, using userInfosRepository as the common insertion point.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 290a719e-37b5-4558-bd2b-8358363f99ac
📒 Files selected for processing (23)
src/main/java/org/gridsuite/useradmin/server/UserAdminApplicationProps.javasrc/main/java/org/gridsuite/useradmin/server/controller/UserAdminController.javasrc/main/java/org/gridsuite/useradmin/server/controller/UserQuotaController.javasrc/main/java/org/gridsuite/useradmin/server/dto/OperationType.javasrc/main/java/org/gridsuite/useradmin/server/dto/UserProfile.javasrc/main/java/org/gridsuite/useradmin/server/entity/UserInfosEntity.javasrc/main/java/org/gridsuite/useradmin/server/entity/UserOperationEntity.javasrc/main/java/org/gridsuite/useradmin/server/service/UserAdminService.javasrc/main/java/org/gridsuite/useradmin/server/service/UserProfileService.javasrc/main/java/org/gridsuite/useradmin/server/service/UserQuotaService.javasrc/main/resources/db/changelog/changesets/changelog_20260703T132737Z.xmlsrc/main/resources/db/changelog/db.changelog-master.yamlsrc/test/java/org/gridsuite/useradmin/server/DtoConverterTest.javasrc/test/java/org/gridsuite/useradmin/server/NoQuotaTest.javasrc/test/java/org/gridsuite/useradmin/server/UserAdminTest.javasrc/test/java/org/gridsuite/useradmin/server/UserIdentityIntegrationTest.javasrc/test/java/org/gridsuite/useradmin/server/UserProfileTest.javasrc/test/java/org/gridsuite/useradmin/server/controller/UserGroupControllerTest.javasrc/test/java/org/gridsuite/useradmin/server/controller/UserInfosControllerTest.javasrc/test/java/org/gridsuite/useradmin/server/controller/UserQuotaControllerTest.javasrc/test/java/org/gridsuite/useradmin/server/service/UserGroupServiceTest.javasrc/test/java/org/gridsuite/useradmin/server/service/UserInfosServiceTest.javasrc/test/java/org/gridsuite/useradmin/server/service/UserQuotaServiceTest.java
✅ Files skipped from review due to trivial changes (4)
- src/test/java/org/gridsuite/useradmin/server/service/UserGroupServiceTest.java
- src/main/resources/db/changelog/changesets/changelog_20260703T132737Z.xml
- src/test/java/org/gridsuite/useradmin/server/service/UserInfosServiceTest.java
- src/test/java/org/gridsuite/useradmin/server/UserIdentityIntegrationTest.java
🚧 Files skipped from review as they are similar to previous changes (17)
- src/main/resources/db/changelog/db.changelog-master.yaml
- src/main/java/org/gridsuite/useradmin/server/dto/OperationType.java
- src/main/java/org/gridsuite/useradmin/server/entity/UserOperationEntity.java
- src/main/java/org/gridsuite/useradmin/server/UserAdminApplicationProps.java
- src/test/java/org/gridsuite/useradmin/server/controller/UserInfosControllerTest.java
- src/test/java/org/gridsuite/useradmin/server/DtoConverterTest.java
- src/test/java/org/gridsuite/useradmin/server/UserAdminTest.java
- src/test/java/org/gridsuite/useradmin/server/controller/UserGroupControllerTest.java
- src/main/java/org/gridsuite/useradmin/server/controller/UserQuotaController.java
- src/main/java/org/gridsuite/useradmin/server/dto/UserProfile.java
- src/test/java/org/gridsuite/useradmin/server/UserProfileTest.java
- src/main/java/org/gridsuite/useradmin/server/service/UserAdminService.java
- src/main/java/org/gridsuite/useradmin/server/service/UserQuotaService.java
- src/main/java/org/gridsuite/useradmin/server/controller/UserAdminController.java
- src/test/java/org/gridsuite/useradmin/server/NoQuotaTest.java
- src/main/java/org/gridsuite/useradmin/server/entity/UserInfosEntity.java
- src/main/java/org/gridsuite/useradmin/server/service/UserProfileService.java



PR Summary