Only use elasticsearch when it is enabled

Prior to this commit, we would sometimes *search* using elasticsearch
when that was disabled.

ee/app/models/ee/application_setting.rb

@@ -167,6 +167,22 @@ module EE
     end
     alias_method :elasticsearch_search?, :elasticsearch_search
 
+    # Determines whether a search should use elasticsearch, taking the scope
+    # (nil for global search, otherwise a namespace or project) into account
+    def search_using_elasticsearch?(scope: nil)
+      return false unless elasticsearch_indexing? && elasticsearch_search?
+      return true unless elasticsearch_limit_indexing?
+
+      case scope
+      when Namespace
+        elasticsearch_indexes_namespace?(scope)
+      when Project
+        elasticsearch_indexes_project?(scope)
+      else
+        false # Never use elasticsearch for the global scope when limiting is on
+      end
+    end
+
     def elasticsearch_url
       read_attribute(:elasticsearch_url).split(',').map(&:strip)
     end

ee/app/services/ee/search/global_service.rb

@@ -8,18 +8,19 @@ module EE
 
       override :execute
       def execute
-        if use_elasticsearch?
-          ::Gitlab::Elastic::SearchResults.new(current_user, params[:search],
-                                               elastic_projects, projects,
-                                               elastic_global)
-        else
-          super
-        end
+        return super unless use_elasticsearch?
+
+        ::Gitlab::Elastic::SearchResults.new(
+          current_user,
+          params[:search],
+          elastic_projects,
+          projects,
+          elastic_global
+        )
       end
 
       def use_elasticsearch?
-        ::Gitlab::CurrentSettings.elasticsearch_search? &&
-          !::Gitlab::CurrentSettings.elasticsearch_limit_indexing?
+        ::Gitlab::CurrentSettings.search_using_elasticsearch?(scope: nil)
       end
 
       def elastic_projects

ee/app/services/ee/search/group_service.rb

@@ -7,7 +7,7 @@ module EE
 
       override :use_elasticsearch?
       def use_elasticsearch?
-        group&.use_elasticsearch?
+        ::Gitlab::CurrentSettings.search_using_elasticsearch?(scope: group)
       end
 
       override :elastic_projects
@@ -22,11 +22,17 @@ module EE
 
       override :execute
       def execute
-        return super unless ::Gitlab::CurrentSettings.elasticsearch_search?
+        return super unless use_elasticsearch?
 
         ::Gitlab::Elastic::GroupSearchResults.new(
-          current_user, elastic_projects, projects, group, params[:search],
-          elastic_global, default_project_filter: default_project_filter)
+          current_user,
+          elastic_projects,
+          projects,
+          group,
+          params[:search],
+          elastic_global,
+          default_project_filter: default_project_filter
+        )
       end
     end
   end

ee/app/services/ee/search/project_service.rb

@@ -7,19 +7,19 @@ module EE
 
       override :execute
       def execute
-        if use_elasticsearch?
-          ::Gitlab::Elastic::ProjectSearchResults.new(current_user,
-                                                    params[:search],
-                                                    project.id,
-                                                    params[:repository_ref])
-        else
-          super
-        end
+        return super unless use_elasticsearch?
+
+        ::Gitlab::Elastic::ProjectSearchResults.new(
+          current_user,
+          params[:search],
+          project.id,
+          params[:repository_ref]
+        )
       end
 
       # This method is used in the top-level SearchService, so cannot be in-lined into #execute
       def use_elasticsearch?
-        project.use_elasticsearch?
+        ::Gitlab::CurrentSettings.search_using_elasticsearch?(scope: project)
       end
     end
   end

ee/app/services/ee/search/snippet_service.rb

@@ -6,18 +6,15 @@ module EE
       extend ::Gitlab::Utils::Override
 
       override :execute
-
       def execute
-        if use_elasticsearch?
-          ::Gitlab::Elastic::SnippetSearchResults.new(current_user, params[:search])
-        else
-          super
-        end
+        return super unless use_elasticsearch?
+
+        ::Gitlab::Elastic::SnippetSearchResults.new(current_user, params[:search])
       end
 
       # This method is used in the top-level SearchService, so cannot be in-lined into #execute
       def use_elasticsearch?
-        ::Gitlab::CurrentSettings.elasticsearch_search?
+        ::Gitlab::CurrentSettings.search_using_elasticsearch?(scope: nil)
       end
     end
   end

ee/changelogs/unreleased/11795-fix-searching-with-search-disabled.yml

+---
+title: Only use elasticsearch when it is enabled
+merge_request: 13495
+author:
+type: fixed

ee/spec/models/application_setting_spec.rb

@@ -238,6 +238,64 @@ describe ApplicationSetting do
     end
   end
 
+  describe '#search_using_elasticsearch?' do
+    # Constructs a truth table with 16 entries to run the specs against
+    where(indexing: [true, false], searching: [true, false], limiting: [true, false])
+
+    with_them do
+      set(:included_project_container) { create(:elasticsearch_indexed_project) }
+      set(:included_namespace_container) { create(:elasticsearch_indexed_namespace) }
+
+      set(:included_project) { included_project_container.project }
+      set(:included_namespace) { included_namespace_container.namespace }
+
+      set(:excluded_project) { create(:project) }
+      set(:excluded_namespace) { create(:namespace) }
+
+      let(:only_when_enabled_globally) { indexing && searching && !limiting }
+
+      subject { setting.search_using_elasticsearch?(scope: scope) }
+
+      before do
+        setting.update!(
+          elasticsearch_indexing: indexing,
+          elasticsearch_search: searching,
+          elasticsearch_limit_indexing: limiting
+        )
+      end
+
+      context 'global scope' do
+        let(:scope) { nil }
+
+        it { is_expected.to eq(only_when_enabled_globally) }
+      end
+
+      context 'namespace (in scope)' do
+        let(:scope) { included_namespace }
+
+        it { is_expected.to eq(indexing && searching) }
+      end
+
+      context 'namespace (not in scope)' do
+        let(:scope) { excluded_namespace }
+
+        it { is_expected.to eq(only_when_enabled_globally) }
+      end
+
+      context 'project (in scope)' do
+        let(:scope) { included_project }
+
+        it { is_expected.to eq(indexing && searching) }
+      end
+
+      context 'project (not in scope)' do
+        let(:scope) { excluded_project }
+
+        it { is_expected.to eq(only_when_enabled_globally) }
+      end
+    end
+  end
+
   describe 'custom project templates' do
     let(:group) { create(:group) }
     let(:projects) { create_list(:project, 3, namespace: group) }

ee/spec/services/search/global_service_spec.rb

@@ -3,13 +3,8 @@ require 'spec_helper'
 describe Search::GlobalService do
   let(:user) { create(:user) }
 
-  describe 'elasticsearch' do
-    before do
-      stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
-    end
-
-    subject(:results) { described_class.new(user, search: '*').execute }
-
-    it { is_expected.to be_a(::Gitlab::Elastic::SearchResults) }
+  it_behaves_like 'EE search service shared examples', ::Gitlab::SearchResults, ::Gitlab::Elastic::SearchResults do
+    let(:scope) { nil }
+    let(:service) { described_class.new(user, search: '*') }
   end
 end

ee/spec/services/search/group_service_spec.rb

 require 'spec_helper'
 
-describe Search::GroupService do
-  shared_examples_for 'group search' do
-    context 'finding projects by name' do
-      let(:user) { create(:user) }
-      let(:term) { "Project Name" }
-      let(:nested_group) { create(:group, :nested) }
-
-      # These projects shouldn't be found
-      let!(:outside_project) { create(:project, :public, name: "Outside #{term}") }
-      let!(:private_project) { create(:project, :private, namespace: nested_group, name: "Private #{term}" )}
-      let!(:other_project)   { create(:project, :public, namespace: nested_group, name: term.reverse) }
+describe Search::GroupService, :elastic do
+  let(:user) { create(:user) }
 
-      # These projects should be found
-      let!(:project1) { create(:project, :internal, namespace: nested_group, name: "Inner #{term} 1") }
-      let!(:project2) { create(:project, :internal, namespace: nested_group, name: "Inner #{term} 2") }
-      let!(:project3) { create(:project, :internal, namespace: nested_group.parent, name: "Outer #{term}") }
-
-      let(:results) { described_class.new(user, search_group, search: term).execute }
-      subject { results.objects('projects') }
+  it_behaves_like 'EE search service shared examples', ::Gitlab::GroupSearchResults, ::Gitlab::Elastic::GroupSearchResults do
+    let(:scope) { create(:group) }
+    let(:service) { described_class.new(user, scope, search: '*') }
+  end
 
-      context 'in parent group' do
-        let(:search_group) { nested_group.parent }
+  describe 'group search' do
+    let(:term) { "Project Name" }
+    let(:nested_group) { create(:group, :nested) }
 
-        it { is_expected.to match_array([project1, project2, project3]) }
-      end
+    # These projects shouldn't be found
+    let(:outside_project) { create(:project, :public, name: "Outside #{term}") }
+    let(:private_project) { create(:project, :private, namespace: nested_group, name: "Private #{term}" )}
+    let(:other_project)   { create(:project, :public, namespace: nested_group, name: term.reverse) }
 
-      context 'in subgroup' do
-        let(:search_group) { nested_group }
+    # These projects should be found
+    let(:project1) { create(:project, :internal, namespace: nested_group, name: "Inner #{term} 1") }
+    let(:project2) { create(:project, :internal, namespace: nested_group, name: "Inner #{term} 2") }
+    let(:project3) { create(:project, :internal, namespace: nested_group.parent, name: "Outer #{term}") }
 
-        it { is_expected.to match_array([project1, project2]) }
-      end
-    end
-  end
+    let(:results) { described_class.new(user, search_group, search: term).execute }
 
-  describe 'elasticsearch' do
     before do
-      stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
-      Gitlab::Elastic::Helper.create_empty_index
+      stub_ee_application_setting(
+        elasticsearch_search: true,
+        elasticsearch_indexing: true
+      )
 
       # Ensure these are present when the index is refreshed
       _ = [
@@ -48,23 +39,20 @@ describe Search::GroupService do
       Gitlab::Elastic::Helper.refresh_index
     end
 
-    after do
-      Gitlab::Elastic::Helper.delete_index
-    end
-
-    include_examples 'group search'
-  end
+    context 'finding projects by name' do
+      subject { results.objects('projects') }
 
-  describe 'elasticsearch result' do
-    let(:user) { create(:user) }
-    let(:group) { create(:group) }
+      context 'in parent group' do
+        let(:search_group) { nested_group.parent }
 
-    before do
-      stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
-    end
+        it { is_expected.to match_array([project1, project2, project3]) }
+      end
 
-    subject(:results) { described_class.new(user, group, search: '*').execute }
+      context 'in subgroup' do
+        let(:search_group) { nested_group }
 
-    it { is_expected.to be_a(::Gitlab::Elastic::GroupSearchResults) }
+        it { is_expected.to match_array([project1, project2]) }
+      end
+    end
   end
 end

ee/spec/services/search/project_service_spec.rb

 require 'spec_helper'
 
 describe Search::ProjectService do
-  let(:project) { create(:project) }
-  let(:user) { project.owner }
-
-  describe 'elasticsearch' do
-    before do
-      stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
-    end
-
-    subject(:results) { described_class.new(project, user, search: '*').execute }
-
-    it { is_expected.to be_a(::Gitlab::Elastic::ProjectSearchResults) }
+  it_behaves_like 'EE search service shared examples', ::Gitlab::ProjectSearchResults, ::Gitlab::Elastic::ProjectSearchResults do
+    let(:user) { scope.owner }
+    let(:scope) { create(:project) }
+    let(:service) { described_class.new(scope, user, search: '*') }
   end
 end

ee/spec/services/search/snippet_service_spec.rb

 require 'spec_helper'
 
 describe Search::SnippetService do
-  let(:user) { create(:user) }
-
-  describe 'elasticsearch' do
-    before do
-      stub_ee_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
-    end
-
-    subject(:results) { described_class.new(user, search: '*').execute }
-
-    it { is_expected.to be_a(::Gitlab::Elastic::SnippetSearchResults) }
+  it_behaves_like 'EE search service shared examples', ::Gitlab::SnippetSearchResults, ::Gitlab::Elastic::SnippetSearchResults do
+    let(:user) { create(:user) }
+    let(:scope) { nil }
+    let(:service) { described_class.new(user, search: '*') }
   end
 end

ee/spec/support/shared_examples/services/search_service_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'EE search service shared examples' do |normal_results, elasticsearch_results|
+  describe '#use_elasticsearch?' do
+    it 'delegates to Gitlab::CurrentSettings.search_using_elasticsearch?' do
+      expect(Gitlab::CurrentSettings)
+        .to receive(:search_using_elasticsearch?)
+        .with(scope: scope)
+        .and_return(:value)
+
+      expect(service.use_elasticsearch?).to eq(:value)
+    end
+  end
+
+  describe '#execute' do
+    subject { service.execute }
+
+    it 'returns an Elastic result object when elasticsearch is enabled' do
+      expect(Gitlab::CurrentSettings)
+        .to receive(:search_using_elasticsearch?)
+        .with(scope: scope)
+        .and_return(true)
+
+      is_expected.to be_a(elasticsearch_results)
+    end
+
+    it 'returns an ordinary result object when elasticsearch is disabled' do
+      expect(Gitlab::CurrentSettings)
+        .to receive(:search_using_elasticsearch?)
+        .with(scope: scope)
+        .and_return(false)
+
+      is_expected.to be_a(normal_results)
+    end
+  end
+end