Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix digest calculation from misuse or tr #1236

Merged
merged 2 commits into from
Jul 15, 2024
Merged

Conversation

haarg
Copy link
Member

@haarg haarg commented Jul 15, 2024

tr/// always transforms character classes. Trying to put a regex character class wrapped in brackets will result in the brackets getting transformed, and other characters getting converted incorrectly.

Fix the tr/// used to generate a digest for an ID. This is only used for the contributors data, which is generally only updated when new releases are indexed. When querying the data, the author and release names are used, not the ID. The old records will continue to be found. If the data is reindexed, an additional record may be created for a release, but it should have the same data.

After making this change, eventually all of the contributor data should be recalculated to make sure the IDs are correct, and the old records purged. But they shouldn't interfere with anything until that is done.

tr/// always transforms character classes. Trying to put a regex
character class wrapped in brackets will result in the brackets getting
transformed, and other characters getting converted incorrectly.

Fix the tr/// used to generate a digest for an ID. This is only used for
the contributors data, which is generally only updated when new releases
are indexed. When querying the data, the author and release names are
used, not the ID. The old records will continue to be found. If the
data is reindexed, an additional record may be created for a release,
but it should have the same data.

After making this change, eventually all of the contributor data should
be recalculated to make sure the IDs are correct, and the old records
purged. But they shouldn't interfere with anything until that is done.
Copy link

codecov bot commented Jul 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.88%. Comparing base (b08ff6f) to head (c3c8e15).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1236      +/-   ##
==========================================
- Coverage   60.90%   60.88%   -0.03%     
==========================================
  Files         163      163              
  Lines        4441     4438       -3     
  Branches      646      646              
==========================================
- Hits         2705     2702       -3     
  Misses       1504     1504              
  Partials      232      232              
Files Coverage Δ
lib/MetaCPAN/Server/Controller/CVE.pm 50.00% <ø> (-3.34%) ⬇️
lib/MetaCPAN/Server/Controller/Contributor.pm 80.00% <ø> (-1.82%) ⬇️
lib/MetaCPAN/Server/Controller/Cover.pm 100.00% <ø> (ø)
lib/MetaCPAN/Util.pm 81.55% <100.00%> (ø)

@mickeyn mickeyn merged commit d3fc66b into master Jul 15, 2024
4 checks passed
@mickeyn mickeyn deleted the haarg/fix-digest-calc branch July 15, 2024 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants