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

V2/Server: Prevent NRE when NormalizedVersion is missing #1503

Conversation

sean-r-williams
Copy link
Contributor

@sean-r-williams sean-r-williams commented Jan 3, 2024

PR Summary

This PR conditionalizes adding NormalizedVersion to the additionalMetadata property on V2 server responses. This should resolve #1486.

PR Context

Services like Artifactory do not present a NormalizedVersion property in their metadata when querying via the NuGet v2 API. This causes PSResourceInfo.TryConvertFromXml() to NRE.

The change in this PR skips trying to add this property to AdditionalMetadata if the server did not present a NormalizedVersion property.

This naturally means AdditionalMetadata is not guaranteed to include this field on responses from v2 Server feeds, but I don't see any documentation indicating this is guaranteed.

PR Checklist

@sean-r-williams
Copy link
Contributor Author

sean-r-williams commented Jan 3, 2024

I tried running tests locally - it seems the docs are a bit out of date (see #1486 (comment)) so I made a best-effort approach (h/t to jborean on the PowerShell Discord for the suggestion of using test.yml)

Running Invoke-ModuleTests -Type Functional seems to work, with the exception of GitHub Packages. These tests appear to depend on PATs from AzDO, so I'm not sure if there's a good way to run them locally.

@alerickson
Copy link
Member

@sean-r-williams unfortunately easily testing against GitHub packages can only really be done with CI. But your PR looks good so I'll merge in. Thanks for opening the related issue and a double thanks for the PR to fix it!

@alerickson alerickson merged commit cf60367 into PowerShell:master Jan 4, 2024
3 checks passed
@sean-r-williams
Copy link
Contributor Author

Thanks @alerickson! Is there any ETA from your team on when 1.0.2 is expected to be released?

This is blocking some of our internal development workflows (we can't use PSResourceGet 1.0.1 because of this bug, and PowerShellGet only works for module packages in Artifactory - not script packages). I want to make sure we adjust our delivery timelines accordingly so we don't churn before 1.0.2 is available.

@sean-r-williams sean-r-williams deleted the sean-r-williams/1486_null-normalizedversion branch January 4, 2024 01:47
alerickson pushed a commit to alerickson/PSResourceGet that referenced this pull request Apr 1, 2024
…ell#1615)

Add Name and Repository pipeline by property name (PowerShell#1451)

Bugfix script parse whitespace (PowerShell#1457)

Bug fix for Import-PSGetRepository in Windows PS (PowerShell#1460)

Update README.md (PowerShell#1458)

update chANGELOG, psd1, csproj for release 1.0.1 (PowerShell#1473)

Bump System.Text.Json from 6.0.0 to 8.0.0 in /src/code (PowerShell#1475)

Verify whether SourceLocation is a UNC path and select the appropriate ApiVersion (PowerShell#1479)

TryConvertFromXml: Prevent NRE when NormalizedVersion is missing (PowerShell#1503)

Update InstallHelper.cs (PowerShell#1510)

Bump BenchmarkDotNet.Diagnostics.Windows in /test/perf/benchmarks (PowerShell#1528)

enable isJFrogRepo flag for domains containing `artifactory` (PowerShell#1532)

Fix 'name' bug with v2 JFrog Artifactory (PowerShell#1535)

Bugfix Update-ModuleManifest throws null pointer exception (PowerShell#1538)

Add tests for ADO v2 server (PowerShell#1539)

Bugfix - Test if InstalledScriptInfos folder exists and create if needed (PowerShell#1542)

* Test InstalledScriptInfos folder and create if needed

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

---------

Co-authored-by: Aditya Patwardhan <[email protected]>

Remove redeclaration of s_tempHome (PowerShell#1544)

Bug fix for Update-PSResource not updating from correct repository (PowerShell#1549)

Update changelog, version, releasenotes
alerickson added a commit to alerickson/PSResourceGet that referenced this pull request Apr 1, 2024
Add verbose and debug messages for Container Registry Server (PowerShell#1615)

Add Name and Repository pipeline by property name (PowerShell#1451)

Bugfix script parse whitespace (PowerShell#1457)

Bug fix for Import-PSGetRepository in Windows PS (PowerShell#1460)

Update README.md (PowerShell#1458)

update chANGELOG, psd1, csproj for release 1.0.1 (PowerShell#1473)

Bump System.Text.Json from 6.0.0 to 8.0.0 in /src/code (PowerShell#1475)

Verify whether SourceLocation is a UNC path and select the appropriate ApiVersion (PowerShell#1479)

TryConvertFromXml: Prevent NRE when NormalizedVersion is missing (PowerShell#1503)

Update InstallHelper.cs (PowerShell#1510)

Bump BenchmarkDotNet.Diagnostics.Windows in /test/perf/benchmarks (PowerShell#1528)

enable isJFrogRepo flag for domains containing `artifactory` (PowerShell#1532)

Fix 'name' bug with v2 JFrog Artifactory (PowerShell#1535)

Bugfix Update-ModuleManifest throws null pointer exception (PowerShell#1538)

Add tests for ADO v2 server (PowerShell#1539)

Bugfix - Test if InstalledScriptInfos folder exists and create if needed (PowerShell#1542)

* Test InstalledScriptInfos folder and create if needed

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

---------

Co-authored-by: Aditya Patwardhan <[email protected]>

Remove redeclaration of s_tempHome (PowerShell#1544)

Bug fix for Update-PSResource not updating from correct repository (PowerShell#1549)

Update changelog, version, releasenotes
alerickson added a commit to alerickson/PSResourceGet that referenced this pull request Apr 1, 2024
…ncies' in PSResourceInfo object (PowerShell#1604)

Create OneBranch build and release pipeline (PowerShell#1605)

Add verbose and debug messages for Container Registry Server (PowerShell#1615)

Add Name and Repository pipeline by property name (PowerShell#1451)

Bugfix script parse whitespace (PowerShell#1457)

Bug fix for Import-PSGetRepository in Windows PS (PowerShell#1460)

Update README.md (PowerShell#1458)

update chANGELOG, psd1, csproj for release 1.0.1 (PowerShell#1473)

Bump System.Text.Json from 6.0.0 to 8.0.0 in /src/code (PowerShell#1475)

Verify whether SourceLocation is a UNC path and select the appropriate ApiVersion (PowerShell#1479)

TryConvertFromXml: Prevent NRE when NormalizedVersion is missing (PowerShell#1503)

Update InstallHelper.cs (PowerShell#1510)

Bump BenchmarkDotNet.Diagnostics.Windows in /test/perf/benchmarks (PowerShell#1528)

enable isJFrogRepo flag for domains containing `artifactory` (PowerShell#1532)

Fix 'name' bug with v2 JFrog Artifactory (PowerShell#1535)

Bugfix Update-ModuleManifest throws null pointer exception (PowerShell#1538)

Add tests for ADO v2 server (PowerShell#1539)

Bugfix - Test if InstalledScriptInfos folder exists and create if needed (PowerShell#1542)

* Test InstalledScriptInfos folder and create if needed

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

* Update src/code/InstallHelper.cs

Co-authored-by: Aditya Patwardhan <[email protected]>

---------

Co-authored-by: Aditya Patwardhan <[email protected]>

Remove redeclaration of s_tempHome (PowerShell#1544)

Bug fix for Update-PSResource not updating from correct repository (PowerShell#1549)

Update changelog, version, releasenotes
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.

NRE/FindNameConvertToPSResourceFailure in Find-PSResource on Artifactory+v2 feed
2 participants