Move constants and update for feedback42572-release-controller

4 files changed, 15 insertions, 18 deletions

diff --git a/lib/gitlab/middleware/read_only.rb b/lib/gitlab/middleware/read_only.rb
index 19b74c0c122..d9d5f90596f 100644
--- a/lib/gitlab/middleware/read_only.rb
+++ b/lib/gitlab/middleware/read_only.rb
@@ -1,8 +1,6 @@
 module Gitlab
   module Middleware
     class ReadOnly
-      DISALLOWED_METHODS = %w(POST PATCH PUT DELETE).freeze
-      APPLICATION_JSON = 'application/json'.freeze
       API_VERSIONS = (3..4)
 
       def self.internal_routes
diff --git a/lib/gitlab/middleware/read_only/controller.rb b/lib/gitlab/middleware/read_only/controller.rb
index 053cb6f0a9f..45b644e6510 100644
--- a/lib/gitlab/middleware/read_only/controller.rb
+++ b/lib/gitlab/middleware/read_only/controller.rb
@@ -2,6 +2,10 @@ module Gitlab
   module Middleware
     class ReadOnly
       class Controller
+        DISALLOWED_METHODS = %w(POST PATCH PUT DELETE).freeze
+        APPLICATION_JSON = 'application/json'.freeze
+        ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance'.freeze
+
         def initialize(app, env)
           @app = app
           @env = env
@@ -10,12 +14,11 @@ module Gitlab
         def call
           if disallowed_request? && Gitlab::Database.read_only?
             Rails.logger.debug('GitLab ReadOnly: preventing possible non read-only operation')
-            error_message = 'You cannot do writing operations on a read-only GitLab instance'
 
             if json_request?
-              return [403, { 'Content-Type' => 'application/json' }, [{ 'message' => error_message }.to_json]]
+              return [403, { 'Content-Type' => APPLICATION_JSON }, [{ 'message' => ERROR_MESSAGE }.to_json]]
             else
-              rack_flash.alert = error_message
+              rack_flash.alert = ERROR_MESSAGE
               rack_session['flash'] = rack_flash.to_session_value
 
               return [301, { 'Location' => last_visited_url }, []]
diff --git a/spec/lib/gitlab/middleware/read_only_spec.rb b/spec/lib/gitlab/middleware/read_only_spec.rb
index b3c85142b82..39ec2f37a83 100644
--- a/spec/lib/gitlab/middleware/read_only_spec.rb
+++ b/spec/lib/gitlab/middleware/read_only_spec.rb
@@ -14,14 +14,14 @@ describe Gitlab::Middleware::ReadOnly do
       alert = middleware.env['rack.session'].to_hash
         .dig('flash', 'flashes', 'alert')
 
-      alert&.include?('You cannot do writing operations')
+      alert&.include?('You cannot perform write operations')
     end
   end
 
   RSpec::Matchers.define :disallow_request_in_json do
     match do |response|
       json_response = JSON.parse(response.body)
-      response.body.include?('You cannot do writing operations') && json_response.key?('message')
+      response.body.include?('You cannot perform write operations') && json_response.key?('message')
     end
   end
 
@@ -47,14 +47,14 @@ describe Gitlab::Middleware::ReadOnly do
     end
   end
 
+  let(:request) { Rack::MockRequest.new(rack_stack) }
+
   subject do
-    app = described_class.new(fake_app)
-    app.extend(observe_env)
-    app
+    described_class.new(fake_app).tap do |app|
+      app.extend(observe_env)
+    end
   end
 
-  let(:request) { Rack::MockRequest.new(rack_stack) }
-
   context 'normal requests to a read-only Gitlab instance' do
     let(:fake_app) { lambda { |env| [200, { 'Content-Type' => 'text/plain' }, ['OK']] } }
 
diff --git a/spec/lib/gitlab/middleware/release_env_spec.rb b/spec/lib/gitlab/middleware/release_env_spec.rb
index 657b705502a..5e3aa877409 100644
--- a/spec/lib/gitlab/middleware/release_env_spec.rb
+++ b/spec/lib/gitlab/middleware/release_env_spec.rb
@@ -1,16 +1,12 @@
 require 'spec_helper'
 
 describe Gitlab::Middleware::ReleaseEnv do
-  let(:inner_app) { double(:app) }
+  let(:inner_app) { double(:app, call: 'yay') }
   let(:app) { described_class.new(inner_app) }
   let(:env) { { 'action_controller.instance' => 'something' } }
 
-  before do
-    expect(inner_app).to receive(:call).with(env).and_return('yay')
-  end
-
   describe '#call' do
-    it 'calls the app and delete the controller' do
+    it 'calls the app and clears the env' do
       result = app.call(env)
 
       expect(result).to eq('yay')