IconUrl deprecation message. (#7556)

* IconUrl deprecation message.
Tests.

Author: agr

src/NuGetGallery/NuGetGallery.csproj

@@ -261,6 +261,7 @@
       <DependentUpon>201906262145129_EmbeddedIconFlag.cs</DependentUpon>
     </Compile>
     <Compile Include="Services\ConfigurationIconFileProvider.cs" />
+    <Compile Include="Services\IconUrlDeprecationValidationMessage.cs" />
     <Compile Include="Services\IconUrlTemplateProcessor.cs" />
     <Compile Include="Services\IIconUrlTemplateProcessor.cs" />
     <Compile Include="Services\IIconUrlProvider.cs" />

src/NuGetGallery/Services/IconUrlDeprecationValidationMessage.cs

@@ -0,0 +1,14 @@
+// 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.
+
+namespace NuGetGallery
+{
+    public class IconUrlDeprecationValidationMessage : IValidationMessage
+    {
+        public string PlainTextMessage => $"{Strings.UploadPackage_IconUrlDeprecated} https://aka.ms/deprecateIconUrl";
+
+        public bool HasRawHtmlRepresentation => true;
+
+        public string RawHtmlMessage => $"{Strings.UploadPackage_IconUrlDeprecated.Replace("<", "&lt;").Replace(">", "&gt;")} <a href=\"https://aka.ms/deprecateIconUrl\">{Strings.UploadPackage_LearnMore}</a>.";
+    }
+}

src/NuGetGallery/Services/PackageUploadService.cs

@@ -19,6 +19,7 @@
 using NuGetGallery.Diagnostics;
 using NuGetGallery.Helpers;
 using NuGetGallery.Packaging;
+using NuGetGallery.Services;
 
 namespace NuGetGallery
 {
@@ -148,7 +149,7 @@ public async Task<PackageValidationResult> ValidateBeforeGeneratePackageAsync(
                 return result;
             }
 
-            result = await CheckIconMetadataAsync(nuGetPackage, currentUser);
+            result = await CheckIconMetadataAsync(nuGetPackage, warnings, currentUser);
             if (result != null)
             {
                 //_telemetryService.TrackIconValidationFailure();
@@ -341,18 +342,22 @@ private async Task<PackageValidationResult> CheckLicenseMetadataAsync(PackageArc
             return null;
         }
 
-        private async Task<PackageValidationResult> CheckIconMetadataAsync(PackageArchiveReader nuGetPackage, User user)
+        private async Task<PackageValidationResult> CheckIconMetadataAsync(PackageArchiveReader nuGetPackage, List<IValidationMessage> warnings, User user)
         {
             var nuspecReader = GetNuspecReader(nuGetPackage);
-
             var iconElement = nuspecReader.IconElement;
+            var embeddedIconsEnabled = _featureFlagService.AreEmbeddedIconsEnabled(user);
 
             if (iconElement == null)
             {
+                if (embeddedIconsEnabled && nuspecReader.GetIconUrl() != null)
+                {
+                    warnings.Add(new IconUrlDeprecationValidationMessage());
+                }
                 return null;
             }
 
-            if (!_featureFlagService.AreEmbeddedIconsEnabled(user))
+            if (!embeddedIconsEnabled)
             {
                 return PackageValidationResult.Invalid(Strings.UploadPackage_EmbeddedIconNotAccepted);
             }

src/NuGetGallery/Strings.Designer.cs

@@ -1006,7 +1006,7 @@ The {1} Team</value>
     <value>Unknown error!</value>
   </data>
   <data name="UploadPackage_DeprecatingLicenseUrl" xml:space="preserve">
-    <value>&lt;licenseUrl&gt; element will be deprecated, please consider switching to specifying the license in the package.</value>
+    <value>The &lt;licenseUrl&gt; element is deprecated. Consider using the &lt;license&gt; element instead.</value>
   </data>
   <data name="UploadPackage_DeprecationUrlUsage" xml:space="preserve">
     <value>The license deprecation URL must be used in conjunction with specifying the license in the package.</value>
@@ -1150,4 +1150,7 @@ The {1} Team</value>
   <data name="SiteAdminNotLoggedInWithRequiredTenant" xml:space="preserve">
     <value>The site admins are required to sign in with the '{0}' tenant only.</value>
   </data>
+  <data name="UploadPackage_IconUrlDeprecated" xml:space="preserve">
+    <value>The &lt;iconUrl&gt; element is deprecated. Consider using the &lt;icon&gt; element instead.</value>
+  </data>
 </root>

src/NuGetGallery/Strings.resx

@@ -554,7 +554,7 @@ public async Task HandlesLegacyLicenseUrlPackageAccordingToSettings(bool blockLe
                     Assert.Equal(PackageValidationResultType.Accepted, result.Type);
                     Assert.Null(result.Message);
                     Assert.Single(result.Warnings);
-                    Assert.StartsWith("<licenseUrl> element will be deprecated, please consider switching to specifying the license in the package.", result.Warnings[0].PlainTextMessage);
+                    Assert.StartsWith("The <licenseUrl> element is deprecated. Consider using the <license> element instead.", result.Warnings[0].PlainTextMessage);
                     Assert.IsType<LicenseUrlDeprecationValidationMessage>(result.Warnings[0]);
                 }
             }
@@ -1141,6 +1141,53 @@ public async Task AcceptsPackagesWithEmbeddedIconForFlightedUsers()
                 Assert.Empty(result.Warnings);
             }
 
+            [Fact]
+            public async Task WarnsAboutPackagesWithIconUrlForFlightedUsers()
+            {
+                _nuGetPackage = GeneratePackageWithUserContent(
+                    iconUrl: new Uri("https://nuget.test/icon"),
+                    iconFilename: null,
+                    licenseExpression: "MIT",
+                    licenseUrl: new Uri("https://licenses.nuget.org/MIT"));
+                _featureFlagService
+                    .Setup(ffs => ffs.AreEmbeddedIconsEnabled(_currentUser))
+                    .Returns(true);
+
+                var result = await _target.ValidateBeforeGeneratePackageAsync(
+                    _nuGetPackage.Object,
+                    GetPackageMetadata(_nuGetPackage),
+                    _currentUser);
+
+                Assert.Equal(PackageValidationResultType.Accepted, result.Type);
+                Assert.Null(result.Message);
+                var warning = Assert.Single(result.Warnings);
+                Assert.IsType<IconUrlDeprecationValidationMessage>(warning);
+                Assert.StartsWith("The <iconUrl> element is deprecated. Consider using the <icon> element instead.", warning.PlainTextMessage);
+                Assert.StartsWith("The &lt;iconUrl&gt; element is deprecated. Consider using the &lt;icon&gt; element instead.", warning.RawHtmlMessage);
+            }
+
+            [Fact]
+            public async Task DoesntWarnAboutPackagesWithIconUrl()
+            {
+                _nuGetPackage = GeneratePackageWithUserContent(
+                    iconUrl: new Uri("https://nuget.test/icon"),
+                    iconFilename: null,
+                    licenseExpression: "MIT",
+                    licenseUrl: new Uri("https://licenses.nuget.org/MIT"));
+                _featureFlagService
+                    .Setup(ffs => ffs.AreEmbeddedIconsEnabled(_currentUser))
+                    .Returns(false);
+
+                var result = await _target.ValidateBeforeGeneratePackageAsync(
+                    _nuGetPackage.Object,
+                    GetPackageMetadata(_nuGetPackage),
+                    _currentUser);
+
+                Assert.Equal(PackageValidationResultType.Accepted, result.Type);
+                Assert.Null(result.Message);
+                Assert.Empty(result.Warnings);
+            }
+
             [Theory]
             [InlineData("<icon><something/></icon>")]
             [InlineData("<icon><something>icon.png</something></icon>")]
@@ -2218,6 +2265,7 @@ protected static Mock<TestPackageReader> GeneratePackageWithUserContent(
                 bool isSigned = true,
                 int? desiredTotalEntryCount = null,
                 Func<string> getCustomNuspecNodes = null,
+                Uri iconUrl = null,
                 Uri licenseUrl = null,
                 string licenseExpression = null,
                 string licenseFilename = null,
@@ -2232,6 +2280,7 @@ protected static Mock<TestPackageReader> GeneratePackageWithUserContent(
                     isSigned: isSigned,
                     desiredTotalEntryCount: desiredTotalEntryCount,
                     getCustomNuspecNodes: getCustomNuspecNodes,
+                    iconUrl: iconUrl,
                     licenseUrl: licenseUrl,
                     licenseExpression: licenseExpression,
                     licenseFilename: licenseFilename,
@@ -2249,6 +2298,7 @@ protected static MemoryStream GeneratePackageStream(
                 bool isSigned = true,
                 int? desiredTotalEntryCount = null,
                 Func<string> getCustomNuspecNodes = null,
+                Uri iconUrl = null,
                 Uri licenseUrl = null,
                 string licenseExpression = null,
                 string licenseFilename = null,
@@ -2265,6 +2315,7 @@ protected static MemoryStream GeneratePackageStream(
                     desiredTotalEntryCount: desiredTotalEntryCount,
                     getCustomNuspecNodes: getCustomNuspecNodes,
                     licenseUrl: licenseUrl,
+                    iconUrl: iconUrl,
                     licenseExpression: licenseExpression,
                     licenseFilename: licenseFilename,
                     licenseFileContents: GetBinaryLicenseFileContents(licenseFileBinaryContents, licenseFileContents),