License expression display in Gallery (#6784)

* Generating markup for license expression display.

* Test for controller to call license expression splitter when license expression is present.

* More comments on why do we need helpers in DisplayPackage.cshtml

Author: agr

src/NuGetGallery/App_Start/DefaultDependenciesModule.cs

@@ -22,6 +22,7 @@
 using Microsoft.WindowsAzure.ServiceRuntime;
 using NuGet.Services.Entities;
 using NuGet.Services.KeyVault;
+using NuGet.Services.Licenses;
 using NuGet.Services.Logging;
 using NuGet.Services.Messaging;
 using NuGet.Services.Messaging.Email;
@@ -345,6 +346,18 @@ protected override void Load(ContainerBuilder builder)
                 .As<IContentFileMetadataService>()
                 .InstancePerLifetimeScope();
 
+            builder.RegisterType<LicenseExpressionSplitter>()
+                .As<ILicenseExpressionSplitter>()
+                .InstancePerLifetimeScope();
+
+            builder.RegisterType<LicenseExpressionParser>()
+                .As<ILicenseExpressionParser>()
+                .InstancePerLifetimeScope();
+
+            builder.RegisterType<LicenseExpressionSegmentator>()
+                .As<ILicenseExpressionSegmentator>()
+                .InstancePerLifetimeScope();
+
             RegisterMessagingService(builder, configuration);
 
             builder.Register(c => HttpContext.Current.User)

src/NuGetGallery/Controllers/PackagesController.cs

@@ -16,6 +16,7 @@
 using System.Web.Mvc;
 using NuGet.Packaging;
 using NuGet.Services.Entities;
+using NuGet.Services.Licenses;
 using NuGet.Services.Messaging.Email;
 using NuGet.Versioning;
 using NuGetGallery.Areas.Admin;
@@ -96,6 +97,7 @@ public partial class PackagesController
         private readonly IDiagnosticsSource _trace;
         private readonly IFlatContainerService _flatContainerService;
         private readonly ICoreLicenseFileService _coreLicenseFileService;
+        private readonly ILicenseExpressionSplitter _licenseExpressionSplitter;
 
         public PackagesController(
             IPackageService packageService,
@@ -122,7 +124,8 @@ public PackagesController(
             ISymbolPackageUploadService symbolPackageUploadService,
             IDiagnosticsService diagnosticsService,
             IFlatContainerService flatContainerService,
-            ICoreLicenseFileService coreLicenseFileService)
+            ICoreLicenseFileService coreLicenseFileService,
+            ILicenseExpressionSplitter licenseExpressionSplitter)
         {
             _packageService = packageService;
             _uploadFileService = uploadFileService;
@@ -149,6 +152,7 @@ public PackagesController(
             _trace = diagnosticsService?.SafeGetSource(nameof(PackagesController)) ?? throw new ArgumentNullException(nameof(diagnosticsService));
             _flatContainerService = flatContainerService;
             _coreLicenseFileService = coreLicenseFileService ?? throw new ArgumentNullException(nameof(coreLicenseFileService));
+            _licenseExpressionSplitter = licenseExpressionSplitter ?? throw new ArgumentNullException(nameof(licenseExpressionSplitter));
         }
 
         [HttpGet]
@@ -624,6 +628,21 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
 
             model.ReadMeHtml = await _readMeService.GetReadMeHtmlAsync(package);
 
+            if (!string.IsNullOrWhiteSpace(package.LicenseExpression))
+            {
+                try
+                {
+                    model.LicenseExpressionSegments = _licenseExpressionSplitter.SplitExpression(package.LicenseExpression);
+                }
+                catch (Exception ex)
+                {
+                    // Any exception thrown while trying to render license expression beautifully
+                    // is not severe enough to break the client experience, view will fall back to
+                    // display license url.
+                    _telemetryService.TraceException(ex);
+                }
+            }
+
             var externalSearchService = _searchService as ExternalSearchService;
             if (_searchService.ContainsAllVersions && externalSearchService != null)
             {

src/NuGetGallery/NuGetGallery.csproj

@@ -2049,6 +2049,9 @@
     <PackageReference Include="Markdig.Signed">
       <Version>0.15.4</Version>
     </PackageReference>
+    <PackageReference Include="NuGet.Services.Licenses">
+      <Version>2.41.0-agr-licenses-2294033</Version>
+    </PackageReference>
     <PackageReference Include="NuGet.StrongName.AnglicanGeek.MarkdownMailer">
       <Version>1.2.0</Version>
     </PackageReference>

src/NuGetGallery/ViewModels/DisplayPackageViewModel.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using NuGet.Services.Entities;
+using NuGet.Services.Licenses;
 using NuGet.Services.Validation.Issues;
 using NuGet.Versioning;
 
@@ -130,6 +131,7 @@ public bool HasNewerRelease
         public string LicenseUrl { get; set; }
         public IEnumerable<string> LicenseNames { get; set; }
         public string LicenseExpression { get; set; }
+        public IReadOnlyCollection<CompositeLicenseExpressionSegment> LicenseExpressionSegments { get; set; }
         public EmbeddedLicenseFileType EmbeddedLicenseType { get; set; }
 
         private IDictionary<User, string> _pushedByCache = new Dictionary<User, string>();

src/NuGetGallery/Views/Packages/DisplayPackage.cshtml

@@ -1,4 +1,5 @@
-@using NuGet.Services.Validation;
+@using NuGet.Services.Validation
+@using NuGet.Services.Licenses
 
 @model DisplayPackageViewModel
 @{
@@ -69,6 +70,14 @@
     }
 }
 
+@functions
+{
+    static bool IsLicenseOrException(CompositeLicenseExpressionSegment segment)
+    {
+        return segment.Type == CompositeLicenseExpressionSegmentType.LicenseIdentifier || segment.Type == CompositeLicenseExpressionSegmentType.ExceptionIdentifier;
+    }
+}
+
 @section SocialMeta {
     @if (!String.IsNullOrWhiteSpace(ViewBag.FacebookAppID))
     {
@@ -146,6 +155,11 @@
     </div>
 }
 
+@* The following two helpers must be on a single line each so no extra whitespace is introduced in the expression when rendered. *@
+@* Helpers themselves are needed not to introduce that extra whitespce, which happens if they are inlined. *@
+@helper MakeLicenseLink(CompositeLicenseExpressionSegment segment) {<a href="@LicenseExpressionRedirectUrlHelper.GetLicenseExpressionRedirectUrl(segment.Value)">@segment.Value</a>}
+@helper MakeLicenseSpan(CompositeLicenseExpressionSegment segment) {<span>@segment.Value</span>}
+
 <section role="main" class="container main-container page-package-details">
     <div class="row">
         <aside aria-label="Package icon" class="col-sm-1">
@@ -638,8 +652,25 @@
                 {
                     if (Model.EmbeddedLicenseType == EmbeddedLicenseFileType.Absent || !Model.Validating)
                     {
-                        <li>
-                            <i class="ms-Icon ms-Icon--Certificate" aria-hidden="true"></i>
+                    <li>
+                        <i class="ms-Icon ms-Icon--Certificate" aria-hidden="true"></i>
+                        @if (Model.LicenseExpressionSegments.AnySafe())
+                        {
+                            @:License:
+                            foreach (var segment in Model.LicenseExpressionSegments)
+                            {
+                                if (IsLicenseOrException(segment))
+                                {
+                                    @MakeLicenseLink(segment);
+                                }
+                                else
+                                {
+                                    @MakeLicenseSpan(segment);
+                                }
+                            }
+                        }
+                        else
+                        {
                             <a href="@Model.LicenseUrl"
                                data-track="outbound-license-url" title="Make sure you agree with the license" rel="nofollow">
                                 @if (Model.LicenseNames.AnySafe())
@@ -651,7 +682,8 @@
                                     @:License Info
                                 }
                             </a>
-                        </li>
+                        }
+                    </li>
                     }
                 }
                 <li>

tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs

@@ -16,6 +16,7 @@
 using Moq;
 using NuGet.Packaging;
 using NuGet.Services.Entities;
+using NuGet.Services.Licenses;
 using NuGet.Services.Messaging.Email;
 using NuGet.Services.Validation;
 using NuGet.Services.Validation.Issues;
@@ -67,7 +68,8 @@ private static PackagesController CreateController(
             Mock<IContentObjectService> contentObjectService = null,
             Mock<ISymbolPackageUploadService> symbolPackageUploadService = null,
             Mock<IFlatContainerService> flatContainerService = null,
-            Mock<ICoreLicenseFileService> coreLicenseFileService = null)
+            Mock<ICoreLicenseFileService> coreLicenseFileService = null,
+            Mock<ILicenseExpressionSplitter> licenseExpressionSplitter = null)
         {
             packageService = packageService ?? new Mock<IPackageService>();
             if (uploadFileService == null)
@@ -185,6 +187,8 @@ private static PackagesController CreateController(
                     .ReturnsAsync(() => new MemoryStream());
             }
 
+            licenseExpressionSplitter = licenseExpressionSplitter ?? new Mock<ILicenseExpressionSplitter>();
+
             var diagnosticsService = new Mock<IDiagnosticsService>();
             var controller = new Mock<PackagesController>(
                 packageService.Object,
@@ -211,7 +215,8 @@ private static PackagesController CreateController(
                 symbolPackageUploadService.Object,
                 diagnosticsService.Object,
                 flatContainerService.Object,
-                coreLicenseFileService.Object);
+                coreLicenseFileService.Object,
+                licenseExpressionSplitter.Object);
 
             controller.CallBase = true;
             controller.Object.SetOwinContextOverride(Fakes.CreateOwinContext());
@@ -838,6 +843,58 @@ public async Task GetsValidationIssues()
                 Assert.Equal(model.PackageValidationIssues, expectedIssues);
             }
 
+            [Fact]
+            public async Task SplitsLicenseExpressionWhenProvided()
+            {
+                const string expression = "some expression";
+                var splitterMock = new Mock<ILicenseExpressionSplitter>();
+                var packageService = new Mock<IPackageService>();
+                var indexingService = new Mock<IIndexingService>();
+
+                var segments = new List<CompositeLicenseExpressionSegment>();
+                splitterMock
+                    .Setup(les => les.SplitExpression(expression))
+                    .Returns(segments);
+
+                var package = new Package()
+                {
+                    PackageRegistration = new PackageRegistration()
+                    {
+                        Id = "Foo",
+                        Owners = new List<User>()
+                    },
+                    Version = "01.1.01",
+                    NormalizedVersion = "1.1.1",
+                    Title = "A test package!",
+                    LicenseExpression = expression,
+                };
+
+                packageService.Setup(p => p.FindPackageByIdAndVersion(
+                                                It.Is<string>(s => s == "Foo"),
+                                                It.Is<string>(s => s == null),
+                                                It.Is<int>(i => i == SemVerLevelKey.SemVer2),
+                                                It.Is<bool>(b => b == true)))
+                    .Returns(package);
+
+                indexingService.Setup(i => i.GetLastWriteTime()).Returns(Task.FromResult((DateTime?)DateTime.UtcNow));
+
+                var controller = CreateController(
+                    GetConfigurationService(),
+                    packageService: packageService,
+                    indexingService: indexingService,
+                    licenseExpressionSplitter: splitterMock);
+
+                var result = await controller.DisplayPackage(id: "Foo", version: null);
+
+                splitterMock
+                    .Verify(les => les.SplitExpression(expression), Times.Once);
+                splitterMock
+                    .Verify(les => les.SplitExpression(It.IsAny<string>()), Times.Once);
+
+                var model = ResultAssert.IsView<DisplayPackageViewModel>(result);
+                Assert.Same(segments, model.LicenseExpressionSegments);
+            }
+
             private class TestIssue : ValidationIssue
             {
                 private readonly string _message;