Fix Npm version parsing for version with build numbers.#528
Fix Npm version parsing for version with build numbers.#528
Conversation
|
Could there be scenario where the build number contains letters or dashes (e.g. 2026-03-05-abc)? |
| public virtual JsonElement? GetVersionElement(JsonDocument contentJSON, string version) | ||
| { | ||
| string typeName = GetType().Name; | ||
| throw new NotImplementedException($"{typeName} does not implement GetVersions."); |
There was a problem hiding this comment.
I think this should be "does not implement GetVersionElement".
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public virtual JsonElement? GetVersionElement(JsonDocument contentJSON, string version) |
There was a problem hiding this comment.
Is this method defined twice?
There was a problem hiding this comment.
No, the new method accepts string versions; I kept the original one for Version types for backwards compatibility
| } | ||
|
|
||
| [Fact] | ||
| public void GetLatestVersionElement_WithNullJsonDocument_ReturnsNull() |
There was a problem hiding this comment.
Should we add another test for testing getting the latest version from the dist tag?
There was a problem hiding this comment.
"GetLatestVersionElement_WithValidJsonDocument_ReturnsLatestVersionElement" is to test that scenario
| JsonElement? versionElement = _projectManager.Object.GetVersionElement(contentJSON, "4.0.20260218200111"); | ||
|
|
||
| Assert.NotNull(versionElement); | ||
| Assert.Equal("4.0.20260218200111", versionElement?.GetProperty("version").GetString()); |
There was a problem hiding this comment.
4.0.20260218200111 is the version number? Do we also want to test the package name?
There was a problem hiding this comment.
Yes. [InlineData("pkg:npm/%40adguard/dnr-rulesets", "4.0.20260218200111")] This is the package in test
| PackageURL purl = new("pkg:npm/%40adguard/dnr-rulesets"); | ||
| string? content = await _projectManager.Object.GetMetadataAsync(purl, useCache: false); | ||
| JsonDocument contentJSON = JsonDocument.Parse(content); | ||
|
|
There was a problem hiding this comment.
Nit: Additional line space is not needed here.
|
I am not sure. Now that the we are accepting version as a string. The current logic should work. |
3 participants
Fix parsing for Npm packages with non-standard version numbers.
Retrieve the Npm package latest version from "dist-tag", or fallback to the most recent by time.
Example @adguard/dnr-rulesets@4.0.20260218200111