Merge branch 'json-legacy-interface' into 'master'

Replicate current json gem behaviour in wrapper

See merge request gitlab-org/gitlab!30849

lib/gitlab/json.rb

@@ -2,13 +2,25 @@
 
 module Gitlab
   module Json
+    INVALID_LEGACY_TYPES = [String, TrueClass, FalseClass].freeze
+
     class << self
-      def parse(*args)
-        adapter.parse(*args)
+      def parse(string, *args, **named_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
 
-      def parse!(*args)
-        adapter.parse!(*args)
+      def parse!(string, *args, **named_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
 
       def dump(*args)
@@ -28,6 +40,20 @@ module Gitlab
       def adapter
         ::JSON
       end
+
+      def parser_error
+        ::JSON::ParserError
+      end
+
+      def legacy_mode_enabled?(arg_value)
+        arg_value.nil? ? false : arg_value
+      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

spec/lib/gitlab/json_spec.rb

@@ -3,47 +3,103 @@
 require "spec_helper"
 
 RSpec.describe Gitlab::Json do
+  before do
+    stub_feature_flags(json_wrapper_legacy_mode: true)
+  end
+
   describe ".parse" do
-    it "parses an object" do
-      expect(subject.parse('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
-    end
+    context "legacy_mode is disabled by default" do
+      it "parses an object" do
+        expect(subject.parse('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
+      end
 
-    it "parses an array" do
-      expect(subject.parse('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
-    end
+      it "parses an array" do
+        expect(subject.parse('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
+      end
 
-    it "raises an error on a string" do
-      expect { subject.parse('"foo"') }.to raise_error(JSON::ParserError)
-    end
+      # These tests will change expectations when the gem is upgraded
 
-    it "raises an error on a true bool" do
-      expect { subject.parse("true") }.to raise_error(JSON::ParserError)
+      it "raises an error on a string" do
+        expect { subject.parse('"foo"') }.to raise_error(JSON::ParserError)
+      end
+
+      it "raises an error on a true bool" do
+        expect { subject.parse("true") }.to raise_error(JSON::ParserError)
+      end
+
+      it "raises an error on a false bool" do
+        expect { subject.parse("false") }.to raise_error(JSON::ParserError)
+      end
     end
 
-    it "raises an error on a false bool" do
-      expect { subject.parse("false") }.to raise_error(JSON::ParserError)
+    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 ".parse!" do
-    it "parses an object" do
-      expect(subject.parse!('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
-    end
+    context "legacy_mode is disabled by default" do
+      it "parses an object" do
+        expect(subject.parse!('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
+      end
 
-    it "parses an array" do
-      expect(subject.parse!('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
-    end
+      it "parses an array" do
+        expect(subject.parse!('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
+      end
 
-    it "raises an error on a string" do
-      expect { subject.parse!('"foo"') }.to raise_error(JSON::ParserError)
-    end
+      # These tests will change expectations when the gem is upgraded
+
+      it "raises an error on a string" do
+        expect { subject.parse!('"foo"') }.to raise_error(JSON::ParserError)
+      end
 
-    it "raises an error on a true bool" do
-      expect { subject.parse!("true") }.to raise_error(JSON::ParserError)
+      it "raises an error on a true bool" do
+        expect { subject.parse!("true") }.to raise_error(JSON::ParserError)
+      end
+
+      it "raises an error on a false bool" do
+        expect { subject.parse!("false") }.to raise_error(JSON::ParserError)
+      end
     end
 
-    it "raises an error on a false bool" do
-      expect { subject.parse!("false") }.to raise_error(JSON::ParserError)
+    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