Commit deaa7c85 authored by Kamil Trzciński's avatar Kamil Trzciński

Remove middlewares for `DetectCrossDatabaseModification`

This removes middlewares as they are already provided
by the `QueryAnalyzer`
parent 91c2cbb1
......@@ -4,7 +4,10 @@
if Gitlab.dev_or_test_env? || Gitlab::Utils.to_boolean(ENV['GITLAB_ENABLE_QUERY_ANALYZERS'], default: false)
Gitlab::Database::QueryAnalyzer.instance.hook!
Gitlab::Database::QueryAnalyzer.instance.all_analyzers.append(::Gitlab::Database::QueryAnalyzers::GitlabSchemasMetrics)
Gitlab::Database::QueryAnalyzer.instance.all_analyzers.append(::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification)
if Rails.env.test? || ENV['DISABLE_CROSS_DATABASE_MODIFICATION_DETECTION']
Gitlab::Database::QueryAnalyzer.instance.all_analyzers.append(::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification)
end
Gitlab::Application.configure do |config|
config.middleware.use(Gitlab::Middleware::QueryAnalyzer)
......
# frozen_string_literal: true
# Do not use middleware in tests since we already wrap all tests with
# `PreventCrossDatabaseModification` logic . Also the environment variable
# offers a quick way to disable this check if it is causing problems
unless Rails.env.test? || ENV['DISABLE_CROSS_DATABASE_MODIFICATION_DETECTION']
require_dependency 'gitlab/middleware/detect_cross_database_modification'
Gitlab::Application.configure do |config|
config.middleware.use(Gitlab::Middleware::DetectCrossDatabaseModification)
end
end
# frozen_string_literal: true
module Gitlab
module Middleware
class DetectCrossDatabaseModification
def initialize(app)
@app = app
end
def call(env)
if Feature.enabled?(:detect_cross_database_modification, default_enabled: :yaml)
::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.with_cross_database_modification_prevented(log_only: true) do
@app.call(env)
end
else
@app.call(env)
end
end
end
end
end
......@@ -41,12 +41,6 @@ module Gitlab
# so we can compare the latest WAL location against replica
chain.add ::Gitlab::SidekiqMiddleware::DuplicateJobs::Server
chain.add ::Gitlab::Database::LoadBalancing::SidekiqServerMiddleware
# Do not use middleware in tests since we already wrap all tests with
# `PreventCrossDatabaseModification` logic . Also the environment
# variable offers a quick way to disable this check if it is causing
# problems
chain.add ::Gitlab::SidekiqMiddleware::DetectCrossDatabaseModification unless Rails.env.test? || ENV['DISABLE_CROSS_DATABASE_MODIFICATION_DETECTION']
end
end
......
# frozen_string_literal: true
module Gitlab
module SidekiqMiddleware
class DetectCrossDatabaseModification
def call(worker, job, queue)
if Feature.enabled?(:detect_cross_database_modification, default_enabled: :yaml)
::Gitlab::Database::QueryAnalyzers::PreventCrossDatabaseModification.with_cross_database_modification_prevented(log_only: true) do
yield
end
else
yield
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Middleware::QueryAnalyzer, query_analyzers: false do
describe 'the PreventCrossDatabaseModification' do
describe '#call' do
let(:app) { double(:app) }
let(:middleware) { described_class.new(app) }
let(:env) { {} }
subject { middleware.call(env) }
context 'when there is a cross modification' do
before do
allow(app).to receive(:call) do
Project.transaction do
Project.where(id: -1).update_all(id: -1)
::Ci::Pipeline.where(id: -1).update_all(id: -1)
end
end
end
it 'detects cross modifications and logs them' do
expect(::Gitlab::ErrorTracking).to receive(:track_exception)
subject
end
context 'when the detect_cross_database_modification is disabled' do
before do
stub_feature_flags(detect_cross_database_modification: false)
end
it 'does not detect cross modifications' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
subject
end
end
end
context 'when there is no cross modification' do
before do
allow(app).to receive(:call) do
Project.transaction do
Project.where(id: -1).update_all(id: -1)
Namespace.where(id: -1).update_all(id: -1)
end
end
end
it 'does not log anything' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
subject
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::SidekiqMiddleware::DetectCrossDatabaseModification do
describe '#call' do
let(:worker) { double(:worker) }
let(:job) { { 'jid' => 'job123' } }
let(:queue) { 'some-queue' }
let(:middleware) { described_class.new }
def do_queries
end
subject { middleware.call(worker, job, queue) { do_queries } }
context 'when there is a cross modification' do
def do_queries
Project.transaction do
Project.where(id: -1).update_all(id: -1)
::Ci::Pipeline.where(id: -1).update_all(id: -1)
end
end
it 'detects cross modifications and logs them' do
expect(::Gitlab::ErrorTracking).to receive(:track_exception)
subject
end
context 'when the detect_cross_database_modification is disabled' do
before do
stub_feature_flags(detect_cross_database_modification: false)
end
it 'does not detect cross modifications' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
subject
end
end
end
context 'when there is no cross modification' do
def do_queries
Project.transaction do
Project.where(id: -1).update_all(id: -1)
Namespace.where(id: -1).update_all(id: -1)
end
end
it 'does not log anything' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
subject
end
end
end
end
......@@ -2,57 +2,59 @@
require 'spec_helper'
RSpec.describe Gitlab::Middleware::DetectCrossDatabaseModification do
describe '#call' do
let(:app) { double(:app) }
let(:middleware) { described_class.new(app) }
let(:env) { {} }
RSpec.describe Gitlab::SidekiqMiddleware::QueryAnalyzer, query_analyzers: false do
describe 'the PreventCrossDatabaseModification' do
describe '#call' do
let(:worker) { double(:worker) }
let(:job) { { 'jid' => 'job123' } }
let(:queue) { 'some-queue' }
let(:middleware) { described_class.new }
def do_queries
end
subject { middleware.call(env) }
subject { middleware.call(worker, job, queue) { do_queries } }
context 'when there is a cross modification' do
before do
allow(app).to receive(:call) do
context 'when there is a cross modification' do
def do_queries
Project.transaction do
Project.where(id: -1).update_all(id: -1)
::Ci::Pipeline.where(id: -1).update_all(id: -1)
end
end
end
it 'detects cross modifications and logs them' do
expect(::Gitlab::ErrorTracking).to receive(:track_exception)
subject
end
it 'detects cross modifications and logs them' do
expect(::Gitlab::ErrorTracking).to receive(:track_exception)
context 'when the detect_cross_database_modification is disabled' do
before do
stub_feature_flags(detect_cross_database_modification: false)
subject
end
it 'does not detect cross modifications' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
context 'when the detect_cross_database_modification is disabled' do
before do
stub_feature_flags(detect_cross_database_modification: false)
end
subject
it 'does not detect cross modifications' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
subject
end
end
end
end
context 'when there is no cross modification' do
before do
allow(app).to receive(:call) do
context 'when there is no cross modification' do
def do_queries
Project.transaction do
Project.where(id: -1).update_all(id: -1)
Namespace.where(id: -1).update_all(id: -1)
end
end
end
it 'does not log anything' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
it 'does not log anything' do
expect(::Gitlab::ErrorTracking).not_to receive(:track_exception)
subject
subject
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