Merge branch 'fix-search-limits' into 'master'

Fix search limits when ES search isn't enabled globally

See merge request gitlab-org/gitlab!22661

app/controllers/search_controller.rb

@@ -5,9 +5,6 @@ class SearchController < ApplicationController
   include SearchHelper
   include RendersCommits
 
-  NON_ES_SEARCH_TERM_LIMIT = 64
-  NON_ES_SEARCH_CHAR_LIMIT = 4096
-
   around_action :allow_gitaly_ref_name_caching
 
   skip_before_action :authenticate_user!
@@ -68,19 +65,13 @@ class SearchController < ApplicationController
   private
 
   def search_term_valid?
-    return true if Gitlab::CurrentSettings.elasticsearch_search?
-
-    chars_count = params[:search].length
-    if chars_count > NON_ES_SEARCH_CHAR_LIMIT
-      flash[:alert] = t('errors.messages.search_chars_too_long', count: NON_ES_SEARCH_CHAR_LIMIT)
-
+    unless search_service.valid_query_length?
+      flash[:alert] = t('errors.messages.search_chars_too_long', count: SearchService::SEARCH_CHAR_LIMIT)
       return false
     end
 
-    search_terms_count = params[:search].split.count { |word| word.length >= 3 }
-    if search_terms_count > NON_ES_SEARCH_TERM_LIMIT
-      flash[:alert] = t('errors.messages.search_terms_too_long', count: NON_ES_SEARCH_TERM_LIMIT)
-
+    unless search_service.valid_terms_count?
+      flash[:alert] = t('errors.messages.search_terms_too_long', count: SearchService::SEARCH_TERM_LIMIT)
       return false
     end
 

app/services/search_service.rb

@@ -3,6 +3,9 @@
 class SearchService
   include Gitlab::Allowable
 
+  SEARCH_TERM_LIMIT = 64
+  SEARCH_CHAR_LIMIT = 4096
+
   def initialize(current_user, params = {})
     @current_user = current_user
     @params = params.dup
@@ -42,6 +45,14 @@ class SearchService
     @show_snippets = params[:snippets] == 'true'
   end
 
+  def valid_query_length?
+    params[:search].length <= SEARCH_CHAR_LIMIT
+  end
+
+  def valid_terms_count?
+    params[:search].split.count { |word| word.length >= 3 } <= SEARCH_TERM_LIMIT
+  end
+
   delegate :scope, to: :search_service
 
   def search_results

ee/app/services/ee/search_service.rb

@@ -53,6 +53,18 @@ module EE
       )
     end
 
+    def valid_query_length?
+      return true if use_elasticsearch?
+
+      super
+    end
+
+    def valid_terms_count?
+      return true if use_elasticsearch?
+
+      super
+    end
+
     private
 
     def logger

spec/controllers/search_controller_spec.rb

@@ -106,8 +106,8 @@ describe SearchController do
 
       context 'check search term length' do
         let(:search_queries) do
-          char_limit = controller.class::NON_ES_SEARCH_CHAR_LIMIT
-          term_limit = controller.class::NON_ES_SEARCH_TERM_LIMIT
+          char_limit = SearchService::SEARCH_CHAR_LIMIT
+          term_limit = SearchService::SEARCH_TERM_LIMIT
           {
             chars_under_limit: ('a' * (char_limit - 1)),
             chars_over_limit: ('a' * (char_limit + 1)),
@@ -116,21 +116,15 @@ describe SearchController do
           }
         end
 
-        where(:es_enabled, :string_name, :expectation) do
-          true  | :chars_under_limit | :not_to_set_flash
-          true  | :chars_over_limit  | :not_to_set_flash
-          true  | :terms_under_limit | :not_to_set_flash
-          true  | :terms_over_limit  | :not_to_set_flash
-          false | :chars_under_limit | :not_to_set_flash
-          false | :chars_over_limit  | :set_chars_flash
-          false | :terms_under_limit | :not_to_set_flash
-          false | :terms_over_limit  | :set_terms_flash
+        where(:string_name, :expectation) do
+          :chars_under_limit | :not_to_set_flash
+          :chars_over_limit  | :set_chars_flash
+          :terms_under_limit | :not_to_set_flash
+          :terms_over_limit  | :set_terms_flash
         end
 
         with_them do
           it do
-            allow(Gitlab::CurrentSettings).to receive(:elasticsearch_search?).and_return(es_enabled)
-
             get :show, params: { scope: 'projects', search: search_queries[string_name] }
 
             case expectation