Add more validation for WithApiVersionSet on an endpoint

Author: commonsensesoftware

src/AspNetCore/WebApi/src/Asp.Versioning.Http/Builder/EndpointBuilderFinalizer.cs

@@ -120,18 +120,16 @@ private static bool ReportApiVersions( IList<object> metadata )
 
     private static ApiVersionSet? GetApiVersionSet( IList<object> metadata )
     {
-        var result = default( ApiVersionSet );
-
         for ( var i = metadata.Count - 1; i >= 0; i-- )
         {
             if ( metadata[i] is ApiVersionSet set )
             {
-                result ??= set;
                 metadata.RemoveAt( i );
+                return set;
             }
         }
 
-        return result;
+        return default;
     }
 
     private static bool TryGetApiVersions( IList<object> metadata, out ApiVersionBuckets buckets )

src/AspNetCore/WebApi/src/Asp.Versioning.Http/Builder/IEndpointConventionBuilderExtensions.cs

@@ -32,7 +32,7 @@ public static TBuilder WithApiVersionSet<TBuilder>(
             throw new ArgumentNullException( nameof( apiVersionSet ) );
         }
 
-        builder.Add( endpoint => endpoint.Metadata.Add( apiVersionSet ) );
+        builder.Add( endpoint => AddWithValidation( endpoint, apiVersionSet ) );
         builder.Finally( EndpointBuilderFinalizer.FinalizeEndpoints );
 
         return builder;
@@ -393,6 +393,27 @@ public static TBuilder ReportApiVersions<TBuilder>( this TBuilder builder )
         return builder;
     }
 
+    private static void AddWithValidation( EndpointBuilder builder, ApiVersionSet versionSet )
+    {
+        var metadata = builder.Metadata;
+        var grouped = builder.ApplicationServices.GetService( typeof( ApiVersionSetBuilder ) ) is not null;
+
+        if ( grouped )
+        {
+            throw new InvalidOperationException( SR.MultipleVersionSets );
+        }
+
+        for ( var i = 0; i < metadata.Count; i++ )
+        {
+            if ( metadata[i] is ApiVersionSet )
+            {
+                throw new InvalidOperationException( SR.MultipleVersionSets );
+            }
+        }
+
+        metadata.Add( versionSet );
+    }
+
     private static void AdvertiseInApiVersionSet( IList<object> metadata, ApiVersion apiVersion )
     {
         for ( var i = metadata.Count - 1; i >= 0; i-- )

src/AspNetCore/WebApi/src/Asp.Versioning.Http/SR.Designer.cs

@@ -129,6 +129,9 @@
   <data name="ConventionAddedAfterEndpointBuilt" xml:space="preserve">
     <value>Conventions cannot be added after building the endpoint.</value>
   </data>
+  <data name="MultipleVersionSets" xml:space="preserve">
+    <value>An endpoint cannot apply multiple API version sets.</value>
+  </data>
   <data name="NoVersionSet" xml:space="preserve">
     <value>The endpoint '{0}' does not have an associated API version set. Are you missing a call to {1} or {2}.</value>
   </data>

src/AspNetCore/WebApi/src/Asp.Versioning.Http/SR.resx

@@ -6,6 +6,7 @@ namespace Asp.Versioning.Builder;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Routing;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Options;
 using static Asp.Versioning.ApiVersionProviderOptions;
 
@@ -251,6 +252,56 @@ public void with_api_version_set_should_collate_across_grouped_endpoints()
                     .BeEquivalentTo( ApiVersionMetadata.Neutral );
     }
 
+    [Fact]
+    public void with_api_version_set_should_not_be_allowed_multiple_times()
+    {
+        // arrange
+        var builder = WebApplication.CreateBuilder();
+        var services = builder.Services;
+
+        services.AddControllers();
+        services.AddApiVersioning();
+
+        var app = builder.Build();
+        var versionSet = new ApiVersionSetBuilder( default ).Build();
+        var get = app.MapGet( "/", () => Results.Ok() );
+        IEndpointRouteBuilder endpoints = app;
+
+        get.WithApiVersionSet( versionSet );
+        get.WithApiVersionSet( versionSet );
+
+        // act
+        var build = () => endpoints.DataSources.Single().Endpoints;
+
+        // assert
+        build.Should().Throw<InvalidOperationException>();
+    }
+
+    [Fact]
+    public void with_api_version_set_should_not_override_existing_metadata()
+    {
+        // arrange
+        var builder = WebApplication.CreateBuilder();
+        var services = builder.Services;
+
+        services.AddControllers();
+        services.AddApiVersioning();
+
+        var app = builder.Build();
+        var versionSet = new ApiVersionSetBuilder( default ).Build();
+        var group = app.MapApiGroup();
+        var get = group.MapGet( "/", () => Results.Ok() );
+        IEndpointRouteBuilder endpoints = app;
+
+        get.WithApiVersionSet( versionSet );
+
+        // act
+        var build = () => endpoints.DataSources.Single().Endpoints;
+
+        // assert
+        build.Should().Throw<InvalidOperationException>();
+    }
+
     [Fact]
     public void report_api_versions_should_add_convention()
     {
@@ -696,11 +747,6 @@ public object GetService( Type serviceType )
                 return options.Value.ApiVersionReader;
             }
 
-            if ( typeof( IApiVersionSetBuilderFactory ) == serviceType )
-            {
-                return new DefaultApiVersionSetBuilderFactory();
-            }
-
             return null;
         }
     }