Refactor NuGetEntityTypeSerializer + unit test coverage (#3879)

src/NuGetGallery/NuGetGallery.csproj

@@ -762,6 +762,10 @@
     </Compile>
     <Compile Include="Security\SecurePushSubscription.cs" />
     <Compile Include="Security\IUserSecurityPolicySubscription.cs" />
+    <Compile Include="OData\Serializers\FeedPackageAnnotationStrategy.cs" />
+    <Compile Include="OData\Serializers\IFeedPackageAnnotationStrategy.cs" />
+    <Compile Include="OData\Serializers\V1FeedPackageAnnotationStrategy.cs" />
+    <Compile Include="OData\Serializers\V2FeedPackageAnnotationStrategy.cs" />
     <Compile Include="Security\UserSecurityPolicyHandler.cs" />
     <Compile Include="Security\ISecurityPolicyService.cs" />
     <Compile Include="Security\RequirePackageVerifyScopePolicy.cs" />

src/NuGetGallery/OData/Serializers/FeedPackageAnnotationStrategy.cs

@@ -0,0 +1,57 @@
+// 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.
+
+using Microsoft.Data.OData;
+using System;
+using System.Net.Http;
+using System.Web.Http.Routing;
+
+namespace NuGetGallery.OData.Serializers
+{
+    internal abstract class FeedPackageAnnotationStrategy<TFeedPackage>
+        : IFeedPackageAnnotationStrategy
+    {
+        private readonly string _contentType;
+
+        protected FeedPackageAnnotationStrategy(string contentType)
+        {
+            if (string.IsNullOrEmpty(contentType))
+            {
+                throw new ArgumentException(nameof(contentType));
+            }
+
+            _contentType = contentType;
+        }
+
+        protected string ContentType => _contentType;
+
+        public bool CanAnnotate(object entityInstance)
+        {
+            return entityInstance != null && entityInstance is TFeedPackage;
+        }
+
+        public abstract void Annotate(HttpRequestMessage request, ODataEntry entry, object entityInstance);
+
+        protected static Uri BuildLinkForStreamProperty(string routePrefix, string id, string version, HttpRequestMessage request)
+        {
+            var url = new UrlHelper(request);
+            var result = url.Route(routePrefix + RouteName.DownloadPackage, new { id, version });
+
+            var builder = new UriBuilder(request.RequestUri);
+            builder.Path = version == null ? EnsureTrailingSlash(result) : result;
+            builder.Query = string.Empty;
+
+            return builder.Uri;
+        }
+
+        private static string EnsureTrailingSlash(string url)
+        {
+            if (url != null && !url.EndsWith("/", StringComparison.OrdinalIgnoreCase))
+            {
+                return url + '/';
+            }
+
+            return url;
+        }
+    }
+}

src/NuGetGallery/OData/Serializers/IFeedPackageAnnotationStrategy.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.
+
+using Microsoft.Data.OData;
+using System.Net.Http;
+
+namespace NuGetGallery.OData.Serializers
+{
+    internal interface IFeedPackageAnnotationStrategy
+    {
+        bool CanAnnotate(object entityInstance);
+        void Annotate(HttpRequestMessage request, ODataEntry entry, object entityInstance);
+    }
+}

src/NuGetGallery/OData/Serializers/NuGetEntityTypeSerializer.cs

@@ -1,153 +1,48 @@
 // 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.
 
-using System;
-using System.Net.Http;
+using Microsoft.Data.OData;
+using System.Collections.Generic;
 using System.Web.Http.OData;
 using System.Web.Http.OData.Formatter.Serialization;
-using System.Web.Http.Routing;
-using Microsoft.Data.OData;
-using Microsoft.Data.OData.Atom;
 
 namespace NuGetGallery.OData.Serializers
 {
     public class NuGetEntityTypeSerializer
         : ODataEntityTypeSerializer
     {
         private readonly string _contentType;
+        private readonly IReadOnlyCollection<IFeedPackageAnnotationStrategy> _annotationStrategies;
 
         public NuGetEntityTypeSerializer(ODataSerializerProvider serializerProvider)
             : base(serializerProvider)
         {
             _contentType = "application/zip";
+            _annotationStrategies = new List<IFeedPackageAnnotationStrategy>
+            {
+                new V1FeedPackageAnnotationStrategy(_contentType),
+                new V2FeedPackageAnnotationStrategy(_contentType)
+            };
         }
 
         public override ODataEntry CreateEntry(SelectExpandNode selectExpandNode, EntityInstanceContext entityInstanceContext)
         {
             var entry = base.CreateEntry(selectExpandNode, entityInstanceContext);
 
-            TryAnnotateV1FeedPackage(entry, entityInstanceContext);
-            TryAnnotateV2FeedPackage(entry, entityInstanceContext);
-
-            return entry;
-        }
-
-        private void TryAnnotateV1FeedPackage(ODataEntry entry, EntityInstanceContext entityInstanceContext)
-        {
-            var instance = entityInstanceContext.EntityInstance as V1FeedPackage;
-            if (instance != null)
-            {
-                // Set Atom entry metadata
-                var atomEntryMetadata = new AtomEntryMetadata();
-                atomEntryMetadata.Title = instance.Title;
-                if (!string.IsNullOrEmpty(instance.Authors))
-                {
-                    atomEntryMetadata.Authors = new[] { new AtomPersonMetadata { Name = instance.Authors } };
-                }
-                if (instance.LastUpdated > DateTime.MinValue)
-                {
-                    atomEntryMetadata.Updated = instance.LastUpdated;
-                }
-                if (!string.IsNullOrEmpty(instance.Summary))
-                {
-                    atomEntryMetadata.Summary = instance.Summary;
-                }
-                entry.SetAnnotation(atomEntryMetadata);
-
-                // Add package download link
-                entry.MediaResource = new ODataStreamReferenceValue
-                {
-                    ContentType = ContentType,
-                    ReadLink = BuildLinkForStreamProperty("v1", instance.Id, instance.Version, entityInstanceContext.Request)
-                };
-            }
-        }
-
-        private void TryAnnotateV2FeedPackage(ODataEntry entry, EntityInstanceContext entityInstanceContext)
-        {
-            var instance = entityInstanceContext.EntityInstance as V2FeedPackage;
-            if (instance != null)
+            foreach (var annotationStrategy in _annotationStrategies)
             {
-                // Patch links to use normalized versions
-                var normalizedVersion = NuGetVersionNormalizer.Normalize(instance.Version);
-                NormalizeNavigationLinks(entry, entityInstanceContext.Request, instance, normalizedVersion);
-
-                // Set Atom entry metadata
-                var atomEntryMetadata = new AtomEntryMetadata();
-                atomEntryMetadata.Title = instance.Id;
-                if (!string.IsNullOrEmpty(instance.Authors))
-                {
-                    atomEntryMetadata.Authors = new[] { new AtomPersonMetadata { Name = instance.Authors } };
-                }
-                if (instance.LastUpdated > DateTime.MinValue)
+                if (annotationStrategy.CanAnnotate(entityInstanceContext.EntityInstance))
                 {
-                    atomEntryMetadata.Updated = instance.LastUpdated;
+                    annotationStrategy.Annotate(entityInstanceContext.Request, entry, entityInstanceContext.EntityInstance);
                 }
-                if (!string.IsNullOrEmpty(instance.Summary))
-                {
-                    atomEntryMetadata.Summary = instance.Summary;
-                }
-                entry.SetAnnotation(atomEntryMetadata);
-
-                // Add package download link
-                entry.MediaResource = new ODataStreamReferenceValue
-                {
-                    ContentType = ContentType,
-                    ReadLink = BuildLinkForStreamProperty("v2", instance.Id, normalizedVersion, entityInstanceContext.Request)
-                };
-            }
-        }
-
-        private static void NormalizeNavigationLinks(ODataEntry entry, HttpRequestMessage request, V2FeedPackage instance, string normalizedVersion)
-        {
-            var idLink = BuildIdLink(instance.Id, normalizedVersion, request);
-
-            if (entry.ReadLink != null)
-            {
-                entry.ReadLink = idLink;
-            }
-
-            if (entry.EditLink != null)
-            {
-                entry.EditLink = idLink;
-            }
-
-            if (entry.Id != null)
-            {
-                entry.Id = idLink.ToString();
             }
+
+            return entry;
         }
 
         public string ContentType
         {
             get { return _contentType; }
         }
-
-        private static Uri BuildLinkForStreamProperty(string routePrefix, string id, string version, HttpRequestMessage request)
-        {
-            var url = new UrlHelper(request);
-            var result = url.Route(routePrefix + RouteName.DownloadPackage, new { id, version });
-
-            var builder = new UriBuilder(request.RequestUri);
-            builder.Path = version == null ? EnsureTrailingSlash(result) : result;
-            builder.Query = string.Empty;
-
-            return builder.Uri;
-        }
-
-        private static Uri BuildIdLink(string id, string version, HttpRequestMessage request)
-        {
-            return new Uri($"{request.RequestUri.Scheme}://{request.RequestUri.Host}{request.RequestUri.LocalPath}(Id='{id}',Version='{version}')");
-        }
-
-        private static string EnsureTrailingSlash(string url)
-        {
-            if (url != null && !url.EndsWith("/", StringComparison.OrdinalIgnoreCase))
-            {
-                return url + '/';
-            }
-
-            return url;
-        }
     }
 }

src/NuGetGallery/OData/Serializers/V1FeedPackageAnnotationStrategy.cs

@@ -0,0 +1,56 @@
+// 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.
+
+using Microsoft.Data.OData;
+using Microsoft.Data.OData.Atom;
+using System;
+using System.Net.Http;
+
+namespace NuGetGallery.OData.Serializers
+{
+    internal class V1FeedPackageAnnotationStrategy
+        : FeedPackageAnnotationStrategy<V1FeedPackage>
+    {
+        public V1FeedPackageAnnotationStrategy(string contentType)
+            : base(contentType)
+        {
+        }
+
+        public override void Annotate(HttpRequestMessage request, ODataEntry entry, object entityInstance)
+        {
+            var feedPackage = entityInstance as V1FeedPackage;
+            if (feedPackage == null)
+            {
+                return;
+            }
+
+            // Set Atom entry metadata
+            var atomEntryMetadata = new AtomEntryMetadata();
+            atomEntryMetadata.Title = feedPackage.Title;
+
+            if (!string.IsNullOrEmpty(feedPackage.Authors))
+            {
+                atomEntryMetadata.Authors = new[] { new AtomPersonMetadata { Name = feedPackage.Authors } };
+            }
+
+            if (feedPackage.LastUpdated > DateTime.MinValue)
+            {
+                atomEntryMetadata.Updated = feedPackage.LastUpdated;
+            }
+
+            if (!string.IsNullOrEmpty(feedPackage.Summary))
+            {
+                atomEntryMetadata.Summary = feedPackage.Summary;
+            }
+
+            entry.SetAnnotation(atomEntryMetadata);
+
+            // Add package download link
+            entry.MediaResource = new ODataStreamReferenceValue
+            {
+                ContentType = ContentType,
+                ReadLink = BuildLinkForStreamProperty("v1", feedPackage.Id, feedPackage.Version, request)
+            };
+        }
+    }
+}