Merge branch 'default-oj' into 'master'

Remove :oj_json feature flag

See merge request gitlab-org/gitlab!46012

config/feature_flags/development/oj_json.yml

----
-name: oj_json
-introduced_by_url: 
-rollout_issue_url: 
-group: 
-type: development
-default_enabled: true

lib/gitlab/json.rb

@@ -67,15 +67,6 @@ module Gitlab
         ::JSON.pretty_generate(object, opts)
       end
 
-      # Feature detection for using Oj instead of the `json` gem.
-      #
-      # @return [Boolean]
-      def enable_oj?
-        return false unless feature_table_exists?
-
-        Feature.enabled?(:oj_json, default_enabled: true)
-      end
-
       private
 
       # Convert JSON string into Ruby through toggleable adapters.
@@ -91,11 +82,7 @@ module Gitlab
       def adapter_load(string, *args, **opts)
         opts = standardize_opts(opts)
 
-        if enable_oj?
-          Oj.load(string, opts)
-        else
-          ::JSON.parse(string, opts)
-        end
+        Oj.load(string, opts)
       rescue Oj::ParseError, Encoding::UndefinedConversionError => ex
         raise parser_error.new(ex)
       end
@@ -120,11 +107,7 @@ module Gitlab
       #
       # @return [String]
       def adapter_dump(object, *args, **opts)
-        if enable_oj?
-          Oj.dump(object, opts)
-        else
-          ::JSON.dump(object, *args)
-        end
+        Oj.dump(object, opts)
       end
 
       # Generates JSON for an object but with fewer options, using toggleable adapters.
@@ -135,11 +118,7 @@ module Gitlab
       def adapter_generate(object, opts = {})
         opts = standardize_opts(opts)
 
-        if enable_oj?
-          Oj.generate(object, opts)
-        else
-          ::JSON.generate(object, opts)
-        end
+        Oj.generate(object, opts)
       end
 
       # Take a JSON standard options hash and standardize it to work across adapters
@@ -149,11 +128,8 @@ module Gitlab
       # @return [Hash]
       def standardize_opts(opts)
         opts ||= {}
-
-        if enable_oj?
-          opts[:mode] = :rails
-          opts[:symbol_keys] = opts[:symbolize_keys] || opts[:symbolize_names]
-        end
+        opts[:mode] = :rails
+        opts[:symbol_keys] = opts[:symbolize_keys] || opts[:symbolize_names]
 
         opts
       end
@@ -213,7 +189,7 @@ module Gitlab
       # @param object [Object]
       # @return [String]
       def self.call(object, env = nil)
-        if Gitlab::Json.enable_oj? && Feature.enabled?(:grape_gitlab_json, default_enabled: true)
+        if Feature.enabled?(:grape_gitlab_json, default_enabled: true)
           Gitlab::Json.dump(object)
         else
           Grape::Formatter::Json.call(object, env)

spec/lib/gitlab/json_spec.rb

@@ -7,342 +7,306 @@ RSpec.describe Gitlab::Json do
     stub_feature_flags(json_wrapper_legacy_mode: true)
   end
 
-  shared_examples "json" do
-    describe ".parse" do
-      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 a string" do
-          expect(subject.parse('"foo"', legacy_mode: false)).to eq("foo")
-        end
-
-        it "parses a true bool" do
-          expect(subject.parse("true", legacy_mode: false)).to be(true)
-        end
-
-        it "parses a false bool" do
-          expect(subject.parse("false", legacy_mode: false)).to be(false)
-        end
+  describe ".parse" do
+    context "legacy_mode is disabled by default" do
+      it "parses an object" do
+        expect(subject.parse('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
       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
+      it "parses an array" do
+        expect(subject.parse('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
       end
 
-      context "feature flag is disabled" do
-        before do
-          stub_feature_flags(json_wrapper_legacy_mode: false)
-        end
-
-        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 "parses a string" do
-          expect(subject.parse('"foo"', legacy_mode: true)).to eq("foo")
-        end
+      it "parses a string" do
+        expect(subject.parse('"foo"', legacy_mode: false)).to eq("foo")
+      end
 
-        it "parses a true bool" do
-          expect(subject.parse("true", legacy_mode: true)).to be(true)
-        end
+      it "parses a true bool" do
+        expect(subject.parse("true", legacy_mode: false)).to be(true)
+      end
 
-        it "parses a false bool" do
-          expect(subject.parse("false", legacy_mode: true)).to be(false)
-        end
+      it "parses a false bool" do
+        expect(subject.parse("false", legacy_mode: false)).to be(false)
       end
     end
 
-    describe ".parse!" do
-      context "legacy_mode is disabled by default" do
-        it "parses an object" do
-          expect(subject.parse!('{ "foo": "bar" }')).to eq({ "foo" => "bar" })
-        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" }]')).to eq([{ "foo" => "bar" }])
-        end
+      it "parses an array" do
+        expect(subject.parse('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
+      end
 
-        it "parses a string" do
-          expect(subject.parse!('"foo"', legacy_mode: false)).to eq("foo")
-        end
+      it "raises an error on a string" do
+        expect { subject.parse('"foo"', legacy_mode: true) }.to raise_error(JSON::ParserError)
+      end
 
-        it "parses a true bool" do
-          expect(subject.parse!("true", legacy_mode: false)).to be(true)
-        end
+      it "raises an error on a true bool" do
+        expect { subject.parse("true", legacy_mode: true) }.to raise_error(JSON::ParserError)
+      end
 
-        it "parses a false bool" do
-          expect(subject.parse!("false", legacy_mode: false)).to be(false)
-        end
+      it "raises an error on a false bool" do
+        expect { subject.parse("false", legacy_mode: true) }.to raise_error(JSON::ParserError)
       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
+    context "feature flag is disabled" do
+      before do
+        stub_feature_flags(json_wrapper_legacy_mode: false)
+      end
 
-        it "parses an array" do
-          expect(subject.parse!('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
-        end
+      it "parses an object" 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 "parses an array" do
+        expect(subject.parse('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
+      end
 
-        it "raises an error on a true bool" do
-          expect { subject.parse!("true", legacy_mode: true) }.to raise_error(JSON::ParserError)
-        end
+      it "parses a string" do
+        expect(subject.parse('"foo"', legacy_mode: true)).to eq("foo")
+      end
 
-        it "raises an error on a false bool" do
-          expect { subject.parse!("false", legacy_mode: true) }.to raise_error(JSON::ParserError)
-        end
+      it "parses a true bool" do
+        expect(subject.parse("true", legacy_mode: true)).to be(true)
       end
 
-      context "feature flag is disabled" do
-        before do
-          stub_feature_flags(json_wrapper_legacy_mode: false)
-        end
+      it "parses a false bool" do
+        expect(subject.parse("false", legacy_mode: true)).to be(false)
+      end
+    end
+  end
 
-        it "parses an object" do
-          expect(subject.parse!('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
-        end
+  describe ".parse!" do
+    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" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
-        end
+      it "parses an array" do
+        expect(subject.parse!('[{ "foo": "bar" }]')).to eq([{ "foo" => "bar" }])
+      end
 
-        it "parses a string" do
-          expect(subject.parse!('"foo"', legacy_mode: true)).to eq("foo")
-        end
+      it "parses a string" do
+        expect(subject.parse!('"foo"', legacy_mode: false)).to eq("foo")
+      end
 
-        it "parses a true bool" do
-          expect(subject.parse!("true", legacy_mode: true)).to be(true)
-        end
+      it "parses a true bool" do
+        expect(subject.parse!("true", legacy_mode: false)).to be(true)
+      end
 
-        it "parses a false bool" do
-          expect(subject.parse!("false", legacy_mode: true)).to be(false)
-        end
+      it "parses a false bool" do
+        expect(subject.parse!("false", legacy_mode: false)).to be(false)
       end
     end
 
-    describe ".dump" do
-      it "dumps an object" do
-        expect(subject.dump({ "foo" => "bar" })).to eq('{"foo":"bar"}')
+    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 "dumps an array" do
-        expect(subject.dump([{ "foo" => "bar" }])).to eq('[{"foo":"bar"}]')
+      it "parses an array" do
+        expect(subject.parse!('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
       end
 
-      it "dumps a string" do
-        expect(subject.dump("foo")).to eq('"foo"')
+      it "raises an error on a string" do
+        expect { subject.parse!('"foo"', legacy_mode: true) }.to raise_error(JSON::ParserError)
       end
 
-      it "dumps a true bool" do
-        expect(subject.dump(true)).to eq("true")
+      it "raises an error on a true bool" do
+        expect { subject.parse!("true", legacy_mode: true) }.to raise_error(JSON::ParserError)
       end
 
-      it "dumps a false bool" do
-        expect(subject.dump(false)).to eq("false")
+      it "raises an error on a false bool" do
+        expect { subject.parse!("false", legacy_mode: true) }.to raise_error(JSON::ParserError)
       end
     end
 
-    describe ".generate" do
-      let(:obj) do
-        { test: true, "foo.bar" => "baz", is_json: 1, some: [1, 2, 3] }
+    context "feature flag is disabled" do
+      before do
+        stub_feature_flags(json_wrapper_legacy_mode: false)
       end
 
-      it "generates JSON" do
-        expected_string = <<~STR.chomp
-          {"test":true,"foo.bar":"baz","is_json":1,"some":[1,2,3]}
-        STR
+      it "parses an object" do
+        expect(subject.parse!('{ "foo": "bar" }', legacy_mode: true)).to eq({ "foo" => "bar" })
+      end
 
-        expect(subject.generate(obj)).to eq(expected_string)
+      it "parses an array" do
+        expect(subject.parse!('[{ "foo": "bar" }]', legacy_mode: true)).to eq([{ "foo" => "bar" }])
       end
 
-      it "allows you to customise the output" do
-        opts = {
-          indent: "  ",
-          space: " ",
-          space_before: " ",
-          object_nl: "\n",
-          array_nl: "\n"
-        }
+      it "parses a string" do
+        expect(subject.parse!('"foo"', legacy_mode: true)).to eq("foo")
+      end
 
-        json = subject.generate(obj, opts)
-
-        expected_string = <<~STR.chomp
-          {
-            "test" : true,
-            "foo.bar" : "baz",
-            "is_json" : 1,
-            "some" : [
-              1,
-              2,
-              3
-            ]
-          }
-        STR
+      it "parses a true bool" do
+        expect(subject.parse!("true", legacy_mode: true)).to be(true)
+      end
 
-        expect(json).to eq(expected_string)
+      it "parses a false bool" do
+        expect(subject.parse!("false", legacy_mode: true)).to be(false)
       end
     end
+  end
 
-    describe ".pretty_generate" do
-      let(:obj) do
-        {
-          test: true,
-          "foo.bar" => "baz",
-          is_json: 1,
-          some: [1, 2, 3],
-          more: { test: true },
-          multi_line_empty_array: [],
-          multi_line_empty_obj: {}
-        }
-      end
+  describe ".dump" do
+    it "dumps an object" do
+      expect(subject.dump({ "foo" => "bar" })).to eq('{"foo":"bar"}')
+    end
 
-      it "generates pretty JSON" do
-        expected_string = <<~STR.chomp
-          {
-            "test": true,
-            "foo.bar": "baz",
-            "is_json": 1,
-            "some": [
-              1,
-              2,
-              3
-            ],
-            "more": {
-              "test": true
-            },
-            "multi_line_empty_array": [
-
-            ],
-            "multi_line_empty_obj": {
-            }
-          }
-        STR
+    it "dumps an array" do
+      expect(subject.dump([{ "foo" => "bar" }])).to eq('[{"foo":"bar"}]')
+    end
 
-        expect(subject.pretty_generate(obj)).to eq(expected_string)
-      end
+    it "dumps a string" do
+      expect(subject.dump("foo")).to eq('"foo"')
+    end
 
-      it "allows you to customise the output" do
-        opts = {
-          space_before: " "
-        }
+    it "dumps a true bool" do
+      expect(subject.dump(true)).to eq("true")
+    end
 
-        json = subject.pretty_generate(obj, opts)
-
-        expected_string = <<~STR.chomp
-          {
-            "test" : true,
-            "foo.bar" : "baz",
-            "is_json" : 1,
-            "some" : [
-              1,
-              2,
-              3
-            ],
-            "more" : {
-              "test" : true
-            },
-            "multi_line_empty_array" : [
-
-            ],
-            "multi_line_empty_obj" : {
-            }
-          }
-        STR
+    it "dumps a false bool" do
+      expect(subject.dump(false)).to eq("false")
+    end
+  end
 
-        expect(json).to eq(expected_string)
-      end
+  describe ".generate" do
+    let(:obj) do
+      { test: true, "foo.bar" => "baz", is_json: 1, some: [1, 2, 3] }
     end
 
-    context "the feature table is missing" do
-      before do
-        allow(Feature::FlipperFeature).to receive(:table_exists?).and_return(false)
-      end
+    it "generates JSON" do
+      expected_string = <<~STR.chomp
+        {"test":true,"foo.bar":"baz","is_json":1,"some":[1,2,3]}
+      STR
+
+      expect(subject.generate(obj)).to eq(expected_string)
+    end
 
-      it "skips legacy mode handling" do
-        expect(Feature).not_to receive(:enabled?).with(:json_wrapper_legacy_mode, default_enabled: true)
+    it "allows you to customise the output" do
+      opts = {
+        indent: "  ",
+        space: " ",
+        space_before: " ",
+        object_nl: "\n",
+        array_nl: "\n"
+      }
 
-        subject.send(:handle_legacy_mode!, {})
-      end
+      json = subject.generate(obj, opts)
 
-      it "skips oj feature detection" do
-        expect(Feature).not_to receive(:enabled?).with(:oj_json, default_enabled: true)
+      expected_string = <<~STR.chomp
+        {
+          "test" : true,
+          "foo.bar" : "baz",
+          "is_json" : 1,
+          "some" : [
+            1,
+            2,
+            3
+          ]
+        }
+      STR
 
-        subject.send(:enable_oj?)
-      end
+      expect(json).to eq(expected_string)
     end
+  end
 
-    context "the database is missing" do
-      before do
-        allow(Feature::FlipperFeature).to receive(:table_exists?).and_raise(PG::ConnectionBad)
-      end
+  describe ".pretty_generate" do
+    let(:obj) do
+      {
+        test: true,
+        "foo.bar" => "baz",
+        is_json: 1,
+        some: [1, 2, 3],
+        more: { test: true },
+        multi_line_empty_array: [],
+        multi_line_empty_obj: {}
+      }
+    end
 
-      it "still parses json" do
-        expect(subject.parse("{}")).to eq({})
-      end
+    it "generates pretty JSON" do
+      expected_string = <<~STR.chomp
+        {
+          "test": true,
+          "foo.bar": "baz",
+          "is_json": 1,
+          "some": [
+            1,
+            2,
+            3
+          ],
+          "more": {
+            "test": true
+          },
+          "multi_line_empty_array": [
+
+          ],
+          "multi_line_empty_obj": {
+          }
+        }
+      STR
 
-      it "still generates json" do
-        expect(subject.dump({})).to eq("{}")
-      end
+      expect(subject.pretty_generate(obj)).to eq(expected_string)
+    end
+
+    it "allows you to customise the output" do
+      opts = {
+        space_before: " "
+      }
+
+      json = subject.pretty_generate(obj, opts)
+
+      expected_string = <<~STR.chomp
+        {
+          "test" : true,
+          "foo.bar" : "baz",
+          "is_json" : 1,
+          "some" : [
+            1,
+            2,
+            3
+          ],
+          "more" : {
+            "test" : true
+          },
+          "multi_line_empty_array" : [
+
+          ],
+          "multi_line_empty_obj" : {
+          }
+        }
+      STR
+
+      expect(json).to eq(expected_string)
     end
   end
 
-  context "oj gem" do
+  context "the feature table is missing" do
     before do
-      stub_feature_flags(oj_json: true)
+      allow(Feature::FlipperFeature).to receive(:table_exists?).and_return(false)
     end
 
-    it_behaves_like "json"
+    it "skips legacy mode handling" do
+      expect(Feature).not_to receive(:enabled?).with(:json_wrapper_legacy_mode, default_enabled: true)
 
-    describe "#enable_oj?" do
-      it "returns true" do
-        expect(subject.enable_oj?).to be(true)
-      end
+      subject.send(:handle_legacy_mode!, {})
     end
   end
 
-  context "json gem" do
+  context "the database is missing" do
     before do
-      stub_feature_flags(oj_json: false)
+      allow(Feature::FlipperFeature).to receive(:table_exists?).and_raise(PG::ConnectionBad)
     end
 
-    it_behaves_like "json"
+    it "still parses json" do
+      expect(subject.parse("{}")).to eq({})
+    end
 
-    describe "#enable_oj?" do
-      it "returns false" do
-        expect(subject.enable_oj?).to be(false)
-      end
+    it "still generates json" do
+      expect(subject.dump({})).to eq("{}")
     end
   end
 
@@ -353,47 +317,25 @@ RSpec.describe Gitlab::Json do
     let(:env) { {} }
     let(:result) { "{\"test\":true}" }
 
-    context "oj is enabled" do
+    context "grape_gitlab_json flag is enabled" do
       before do
-        stub_feature_flags(oj_json: true)
+        stub_feature_flags(grape_gitlab_json: true)
       end
 
-      context "grape_gitlab_json flag is enabled" do
-        before do
-          stub_feature_flags(grape_gitlab_json: true)
-        end
-
-        it "generates JSON" do
-          expect(subject).to eq(result)
-        end
-
-        it "uses Gitlab::Json" do
-          expect(Gitlab::Json).to receive(:dump).with(obj)
-
-          subject
-        end
+      it "generates JSON" do
+        expect(subject).to eq(result)
       end
 
-      context "grape_gitlab_json flag is disabled" do
-        before do
-          stub_feature_flags(grape_gitlab_json: false)
-        end
-
-        it "generates JSON" do
-          expect(subject).to eq(result)
-        end
+      it "uses Gitlab::Json" do
+        expect(Gitlab::Json).to receive(:dump).with(obj)
 
-        it "uses Grape::Formatter::Json" do
-          expect(Grape::Formatter::Json).to receive(:call).with(obj, env)
-
-          subject
-        end
+        subject
       end
     end
 
-    context "oj is disabled" do
+    context "grape_gitlab_json flag is disabled" do
       before do
-        stub_feature_flags(oj_json: false)
+        stub_feature_flags(grape_gitlab_json: false)
       end
 
       it "generates JSON" do