feat(quarkus): use-bson-date-for-java-time covers all four java.time types, not just LocalDateTime

[Bardioc1977]

What

quarkus.morphium.use-bson-date-for-java-time — one property that reaches all four java.time types the object mapper maps specially, instead of just LocalDateTime.

Why

The extension could configure the on-disk format of exactly one of those four types. quarkus.morphium.local-date-time.use-bson-date replaced the registered LocalDateTimeMapper with one built from that property (MorphiumProducer:614-615); Instant, LocalDate and LocalTime stayed on the legacy formats with no property able to change them — grep -rn "objectMappingSettings" quarkus-morphium/ --include=*.java came back empty, the extension never touched ObjectMappingSettings at all.

So an application whose timestamps are Instant — the usual choice for a stored point in time — could not get native BSON dates from Quarkus, and with them no native range queries, sorts or TTL indexes on those fields. Core has supported this since #333/#335; only the extension could not reach it.

Found while trying to retire an Instant-vs-Long shadow-field workaround in a Quarkus application whose 17 timestamp fields are all Instant, after 6.3.7 landed on Maven Central.

How

applyJavaTimeFormat sets ObjectMappingSettings#setUseBsonDateForJavaTime(boolean) while the config is assembled. That is what makes one switch reach all four types: ObjectMapperImpl:139-143 registers the mappers with a BooleanSupplier reading the flag fresh on every marshall(), so the value has to be on the config before Morphium builds the mapper.

No default, deliberately. The core flag is all-or-nothing across the four types, so it cannot reproduce the per-type split the old property produces (LocalDateTime native by default, the other three legacy). Defaulting the new property would silently rewrite the on-disk format of existing Instant/LocalDate/LocalTime fields on upgrade. Left unset, the extension behaves exactly as before — Optional<Boolean> is the opt-in marker, not a tri-state for its own sake.

The deprecated override is skipped once the new property is set, and that is not tidiness: LocalDateTimeMapper(boolean) holds a fixed value and stops consulting the config, so keeping both paths would pin LocalDateTime while the other three follow the flag — reintroducing exactly the split this property removes. The condition lives in registersDeprecatedLocalDateTimeOverride() so a test can reach it.

local-date-time.use-bson-date is @Deprecated(since = "6.3.8", forRemoval = true), keeps working, and still governs LocalDateTime while the new property is unset. Nothing breaks on upgrade.

Verification

MorphiumProducerJavaTimeFormatTest, 5 tests. Asserted at the mapper level, not through a store-and-read round trip: InMemoryDriver normalises nothing on its write path (#336), so a round-trip assertion would pass regardless of the flag and prove nothing — the same trap that cost time twice while reviewing #333.

Two mutation probes run, both kill tests:

Mutation	Result
flag never applied (applyJavaTimeFormat body removed)	Tests run: 5, Failures: 2
guard forced to return true	Tests run: 5, Failures: 1

Suites:

• quarkus-morphium/runtime: Tests run: 73, Failures: 0, Errors: 0
• quarkus-morphium/integration-tests: Tests run: 255, Failures: 0, Errors: 0 — includes the existing MorphiumLocalDateTimeTest, unchanged, confirming the legacy path is untouched

Core is not modified by this PR.

Open question for the maintainer

Whether the deprecated property should be removed in 6.4.0 as the Javadoc states, or kept longer. Its only remaining purpose is the per-type split (native LocalDateTime, legacy Instant/LocalDate/LocalTime), which is a shape no application deliberately asked for — it was the incidental result of the extension being able to configure only one type.

[sboesebeck]

Thanks Heiko, the analysis behind this is correct and I checked it against the core: ObjectMapperImpl:139-143 really does register the four mappers with this::useBsonDateForJavaTime, and ObjectMapperImpl:208 reads objectMappingSettings().isUseBsonDateForJavaTime() on every call. So setting the flag on the config before the mapper is built is the right mechanism, and skipping the legacy override once the new property is set is the right call for the reason you give: new LocalDateTimeMapper(boolean) holds a fixed value and stops following the flag.

Leaving the new property without a default is also right. Java API compatibility is fine, the change is additive: the new method sits on a @ConfigMapping interface that nobody implements by hand, and the deprecation on LocalDateTimeConfig.useBsonDate() only produces warnings. With the property unset, registersDeprecatedLocalDateTimeOverride(Optional.empty()) restores the old path exactly.

Seven things before this goes in.

1. The removal target contradicts our own policy

LocalDateTimeConfig.java announces removal in 6.4.0. That is a minor. Everything else in the repo says removal happens in a major: docs/howtos/migration-v6_2-to-v6_3.md:336 documents the "7.0-removal wave (#218)", and 181 members currently carry @Deprecated(since = "6.3", forRemoval = true) pointing at 7.0. Please make this Removal: 7.0 in the Javadoc, the README table and the CHANGELOG.

2. Open question: should the deprecation land in a patch at all?

Not a rule, just something I want to settle before merging. Our migration guides are written per minor, so a deprecation that first appears in a patch is documented in no place a user looks. I am drafting a deprecation policy at the moment and this is one of its open points, so treat this as a question rather than a change request: would it bother you if the new property ships in 6.3.8 and the @Deprecated marker follows in 6.4?

3. use-bson-date-for-java-time=false does the opposite of what a reader expects

local-date-time.use-bson-date defaults to true, so LocalDateTime is ISODate today. Somebody who sets the new property to false, reasonably assuming it means "keep the old behaviour", flips LocalDateTime from ISODate to the legacy map format and starts writing a second representation into an existing collection. That consequence is currently only in the Javadoc of MorphiumRuntimeConfig.useBsonDateForJavaTime(). It needs to be in the README table and the CHANGELOG, in the same sentence as the property name.

4. No warning when both properties are set

The deprecated property is silently ignored (MorphiumProducer:656ff). Config properties have no compiler to warn for them, so please log once at startup when both are present, naming which one won and what the effect on LocalDateTime is. This is the only signal the affected users get.

5. The migration story is missing

Existing documents are not rewritten. That is fine for reading, all four unmarshall() methods accept both shapes (InstantMapper:54, LocalDateMapper:51, LocalTimeMapper:55, LocalDateTimeMapper:57), and worth stating explicitly in the CHANGELOG. But the features the property advertises, native range queries, sorts and TTL indexes, only work on the newly written documents. A TTL index over a field that still holds legacy sub-documents expires nothing. Please add a short migration note: turning this on does not convert existing data, and until documents are rewritten they will not match date queries.

6. Two docs still describe the old world

quarkus-morphium/README.md was updated, these were not:

• quarkus-morphium/docs/modules/ROOT/pages/configuration.adoc:92, section "LocalDateTime Storage", documents only local-date-time.use-bson-date with no deprecation marker. The section heading itself is now wrong, since the setting is no longer about LocalDateTime.
• docs/quarkus-extension.md:95 lists only the old property, and with default -- where the README says true.

7. Test instances are never closed

MorphiumProducerJavaTimeFormatTest.mapperWith() builds a Morphium and calls setConfig(), which connects, but nothing shuts the instance down. Three instances per class, with their InMemDriver threads left running. An @AfterEach or try-with-resources fixes it.

---

Items 1 and 3 are the ones I would like resolved before merge. The rest can follow in the same PR or a quick follow-up.

[sboesebeck]

Heiko, a short status note - and an answer to my own open question from point 2.

This PR moves to 6.4.0, not 6.3.8. The milestone is set accordingly. That is not a judgement on the work here, it is two things coming together:

First, 6.3.8 picked up a cluster data-loss fix that is time-critical: a healthy, data-bearing replica set node could wipe itself, because the dbHash comparison was sensitive to field order and two nodes holding identical data reported different hashes. The consistency check read that as divergence and resolved it with drop-then-copy. The release should not have to wait behind anything else.

Second - and this answers my question 2 - I think the new property and the deprecation belong in the same minor. Our migration guides are written per minor, so a deprecation that first appears in a patch is documented in no place a user actually looks. In 6.4.0 both land where they can be found.

That also resolves point 1 on its own: if the deprecation is introduced in 6.4.0, Removal: 7.0 is the consistent target - the same wave the other 181 @Deprecated(since = "6.3", forRemoval = true) members in the repo point at. Please put that in the javadoc, the README table and the CHANGELOG, and set since to 6.4.0.

That leaves point 3 as the only real blocker: use-bson-date-for-java-time=false flips LocalDateTime from ISODate to the legacy format - the opposite of what somebody expects who sets the property to false in order to keep things as they are. That needs to be in the README table and the CHANGELOG, in the same sentence as the property name; nobody setting a property reads the javadoc first.

Points 4 through 7 (warning when both properties are set, the migration note, the two stale doc pages, the unclosed test instances) stand as described: welcome in this PR, none of them blocking.

No time pressure now, 6.4.0 is still some way off. The analysis behind this was sound and I checked it against the core - the BooleanSupplier mechanism in ObjectMapperImpl is exactly as you describe, and skipping the legacy override once the new property is set is correct for the reason you give.