Skip to content

Add MarshalJSON to xtime.Duration - #242

Merged
harshavardhana merged 1 commit into
minio:mainfrom
sueun-dev:xtime-duration-marshaljson
Jul 16, 2026
Merged

harshavardhana merged 1 commit into
minio:mainfrom
sueun-dev:xtime-duration-marshaljson

Conversation

@sueun-dev

@sueun-dev sueun-dev commented Jul 9, 2026 •

Copy link
Copy Markdown
Contributor

Duration has an UnmarshalJSON that expects a human-readable string (it strips the surrounding quotes and calls ParseDuration), but no MarshalJSON. json.Marshal therefore emits the raw int64 nanoseconds, which UnmarshalJSON cannot read back:

b, _ := json.Marshal(xtime.Duration(time.Hour))   // "3600000000000"
var d xtime.Duration
json.Unmarshal(b, &d)                              // missing unit in duration "60000000000"

The type is documented as supporting YAML and JSON. #136 added MarshalYAML to match the existing UnmarshalYAML; this does the same for JSON, emitting the human-readable string that UnmarshalJSON reads.

Tested with go test -race ./xtime/ (the existing TestDuration_Marshal is extended to cover the JSON marshal output and a round-trip). gofmt, go vet, and golangci-lint are clean.

Summary by CodeRabbit

  • New Features
    • Custom durations now serialize to JSON as readable duration strings.
    • Added coverage to ensure duration values round-trip correctly through JSON, including pointer fields.

Duration has UnmarshalJSON that expects a human-readable string (it
strips the surrounding quotes and calls ParseDuration), but no
MarshalJSON. So json.Marshal emits the raw int64 nanoseconds, which
UnmarshalJSON cannot read back:

    {"a":1000000000,"dur":0,"durationPointer":604801000000000}
    missing unit in duration "00000000"

The type is documented as supporting YAML and JSON. #136 added
MarshalYAML to match the existing UnmarshalYAML; this does the same
for JSON, emitting the human-readable string that UnmarshalJSON reads.
@coderabbitai

coderabbitai Bot commented Jul 9, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e3587b99-f55b-4143-8d34-492b6726b80d

📥 Commits

Reviewing files that changed from the base of the PR and between d166243 and 334c275.

📒 Files selected for processing (2)
  • xtime/time.go
  • xtime/time_unmarshal_test.go

📝 Walkthrough

Walkthrough

Adds a MarshalJSON method to the Duration type in xtime/time.go, serializing durations as human-readable strings via time.Duration.String(). The corresponding test file is updated to verify JSON marshaling output and round-trip unmarshaling correctness.

Changes

Duration JSON Marshaling

Layer / File(s) Summary
MarshalJSON implementation and round-trip test
xtime/time.go, xtime/time_unmarshal_test.go
Adds encoding/json import and Duration.MarshalJSON method returning the string form of the duration; test now marshals a testDuration struct to JSON, checks the exact expected output, and unmarshals it back to verify fields including a non-nil pointer round-trip.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Poem

A hop, a skip, a JSON hop,
Durations now in strings will pop!
"2h", "30m", so neat and clear,
Marshaled out without a fear.
Test cases checked, round-trip complete —
This bunny's work is small but sweet. 🐰

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: adding MarshalJSON to xtime.Duration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@harshavardhana
harshavardhana merged commit 11e724c into minio:main Jul 16, 2026
11 checks passed
Vonng added a commit to pgsty/silo-pkg that referenced this pull request Aug 2, 2026
Cherry-pick of minio#242 (11e724c).

Duration had UnmarshalJSON but no MarshalJSON, so it encoded as a bare
int64 of nanoseconds while the decoder unconditionally strips the first
and last byte expecting a quoted string. The round trip was broken in
both directions: json.Unmarshal on 5400000000000 fails with
`missing unit in duration "40000000000"`. Encoding as time.Duration's own
string form makes the pair symmetric.

Nothing in minio encodes these fields as JSON today -- batch job
definitions persist as YAML, DescribeBatchJob answers YAML, and the msgp
path is int64 -- but cmd/batch-expire.go and cmd/batch-replicate.go do
carry `json:` tags on xtime.Duration fields, so the trap is one refactor
away from being live.

This changes the JSON wire form. Anything that already persisted these
fields as numbers will not read back; nothing found in minio or mc does.

(cherry picked from commit 96870e988f011b0075383948cf178d0512beacdf)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants