Rename search_scope method name in search api

What does this MR do and why?

I have found the number of things referenced as search_scope or scope in the code hard to follow. I opened this MR to make it very clear when the scope being referenced was the user requested scope vs what search scope was actually run in the search API.

  • Any pre-search checks/uses should use user_requested_search_scope
  • any post-search checks/uses should use search_service.scope
    • exceptions for errors where no search was run, SLIs get attributed to the user_requested_search_scope

AI summary

This code refactors how search functionality validates and handles different search scopes (like searching through code blobs vs other content types).

The main changes include:

  1. Simplified validation logic: The code that checks if a search scope is allowed was reorganized into cleaner, separate functions that are easier to understand and maintain.

  2. Better error messaging: Instead of having error messages scattered throughout the code, they're now centralized in a dedicated function that provides appropriate messages based on the specific search scope being used.

  3. Improved data consistency: The code now consistently uses the search service's scope property instead of mixing between user input and processed values, which reduces potential bugs and makes the data flow more predictable.

  4. Enhanced code organization: Complex validation logic was broken down into smaller, focused functions (scope_allowed? and scope_error_message) that each handle one specific responsibility.

These changes make the search validation more reliable and the codebase easier to maintain, while preserving the same functionality that users experience.

References

Related to #588341 (closed)

Screenshots or screen recordings

Before After

How to set up and validate locally

if you want to set up and validate this locally, you could test it two ways:

  1. with basic search only (gdk not setup with elasticsearch enabled and advanced search turned off)
  2. with advanced search (gdk setup with elasticsearch enabled and advanced search enabled)

run through search API requests to make sure everything works.

https://docs.gitlab.com/api/search/

Automated validation

Note: advanced search is required to be enabled for this script to work

i asked duo to help me write a local test script to help and it produced this:

Click to expand test script
#!/usr/bin/env ruby

require 'net/http'
require 'json'
require 'uri'

API_KEY = ENV['GDK_API_KEY']
BASE_URL = 'https://gdk.test:3443'

if API_KEY.nil? || API_KEY.empty?
  puts "Error: GDK_API_KEY environment variable not set"
  exit 1
end

class SearchScopeTester
  BASIC_SCOPES = %w[projects issues merge_requests milestones users]
  ADVANCED_SCOPES = %w[commits notes wiki_blobs]
  BLOB_SCOPE = 'blobs'

  def initialize
    @results = []
  end

  def test_all
    puts "Testing search API scopes..."
    puts "=" * 80

    test_scopes
    print_summary
  end

  private

  def test_scopes
    # Test basic scopes (should work with basic search)
    BASIC_SCOPES.each do |scope|
      test_search(
        level: 'global',
        search_type: 'basic',
        scope: scope,
        search_term: 'test'
      )
    end

    # Test advanced scopes (require advanced search)
    ADVANCED_SCOPES.each do |scope|
      test_search(
        level: 'global',
        search_type: 'basic',
        scope: scope,
        search_term: 'test',
        expect_error: true
      )
    end

    # Test blob scope (requires advanced or zoekt)
    test_search(
      level: 'global',
      search_type: 'basic',
      scope: BLOB_SCOPE,
      search_term: 'class',
      expect_error: true
    )

    # Test with advanced search type
    test_search(
      level: 'global',
      search_type: 'advanced',
      scope: BLOB_SCOPE,
      search_term: 'class'
    )
  end

  def test_search(level:, search_type:, scope:, search_term:, expect_error: false)
    params = {
      search_type: search_type,
      scope: scope,
      search: search_term
    }

    response = make_request(params)
    status = response.code.to_i
    result_count = parse_result_count(response)

    passed = if expect_error
               status >= 400
             else
               status == 200
             end

    @results << {
      level: level,
      search_type: search_type,
      scope: scope,
      status: status,
      count: result_count,
      passed: passed,
      expect_error: expect_error,
      error: status >= 400 ? parse_error(response) : nil
    }

    print '.'
  end

  def make_request(params)
    uri = URI("#{BASE_URL}/api/v4/search")
    uri.query = URI.encode_www_form(params)

    http = Net::HTTP.new(uri.host, uri.port)
    http.use_ssl = true
    http.verify_mode = OpenSSL::SSL::VERIFY_NONE

    request = Net::HTTP::Get.new(uri)
    request['PRIVATE-TOKEN'] = API_KEY

    http.request(request)
  end

  def parse_result_count(response)
    return 0 unless response.code.to_i == 200

    body = JSON.parse(response.body)
    body.is_a?(Array) ? body.length : 0
  rescue JSON::ParserError
    0
  end

  def parse_error(response)
    body = JSON.parse(response.body)
    body['error'] || body['message']
  rescue JSON::ParserError
    'Unknown error'
  end

  def truncate_text(str, max_length)
    return str if str.length <= max_length
    "#{str[0...max_length - 3]}..."
  end

  def print_summary
    puts "\n\n"
    puts "=" * 100
    puts "SUMMARY"
    puts "=" * 100

    printf "%-15s %-15s %-20s %-8s %-8s %-50s %-6s\n",
           "Level", "Search Type", "Scope", "Status", "Count", "Error", "Result"
    puts "-" * 100

    @results.each do |r|
      result_text = r[:passed] ? "✓ PASS" : "✗ FAIL"
      error_text = r[:error] || (r[:expect_error] ? "Expected error" : "")
      printf "%-15s %-15s %-20s %-8s %-8s %-50s %-6s\n",
             r[:level], r[:search_type], r[:scope],
             r[:status], r[:count], truncate_text(error_text, 48), result_text
    end

    puts "=" * 100

    passed = @results.count { |r| r[:passed] }
    total = @results.length
    puts "\nTotal: #{passed}/#{total} passed"
  end
end

tester = SearchScopeTester.new
tester.test_all
Testing search API scopes...
================================================================================
..........

====================================================================================================
SUMMARY
====================================================================================================
Level           Search Type     Scope                Status   Count    Error                                              Result
----------------------------------------------------------------------------------------------------
global          basic           projects             200      14                                                          ✓ PASS
global          basic           issues               200      4                                                           ✓ PASS
global          basic           merge_requests       200      1                                                           ✓ PASS
global          basic           milestones           200      0                                                           ✓ PASS
global          basic           users                200      1                                                           ✓ PASS
global          basic           commits              400      0        {"error"=>"Scope supported only with advanced search"} ✓ PASS
global          basic           notes                400      0        {"error"=>"Scope supported only with advanced search"} ✓ PASS
global          basic           wiki_blobs           400      0        {"error"=>"Scope supported only with advanced search"} ✓ PASS
global          basic           blobs                400      0        {"error"=>"Scope supported only with advanced search or exact code search"} ✓ PASS
global          advanced        blobs                200      20                                                          ✓ PASS
====================================================================================================

Total: 10/10 passed

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Terri Chu

Merge request reports

Loading