Commit 222fcf36 authored by Dylan Griffith's avatar Dylan Griffith

Avoid use of Elasticsearch joins in Global Merge Request searches

This is the 2nd last step in our efforts to avoid using joins in
Elasticsearch and move merge requests to a new index
https://gitlab.com/groups/gitlab-org/-/epics/5468 . This changes the
behaviour of searches at the global (any group, any project) level to no
longer use joins (`has_parent`) queries and instead just use the new
`project_id` and `merge_requests_access_level` fields that were added to
the `merge_requests` type (denormalized).

These new fields needed to avoid joins are migrated in the migration
added in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59836 .

This MR just needs to check that the migration is finished and if it is
we can set `no_join_project` which was implemented in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48583 .

We also need to make sure we have tests for the state where the
migration is not run so we don't accidentally break permission logic for
unmigrated indices in any future refactorings.

Changelog: changed
parent 18d9d2d1
---
title: Avoid use of Elasticsearch joins in Global Merge Request searches
merge_request: 60467
author:
type: changed
......@@ -18,6 +18,7 @@ module Elastic
end
options[:features] = 'merge_requests'
options[:no_join_project] = Elastic::DataMigrationService.migration_has_finished?(:add_new_data_to_merge_requests_documents)
context.name(:merge_request) do
query_hash = context.name(:authorized) { project_ids_filter(query_hash, options) }
query_hash = context.name(:match) { state_filter(query_hash, options) }
......
......@@ -30,6 +30,12 @@ RSpec.describe Search::GlobalService do
it_behaves_like 'search query applies joins based on migrations shared examples', :add_permissions_data_to_notes_documents
end
context 'merge_requests search' do
let(:results) { described_class.new(nil, search: '*').execute.objects('merge_requests') }
it_behaves_like 'search query applies joins based on migrations shared examples', :add_new_data_to_merge_requests_documents
end
context 'visibility', :elastic, :sidekiq_inline do
include_context 'ProjectPolicyTable context'
......@@ -62,6 +68,28 @@ RSpec.describe Search::GlobalService do
with_them do
it_behaves_like 'search respects visibility'
end
# Since newly created indices automatically have all migrations as
# finished we need a test to verify the old style searches work for
# instances which haven't finished the migration yet
context 'when add_new_data_to_merge_requests_documents migration is not finished' do
before do
set_elasticsearch_migration_to :add_new_data_to_merge_requests_documents, including: false
end
# merge_request cannot be defined prior to the migration mocks because it
# will cause the incorrect value to be passed to `use_separate_indices` when creating
# the proxy
let!(:merge_request) { create(:merge_request, target_project: project, source_project: project) }
where(:project_level, :feature_access_level, :membership, :admin_mode, :expected_count) do
permission_table_for_reporter_feature_access
end
with_them do
it_behaves_like 'search respects visibility'
end
end
end
context 'blob and commit' do
......
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