Skip to content
This repository has been archived by the owner on Mar 16, 2021. It is now read-only.

V3 monitoring job: make RegistrationIdValidator case insensitive #197

Merged
merged 2 commits into from
Jun 7, 2017

Conversation

scottbommarito
Copy link

Currently it's just noise because all of the RegistrationIdValidator failures appear to be due to casing issues.

Also added unit tests (which was non-trivial because the testing framework did not support what I was trying to test).

@joelverhagen
Copy link
Member

joelverhagen commented Jun 7, 2017

In a Perfect World, this case insensitivity should not be done. There is a well known ID casing per version. This is defined by the package's .nuspec. The reason this PR is necessary is the casing of IDs in V2 API (and everywhere in the gallery) is determined by the first version uploaded with that ID. This is what defines the Id string the PackageRegistrations table. This field is what is JOINed into the query that produces non-hijacked V2 results (which include that original Id string repeated over and over for all versions of that ID).

Ideally the Package table would have a package ID column which has the correct casing for that specific version.

I understand this is a limitation of our database schema today, but we need to create an issue tracking this schema improvement and the corresponding revert of this PR.

Note that catalog and registration have the correct casing per version because they are derived off of the .nuspec. I think the search service is wrong because db2lucene takes the DBs value, not the .nuspec's value 😢.

Example: SQLite 3.12.2-alpha
This version's casing: SQLite
Original casing: sqlite
Catalog: https://api.nuget.org/v3/catalog0/data/2016.05.26.23.26.16/sqlite.3.12.2-alpha.json
Registration: https://api.nuget.org/v3/registration2/sqlite/index.json
Search (wrong): https://api-v2v3search-0.nuget.org/search/query?q=packageid:sqlite&ignoreFilter=true

Copy link
Member

@joelverhagen joelverhagen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File issue to revert this change when this DB change is possible then :shipit:

@skofman1
Copy link
Contributor

skofman1 commented Jun 7, 2017

@jver, what is the change you are proposing to the DB? To make queries key sensitive? To enforce the initial casing on all subsequent versions?

@joelverhagen
Copy link
Member

@skofman, I am suggesting to add a column to Packages table which has the correct ID casing for that specific version.

@scottbommarito
Copy link
Author

Issue has already been filed at NuGet/NuGetGallery#3349. Will add this additional discussion to it.

@scottbommarito scottbommarito merged commit 1ac6ca7 into dev Jun 7, 2017
@scottbommarito scottbommarito deleted the sb-v3idcase branch June 7, 2017 23:15
shishirx34 added a commit that referenced this pull request Jun 9, 2017
* Fixed Application Insights Sampling and Duplication Issues (#183)
* Removed Serilog's Request tracing
* Don't sample traces and exceptions
* Tweaked Search Service's Successful Request Telemetry Logic (#193)
* V3 monitoring job: emit validation status of packages to blob storage (#192)
* Enable blob snapshot (#177)
* Add V3 monitoring deployment scripts (#196)
* Fixed RoleInstance log value on Search Service (#194)
* Update telemetry processors (#198)
* V3 monitoring job: make RegistrationIdValidator case insensitive (#197)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants