fix(server): keep schema ~create_time as Date after reload by dpol1 · Pull Request #3026 · apache/hugegraph

dpol1 · 2026-05-14T13:24:55Z

close [Bug] schema ~create_time userdata round-trips Date as String after cache reload #3013

Userdata.CREATE_TIME ("~create_time") is written as java.util.Date
by SchemaTransaction.setCreateTimeIfNeeded(), then persisted through backend
serializers as JSON. On reload, userdata is deserialized through raw
Map.class paths, so Jackson cannot restore the value type and
schemaElement.userdata().get(Userdata.CREATE_TIME) can come back as String.

That breaks callers and existing tests that expect ~create_time to keep the
server-side Date contract after a schema is reloaded from backend storage.

Main Changes
- Added Userdata.normalizeValue(key, value) as a static helper on the Userdata class.
  - Converts Userdata.CREATE_TIME string values back with DateUtil.parse(...).
  - Leaves existing Date values and all other userdata keys unchanged.
  - Throws a clearer IllegalArgumentException if a ~create_time string is invalid.
- Routed both userdata setters through Userdata.put(key, value), which applies
  normalization before storing:
  - userdata(String, Object) covers serializer reload paths such as
    TextSerializer, BinarySerializer, MysqlSerializer, and
    CassandraSerializer.
  - userdata(Userdata) covers schema fromMap() hydration paths for
    PropertyKey, VertexLabel, EdgeLabel, and IndexLabel.
- Added a null check to userdata(Userdata) for consistent argument validation.
- Added regression coverage for schema userdata normalization and serializer
  round trips.
This PR is intentionally limited to the server-side SchemaElement. The mirror
class in hugegraph-struct has a similar userdata shape and can be handled in a
separate follow-up PR.

Verifying these changes
- Trivial rework / code cleanup without any test coverage. (No Need)
- Already covered by existing tests, such as (please modify tests here).
- Need tests and can be verified as follows:
  - SchemaElementTest covers single-key normalization, bulk normalization,
    idempotency for Date, invalid ~create_time strings, null bulk userdata,
    and non-CREATE_TIME keys.
  - TextSerializerTest#testPropertyKeyUserdataCreateTimeRoundTripsAsDate
    verifies the text serializer schema round trip keeps CREATE_TIME as Date.
  - BinarySerializerTest#testPropertyKeyUserdataCreateTimeRoundTripsAsDate
    verifies the binary serializer schema round trip keeps CREATE_TIME as Date.
  - The new test class is registered in UnitTestSuite.
Does this PR potentially affect the following parts?
- Dependencies (add/update license (https://hugegraph.apache.org/docs/contribution-guidelines/contribute/#321-check-licenses) info & ../install-dist/scripts/dependency/regenerate_known_dependencies.sh)
- Modify configurations
- The public API
- Other affects (typed here)
- Nope
The public SchemaElement.userdata(...) signatures are unchanged. The behavior change is limited to internal userdata normalization for Userdata.CREATE_TIME, handled via Userdata.normalizeValue / Userdata.put().

Documentation Status
- Doc - TODO
- Doc - Done
- Doc - No Need

Notes

This PR fixes the server-side schema reload path for Userdata.CREATE_TIME (~create_time) only.
~create_time is a server-reserved userdata key; valid string values are normalized back to java.util.Date, while malformed values now fail with IllegalArgumentException.
Follow-up: [Bug] Preserve typed DEFAULT_VALUE and struct userdata after JSON reload #3028 tracks preserving typed Userdata.DEFAULT_VALUE (~default_value) reloads and checking the same raw userdata deserialization gap in hugegraph-struct.

Normalize server-side schema ~create_time userdata in SchemaElement so serializer reloads and fromMap paths keep the Date contract. Add SchemaElement, TextSerializer, and BinarySerializer coverage. Closes apache#3013

Copilot

Pull request overview

This PR fixes a schema reload regression where internal schema userdata Userdata.CREATE_TIME ("~create_time") can round-trip through backend JSON serializers as a String and come back as String after reload, violating the server-side contract that it should be a java.util.Date.

Changes:

Added SchemaElement.normalizeUserdataValue(key, value) to re-type ~create_time string values back into Date (and throw a clearer error on invalid values).
Routed both SchemaElement.userdata(String, Object) and SchemaElement.userdata(Userdata) through the normalization logic (and added a null-argument check for the bulk setter).
Added regression tests for normalization behavior and for Text/Binary serializer round-trips, and registered them in UnitTestSuite.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
hugegraph-server/hugegraph-core/src/main/java/org/apache/hugegraph/schema/SchemaElement.java	Normalizes `~create_time` userdata on set (single and bulk), ensuring reload paths restore `Date` instead of `String`.
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/core/SchemaElementTest.java	Unit coverage for normalization behavior, idempotency, invalid strings, and null bulk userdata validation.
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/serializer/TextSerializerTest.java	Regression test verifying TextSerializer schema round-trip keeps `CREATE_TIME` as `Date`.
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/serializer/BinarySerializerTest.java	Regression test verifying BinarySerializer schema round-trip keeps `CREATE_TIME` as `Date`.
hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/unit/UnitTestSuite.java	Registers the new unit tests in the suite.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

imbajin

Reviewed correctness, design, and tests. No blocking issues — fix is correct and narrowly scoped, all backend reload paths (Text / Binary / Mysql / Cassandra / HBase / Postgres / Palo / Hstore + the four fromMap paths) route through the new helper, and DateUtil.parse is symmetric with the "yyyy-MM-dd HH:mm:ss.SSS" format used by JsonUtil/HugeGraphSONModule. Inline comments cover important refinements (helper placement, public setter docs, test depth). A few additional minor notes below that don't have a clean inline target:

🧹 removeUserdata(Userdata) (SchemaElement.java:114-118) lacks a null check — adding E.checkArgumentNotNull(userdata, ...) keeps the bulk setters symmetric with the new check on userdata(Userdata).
🧹 Behavior change worth a release note: any REST client that posts ~create_time as a JSON string in user-data now silently receives a Date (or an IllegalArgumentException if malformed). ~create_time is a server-reserved key (~ prefix, written by SchemaTransaction.setCreateTimeIfNeeded), so risk is small, but flagging it in the PR body / changelog is helpful.
🧹 Follow-up tracking: Userdata.DEFAULT_VALUE has the same Jackson type-erasure problem (e.g., a property key whose data type is Date will reload its default value as String). The PR description already defers the hugegraph-struct mirror — please add DEFAULT_VALUE to the same follow-up list, since the root cause and fix shape are identical.

codecov · 2026-05-15T10:44:10Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 32.44%. Comparing base (8d095e1) to head (1977a44).

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #3026      +/-   ##
============================================
- Coverage     36.15%   32.44%   -3.72%     
- Complexity      403      493      +90     
============================================
  Files           812      803       -9     
  Lines         68307    68053     -254     
  Branches       8927     8907      -20     
============================================
- Hits          24694    22077    -2617     
- Misses        40978    43498    +2520     
+ Partials       2635     2478     -157

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

imbajin

No blocking issues. The core fix (normalizing CREATE_TIME String→Date on deserialization) is correct and the main round-trip paths are tested. Four non-blocking observations below.

imbajin · 2026-05-18T08:23:51Z

+                          "was " + (value == null ? "null" : value.getClass()),
+                          value instanceof Date);
+        Assert.assertEquals(created, value);
+    }


⚠️ The PR description states that MysqlSerializer and CassandraSerializer are also affected by the same CREATE_TIME deserialization gap:

"userdata(String, Object) covers serializer reload paths such as TextSerializer, BinarySerializer, MysqlSerializer, and CassandraSerializer"

Only TextSerializer and BinarySerializer have round-trip tests added. Adding equivalent testPropertyKeyUserdataCreateTimeRoundTripsAsDate tests for MysqlSerializer and CassandraSerializer would close this coverage gap for the stated fix surface.

Holding off, with reasoning:

MysqlSerializer.readUserdata (L167) and CassandraSerializer.readUserdata
(L222) both call the same schema.userdata(key, value) setter as
TextSerializer/BinarySerializer. That setter now routes through the
single Userdata.put normalization path, which is already exercised by:

TextSerializerTest / BinarySerializerTest round-trips

SchemaElementTest (constructor, single setter, bulk, fromMap)

There's no existing unit-test harness for the SQL serializers — adding one
means backend/mock infra for zero new code paths. The marginal coverage
doesn't justify it.

Better closure for the shared logic: a direct UserdataTest.normalizeValue
unit test, serializer-agnostic. Happy to add that as a follow-up; proposing
not to gate this PR on it. Pushback welcome if you see a path difference
I'm missing.

…serdata

…holder The builder accumulates userdata via Userdata.put() before eliminate() runs, so `.userdata(CREATE_TIME, "").eliminate()` parsed "" as a date and threw before the key-only removal path. Pass a blank ~create_time through unchanged; non-empty malformed values still throw on the add path, so the existing contract is unchanged.

dpol1 · 2026-05-19T08:51:14Z

@imbajin WDYT about this approach?

imbajin

LGTM, thanks for addressing the eliminate placeholder case. The current approach looks good to me.

One small follow-up: the PR description still mentions SchemaElement.normalizeUserdataValue, but the final implementation is centered on Userdata.normalizeValue / Userdata.put(). It would be good to update the description so it matches the current code.

dpol1 · 2026-05-19T09:18:34Z

Changed

dpol1 · 2026-05-19T09:38:52Z

Thanks @imbajin for lead the direction of this PR with your sharp and precise review

fix(server): restore schema create_time Date

62680de

Normalize server-side schema ~create_time userdata in SchemaElement so serializer reloads and fromMap paths keep the Date contract. Add SchemaElement, TextSerializer, and BinarySerializer coverage. Closes apache#3013

dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working tests Add or improve test cases labels May 14, 2026

imbajin requested a review from Copilot May 14, 2026 14:06

Copilot started reviewing on behalf of imbajin May 14, 2026 14:06 View session

Copilot AI reviewed May 14, 2026

View reviewed changes

imbajin reviewed May 15, 2026

View reviewed changes

fix(schema): preserve typed userdata values in struct and default values

11adabf

dpol1 requested a review from imbajin May 15, 2026 11:12

dpol1 added 2 commits May 18, 2026 09:14

Merge branch 'master' into fix/3013-create-time-userdata-roundtrip

bc48eec

test(schema): cover userdata constructor normalization

dccc0f0

imbajin reviewed May 18, 2026

View reviewed changes

refactor(schema): centralize userdata create_time normalization in U…

8110bc4

…serdata

imbajin reviewed May 18, 2026

View reviewed changes

Comment thread ...er/hugegraph-test/src/main/java/org/apache/hugegraph/unit/serializer/TextSerializerTest.java

dpol1 requested a review from imbajin May 18, 2026 11:55

imbajin approved these changes May 19, 2026

View reviewed changes

dosubot Bot added the lgtm This PR has been approved by a maintainer label May 19, 2026

imbajin changed the title ~~fix(server): keep schema ~create_time userdata as Date after reload~~ fix(server): keep schema ~create_time as Date after reload May 19, 2026

imbajin merged commit 454dd3d into apache:master May 19, 2026
16 of 17 checks passed

dpol1 deleted the fix/3013-create-time-userdata-roundtrip branch May 19, 2026 09:36

Conversation

dpol1 commented May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Main Changes

Verifying these changes

Does this PR potentially affect the following parts?

Documentation Status

Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

imbajin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

imbajin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

imbajin May 18, 2026

Choose a reason for hiding this comment

Uh oh!

dpol1 May 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dpol1 commented May 19, 2026

Uh oh!

imbajin left a comment

Choose a reason for hiding this comment

Uh oh!

dpol1 commented May 19, 2026

Uh oh!

Uh oh!

dpol1 commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dpol1 commented May 14, 2026 •

edited

Loading

codecov Bot commented May 15, 2026 •

edited

Loading