Commit fd4ced7a authored by Dylan Griffith's avatar Dylan Griffith

Allow prevent_cross_joins to be used directly in tests

This method can be passed a block and we can validate that no SQL query
in that code block attempts to join across ci/main databases. This is
useful in the intermediate time while we attempt to fix many small
issues until we enable this as the default in all RSpec tests.
parent ea72d0ff
......@@ -243,3 +243,38 @@ A quick checklist for fixing a specific join query would be:
adding a new column
1. Can we remove the join by adding a new table in the correct database that
replicates the minimum data needed to do the join
#### How to validate you have correctly removed a cross-join
We have introduced a method you can use in RSpec tests to validate all SQL
queries within a code block to ensure that none of them are joining across the
2 databases. This is a useful tool to confirm you have correctly fixed an
existing cross-join.
At some point in the future we will have fixed all cross-joins and this tool
will run by default in all tests but for now it needs to be explicitly enabled
for your test.
You can use this method like so:
```ruby
it 'does not join across databases' do
with_cross_joins_prevented do
::Ci::Build.joins(:project).to_a
end
end
```
This will raise an exception if the query joins across the 2 databases. The
above example is fixed by removing the join like so:
```ruby
it 'does not join across databases' do
with_cross_joins_prevented do
::Ci::Build.preload(:project).to_a
end
end
```
You can see a real example of using this method for fixing a cross-join in
<https://gitlab.com/gitlab-org/gitlab/-/merge_requests/67655>.
......@@ -56,6 +56,20 @@ module Database
end
end
module SpecHelpers
def with_cross_joins_prevented
subscriber = ActiveSupport::Notifications.subscribe('sql.active_record') do |event|
::Database::PreventCrossJoins.validate_cross_joins!(event.payload[:sql])
end
Thread.current[:allow_cross_joins_across_databases] = false
yield
ensure
ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber
end
end
module GitlabDatabaseMixin
def allow_cross_joins_across_databases(url:)
Thread.current[:allow_cross_joins_across_databases] = true
......@@ -69,16 +83,10 @@ Gitlab::Database.singleton_class.prepend(
Database::PreventCrossJoins::GitlabDatabaseMixin)
RSpec.configure do |config|
config.include(::Database::PreventCrossJoins::SpecHelpers)
# TODO: remove `:prevent_cross_joins` to enable the check by default
config.around(:each, :prevent_cross_joins) do |example|
subscriber = ActiveSupport::Notifications.subscribe('sql.active_record') do |event|
::Database::PreventCrossJoins.validate_cross_joins!(event.payload[:sql])
end
Thread.current[:allow_cross_joins_across_databases] = false
example.run
ensure
ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber
with_cross_joins_prevented { example.run }
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