Spec: TextField in a numeric column makes the whole contentlet unindexable (#37272)

[fabrizzio-dotCMS]

PR 1 of 2 — spec only. Carries spec.md and nothing else. Please review it as a spec, not as code: is this the right problem, scoped right, with measurable criteria? Once approved I branch off this branch and open PR 2 with the implementation. No need to wait for this to merge.

Refs #37272

TL;DR

What's broken: a Text field whose value is stored as a String but whose backing column is numeric (integer1) makes the entire contentlet disappear from the index. Silent — the save works, the UI flags nothing, the reindex shows only a failure count.

Why: ESMappingAPIImpl.loadFields picks the serialization branch by the storage column name instead of by the value. "integer1" starts with integer, so the String reaches DecimalFormat.format(), which accepts only Number and always throws. The per-field catch then logs at WARN and rethrows, aborting the whole document.

Context worth knowing before reviewing:

• The modelling is legitimate and supported — TextField.acceptedDataTypes() includes INTEGER/FLOAT, and dotCMS ships content types shaped this way (htmlpageasset.sortOrder, Vanity URL.order). What decides the failure is the Java class of the value, not the modelling.
• Not a regression. Both defects are verbatim in e8ef584ec9 — initial trunk import, 2012. The ES→OpenSearch work neither introduced nor worsened it; the bulk refresh just pushed enough documents through the same path to expose it.
• ESMappingAPIImpl is the shared mapper for both engines (OSBulkHelper:250 calls the same toMap), so the fix covers ES and OS at once.

Proposed fix

Best-effort conversion at index time. If the value converts, the numeric branch behaves exactly as today. If it does not, emit 0 under the numeric key (consistent with the field's long mapping) with the original text preserved in _dotraw, plus a WARN naming the field and content type. And a failing field no longer kills the document.

Out of scope, with the reasoning written down

• Content-type-save-time validation (the issue's "consider" bullet) — that validation already exists and already says this modelling is legal (FieldFactoryImpl:478). Forbidding the combination means removing INTEGER from TextField.acceptedDataTypes(), which breaks dotCMS's own built-in content types at bootstrap.
• Field.fieldValue() respecting dataType() — the preventive counterpart, and it would also close the ImportStarterUtil route, but it changes every save of every contentlet in the system. Different blast radius, deserves its own cycle.
• Changing the index mapping generator; repairing existing stored values; backfilling lost documents.

Identified risk

ImportStarterUtil writes contentlet JSON verbatim, bypassing the API entirely (:894 → toMutableContentlet, :782 → factory save). Every user-facing path coerces (new editor, legacy editor, Content Import portlet and the new import job all funnel through setContentletProperty → integerStrategy), so the starter import is the most plausible provenance of the observed rows — and it explains why the defect cannot be reproduced through the UI. Named as a known gap; not fixed here.

What to scrutinise

AC-005. htmlpageasset.sortOrder has exactly this field shape, and its zero-padded _dotraw (0000000000000000054.000000000000000000) is what makes lexicographic sorting equal numeric sorting for page ordering. That document must stay byte-identical. A type-based rule instead of a value-based one would have broken page ordering on every installation — that trade-off is recorded under Resolved Decisions → C-1.

🤖 Generated with Claude Code

[claude]

Claude finished @fabrizzio-dotCMS's task in 1m 22s —— View job

---

Spec review — TextField in a numeric column makes the contentlet unindexable (#37272)

Reviewed as a spec (PR 1 of 2): right problem, right scope, measurable criteria. I verified the load-bearing code claims against the branch — they hold.

Verified against the code

• Wrong discriminator — ESMappingAPIImpl.loadFields branches on field.getFieldContentlet() (the storage column name), startsWith FLOAT/INTEGER at ESMappingAPIImpl.java:1113-1116, then hands the value straight to numFormatter.format(valueObj) at :1118. numFormatter is a DecimalFormat (:996), which accepts only Number — a String throws IllegalArgumentException. ✅ as described.
• Fatal to the whole document — the per-field catch at :1142-1145 logs at WARN and rethrows as DotDataException, aborting toMap. The Date branch immediately above (:1120-1128) degrades to toString() instead. ✅ the asymmetry the spec calls out is real.
• Modelling is legitimate — TextField.acceptedDataTypes() returns TEXT, LONG_TEXT, FLOAT, INTEGER (TextField.java:33), so the value-based (not type-based) discriminator is the correct call. ✅
• Padding / byte-identical claim (AC-005/AC-008) — the padded _dotraw comes from the 0000000000000000000.000000000000000000 pattern at :996. Converting "54" → Long 54 and re-running it through the same formatter yields the same string, so "byte-identical to a natively-stored number" is achievable exactly as claimed. ✅

The problem framing, the "not a regression" evidence, and the out-of-scope reasoning (esp. why content-type-save validation is the wrong lever, and the ImportStarterUtil provenance) are unusually well-argued and self-consistent. No objections to scope.

Questions / points to nail down before PR 2

• 🟡 AC-007 — is 0 the right sentinel for an unconvertible value? Indexing garbage text as 0 under the numeric key makes it indistinguishable from a genuine 0 in range queries and sorts (field:[0 TO 10], sort-by). An operator eyeballing results won't see the difference; only the WARN reveals it. The alternative — omit the numeric key entirely for that document and keep only _dotraw — leaves the field unset for that one doc (no false 0) while still avoiding the mapper parse exception. The spec's own "Data considerations" note (spec.md:196-197) says the text-degradation path must not write a non-numeric string under the numeric key, which 0 satisfies — but it doesn't weigh 0-vs-omit. Worth an explicit line in Resolved Decisions on why 0 beats omission (queryability? consistency with long mapping default?), since AC-007 hard-codes the choice.

• 🟡 The best-effort helper must cover the FLOAT column too, not just integer. The failing branch at :1113-1116 covers both float% and integer% columns. The in-scope text lists NumberUtil.toInt / toLong / pad but no float conversion — a "54.3" in a float-backed Text field is squarely in scope (AC-001 says "numeric column", not "integer column"). Make sure the helper and a test case cover the float path, otherwise a float-column String still falls to the 0 fallback when it was in fact convertible.

• 🟡 AC-005/AC-006 "byte-identical" — pin the assertion layer. The verification method says unit tests assert the emitted map entries, which is the right level (pre-serialization Map, not literal JSON bytes). Recommend the AC wording say "identical emitted map entries" to avoid a reviewer of PR 2 reading "byte-identical" as literal serialized-JSON comparison, which would be brittle across key ordering.

• 🟢 Minor — AC-004 covers the log-level fix but there's no AC asserting the WARN-then-rethrow is gone on the success path. AC-002 covers "other fields still indexed"; consider one explicit assertion that a converted value produces no WARN at all (today's happy path for a natively-stored number must stay silent), so the fix doesn't start logging on every sortOrder.

None of these are blocking — the spec is sound and the fix direction is correct. They're the details I'd want resolved in /speckit-plan / the ACs before the Red tests are written.

· 37272-textfield-numeric-column