diff --git a/DevProxy.Integration.Tests/GenerationPluginsIntegrationTests.cs b/DevProxy.Integration.Tests/GenerationPluginsIntegrationTests.cs index b120d019..66ebe296 100644 --- a/DevProxy.Integration.Tests/GenerationPluginsIntegrationTests.cs +++ b/DevProxy.Integration.Tests/GenerationPluginsIntegrationTests.cs @@ -365,6 +365,252 @@ public async Task OpenApiSpecGenerator_WritesSpec() } } + private static async Task GenerateOpenApiSpecAsync(params RequestLog[] logs) + { + var plugin = new OpenApiSpecGeneratorPlugin( + TestDefaults.HttpClient, + NullLogger.Instance, + Watch, + new DisabledLanguageModelClient(), + new TestProxyConfiguration(), + PluginConfig.Empty()); + + var args = Recording(logs); + var dir = await InTempCwdAsync(() => plugin.AfterRecordingStopAsync(args, CancellationToken.None)); + try + { + var spec = dir.GetFiles("*.json").Single(); + return JsonDocument.Parse(await File.ReadAllTextAsync(spec.FullName)); + } + finally + { + dir.Delete(recursive: true); + } + } + + private static string[] ParameterNames(JsonElement element, string location) => + [.. element.GetProperty("parameters").EnumerateArray() + .Where(p => p.GetProperty("in").GetString() == location) + .Select(p => p.GetProperty("name").GetString()!)]; + + [Fact] + public async Task OpenApiSpecGenerator_IncludesParametersAndResponseHeaders() + { + var exchange = TestExchange + .Request( + "GET", + "https://api.contoso.com/users/123?$select=name", + headers: [("Accept", "application/json"), ("X-Custom-Request", "foo")]) + .WithResponse( + HttpStatusCode.OK, + headers: [("Content-Type", "application/json"), ("X-Custom-Response", "bar")], + body: """{ "id": 123, "name": "alpha" }""") + .AsRequestLog(MessageType.InterceptedResponse); + + using var spec = await GenerateOpenApiSpecAsync(exchange); + + var pathItem = spec.RootElement.GetProperty("paths").GetProperty("/users/{users-id}"); + Assert.Equal(["users-id"], ParameterNames(pathItem, "path")); + + var operation = pathItem.GetProperty("get"); + Assert.Equal(["$select"], ParameterNames(operation, "query")); + Assert.Equal(["X-Custom-Request"], ParameterNames(operation, "header")); + + var response = operation.GetProperty("responses").GetProperty("200"); + var responseHeaders = response.GetProperty("headers").EnumerateObject().Select(h => h.Name).ToArray(); + Assert.Equal(["X-Custom-Response"], responseHeaders); + Assert.True(response.GetProperty("content").TryGetProperty("application/json", out _)); + } + + [Fact] + public async Task OpenApiSpecGenerator_MergesParametersAcrossRequests() + { + static RequestLog Exchange(string url) => + TestExchange + .Request("GET", url) + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: """[]""") + .AsRequestLog(MessageType.InterceptedResponse); + + using var spec = await GenerateOpenApiSpecAsync( + Exchange("https://api.contoso.com/users?$top=1"), + Exchange("https://api.contoso.com/users?$top=2&$skip=1")); + + var operation = spec.RootElement.GetProperty("paths").GetProperty("/users").GetProperty("get"); + Assert.Equal(["$top", "$skip"], ParameterNames(operation, "query")); + } + + [Fact] + public async Task OpenApiSpecGenerator_DeduplicatesRepeatedHeaders() + { + var exchange = TestExchange + .Request("GET", "https://api.contoso.com/users", headers: [("X-Custom-Request", "a"), ("x-custom-request", "b")]) + .WithResponse( + HttpStatusCode.OK, + headers: [("Content-Type", "application/json"), ("X-Custom-Response", "a"), ("x-custom-response", "b")], + body: """[]""") + .AsRequestLog(MessageType.InterceptedResponse); + + using var spec = await GenerateOpenApiSpecAsync(exchange); + + var operation = spec.RootElement.GetProperty("paths").GetProperty("/users").GetProperty("get"); + Assert.Equal(["X-Custom-Request"], ParameterNames(operation, "header")); + var responseHeaders = operation.GetProperty("responses").GetProperty("200").GetProperty("headers").EnumerateObject().Select(h => h.Name).ToArray(); + Assert.Equal(["X-Custom-Response"], responseHeaders); + } + + [Fact] + public async Task OpenApiSpecGenerator_GeneratesUniquePathParameterNames() + { + var exchange = TestExchange + .Request("GET", "https://api.contoso.com/tiles/12/3456/7890") + .WithResponse(HttpStatusCode.NoContent) + .AsRequestLog(MessageType.InterceptedResponse); + + using var spec = await GenerateOpenApiSpecAsync(exchange); + + var pathItem = spec.RootElement.GetProperty("paths").GetProperty("/tiles/{tiles-id}/{tiles-id-2}/{tiles-id-3}"); + Assert.Equal(["tiles-id", "tiles-id-2", "tiles-id-3"], ParameterNames(pathItem, "path")); + } + + [Fact] + public async Task OpenApiSpecGenerator_MergesHeadersAcrossRequests() + { + static RequestLog Exchange(string requestHeader, (string Name, string Value)[] responseHeaders) => + TestExchange + .Request("GET", "https://api.contoso.com/users", headers: [(requestHeader, "a")]) + .WithResponse(HttpStatusCode.NoContent, headers: responseHeaders) + .AsRequestLog(MessageType.InterceptedResponse); + + using var spec = await GenerateOpenApiSpecAsync( + Exchange("X-Custom-Request", []), + Exchange("x-custom-request", [("X-Custom-Response", "a")]), + Exchange("X-Custom-Request", [("x-custom-response", "b")])); + + var operation = spec.RootElement.GetProperty("paths").GetProperty("/users").GetProperty("get"); + Assert.Equal(["X-Custom-Request"], ParameterNames(operation, "header")); + var responseHeaders = operation.GetProperty("responses").GetProperty("204").GetProperty("headers").EnumerateObject().Select(h => h.Name).ToArray(); + Assert.Equal(["X-Custom-Response"], responseHeaders); + } + + [Fact] + public async Task OpenApiSpecGenerator_MergesBodySchemasAcrossRequests() + { + static RequestLog Exchange(string requestBody, string responseBody) => + TestExchange + .Request("POST", "https://api.contoso.com/users", headers: [("Content-Type", "application/json")], body: requestBody) + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: responseBody) + .AsRequestLog(MessageType.InterceptedResponse); + + using var spec = await GenerateOpenApiSpecAsync( + Exchange("""{ "id": 1 }""", """{ "id": 1 }"""), + Exchange("""{ "id": 2, "name": "alpha" }""", """{ "id": 2, "name": "alpha" }""")); + + var operation = spec.RootElement.GetProperty("paths").GetProperty("/users").GetProperty("post"); + static string[] PropertyNames(JsonElement content) => + [.. content.GetProperty("application/json").GetProperty("schema").GetProperty("properties").EnumerateObject().Select(p => p.Name)]; + + Assert.Equal(["id", "name"], PropertyNames(operation.GetProperty("requestBody").GetProperty("content"))); + Assert.Equal(["id", "name"], PropertyNames(operation.GetProperty("responses").GetProperty("200").GetProperty("content"))); + } + + [Fact] + public async Task OpenApiSpecGenerator_MergesArraySchemasAcrossRequests() + { + static RequestLog Exchange(string responseBody) => + TestExchange + .Request("GET", "https://api.contoso.com/users") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: responseBody) + .AsRequestLog(MessageType.InterceptedResponse); + + using var spec = await GenerateOpenApiSpecAsync( + Exchange("""[ { "id": 1, "tags": [ { "id": 1 } ] } ]"""), + Exchange("""[ { "id": 2, "name": "alpha", "tags": [ { "id": 2, "label": "beta" } ] } ]""")); + + var items = spec.RootElement.GetProperty("paths").GetProperty("/users").GetProperty("get") + .GetProperty("responses").GetProperty("200").GetProperty("content").GetProperty("application/json") + .GetProperty("schema").GetProperty("items"); + static string[] PropertyNames(JsonElement schema) => + [.. schema.GetProperty("properties").EnumerateObject().Select(p => p.Name)]; + + Assert.Equal(["id", "tags", "name"], PropertyNames(items)); + Assert.Equal(["id", "label"], PropertyNames(items.GetProperty("properties").GetProperty("tags").GetProperty("items"))); + } + + [Theory] + [InlineData("[]", """[ { "id": 1, "name": "alpha" } ]""", "items")] + [InlineData("{}", """{ "id": 1, "name": "alpha" }""", null)] + public async Task OpenApiSpecGenerator_MergesIntoEmptySchemaRecordedFirst(string firstBody, string secondBody, string? itemsProperty) + { + static RequestLog Exchange(string responseBody) => + TestExchange + .Request("GET", "https://api.contoso.com/users") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: responseBody) + .AsRequestLog(MessageType.InterceptedResponse); + + using var spec = await GenerateOpenApiSpecAsync(Exchange(firstBody), Exchange(secondBody)); + + var schema = spec.RootElement.GetProperty("paths").GetProperty("/users").GetProperty("get") + .GetProperty("responses").GetProperty("200").GetProperty("content").GetProperty("application/json") + .GetProperty("schema"); + if (itemsProperty is not null) + { + schema = schema.GetProperty(itemsProperty); + } + + Assert.Equal(["id", "name"], schema.GetProperty("properties").EnumerateObject().Select(p => p.Name).ToArray()); + } + + [Fact] + public async Task OpenApiSpecGenerator_ReplacesUntypedItemsOfEmptyArrayRecordedFirst() + { + static RequestLog Exchange(string responseBody) => + TestExchange + .Request("GET", "https://api.contoso.com/users") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: responseBody) + .AsRequestLog(MessageType.InterceptedResponse); + + using var spec = await GenerateOpenApiSpecAsync( + Exchange("""{ "tags": [] }"""), + Exchange("""{ "tags": [ "a", "b" ] }""")); + + var tags = spec.RootElement.GetProperty("paths").GetProperty("/users").GetProperty("get") + .GetProperty("responses").GetProperty("200").GetProperty("content").GetProperty("application/json") + .GetProperty("schema").GetProperty("properties").GetProperty("tags"); + Assert.Equal("array", tags.GetProperty("type").GetString()); + Assert.Equal("string", tags.GetProperty("items").GetProperty("type").GetString()); + } + + [Fact] + public async Task OpenApiSpecGenerator_EmitsUntypedItemsForEmptyArray() + { + var exchange = TestExchange + .Request("GET", "https://api.contoso.com/users") + .WithResponse(HttpStatusCode.OK, headers: [("Content-Type", "application/json")], body: "[]") + .AsRequestLog(MessageType.InterceptedResponse); + + using var spec = await GenerateOpenApiSpecAsync(exchange); + + var schema = spec.RootElement.GetProperty("paths").GetProperty("/users").GetProperty("get") + .GetProperty("responses").GetProperty("200").GetProperty("content").GetProperty("application/json") + .GetProperty("schema"); + Assert.Equal("array", schema.GetProperty("type").GetString()); + Assert.Empty(schema.GetProperty("items").EnumerateObject()); + } + + [Fact] + public async Task OpenApiSpecGenerator_SkipsResponseBodyWithoutContentType() + { + var exchange = TestExchange + .Request("GET", "https://api.contoso.com/users") + .WithResponse(HttpStatusCode.OK, body: "alpha") + .AsRequestLog(MessageType.InterceptedResponse); + + using var spec = await GenerateOpenApiSpecAsync(exchange); + + var response = spec.RootElement.GetProperty("paths").GetProperty("/users").GetProperty("get").GetProperty("responses").GetProperty("200"); + Assert.False(response.TryGetProperty("content", out _)); + } + [Fact] public async Task TypeSpecGenerator_WritesSpec() { diff --git a/DevProxy.Plugins/Generation/OpenApiSpecGeneratorPlugin.cs b/DevProxy.Plugins/Generation/OpenApiSpecGeneratorPlugin.cs index 9963c038..68b5536f 100644 --- a/DevProxy.Plugins/Generation/OpenApiSpecGeneratorPlugin.cs +++ b/DevProxy.Plugins/Generation/OpenApiSpecGeneratorPlugin.cs @@ -230,7 +230,8 @@ private OpenApiPathItem GetOpenApiPathItem(IProxySession session) var resource = GetLastNonTokenSegment(request.RequestUri.Segments); var path = new OpenApiPathItem { - Operations = [] + Operations = [], + Parameters = [] }; var method = request.Method?.ToUpperInvariant() switch @@ -251,7 +252,8 @@ private OpenApiPathItem GetOpenApiPathItem(IProxySession session) // will be replaced later after the path has been parametrized Description = $"{method} {resource}", // will be replaced later after the path has been parametrized - OperationId = $"{method}.{resource}" + OperationId = $"{method}.{resource}", + Parameters = [] }; SetParametersFromQueryString(operation, HttpUtility.ParseQueryString(request.RequestUri.Query)); SetParametersFromRequestHeaders(operation, request.Headers); @@ -303,6 +305,7 @@ private void SetParametersFromRequestHeaders(OpenApiOperation operation, IHeader } Logger.LogDebug(" Processing request headers..."); + var addedHeaders = new HashSet(StringComparer.OrdinalIgnoreCase); foreach (var header in headers) { var lowerCaseHeaderName = header.Name.ToLowerInvariant(); @@ -318,6 +321,12 @@ private void SetParametersFromRequestHeaders(OpenApiOperation operation, IHeader continue; } + if (!addedHeaders.Add(header.Name)) + { + Logger.LogDebug(" Header {HeaderName} already added", header.Name); + continue; + } + operation.Parameters?.Add(new OpenApiParameter() { Name = header.Name, @@ -385,16 +394,23 @@ private void SetResponseFromSession(OpenApiOperation operation, IHttpResponse re var openApiResponse = new OpenApiResponse { - Description = response.StatusDescription + Description = response.StatusDescription, + Headers = new Dictionary(StringComparer.OrdinalIgnoreCase), + Content = new Dictionary() }; var responseCode = ((int)response.StatusCode).ToString(CultureInfo.InvariantCulture); - if (response.HasBody) + var responseMediaType = GetMediaType(response.ContentType); + if (response.HasBody && string.IsNullOrEmpty(responseMediaType)) + { + Logger.LogDebug(" Response has body but no content type"); + } + else if (response.HasBody) { Logger.LogDebug(" Response has body"); - openApiResponse.Content?.Add(GetMediaType(response.ContentType), new OpenApiMediaType() + openApiResponse.Content?.Add(responseMediaType, new OpenApiMediaType() { - Schema = GetSchemaFromBody(GetMediaType(response.ContentType), response.BodyString) + Schema = GetSchemaFromBody(responseMediaType, response.BodyString) }); } else @@ -551,11 +567,12 @@ private void AddOrMergeParameters(OpenApiOperation operation, IList p.Name == parameter.Name && p.In == parameter.In); + var nameComparison = parameter.In == ParameterLocation.Header ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; + var paramFromOperation = operation.Parameters?.FirstOrDefault(p => p.In == parameter.In && string.Equals(p.Name, parameter.Name, nameComparison)); if (paramFromOperation is null) { Logger.LogDebug(" Adding parameter {ParameterName} to operation...", parameter.Name); - operation.Parameters?.Add(parameter); + (operation.Parameters ??= []).Add(parameter); continue; } @@ -572,6 +589,21 @@ private void MergeSchema(IOpenApiSchema? source, IOpenApiSchema? target) return; } + if (source.Type == JsonSchemaType.Array && target.Type == JsonSchemaType.Array) + { + // replace the untyped items placeholder generated for an empty array + if (target is OpenApiSchema targetArray && targetArray.Items is { Type: null } && source.Items?.Type is not null) + { + Logger.LogDebug(" Replacing untyped array items..."); + targetArray.Items = source.Items; + return; + } + + Logger.LogDebug(" Merging array items..."); + MergeSchema(source.Items, target.Items); + return; + } + if (source.Type != JsonSchemaType.Object || target.Type != JsonSchemaType.Object) { Logger.LogDebug(" Source or target schema is not an object. Skipping..."); @@ -584,25 +616,31 @@ private void MergeSchema(IOpenApiSchema? source, IOpenApiSchema? target) return; } - if (target.Properties is null || !target.Properties.Any()) + var targetProperties = target.Properties; + if (targetProperties is null) { - Logger.LogDebug(" Target has no properties. Skipping..."); - return; + if (target is not OpenApiSchema targetSchema) + { + Logger.LogDebug(" Target has no properties. Skipping..."); + return; + } + + targetSchema.Properties = targetProperties = new Dictionary(); } foreach (var property in source.Properties) { - var propertyFromTarget = target.Properties.FirstOrDefault(p => p.Key == property.Key); + var propertyFromTarget = targetProperties.FirstOrDefault(p => p.Key == property.Key); if (propertyFromTarget.Value is null) { Logger.LogDebug(" Adding property {PropertyKey} to schema...", property.Key); - target.Properties.Add(property); + targetProperties.Add(property); continue; } - if (property.Value.Type != JsonSchemaType.Object) + if (property.Value.Type is not (JsonSchemaType.Object or JsonSchemaType.Array)) { - Logger.LogDebug(" Property already found but is not an object. Skipping..."); + Logger.LogDebug(" Property already found but is not an object or array. Skipping..."); continue; } @@ -640,7 +678,7 @@ private void AddOrMergeRequestBody(OpenApiOperation operation, OpenApiRequestBod } Logger.LogDebug(" Merging request body into operation..."); - MergeSchema(bodyFromOperation.Schema, requestBody.Content.FirstOrDefault().Value.Schema); + MergeSchema(requestBody.Content.FirstOrDefault().Value.Schema, bodyFromOperation.Schema); } private void AddOrMergeResponse(OpenApiOperation operation, OpenApiResponses? apiResponses) @@ -669,6 +707,8 @@ private void AddOrMergeResponse(OpenApiOperation operation, OpenApiResponses? ap return; } + MergeResponseHeaders(responseFromOperation, apiResponse); + if (apiResponse.Content is null || !apiResponse.Content.Any()) { Logger.LogDebug(" No response content to process"); @@ -692,7 +732,28 @@ private void AddOrMergeResponse(OpenApiOperation operation, OpenApiResponses? ap } Logger.LogDebug(" Merging response {ApiResponseStatusCode}/{ApiResponseContentType} into operation...", apiResponseStatusCode, apiResponseContentType); - MergeSchema(contentFromOperation.Schema, apiResponse.Content.First().Value.Schema); + MergeSchema(apiResponse.Content.First().Value.Schema, contentFromOperation.Schema); + } + + private void MergeResponseHeaders(IOpenApiResponse responseFromOperation, IOpenApiResponse apiResponse) + { + if (apiResponse.Headers is null || apiResponse.Headers.Count == 0 || + responseFromOperation is not OpenApiResponse retainedResponse) + { + return; + } + + retainedResponse.Headers ??= new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var (name, header) in apiResponse.Headers) + { + if (retainedResponse.Headers.ContainsKey(name)) + { + continue; + } + + Logger.LogDebug(" Adding response header {HeaderName}...", name); + retainedResponse.Headers.Add(name, header); + } } /** @@ -706,6 +767,7 @@ private static string ParametrizePath(OpenApiPathItem pathItem, Uri requestUri) { var segments = requestUri.Segments; var previousSegment = "item"; + var parameterNameCounts = new Dictionary(StringComparer.Ordinal); for (var i = 0; i < segments.Length; i++) { @@ -718,6 +780,11 @@ private static string ParametrizePath(OpenApiPathItem pathItem, Uri requestUri) if (IsParametrizable(segment)) { var parameterName = $"{previousSegment}-id"; + var count = parameterNameCounts[parameterName] = parameterNameCounts.GetValueOrDefault(parameterName) + 1; + if (count > 1) + { + parameterName = $"{parameterName}-{count}"; + } segments[i] = $"{{{parameterName}}}{(requestUri.Segments[i].EndsWith('/') ? "/" : "")}"; pathItem.Parameters?.Add(new OpenApiParameter @@ -829,7 +896,11 @@ private static OpenApiSchema GetSchemaFromJsonElement(JsonElement jsonElement) break; case JsonValueKind.Array: schema.Type = JsonSchemaType.Array; - schema.Items = GetSchemaFromJsonElement(jsonElement.EnumerateArray().FirstOrDefault()); + var firstItem = jsonElement.EnumerateArray().FirstOrDefault(); + // an empty array gives no type information, so use an untyped (any) items schema + schema.Items = firstItem.ValueKind == JsonValueKind.Undefined + ? new OpenApiSchema() + : GetSchemaFromJsonElement(firstItem); break; default: schema.Type = JsonSchemaType.Object;