Merge branch 'test-api_json_content_type' into 'master'

Test deprecation of `text/plain` on all endpoints

See merge request gitlab-org/gitlab!46133

config/feature_flags/development/api_json_content_type.yml → config/feature_flags/development/api_always_use_application_json.yml

 ---
-name: api_json_content_type
+name: api_always_use_application_json
 introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/42229
 rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/270067
 milestone: '13.6'

lib/api/api.rb

@@ -123,13 +123,32 @@ module API
 
     format :json
     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.
     # 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)
-      content_type :json, 'application/json'
+    # The `+deprecated` is added to distinguish content type
+    # 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
-      content_type :txt, 'text/plain'
+          content_type 'text/plain'
+        end
+      end
     end
 
     # Ensure the namespace is right, otherwise we might load Grape::API::Helpers

lib/api/concerns/packages/conan_endpoints.rb

@@ -72,6 +72,7 @@ module API
 
           namespace 'users' do
             format :txt
+            content_type :txt, 'text/plain'
 
             desc 'Authenticate user against conan CLI' do
               detail 'This feature was introduced in GitLab 12.2'

lib/api/debian_package_endpoints.rb

@@ -32,6 +32,7 @@ module API
       helpers ::API::Helpers::Packages::BasicAuthHelpers
 
       format :txt
+      content_type :txt, 'text/plain'
 
       rescue_from ArgumentError do |e|
         render_api_error!(e.message, 400)

lib/api/jobs.rb

@@ -82,7 +82,8 @@ module API
         content_type 'text/plain'
         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
       end
 

spec/requests/api/api_spec.rb

@@ -126,4 +126,34 @@ RSpec.describe API::API do
       get(api('/users'))
     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

tmp/feature_flags/api_json_content_type.used

-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`