Skip to content

Commit bbc73fb

Browse files
youdie006byroot
authored andcommitted
Make GeneratorState#_configure only write the options it is given
The C extension walks the given hash and writes only the keys present (generator.c:1860), and since #1076 the pure generator does the same. The Java one re-wrote every option with its class default, so any configure or merge silently reset max_nesting, allow_nan, ascii_only, script_safe, strict, buffer_initial_length, allow_duplicate_key, as_json and sort_keys. Re-enables the two tests #1076 marked pending on JRuby.
1 parent 46fbe24 commit bbc73fb

2 files changed

Lines changed: 42 additions & 30 deletions

File tree

java/src/json/ext/GeneratorState.java

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -545,43 +545,41 @@ public IRubyObject _configure(ThreadContext context, IRubyObject vOpts) {
545545
checkFrozen();
546546
OptionsReader opts = new OptionsReader(context, vOpts);
547547

548-
ByteList indent = opts.getString("indent");
549-
if (indent != null) this.indent = indent;
550-
551-
ByteList space = opts.getString("space");
552-
if (space != null) this.space = space;
553-
554-
ByteList spaceBefore = opts.getString("space_before");
555-
if (spaceBefore != null) this.spaceBefore = spaceBefore;
556-
557-
ByteList arrayNl = opts.getString("array_nl");
558-
if (arrayNl != null) this.arrayNl = arrayNl;
559-
560-
this.asJSON = opts.getProc("as_json");
561-
562-
ByteList objectNl = opts.getString("object_nl");
563-
if (objectNl != null) this.objectNl = objectNl;
564-
565-
maxNesting = opts.getInt("max_nesting", DEFAULT_MAX_NESTING);
566-
allowNaN = opts.getBool("allow_nan", DEFAULT_ALLOW_NAN);
567-
asciiOnly = opts.getBool("ascii_only", DEFAULT_ASCII_ONLY);
568-
scriptSafe = opts.getBool("script_safe", DEFAULT_SCRIPT_SAFE);
569-
strict = opts.getBool("strict", DEFAULT_STRICT);
570-
bufferInitialLength = opts.getInt("buffer_initial_length", DEFAULT_BUFFER_INITIAL_LENGTH);
571-
572-
depth = opts.getInt("depth", 0);
548+
this.indent = stringConfig(opts, "indent", this.indent);
549+
this.space = stringConfig(opts, "space", this.space);
550+
this.spaceBefore = stringConfig(opts, "space_before", this.spaceBefore);
551+
this.arrayNl = stringConfig(opts, "array_nl", this.arrayNl);
552+
this.objectNl = stringConfig(opts, "object_nl", this.objectNl);
553+
554+
if (opts.hasKey("as_json")) this.asJSON = opts.getProc("as_json");
555+
556+
maxNesting = opts.getInt("max_nesting", maxNesting);
557+
allowNaN = opts.getBool("allow_nan", allowNaN);
558+
asciiOnly = opts.getBool("ascii_only", asciiOnly);
559+
scriptSafe = opts.getBool("script_safe", scriptSafe);
560+
strict = opts.getBool("strict", strict);
561+
bufferInitialLength = opts.getInt("buffer_initial_length", bufferInitialLength);
562+
563+
depth = opts.getInt("depth", depth);
573564
if (depth < 0) {
574565
throw context.runtime.newArgumentError("depth must be >= 0 (got: " + depth + ")");
575566
}
576-
this.allowDuplicateKey = opts.getBool("allow_duplicate_key", false);
567+
this.allowDuplicateKey = opts.getBool("allow_duplicate_key", allowDuplicateKey);
577568

578-
sortKeys = normalizeSortKeys(context, opts.get("sort_keys"));
569+
if (opts.hasKey("sort_keys")) sortKeys = normalizeSortKeys(context, opts.get("sort_keys"));
579570

580571
opts.ensureEmpty();
581572

582573
return this;
583574
}
584575

576+
// A falsy value writes the empty string, as string_config() does in the C extension.
577+
private static ByteList stringConfig(OptionsReader opts, String key, ByteList current) {
578+
if (!opts.hasKey(key)) return current;
579+
ByteList value = opts.getString(key);
580+
return value == null ? ByteList.EMPTY_BYTELIST : value;
581+
}
582+
585583
/**
586584
* <code>State#to_h()</code>
587585
*

test/json/json_generator_test.rb

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -539,18 +539,32 @@ def test_configure_keeps_the_layout_of_a_pretty_state
539539
end
540540

541541
def test_configure_only_writes_the_other_options_it_is_given
542-
omit 'JRuby resets the non-string options' if RUBY_ENGINE == 'jruby'
543-
state = JSON.state.new(max_nesting: 3, allow_nan: true, ascii_only: true, script_safe: true)
542+
state = JSON.state.new(max_nesting: 3, allow_nan: true, ascii_only: true, script_safe: true,
543+
strict: true, buffer_initial_length: 32)
544544
state.configure(indent: '1')
545545
assert_equal '1', state.indent
546546
assert_equal 3, state.max_nesting
547547
assert_equal true, state.allow_nan?
548548
assert_equal true, state.ascii_only?
549549
assert_equal true, state.script_safe?
550+
assert_equal true, state.strict?
551+
assert_equal 32, state.buffer_initial_length
552+
end
553+
554+
def test_configure_keeps_sort_keys
555+
state = JSON.state.new(sort_keys: true)
556+
state.configure(depth: 0)
557+
assert_equal '{"a":2,"b":1}', state.generate({ 'b' => 1, 'a' => 2 })
558+
end
559+
560+
def test_configure_keeps_as_json
561+
as_json = ->(object, _is_key) { object.to_s }
562+
state = JSON.state.new(strict: true, as_json: as_json)
563+
state.configure(depth: 0)
564+
assert_equal as_json, state.as_json
550565
end
551566

552567
def test_configure_writes_a_string_option_given_as_nil
553-
omit 'JRuby keeps the previous value for an explicit nil' if RUBY_ENGINE == 'jruby'
554568
state = JSON.state.new(indent: '1', space: '2')
555569
state.configure(indent: nil)
556570
assert_equal '', state.indent

0 commit comments

Comments
 (0)