fix(api): #197 Bundle C review nits — order-independent operationIds + nullable MediaSources fields

Refs #287 #288 #197

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

ErsatzTV.Core/Api/MediaSources/PathReplacementResponseModel.cs

@@ -1,4 +1,4 @@
 #nullable enable
 namespace ErsatzTV.Core.Api.MediaSources;
 
-public record PathReplacementResponseModel(int Id, string RemotePath, string LocalPath);
+public record PathReplacementResponseModel(int Id, string? RemotePath, string? LocalPath);

ErsatzTV.Core/Api/MediaSources/RemoteLibraryResponseModel.cs

@@ -3,4 +3,4 @@ using ErsatzTV.Core.Domain;
 
 namespace ErsatzTV.Core.Api.MediaSources;
 
-public record RemoteLibraryResponseModel(int Id, string Name, LibraryMediaKind MediaKind, bool ShouldSyncItems);
+public record RemoteLibraryResponseModel(int Id, string? Name, LibraryMediaKind MediaKind, bool ShouldSyncItems);

ErsatzTV.Core/Api/MediaSources/RemoteMediaSourceItemResponseModel.cs

@@ -1,4 +1,4 @@
 #nullable enable
 namespace ErsatzTV.Core.Api.MediaSources;
 
-public record RemoteMediaSourceItemResponseModel(int Id, string Name, string Address);
+public record RemoteMediaSourceItemResponseModel(int Id, string? Name, string? Address);

ErsatzTV.Tests/Controllers/OpenApiContractHonestyTests.cs

@@ -71,6 +71,42 @@ public class OpenApiContractHonestyTests
         }
     }
 
+    [Test]
+    public void Colliding_Troubleshoot_HeadGet_Pairs_Should_Carry_VerbDerived_OperationIds()
+    {
+        // The 3 Troubleshoot HEAD/GET pairs share a controller+action and differ only by verb. Their
+        // synthesized operationIds must be verb-suffixed (structural, order-independent) — NOT one bare
+        // base + one arbitrary suffix that depends on ApiExplorer visitation order. See #197 Bundle C.
+        var ids = EnumerateOperations().Select(op => op.Operation.OperationId!).ToHashSet(StringComparer.Ordinal);
+
+        foreach (string expected in new[]
+                 {
+                     "TroubleshootTroubleshootPlaybackGet",
+                     "TroubleshootTroubleshootPlaybackHead",
+                     "TroubleshootTroubleshootPlaybackArchiveGet",
+                     "TroubleshootTroubleshootPlaybackArchiveHead",
+                     "TroubleshootTroubleshootPlaybackSampleGet",
+                     "TroubleshootTroubleshootPlaybackSampleHead"
+                 })
+        {
+            ids.ShouldContain(expected);
+        }
+
+        // The old order-dependent forms (bare base + "...GET" suffix) must be gone.
+        foreach (string stale in new[]
+                 {
+                     "TroubleshootTroubleshootPlayback",
+                     "TroubleshootTroubleshootPlaybackArchive",
+                     "TroubleshootTroubleshootPlaybackSample",
+                     "TroubleshootTroubleshootPlaybackGET",
+                     "TroubleshootTroubleshootPlaybackArchiveGET",
+                     "TroubleshootTroubleshootPlaybackSampleGET"
+                 })
+        {
+            ids.ShouldNotContain(stale);
+        }
+    }
+
     [Test]
     public void ValidationProblemDetails_Schema_And_400_Should_Be_Documented_For_Binding_Operations()
     {

ErsatzTV/Serialization/ApiSecuritySchemeDocumentTransformer.cs

@@ -15,6 +15,7 @@ namespace ErsatzTV.Serialization;
 public static class ApiSecuritySchemeDocumentTransformer
 {
     public const string ValidationProblemDetailsSchemaId = "ValidationProblemDetails";
+    public const string ProblemDetailsSchemaId = "ProblemDetails";
 
     public static Task TransformAsync(
         OpenApiDocument document,
@@ -40,9 +41,37 @@ public static class ApiSecuritySchemeDocumentTransformer
             document.Components.Schemas[ValidationProblemDetailsSchemaId] = BuildValidationProblemDetailsSchema();
         }
 
+        // The injected 401 (ApiSecurityOperationTransformer) references the ProblemDetails schema. It
+        // normally already exists because other actions declare ProducesResponseType(typeof(ProblemDetails)),
+        // but self-provision it if absent so the 401 reference never dangles (belt-and-suspenders).
+        if (!document.Components.Schemas.ContainsKey(ProblemDetailsSchemaId))
+        {
+            document.Components.Schemas[ProblemDetailsSchemaId] = BuildProblemDetailsSchema();
+        }
+
         return Task.CompletedTask;
     }
 
+    // The RFC 7807 shape ASP.NET Core returns for a plain ProblemDetails response (the ValidationProblemDetails
+    // members minus the "errors" map).
+    private static OpenApiSchema BuildProblemDetailsSchema() =>
+        new()
+        {
+            Type = JsonSchemaType.Object,
+            Properties = new Dictionary<string, IOpenApiSchema>
+            {
+                ["type"] = new OpenApiSchema { Type = JsonSchemaType.String | JsonSchemaType.Null },
+                ["title"] = new OpenApiSchema { Type = JsonSchemaType.String | JsonSchemaType.Null },
+                ["status"] = new OpenApiSchema
+                {
+                    Type = JsonSchemaType.Integer | JsonSchemaType.Null,
+                    Format = "int32"
+                },
+                ["detail"] = new OpenApiSchema { Type = JsonSchemaType.String | JsonSchemaType.Null },
+                ["instance"] = new OpenApiSchema { Type = JsonSchemaType.String | JsonSchemaType.Null }
+            }
+        };
+
     // The RFC 7807 shape ASP.NET Core's [ApiController] + FluentValidation return on a model-binding /
     // validation failure: the standard ProblemDetails members plus an "errors" map of field -> messages.
     private static OpenApiSchema BuildValidationProblemDetailsSchema() =>

ErsatzTV/Serialization/OperationIdOpenApiTransformer.cs

@@ -1,7 +1,9 @@
 #nullable enable
+using Microsoft.AspNetCore.Mvc.ApiExplorer;
 using Microsoft.AspNetCore.Mvc.Controllers;
 using Microsoft.AspNetCore.OpenApi;
 using Microsoft.OpenApi;
+using HashSet = System.Collections.Generic.HashSet<string>;
 
 namespace ErsatzTV.Serialization;
 
@@ -13,15 +15,26 @@ namespace ErsatzTV.Serialization;
 /// <see cref="ControllerActionDescriptor" /> (<c>ControllerName</c> + <c>ActionName</c>) whenever the
 /// id is null/empty, and never rename an id the developer set explicitly.
 /// <para>
-/// A single instance is created per document generation and reused across every operation, so the
-/// <see cref="_used" /> set gives whole-document uniqueness: a synthesized id that collides is
-/// disambiguated deterministically (append the HTTP method, then a counter). Explicit ids are also
-/// recorded so a later synthesized id can't shadow one.
+/// Disambiguation is <b>structural, not visitation-order-dependent</b>: the colliding cases are the
+/// HEAD/GET pairs that share a controller+action but differ only by verb. We precompute (from the
+/// full <see cref="IApiDescriptionGroupCollectionProvider" />, filtered to the same
+/// <c>general</c> group the "v1" document includes) the set of synthesized base ids that 2+ operations
+/// share, and suffix <b>each</b> such operation by its HTTP verb (<c>&lt;base&gt;Get</c>,
+/// <c>&lt;base&gt;Head</c>). A base id that is unique across the document stays unsuffixed. The result
+/// is a pure function of (controller, action, verb): an SDK/framework change to discovery order can
+/// no longer rename a generated client method. Explicit ids are recorded so a synthesized id can't
+/// shadow one; a defensive counter remains a last-resort backstop for an impossible residual clash.
 /// </para>
 /// </summary>
 public sealed class OperationIdOpenApiTransformer : IOpenApiOperationTransformer
 {
-    private readonly System.Collections.Generic.HashSet<string> _used = new(StringComparer.Ordinal);
+    private const string GeneralGroupName = "general";
+
+    private readonly HashSet _used = new(StringComparer.Ordinal);
+    private readonly HashSet _collidingBaseIds;
+
+    public OperationIdOpenApiTransformer(IApiDescriptionGroupCollectionProvider descriptionProvider)
+        => _collidingBaseIds = ComputeCollidingBaseIds(descriptionProvider);
 
     public Task TransformAsync(
         OpenApiOperation operation,
@@ -41,22 +54,67 @@ public sealed class OperationIdOpenApiTransformer : IOpenApiOperationTransformer
         }
 
         string baseId = controllerAction.ControllerName + controllerAction.ActionName;
-        string candidate = baseId;
 
-        if (_used.Contains(candidate))
-        {
-            // Overloaded action (same controller+action name, different verb/route): qualify by verb.
-            candidate = baseId + (context.Description.HttpMethod ?? string.Empty);
-        }
+        // Structural, order-independent: if 2+ synthesized operations share this base id, suffix EACH
+        // by its verb; otherwise keep the bare base. Pure function of (controller, action, verb).
+        string candidate = _collidingBaseIds.Contains(baseId)
+            ? baseId + PascalCaseVerb(context.Description.HttpMethod)
+            : baseId;
 
+        // Defensive backstop only — with verb suffixing this cannot fire for the known document, but
+        // guarantees uniqueness if some future shape produces an unexpected clash.
         int suffix = 2;
-        while (!_used.Add(candidate))
+        string finalId = candidate;
+        while (!_used.Add(finalId))
         {
-            candidate = baseId + (context.Description.HttpMethod ?? string.Empty) + suffix;
+            finalId = candidate + suffix;
             suffix++;
         }
 
-        operation.OperationId = candidate;
+        operation.OperationId = finalId;
         return Task.CompletedTask;
     }
+
+    private static HashSet ComputeCollidingBaseIds(IApiDescriptionGroupCollectionProvider provider)
+    {
+        var counts = new Dictionary<string, int>(StringComparer.Ordinal);
+        foreach (ApiDescriptionGroup group in provider.ApiDescriptionGroups.Items)
+        {
+            foreach (ApiDescription description in group.Items)
+            {
+                // Mirror the "v1" document's ShouldInclude predicate.
+                if (description.GroupName != GeneralGroupName)
+                {
+                    continue;
+                }
+
+                if (description.ActionDescriptor is not ControllerActionDescriptor controllerAction)
+                {
+                    continue;
+                }
+
+                // Only operations that will actually be *synthesized* participate — an explicit
+                // Name= sets the route name (the framework's operationId), so it is excluded.
+                if (!string.IsNullOrEmpty(controllerAction.AttributeRouteInfo?.Name))
+                {
+                    continue;
+                }
+
+                string baseId = controllerAction.ControllerName + controllerAction.ActionName;
+                counts[baseId] = counts.TryGetValue(baseId, out int c) ? c + 1 : 1;
+            }
+        }
+
+        return counts.Where(kvp => kvp.Value >= 2).Select(kvp => kvp.Key).ToHashSet(StringComparer.Ordinal);
+    }
+
+    private static string PascalCaseVerb(string? httpMethod)
+    {
+        if (string.IsNullOrEmpty(httpMethod))
+        {
+            return string.Empty;
+        }
+
+        return char.ToUpperInvariant(httpMethod[0]) + httpMethod[1..].ToLowerInvariant();
+    }
 }

ErsatzTV/wwwroot/openapi/v1.json

@@ -21065,7 +21065,7 @@
           "Troubleshooting"
         ],
         "summary": "Start a troubleshooting playback session",
-        "operationId": "TroubleshootTroubleshootPlayback",
+        "operationId": "TroubleshootTroubleshootPlaybackHead",
         "parameters": [
           {
             "name": "mediaItem",
@@ -21243,7 +21243,7 @@
           "Troubleshooting"
         ],
         "summary": "Start a troubleshooting playback session",
-        "operationId": "TroubleshootTroubleshootPlaybackGET",
+        "operationId": "TroubleshootTroubleshootPlaybackGet",
         "parameters": [
           {
             "name": "mediaItem",
@@ -21423,7 +21423,7 @@
           "Troubleshooting"
         ],
         "summary": "Download the last troubleshooting playback session archive",
-        "operationId": "TroubleshootTroubleshootPlaybackArchive",
+        "operationId": "TroubleshootTroubleshootPlaybackArchiveHead",
         "responses": {
           "200": {
             "description": "OK"
@@ -21448,7 +21448,7 @@
           "Troubleshooting"
         ],
         "summary": "Download the last troubleshooting playback session archive",
-        "operationId": "TroubleshootTroubleshootPlaybackArchiveGET",
+        "operationId": "TroubleshootTroubleshootPlaybackArchiveGet",
         "responses": {
           "200": {
             "description": "OK"
@@ -21475,7 +21475,7 @@
           "Troubleshooting"
         ],
         "summary": "Download a media sample archive for troubleshooting",
-        "operationId": "TroubleshootTroubleshootPlaybackSample",
+        "operationId": "TroubleshootTroubleshootPlaybackSampleHead",
         "parameters": [
           {
             "name": "mediaItemId",
@@ -21521,7 +21521,7 @@
           "Troubleshooting"
         ],
         "summary": "Download a media sample archive for troubleshooting",
-        "operationId": "TroubleshootTroubleshootPlaybackSampleGET",
+        "operationId": "TroubleshootTroubleshootPlaybackSampleGet",
         "parameters": [
           {
             "name": "mediaItemId",
@@ -27446,10 +27446,16 @@
             "format": "int32"
           },
           "remotePath": {
-            "type": "string"
+            "type": [
+              "null",
+              "string"
+            ]
           },
           "localPath": {
-            "type": "string"
+            "type": [
+              "null",
+              "string"
+            ]
           }
         }
       },
@@ -28540,7 +28546,10 @@
             "format": "int32"
           },
           "name": {
-            "type": "string"
+            "type": [
+              "null",
+              "string"
+            ]
           },
           "mediaKind": {
             "$ref": "#/components/schemas/LibraryMediaKind"
@@ -28563,10 +28572,16 @@
             "format": "int32"
           },
           "name": {
-            "type": "string"
+            "type": [
+              "null",
+              "string"
+            ]
           },
           "address": {
-            "type": "string"
+            "type": [
+              "null",
+              "string"
+            ]
           }
         }
       },

docs/api-conventions.md

@@ -536,7 +536,11 @@ the runtime filter enforces, so the spec can never drift from enforcement. The d
 against the effective default (`Api:RequireKeyForReads=true`), under which every documented operation
 requires the key. Two companion transformers run on the "v1" document only:
 `OperationIdOpenApiTransformer` synthesizes a stable `operationId` (from controller+action) for the ~90
-operations that lacked a `Name=`, and `ValidationProblemOperationTransformer` documents the
+operations that lacked a `Name=` — and disambiguates the HEAD/GET pairs that share a controller+action
+**structurally, independent of ApiExplorer visitation order** (#197 Bundle C): when 2+ synthesized ops
+share a base id it suffixes **each** by its verb (`…Get`/`…Head`), so discovery-order churn can't rename
+a generated client method (a base id unique across the document stays unsuffixed; explicit `Name=` ids are
+never touched). `ValidationProblemOperationTransformer` documents the
 `400 ValidationProblemDetails` a model-binding/FluentValidation failure actually returns for any
 body/param-binding operation. All four are registered in `Startup.cs` on "v1" only, mirroring
 `NewtonsoftSchemaNamingTransformer`. `/api/v1` route versioning remains #286.

docs/endpoint-index.md

@@ -415,12 +415,12 @@
 | Method | Path | Operation | Summary |
 |---|---|---|---|
 | GET | `/api/troubleshoot/info` | GetTroubleshootingInfo | Get troubleshooting diagnostic info |
-| GET | `/api/troubleshoot/playback.m3u8` | TroubleshootTroubleshootPlaybackGET | Start a troubleshooting playback session |
-| HEAD | `/api/troubleshoot/playback.m3u8` | TroubleshootTroubleshootPlayback | Start a troubleshooting playback session |
-| GET | `/api/troubleshoot/playback/archive` | TroubleshootTroubleshootPlaybackArchiveGET | Download the last troubleshooting playback session archive |
-| HEAD | `/api/troubleshoot/playback/archive` | TroubleshootTroubleshootPlaybackArchive | Download the last troubleshooting playback session archive |
-| GET | `/api/troubleshoot/playback/sample/{mediaItemId}` | TroubleshootTroubleshootPlaybackSampleGET | Download a media sample archive for troubleshooting |
-| HEAD | `/api/troubleshoot/playback/sample/{mediaItemId}` | TroubleshootTroubleshootPlaybackSample | Download a media sample archive for troubleshooting |
+| GET | `/api/troubleshoot/playback.m3u8` | TroubleshootTroubleshootPlaybackGet | Start a troubleshooting playback session |
+| HEAD | `/api/troubleshoot/playback.m3u8` | TroubleshootTroubleshootPlaybackHead | Start a troubleshooting playback session |
+| GET | `/api/troubleshoot/playback/archive` | TroubleshootTroubleshootPlaybackArchiveGet | Download the last troubleshooting playback session archive |
+| HEAD | `/api/troubleshoot/playback/archive` | TroubleshootTroubleshootPlaybackArchiveHead | Download the last troubleshooting playback session archive |
+| GET | `/api/troubleshoot/playback/sample/{mediaItemId}` | TroubleshootTroubleshootPlaybackSampleGet | Download a media sample archive for troubleshooting |
+| HEAD | `/api/troubleshoot/playback/sample/{mediaItemId}` | TroubleshootTroubleshootPlaybackSampleHead | Download a media sample archive for troubleshooting |
 | GET | `/api/troubleshoot/playback/status` | GetTroubleshootingPlaybackStatus | Get the status of the current or last troubleshooting playback session |
 | GET | `/api/troubleshoot/playback/stream-selectors` | GetTroubleshootingStreamSelectors | List available channel stream selectors |
 | GET | `/api/troubleshoot/playback/subtitles/{mediaItemId}` | GetTroubleshootingSubtitles | List selectable subtitle streams for a media item |

web/src/api/generated/v1.d.ts

@@ -956,8 +956,8 @@ export interface components {
   };
     "PathReplacementResponseModel": {
     "id": number;
-    "remotePath": string;
-    "localPath": string;
+    "remotePath": null | string;
+    "localPath": null | string;
   };
     "PlaybackOrder": "None" | "Chronological" | "Random" | "Shuffle" | "ShuffleInOrder" | "MultiEpisodeShuffle" | "SeasonEpisode" | "RandomRotation" | "Marathon";
     "PlaylistGroupResponseModel": {
@@ -1164,14 +1164,14 @@ export interface components {
   };
     "RemoteLibraryResponseModel": {
     "id": number;
-    "name": string;
+    "name": null | string;
     "mediaKind": components["schemas"]["LibraryMediaKind"];
     "shouldSyncItems": boolean;
   };
     "RemoteMediaSourceItemResponseModel": {
     "id": number;
-    "name": string;
-    "address": string;
+    "name": null | string;
+    "address": null | string;
   };
     "RemoteMediaSourceStateResponseModel": {
     "isAuthorized": boolean;