Create OwnerDetailsUriTemplateResourceV3 and provider #5763
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| #nullable enable | ||
|
|
||
| using System; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using NuGet.Protocol.Core.Types; | ||
| using NuGet.Protocol.Resources; | ||
|
|
||
| namespace NuGet.Protocol.Providers | ||
| { | ||
| /// <summary>NuGet.Protocol resource provider for <see cref="OwnerDetailsUriTemplateResourceV3"/> in V3 HTTP feeds.</summary> | ||
| /// <remarks>When successful, returns an instance of <see cref="OwnerDetailsUriTemplateResourceV3"/>.</remarks> | ||
| public class OwnerDetailsUriResourceV3Provider : ResourceProvider | ||
| { | ||
| public OwnerDetailsUriResourceV3Provider() | ||
| : base(typeof(OwnerDetailsUriTemplateResourceV3), | ||
| nameof(OwnerDetailsUriTemplateResourceV3), | ||
| NuGetResourceProviderPositions.Last) | ||
| { | ||
| } | ||
|
|
||
| /// <inheritdoc cref="ResourceProvider.TryCreate(SourceRepository, CancellationToken)"/> | ||
| public override async Task<Tuple<bool, INuGetResource?>> TryCreate(SourceRepository source, CancellationToken token) | ||
| { | ||
| OwnerDetailsUriTemplateResourceV3? resource = null; | ||
| ServiceIndexResourceV3? serviceIndex = await source.GetResourceAsync<ServiceIndexResourceV3>(token); | ||
| if (serviceIndex != null) | ||
| { | ||
| Uri uriTemplate = serviceIndex.GetServiceEntryUri(ServiceTypes.OwnerDetailsUriTemplate); | ||
| resource = OwnerDetailsUriTemplateResourceV3.CreateOrNull(uriTemplate); | ||
| } | ||
|
|
||
| return new Tuple<bool, INuGetResource?>(resource != null, resource); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| #nullable enable | ||
|
|
||
| using System; | ||
| using NuGet.Protocol.Core.Types; | ||
|
|
||
| namespace NuGet.Protocol.Resources | ||
| { | ||
| /// <summary>Owner Details Uri Template for NuGet V3 HTTP feeds.</summary> | ||
| /// <remarks>Not intended to be created directly. Use <see cref="SourceRepository.GetResourceAsync{T}(CancellationToken)"/> | ||
| /// with <see cref="OwnerDetailsUriTemplateResourceV3"/> for T, and typecast to this class. | ||
| public class OwnerDetailsUriTemplateResourceV3 : INuGetResource | ||
| { | ||
| private readonly string _template; | ||
|
|
||
| private OwnerDetailsUriTemplateResourceV3(string template) | ||
| { | ||
| _template = template ?? throw new ArgumentNullException(nameof(template)); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Creates the specified Owner Details Uri template provided by the server if it exists and is valid. | ||
| /// </summary> | ||
| /// <param name="uriTemplate">The Absolute Uri template provided by the server.</param> | ||
| /// <returns>A valid Owner Details Uri template, or null.</returns> | ||
| public static OwnerDetailsUriTemplateResourceV3? CreateOrNull(Uri? uriTemplate) | ||
|
donnie-msft marked this conversation as resolved.
Outdated
|
||
| { | ||
| if (uriTemplate == null || uriTemplate.OriginalString.Length == 0 || !uriTemplate.IsAbsoluteUri) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| string absoluteUri = uriTemplate.OriginalString; | ||
| if (string.IsNullOrWhiteSpace(absoluteUri) | ||
| || !IsValidUriTemplate(absoluteUri)) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| return new OwnerDetailsUriTemplateResourceV3(absoluteUri); | ||
| } | ||
|
|
||
| private static bool IsValidUriTemplate(string absoluteUri) | ||
|
Member
There was a problem hiding this comment. You have a Uri type in the method calling this, so you shouldn't need to pass a string. I'd just all the validations in 1 method and use the Uri type that's already there.
Contributor
Author
There was a problem hiding this comment. Hm, I was trying not to invent any new validation strategy, as mentioned in #5763 (comment), PackageDetailsUriResourceV3 does this custom validation, and it's why I copied that for the I'll see if I can reduce this validation.
Contributor
Author
There was a problem hiding this comment. Ok, I don't like this any better than what was there, but now I'm throwing if a null is provided, and now must check for null in the Provider. We don't want providers to throw when they can't find the resource, because it's not required for a package source to implement this resource. |
||
| { | ||
| Uri? uri; | ||
| var isValidUri = Uri.TryCreate(absoluteUri, UriKind.Absolute, out uri); | ||
|
Contributor
There was a problem hiding this comment. We are converting the URI parameter value to a string in the CreateOrNull method, which then calls the IsValidUriTemplate method, passing in that string. I am not sure why we are converting that string again to verify if it is a valid URI. How about just passing the URI object that was passed to the CreateOrNull method instead of creating another URI object here? Security considerations section in docs suggest that, You can check a URI string for validity by calling the Uri.IsWellFormedOriginalString method. Can this method be used in this case?
Contributor
Author
There was a problem hiding this comment. I noticed that method as well, but unfortunately, it returns My suspicion is this is why the PackageDetailsUriResourceV3 does this custom validation, and it's why I copied that for the If you have other suggestions, let me know. I'm a little hesitant to modify the
Contributor
There was a problem hiding this comment.
Contributor
Author
There was a problem hiding this comment. I've reduced some of the validation logic there, have a look and see if that addresses your concern. |
||
|
|
||
| // Only allow HTTPS owner details URLs. | ||
| if (isValidUri && uri?.Scheme.Equals("https", StringComparison.OrdinalIgnoreCase) != true) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| return isValidUri; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets a URL for viewing package Owner URL outside of Visual Studio. The URL will not be verified to exist. | ||
| /// </summary> | ||
| /// <param name="owner">The owner username.</param> | ||
| /// <returns>The first URL from the resource, with the URI template applied.</returns> | ||
| public Uri GetUri(string owner) | ||
|
donnie-msft marked this conversation as resolved.
|
||
| { | ||
| var uriString = _template | ||
| #if NETCOREAPP | ||
| .Replace("{owner}", owner, StringComparison.OrdinalIgnoreCase); | ||
|
kartheekp-ms marked this conversation as resolved.
|
||
| #else | ||
| .Replace("{owner}", owner); | ||
| #endif | ||
|
|
||
| return new Uri(uriString); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| #nullable enable | ||
|
|
||
| using System; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using FluentAssertions; | ||
| using NuGet.Configuration; | ||
| using NuGet.Packaging; | ||
| using NuGet.Protocol.Core.Types; | ||
| using NuGet.Protocol.Providers; | ||
| using Xunit; | ||
|
|
||
| namespace NuGet.Protocol.Tests.Providers | ||
| { | ||
| public class OwnerDetailsUriResourceV3ProviderTests | ||
| { | ||
| [Fact] | ||
| public async Task TryCreate_NoResourceInServiceIndex_ReturnsFalseAsync() | ||
| { | ||
| // Arrange | ||
| var serviceIndexProvider = MockServiceIndexResourceV3Provider.Create(); | ||
| var target = new OwnerDetailsUriResourceV3Provider(); | ||
| var providers = new INuGetResourceProvider[] { serviceIndexProvider, target }; | ||
|
|
||
| PackageSource packageSource = new PackageSource("https://nuget.test/v3/index.json"); | ||
| SourceRepository sourceRepository = new SourceRepository(packageSource, providers); | ||
|
|
||
| // Act | ||
| Tuple<bool, INuGetResource?> result = await target.TryCreate(sourceRepository, CancellationToken.None); | ||
|
|
||
| // Assert | ||
| bool providerHandlesInputSource = result.Item1; | ||
| INuGetResource? resource = result.Item2; | ||
|
|
||
| providerHandlesInputSource.Should().BeFalse(); | ||
| resource.Should().BeNull(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public async Task TryCreate_ResourceInServiceIndex_ReturnsTrueAsync() | ||
| { | ||
| // Arrange | ||
| var ownerDetailsResourceEntry = new ServiceIndexEntry(new Uri("https://nuget.test/profiles/{owner}?_src=template"), ServiceTypes.OwnerDetailsUriTemplate[0], MinClientVersionUtility.GetNuGetClientVersion()); | ||
| var serviceIndexProvider = MockServiceIndexResourceV3Provider.Create(); | ||
| var target = new OwnerDetailsUriResourceV3Provider(); | ||
| var providers = new INuGetResourceProvider[] { serviceIndexProvider, target }; | ||
|
|
||
| PackageSource packageSource = new PackageSource("https://nuget.test/v3/index.json"); | ||
| SourceRepository sourceRepository = new SourceRepository(packageSource, providers); | ||
|
|
||
| // Act | ||
| Tuple<bool, INuGetResource?> result = await target.TryCreate(sourceRepository, CancellationToken.None); | ||
|
|
||
| // Assert | ||
| bool providerHandlesInputSource = result.Item1; | ||
| INuGetResource? resource = result.Item2; | ||
|
|
||
| providerHandlesInputSource.Should().BeFalse(); | ||
| resource.Should().BeNull(); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| // Copyright (c) .NET Foundation. All rights reserved. | ||
| // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. | ||
|
|
||
| #nullable enable | ||
|
|
||
| using System; | ||
| using FluentAssertions; | ||
| using NuGet.Protocol.Resources; | ||
| using Xunit; | ||
|
|
||
| namespace NuGet.Protocol.Tests.Resources | ||
| { | ||
| public class OwnerDetailsUriTemplateResourceV3Tests | ||
| { | ||
| private readonly Uri _template = new Uri("https://nuget.test/profiles/{owner}?_src=template"); | ||
|
donnie-msft marked this conversation as resolved.
|
||
|
|
||
| [Fact] | ||
| public void CreateOrNull_WhenNullTemplate_CreatesNullResource() | ||
| { | ||
| // Arrange | ||
| Uri? template = null; | ||
|
|
||
| // Act | ||
| var target = OwnerDetailsUriTemplateResourceV3.CreateOrNull(template); | ||
|
|
||
| // Assert | ||
| target.Should().BeNull(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void CreateOrNull_WhenTemplateNotAbsoluteUri_CreatesNullResource() | ||
| { | ||
| // Arrange | ||
| var template = new Uri("/owner/profile", UriKind.Relative); | ||
|
|
||
| // Act | ||
| var target = OwnerDetailsUriTemplateResourceV3.CreateOrNull(template); | ||
|
|
||
| // Assert | ||
| target.Should().BeNull(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void CreateOrNull_WhenTemplateNotHttps_CreatesNullResource() | ||
| { | ||
| // Arrange | ||
| var template = new Uri("http://nuget.test/profiles/{owner}?_src=template"); | ||
|
donnie-msft marked this conversation as resolved.
|
||
|
|
||
| // Act | ||
| var target = OwnerDetailsUriTemplateResourceV3.CreateOrNull(template); | ||
|
|
||
| // Assert | ||
| target.Should().BeNull(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void CreateOrNull_WhenValidTemplateHttps_CreatesResource() | ||
| { | ||
| // Arrange & Act | ||
| var target = OwnerDetailsUriTemplateResourceV3.CreateOrNull(_template); | ||
|
|
||
| // Assert | ||
| target.Should().NotBeNull(); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void GetUri_WithSpacesInOwnerParameter_CreatesValidOwnerUriWithEncoding() | ||
| { | ||
| // Arrange | ||
| string owner = "Microsoft Microsoft Microsoft"; | ||
| string formattedOwner = "Microsoft%20Microsoft%20Microsoft"; | ||
|
|
||
| var target = OwnerDetailsUriTemplateResourceV3.CreateOrNull(_template); | ||
|
|
||
| // Act | ||
| Uri ownerUri = target!.GetUri(owner); | ||
|
|
||
| // Assert | ||
| ownerUri.Should().NotBeNull(); | ||
| ownerUri.IsAbsoluteUri.Should().BeTrue(); | ||
| ownerUri.AbsoluteUri.Should().Be($"https://nuget.test/profiles/{formattedOwner}?_src=template"); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData("microsoft")] | ||
| [InlineData("MiCroSoFT")] | ||
| public void GetUri_WithValidOwnerParameter_CreatesValidOwnerUriWithSameCasing(string owner) | ||
| { | ||
| // Arrange | ||
| var target = OwnerDetailsUriTemplateResourceV3.CreateOrNull(_template); | ||
|
|
||
| // Act | ||
| Uri ownerUri = target!.GetUri(owner); | ||
|
|
||
| // Assert | ||
| ownerUri.Should().NotBeNull(); | ||
| ownerUri.IsAbsoluteUri.Should().BeTrue(); | ||
| ownerUri.AbsoluteUri.Should().Be($"https://nuget.test/profiles/{owner}?_src=template"); | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(null)] | ||
| [InlineData("")] | ||
| public void GetUri_WithInvalidOwnerParameter_ReturnsOriginalTemplate(string owner) | ||
| { | ||
| // Arrange | ||
| var target = OwnerDetailsUriTemplateResourceV3.CreateOrNull(_template); | ||
| string templateWithoutOwner = "https://nuget.test/profiles/?_src=template"; | ||
|
|
||
| // Act | ||
| Uri ownerUri = target!.GetUri(owner); | ||
|
|
||
| // Assert | ||
| ownerUri.Should().NotBeNull(); | ||
| ownerUri.IsAbsoluteUri.Should().BeTrue(); | ||
| ownerUri.AbsoluteUri.Should().Be(templateWithoutOwner); | ||
| } | ||
| } | ||
| } | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.