Populate sigs.k8s.io/json #1

liggitt · 2021-10-07T03:05:40Z

Best reviewed commit by commit

First few commits update template content, initialize the module, and set up CI (example run at https://github.com/liggitt/json/runs/3842971665) to run unit tests, vet/unsafe/dependency checks, and catch go API incompatibilities
The next set of commits copy the stdlib encoding/json package to an internal package, get the existing unit tests running, and alias the types to be compatible with encoding/json
The next set make tweaks to
- allow setting decoder options when unmarshaling
- allow accumulating/distinguishing strict errors while maintaining identical decoding behavior
- add case-sensitive decoder option
- add int preserving decoder option
- add disallow duplicate fields decoder option
The final commit exposes UnmarshalCaseSensitivePreserveInts, UnmarshalStrict, and NewDecoderCaseSensitivePreserveInts functions

benchmarks vs stdlib with strict error checking additions:

BenchmarkUnmarshal/typed_stdlib-12            25560     45859 ns/op   13240 B/op     161 allocs/op
BenchmarkUnmarshal/typed_unmarshal-12         26212     45366 ns/op   13272 B/op     161 allocs/op
BenchmarkUnmarshal/typed_strict-12            25543     47007 ns/op   13784 B/op     167 allocs/op

BenchmarkUnmarshal/untyped_stdlib-12          23452     50936 ns/op   28214 B/op     517 allocs/op
BenchmarkUnmarshal/untyped_unmarshal-12       23703     51473 ns/op   28186 B/op     513 allocs/op
BenchmarkUnmarshal/untyped_strict-12          21214     55943 ns/op   28412 B/op     523 allocs/op

liggitt · 2021-10-07T21:28:06Z

This is ready for review

/assign @deads2k @lavalamp
/hold for reviews

internal/golang/encoding/json/kubernetes_patch.go

internal/golang/encoding/json/kubernetes_patch_test.go

internal/golang/encoding/json/kubernetes_patch.go

internal/golang/encoding/json/decode.go

internal/golang/encoding/json/kubernetes_patch_test.go

json.go

deads2k · 2021-10-13T19:20:30Z

I'm happy with this. Squash in your fix commits and I'm good to lgtm

mkdir -p internal/golang/encoding/json tmpdir=$(mktemp -d -t golang.XXXXX) git clone --depth 1 --branch go1.17.1 git@github.com:golang/go.git $tmpdir cp -fr "${tmpdir}/src/encoding/json/*" "internal/golang/encoding/json/"

This ensures compatibility with target objects making use of stdlib exported types like RawMessage, or error return values making use of exported types like InvalidUnmarshalError

Using NewDecoder().Decode() adds ~10 additional allocs and ~doubles memory consumption for callers which already have []byte data over use of Unmarshal(), due to the Decoder only accepting a reader and then immediately reading the data into an internal buffer. To avoid that cost, this commit exposes the ability to set decoder options while using Unmarshal().

This commit adds the ability to distinguish "strict" errors, and treats unknown fields (as enabled by DisallowUnknownFields()) as a strict errors. Strict errors have the following behavior: - strict errors do not short-circuit Unmarshal() or change what is stored in the provided value - strict errors are only returned if Unmarshal() was otherwise successful (syntax errors or type errors take precedence) - strict errors are accumulated and deduped (up to an internal limit) - strict errors are returned in an instance of *UnmarshalStrictError

This adds a CaseSensitive decoder option (defaulting to false). When decoding a JSON object to a struct, JSON keys are treated case-sensitively when locating corresponding struct fields. Keys must exactly match `json` tag names (for tagged struct fields) or struct field names (for untagged struct fields), or are treated as unknown fields.

This adds a PreserveInts decoder option (defaulting to false). When decoding JSON numbers to interface{} fields, numbers are decoded as int64 when possible, falling back to `float64` on any error.

This adds a DisallowDuplicateFields decoder option (defaulting to false). Duplicate JSON object keys encountered produce strict errors. - When decoding to a struct, duplicate keys are keys that map to the same struct field. - When decoding to a map, duplicate keys are identical strings (case-sensitive). Duplicate detection is implemented as inlined closures to minimize heap allocs.

…coderCaseSensitivePreserveInts

deads2k · 2021-10-20T16:51:32Z

/lgtm
/approve

k8s-ci-robot · 2021-10-20T16:51:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: deads2k, liggitt

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [deads2k,liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

liggitt · 2021-10-20T16:57:04Z

/hold cancel

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 7, 2021

k8s-ci-robot requested review from deads2k and lavalamp October 7, 2021 03:05

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 7, 2021

liggitt force-pushed the stdlib branch 3 times, most recently from c0d26d9 to af560dc Compare October 7, 2021 05:01

liggitt changed the title ~~WIP: Populate sigs.k8s.io/json~~ Populate sigs.k8s.io/json Oct 7, 2021

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 7, 2021

liggitt mentioned this pull request Oct 7, 2021

switch from json-iterator to forked stdlib json decoder kubernetes/kubernetes#105030

Merged

4 tasks

liggitt force-pushed the stdlib branch 14 times, most recently from e3a002f to c53f720 Compare October 7, 2021 21:24

k8s-ci-robot assigned deads2k and lavalamp Oct 7, 2021

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 7, 2021