Commit 996f4b65 authored by Kamil Trzciński's avatar Kamil Trzciński Committed by Shinya Maeda

Revert "Silence `api_json_content_type` being not used"

This reverts commit 3211640a.
parent 320f7268
--- ---
name: api_json_content_type name: api_always_use_application_json
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/42229 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/42229
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/270067 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/270067
milestone: '13.6' milestone: '13.6'
......
...@@ -123,13 +123,32 @@ module API ...@@ -123,13 +123,32 @@ module API
format :json format :json
formatter :json, Gitlab::Json::GrapeFormatter formatter :json, Gitlab::Json::GrapeFormatter
content_type :json, 'application/json'
# Remove the `text/plain+deprecated` with `api_always_use_application_json` feature flag
# There is a small chance some users depend on the old behavior. # There is a small chance some users depend on the old behavior.
# We this change under a feature flag to see if affects GitLab.com users. # We this change under a feature flag to see if affects GitLab.com users.
if Gitlab::Database.cached_table_exists?('features') && Feature.enabled?(:api_json_content_type) # The `+deprecated` is added to distinguish content type
content_type :json, 'application/json' # as defined by `API::API` vs ex. `API::Repositories`
content_type :txt, 'text/plain+deprecated'
before do
# the feature flag workaround is only for `.txt`
api_format = env[Grape::Env::API_FORMAT]
next unless api_format == :txt
# get all defined content-types for the endpoint
api_endpoint = env[Grape::Env::API_ENDPOINT]
content_types = api_endpoint&.namespace_stackable_with_hash(:content_types).to_h
# Only overwrite `text/plain+deprecated`
if content_types[api_format] == 'text/plain+deprecated'
if Feature.enabled?(:api_always_use_application_json)
content_type 'application/json'
else else
content_type :txt, 'text/plain' content_type 'text/plain'
end
end
end end
# Ensure the namespace is right, otherwise we might load Grape::API::Helpers # Ensure the namespace is right, otherwise we might load Grape::API::Helpers
......
...@@ -72,6 +72,7 @@ module API ...@@ -72,6 +72,7 @@ module API
namespace 'users' do namespace 'users' do
format :txt format :txt
content_type :txt, 'text/plain'
desc 'Authenticate user against conan CLI' do desc 'Authenticate user against conan CLI' do
detail 'This feature was introduced in GitLab 12.2' detail 'This feature was introduced in GitLab 12.2'
......
...@@ -32,6 +32,7 @@ module API ...@@ -32,6 +32,7 @@ module API
helpers ::API::Helpers::Packages::BasicAuthHelpers helpers ::API::Helpers::Packages::BasicAuthHelpers
format :txt format :txt
content_type :txt, 'text/plain'
rescue_from ArgumentError do |e| rescue_from ArgumentError do |e|
render_api_error!(e.message, 400) render_api_error!(e.message, 400)
......
...@@ -82,7 +82,8 @@ module API ...@@ -82,7 +82,8 @@ module API
content_type 'text/plain' content_type 'text/plain'
env['api.format'] = :binary env['api.format'] = :binary
trace = build.trace.raw # The trace can be nil bu body method expects a string as an argument.
trace = build.trace.raw || ''
body trace body trace
end end
......
...@@ -126,4 +126,34 @@ RSpec.describe API::API do ...@@ -126,4 +126,34 @@ RSpec.describe API::API do
get(api('/users')) get(api('/users'))
end end
end end
describe 'supported content-types' do
context 'GET /user/:id.txt' do
let_it_be(:user) { create(:user) }
subject { get api("/users/#{user.id}.txt", user) }
it 'returns application/json' do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(response.media_type).to eq('application/json')
expect(response.body).to include('{"id":')
end
context 'when api_always_use_application_json is disabled' do
before do
stub_feature_flags(api_always_use_application_json: false)
end
it 'returns text/plain' do
subject
expect(response).to have_gitlab_http_status(:ok)
expect(response.media_type).to eq('text/plain')
expect(response.body).to include('#<API::Entities::User:')
end
end
end
end
end end
This is added to silence warning for `api_json_content_type` being not used.
It is hard to properly test this feature flag as part of specs.
Ref.: `lib/api/api.rb`
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