Fix Git actions on .json files returning JSON responses

app/controllers/git_controller.rb

@@ -8,7 +8,6 @@ class GitController < ApplicationController
   before_action :fetch_git_version
   before_action :get_tree, only: [:tree]
   before_action :get_blob, only: [:blob, :download, :raw]
-  before_action :coerce_format
 
   user_content_actions :raw
 
@@ -17,6 +16,11 @@ class GitController < ApplicationController
   rescue_from Git::InvalidPathException, with: :render_invalid_path_error
   rescue_from URI::InvalidURIError, with: :render_invalid_url_error
 
+  # See: config/initializers/action_dispatch_http_mime_negotiation.rb
+  def self.ignore_format_from_extension
+    true
+  end
+
   def browse
     respond_to do |format|
       format.html
@@ -25,12 +29,12 @@ def browse
 
   def tree
     respond_to do |format|
-      format.json { render json: @tree, adapter: :attributes, root: '' }
       if request.xhr?
         format.html { render partial: 'tree' }
       else
         format.html
       end
+      format.json { render json: @tree, adapter: :attributes, root: '' }
     end
   end
 
@@ -40,12 +44,12 @@ def download
 
   def blob
     respond_to do |format|
-      format.json { render json: @blob, adapter: :attributes, root: '' }
       if request.xhr?
         format.html { render partial: 'blob' }
       else
         format.html
       end
+      format.json { render json: @blob, adapter: :attributes, root: '' }
     end
   end
 
@@ -118,7 +122,6 @@ def update
 
   def operation_response(notice = nil, status: 200)
     respond_to do |format|
-      format.json { render json: { }, status: status, adapter: :attributes, root: '' }
       format.html do
         if request.xhr?
           render partial: 'files', locals: { resource: @parent_resource, git_version: @git_version }, status: status
@@ -127,6 +130,7 @@ def operation_response(notice = nil, status: 200)
           redirect_to polymorphic_path(@parent_resource, tab: 'files')
         end
       end
+      format.json { render json: { }, status: status, adapter: :attributes, root: '' }
     end
   end
 
@@ -226,17 +230,6 @@ def add_remote_file
     @git_version.save!
   end
 
-  def coerce_format
-    # I have to do this because Rails doesn't seem to be behaving as expected.
-    # In routes.rb, the git routes are scoped with "format: false", so Rails should disregard the extension
-    # (e.g. /git/1/blob/my_file.yml) when determining the response format.
-    # However this results in an UnknownFormat error when trying to load the HTML view, as Rails still seems to be
-    # looking for an e.g. application/yaml view.
-    # You can fix this by adding { defaults: { format: :html } }, but then it is not possible to request JSON,
-    # even with an explicit `Accept: application/json` header! -Finn
-    request.format = :html unless json_api_request?
-  end
-
   def file_content
     file_params.key?(:content) ? StringIO.new(Base64.decode64(file_params[:content])) : file_params[:data]
   end

config/initializers/action_dispatch_http_mime_negotiation.rb

@@ -0,0 +1,31 @@
+# Monkeypatch because Rails doesn't seem to be behaving as expected.
+# In routes.rb, the git routes are scoped with "format: false", so Rails should disregard the extension
+# (e.g. /git/1/blob/my_file.yml) when determining the response format.
+# However this results in an UnknownFormat error when trying to load the HTML view, as Rails still seems to be
+# looking for an e.g. application/yaml view.
+# You can fix this by adding { defaults: { format: :html } }, but then it is not possible to request JSON,
+# even with an explicit `Accept: application/json` header!
+#
+# Inspired by GitLab's change:
+# https://gitlab.com/gitlab-org/gitlab/-/blob/7a0c278e/config/initializers/action_dispatch_http_mime_negotiation.rb
+# -Finn
+
+module ActionDispatch
+  module Http
+    module MimeNegotiation
+      alias original_format_from_path_extension format_from_path_extension
+
+      def format_from_path_extension
+        controller_class&.ignore_format_from_extension ? nil : original_format_from_path_extension
+      end
+    end
+  end
+end
+
+module ActionController
+  class Base
+    def self.ignore_format_from_extension
+      false
+    end
+  end
+end

test/integration/git_format_test.rb

@@ -0,0 +1,51 @@
+require 'test_helper'
+
+class GitFormatTest < ActionDispatch::IntegrationTest
+  test 'gets appropriate response format from git controller' do
+    workflow = FactoryBot.create(:local_git_workflow, policy: FactoryBot.create(:public_policy))
+    git_version = workflow.git_version
+    disable_authorization_checks do
+      git_version.add_file('dmp.json', open_fixture_file('dmp.json'))
+      git_version.add_file('file', open_fixture_file('file'))
+      git_version.add_file('html_file.html', open_fixture_file('html_file.html'))
+      git_version.save!
+    end
+
+    ['dmp.json', 'file', 'html_file.html', 'diagram.png'].each do |path|
+      # Default to HTML response
+      get "/workflows/#{workflow.id}/git/1/blob/#{path}", headers: { 'Accept' => '*/*' }
+      assert_response :success
+      assert_equal 'text/html; charset=utf-8', response.headers['Content-Type']
+      assert response.body.start_with?('<!doctype html>')
+
+      # Even without any Accept
+      get "/workflows/#{workflow.id}/git/1/blob/#{path}", headers: { 'Accept' => '' }
+      assert_response :success
+      assert_equal 'text/html; charset=utf-8', response.headers['Content-Type']
+      assert response.body.start_with?('<!doctype html>')
+
+      # Default headers
+      get "/workflows/#{workflow.id}/git/1/blob/#{path}", headers: { 'Accept' => 'text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5'}
+      assert_response :success
+      assert_equal 'text/html; charset=utf-8', response.headers['Content-Type']
+      assert response.body.start_with?('<!doctype html>')
+
+      # Explicit HTML
+      get "/workflows/#{workflow.id}/git/1/blob/#{path}", headers: { 'Accept' => 'text/html' }
+      assert_response :success
+      assert_equal 'text/html; charset=utf-8', response.headers['Content-Type']
+      assert response.body.start_with?('<!doctype html>')
+
+      # Permit JSON response
+      get "/workflows/#{workflow.id}/git/1/blob/#{path}", headers: { 'Accept' => 'application/json' }
+      assert_response :success
+      assert_equal 'application/vnd.api+json; charset=utf-8', response.headers['Content-Type']
+      assert_equal path, JSON.parse(response.body)['path']
+
+      # Otherwise unrecognized format
+      assert_raises(ActionController::UnknownFormat) do
+        get "/workflows/#{workflow.id}/git/1/blob/#{path}", headers: { 'Accept' => 'application/zip' }
+      end
+    end
+  end
+end