Development#1263
Conversation
…handler to mongo services to optionally handle adding objects. Used this to create mutex for items.
…ensure no deadlocking
…n appliers beans instead of pojos to allow for metrics and similar
…core-api-add-item-mutex Dev/1227 fr core api add item mutex
📝 WalkthroughWalkthroughA single large change: migrate Stored -> polymorphic StoredState, add StoredSchema upgrader, add mutex await/timeout and resource locking, add DB index/init hooks and MongoDbInit startup backfill, refactor transaction appliers to CDI/state checks, update client/UI callsites, and add many integration tests and version bumps. ChangesCore State, Mutex, and Initialization
Sequence Diagram(s)sequenceDiagram
participant Client
participant AppliedTransactionService
participant InstanceMutexService
participant MongoDB
Client->>AppliedTransactionService: apply(transaction)
AppliedTransactionService->>InstanceMutexService: getResource(mutexId)
InstanceMutexService-->>AppliedTransactionService: locked resource
AppliedTransactionService->>MongoDB: perform mutations within session
AppliedTransactionService->>InstanceMutexService: free(mutexId)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
…d search to reflect this
…core-api-change-stored-field-to-object Dev/1228 fr core api change stored field to object
Code Coverage
|
Code Coverage
|
basic mongodb indexig added
Code Coverage
|
Code Coverage
|
Code Coverage
|
2 similar comments
Code Coverage
|
Code Coverage
|
Code Coverage
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/AddWholeTransactionApplier.java (1)
44-60:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvast! Potential null on
transaction.getToBlock()could pitch ye overboard with an NPE.Two foul winds in this stretch:
- Lines 44-45: if
stored.getState()is null andtransaction.getToBlock()is also null, aStoredInBlockis built with a nullstorageBlock— leaving an unanchored stored in the hold.- Line 58:
transaction.getToBlock().equals(...)is called without any null check. If the transaction was submitted without atoBlock(and the caller supplied a state onstored), this throwsNullPointerExceptioninstead of the intendedIllegalArgumentException.Recommend rejecting a null
toBlockup front, or comparing in a null-safe way (e.g.Objects.equals(...)).🪢 Suggested null-safety patch
+ if (transaction.getToBlock() == null) { + throw new IllegalArgumentException("No to block given for whole add."); + } + if(stored.getState() == null) { stored.setState(StoredInBlock.builder().storageBlock(transaction.getToBlock()).build()); } else if(!stored.isState(StoredStateType.STORED)) { throw new IllegalArgumentException("Cannot add whole stored that is not stored in a block."); } @@ - if (!transaction.getToBlock().equals(((StoredInBlock)stored.getState()).getStorageBlock())) { + if (!transaction.getToBlock().equals(((StoredInBlock) stored.getState()).getStorageBlock())) { throw new IllegalArgumentException("To Block given does not match block marked in stored."); }
🧹 Nitpick comments (13)
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/state/StoredInBlock.java (1)
4-4: 💤 Low valueStowaway import!
ApplicationScopedbe aboard but never called to duty.
jakarta.enterprise.context.ApplicationScopedis imported but unused — this is a plain POJO, not a CDI bean. Toss it overboard.🧹 Suggested fix
-import jakarta.enterprise.context.ApplicationScoped; import jakarta.validation.constraints.NotNull;software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/scheduled/MongoDbInit.java (1)
41-55: ⚡ Quick winSteady the tiller — backfill be unbounded with nary an error rail.
A few concerns about
ensureItemMutexesExist:
- No error handling per item/db. If a single
instanceMutexService.register(...)call throws (e.g., mutex already exists in a state that rejects re-registration, or a transient DB hiccup mid-iteration), the entire startup fails. Consider wrapping each per-item registration in a try/catch and logging the failure so other items still get backfilled.- Idempotency. The doc-comment says this "can be removed once all inventory items have mutexes" but the code unconditionally calls
registeron every startup. Confirmregisteris idempotent (or guard with a "registered?" check), else restarts may produce duplicate-registration errors or wasted work on large inventories.- Observability. For large inventories, a count of items processed per DB would help operators distinguish "ran fast because empty" from "skipped silently".
♻️ Suggested per-item guard + counters
private void ensureItemMutexesExist() { log.info("Ensuring inventory item mutexes exist."); for (DbCacheEntry curDb : this.oqmDatabaseService.getDatabases()) { log.info("Ensuring inventory item mutexes exist for database: {}", curDb.getDbName()); + int registered = 0, failed = 0; this.inventoryItemService.iterator(curDb.getDbId().toHexString()).forEachRemaining((item) -> { - this.instanceMutexService.register( - this.instanceMutexService.getMutexIdFor(curDb.getDbId().toHexString(), item) - ); + try { + this.instanceMutexService.register( + this.instanceMutexService.getMutexIdFor(curDb.getDbId().toHexString(), item) + ); + } catch (Exception e) { + log.warn("Failed registering mutex for item {} in db {}: {}", item.getId(), curDb.getDbName(), e.toString()); + } }); - log.info("DONE Ensuring inventory item mutexes exist for database: {}", curDb.getDbName()); + log.info("DONE Ensuring inventory item mutexes exist for database: {}", curDb.getDbName()); } log.info("DONE Ensuring inventory item mutexes exist."); }software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/MongoService.java (1)
117-122: 💤 Low valueSet sail to remove the no-op
background(true)
InMongoService.setupIndexes,new IndexOptions().background(true)won’t steer anything on your current MongoDB waters: MongoDB 4.2+ ignores thebackgroundoption forcreateIndex/createIndexes. Since you’re usingmongo:7in both dev services and tests, this is just extra rope—safe to drop (keep the same startup-fail behavior if an index build errors).🛠️ Proposed cleanup
protected static void setupIndexes(MongoCollection<?> collection, List<Bson> indexes) { - IndexOptions options = new IndexOptions().background(true); + IndexOptions options = new IndexOptions(); for (Bson index : indexes) { collection.createIndex(index, options); } }software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/AppliedTransactionService.java (1)
47-66: 💤 Low valueSteady as she goes — registry build via CDI looks ship-shape.
The
@All-injected applier list populated in@PostConstructintoapplierTypeMapis sound for an@ApplicationScopedbean (single init, concurrent reads thereafter). One small caution from the crow's nest: if twoTransactionApplierbeans ever return the sameTransactionTypefromgetTransactionType(), the latter would silently overwrite the former in the map. AputIfAbsentwith a thrown exception (or a startup-time duplicate check) would prevent a stowaway applier from quietly taking over.🧭 Suggested guard against duplicate applier types
`@PostConstruct` void setupAppliers() { for (TransactionApplier<?> applier : this.transactionAppliers) { - this.applierTypeMap.put(applier.getTransactionType(), applier); + TransactionApplier<?> prev = this.applierTypeMap.putIfAbsent(applier.getTransactionType(), applier); + if (prev != null) { + throw new IllegalStateException( + "Duplicate TransactionApplier registered for type " + applier.getTransactionType() + + ": " + prev.getClass().getName() + " and " + applier.getClass().getName() + ); + } } }software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/AddAmountTransactionApplier.java (1)
78-103: 💤 Low valueTwo lookouts hollerin' the same warning — duplicate STORED state check.
The
isState(StoredStateType.STORED)check on lines 80-82 is repeated almost verbatim at lines 101-103 after the switch. Since the post-switch check (line 101) covers all branches, the inner one in theAMOUNT_LISTexisting-stored path is redundant. Trim one of them to keep the rigging tidy.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/TransferWholeTransactionApplier.java (1)
42-71: ⚡ Quick winThree checks of the same knot — and the rope's grammar be frayed.
Two matters t' tidy on this deck:
- Redundant state checks. The
STOREDcheck be raised thrice — once in each branch of the switch (lines 42-44, 53-55) and again after the switch (lines 69-71). The inner check at lines 53-55 be needed to guard the cast at line 57, but the BULK/UNIQUE_SINGLE check at 42-44 hath no cast that follows, and the outer check at 69-71 be entirely redundant once both branches have run. Hoist 'em down to one well-placed check apiece.- Grammar in the error message. "Cannot transfer whole stored that not stored." be missin' the word "is" — appears three times. Should read "...that is not stored."
♻️ Proposed tidyin' of the riggin'
switch (inventoryItem.getStorageType()) { case BULK, UNIQUE_SINGLE -> { toTransfer = this.getStoredService().getSingleStoredForItemBlock(oqmDbIdOrName, cs, inventoryItem.getId(), transaction.getFromBlock(), Stored.class); - - if(!toTransfer.isState(StoredStateType.STORED)){ - throw new IllegalArgumentException("Cannot transfer whole stored that not stored."); - } - + if(transaction.getStoredToTransfer() != null && !toTransfer.getId().equals(transaction.getStoredToTransfer())) { throw new IllegalArgumentException("Stored item from block does not match stored specified."); } } case AMOUNT_LIST, UNIQUE_MULTI -> { toTransfer = this.getStoredService().get(oqmDbIdOrName, cs, transaction.getStoredToTransfer()); - + if(!toTransfer.isState(StoredStateType.STORED)){ - throw new IllegalArgumentException("Cannot transfer whole stored that not stored."); + throw new IllegalArgumentException("Cannot transfer whole stored that is not stored."); } - + if(transaction.getFromBlock() != null && !((StoredInBlock)toTransfer.getState()).getStorageBlock().equals(transaction.getFromBlock())) { throw new IllegalArgumentException("Stored item specified does not match stored from storage block."); } } default -> throw new IllegalArgumentException("Unsupported storage type. This should never happen."); } if (!inventoryItem.getId().equals(toTransfer.getItem())) { throw new IllegalArgumentException("Stored is not associated with the item."); } - + if(!toTransfer.isState(StoredStateType.STORED)){ - throw new IllegalArgumentException("Cannot transfer whole stored that not stored."); + throw new IllegalArgumentException("Cannot transfer whole stored that is not stored."); }software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/serviceState/InstanceMutexService.java (2)
254-256: 💤 Low valueSpellin' mutiny: "maitTime" be no proper word, matey.
maitTimeMin/maitTimeMaxshould bewaitTimeMin/waitTimeMax. Same misspell sails on into the debug log on line 260. Set the rigging right.🪛 Trim the typo
- long maitTimeMin = this.getMutexConfig().await().loopPauseMin().toMillis(); - long maitTimeMax = this.getMutexConfig().await().loopPauseMax().toMillis(); + long waitTimeMin = this.getMutexConfig().await().loopPauseMin().toMillis(); + long waitTimeMax = this.getMutexConfig().await().loopPauseMax().toMillis(); ... - log.debug("Awaiting lock for {} with timeout of {}. Max wait time: {}. Min wait time: {}", mutexId, toWait, maitTimeMax, maitTimeMin); + log.debug("Awaiting lock for {} with timeout of {}. Max wait time: {}. Min wait time: {}", mutexId, toWait, waitTimeMax, waitTimeMin); ... - long sleepTime = random.nextLong(maitTimeMin, maitTimeMax); + long sleepTime = random.nextLong(waitTimeMin, waitTimeMax);Also applies to: 260-260
134-161: ⚡ Quick winBelay the redundant find, the upsert can carry the load alone.
The pre-check
find().into(ArrayList)at line 136 is followed by an idempotent upsert at line 145 that already does the right thing on its own; the read-then-write window also leaves a wee gap for races. Strikin' the read would simplifyregisterand remove a dead reef — theclearDuplicateMutexespath on line 156 won't ever be reached anyway since the upsert prevents duplicates by id of the mutexId field (assuming a unique index — see related comment inOqmDatabaseServicefor the same indexing concern; should also add a unique index onmutexIdfor this collection).software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/items/InventoryItemsCrudTest.java (1)
216-216: ⚡ Quick winTODO be flyin' the black flag.
//TODO:: check results— the test asserts only the HTTP 200 on each call and the final GET status. With nothing checked, a regression that silently drops updates (the very thing a mutex test ought to catch) would sail right past. At minimum, assert that the final item'sattributescontains an entry per thread keyedtestThread-Nwith valuenumIterations.Want me to draft the assertions and open a follow-up issue to track the rest of the result verification?
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/serviceState/InstanceMutexServiceTest.java (1)
157-164: 💤 Low valueThat there be a busy-waitin' loop, mate!
Ye've got an empty while loop here when
awaitbe false - just spinnin' the wheel until the lock be available. Now, this ain't technically wrong, but it be wastin' CPU cycles like throwin' rum overboard!Consider addin' a
Thread.yield()orThread.onSpinWait()inside that loop to give other threads a chance to do their work, or at least add a comment explainin' why ye chose this approach. Makes the code easier for the next sailor who comes aboard to understand yer intentions.🔄 Suggested improvement to ease the CPU burden
if(await){ instanceMutexService.awaitLock(this.mutexId, threadIdOpt); } else { - //noinspection StatementWithEmptyBody - while (!instanceMutexService.lock(this.mutexId, threadIdOpt)) { - //nothing to do - } + while (!instanceMutexService.lock(this.mutexId, threadIdOpt)) { + Thread.onSpinWait(); // Give other threads a chance + } }software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/UiProvider.java (1)
70-82: 💤 Low valueSwab these commented lines from the deck, sailor!
This here be a sizeable block o' commented code cluttering up yer hull. Dead code like this makes the ship harder to navigate and maintain. Best to heave it overboard entirely rather than lettin' it drift about in yer codebase.
software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/service/modelTweak/SearchResultTweak.java (1)
39-39: ⚖️ Poor tradeoffAye—your callers be sailing the new signature cleanly (old-order use lives only in commented code).
- Active invocations in
OverviewUi,ItemStoredPassthrough,ItemInBlockPassthrough,ItemCheckoutUi, andItemCheckoutPassthroughall pass(results, oqmDb, apiToken, "...keys"), matching(ObjectNode, String oqmDb, String apiToken, String... keys).ItemStoredPassthrough.javastill has a commented-out call using the old argument order; safe for now, but best to delete or update it to keep the deck shipshape.software/libs/core-api-lib-quarkus/runtime/src/main/java/tech/ebp/oqm/lib/core/api/quarkus/runtime/restClient/models/InteractingEntity.java (1)
11-24: ⚡ Quick winAdd
@JsonIgnoreProperties(ignoreUnknown = true)to the client DTO for forward-compatibility.No repo-wide Quarkus/Jackson setting was found that would ignore unknown fields for this rest-client deserialization; only server-side Mappers/readers toggle
FAIL_ON_UNKNOWN_PROPERTIES. Batten downInteractingEntityagainst surprise payload sails.♻️ Suggested charm against unknown-field shipwreck
package tech.ebp.oqm.lib.core.api.quarkus.runtime.restClient.models; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import lombok.AllArgsConstructor; import lombok.Data; import lombok.NoArgsConstructor; import java.util.List; import java.util.Map; import java.util.Set; `@Data` `@AllArgsConstructor` `@NoArgsConstructor` +@JsonIgnoreProperties(ignoreUnknown = true) public class InteractingEntity {
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3c367179-8474-4d3e-8c18-1f735d887804
📒 Files selected for processing (92)
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/exception/mutex/MutexNotRegisteredException.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/exception/mutex/MutexWaitInterruptedException.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/exception/mutex/MutexWaitTimeoutException.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/interfaces/endpoints/info/RunByUtils.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/model/object/storage/checkout/ItemCheckout.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/checkout/ItemWholeCheckout.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/Stored.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/state/StoredInBlock.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/state/StoredState.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/state/StoredStateType.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/rest/search/StoredSearch.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/scheduled/LifecycleBean.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/ItemListService.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/MongoDbAwareService.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/MongoService.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/StorageBlockService.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/TopLevelMongoService.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/schemaVersioning/upgraders/stored/StoredSchemaUpgrader.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/schemaVersioning/upgraders/stored/bumpers/StoredItemBumper5.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/serviceState/InstanceMutexService.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/serviceState/db/OqmDatabaseService.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/interfaces/endpoints/inventory/items/InventoryItemsStatsTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/items/StoredInItemEndpointsTest.javasoftware/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/StoredTest.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/StoredServiceTest.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/java/tech/ebp/oqm/core/api/testResources/data/StoredTestObjectCreator.javasoftware/core/oqm-core-api/src/test/resources/application.yamlsoftware/core/oqm-core-base-station/build.gradlesoftware/core/oqm-core-base-station/installerSrc/installerProperties.jsonsoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/rest/passthrough/inventory/ItemCheckoutPassthrough.javasoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/rest/passthrough/inventory/ItemInBlockPassthrough.javasoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/rest/passthrough/inventory/ItemStoredPassthrough.javasoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/ItemCheckoutUi.javasoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/OverviewUi.javasoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/UiProvider.javasoftware/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/service/modelTweak/SearchResultTweak.javasoftware/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/obj/item/ItemView.jssoftware/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/obj/itemStored/ItemStoredTransaction.jssoftware/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/obj/itemStored/StoredView.jssoftware/core/oqm-core-base-station/src/main/resources/templates/tags/search/itemStored/searchResults.htmlsoftware/core/oqm-core-base-station/src/main/resources/templates/webui/mainWebPageTemplate.htmlsoftware/core/oqm-core-base-station/src/test/java/tech/ebp/oqm/core/baseStation/testResources/testUsers/TestUser.javasoftware/libs/core-api-lib-quarkus/deployment/pom.xmlsoftware/libs/core-api-lib-quarkus/deployment/src/main/java/tech/ebp/oqm/lib/core/api/quarkus/deployment/config/CoreApiLibBuildTimeConfig.javasoftware/libs/core-api-lib-quarkus/integration-tests/pom.xmlsoftware/libs/core-api-lib-quarkus/pom.xmlsoftware/libs/core-api-lib-quarkus/runtime/pom.xmlsoftware/libs/core-api-lib-quarkus/runtime/src/main/java/tech/ebp/oqm/lib/core/api/quarkus/runtime/restClient/OqmCoreApiClientService.javasoftware/libs/core-api-lib-quarkus/runtime/src/main/java/tech/ebp/oqm/lib/core/api/quarkus/runtime/restClient/models/InteractingEntity.javasoftware/libs/core-api-lib-quarkus/runtime/src/main/java/tech/ebp/oqm/lib/core/api/quarkus/runtime/restClient/searchObjects/StoredSearch.javasoftware/plugins/storagotchi/src/main/docker/Dockerfile.native
💤 Files with no reviewable changes (2)
- software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/interfaces/endpoints/info/RunByUtils.java
- software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/items/StoredEndpoints.java
📜 Review details
🧰 Additional context used
🪛 HTMLHint (1.9.2)
software/core/oqm-core-base-station/src/main/resources/templates/tags/search/itemStored/searchResults.html
[error] 97-97: Special characters must be escaped : [ < ].
(spec-char-escape)
[error] 99-99: Special characters must be escaped : [ > ].
(spec-char-escape)
[error] 101-101: Tag must be paired, no start tag: [ ]
(tag-pair)
[error] 96-96: Tag must be paired, missing: [ ], start tag match failed [ ] on line 96.
(tag-pair)
software/core/oqm-core-base-station/src/main/resources/templates/webui/mainWebPageTemplate.html
[error] 275-275: The id value [ messageDiv ] must be unique.
(id-unique)
🪛 PMD (7.24.0)
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/AddAmountAppliedTransactionTest.java
[Low] 57-57: 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/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))
🔇 Additional comments (85)
software/core/oqm-core-api/build.gradle (1)
11-11: LGTM!Also applies to: 47-47
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/AttKeywordMainObject.java (1)
44-44: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/checkout/ItemCheckout.java (1)
42-115: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/exception/mutex/MutexWaitTimeoutException.java (1)
1-1: ⚡ Quick winNo stowaways left after hoisting
MutexWaitTimeoutExceptioninto.mutex
Aye—no callsites still reference the old FQNtech.ebp.oqm.core.api.exception.MutexWaitTimeoutExceptionanywhere in the repo (Java or non-Java resources). All usages point totech.ebp.oqm.core.api.exception.mutex.MutexWaitTimeoutException(e.g.,InstanceMutexServiceandInstanceMutexServiceTest).software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/config/MutexConfig.java (1)
11-12: ⚡ Quick win
${quarkus.uuid}expands correctly in@WithDefault(no literal string risk)Avast! SmallRye Config expands
${...}property expressions for configuration values used via@WithDefault, soMutexConfig.instanceId()resolves to Quarkus’s generatedquarkus.uuid(not the literal"${quarkus.uuid}") at runtime. [software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/config/MutexConfig.java:11-12]software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/items/StoredInItemEndpoints.java (1)
151-164: ⚡ Quick winProvide the original
<review_comment>(and any related diff/context) so I can rewrite it.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/state/StoredStateType.java (1)
3-5: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/rest/search/StoredSearch.java (2)
88-108: LGTM!
74-74: ⚡ Quick winMissing input: provide the
<review_comment>text Paste the original review comment (and any verification outputs/results you want me to use) so I can rewrite it in the required format.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/scheduled/LifecycleBean.java (1)
45-45: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/state/StoredState.java (1)
12-25: 🏗️ Heavy liftMissing input to rewrite the review comment: Provide the original text inside the
<review_comment>tags (and any verification outputs/results), otherwise the comment can’t be rewritten accurately.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/Stored.java (1)
72-72: ⚡ Quick winNo gap in v4→v5 Stored migration — StoredItemBumper5 moves
storageBlockintostate.storageBlockand clears the old field
Ahoy matey:StoredItemBumper5normalizes the legacy top-levelstorageBlock, removes it (oldObj.remove("storageBlock")), and writesstatewithtype: "STORED"plusstate.storageBlock.StoredSearchthen queriesstate.typeandstate.storageBlock, andstate.storageBlocklands inStoredInBlock.storageBlock(anObjectId) viaMongoObjectIdModule’s String→ObjectIddeserializer, so the upgraded records should surface correctly.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/scheduled/MongoDbInit.java (1)
58-70: ⚡ Quick winMissing input: Provide the original review comment text (inside
<review_comment>tags) and the verification outputs/logs you want me to base the rewrite on.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/InventoryItemService.java (2)
286-290: LGTM!
453-458: ⚡ Quick winProvide the original
<review_comment>(including any diff snippets and file/line context) plus the verification outputs you have (shell/web results).
Without those, I can’t rewrite the comment or apply the correct verification-based updates.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/ItemListService.java (1)
70-75: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/MongoDbAwareService.java (1)
180-191: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/MongoObjectService.java (1)
423-425: LGTM!Also applies to: 452-452
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/MongoService.java (1)
113-115: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/StorageBlockService.java (1)
196-201: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/StoredService.java (1)
177-181: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/ItemStatsService.java (1)
144-146: ⚡ Quick winRequest missing context: Paste the original review comment (inside
<review_comment>...</review_comment>) and the verification outputs/results you have, so the comment can be rewritten accurately.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/TopLevelMongoService.java (1)
207-216: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckinLossTransactionApplier.java (1)
19-20: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckinOutTransactionApplier.java (1)
22-27: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckinPartTransactionApplier.java (1)
18-25: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckinFullTransactionApplier.java (1)
71-83: Need the original review comment to rewrite: Paste the full<review_comment>content (and any verification output/logs you want applied) so I can produce the corrected rewritten comment.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/AppliedTransactionService.java (1)
87-91: ⚡ Quick winMutex wait isn’t unbounded here;
Optional.empty()is for identity, not timeoutThat
Optional.empty()passed toInstanceMutexService.getResource(mutexId, Optional.empty())is theadditionalIdentityslot. This resource always starts locked (getResource(... )callsawaitLock()), andawaitLock()caps the wait usingservice.mutex.await.timeout(default 30s). So the “becalmed forever” risk fromOptional.empty()alone doesn’t hold—only adjusts if the timeout config is set too high.> Likely an incorrect or invalid review comment.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckoutWholeTransactionApplier.java (1)
23-23: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/SetAmountTransactionApplier.java (1)
46-52: LGTM!Also applies to: 70-77
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/SubtractAmountTransactionApplier.java (1)
20-20: LGTM!Also applies to: 67-74
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/SubtractWholeTransactionApplier.java (1)
19-19: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/TransferAmountTransactionApplier.java (1)
21-23: LGTM!Also applies to: 53-53, 64-64, 101-106
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/TransactionApplier.java (1)
31-41: ⚡ Quick winPlease include the original review comment text inside
<review_comment>...</review_comment>(and any relevant diff/snippet) so I can rewrite it in the required format.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/schemaVersioning/upgraders/stored/StoredSchemaUpgrader.java (1)
10-10: LGTM!Also applies to: 22-23
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/items/InventoryItemsStatsTest.java (1)
11-11: LGTM!Also applies to: 21-21, 130-130, 226-226
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/interfaces/endpoints/inventory/items/StoredInItemEndpointsTest.java (1)
14-14: LGTM!Also applies to: 23-23, 140-140, 263-263
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/StoredTest.java (1)
11-11: LGTM!Also applies to: 53-53, 64-64
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/serviceState/db/OqmDatabaseService.java (1)
192-197: ⚡ Quick winMissing context: Provide the original review text inside the
<review_comment>tags (and any code/diff snippets it references) so it can be rewritten correctly.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/schemaVersioning/upgraders/stored/bumpers/StoredItemBumper5.java (1)
39-40: 💤 Low valueProvide the original review comment to rewrite (the content inside
<review_comment>tags).
Cannot rewrite without the original comment text.software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/StoredServiceTest.java (1)
15-15: LGTM!Also applies to: 100-110, 124-134, 148-158, 173-181, 204-213, 227-237, 251-260, 274-283, 302-311, 331-341
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/AddAmountAppliedTransactionTest.java (2)
54-58:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvast! Yer log message be improperly rigged, sailor!
Line 57's log statement be passin' an exception to
log.warn()without a proper placeholder. The logger expects a format string with{}markers, but ye're providin' the exception as a parameter without tellin' it what to do with the beast.⚓ Proper log format, as per the old sea charts
} catch(UnknownTopicOrPartitionException e) { - log.warn("Failed to clear queues", e); + log.warn("Failed to clear queues: {}", e.getMessage(), e); }Or if ye want the full stack trace in yer ship's log:
} catch(UnknownTopicOrPartitionException e) { - log.warn("Failed to clear queues", e); + log.warn("Failed to clear queues", e); }Actually, belay that - the second option be what ye already have. The PMD tool be raisin' a false alarm here, as SLF4J does support passin' the exception as the last parameter without a placeholder. Ye can safely ignore this warnin', or add a
@SuppressWarnings("PMD.InvalidLogMessageFormat")annotation if it bothers ye.
43-342: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/AddWholeAppliedTransactionTest.java (1)
46-294: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/AppliedTransactionServiceTest.java (1)
82-234: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/CheckinFullAmountAppliedTransactionTest.java (1)
52-427: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/CheckinWholeAppliedTransactionTest.java (1)
53-275: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/CheckoutAmountAppliedTransactionTest.java (1)
50-487: LGTM!software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/GeneratedIdentifierGenerationServiceTest.java (1)
244-264: ⚖️ Poor tradeoffProvide the original <review_comment> content (and any verification outputs/results) so it can be rewritten in the required format.
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/CheckoutWholeAppliedTransactionTest.java (1)
49-103: LGTM!Also applies to: 105-159, 161-214, 216-269
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/SetAmountAppliedTransactionTest.java (1)
49-84: LGTM!Also applies to: 87-132, 135-180, 182-196, 198-212, 214-227, 229-245, 247-263, 265-293, 295-309
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/SubtractAmountAppliedTransactionTest.java (1)
43-89: LGTM!Also applies to: 91-139, 141-189, 191-215, 217-241, 243-268, 270-295, 297-337, 339-379, 381-421, 423-463, 465-489
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/TransferAmountAppliedTransactionTest.java (1)
41-113: LGTM!Also applies to: 115-195, 197-278, 280-361, 363-445, 447-520, 523-545, 547-569, 571-603, 605-638, 640-681, 683-724
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/mongo/transactions/transactionApplier/TransferWholeAppliedTransactionTest.java (1)
43-99: LGTM!Also applies to: 101-158, 160-215, 218-273
software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/service/serviceState/InstanceMutexServiceTest.java (2)
184-261: LGTM!
30-51: 💤 Low valueMissing review comment payload
No<review_comment>...</review_comment>content was provided to rewrite—please paste the original review comment (and any related code diff/snippet if included).software/core/oqm-core-api/src/test/java/tech/ebp/oqm/core/api/testResources/data/StoredTestObjectCreator.java (1)
31-41: LGTM!software/core/oqm-core-api/src/test/resources/application.yaml (1)
12-15: LGTM!software/core/oqm-core-base-station/build.gradle (2)
8-8: LGTM!Also applies to: 48-48, 52-52
35-35: ⚡ Quick winRequest the missing inputs to rewrite the review comment: Provide the original
<review_comment>content (and any shell/web outputs you have) so I can rewrite it accurately.software/core/oqm-core-base-station/installerSrc/installerProperties.json (1)
9-9: LGTM!software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/rest/passthrough/inventory/ItemInBlockPassthrough.java (1)
79-79: LGTM!software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/rest/passthrough/inventory/ItemStoredPassthrough.java (1)
73-74: LGTM!software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/ItemCheckoutUi.java (1)
64-64: LGTM!software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/OverviewUi.java (1)
106-112: LGTM!software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/rest/passthrough/inventory/ItemCheckoutPassthrough.java (1)
73-73: 💤 Low valueMissing input: Provide the original review comment inside
<review_comment>...</review_comment>(and any relevant diff/context) so I can rewrite it per the guidelines.software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/interfaces/ui/pages/UiProvider.java (1)
84-88: LGTM!software/core/oqm-core-base-station/src/main/java/tech/ebp/oqm/core/baseStation/service/modelTweak/SearchResultTweak.java (1)
31-37: LGTM!software/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/obj/itemStored/ItemStoredTransaction.js (2)
1193-1193: LGTM!
1798-1798: LGTM!software/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/obj/itemStored/StoredView.js (2)
233-233: LGTM!Also applies to: 239-248
40-86: ⚡ Quick winCheck for lingering calls to renamed
StoredViewhelpers ⚓The old helper names still exist in
software/plugins/mss-controller/mss-controller-plugin/src/main/resources/META-INF/resources/res/js/obj/item/storedView.js(they’re defined and called from within that same file), so this doesn’t yet prove any break against the coreStoredViewyou renamed.Next thing to hunt: any code importing/calling the core
.../obj/itemStored/StoredView.jsversion with the oldgetStorageBlock*names (search for imports/usage of that core module path).software/core/oqm-core-base-station/src/main/resources/templates/tags/search/itemStored/searchResults.html (2)
54-54: LGTM!Also applies to: 99-99
104-104: LGTM!software/core/oqm-core-base-station/src/test/java/tech/ebp/oqm/core/baseStation/testResources/testUsers/TestUser.java (1)
28-33: ⚡ Quick winSmart sailin', removin' them troublesome quotes!
Aye, this be good defensive work here, matey! Stripping out single quotes from the generated username prevents all manner of mischief - SQL injection, string escaping troubles, and command injection if the username ever finds its way into shell commands. Keeps the ship secure and the tests runnin' smooth.
software/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/obj/item/ItemView.js (1)
467-467: ⚡ Quick winFair winds:
stored.state.storageBlockmatches the Stored model’s state shape.
Ye be right to compare viaif (curBlock === stored.state.storageBlock): the backend modelsstorageBlockonStoredInBlock(aStoredState) and migrations/tests place it understate.storageBlock.software/core/oqm-core-base-station/src/main/resources/templates/webui/mainWebPageTemplate.html (1)
207-207: ⚡ Quick winRe-check JS/TS reads of
data-userid: The template now usesdata-useridon#userNameDisplay, and there are no repo occurrences ofdata-userIdnor anygetAttribute("data-userId"/"data-userid")ordataset.userId/dataset.useriddot-notation usage found.software/libs/core-api-lib-quarkus/deployment/pom.xml (1)
11-11: Aye, version bump be shipshape! The parent reference at line 11 sails in lockstep with the parent POM's5.0.0-SNAPSHOTdeclaration. Major-version jump be fittin' for the breaking changes carried in this cargo (REST client return type, removed endpoint, and the like).software/libs/core-api-lib-quarkus/deployment/src/main/java/tech/ebp/oqm/lib/core/api/quarkus/deployment/config/CoreApiLibBuildTimeConfig.java (1)
99-100: Steady as she goes — default image tag follows the new snapshot. Just be sure the matchingdocker.io/ebprod/oqm-core-api:5.0.0-SNAPSHOTimage be published to the registry afore any sailor consuming this extension hoists sail, else their devservices'll find naught but an empty hold.software/libs/core-api-lib-quarkus/integration-tests/pom.xml (1)
8-8: LGTM!software/libs/core-api-lib-quarkus/pom.xml (1)
10-10: LGTM!software/libs/core-api-lib-quarkus/runtime/pom.xml (1)
11-11: LGTM!software/libs/core-api-lib-quarkus/runtime/src/main/java/tech/ebp/oqm/lib/core/api/quarkus/runtime/restClient/OqmCoreApiClientService.java (2)
441-450: 💤 Low valueSummary be tellin' tales —
invItemStoredUpdatestill aboard.The AI-generated summary claims the PUT
invItemStoredUpdatemethod was struck from the manifest, yet here she sits proud at lines 441–450. Either the summary mis-logged the change or the deletion was reverted; worth a sanity check before merging so the ship's log matches the cargo.
136-138: ⚡ Quick winConfirm downstream compilation for
interactingEntityGetSelf’s typed returnArr, the REST client now returns
Uni<InteractingEntity>(instead ofUni<ObjectNode>), so any consumer expecting the old untyped JSON shape will be a proper breaker. Inoqm-core-base-station, theuserInfovalue fed byinteractingEntityGetSelfis already consumed viauserInfo.getRoles()/getId()/getName()in the WebUI templates, with the formerObjectNode-style mapping left commented out—still, check any other downstream callers beyond base-station that may rely on the oldObjectNodeshape.software/plugins/storagotchi/src/main/docker/Dockerfile.native (1)
19-19: ⚡ Quick winCannot rewrite review comment: missing
<review_comment>content and any verification outputs
Provide the original<review_comment>...</review_comment>text (and any shell/web results) so the comment can be rewritten accurately.
…lso stored in list for unique single item viewing
Code Coverage
|
There was a problem hiding this comment.
Actionable comments posted: 1
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-base-station/src/main/resources/META-INF/resources/res/js/obj/item/ItemView.js (1)
455-492:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winEmpty "Stored in:" label when state ain't
StoredInBlock, captain.If
stored.state.storageBlockbe undefined (e.g., the unique stored is not currently in a block — checked out or otherwise), the loop on Line 471 will never match, every block falls through to "Also found in", and theStored in:header span ends up blank with no link beside it. Ye already hide the "Also found in" container when empty (Lines 489–491); consider the symmetric treatment for the "Stored in" span, or render a fallback like "Not currently stored" so the swabbie reading the page knows where the unique item actually be.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/CheckoutAmountTransactionApplier.java (1)
51-54:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard the
AmountStoredcast before it sends a bad request overboard.Matey, Line 54 and Line 63 cast request-selected
Storedrecords straight toAmountStored. IffromStoredpoints at the wrong subtype, this blows up as aClassCastExceptionand turns invalid input into a 500 instead of a clean validation error.⚓ Proposed fix
case BULK -> { if (transaction.getFromBlock() != null) { stored = this.getStoredService().getSingleStoredForItemBlock(oqmDbIdOrName, cs, inventoryItem.getId(), transaction.getFromBlock(), AmountStored.class); } else if (transaction.getFromStored() != null) { - stored = (AmountStored) this.getStoredService().get(oqmDbIdOrName, cs, transaction.getFromStored()); + Stored resolvedStored = this.getStoredService().get(oqmDbIdOrName, cs, transaction.getFromStored()); + if (!(resolvedStored instanceof AmountStored amountStored)) { + throw new IllegalArgumentException("Cannot checkout an amount from a non-amount stored."); + } + stored = amountStored; } else { throw new IllegalArgumentException("No stored or block given to checkout from."); } } case AMOUNT_LIST -> { if (transaction.getFromStored() == null) { throw new IllegalArgumentException("No stored given to checkout from."); } - stored = (AmountStored) this.getStoredService().get(oqmDbIdOrName, cs, transaction.getFromStored()); + Stored resolvedStored = this.getStoredService().get(oqmDbIdOrName, cs, transaction.getFromStored()); + if (!(resolvedStored instanceof AmountStored amountStored)) { + throw new IllegalArgumentException("Cannot checkout an amount from a non-amount stored."); + } + stored = amountStored; }Also applies to: 59-64
🧹 Nitpick comments (3)
software/core/oqm-core-characteristics/app/UIs.py (2)
115-129: 💤 Low valueDuplicate-id check be sweepin' the deck twice for every barrel.
The current implementation filters
uiListfor each entry, makin' it O(n²). With small UI lists this be no kraken, but a single pass with asetmakes the intent clearer and avoids the quadratic dance. Also, raisin' a bareExceptionmakes it hard for callers to catch specifically —ValueErrorwould be more in keeping with Python's customs.♻️ Suggested refactor
`@classmethod` def validate_ui_list(cls, uiList: list[UiCache]) -> None: - # ensure id uniqueness - for uiCache in uiList: - if not uiCache.id: - continue - filtered_list = list( - filter( - lambda - x: uiCache.id == x.id, - uiList - ) - ) - if len(filtered_list) != 1: - raise Exception("Invalid UI ID (duplicates): " + uiCache.id) + # ensure id uniqueness + seen: set[str] = set() + for uiCache in uiList: + if not uiCache.id: + continue + if uiCache.id in seen: + raise ValueError("Invalid UI ID (duplicates): " + uiCache.id) + seen.add(uiCache.id)
149-161: ⚡ Quick winUnkeyed
typefield will scuttle the whole load, matey.
rawUi["type"](Lines 152–158) andrawUi["name"]/rawUi["description"]/rawUi["url"]in__get_uiwill raiseKeyErrorfor any malformed.jsonfile inUIS_DATA_DIR, abortin' the entire cache build and leavin'cls.ui_cacheset toNoneforever — every subsequent call re-tries and re-fails. Consider validatin' each raw file, loggin' and skippin' the bad ones rather than scuttlin' the whole convoy.software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/service/mongo/transactions/appliers/AddAmountTransactionApplier.java (1)
84-107: 💤 Low valueTwo state-checks doin' the work o' one — and a strange message at the second.
The state guard on Lines 84–86 (inside the
AMOUNT_LIST/toStored != nullbranch) protects the cast on Line 90. The very same guard then fires again on Lines 105–107 after the switch closes. For theAMOUNT_LISTpath the second one can never trip; for theBULKpath it be the only check. Either lean on the outer guard and remove the inner one (the cast on Line 90 still needs some check — keep the inner, or hoist it), or document why both exist. As written it reads like one was forgotten when the other was added, and the duplicate error message ("Cannot add to stored that is not stored in a block.") will mask which branch raised it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 34da9a4d-bad4-4093-b27e-42b2f15060a7
📒 Files selected for processing (9)
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/exception/mutex/MutexNotRegisteredException.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/exception/mutex/MutexWaitInterruptedException.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/health/ConfigHealthCheck.javasoftware/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/state/StoredInBlock.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/CheckoutAmountTransactionApplier.javasoftware/core/oqm-core-base-station/src/main/resources/META-INF/resources/res/js/obj/item/ItemView.jssoftware/core/oqm-core-base-station/src/main/resources/templates/webui/mainWebPageTemplate.htmlsoftware/core/oqm-core-characteristics/app/UIs.py
📜 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). (3)
- GitHub Check: CI-Pipeline / Unit Tests
- GitHub Check: CI-Pipeline / Integration Tests
- GitHub Check: CI-Pipeline / Build
🔇 Additional comments (5)
software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/exception/mutex/MutexWaitInterruptedException.java (1)
1-11: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/exception/mutex/MutexNotRegisteredException.java (1)
1-11: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/health/ConfigHealthCheck.java (1)
34-55: LGTM!software/core/oqm-core-api/src/main/java/tech/ebp/oqm/core/api/model/object/storage/items/stored/state/StoredInBlock.java (1)
1-39: LGTM!software/core/oqm-core-base-station/src/main/resources/templates/webui/mainWebPageTemplate.html (1)
1-469: LGTM!
Code Coverage
|
2 participants
Checklist:
Summary by CodeRabbit
New Features
Changes
Removed Features
/media/runBy/{image}endpoint removed