Commit 34f964f7 authored by Robert May's avatar Robert May

Change default legacy_mode state to off

Swaps the default to be off, so that individual cases can be
toggled on in when the json gem is upgraded.
parent b02a0313
...@@ -9,13 +9,18 @@ module Gitlab ...@@ -9,13 +9,18 @@ module Gitlab
legacy_mode = legacy_mode_enabled?(named_args.delete(:legacy_mode)) legacy_mode = legacy_mode_enabled?(named_args.delete(:legacy_mode))
data = adapter.parse(string, *args, **named_args) data = adapter.parse(string, *args, **named_args)
raise parser_error if legacy_mode && INVALID_LEGACY_TYPES.any? { |type| data.is_a?(type) } handle_legacy_mode!(data) if legacy_mode
data data
end end
def parse!(*args) def parse!(string, *args, **named_args)
adapter.parse!(*args) legacy_mode = legacy_mode_enabled?(named_args.delete(:legacy_mode))
data = adapter.parse!(string, *args, **named_args)
handle_legacy_mode!(data) if legacy_mode
data
end end
def dump(*args) def dump(*args)
...@@ -41,11 +46,13 @@ module Gitlab ...@@ -41,11 +46,13 @@ module Gitlab
end end
def legacy_mode_enabled?(arg_value) def legacy_mode_enabled?(arg_value)
if ::JSON::VERSION_MAJOR >= 2 arg_value.nil? ? false : arg_value
arg_value.nil? ? true : arg_value
else
true
end end
def handle_legacy_mode!(data)
return data unless Feature.enabled?(:json_wrapper_legacy_mode, default_enabled: true)
raise parser_error if INVALID_LEGACY_TYPES.any? { |type| data.is_a?(type) }
end end
end end
end end
......
...@@ -3,8 +3,12 @@ ...@@ -3,8 +3,12 @@
require "spec_helper" require "spec_helper"
RSpec.describe Gitlab::Json do RSpec.describe Gitlab::Json do
before do
stub_feature_flags(json_wrapper_legacy_mode: true)
end
describe ".parse" do describe ".parse" do
context "legacy_mode is on by default" do context "legacy_mode is disabled by default" do
it "parses an object" do it "parses an object" do
expect(subject.parse('{ "foo": "bar" }')).to eq({ "foo" => "bar" }) expect(subject.parse('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
end end
...@@ -13,6 +17,8 @@ RSpec.describe Gitlab::Json do ...@@ -13,6 +17,8 @@ RSpec.describe Gitlab::Json do
expect(subject.parse('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }]) expect(subject.parse('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
end end
# These tests will change expectations when the gem is upgraded
it "raises an error on a string" do it "raises an error on a string" do
expect { subject.parse('"foo"') }.to raise_error(JSON::ParserError) expect { subject.parse('"foo"') }.to raise_error(JSON::ParserError)
end end
...@@ -26,34 +32,31 @@ RSpec.describe Gitlab::Json do ...@@ -26,34 +32,31 @@ RSpec.describe Gitlab::Json do
end end
end end
context "legacy_mode is disabled" do context "legacy_mode is enabled" do
it "parses an object" do it "parses an object" do
expect(subject.parse('{ "foo": "bar" }', legacy_mode: false)).to eq({ "foo" => "bar" }) expect(subject.parse('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
end end
it "parses an array" do it "parses an array" do
expect(subject.parse('[{ "foo": "bar" }]', legacy_mode: false)).to eq([{ "foo" => "bar" }]) expect(subject.parse('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
end end
# These are expected errors now until we upgrade the `json` gem,
# and then it will be expected to not raise errors and these tests
# will be updated accordingly.
it "raises an error on a string" do it "raises an error on a string" do
expect { subject.parse('"foo"', legacy_mode: false) }.to raise_error(JSON::ParserError) expect { subject.parse('"foo"', legacy_mode: true) }.to raise_error(JSON::ParserError)
end end
it "raises an error on a true bool" do it "raises an error on a true bool" do
expect { subject.parse("true", legacy_mode: false) }.to raise_error(JSON::ParserError) expect { subject.parse("true", legacy_mode: true) }.to raise_error(JSON::ParserError)
end end
it "raises an error on a false bool" do it "raises an error on a false bool" do
expect { subject.parse("false", legacy_mode: false) }.to raise_error(JSON::ParserError) expect { subject.parse("false", legacy_mode: true) }.to raise_error(JSON::ParserError)
end end
end end
end end
describe ".parse!" do describe ".parse!" do
context "legacy_mode is disabled by default" do
it "parses an object" do it "parses an object" do
expect(subject.parse!('{ "foo": "bar" }')).to eq({ "foo" => "bar" }) expect(subject.parse!('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
end end
...@@ -62,6 +65,8 @@ RSpec.describe Gitlab::Json do ...@@ -62,6 +65,8 @@ RSpec.describe Gitlab::Json do
expect(subject.parse!('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }]) expect(subject.parse!('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
end end
# These tests will change expectations when the gem is upgraded
it "raises an error on a string" do it "raises an error on a string" do
expect { subject.parse!('"foo"') }.to raise_error(JSON::ParserError) expect { subject.parse!('"foo"') }.to raise_error(JSON::ParserError)
end end
...@@ -75,6 +80,29 @@ RSpec.describe Gitlab::Json do ...@@ -75,6 +80,29 @@ RSpec.describe Gitlab::Json do
end end
end end
context "legacy_mode is enabled" do
it "parses an object" do
expect(subject.parse!('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
end
it "parses an array" do
expect(subject.parse!('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
end
it "raises an error on a string" do
expect { subject.parse!('"foo"', legacy_mode: true) }.to raise_error(JSON::ParserError)
end
it "raises an error on a true bool" do
expect { subject.parse!("true", legacy_mode: true) }.to raise_error(JSON::ParserError)
end
it "raises an error on a false bool" do
expect { subject.parse!("false", legacy_mode: true) }.to raise_error(JSON::ParserError)
end
end
end
describe ".dump" do describe ".dump" do
it "dumps an object" do it "dumps an object" do
expect(subject.dump({ "foo" => "bar" })).to eq('{"foo":"bar"}') expect(subject.dump({ "foo" => "bar" })).to eq('{"foo":"bar"}')
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment