Dev/1227 fr core api add item mutex#1260
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
Code Coverage
|
Code Coverage
|
…handler to mongo services to optionally handle adding objects. Used this to create mutex for items.
Code Coverage
|
Code Coverage
|
Code Coverage
|
…ensure no deadlocking
Code Coverage
|
Code Coverage
|
…n appliers beans instead of pojos to allow for metrics and similar
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckinLossTransactionApplier.java (1)
19-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBelay this unfinished applier from active duty.
Line 19 registers this class as a CDI bean, but Line 39 still throws
NotImplementedException. With appliers auto-collected, this transaction type can be dispatched and fail at runtime. Either implementapply(...)now or keep this class out of bean discovery until it’s seaworthy.⚓ Proposed short-term safety patch
-@ApplicationScoped public class CheckinLossTransactionApplier extends CheckinOutTransactionApplier<CheckinLossTransaction> {software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckinPartTransactionApplier.java (1)
18-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not ship this applier as a live bean while it still throws.
Line 18 makes this handler discoverable, and Line 39 still hard-fails with
NotImplementedException. That sets a course for runtime blowups whenCHECKIN_PARTis used. Best to implement it now or remove it from CDI discovery until complete.⚓ Proposed short-term safety patch
-@ApplicationScoped public class CheckinPartTransactionApplier extends CheckinOutTransactionApplier<CheckinPartTransaction> {
🧹 Nitpick comments (2)
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/TransferAmountAppliedTransactionTest.java (1)
83-85: ⚡ Quick winDon’t lash these assertions to list order, sailor.
These checks depend on
search(...).getResults().get(0/1)ordering, which can drift and make tests flaky. Assert by concrete stored IDs (or block IDs) instead of index position.Also applies to: 165-167, 248-250, 331-333, 415-417, 490-492
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/TransferWholeAppliedTransactionTest.java (1)
42-275: 🏗️ Heavy liftGood wind on success paths, but failure paths be uncharted waters.
This suite only tests happy flows. Add negative cases (mismatched block/stored IDs, missing required transfer references, invalid storage-type combinations) so transfer-whole guards hold fast.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0e2c94a0-0d5d-4e46-b4be-c80cc3cf6a6f
📒 Files selected for processing (44)
software/core/oqm-core-api/build.gradlesoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/config/MutexConfig.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/items/StoredEndpoints.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/items/StoredInItemEndpoints.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/AttKeywordMainObject.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/scheduled/MongoDbInit.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/ItemStatsService.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/InventoryItemService.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/MongoObjectService.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/StoredService.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/AppliedTransactionService.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/AddAmountTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/AddWholeTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckinFullTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckinLossTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckinOutTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckinPartTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckoutAmountTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckoutWholeTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/SetAmountTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/SubtractAmountTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/SubtractWholeTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/TransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/TransferAmountTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/TransferWholeTransactionApplier.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/serviceState/InstanceMutexService.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/items/InventoryItemsCrudTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/AppliedTransactionServiceTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/GeneratedIdentifierGenerationServiceTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/AddAmountAppliedTransactionTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/AddWholeAppliedTransactionTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/AppliedTransactionServiceTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/CheckinFullAmountAppliedTransactionTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/CheckinWholeAppliedTransactionTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/CheckoutAmountAppliedTransactionTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/CheckoutWholeAppliedTransactionTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/SetAmountAppliedTransactionTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/SubtractAmountAppliedTransactionTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/SubtractWholeAppliedTransactionTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/TransferAmountAppliedTransactionTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/TransferWholeAppliedTransactionTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/serviceState/InstanceMutexServiceTest.javasoftware/core/oqm-core-api/src/test/resources/application.yamlsoftware/libs/core-api-lib-quarkus/runtime/src/main/java/tech/ebp/oqm/lib/core/api/quarkus/runtime/restClient/OqmCoreApiClientService.java
💤 Files with no reviewable changes (2)
- software/libs/core-api-lib-quarkus/runtime/src/main/java/tech/ebp/oqm/lib/core/api/quarkus/runtime/restClient/OqmCoreApiClientService.java
- software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/items/StoredEndpoints.java
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: CI-Pipeline / Unit Tests
🧰 Additional context used
🪛 PMD (7.24.0)
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/AppliedTransactionServiceTest.java
[Low] 231-231: InvalidLogMessageFormat (Error Prone): Too many arguments, expected 0 argument but found 1
(InvalidLogMessageFormat (Error Prone))
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/serviceState/InstanceMutexServiceTest.java
[Medium] 127-127: OverrideBothEqualsAndHashCodeOnComparable (Error Prone): When implementing Comparable, both equals() and hashCode() should be overridden
(OverrideBothEqualsAndHashCodeOnComparable (Error Prone))
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/AddAmountAppliedTransactionTest.java
[Low] 56-56: InvalidLogMessageFormat (Error Prone): Too many arguments, expected 0 argument but found 1
(InvalidLogMessageFormat (Error Prone))
🔇 Additional comments (17)
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/ItemStatsService.java (1)
186-187: ⚡ Quick winYer concern about self-calls be unfounded in these waters.
Quarkus's CDI (ArC) be deliberately crafted to support self-interception for methods bearing
@WithSpan—unlike standard Weld. When a CDI bean calls its own annotated method (and the method be public, which thine methods are), the span shall be created proper-like. No observability gap troubles this vessel.> Likely an incorrect or invalid review comment.software/core/oqm-core-api/build.gradle (1)
11-11: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/config/MutexConfig.java (1)
8-29: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/scheduled/MongoDbInit.java (1)
27-57: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckoutWholeTransactionApplier.java (1)
4-4: LGTM!Also applies to: 23-24
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/SetAmountTransactionApplier.java (1)
4-4: LGTM!Also applies to: 19-20
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/SubtractAmountTransactionApplier.java (1)
4-4: LGTM!Also applies to: 18-19
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/SubtractWholeTransactionApplier.java (1)
4-4: LGTM!Also applies to: 19-20
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/TransferWholeTransactionApplier.java (1)
4-4: LGTM!Also applies to: 16-17
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/AppliedTransactionService.java (1)
4-4: LGTM!Also applies to: 30-30, 47-56, 63-66, 87-91, 112-112
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/items/StoredInItemEndpoints.java (1)
157-164: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/AddAmountAppliedTransactionTest.java (1)
42-338: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/AddWholeAppliedTransactionTest.java (1)
45-290: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/SetAmountAppliedTransactionTest.java (1)
48-308: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/CheckoutAmountAppliedTransactionTest.java (1)
49-482: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/CheckoutWholeAppliedTransactionTest.java (1)
48-268: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/CheckinWholeAppliedTransactionTest.java (1)
52-265: LGTM!
Code Coverage
|
Code Coverage
|
Code Coverage
|
Checklist:
Closes #1227
Summary by CodeRabbit
Release Notes - Version 4.4.8-SNAPSHOT
New Features
Tests
Improvements