Skip to content
Snippets Groups Projects

Make nuget symbol files search case-insensitive

Context

In Endpoint for symbol file download (!134564 - merged), we added an endpoint to download the pdb symbol files. To find the file in the packages_nuget_symbols database table, we use 3 pieces of information we receive in the request:

  • file signature
  • file name
  • file checksum

The search in the database is case-sensitive. So if the file signature is sent in a different case than the one saved in the database, the server will return 404. Some users reported that their debuggers are sending those parameters in a different case than the one Gitlab expects.

To avoid this discrepancy in how debuggers (Visual Studio for example) handle the case of the sent parameters, we need to make our search case-insensitive.

What does this MR do?

  • Change the search-related scopes in the Packages::Nuget::Symbol model to make them case-insensitive.
  • Add a database index on the lowercase version of these columns.
  • Update related specs.

MR acceptance checklist

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

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

  1. Open rails console and run the following commands to create a symbol file that we can test with:
# stub file upload
def fixture_file_upload(*args, **kwargs)
  Rack::Test::UploadedFile.new(*args, **kwargs)
end

package = FactoryBot.create(:nuget_package, project: Project.last)
symbol = FactoryBot.create(:nuget_symbol, package: package)
# keep a record of the `signature` & `file` name fields of the created symbol.
  1. Uppercase the signature & file and look for the symbol using the uppercase values:
Packages::Nuget::Symbol.with_signature(uppercase_signature).with_file_name(uppercase_filename)
  1. The symbol record should be returned. On master branch, it will not be found.

Related to #438888 (closed)

Edited by Moaz Khalifa

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • assigned to @mkhalifa3

  • 3 Warnings
    :warning: bca5eb51: The commit body should not contain more than 72 characters per line. For more information, take a look at our Commit message guidelines.
    :warning: c64c234d: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines.
    :warning: This merge request does not refer to an existing milestone.

    Reviewer roulette

    Changes that require review have been detected!

    Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

    Category Reviewer Maintainer
    backend @syarynovskyi profile link current availability (UTC+2, 1 hour ahead of author) @rymai profile link current availability (UTC+1, same timezone as author)
    database @carlad-gl profile link current availability (UTC+1, same timezone as author) @praba.m7n profile link current availability (UTC+5.5, 4.5 hours ahead of author)

    Please check reviewer's status!

    • available Reviewer is available!
    • unavailable Reviewer is unavailable!

    Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

    To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

    Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

    If needed, you can retry the :repeat: danger-review job that generated this comment.

    Generated by :no_entry_sign: Danger

    Edited by Ghost User
  • Database migrations (on the main database)

    Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access).

    Migration Type Total runtime Result DB size change
    20240126101101 - AddIndexPackagesNugetSymbolsOnLowercaseSignatureAndFileName Post deploy 3.8 s :white_check_mark: +4.41 MiB
    Runtime Histogram for all migrations
    Query Runtime Count
    0 seconds - 0.01 seconds 0
    0.01 seconds - 0.1 seconds 2
    0.1 seconds - 1 second 0
    1 second - 5 seconds 1
    5 seconds - 15 seconds 0
    15 seconds - 5 minutes 0
    5 minutes + 0

    Migration: 20240126101101 - AddIndexPackagesNugetSymbolsOnLowercaseSignatureAndFileName

    • Type: Post deploy
    • Duration: 3.8 s
    • Database size change: +4.41 MiB
    Calls Total Time Max Time Mean Time Rows Query
    1 1885.5 ms 1885.5 ms 1885.5 ms 0
    CREATE INDEX CONCURRENTLY "idx_pkgs_nuget_symbols_on_lowercase_signature_and_file_name" ON "packages_nuget_symbols" (lower(signature), lower(file))
    2 0.0 ms 0.0 ms 0.0 ms 2
    SELECT pg_backend_pid()
    Histogram for AddIndexPackagesNugetSymbolsOnLowercaseSignatureAndFileName
    Query Runtime Count
    0 seconds - 0.01 seconds 0
    0.01 seconds - 0.1 seconds 2
    0.1 seconds - 1 second 0
    1 second - 5 seconds 1
    5 seconds - 15 seconds 0
    15 seconds - 5 minutes 0
    5 minutes + 0

    Other information

    Other migrations pending on GitLab.com
    Migration Type Total runtime Result DB size change
    20240122071840 - EnsureBackfillForCiBuildsIntegerColumnsIsFinished Post deploy 1.7 s :white_check_mark: +0.00 B
    20240123071840 - PrepareAsyncIndexesForPCiBuildsAutoCanceledById Post deploy 3.9 s :white_check_mark: +8.00 KiB [note]
    20240124081840 - PrepareAsyncIndexesForPCiBuildsCommitIdPart1 Post deploy 5.9 s :white_check_mark: +0.00 B
    Clone details
    Clone ID Clone Created At Clone Data Timestamp Expected Removal Time
    database-testing-2791782-12639222-main 2024-01-26T10:38:57Z 2024-01-26T05:08:48Z 2024-01-26 22:43:14 +0000
    database-testing-2791782-12639222-ci 2024-01-26T10:38:57Z 2024-01-26T08:45:46Z 2024-01-26 22:43:14 +0000

    Job artifacts

    Database migrations (on the ci database)

    Migrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access).

    Migration Type Total runtime Result DB size change
    20240126101101 - AddIndexPackagesNugetSymbolsOnLowercaseSignatureAndFileName Post deploy 2.8 s :white_check_mark: +8.00 KiB [note]
    Runtime Histogram for all migrations
    Query Runtime Count
    0 seconds - 0.01 seconds 0
    0.01 seconds - 0.1 seconds 2
    0.1 seconds - 1 second 1
    1 second - 5 seconds 0
    5 seconds - 15 seconds 0
    15 seconds - 5 minutes 0
    5 minutes + 0

    Migration: 20240126101101 - AddIndexPackagesNugetSymbolsOnLowercaseSignatureAndFileName

    • Type: Post deploy
    • Duration: 2.8 s
    • Database size change: +8.00 KiB [note]
    Calls Total Time Max Time Mean Time Rows Query
    1 72.2 ms 72.2 ms 72.2 ms 0
    CREATE INDEX CONCURRENTLY "idx_pkgs_nuget_symbols_on_lowercase_signature_and_file_name" ON "packages_nuget_symbols" (lower(signature), lower(file))
    2 0.0 ms 0.0 ms 0.0 ms 2
    SELECT pg_backend_pid()
    Histogram for AddIndexPackagesNugetSymbolsOnLowercaseSignatureAndFileName
    Query Runtime Count
    0 seconds - 0.01 seconds 0
    0.01 seconds - 0.1 seconds 2
    0.1 seconds - 1 second 1
    1 second - 5 seconds 0
    5 seconds - 15 seconds 0
    15 seconds - 5 minutes 0
    5 minutes + 0

    Other information

    Other migrations pending on GitLab.com
    Migration Type Total runtime Result DB size change
    20240122071840 - EnsureBackfillForCiBuildsIntegerColumnsIsFinished Post deploy 2.6 s :white_check_mark: +0.00 B
    20240123071840 - PrepareAsyncIndexesForPCiBuildsAutoCanceledById Post deploy 4.7 s :white_check_mark: +8.00 KiB [note]
    20240124081840 - PrepareAsyncIndexesForPCiBuildsCommitIdPart1 Post deploy 6.6 s :white_check_mark: +0.00 B
    Clone details
    Clone ID Clone Created At Clone Data Timestamp Expected Removal Time
    database-testing-2791782-12639222-main 2024-01-26T10:38:57Z 2024-01-26T05:08:48Z 2024-01-26 22:43:14 +0000
    database-testing-2791782-12639222-ci 2024-01-26T10:38:57Z 2024-01-26T08:45:46Z 2024-01-26 22:43:14 +0000

    Job artifacts


    Brought to you by gitlab-org/database-team/gitlab-com-database-testing. Epic

    Edited by Ghost User
  • E2E Test Result Summary

    allure-report-publisher generated test report!

    e2e-test-on-gdk: :white_check_mark: test report for c64c234d

    expand test summary
    +------------------------------------------------------------------+
    |                          suites summary                          |
    +-------------+--------+--------+---------+-------+-------+--------+
    |             | passed | failed | skipped | flaky | total | result |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Plan        | 4      | 0      | 0       | 0     | 4     | ✅     |
    | Package     | 15     | 0      | 1       | 0     | 16    | ✅     |
    | Monitor     | 4      | 0      | 0       | 0     | 4     | ✅     |
    | Create      | 8      | 0      | 3       | 0     | 11    | ✅     |
    | Data Stores | 2      | 0      | 0       | 0     | 2     | ✅     |
    | Govern      | 3      | 0      | 0       | 0     | 3     | ✅     |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Total       | 36     | 0      | 4       | 0     | 40    | ✅     |
    +-------------+--------+--------+---------+-------+-------+--------+

    e2e-package-and-test: :x: test report for c64c234d

    expand test summary
    +------------------------------------------------------------------+
    |                          suites summary                          |
    +-------------+--------+--------+---------+-------+-------+--------+
    |             | passed | failed | skipped | flaky | total | result |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Package     | 233    | 0      | 11      | 24    | 244   | ✅     |
    | Create      | 148    | 1      | 19      | 2     | 168   | ❌     |
    | Monitor     | 8      | 0      | 0       | 0     | 8     | ✅     |
    | Govern      | 6      | 0      | 0       | 0     | 6     | ✅     |
    | Plan        | 8      | 0      | 0       | 0     | 8     | ✅     |
    | Data Stores | 4      | 0      | 0       | 0     | 4     | ✅     |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Total       | 407    | 1      | 30      | 26    | 438   | ❌     |
    +-------------+--------+--------+---------+-------+-------+--------+
    Edited by Ghost User
  • Moaz Khalifa changed the description

    changed the description

  • Moaz Khalifa requested review from @ahegyi and @carlad-gl

    requested review from @ahegyi and @carlad-gl

  • mentioned in issue #438888 (closed)

  • Carla Drago requested review from @cablett and removed review request for @carlad-gl

    requested review from @cablett and removed review request for @carlad-gl

  • Moaz Khalifa
  • charlie ablett
  • charlie ablett
  • charlie ablett
  • charlie ablett
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading