Commit 2ff0282b authored by Bob Van Landuyt's avatar Bob Van Landuyt

Merge branch 'gitlab-json-grape' into 'master'

Swap Grape over to Gitlab::Json

See merge request gitlab-org/gitlab!36472
parents 853a0fbe aea70c3f
---
title: Swap Grape over to Gitlab::Json
merge_request: 36472
author:
type: performance
...@@ -110,6 +110,7 @@ module API ...@@ -110,6 +110,7 @@ module API
end end
format :json format :json
formatter :json, Gitlab::Json::GrapeFormatter
content_type :txt, "text/plain" content_type :txt, "text/plain"
# Ensure the namespace is right, otherwise we might load Grape::API::Helpers # Ensure the namespace is right, otherwise we might load Grape::API::Helpers
......
...@@ -67,6 +67,15 @@ module Gitlab ...@@ -67,6 +67,15 @@ module Gitlab
::JSON.pretty_generate(object, opts) ::JSON.pretty_generate(object, opts)
end 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 private
# Convert JSON string into Ruby through toggleable adapters. # Convert JSON string into Ruby through toggleable adapters.
...@@ -176,13 +185,6 @@ module Gitlab ...@@ -176,13 +185,6 @@ module Gitlab
raise parser_error if INVALID_LEGACY_TYPES.any? { |type| data.is_a?(type) } raise parser_error if INVALID_LEGACY_TYPES.any? { |type| data.is_a?(type) }
end end
# @return [Boolean]
def enable_oj?
return false unless feature_table_exists?
Feature.enabled?(:oj_json, default_enabled: true)
end
# There are a variety of database errors possible when checking the feature # There are a variety of database errors possible when checking the feature
# flags at the wrong time during boot, e.g. during migrations. We don't care # flags at the wrong time during boot, e.g. during migrations. We don't care
# about these errors, we just need to ensure that we skip feature detection # about these errors, we just need to ensure that we skip feature detection
...@@ -195,5 +197,28 @@ module Gitlab ...@@ -195,5 +197,28 @@ module Gitlab
false false
end end
end end
# GrapeFormatter is a JSON formatter for the Grape API.
# This is set in lib/api/api.rb
class GrapeFormatter
# Convert an object to JSON.
#
# This will default to the built-in Grape formatter if either :oj_json or :grape_gitlab_json
# flags are disabled.
#
# The `env` param is ignored because it's not needed in either our formatter or Grape's,
# but it is passed through for consistency.
#
# @param object [Object]
# @return [String]
def self.call(object, env = nil)
if Gitlab::Json.enable_oj? && Feature.enabled?(:grape_gitlab_json, default_enabled: true)
Gitlab::Json.dump(object)
else
Grape::Formatter::Json.call(object, env)
end
end
end
end end
end end
...@@ -324,6 +324,12 @@ RSpec.describe Gitlab::Json do ...@@ -324,6 +324,12 @@ RSpec.describe Gitlab::Json do
end end
it_behaves_like "json" it_behaves_like "json"
describe "#enable_oj?" do
it "returns true" do
expect(subject.enable_oj?).to be(true)
end
end
end end
context "json gem" do context "json gem" do
...@@ -332,5 +338,73 @@ RSpec.describe Gitlab::Json do ...@@ -332,5 +338,73 @@ RSpec.describe Gitlab::Json do
end end
it_behaves_like "json" it_behaves_like "json"
describe "#enable_oj?" do
it "returns false" do
expect(subject.enable_oj?).to be(false)
end
end
end
describe Gitlab::Json::GrapeFormatter do
subject { described_class.call(obj, env) }
let(:obj) { { test: true } }
let(:env) { {} }
let(:result) { "{\"test\":true}" }
context "oj is enabled" do
before do
stub_feature_flags(oj_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
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 Grape::Formatter::Json" do
expect(Grape::Formatter::Json).to receive(:call).with(obj, env)
subject
end
end
end
context "oj is disabled" do
before do
stub_feature_flags(oj_json: false)
end
it "generates JSON" do
expect(subject).to eq(result)
end
it "uses Grape::Formatter::Json" do
expect(Grape::Formatter::Json).to receive(:call).with(obj, env)
subject
end
end
end end
end end
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