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
- 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.
- 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)
- The symbol record should be returned. On master branch, it will not be found.
Related to #438888 (closed)
Merge request reports
Activity
assigned to @mkhalifa3
- A deleted user
added database databasereview pending labels
3 Warnings bca5eb51: The commit body should not contain more than 72 characters per line. For more information, take a look at our Commit message guidelines. 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. 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
(UTC+2, 1 hour ahead of author)
@rymai
(UTC+1, same timezone as author)
database @carlad-gl
(UTC+1, same timezone as author)
@praba.m7n
(UTC+5.5, 4.5 hours ahead of author)
Please check reviewer's status!
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
danger-review
job that generated this comment.Generated by
DangerEdited by Ghost UserDatabase 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 +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 +0.00 B 20240123071840 - PrepareAsyncIndexesForPCiBuildsAutoCanceledById Post deploy 3.9 s +8.00 KiB [note] 20240124081840 - PrepareAsyncIndexesForPCiBuildsCommitIdPart1 Post deploy 5.9 s +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 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 +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 +0.00 B 20240123071840 - PrepareAsyncIndexesForPCiBuildsAutoCanceledById Post deploy 4.7 s +8.00 KiB [note] 20240124081840 - PrepareAsyncIndexesForPCiBuildsCommitIdPart1 Post deploy 6.6 s +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
Brought to you by gitlab-org/database-team/gitlab-com-database-testing. Epic
Edited by Ghost User- A deleted user
added database-testing-automation label
E2E Test Result Summary
allure-report-publisher
generated test report!e2e-test-on-gdk:
test report for c64c234dexpand 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:
test report for c64c234dexpand 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 Userrequested review from @ahegyi and @carlad-gl
- Resolved by euko
Hi
can you please- @ahegyi do the initial backend review?
- @carlad-gl do the initial database reiew?
If both of you have the capacity for sure. Thank you
added workflowin review label and removed workflowready for development label
mentioned in issue #438888 (closed)
requested review from @cablett and removed review request for @carlad-gl
- Resolved by euko
- Resolved by euko
- Resolved by euko
- Resolved by euko
- Resolved by euko
- Resolved by charlie ablett
- Resolved by charlie ablett