Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix State::configure alias and implementation #152

Merged
merged 2 commits into from

3 participants

@jvshahid

I found out this error while I was investigating errors while running the active record test suite under JRuby. The error is:

NoMethodError: undefined method `merge' for #<JSON::Ext::Generator::State:0x53c20385>
    from /home/jvshahid/codez/jruby-pg/tmp_rails/activesupport/lib/active_support/json/encoding.rb:67:in `options_for'
    from /home/jvshahid/codez/jruby-pg/tmp_rails/activesupport/lib/active_support/json/encoding.rb:50:in `encode'
    from /home/jvshahid/codez/jruby-pg/tmp_rails/activesupport/lib/active_support/json/encoding.rb:82:in `check_for_circular_references'
    from /home/jvshahid/codez/jruby-pg/tmp_rails/activesupport/lib/active_support/json/encoding.rb:49:in `encode'
    from /home/jvshahid/codez/jruby-pg/tmp_rails/activesupport/lib/active_support/json/encoding.rb:34:in `encode'
    from /home/jvshahid/codez/jruby-pg/tmp_rails/activesupport/lib/active_support/core_ext/object/to_json.rb:16:in `to_json'
    from json/ext/GeneratorState.java:210:in `generate

It turns out that the configure method wasn't aliased properly in the Java version and when I tried to run the new test under MRI I found out that the configuration strings aren't stored with the null character so I fixed that as well.

This patch includes a test that covers both merge and configure as well as a fix for both implementations to make the test pass.

@jvshahid

Ping, is there any update on this PR ?

@flori flori merged commit bdb8e00 into flori:master
@dwbutler

It's strange, but even with the merge alias added, I'm still having the same issue. I've tested against both JSON 1.7.6 and JSON-master.

https://gist.github.com/4632270

@jvshahid jvshahid referenced this pull request from a commit in jvshahid/ruby-pg
@jvshahid jvshahid use my clone of the json gem since a big number of tests are failing …
…because of a bug in 1.7.5, see flori/json#152.
a4c1613
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
10 ext/json/ext/generator/generator.c
@@ -522,7 +522,7 @@ static VALUE cState_configure(VALUE self, VALUE opts)
unsigned long len;
Check_Type(tmp, T_STRING);
len = RSTRING_LEN(tmp);
- state->indent = fstrndup(RSTRING_PTR(tmp), len);
+ state->indent = fstrndup(RSTRING_PTR(tmp), len + 1);
state->indent_len = len;
}
tmp = rb_hash_aref(opts, ID2SYM(i_space));
@@ -530,7 +530,7 @@ static VALUE cState_configure(VALUE self, VALUE opts)
unsigned long len;
Check_Type(tmp, T_STRING);
len = RSTRING_LEN(tmp);
- state->space = fstrndup(RSTRING_PTR(tmp), len);
+ state->space = fstrndup(RSTRING_PTR(tmp), len + 1);
state->space_len = len;
}
tmp = rb_hash_aref(opts, ID2SYM(i_space_before));
@@ -538,7 +538,7 @@ static VALUE cState_configure(VALUE self, VALUE opts)
unsigned long len;
Check_Type(tmp, T_STRING);
len = RSTRING_LEN(tmp);
- state->space_before = fstrndup(RSTRING_PTR(tmp), len);
+ state->space_before = fstrndup(RSTRING_PTR(tmp), len + 1);
state->space_before_len = len;
}
tmp = rb_hash_aref(opts, ID2SYM(i_array_nl));
@@ -546,7 +546,7 @@ static VALUE cState_configure(VALUE self, VALUE opts)
unsigned long len;
Check_Type(tmp, T_STRING);
len = RSTRING_LEN(tmp);
- state->array_nl = fstrndup(RSTRING_PTR(tmp), len);
+ state->array_nl = fstrndup(RSTRING_PTR(tmp), len + 1);
state->array_nl_len = len;
}
tmp = rb_hash_aref(opts, ID2SYM(i_object_nl));
@@ -554,7 +554,7 @@ static VALUE cState_configure(VALUE self, VALUE opts)
unsigned long len;
Check_Type(tmp, T_STRING);
len = RSTRING_LEN(tmp);
- state->object_nl = fstrndup(RSTRING_PTR(tmp), len);
+ state->object_nl = fstrndup(RSTRING_PTR(tmp), len + 1);
state->object_nl_len = len;
}
tmp = ID2SYM(i_max_nesting);
View
2  java/src/json/ext/GeneratorState.java
@@ -445,7 +445,7 @@ private ByteList prepareByteList(ThreadContext context, IRubyObject value) {
* @param vOpts The options hash
* @return The receiver
*/
- @JRubyMethod
+ @JRubyMethod(alias = "merge")
public IRubyObject configure(ThreadContext context, IRubyObject vOpts) {
OptionsReader opts = new OptionsReader(context, vOpts);
View
16 tests/test_json_generate.rb
@@ -228,6 +228,22 @@ def test_gc
end if GC.respond_to?(:stress=)
if defined?(JSON::Ext::Generator)
+ [:merge, :configure].each do |method|
+ define_method "test_configure_using_#{method}" do
+ state = JSON::Ext::Generator::State.new
+ state.send method, :indent => "1",
+ :space => '2',
+ :space_before => '3',
+ :object_nl => '4',
+ :array_nl => '5'
+ assert_equal '1', state.indent
+ assert_equal '2', state.space
+ assert_equal '3', state.space_before
+ assert_equal '4', state.object_nl
+ assert_equal '5', state.array_nl
+ end
+ end
+
def test_broken_bignum # [ruby-core:38867]
pid = fork do
Bignum.class_eval do
Something went wrong with that request. Please try again.