Skip to content

fix(encoding): generate transitive MarshalJSON for wrapper messages with int64_encoding=NUMBER nested fields#125

Merged
SebastienMelki merged 5 commits intomainfrom
fix/int64-encoding-nested-marshaljson
Mar 2, 2026
Merged

fix(encoding): generate transitive MarshalJSON for wrapper messages with int64_encoding=NUMBER nested fields#125
SebastienMelki merged 5 commits intomainfrom
fix/int64-encoding-nested-marshaljson

Conversation

@hasanMshawrab
Copy link
Collaborator

Summary

Fixes a bug where response/wrapper messages containing nested messages with int64_encoding=NUMBER fields had their int64 values serialized as strings instead of JSON numbers.

  • marshalResponse checks for json.Marshaler before dispatching — only the inner message had MarshalJSON() generated, not the outer wrapper
  • protojson.Marshal (the fallback) uses protobuf reflection and never calls json.Marshaler methods on nested messages
  • Both httpgen and clientgen had the identical gap

Root cause

// marshalResponse in *_http_binding.pb.go
if marshaler, ok := response.(json.Marshaler); ok {
    return marshaler.MarshalJSON()  // works for inner message directly
}
return protojson.Marshal(msg)       // called for wrapper — bypasses inner MarshalJSON

collectInt64EncodingMessages only generated MarshalJSON() for messages with direct NUMBER fields. Wrapper messages that merely referenced such types were not detected.

Fix

Two-phase collection in generateInt64EncodingFile:

  1. Phase 1 (existing): detect messages with direct NUMBER-encoded int64 fields
  2. Phase 2 (new): detect wrapper messages whose fields are of a type from phase 1
  3. Generate wrapper MarshalJSON/UnmarshalJSON that use protojson.Marshal for base serialization, then re-marshal each nested field via json.Marshal — triggering the inner message's custom method

Changes

  • internal/httpgen/encoding.go — added Int64WrapperContext, collectWrapperContexts, collectWrapperMessages, generateWrapperMarshalJSON, generateWrapperUnmarshalJSON
  • internal/clientgen/encoding.go — identical fix
  • internal/httpgen/testdata/proto/int64_nested_encoding.proto — new test proto with SensorReading (inner, NUMBER fields) and GetSensorReadingResponse/GetMultiSensorResponse (wrappers)
  • internal/clientgen/testdata/proto/int64_nested_encoding.proto — symlink to above
  • Golden files updated for both generators

Test plan

  • go test -run TestHTTPGenGoldenFiles ./internal/httpgen — PASS (13 cases)
  • go test -run TestClientGenGoldenFiles ./internal/clientgen — PASS (14 cases)
  • ./scripts/run_tests.sh --fast — all relevant packages PASS
  • Golden file int64_nested_encoding_encoding.pb.go contains MarshalJSON/UnmarshalJSON for SensorReading, GetSensorReadingResponse, and GetMultiSensorResponse

Closes #124

🤖 Generated with Claude Code

@github-actions
Copy link

github-actions bot commented Feb 27, 2026

🔍 CI Pipeline Status

Lint: success
Test: success
Coverage: success
Build: success
Integration: success


📊 Coverage Report: Available in checks above
🔗 Artifacts: Test results and coverage reports uploaded

hasanMshawrab and others added 5 commits February 28, 2026 00:03
…ing in wrapper responses

Add SensorReading (inner, direct NUMBER fields) and GetSensorReadingResponse/GetMultiSensorResponse
(wrapper messages) test proto. Golden files for both httpgen and clientgen cover the new cases.
Test cases added to both golden_test.go files.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…th nested NUMBER fields

When a wrapper/response message contains a field whose type has int64_encoding=NUMBER fields,
protojson.Marshal bypasses the inner message's MarshalJSON — producing strings instead of numbers.

Fix adds a two-phase collection in generateInt64EncodingFile:
- Phase 1 (existing): collect messages with direct NUMBER fields
- Phase 2 (new): collect wrapper messages whose fields reference phase-1 types

Wrapper messages get generated MarshalJSON/UnmarshalJSON that use protojson for base
serialization, then re-marshal each nested NUMBER field via json.Marshal to trigger
the inner message's custom MarshalJSON.

Closes #124

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…with nested NUMBER fields

Identical fix to httpgen: two-phase collection detects wrapper messages containing fields
whose type has int64_encoding=NUMBER, generating MarshalJSON/UnmarshalJSON that re-marshal
those nested fields via json.Marshal to trigger the inner message's custom marshaling.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…coding.go

golangci-lint (golines) flagged collectWrapperMessages and two gf.P() calls
in generateWrapperMarshalJSON/UnmarshalJSON for exceeding the line length limit.
Auto-fixed with golangci-lint --fix; golden files regenerated to match.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@hasanMshawrab hasanMshawrab force-pushed the fix/int64-encoding-nested-marshaljson branch from 3f3b2f5 to e8e52a6 Compare February 27, 2026 22:04
@codecov
Copy link

codecov bot commented Feb 27, 2026

Codecov Report

❌ Patch coverage is 0% with 218 lines in your changes missing coverage. Please review.
✅ Project coverage is 3.09%. Comparing base (917521f) to head (e8e52a6).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
internal/clientgen/encoding.go 0.00% 109 Missing ⚠️
internal/httpgen/encoding.go 0.00% 109 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main    #125      +/-   ##
========================================
- Coverage   3.17%   3.09%   -0.09%     
========================================
  Files         47      47              
  Lines       7485    7701     +216     
========================================
  Hits         238     238              
- Misses      7243    7459     +216     
  Partials       4       4              
Flag Coverage Δ
unittests 3.09% <0.00%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@SebastienMelki SebastienMelki merged commit ac560bb into main Mar 2, 2026
8 of 10 checks passed
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.

fix(httpgen): wrapper response messages missing transitive MarshalJSON for int64_encoding=NUMBER

2 participants