Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

replace encoding/json with github.com/goccy/go-json #1360

Merged
merged 3 commits into from
Aug 7, 2023

Conversation

jacobbednarz
Copy link
Member

The stdlib has some, ermm, quirks with how certain methods handle strings
without a way to customise that behaviour. This bites us in cases where our API
payloads (represented as bytes) shouldn't be escaping HTML entities however
they end up escaped and won't match our internal representation of them.

Instead of doing hacky work arounds, I've just swapped out encoding/json with
github.com/goccy/go-json which mentions it is a drop in replacement but, it
includes a way to customise certain behaviours as needed.

As a side benefit, this will also considerably speed up JSON
marshaling/unmarshaling on larger and more complex payloads.

Closes #1350

@github-actions
Copy link
Contributor

github-actions bot commented Aug 7, 2023

changelog detected ✅

@codecov-commenter
Copy link

codecov-commenter commented Aug 7, 2023

Codecov Report

Merging #1360 (c4897fb) into master (b9ac804) will increase coverage by 0.14%.
Report is 106 commits behind head on master.
The diff coverage is 57.51%.

@@            Coverage Diff             @@
##           master    #1360      +/-   ##
==========================================
+ Coverage   48.33%   48.47%   +0.14%     
==========================================
  Files         133      135       +2     
  Lines       13023    13208     +185     
==========================================
+ Hits         6295     6403     +108     
- Misses       5201     5253      +52     
- Partials     1527     1552      +25     
Files Changed Coverage Δ
access_application.go 72.72% <ø> (ø)
access_audit_log.go 79.31% <ø> (ø)
access_bookmark.go 72.44% <ø> (ø)
access_ca_certificate.go 68.00% <ø> (ø)
access_group.go 70.00% <ø> (ø)
access_identity_provider.go 66.32% <ø> (ø)
access_keys.go 71.42% <ø> (ø)
access_mutual_tls_certificates.go 68.86% <ø> (ø)
access_organization.go 53.84% <ø> (ø)
access_service_tokens.go 51.85% <ø> (ø)
... and 117 more

The stdlib has some, ermm, quirks with how certain methods handle strings
without a way to customise that behaviour. This bites us in cases where our API
payloads (represented as bytes) *shouldn't* be escaping HTML entities however
they end up escaped and won't match our internal representation of them.

Instead of doing hacky work arounds, I've just swapped out `encoding/json` with
`github.com/goccy/go-json` which mentions it is a drop in replacement but, it
includes a way to customise certain behaviours as needed.
Ensures the exact cache key will match that which we have stored.

Closes #1350
@jacobbednarz jacobbednarz merged commit 1ff2145 into master Aug 7, 2023
@jacobbednarz jacobbednarz deleted the get-off-stdlib-json branch August 7, 2023 06:25
@github-actions github-actions bot added this to the v0.75.0 milestone Aug 7, 2023
github-actions bot pushed a commit that referenced this pull request Aug 7, 2023
@github-actions
Copy link
Contributor

This functionality has been released in v0.75.0.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with Purging Cache for API Responses in cloudflare-go library
2 participants