feat: add MarshalString and UnmarshalString method#1025
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces two new JSON helper methods—MarshalString and UnmarshalString—to simplify JSON (de)serialization throughout the codebase. Key changes include:
- Replacing manual conversions with the new JSON string methods in various modules (HTTP, session, logging, caching, etc.).
- Updating conversion logic in the support/convert package to use unsafe conversions.
- Adjusting tests to cover the new conversion functions.
Reviewed Changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| testing/http/assertable_json.go | Updated to use UnmarshalString for JSON deserialization. |
| support/json/json.go | Added new MarshalString and UnmarshalString methods. |
| support/file/file.go | Uses convert.UnsafeString instead of string conversion. |
| support/convert/convert_test.go | Added tests for new string/byte conversion utilities. |
| support/convert/convert.go | Introduced unsafe conversion functions with benchmarking comment. |
| session/session.go | Updated JSON (de)serialization in session Save and read functions. |
| log/formatter/general.go | Switched to MarshalString for logging field output. |
| http/limit/store.go | Replaced Unmarshal and Marshal conversion logic with string versions. |
| http/client/response.go & http/client/request.go | Updated to use UnmarshalString and unsafe string conversion. |
| hash/bcrypt.go & hash/argon2id.go | Changed to use unsafe byte conversion for hash operations. |
| foundation/json/json.go | Extended Json type with new MarshalString and UnmarshalString. |
| crypt/aes.go | Updated key and plaintext conversion to use unsafe methods. |
| contracts/foundation/json.go | Updated JSON interface to include string-based (de)serialization. |
| auth/jwt_guard.go | Modified to use unsafe conversion for secret handling. |
Files not reviewed (1)
- tests/go.mod: Language not supported
# Conflicts: # tests/go.mod # tests/go.sum
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: b9464fe | Previous: a232615 | Ratio |
|---|---|---|---|
BenchmarkFile_ReadWrite |
382669 ns/op 2073 B/op 28 allocs/op |
172419 ns/op 2072 B/op 28 allocs/op |
2.22 |
BenchmarkFile_ReadWrite - ns/op |
382669 ns/op |
172419 ns/op |
2.22 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1025 +/- ##
==========================================
- Coverage 71.16% 71.10% -0.06%
==========================================
Files 172 172
Lines 11807 11833 +26
==========================================
+ Hits 8402 8414 +12
- Misses 3052 3064 +12
- Partials 353 355 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hwbrzzl
left a comment
There was a problem hiding this comment.
Great optimization, LGTM.
| func CopyString(s string) string { | ||
| return string(UnsafeBytes(s)) | ||
| } |
There was a problem hiding this comment.
Nit: It's the same with a := b.
📑 Description
Closes https://github.com/goravel/goravel/issues/
Add MarshalString and UnmarshalString method and zero-copy string-bytes methods.
✅ Checks