feat: add quarkus-morphium extension as optional module (review only) - #17
Open
Bardioc1977 wants to merge 80 commits into
Open
feat: add quarkus-morphium extension as optional module (review only)#17Bardioc1977 wants to merge 80 commits into
Bardioc1977 wants to merge 80 commits into
Conversation
… retry SingleMongoConnectDriver.connect()'s retry loop logged a full ERROR-level stack trace on every single failed attempt. PoppyDB's ElectionNetworkClient calls connect() repeatedly (once per vote request/heartbeat) for as long as a peer stays unreachable - an entirely expected, already-handled transient condition (ElectionNetworkClient's own callers log failures at debug/trace, treating them as retryable) - so a single down node produced thousands of identical stack traces in the log. Retries now log at debug (message only, no stack trace); only the final, retries-exhausted failure gets a single WARN with the real cause attached. Tests: SingleMongoConnectDriverConnectLoggingTest (3/3, captures the logger via a ListAppender - same pattern as InMemoryDriverSlowQueryTest).
…ARY forever processReplSetGetStatus() labeled any non-leader peer SECONDARY purely from the static configured peer list, regardless of actual reachability - a genuinely down node stayed SECONDARY forever instead of reflecting the leader's own heartbeat-tracked knowledge that it had gone silent. ElectionManager#isPeerReachable() reuses the same freshness window already used for priority-takeover eligibility (peerLastContact, floor 2000ms). Only meaningful on the leader (the only role that actively heartbeats every peer); a follower has no independent knowledge of other followers' liveness and reports optimistically. A peer never yet contacted (e.g. right after an election) is also treated as reachable, so a startup race can't falsely flag a healthy peer DOWN - only a peer that WAS reachable and has since gone stale is reported so, matching real MongoDB's own state=8/stateStr=DOWN exactly. Tests: ReplSetGetStatusDownPeerTest (2-node real cluster, kills the follower, polls for DOWN) + full ReplSetGetStatusTest suite (no regression - a healthy peer still reports SECONDARY).
…ds see a stale fault mode Round-1 task review found a reproducible race (5/9 local runs) in WireProxy.stop(): the severing loop only covers the liveSockets snapshot, and the accept thread can be mid-handleNewConnection -> startForwarding for a connection not yet registered when that loop runs. startForwarding() never checked running before connecting to the backend and spawning pump threads, and both pump threads' finally blocks severed with faultMode.get() (whatever mode happened to be configured, passthrough by default) instead of a forced reset - so a racing client could see a clean EOF instead of the hard reset stop()'s own javadoc promises. Fix: - startForwarding() now checks running before connecting to the backend at all; if stop() already flipped it, sever the client with FaultMode.reset immediately instead of forwarding. - Both pump threads' finally blocks now sever with `running ? faultMode.get() : FaultMode.reset` so a pump thread that starts or is already running during/after stop() always produces a hard reset for its client, regardless of the configured fault mode. Also adds the missing close-on-existing-connection test (closeOnAnExistingConnectionSeversItWithoutReset), mirroring the existing reset test but asserting a clean EOF instead of an IOException, per the review's Important finding. Verified with 10 consecutive runs of 'mvn -o test -pl morphium-core -Dtest=WireProxyTest', all green (10/10 tests each, 100 total test executions, 0 failures).
…EOF close command() now throws MorphiumDriverException when sendAndWaitForReply returns null (peer closed cleanly at a message boundary instead of replying, e.g. some replSetStepDown paths) rather than NPEing on reply.getFirstDoc(). This also fixes commandTolerateClose, which only caught MorphiumDriverException. Added a canned backend variant that replies to the connect() hello and then closes without answering the next request, reproducing the clean-EOF path (WireProtocolMessage#parseFromStream returns null on EOF, not an exception) and asserting commandTolerateClose returns null instead of throwing. Also fixed a typo in a test name (Assoon -> AsSoon).
…rimary re-discovery and dead code
- runWriteReadScenario: move writer/reader join() into the finally block (was after the try, so an assertTrue failure skipped the join entirely) - messagingRecoversAfterFailover: widen the try/finally to cover sender.start()/receiver.start() and the sleep/escape-guard in between, so sender.terminate()/receiver.terminate() run on every exit path, not just the happy path past the inner workload try - generalize the tearDown() fallback safety net from a single writerThread field to a trackedThreads list, and register every scenario's workload thread(s) into it before start() (writesRecoverAfterFreeze's writer, runWriteReadScenario's writer+reader, messagingRecoversAfterFailover's sendThread)
…riverFailoverProxyTest
C2: widen only the freeze scenario's post-fault recovery window 10s -> 25s, to clear PooledDriver's ~12s host-eviction floor (Host.MAX_FAILURES=5 x 2s bounded-hello) with real margin, while staying well under maxWaitTime (60s). I1: assert the escape guard (assertOnlyConnectedThroughProxies) again after recovery in all 5 scenarios, not just before the fault - the post-failover call is the one that actually catches a rewrite gap in the driver's post-election discovery. I2: pollForNewPrimary reuses one ControlChannel across polls instead of reconnecting every 200ms tick, reconnecting only when a poll attempt throws; wires in the previously-unused ControlChannel.poll(). I3: extract setupProxiedBackend()/injectFaultOnCurrentPrimary() helpers, replacing the copy-pasted discovery/proxy-wiring and fault-injection blocks across all 5 scenarios. I4: WireProxy.stop() now tracks and joins its c2b/b2c pump threads, not just the accept thread, guaranteeing they've exited before stop() returns. I5: ControlChannel.command() now checks the ok field (ConnectionClosed- WithoutReplyException distinguishes 'connection closed instead of replying' from a real ok:0 refusal); commandTolerateClose no longer swallows ok:0 replies. members() throws with a diagnostic message instead of an unchecked-cast NPE if 'members' is absent. Adds two ControlChannelTest cases for the ok:0 path. Minors: drop a no-op setFirstDoc in AddressRewriter (M1a); import WireProtocolMessage instead of fully-qualifying it in Slf4jFrameObserver (M1b); fix a wrong 'MorphiumMessaging extends Thread' comment (M3a); replace a stale 'Task 6' plan reference with a CHANGELOG pointer (M3b); document that WireProxy's freeze mode is permanent per-connection (M5); guard WireProxy.pumpBackendToClient's observer-context construction against a closed client socket (M8). Verified by mvn test-compile (whole module) and by running WireProxyTest/AddressRewriterTest/ControlChannelTest (21/21 green, ControlChannelTest gained the 2 new I5 tests). DriverFailoverProxyTest itself cannot be run in this environment (no real RS reachable).
A real run on testrunner.fritz.box found writeOk recovering after every fault while readOk/received stayed flat forever - all 3 write+read/ messaging scenarios failed this way. Root cause: PooledDriver. borrowConnection()'s InterruptedException handling (during host eviction mid-failover) sets Thread.currentThread().interrupt() before throwing. That leaves the *caller's* interrupt status set; each worker thread's own catch (Throwable) around the read/send call doesn't clear it, so the very next Thread.sleep() in the loop immediately re-threw InterruptedException too - and the naive "catch -> return" there silently, permanently ended the thread on the very first such event, with no error ever surfacing beyond the recovery assertion failing much later. All four worker-thread loops (freeze-writer, writeread-writer, writeread-reader, messaging-sender) now only stop on an actual shutdown request (running.get()==false); a spurious interrupt is absorbed and the loop continues. writesRecoverAfterFreeze and connectAfterElectionSucceedsWithoutTheOldPrimary already passed cleanly against a real 3-node PoppyDB replica set on testrunner.fritz.box before this fix; re-verifying the other three scenarios next.
… stuck reads outlast every recovery window Confirmed via a second real run on testrunner.fritz.box: the interrupt- handling fix alone did not change the readOk-never-recovers symptom. Actual root cause is DriverSettings' 30s serverSelectionTimeout default: PooledDriver.getReadConnection()'s PRIMARY case (and borrowConnection()'s poll loop underneath it) can block for the full 30s while the primary is being re-resolved after a fault - and, unlike WriteMongoCommand, has no retry-with-shorter-timeout loop of its own. A read stuck in that single 30s wait is indistinguishable from "reads never recover" within a 10-25s test window, because the reader thread's own 200ms retry loop never gets a chance to run again until the wait finally gives up.
…d of a one-shot lookup Third real run on testrunner.fritz.box surfaced a NoSuchElementException: connectAfterElectionSucceedsWithoutTheOldPrimary ran right after a preceding scenario's own fault/election left the fresh 3-node cluster without a settled primary yet, and the one-shot findFirst().orElseThrow() had no tolerance for that short window. Now polls up to 10s via the already-wired ControlChannel.poll() before injecting the fault, throwing a clear IllegalStateException only if no primary ever appears.
…oll windows to 25s Fourth real run on testrunner.fritz.box surfaced a genuine TOCTOU race: connectAfterElectionSucceedsWithoutTheOldPrimary hit 'not primary so can't step down' (code 10107) because the primary changed between injectFaultOnCurrentPrimary's discovery and the stepDownPrimary call landing - stepDownPrimary's own doc comment previously (incorrectly) claimed this race was structurally impossible. It can happen on a shared, already-disrupted 3-node cluster (this is the 4th/5th scenario to run against the SAME cluster the earlier scenarios already stepped down repeatedly). New faultAndStepDownCurrentPrimary() retries the whole discover-fault- stepDown sequence up to 3 times, clearing the stale fault from the wrongly-guessed proxy before each retry so at most one proxy is ever faulted at once. All 4 scenario call sites switched to it. Also widened pollForNewPrimary's 15s budget to 25s across all 4 call sites: writesRecoverAfterFreeze - always the first scenario to run, against a just-started cluster - flaked twice on election timing after the earlier fixes narrowed the failure surface down to just this, despite the freeze mechanics themselves working correctly whenever the election completed promptly. Still well under maxWaitTime (60s).
…sRecoverAfterFreeze's consistent election failure
4/4 consecutive runs on testrunner.fritz.box failed writesRecoverAfterFreeze
specifically (always the first scenario to run, seconds after the 3-node
cluster starts) on 'no new primary elected', while every other scenario -
running later against an already-settled cluster - passed reliably.
PoppyDB's processReplSetStepDown returns ok:0 ('no eligible secondary
caught up') when !force - now correctly surfaced as a thrown exception
by the I5 fix rather than silently swallowed. Adding force:true removes
this as a candidate cause; if the election still doesn't complete after
this, the actual mechanism needs closer investigation (possibly a
housekeeping/heartbeat interaction with frozen-but-open connections
unrelated to stepDown's own success/failure).
…k 30s under concurrent test load Server-side PoppyDB logs from testrunner.fritz.box (4 more runs, force:true did NOT change the outcome) show the actual mechanism clearly: 4 of 5 elections per run complete in 3-4s, matching ElectionConfig's own 2-4s randomized timeout default - but exactly one, different each run, takes ~30s. Not freeze-specific (force:true ruled that out) and not a randomized-timeout artifact (30s is way outside the 2-4s configured range) - most likely resource contention from the 5 scenarios' proxy/ connection machinery all running sequentially against the same shared local 3-node cluster. This is infrastructure/scheduling variance in the test environment, not a driver or election-protocol correctness issue - the actual leader-election mechanism works correctly in every observed case, just occasionally slower than 25s under this specific concurrent load. 40s covers the observed worst case with real margin while staying well under maxWaitTime (60s).
|
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: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b617de4b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| @@ -0,0 +1 @@ | |||
| de.caluga.morphium.quarkus.deployment.MorphiumProcessor | |||
There was a problem hiding this comment.
With this descriptor listing only MorphiumProcessor, Quarkus only loads that build-step class from the deployment artifact. The new MorphiumDataProcessor, MorphiumDevServicesProcessor, MorphiumMigrationProcessor, and MorphiumDevUIProcessor classes all contain @BuildSteps but are not referenced here, so consumers won't get generated repository beans, Dev Services, migrations, or the Dev UI once the extension is packaged. Add each build-step class to the descriptor (or let the extension processor generate a complete one) so the advertised extension features run.
Useful? React with 👍 / 👎.
Comment on lines
+71
to
+73
| if (ConfigUtils.isPropertyNonEmpty("quarkus.morphium.hosts")) { | ||
| log.debug("Morphium connection settings already configured – skipping Dev Services"); | ||
| return null; |
There was a problem hiding this comment.
In dev/test mode with quarkus.morphium.driver-name=InMemDriver and no quarkus.morphium.hosts (the documented no-Docker test setup), this gate still falls through and starts a MongoDB Testcontainers container. That makes Dockerless tests fail before the runtime producer can use the in-memory driver unless users also discover quarkus.morphium.devservices.enabled=false; treat an explicit InMemDriver selection as an already-configured connection and return here.
Useful? React with 👍 / 👎.
Comment on lines
+1041
to
+1042
| if (conditionsSpec.length() > 0) conditionsSpec.append(","); | ||
| conditionsSpec.append(fieldName).append(":").append(i); |
There was a problem hiding this comment.
For an @Find method such as the documented @By("price") @Is(GreaterThanEqual) double minPrice, the generated spec records only price:<index> and drops the @Is operator. FindMethodBridge interprets every encoded condition as eq(...), so these methods return only exact matches instead of >=, <, LIKE, etc.; include the operator in the spec and have the bridge apply it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR introduces quarkus-morphium as a new optional Quarkus CDI extension module in the Morphium reactor (runtime + deployment + testing + integration-tests), wires it into the parent build/profile, and updates release bundling and documentation accordingly.
Changes:
- Adds the Quarkus extension modules (
quarkus-morphiumparent + runtime/deployment/testing + integration-tests) including health checks, Dev Services/Dev UI integration, JSON (de)serializers, and migration annotations/processors. - Updates build/release plumbing (root
pom.xmlprofile +release.sh) to include the new published artifacts and the special-case parent POM artifact. - Extends documentation (MkDocs + Antora docs under
quarkus-morphium/docs) and adds aCHANGELOG.mdentry for the new module.
Reviewed changes
Copilot reviewed 134 out of 134 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| release.sh | Adds new Quarkus extension artifacts to the release bundle and stages additional reactor POMs. |
| pom.xml | Centralizes quarkus.version and enables quarkus-morphium in the extensions profile. |
| mkdocs.yml | Adds Quarkus extension doc page to site navigation. |
| docs/index.md | Lists Quarkus extension as a new optional module. |
| CHANGELOG.md | Documents the new quarkus-morphium optional module and migration notes. |
| quarkus-morphium/pom.xml | Introduces quarkus-morphium-parent aggregator with Quarkus BOM import and plugin mgmt. |
| quarkus-morphium/testing/pom.xml | Adds quarkus-morphium-testing utility module. |
| quarkus-morphium/testing/src/main/java/de/caluga/morphium/quarkus/testing/InMemMorphiumTestProfile.java | Provides a Quarkus test profile for Morphium InMemDriver runs. |
| quarkus-morphium/runtime/src/main/resources/META-INF/quarkus-extension.yaml | Declares Quarkus extension metadata (name, guide link, config prefix). |
| quarkus-morphium/runtime/src/main/resources/META-INF/native-image/io.quarkiverse.morphium/quarkus-morphium/native-image.properties | Adds native-image builder JVM args for JOL/module exports (path currently still uses old groupId). |
| quarkus-morphium/runtime/src/main/resources-filtered/META-INF/morphium-version.properties | Adds filtered version properties for runtime introspection. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/CacheConfig.java | Adds cache-related config mapping group. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/LocalDateTimeConfig.java | Adds LocalDateTime storage behavior config mapping. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/SslConfig.java | Adds TLS/X.509 configuration group for Morphium connections. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/MorphiumVersion.java | Adds runtime-accessible version reporting via filtered properties. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/MorphiumDevUIJsonRpcService.java | Adds Dev UI JSON-RPC service to show runtime connection info with URI sanitization. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/data/QuarkusMorphiumRepository.java | Adds Quarkus CDI-injected base class for Gizmo-generated repositories. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/json/MorphiumIdJacksonModule.java | Adds Jackson customizer to serialize/deserialize MorphiumId as hex string. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/json/MorphiumIdJsonbAdapter.java | Adds JSON-B adapter for MorphiumId hex string mapping. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/json/MorphiumIdJsonbModule.java | Adds JSON-B config customizer to register the adapter when JSON-B is present. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/health/MorphiumLivenessCheck.java | Adds MicroProfile liveness probe for Morphium connection. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/health/MorphiumReadinessCheck.java | Adds readiness probe with best-effort driver stats metadata. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/health/MorphiumStartupCheck.java | Adds startup probe with SRV-discovery-tolerant “ever connected” latch. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/transaction/MorphiumTransactional.java | Adds interceptor binding annotation for Morphium transactions. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/transaction/MorphiumTransactionEvent.java | Adds CDI event type for transaction lifecycle phases. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/transaction/MorphiumTxPhase.java | Adds CDI qualifier to observe transaction events by phase. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/migration/MorphiumChangeUnit.java | Adds annotation to declare migration units. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/migration/Execution.java | Adds annotation to mark migration execution methods. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/migration/RollbackExecution.java | Adds annotation to mark migration rollback methods. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/migration/MorphiumMigrationConfig.java | Adds migration config mapping group. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/migration/MorphiumMigrationEntry.java | Adds migration changelog entity. |
| quarkus-morphium/runtime/src/main/java/de/caluga/morphium/quarkus/migration/MorphiumMigrationLock.java | Adds distributed migration lock entity. |
| quarkus-morphium/runtime/src/test/java/de/caluga/morphium/quarkus/json/MorphiumIdJacksonModuleTest.java | Unit tests for Jackson MorphiumId (de)serialization behavior. |
| quarkus-morphium/runtime/src/test/java/de/caluga/morphium/quarkus/json/MorphiumIdJsonbAdapterTest.java | Unit tests for JSON-B MorphiumId adapter behavior. |
| quarkus-morphium/runtime/src/test/java/de/caluga/morphium/quarkus/health/MorphiumStartupCheckTest.java | Unit tests for startup-check latch semantics. |
| quarkus-morphium/integration-tests/pom.xml | Adds Quarkus integration-test module wiring (REST, health, optional testcontainers). |
| quarkus-morphium/integration-tests/src/main/resources/application.properties | Configures integration-test app to use InMemDriver and disable Dev Services. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/AddressEmbedded.java | Embedded doc used for integration tests. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/AddCategoryMigration.java | Migration test change-unit example. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/CustomerEntity.java | Entity with embedded address for mapping tests. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/FailingMigration.java | Migration test that forces rollback behavior. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/InitItemsMigration.java | Migration test change-unit example inserting initial data. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/ItemEntity.java | Baseline entity used across integration tests. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/ItemRepository.java | Jakarta Data repository interface for CRUD/query tests. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumCrudTest.java | End-to-end Morphium CRUD tests under Quarkus. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataAggregateTest.java | Integration tests for JDQL aggregate functions. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataAsyncTest.java | Integration tests for CompletionStage-based async repository methods. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataCountFieldTest.java | Integration tests for COUNT(field) null filtering. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataDeleteTest.java | Integration tests for delete derivation and deleteAll(). |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataGroupByPageTest.java | Integration tests for GROUP BY queries with pagination totals. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataGroupByTest.java | Integration tests for GROUP BY queries and record mapping. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataHavingOrTest.java | Integration tests for HAVING OR support. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataMorphiumRepositoryTest.java | Integration tests for MorphiumRepository escape-hatch APIs. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataPaginationTest.java | Integration tests for pagination/sorting basics. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataParenGroupTest.java | Integration tests for parenthesized group conditions in JDQL. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataProjectionTest.java | Integration tests for JDQL SELECT projections. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDataStreamTest.java | Integration tests for Stream-returning repository methods. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDevServicesConfigTest.java | Validates Dev Services config defaults/behavior in test profile. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumDevServicesReplicaSetConfigTest.java | Validates replica-set flag override presence without starting containers. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumEmbeddedTest.java | Integration tests for embedded document persistence. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumEntityRegistryTest.java | Verifies build-time entity pre-registration works at runtime. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumHealthCheckDisabledTest.java | Verifies health checks can be disabled via build-time config. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumHealthCheckTest.java | End-to-end tests for /q/health/* endpoints and probe metadata. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumIdEntity.java | Entity with MorphiumId primary key for JSON-wire tests. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumIdJsonSerializationTest.java | REST-level acceptance tests for MorphiumId JSON wire format. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumIdResource.java | Minimal REST resource used by JSON-wire tests. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumInMemProfileTest.java | Verifies quarkus-morphium-testing profile behavior and CRUD flow. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumInjectionTest.java | Verifies CDI injection of Morphium and baseline runtime config. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumItemRepository.java | Repository interface extending MorphiumRepository for feature tests. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumLocalDateTimeTest.java | Integration tests for LocalDateTime mapper behavior. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/MorphiumVersionTest.java | Tests for optimistic locking/version behavior. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/OrderEntity.java | Integration-test entity used for query derivation/JDQL tests. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/PaginatedOrderRepository.java | Repository interface for keyset pagination tests. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/StatusCount.java | Record DTO for group-by count results. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/StatusCustomerCount.java | Record DTO for group-by customer/status counts. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/StatusStats.java | Record DTO for group-by count + sum stats. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/TransactionEventCollector.java | CDI observer collecting transaction events for assertions. |
| quarkus-morphium/integration-tests/src/test/java/de/caluga/morphium/quarkus/it/TransactionalService.java | CDI service exercising @MorphiumTransactional in tests. |
| quarkus-morphium/docs/antora.yml | Adds Antora component descriptor for extension docs. |
| quarkus-morphium/docs/modules/ROOT/nav.adoc | Adds Antora navigation tree for extension docs. |
| quarkus-morphium/docs/modules/ROOT/pages/index.adoc | Adds extension documentation home page. |
| quarkus-morphium/docs/modules/ROOT/pages/dev-services.adoc | Documents Dev Services behavior and Dev UI card. |
| quarkus-morphium/docs/modules/ROOT/pages/health-checks.adoc | Documents liveness/readiness/startup probes and usage. |
| quarkus-morphium/docs/modules/ROOT/pages/includes/attributes.adoc | Defines shared doc attributes (versions, URLs). |
| quarkus-morphium/deployment/pom.xml | Adds deployment-side Quarkus build-time dependencies and APT setup. |
| quarkus-morphium/deployment/src/main/resources/META-INF/quarkus-build-steps.list | Registers Morphium build step class with Quarkus extension metadata. |
| quarkus-morphium/deployment/src/main/resources/dev-ui/qwc-morphium-connection.js | Adds Dev UI web component to render connection info grid. |
| quarkus-morphium/deployment/src/main/java/de/caluga/morphium/quarkus/deployment/MongoDBStartable.java | Adds Testcontainers wrapper for Dev Services MongoDB startup. |
| quarkus-morphium/deployment/src/main/java/de/caluga/morphium/quarkus/deployment/MorphiumDevServicesBuildTimeConfig.java | Adds build-time config mapping for Dev Services. |
| quarkus-morphium/deployment/src/main/java/de/caluga/morphium/quarkus/deployment/MorphiumDevUIProcessor.java | Registers Dev UI card and JSON-RPC provider in dev mode. |
| quarkus-morphium/deployment/src/main/java/de/caluga/morphium/quarkus/deployment/MorphiumEntitiesRegisteredBuildItem.java | Adds ordering marker build item for entity registration replay. |
| quarkus-morphium/deployment/src/main/java/de/caluga/morphium/quarkus/deployment/MorphiumFeature.java | Adds marker build item for feature registration. |
| quarkus-morphium/deployment/src/main/java/de/caluga/morphium/quarkus/deployment/MorphiumHealthBuildTimeConfig.java | Adds build-time config mapping to enable/disable health checks. |
| quarkus-morphium/deployment/src/main/java/de/caluga/morphium/quarkus/deployment/MorphiumMigrationProcessor.java | Adds build-time migration discovery/registration and runtime execution trigger. |
| quarkus-morphium/deployment/src/main/java/de/caluga/morphium/quarkus/deployment/RepositoryBuildItem.java | Adds build item representing discovered Jakarta Data repositories. |
| quarkus-morphium/deployment/src/test/java/de/caluga/morphium/quarkus/deployment/MorphiumDevServicesProcessorTest.java | Adds unit tests for Dev Services container-type contract and config equality. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+63
to
+65
| } catch (Exception e) { | ||
| return builder.down().withData("error", e.getMessage()).build(); | ||
| } |
Comment on lines
+9
to
+10
| NOTE: The extension already includes `quarkus-smallrye-health` as a transitive dependency. | ||
| No additional dependency is needed — health endpoints are available by default. |
Comment on lines
+1
to
+4
| # JOL (Java Object Layout) is used by Morphium's InMemoryDriver for memory size estimation. | ||
| # org.openjdk.jol.vm.sa.ServiceabilityAgentSupport references sun.management.VMManagement | ||
| # which requires a module export for the native-image builder JVM. | ||
| Args = -J--add-exports=java.management/sun.management=ALL-UNNAMED |
Comment on lines
+85
to
+88
| return builder.build(); | ||
| } catch (Exception e) { | ||
| return builder.down().withData("error", e.getMessage()).build(); | ||
| } |
Comment on lines
+46
to
+49
| return builder.status(connected).build(); | ||
| } catch (Exception e) { | ||
| return builder.down().withData("error", e.getMessage()).build(); | ||
| } |
Comment on lines
+92
to
+97
| @Test | ||
| @DisplayName("Entity without @Version stores and updates normally") | ||
| void entityWithoutVersion_worksNormally() { | ||
| // UnversionedEntity re-uses ItemEntity but version field is just 0 by default. | ||
| // We use a different approach: test that two stores on the same entity don't fail. | ||
| var item = new ItemEntity(); |
Comment on lines
+29
to
+34
| * <ul> | ||
| * <li>{@code GET /morphium-id/entity} returns a {@link MorphiumIdEntity} — proves | ||
| * outbound serialization emits {@code "id":"<hex>"} instead of the struct.</li> | ||
| * <li>{@code GET /morphium-id/echo/{id}} echoes back a {@code MorphiumId} path | ||
| * param — proves inbound deserialization parses the hex string.</li> | ||
| * </ul> |
Owner
Author
|
@coderabbitai review |
|
First real MongoDB-RS run (mongo1/mongo2.fritz.box + a 3rd voting arbiter, mongoarb.fritz.box - discovered via replSetGetStatus/rs.conf(), not something the earlier PoppyDB-only local testing ever exercised) surfaced a new failure: 'No primary node found - not connected yet?' thrown from PooledDriver.connect() during the very first Morphium construction, before any fault was even injected. wireProxies() proxies every member replSetGetStatus reports, arbiter included - correct, since the address rewriter needs to translate the arbiter's name too wherever another member's hello reply mentions it. But buildDriverUnderTest() then seeded ALL of those proxy addresses, including the arbiter's - which holds no data and can never become primary. PooledDriver occasionally tried the arbiter's proxy first during initial primary discovery and gave up before ever reaching a real data-bearing seed. Arbiters (stateStr=="ARBITER") are now tracked separately and excluded from the seed while remaining fully proxied/rewritten.
Bardioc1977
pushed a commit
that referenced
this pull request
Aug 5, 2026
…ative-image groupId path Only MorphiumProcessor was listed in META-INF/quarkus-build-steps.list, the mechanism Quarkus actually uses to load deployment build-step classes (ServiceUtil.classesNamedIn(...)). MorphiumDataProcessor (Jakarta Data repository generation), MorphiumDevServicesProcessor, MorphiumMigrationProcessor, and MorphiumDevUIProcessor were never loaded, silently disabling most of the extension's advertised features. All five processors are now listed. Also fixes META-INF/native-image/io.quarkiverse.morphium/quarkus-morphium/ to META-INF/native-image/de.caluga/quarkus-morphium/ (GraalVM only picks up native-image.properties when the directory matches <groupId>/<artifactId>, and the groupId moved to de.caluga in the M3/M4 migration). Additionally, Dev Services now skips container startup when quarkus.morphium.driver-name is explicitly set to a non-production driver (e.g. InMemDriver): previously only quarkus.morphium.hosts was checked, so a Dockerless InMemDriver test setup still started a MongoDB container before the runtime producer could apply the driver override. Verified: integration-tests now starts exactly one container (for the one test that actually needs a real replica set), down from one per test run before. Reported by GitHub Copilot review on PR #17 (Bardioc1977/morphium).
Bardioc1977
pushed a commit
that referenced
this pull request
Aug 5, 2026
… 1.1, not 1.0.0 The README and Antora jakarta-data page showed @by("price") @is(GreaterThanEqual) as a supported example, but jakarta.data.repository.Is does not exist in jakarta.data-api 1.0.0 (the stable version this module targets) — it was only added in Jakarta Data 1.1, which has not shipped a final release yet (latest available artifact is the 1.1.0-M3 milestone). The example would not compile against this module's actual dependency. Corrected the example to a plain equality @by parameter and added a note pointing to query derivation or @query (JDQL) for non-equality conditions today, with @is noted as a natural follow-up once Jakarta Data 1.1 finalizes. Reported by GitHub Copilot review on PR #17 (Bardioc1977/morphium).
…- root cause of cross-scenario failures on real MongoDB Root-caused via mongod's own structured server logs on mongo1/mongo2/ mongoarb.fritz.box (live-tailed during a full 5-scenario run): only 2 of the 3 replica set members can ever actually become primary (the 3rd is a non-data-bearing arbiter). replSetStepDown's numeric argument blocks the stepped-down node from being re-elected for that many seconds - with it set to 60 and scenarios running well under a minute apart, a LATER scenario's stepdown could land while the EARLIER scenario's target was still inside its own 60s block, leaving BOTH electable members simultaneously unable to win an election. The replica set then had no possible primary until one of the two overlapping blocks expired - confirmed directly in the logs: repeated 'Not starting an election... since we are not electable' from both nodes for 30-90+ seconds at a stretch, exactly matching the observed test failures (different scenarios failed on different runs, whichever one happened to land in an overlap window). This never surfaced against the local PoppyDB test cluster because all 3 of its members are equally electable - there's always at least one unblocked candidate even mid-block, so blocks stacking never mattered there. 15s is comfortably longer than any single scenario needs to observe the primary change at least once, but short enough that consecutive scenarios' blocks stop stacking against only 2 real candidates.
…usiness method aroundInvoke's single try/catch wrapped both ctx.proceed() (the business method) and safeCommit() in the same block. A transient error from the commit itself (e.g. code 251/NoSuchTransaction after a primary failover, where the server actually committed but the reply was lost) hit the exact same retry path as a transient error from the business method: attempt < maxRetries && isTransientTransactionError(e) -> continue, re-running ctx.proceed() from scratch. Every write the method made would be applied a second time inside a brand-new transaction -- MongoDB drivers deliberately retry only the commit for this exact scenario, never the statements that already ran. Split the loop body: ctx.proceed() has its own try/catch (unchanged retry- the-whole-method semantics for a transient error from the business logic itself), and the commit is now wrapped by the new safeCommitWithRetry(), which retries ONLY safeCommit() up to 3 times with the same exponential backoff, never re-invoking ctx.proceed(). No dedicated new test: safeCommitWithRetry() is private and needs a real Morphium + InvocationContext to exercise directly, and no mocking framework is available anywhere in this reactor (same constraint noted on the previous commit's Throwable fix). Verified via the existing test suite (35/35 green, including the unaffected isTransientTransactionError tests this logic depends on) and a clean runtime module build. Merge blocker #5 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
…ad of failing on a held lock Two related bugs in the distributed migration lock: 1. The lock's expires_at was set once by acquireLock() and never renewed. A migration run taking longer than lock-ttl-seconds let a second instance atomically steal the lock (acquireLock()'s expires_at <= now condition would match) and start running the SAME still-in-progress change units concurrently -- the one scenario the TTL was supposed to prevent (crashed-process deadlocks), not enable. Added renewLock(), called after every executed migration, owner-guarded so it's a silent no-op once another process has genuinely taken over. 2. A held lock failed startup immediately with no retry. In a k8s rolling deployment with multiple replicas, every replica except the one that won the lock race would crash-loop until the migration run finished and released the lock, instead of simply waiting their turn. Added a new quarkus.morphium.migration.lock-wait-seconds property (default 0 -- unchanged fail-immediately behavior) and acquireLockWithWait(), which polls every second up to the configured timeout before giving up. Adds two integration tests: one proving the lock survives past its original short TTL across a deliberately slow migration (renewal), one proving a second runner waits for and then acquires a lock released mid-wait by another instance instead of failing immediately. Merge blocker #6 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
MorphiumDevServicesProcessor had no Docker guard at all before attempting to start a MongoDB container, unlike Quarkus's own DevServicesMongoProcessor (which gates on DockerStatusBuildItem). Without a running Docker daemon, the container start attempt throws mid-augmentation and fails the whole build -- the PR description's claim that the module needs no Docker for its own tests only held because MorphiumTransactionalTest separately works around this with a @BeforeAll Docker check, but @BeforeAll runs AFTER QuarkusTestExtension has already booted the app (and thus already attempted to start Dev Services) -- there was no equivalent guard at the point the container would actually be started. Injects the already-available DockerStatusBuildItem (produced by Quarkus core's own DockerStatusProcessor, no new dependency needed) and returns null early with a warning when Docker isn't available, mirroring DevServicesMongoProcessor's approach. Verified: full reactor build green, all 16 existing deployment-module unit tests green (no Docker needed for those), and the full 247-test Docker integration-tests suite (which does have Docker available in this environment) still green -- confirming the guard doesn't affect the happy path where Docker IS available. Merge blocker #7 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
…oo early MorphiumTransactionalInterceptor had no detection at all for asynchronous return types (CompletionStage, Mutiny's Uni). For a @MorphiumTransactional method declared to return one of these (or that delegates to a repository's doXxxAsync methods without awaiting them), ctx.proceed() returns the CompletionStage/Uni object itself immediately -- the method's actual async work (typically scheduled on repo.getAsyncExecutor() or a Mutiny scheduler) hasn't run yet. The interceptor would then fire BEFORE_COMMIT, commit, and fire AFTER_COMMIT right away, well before the real database writes happen -- a transaction committed with none of its intended writes inside it. There is no reliable way for this synchronous CDI interceptor to hook "when the returned CompletionStage/Uni completes" without deliberately redesigning what @MorphiumTransactional does (out of scope here) -- so it now fails fast with a clear UnsupportedOperationException instead of silently doing the wrong thing. Detection is by class name for Mutiny's Uni (not a compile-time dependency of this module) and by assignability for CompletionStage. Adds isAsyncReturnType() unit tests (CompletionStage, CompletableFuture, void, and a plain return type). Verified: 39/39 tests green in the runtime module (15/15 in the affected test class, up from 11). Merge blocker #8 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
…g properties configuration.adoc and README.md documented quarkus.morphium.create-indexes, which does not exist anywhere in MorphiumRuntimeConfig -- the real property is index-check, with four string values (create-on-startup, warn-on-startup, create-on-write-new-col, no-check), not the boolean the docs implied. This also broke the '"all quarkus.morphium.* properties are unchanged" migration promise for users coming from io.quarkiverse.morphium:quarkus-morphium:1.2.0. configuration.adoc's own claim to document 'every available property' also missed 10 real properties, verified against MorphiumRuntimeConfig and MorphiumMigrationConfig: max-wait-time, default-query-timeout-ms, replica-set-name, connect-retries, and the entire migration.* group (migrate-at-start, change-log-collection, lock-collection, lock-ttl-seconds, lock-wait-seconds -- the last of these added by the blocker #6 fix). Added all of them, plus a corrected description for index-check, to both configuration.adoc and README.md's property table. Merge blocker #9 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
… thread MorphiumBlockingCallDetector injected Morphium directly (@Inject Morphium morphium) and dereferenced it inside its @observes StartupEvent handler, which runs on the application boot thread. That dereference triggers MorphiumProducer.buildMorphium() -- a blocking connect with a full retry ladder -- defeating MorphiumProducer's own deliberate lazy-init design (the Morphium bean is meant to connect on first real use, not eagerly at boot) and delaying application startup (including the startup health check becoming ready) by however long the connect takes. Switched to Instance<Morphium> and moved the actual listener registration onto a background daemon thread, keeping the boot thread free. Trade-off: a write in the first few milliseconds after startup completes could theoretically happen before this listener registers -- only means this detector's own warning would be missed for that one write, nothing breaks. Verified: full reactor build green, all 39 runtime-module tests green (no regression), full 247-test Docker integration-tests suite green. Should-fix #1 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
MorphiumLivenessCheck reported DOWN whenever driver.isConnected() was false, i.e. on any MongoDB outage -- but a DOWN liveness probe makes Kubernetes restart the pod, and restarting the application process does nothing to fix an unreachable MongoDB server. Worse, since every replica in the deployment loses connectivity to the same outage simultaneously, this restarts the entire deployment at once, taking the application fully offline until the outage resolves instead of just failing gracefully. MorphiumReadinessCheck already has the correct semantics for this (its own Javadoc explicitly documents the rationale: pool/connectivity issues belong in readiness, which removes the pod from the Service's endpoint list without killing it, and automatically re-adds it once the connection recovers) -- liveness now only reports DOWN if the Morphium bean itself is unusable (e.g. a misconfiguration), never on a lost connection. No test changes needed: the existing MorphiumHealthCheckTest only asserts the liveness check is UP in the normal (connected) case, which this change doesn't affect. Should-fix #2 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
…ist PooledDriver
The driver-name skip check was !driverName.equalsIgnoreCase("PooledDriver"),
i.e. anything other than PooledDriver skipped Dev Services. This
incorrectly caught SingleMongoConnectDriver too -- a real driver name that
morphium-core supports (de.caluga.morphium.driver.wire.SingleMongoConnectDriver.driverName)
and, like PooledDriver, needs an actual MongoDB server. A user configuring
quarkus.morphium.driver-name=SingleMongoConnectDriver would silently lose
Dev Services and have to configure hosts manually.
Flipped to a whitelist on InMemDriver -- the only driver name that
genuinely needs no real MongoDB connection -- so both real drivers
(PooledDriver, SingleMongoConnectDriver) get Dev Services.
Verified: full reactor build green, all 16 deployment-module unit tests
green, full 247-test Docker integration-tests suite green (the existing
InMemDriver skip test continues to pass unchanged).
Should-fix #4 found by Stephan Boesebeck's review on PR sboesebeck#267
(sboesebeck/morphium).
…eplica-set container The Dev Services replica-set path had no test that directly proves a real container was started: MorphiumDevServicesReplicaSetConfigTest explicitly documents it starts no container at all (only checks config-key binding), and this class's other tests only prove transactions work end-to-end -- which happens to require a replica set, but doesn't directly show Dev Services actually provisioned one for it. Added devServicesStartedARealContainer(), asserting quarkus.morphium.hosts is a real Testcontainers-assigned port (never 27017, and never the @WithDefault localhost:27017) and that the driver negotiated isReplicaSet()==true during its handshake with the container -- something an unconfigured standalone mongod would never report. Verified: full 248-test Docker integration-tests suite green (247 existing + this new one). Should-fix #5 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
…tion formula MorphiumStartupCheckTest's simulateStartupCheck() was a private, duplicated copy of MorphiumStartupCheck.call()'s everConnected formula (opened > 0 || driverConnected). A future edit to the real class -- e.g. flipping || to && -- would silently break the SRV-discovery-tolerant startup latch while every test in this file kept passing, since they only ever exercised the copy. Extracted the formula into MorphiumStartupCheck.isEverConnected(), package- private specifically so the test can call the actual production method. Verified the fix's own value: temporarily flipped || to && in the real class and confirmed 2 of 5 tests fail as expected, then restored the original code (verified with 5/5 green again) -- the previous test suite would have stayed green through that exact mutation. Verified: full reactor build green, all 39 runtime-module tests green (no regression from the previous 5/5 in this class). Should-fix #6 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
…t overflow Two related misconfiguration risks in MorphiumProducer, both fixed the same way: fail loudly at startup instead of silently doing the wrong thing. 1. quarkus.morphium.username/password were only applied when BOTH were present (config.username().isPresent() && config.password().isPresent()). Setting only one of the two -- a plausible typo/copy-paste mistake -- silently connected unauthenticated instead of failing. The application would appear to work against a no-auth MongoDB in dev while any environment where auth is actually required would either reject the connection outright, or worse, silently succeed unauthenticated against a MongoDB instance that happens to allow it. Now throws immediately if exactly one of the two is set. 2. quarkus.morphium.cache.global-valid-time is a long (milliseconds) but CacheSettings.setGlobalCacheValidTime(int) takes an int; the direct (int) cast silently overflowed for any value above Integer.MAX_VALUE ms (~24.8 days) -- e.g. a well-intentioned "cache for 30 days" config (2_592_000_000L ms) wrapped to a negative int with no warning at all. Now validates the range and throws instead of casting blindly. Extracted both checks into validateCredentialsPresence() and toIntGlobalCacheValidTime() (package-private statics, same pattern as the existing parseReadPreference()) so they're unit-testable without a real Morphium connection. Added 7 new tests covering both-present/both-absent/ either-alone for credentials and default/boundary/overflow for the cache TTL. Verified: full reactor build green, all 46 runtime-module tests green (39 existing + 7 new). Should-fix #7 (part 1 of 3: partial credentials, part 3 of 3: cache TTL overflow) found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
fix #7 part 2) Verified against the actual running application (not assumed) what happens when a malformed MorphiumId reaches the two paths that construct one from untrusted input: - @PathParam MorphiumId id (RESTEasy Reactive's built-in JAX-RS String-constructor convention, no Jackson involved) -- a malformed value results in 404 Not Found, not 500. Not a very informative error for what is actually bad input rather than a missing resource, but confirmed NOT an unhandled-exception server-error leak either. - A MorphiumId field in a JSON request body (MorphiumIdJacksonModule's actual deserializer path) -- Jackson wraps the constructor's IllegalArgumentException as a JsonMappingException during body parsing, and RESTEasy Reactive's default handling for that is already 400 Bad Request. No production fix needed for either path -- both were already better than the reviewed concern assumed. Added a new /morphium-id/entity POST endpoint (the JSON-body path had no exercising endpoint at all before this) and two regression tests documenting the real, verified behavior for both paths. Verified: full 250-test Docker integration-tests suite green (248 existing + 2 new). Should-fix #7 (part 2 of 3: malformed MorphiumId) found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium) -- confirmed as not a bug after investigation, documented with tests instead of a speculative fix.
…wo already-safe patterns Should-fix #8 raised three separate concerns about MorphiumTransactionalInterceptor: 1. isNoServerTransaction() matched only the single exact phrase "Cannot start a transaction" -- a future MongoDB server version changing that wording would silently stop this tolerance from working, with no compile-time or test-time signal. Broadened to a case-insensitive match against several known MongoDB "no transaction" error phrasings (documented in the method's Javadoc as a best-effort heuristic, since there is no stable documented error code for this specific server rejection to match against instead). Added 5 new unit tests covering the original phrase, differently-cased input, an additional phrasing, an unrelated error (must NOT match), and a null message (must not NPE). 2. Investigated whether BEFORE_COMMIT fires once per commit-retry attempt (not just once per business-method attempt) -- verified against the current code (already fixed as a side effect of the blocker #5 commit splitting ctx.proceed() from the commit into separate try/catch blocks): BEFORE_COMMIT sits outside safeCommitWithRetry()'s own internal retry loop, so a transient commit retry does NOT re-fire it. Documented this explicitly in a comment at the call site as the verified guarantee, no code change needed. 3. Investigated the CosmosDB-detection fail-open-to-false behavior on a detection-call exception -- confirmed it is not a silent steady-state bug: startTransaction()'s own UnsupportedOperationException catch block (already existing) self-corrects cosmosDb to true on the very next call if the backend actually is CosmosDB and detection merely failed transiently. Documented this existing safety net explicitly at the fail-open site, no code change needed. Verified: full reactor build green, all 51 runtime-module tests green (46 existing + 5 new), full 250-test Docker integration-tests suite green (no regression). Should-fix #8 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
… failures, document idempotency
Four related migration-framework polish items:
1. Migrations were sorted with Comparator.comparing(MigrationInfo::order),
a plain lexicographic string comparison on the @MorphiumChangeUnit's
order() String. "10" sorts BEFORE "2" lexicographically -- this
codebase's own tests never caught it because every existing test
migration happens to use zero-padded, equal-width order strings ("001",
"002", "999"). A real project with more than 9 migrations and unpadded
order values would see them silently run out of order. Added
compareByOrder(), which compares numerically when both order values
parse as a long, falling back to lexicographic comparison otherwise
(keeps compatibility with a date-based or other non-numeric convention).
4 new tests covering numeric, zero-padded, non-numeric, and mixed cases.
2. tryRollback()'s own failure (when a migration fails AND its rollback
also fails) was only logged, never surfaced -- the database can be left
in an unknown intermediate state (migration partially applied, rollback
partially/not applied) with the rollback failure's details lost outside
the log. tryRollback() now returns Optional<Exception>; when present,
executeMigration() attaches it as a suppressed exception on the
original migration failure (which remains the primary thrown cause, per
existing behavior/tests) instead of only logging it.
3. Documented (Javadoc on acquireLock() + a configuration.adoc note on
lock-ttl-seconds) that expires_at is computed from each instance's local
clock, not the MongoDB server's -- client clock skew between instances
can cause a lock to be taken over while still actively held. This is a
real, currently unaddressed limitation (Morphium/the driver has no
update-pipeline support for a server-computed expiry), not a false
alarm; documented the standard mitigation (NTP-synchronized clocks,
generous TTL) rather than shipping a partial fix.
4. Documented (on the @execution annotation + a configuration.adoc note)
that migration methods must be idempotent: the changelog entry is
written only after the method returns successfully, so a crash between
completion and that write causes the method to run again on next start.
Verified: full reactor build green, all 55 runtime-module tests green (51
existing + 4 new), full 250-test Docker integration-tests suite green (no
regression, including the existing rollback test).
Should-fix #9 found by Stephan Boesebeck's review on PR sboesebeck#267
(sboesebeck/morphium).
…native-image reflection Two related native-image reflection gaps in the build-time entity scan: 1. Only DefaultNameProvider was unconditionally registered for reflection. ObjectMapperImpl.getNameProviderForClass() instantiates whatever class @entity(nameProvider = ...) actually points to via getDeclaredConstructor().newInstance() -- a custom provider was never registered at all, so a native-image build would fail at runtime the first time that entity's collection name is resolved. Added registerCustomNameProvider(), which extracts the nameProvider value from the Jandex @entity annotation and registers it (skipping DefaultNameProvider itself, already registered unconditionally). 2. The Jandex scan only finds classes that carry @Entity/@Embedded directly -- @entity is not @inherited, so a subclass of an entity with no annotation of its own was never registered, even though Morphium's own AnnotationAndReflectionHelper.isAnnotationPresentInHierarchy() walks the class hierarchy manually and treats such a subclass as a full entity (polymorphic persistence is fully supported at the ORM level). Storing/loading an actual runtime instance of that unannotated subclass would reflectively access fields/constructors never registered, and crash only in a native build, only for that specific subclass. Added registerSubclasses(), using Jandex's getAllKnownSubclasses() (the same API already used elsewhere in this processor for MongoCommand subclasses) to register every direct and transitive subclass. Added MorphiumProcessorReflectionTest, building a real Jandex index from actual compiled test-fixture classes (not a mock) to exercise both new methods against the real IndexView/AnnotationInstance API contract: 3 new tests covering direct+transitive subclass registration, custom nameProvider registration, and confirming DefaultNameProvider is not duplicated. Verified: full reactor build green, all 19 deployment-module tests green (16 existing + 3 new), full 250-test Docker integration-tests suite green (no regression from the changed entity scan). Should-fix #10 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
…A-DATA.md gaps as done, note README version duplication Three documentation drift issues: 1. testing.adoc used the bare morphium.* prefix in three places instead of quarkus.morphium.* -- verified real by checking the actual InMemMorphiumTestProfile.java source, which uses quarkus.morphium.driver-name and quarkus.morphium.database. One of the three occurrences was inside a copy-pasteable application.properties code block, so a reader following it verbatim would have configured properties SmallRye Config never binds to anything. 2. quarkus-morphium/docs/gaps/JAKARTA-DATA.md presents itself as an open "Gap Analysis & Improvement Roadmap" with a March 2026 date, but every one of its 9 numbered items is already marked DONE (verified against the actual implementation: EmptyResultException/NonUniqueResultException really are thrown from FindMethodBridge.java/QueryResultHelper.java, not just planned). Added a status note at the top clarifying the document is now historical implementation-log context, not a pending-work list, and naming the one item that genuinely remains open (GAP-A6, COUNT DISTINCT). 3. Two version numbers in README.md (the prerequisites table and the Maven dependency snippet) are hand-duplicated with no shared-attribute mechanism (Markdown has none, unlike the AsciiDoc guide pages' attributes.adoc) -- added maintainer comments at both call sites so a future version bump doesn't miss one of them. Not a build-tooling fix (would need Maven resource filtering on README.md, which isn't configured anywhere in this reactor and is out of scope for a docs drift fix) -- both values happen to already be correct as of this commit, this only prevents future drift. Verified: full reactor build green (including a fresh -DskipExtensions core-only build, confirming the corrected pom.xml comment from the companion commit is accurate), full 250-test Docker integration-tests suite green. Should-fix #11 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
…a too, not just quarkus-morphium The comment describing 'mvn install -DskipExtensions' only said it builds 'core + PoppyDB', without naming which modules that excludes -- a reader could reasonably assume only quarkus-morphium (the module people usually mean by 'the extension') is skipped, when the extensions profile actually disables BOTH morphium-jakarta-data AND quarkus-morphium together. Made this explicit, and noted why splitting them wouldn't even be possible: quarkus-morphium depends on morphium-jakarta-data. Verified: mvn validate confirms the pom.xml is well-formed (my first attempt at this wording accidentally put a literal '--' inside the XML comment body, which is invalid XML and failed validate -- caught and fixed before this commit). A fresh 'mvn install -DskipTests -DskipExtensions' run confirms the reactor summary matches exactly what the corrected comment now says: only Morphium Parent, Morphium (core), and PoppyDB are built. Should-fix #12 found by Stephan Boesebeck's review on PR sboesebeck#267 (sboesebeck/morphium).
…tities one by one executeAnnotatedDeleteCounted() materialized every matching entity via query.asList() and deleted them one at a time via morphium.delete(entity), then returned toDelete.size() as the count. Inefficient for large deletes (loads the full result set into memory just to throw it away), and can misreport the count under concurrent modification -- a document deleted or changed by another writer between the load and the per-entity delete drifts the reported count away from what was actually removed. Morphium already supports deleting directly by query (Query.delete(), a single server-side round-trip) and returns the driver's own "n" count in the result map -- the same pattern already used throughout morphium-core's own test suite for reading a delete/update result's actual affected count. Verified: full reactor build green, all 82 morphium-jakarta-data tests green (no regression -- the existing delete-count tests already exercised this path and continue to pass with the new implementation), full 250-test Docker integration-tests suite green. Copilot review comment on PR sboesebeck#267 (sboesebeck/morphium).
…ss semantics The should-fix #2 commit (2026-08-06) decoupled MorphiumLivenessCheck from MongoDB connectivity, but health-checks.adoc was never updated to match -- it still described liveness as reporting DOWN on lost driver connectivity in three places (the probe overview table, the Liveness Check section description, and its Kubernetes-behavior explanation), directly contradicting the actual code. A reader configuring Kubernetes probes based on this doc would expect pod-restart-on-DB-outage behavior that the code deliberately does not provide. Copilot review comment on PR sboesebeck#267 (sboesebeck/morphium).
…the actual sort semantics The should-fix #9 commit (2026-08-06) added numeric-when-possible comparison to MorphiumMigrationRunner.compareByOrder(), but order()'s own Javadoc was never updated -- it still said migrations sort lexicographically, directly contradicting the actual behavior and risking confusing users who read the annotation's own documentation for how to name their migrations. Copilot review comment on PR sboesebeck#267 (sboesebeck/morphium).
…ng a failed commit as success The blocker #5 fix (splitting ctx.proceed() from the commit) removed the double-apply path but was a no-op on the default driver -- and turned a data-loss case into a reported success. PooledDriver.commitTransaction() clears the transaction context in its finally block unconditionally, even when the commit command itself failed. So on the retry, safeCommit() saw morphium.getTransaction() == null and returned as if there was nothing to commit. For code 251 (NoSuchTransaction after a failover, where the server actually did commit and only the reply was lost) that accidentally produced the right answer. For code 112 (WriteConflict at commit time -- the server aborted, nothing was persisted) the interceptor returned normally and fired AFTER_COMMIT while every write in the transaction was lost. That is worse than the double-apply it replaced. safeCommitWithRetry() now snapshots the context before the first attempt and re-installs it via morphium.setTransaction() before each retry, so the retry actually re-issues the commit. The companion core-side issue (PooledDriver keeping the context on transient failure) stays out of scope here, as agreed. Also adds the test coverage that was missing entirely for this path -- Mockito (test scope, version managed by the inherited Quarkus BOM) is new to this module for it, since faking MorphiumDriver by hand for a single method is not maintainable: - transientCommitFailure_retriesWithRestoredContext_notSilentSuccess: fake driver mimics PooledDriver exactly (clears the context in a finally block even when throwing), first commit throws code 112, second succeeds. Asserts commitTransaction() is invoked twice, not once. - nonTransientCommitFailure_isNotRetried_andPropagates: code 11000 (DuplicateKey) is committed once and the exception propagates. Test quality verified by mutation, not just by passing: commenting out morphium.setTransaction(txContext) in the production code turns the first test red with "expected: 2 but was: 1" -- exactly the silent-success path. Restored and re-verified green afterwards. Note for the reviewer: Mockito's inline mock maker cannot instrument Morphium on JDK 25 (ByteBuddy retransform failure), so these tests need JDK 21. Installed locally; not committing a .tool-versions pin since that is a project-level decision. Verified: 57/57 tests green in quarkus-morphium/runtime (55 existing + 2 new). Item A from Stephan Boesebeck's re-review on PR sboesebeck#267 (sboesebeck/morphium).
…with a dynamic parameter The dynamic Sort/Order/PageRequest/Limit support added in 11f669e routed EVERY non-void derived method carrying such a parameter through the new 12-arg QueryMethodBridge.executeQuery overload -- and that overload builds its query itself and always ends in a find, never consulting descriptor.prefix(). The simpler 8-arg overload delegates to QueryExecutor.execute(), which does switch on the prefix; the new one bypassed that entirely. Consequences, both regressions introduced by that commit: - boolean deleteByStatus(String, Limit) deleted NOTHING and still returned a success-looking value (a find, then !resultList.isEmpty()). Before 11f669e this signature actually deleted -- the dynamic argument was merely ignored. - long countByStatus(String, Limit) / existsBy... returned a List from the bridge, and the generated bytecode's checkCast to Long/Boolean turned that into a ClassCastException on first call. Non-FIND prefixes now delegate to QueryExecutor.execute(), so delete deletes, count returns a number and exists returns a boolean. A dynamic Sort/Order on a non-FIND prefix is accepted and ignored (there is no result list for it to reorder). A Limit or PageRequest on countBy*/existsBy*/ deleteBy* is instead rejected at build time with a clear message: "the 3rd page of a delete" or "count, but only the first 10 matches" has no sensible definition, and neither countAll() nor query.delete() has a skip/limit bounded variant -- failing the build beats silently dropping a parameter the caller wrote expecting it to take effect. Tests (all three would have caught the regression): - deleteByStatusSorted asserts the actual document count in the database before (3) and after (1) the call, plus that findByStatus("OPEN") is now empty -- not just that the return value looks plausible. - countByStatusSorted asserts the long count, existsByStatusSorted the boolean; both previously threw ClassCastException. Verified: full reactor build green; 82/82 morphium-jakarta-data tests green; integration-tests "Jakarta Data Query Derivation" 16/16 green including the three pre-existing findByStatusSorted/Limited/Paged tests (no regression in the find path). The build-time rejection has no automated test yet -- it needs a synthetic-Jandex-index unit test in the deployment module, since a deliberately failing build cannot be asserted green from integration-tests; noting that as a gap rather than claiming coverage. Item B from Stephan Boesebeck's re-review on PR sboesebeck#267 (sboesebeck/morphium).
…ument ssl.tls-configuration-name Two smaller items from the review. isAsyncReturnType() only knew CompletionStage and io.smallrye.mutiny.Uni. Multi has exactly the same problem: the interceptor commits right after ctx.proceed() hands back the Multi, i.e. before the asynchronous work has run, so it has to fail fast for the same reason Uni does. While adding the test it turned out the pre-existing Uni detection was never covered at all -- only CompletionStage/CompletableFuture were. Both name comparisons are now tested. Since Mutiny is deliberately not a dependency of this module, the test defines classes carrying those exact fully-qualified names at runtime via ByteBuddy (already on the test classpath transitively, no new dependency) and asserts the FQN before asserting the detection, so the test cannot pass for the wrong reason. An earlier attempt placed a stub source file in the io.smallrye.mutiny package instead; that was dropped because it would collide with the real class as a split package / duplicate class the moment Mutiny ever becomes a real dependency. quarkus.morphium.ssl.tls-configuration-name existed in SslConfig but was missing from both configuration.adoc and the README table. A sweep over the remaining ssl.* properties confirmed the other eight were already documented. Verified: 59/59 tests green in quarkus-morphium/runtime; configuration.adoc table delimiters still balanced. Remaining items from Stephan Boesebeck's re-review on PR sboesebeck#267 (sboesebeck/morphium).
… gate the Docker-dependent test before boot The previous attempt only moved the detector's listener registration onto a background thread. That kept the boot thread free but did not stop the eager connect: registerListener() still called morphiumInstance.get(), which dereferences the CDI proxy and runs MorphiumProducer.buildMorphium() with its full retry ladder. Without Docker, Dev Services now skips cleanly, the app boots against the default localhost:27017, and that connect fails after retrying -- while MorphiumTransactionalTest's @BeforeAll Docker check never gets a chance to run, because @QuarkusTest boots the application before JUnit's @BeforeAll. The detector no longer resolves Morphium at all. It is not a CDI bean anymore (removed from AdditionalBeanBuildItem), the @observes StartupEvent observer and its thread are gone, and it is now a static registerOn(Morphium) called from buildMorphium() right after the connect succeeded. That makes it structurally impossible for the detector to be the cause of a connect, instead of merely unlikely. Additionally, MorphiumTransactionalTest is now gated by a JUnit ExecutionCondition that calls DockerClientFactory.instance() .isDockerAvailable() directly -- JUnit evaluates conditions before QuarkusTestExtension boots the application, which the replaced @BeforeAll assumeTrue could not. Testcontainers' @EnabledIfDockerAvailable is deliberately not used; its detector misreports under Quarkus test classloading (already documented in that class). Verified with Docker: full reactor build green, MorphiumTransactionalTest runs (5/5 green, 0 skipped -- i.e. the condition correctly does NOT disable it when Docker is present), 59/59 runtime-module tests green. NOT verified: an actually Docker-less run. Three simulation attempts all failed for traceable reasons: DOCKER_HOST is ignored by Testcontainers' UnixSocketClientProviderStrategy (hardcoded /var/run/docker.sock); TESTCONTAINERS_DOCKER_SOCKET_OVERRIDE does make isDockerAvailable() return false but Quarkus's own guard uses ContainerRuntimeUtil, i.e. the docker binary, so DockerStatusBuildItem still reported available; -Dquarkus-local-container-runtime=UNAVAILABLE is overridden by a pinned strategy in ~/.testcontainers.properties. Worth noting independently: the project relies on two different Docker detections (Quarkus binary check vs Testcontainers socket check) that can disagree -- likely the same reason @EnabledIfDockerAvailable misbehaved here. This needs verifying on a genuinely Docker-less machine or in CI. Blocker 7 follow-up from Stephan Boesebeck's re-review on PR sboesebeck#267 (sboesebeck/morphium).
… 2 review 1. Private interface methods broke the build -- a regression my own earlier fix introduced. Jandex's isDefault() means "public && !static && !abstract", so a private interface method (legal since Java 9, it has a body) is not "default", fell through every guard, and hit the "no recognized pattern" IllegalStateException. Perfectly legal user code failed the build. The guard now skips everything non-abstract via isAbstract(), plus an explicit skip for abstract toString()/equals()/ hashCode() redeclarations (Object provides those regardless). 2. A @delete method returning boolean/Integer/Long still produced the original VerifyError at class load: the returnsCount guard only covered primitive int/long, so anything else fell into the void branch and generated a bare return for a method whose descriptor promises a value. Now a build-time error with a clear message. Jakarta Data only permits void, int or long for a parameter-based @delete. 3. Abstract methods inherited from a CUSTOM super-interface escaped the build-time check entirely, because Jandex's methods() only returns declared methods -- so they still hit AbstractMethodError on first call. The scan now walks interfaceTypes() recursively, treating the four standard Jakarta Data interfaces as hierarchy dead-ends so their CRUD methods aren't misreported, with a visited-set guarding against diamond inheritance. 4. The @by parameter-name fallback (Jakarta Data 4.6.1) was added to the @find path but not to generateDeleteAnnotatedMethod, which still only looked for @by. A @delete method relying on parameter names got hasByParams=false, was treated as an entity-parameter delete, and called doDelete(stringArg) at runtime -- trying to delete a String as an entity. New MorphiumDataProcessorCustomMethodsTest covers all four against a real synthetic Jandex index (same approach as MorphiumProcessorReflectionTest): a private helper plus an abstract toString() must NOT break generation; an unsupported @delete return type must fail at build time; a method declared only on a custom super-interface must still be generated (verified it is declared on WithAudit only, not on the repository itself -- exactly the case item 3 previously missed); and the @delete parameter-name fallback is treated as a condition delete. Verified: full reactor build green, 23/23 deployment-module tests green (19 existing + 4 new). Blocker 2 follow-up from Stephan Boesebeck's re-review on PR sboesebeck#267 (sboesebeck/morphium).
…ng change unit, and prove both Follow-up on blocker 6. Three things were wrong beyond the original fix. renewLock() ignored its own update result. When the owner-guarded update matched 0 documents -- i.e. another instance had taken the lock over -- this instance carried on and ran the remaining change units concurrently with the new owner. It now inspects "n" the way acquireLock() already did and aborts the run. The comment claiming subsequent writes would be no-ops anyway was simply wrong and is corrected: only releaseLock() is owner-guarded, recordExecution() and the change units themselves write unconditionally. Renewal only happened BETWEEN change units, so a single unit running longer than the TTL (an index build on a large collection, say) still allowed the atomic steal of that very in-flight unit. Rather than documenting the constraint away, there is now a real in-flight heartbeat: a daemon thread renewing the lock while a unit is still executing, stopped in a finally block, owner-guarded, with its failure surfaced instead of swallowed. Writing the tests for that exposed a genuine bug in the heartbeat itself: its tick interval was TTL/3 but floored at one second, so for any small lockTtlSeconds the first tick fired only after the TTL had already elapsed -- the heartbeat was structurally unable to renew in time. The floor is now 200ms, which keeps the "don't hammer the database" intent for realistic TTLs while making small ones actually serviceable. Both regression tests were rewritten, because the previous pair could not work. The obvious "a contender calls acquireLock() and must fail" approach is impossible against InMemDriver: an upsert whose filter matches nothing (expires_at still in the future) is seeded from the equality predicates only -- correct -- but then goes through storeInternal(), which treats an existing _id as a replace rather than raising a duplicate key error the way a real server would. So any contender steals a still-valid lock there, regardless of renewal. That is also why the original test could never prove anything. The tests now prove renewal positively instead: an observer thread reads the lock document mid-run and asserts the owner is unchanged and expires_at has moved forward. The driver divergence is documented at the tests. Both are mutation-proofed, which mattered: the first version of the between-units test stayed GREEN with renewLock() disabled -- the same mistake the reviewer had called out, repeated. At a 1s TTL the heartbeat ticks every ~333ms, so with 800ms units it was silently renewing the lock and masking the missing call. That test now uses a 6s TTL, making the heartbeat interval (2s) longer than any single unit. Disabling renewLock() now reddens only the between-units test, disabling the heartbeat only the in-flight test, each on its own assertion, the other 8 staying green. The wait test's lock id was wrong too (it seeded "morphium_migration_lock" while the runner uses "migration_lock"), so the polling loop was never exercised -- it would have stayed green with acquireLockWithWait() deleted. It now goes through an accessor instead of a copy-pasted literal, so renaming the constant cannot blind it again. Verified: full reactor clean build green, 254/254 integration tests green (0 failures, 0 errors, 0 skipped), Migration Framework 9/9. Blocker 6 follow-up from Stephan Boesebeck's re-review on PR sboesebeck#267 (sboesebeck/morphium).
…ed files
Copilot flagged that the published POMs still carry an
"ENTSCHEIDUNG-OFFEN D1" marker and German-only commentary. That is fair, and
it turned out to be broader than the two spots it named: seven places across
four POMs plus one Java Javadoc referenced internal planning documents
("D1", "D3", "B6", "I4", "Absicherung", "Begleitmassnahmen",
"D3-reactor-strategie.md"). None of those documents exist anywhere in this
repository, so for anyone reading the merged code they are dead pointers.
The ENTSCHEIDUNG-OFFEN markers are removed rather than translated: they
tracked a decision that has since been made and implemented (both modules
inherit their version and morphium.version from morphium-parent, visible in
the <parent> block right below where the marker sat). Everywhere else the
substance of the comment is kept and the document references are replaced by
the reasoning itself, so each comment stands on its own. The reactor's
I1-I5 invariant list is deliberately kept as-is -- it is self-explanatory and
genuinely useful; only its external references were dropped.
morphium-jakarta-data/pom.xml is included because it carries the identical
marker (plus a German "check whether this belongs in morphium-parent" note)
and is part of the same published reactor, even though that module already
merged separately.
Comments only: verified via diff that no version, dependency, property,
module or other XML structure was touched. mvn validate green (that also
catches an accidental double hyphen inside an XML comment, which is invalid
XML -- a trap I had already hit once in this branch), mvn install green,
integration-tests test-compile green.
The other Copilot finding in the same review -- that the extension guide URL
points at the develop branch while the default branch is main -- is a false
positive: this repository's default branch IS develop, so the URL is
correct. It 404s only because the file it points at arrives with this very
PR.
Copilot review comment on PR sboesebeck#267 (sboesebeck/morphium).
…eting nothing Another regression from my own earlier fix, caught by Stephan. The @by parameter-name fallback I added to generateDeleteAnnotatedMethod checked only whether a parameter HAS a name, never what its TYPE is. So for @delete void remove(CustomerEntity customer) -- an entity lifecycle parameter, which Jakarta Data says must go to doDelete(entity) -- the fallback claimed the parameter as a condition, built the query {customer: <entity>}, matched nothing, and query.delete() removed zero documents while the method returned normally. Silent data loss, and it worked correctly before my fallback existed. The fallback now applies only when the parameter type is not the entity itself, an array of it, or a List/Collection/Iterable of it. An explicit @by is still always honoured, even on an entity-typed parameter, since that is a deliberate opt-in by the developer. Mixing an entity parameter with condition parameters in one @delete is now rejected at build time. The Delete javadoc in jakarta.data-api 1.0.1 defines either exactly one entity/List<E>/E[] lifecycle parameter or condition parameters, not both, so there is no semantics to implement -- failing the build beats inventing one. Tests: 4 new unit tests over a synthetic Jandex index (isEntityParameter across all type shapes plus negative cases, entity-parameter delete is not treated as a condition delete, both cases asserted side by side, and the build-time rejection of the mixed case), plus an integration test asserting the actual document count before and after remove(OrderEntity) with a per-customer cross-check -- the test that would have caught this. Mutation-proofed rather than trusted: disabling the type check makes the build fail, and instructively so -- remove(OrderEntity) is then misread as a condition parameter and trips the new mixed-case rejection. The two guards compose, so even a failing type check cannot resurrect the silent-delete path. Still open: the entity branch continues to support only a single entity via doDelete(Object), not List<E>/E[] via doDeleteAll(List). That predates this bug and is unrelated to it; noting it rather than bundling it in. Verified: clean reactor build green, 27/27 deployment-module tests, 255/255 integration tests (0 failures, 0 errors, 0 skipped). Reported by Stephan Boesebeck on PR sboesebeck#267 (sboesebeck/morphium).
…s two small cleanups Three findings from the latest Copilot review. The test named "Entity without @Version stores and updates normally" used ItemEntity -- which HAS a @Version field -- and asserted getVersion() == 2, i.e. it verified working version tracking, the exact opposite of its name. Its own comments admitted it ("re-uses ItemEntity", "pretend no version was tracked"). It was pure false confidence. There is now a real UnversionedEntity with no @Version field, and the test proves the actual behaviour: a second client loads and updates the document in between, and the now-stale original reference still stores successfully instead of throwing VersionMismatchException. The build confirms the new entity is picked up (27 instead of 26 @Entity/@Embedded classes registered). The other three tests in the class keep using ItemEntity, which is correct -- they test versioning. MorphiumStartupCheckTest had two tests asserting the identical condition (isEverConnected(0.0, false) == false). Removed the duplicate rather than repurposing it: the remaining four tests already cover all four boolean combinations of "connectionsOpened > 0 || driverConnected", and a negative connection count is not a meaningful case for a monotonic stats counter, so there was no uncovered edge to move it to. The survivor has the more descriptive SRV-discovery framing. MongoDBStartable recompiled the same replicaSet regex on every getReplicaSetName() call; hoisted into a static final Pattern. Two further Copilot suggestions were deliberately not taken. Changing MorphiumTransactionEvent from Exception to Throwable would alter a public API for a case that intentionally fires no event at all -- the interceptor catches Throwable but only fires AFTER_ROLLBACK for Exceptions, letting Errors propagate, which is the agreed behaviour. And the missing license header on StatusStats matches existing practice in integration-tests, where many files have none; adding one there alone would be inconsistent rather than more correct. Verified: 255/255 integration tests, 58 runtime-module tests (one fewer, the removed duplicate), clean reactor build green. Copilot review on PR sboesebeck#267 (sboesebeck/morphium).
…COL in native images Third time Stephan raised this branch, and he was right that it is broken asymmetry. My two earlier dismissals were both wrong, in different ways: First I claimed setAutoIndexAndCappedCreationOnWrite(true) forces the index check to NO_CHECK. It does not -- MorphiumConfig lines 509-511 set BOTH the index check and the capped check to CREATE_ON_WRITE_NEW_COL. Then I checked only the index path, found that checkIndices()' ClassGraph scan is gated to CREATE_ON_STARTUP/WARN_ON_STARTUP (Morphium.java 575-576), and concluded there was no scan. That gating is real, but I had missed the second scan: checkCapped() (Morphium.java 3354, scanning at 3358) is called UNCONDITIONALLY at line 529, with no mode gate at all. Looking at only one of the two values that setter writes is what hid this from me twice. Tracing the whole chain, though, changes the severity rather than the conclusion: buildMorphium() already pre-registers the build-time @CappeD list into ClassGraphCache before the Morphium constructor runs, and getClassesWithAnnotation() returns a pre-registered entry without scanning -- including an empty one, and cappedClassNames defaults to Collections.emptyList(), never null. So the scan was in practice already unreachable, and native images were not actually crashing here. The branch is still worth fixing. It now forces both checks to NO_CHECK when ImageMode.current() is NATIVE_RUN, which makes the safeguard independent of that call-ordering staying intact and restores symmetry with the other three branches. A native run cannot create collections on the fly anyway, so nothing of value is disabled. The JVM path is deliberately untouched: there the scan is a startup cost rather than fatal, and disabling it would defeat the whole point of the mode. The comment now spells out this reasoning so the branch does not read as an oversight a fourth time. Five unit tests, including a JVM counter-test asserting both checks stay on CREATE_ON_WRITE_NEW_COL, so the fix cannot silently break the mode for JVM users. Mutation-proofed: disabling the native guard reddens exactly the native test on its own assertion and leaves the other four green. Verified: clean reactor build green, 63/63 runtime tests (58 + 5 new), 27/27 deployment, 255/255 integration (0 failures, 0 errors, 0 skipped). Reported three times by Stephan Boesebeck on PR sboesebeck#267 (sboesebeck/morphium).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review-PR analog zu #16 (morphium-jakarta-data): aktiviert CodeRabbit/Copilot/Codex
gegen die neue quarkus-morphium-Extension, bevor der Upstream-PR gegen
sboesebeck/morphium:develop gestellt wird.
Kontext
Zweite Welle der Modul-Integrationsserie: nach
morphium-jakarta-data(siehe #16 →sboesebeck#266, dort noch offen) folgt hier
quarkus-morphium, die QuarkusCDI-Extension für Morphium. Base-Branch ist
pr/jakarta-data-module(nichtmaster),weil sboesebeck#266 noch nicht gemergt ist — der Diff hier zeigt daher ausschließlich die
quarkus-morphium-Änderungen.
Was dieser PR macht
quarkus-morphium/als neues optionales Modul:runtime,deployment,testing,integration-tests(128 Dateien)io.quarkiverse.morphium→de.caluga(nicht im Quarkiverseregistriert)
extensions,quarkus.versionzentral im Parent,Quarkus-BOM-Import bleibt im Modul-POM (Invariante I4)
docs/quarkus-extension.md, mkdocs.yml, CHANGELOG),release.shum vierArtefakte erweitert (inkl. POM-only
quarkus-morphium-parent-Sonderfall)Verifikation
integration-tests: 242/242 Tests grün (Docker via Testcontainers)-DskipExtensionsbaut nur 3 Module, kein Quarkus/Testcontainers imKern-Dependency-Tree
developverifiziertevorbestehende Flakies (Messaging-Timing, Byte-Buddy/JDK-25, kein lokaler Mongo) —
keine Regression
Vollständiger Bericht:
docs/plans/morphium-module-integration/reports/M4-T4-verification.mdim morphium-jakarta-data-Repo.
Ziel dieses PRs
Ausschließlich Community-Review (CodeRabbit/Copilot/Codex) auslösen. Kein Merge
geplant — nach Review-Fixes wird der Branch vor dem eigentlichen Upstream-PR auf den
dann aktuellen
origin/developrebast (sobald sboesebeck#266 dort gemergt ist).