Skip to content

Commit

Permalink
Preserve semantic options in Encoder and Decoder (#152)
Browse files Browse the repository at this point in the history 
A jsontext.Encoder and jsontext.Decoder could be constructed
with semantic options (e.g., json.Deterministic) where
such options are ignored for all encode/decode specific operations.

However, allow semantic options to take effect when such an
Encoder or Decoder is passed to MarshalEncode or UnmarshalDecode.
The semantic option can still be overridden.

One reason for this behavior is for easier migration.
For example, this v1 code:

	dec := jsonv1.NewDecoder(in)
	dec.DisallowUnknownFields()
	for {
		... := dec.Decode(...)
	}

can be migrated as:

	dec := jsontext.NewDecoder(in,
		json.RejectUnknownMembers(true))
	for {
		... := json.UnmarshalDecode(dec, ...)
	}

Notice that RejectUnknownMembers does not need
to be repeatedly passed to every UnmarshalDecode call
because it is implicitly stored on the Decoder.

The alternative behavior is to have the construction of
an Encoder or Decoder explicitly drop any semantic options.
However, this seems like extra work for no benefit.
  • Loading branch information
@dsnet
dsnet authored Feb 12, 2025
1 parent 07af0ca commit 60a0516
Show file tree
Hide file tree
Showing 4 changed files with 250 additions and 56 deletions.
17 changes: 11 additions & 6 deletions arshal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,8 @@ func mayReuseOpt(coderOpts *jsonopts.Struct, opts []Options) *jsonopts.Struct {
return coderOpts
}
// If the caller provides no options, then just reuse the coder's options,
// which should only contain encoding/decoding related flags.
// which may contain both marshaling/unmarshaling and encoding/decoding flags.
case 0:
// TODO: This is buggy if coderOpts ever contains non-coder options.
return coderOpts
}
return nil
Expand Down Expand Up @@ -224,17 +223,20 @@ func MarshalWrite(out io.Writer, in any, opts ...Options) (err error) {

// MarshalEncode serializes a Go value into an [jsontext.Encoder] according to
// the provided marshal options (while ignoring unmarshal, encode, or decode options).
// Any marshal-relevant options already specified on the [jsontext.Encoder]
// take lower precedence than the set of options provided by the caller.
// Unlike [Marshal] and [MarshalWrite], encode options are ignored because
// they must have already been specified on the provided [jsontext.Encoder].
//
// See [Marshal] for details about the conversion of a Go value into JSON.
func MarshalEncode(out *jsontext.Encoder, in any, opts ...Options) (err error) {
xe := export.Encoder(out)
mo := mayReuseOpt(&xe.Struct, opts)
if mo == nil {
mo = getStructOptions()
defer putStructOptions(mo)
mo.Join(opts...)
mo.CopyCoderOptions(&xe.Struct)
*mo = xe.Struct // initialize with encoder options before joining
mo.JoinWithoutCoderOptions(opts...)
}
err = marshalEncode(out, in, mo)
if err != nil && mo.Flags.Get(jsonflags.ReportErrorsWithLegacySemantics) {
Expand Down Expand Up @@ -467,8 +469,11 @@ func unmarshalFull(in *jsontext.Decoder, out any, uo *jsonopts.Struct) error {

// UnmarshalDecode deserializes a Go value from a [jsontext.Decoder] according to
// the provided unmarshal options (while ignoring marshal, encode, or decode options).
// Any unmarshal options already specified on the [jsontext.Decoder]
// take lower precedence than the set of options provided by the caller.
// Unlike [Unmarshal] and [UnmarshalRead], decode options are ignored because
// they must have already been specified on the provided [jsontext.Decoder].
//
// The input may be a stream of one or more JSON values,
// where this only unmarshals the next JSON value in the stream.
// The output must be a non-nil pointer.
Expand All @@ -479,8 +484,8 @@ func UnmarshalDecode(in *jsontext.Decoder, out any, opts ...Options) (err error)
if uo == nil {
uo = getStructOptions()
defer putStructOptions(uo)
uo.Join(opts...)
uo.CopyCoderOptions(&xd.Struct)
*uo = xd.Struct // initialize with decoder options before joining
uo.JoinWithoutCoderOptions(opts...)
}
err = unmarshalDecode(in, out, uo)
if err != nil && uo.Flags.Get(jsonflags.ReportErrorsWithLegacySemantics) {
Expand Down
124 changes: 124 additions & 0 deletions arshal_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9300,6 +9300,66 @@ func TestUintSet(t *testing.T) {
}
}

func TestUnmarshalDecodeOptions(t *testing.T) {
var calledFuncs int
var calledOptions Options
in := strings.NewReader(strings.Repeat("\"\xde\xad\xbe\xef\"\n", 5))
dec := jsontext.NewDecoder(in,
jsontext.AllowInvalidUTF8(true), // decoder-specific option
WithUnmarshalers(UnmarshalFromFunc(func(_ *jsontext.Decoder, _ any, opts Options) error {
if v, _ := GetOption(opts, jsontext.AllowInvalidUTF8); !v {
t.Errorf("nested Options.AllowInvalidUTF8 = false, want true")
}
calledFuncs++
calledOptions = opts
return SkipFunc
})), // unmarshal-specific option; only relevant for UnmarshalDecode
)

if err := UnmarshalDecode(dec, new(string)); err != nil {
t.Fatalf("UnmarshalDecode: %v", err)
}
if calledFuncs != 1 {
t.Fatalf("calledFuncs = %d, want 1", calledFuncs)
}
if err := UnmarshalDecode(dec, new(string), calledOptions); err != nil {
t.Fatalf("UnmarshalDecode: %v", err)
}
if calledFuncs != 2 {
t.Fatalf("calledFuncs = %d, want 2", calledFuncs)
}
if err := UnmarshalDecode(dec, new(string),
jsontext.AllowInvalidUTF8(false), // should be ignored
WithUnmarshalers(nil), // should override
); err != nil {
t.Fatalf("UnmarshalDecode: %v", err)
}
if calledFuncs != 2 {
t.Fatalf("calledFuncs = %d, want 2", calledFuncs)
}
if err := UnmarshalDecode(dec, new(string)); err != nil {
t.Fatalf("UnmarshalDecode: %v", err)
}
if calledFuncs != 3 {
t.Fatalf("calledFuncs = %d, want 3", calledFuncs)
}
if err := UnmarshalDecode(dec, new(string), JoinOptions(
jsontext.AllowInvalidUTF8(false), // should be ignored
WithUnmarshalers(UnmarshalFromFunc(func(_ *jsontext.Decoder, _ any, opts Options) error {
if v, _ := GetOption(opts, jsontext.AllowInvalidUTF8); !v {
t.Errorf("nested Options.AllowInvalidUTF8 = false, want true")
}
calledFuncs = math.MaxInt
return SkipFunc
})), // should override
)); err != nil {
t.Fatalf("UnmarshalDecode: %v", err)
}
if calledFuncs != math.MaxInt {
t.Fatalf("calledFuncs = %d, want %d", calledFuncs, math.MaxInt)
}
}

// BenchmarkUnmarshalDecodeOptions is a minimal decode operation to measure
// the overhead options setup before the unmarshal operation.
func BenchmarkUnmarshalDecodeOptions(b *testing.B) {
Expand All @@ -9323,6 +9383,70 @@ func BenchmarkUnmarshalDecodeOptions(b *testing.B) {
b.Run("New", makeBench(DefaultOptionsV2()))
}

func TestMarshalEncodeOptions(t *testing.T) {
var calledFuncs int
var calledOptions Options
out := new(bytes.Buffer)
enc := jsontext.NewEncoder(
out,
jsontext.AllowInvalidUTF8(true), // encoder-specific option
WithMarshalers(MarshalToFunc(func(_ *jsontext.Encoder, _ any, opts Options) error {
if v, _ := GetOption(opts, jsontext.AllowInvalidUTF8); !v {
t.Errorf("nested Options.AllowInvalidUTF8 = false, want true")
}
calledFuncs++
calledOptions = opts
return SkipFunc
})), // marshal-specific option; only relevant for MarshalEncode
)

if err := MarshalEncode(enc, "\xde\xad\xbe\xef"); err != nil {
t.Fatalf("MarshalEncode: %v", err)
}
if calledFuncs != 1 {
t.Fatalf("calledFuncs = %d, want 1", calledFuncs)
}
if err := MarshalEncode(enc, "\xde\xad\xbe\xef", calledOptions); err != nil {
t.Fatalf("MarshalEncode: %v", err)
}
if calledFuncs != 2 {
t.Fatalf("calledFuncs = %d, want 2", calledFuncs)
}
if err := MarshalEncode(enc, "\xde\xad\xbe\xef",
jsontext.AllowInvalidUTF8(false), // should be ignored
WithMarshalers(nil), // should override
); err != nil {
t.Fatalf("MarshalEncode: %v", err)
}
if calledFuncs != 2 {
t.Fatalf("calledFuncs = %d, want 2", calledFuncs)
}
if err := MarshalEncode(enc, "\xde\xad\xbe\xef"); err != nil {
t.Fatalf("MarshalEncode: %v", err)
}
if calledFuncs != 3 {
t.Fatalf("calledFuncs = %d, want 3", calledFuncs)
}
if err := MarshalEncode(enc, "\xde\xad\xbe\xef", JoinOptions(
jsontext.AllowInvalidUTF8(false), // should be ignored
WithMarshalers(MarshalToFunc(func(_ *jsontext.Encoder, _ any, opts Options) error {
if v, _ := GetOption(opts, jsontext.AllowInvalidUTF8); !v {
t.Errorf("nested Options.AllowInvalidUTF8 = false, want true")
}
calledFuncs = math.MaxInt
return SkipFunc
})), // should override
)); err != nil {
t.Fatalf("MarshalEncode: %v", err)
}
if calledFuncs != math.MaxInt {
t.Fatalf("calledFuncs = %d, want %d", calledFuncs, math.MaxInt)
}
if out.String() != strings.Repeat("\"\xde\xad\ufffd\ufffd\"\n", 5) {
t.Fatalf("output mismatch:\n\tgot: %s\n\twant: %s", out.String(), strings.Repeat("\"\xde\xad\xbe\xef\"\n", 5))
}
}

// BenchmarkMarshalEncodeOptions is a minimal encode operation to measure
// the overhead of options setup before the marshal operation.
func BenchmarkMarshalEncodeOptions(b *testing.B) {
Expand Down
58 changes: 35 additions & 23 deletions internal/jsonopts/options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,6 @@ var DefaultOptionsV1 = Struct{
},
}

// CopyCoderOptions copies coder-specific options from src to dst.
// This is used by json.MarshalEncode and json.UnmarshalDecode since those
// functions ignore any coder-specific options and uses the options from the
// Encoder or Decoder that is passed in.
func (dst *Struct) CopyCoderOptions(src *Struct) {
srcFlags := src.Flags
srcFlags.Clear(^jsonflags.AllCoderFlags)
dst.Flags.Join(srcFlags)
dst.CoderValues = src.CoderValues
}

func (*Struct) JSONOptions(internal.NotForPublicUse) {}

// GetUnknownOption is injected by the "json" package to handle Options
Expand Down Expand Up @@ -123,50 +112,73 @@ func GetOption[T any](opts Options, setter func(T) Options) (T, bool) {
var JoinUnknownOption = func(*Struct, Options) { panic("unknown option") }

func (dst *Struct) Join(srcs ...Options) {
dst.join(false, srcs...)
}

func (dst *Struct) JoinWithoutCoderOptions(srcs ...Options) {
dst.join(true, srcs...)
}

func (dst *Struct) join(excludeCoderOptions bool, srcs ...Options) {
for _, src := range srcs {
switch src := src.(type) {
case nil:
continue
case jsonflags.Bools:
if excludeCoderOptions {
src &= ^jsonflags.AllCoderFlags
}
dst.Flags.Set(src)
case Indent:
if excludeCoderOptions {
continue
}
dst.Flags.Set(jsonflags.Multiline | jsonflags.Indent | 1)
dst.Indent = string(src)
case IndentPrefix:
if excludeCoderOptions {
continue
}
dst.Flags.Set(jsonflags.Multiline | jsonflags.IndentPrefix | 1)
dst.IndentPrefix = string(src)
case ByteLimit:
if excludeCoderOptions {
continue
}
dst.Flags.Set(jsonflags.ByteLimit | 1)
dst.ByteLimit = int64(src)
case DepthLimit:
if excludeCoderOptions {
continue
}
dst.Flags.Set(jsonflags.DepthLimit | 1)
dst.DepthLimit = int(src)
case *Struct:
dst.Flags.Join(src.Flags)
if src.Flags.Has(jsonflags.NonBooleanFlags) {
if src.Flags.Has(jsonflags.Indent) {
srcFlags := src.Flags // shallow copy the flags
if excludeCoderOptions {
srcFlags.Clear(jsonflags.AllCoderFlags)
}
dst.Flags.Join(srcFlags)
if srcFlags.Has(jsonflags.NonBooleanFlags) {
if srcFlags.Has(jsonflags.Indent) {
dst.Indent = src.Indent
}
if src.Flags.Has(jsonflags.IndentPrefix) {
if srcFlags.Has(jsonflags.IndentPrefix) {
dst.IndentPrefix = src.IndentPrefix
}
if src.Flags.Has(jsonflags.ByteLimit) {
if srcFlags.Has(jsonflags.ByteLimit) {
dst.ByteLimit = src.ByteLimit
}
if src.Flags.Has(jsonflags.DepthLimit) {
if srcFlags.Has(jsonflags.DepthLimit) {
dst.DepthLimit = src.DepthLimit
}
if src.Flags.Has(jsonflags.Marshalers) {
if srcFlags.Has(jsonflags.Marshalers) {
dst.Marshalers = src.Marshalers
}
if src.Flags.Has(jsonflags.Unmarshalers) {
if srcFlags.Has(jsonflags.Unmarshalers) {
dst.Unmarshalers = src.Unmarshalers
}
}
if src.Format != "" {
dst.Format = src.Format
dst.FormatDepth = src.FormatDepth
}
default:
JoinUnknownOption(dst, src)
}
Expand Down
Loading

0 comments on commit 60a0516

Please sign in to comment.