diff --git a/CHANGELOG.md b/CHANGELOG.md index 8db62b0d..161061bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +# 0.12.3 2025-10-13 + +- Fix crash in logging when property named "$ref" + ## 0.12.2 2025-09-22 - bump dependency `js-yaml` from `^3.13.0` to `^4.1.0` diff --git a/openapi-diff/src/core/OpenApiDiff.Core/Logging/ObjectPath.cs b/openapi-diff/src/core/OpenApiDiff.Core/Logging/ObjectPath.cs index b38c83fa..d0b3dc7c 100644 --- a/openapi-diff/src/core/OpenApiDiff.Core/Logging/ObjectPath.cs +++ b/openapi-diff/src/core/OpenApiDiff.Core/Logging/ObjectPath.cs @@ -76,9 +76,23 @@ private static JToken FromObject(JObject o, string name) { return null; } + var @ref = o["$ref"]; - var unrefed = @ref != null ? ParseRef(@ref.Value()).CompletePath(o.Root).Last().token : o; - return unrefed[name]; + + // Handle $ref resolution based on its type + if (@ref != null && @ref.Type == JTokenType.String) + { + // Case 1: $ref is a string reference (e.g., "#/definitions/FieldType") + // Resolve the reference by parsing the path and following it to the target + var unrefed = ParseRef(@ref.Value()).CompletePath(o.Root).Last().token; + return unrefed[name]; + } + else + { + // Case 2: $ref is not a string (e.g., a JSON object defining a schema) + // or $ref doesn't exist - use the current object directly + return o[name]; + } } private static IEnumerable<(JToken token, string name)> CompletePath(IEnumerable> path, JToken token) diff --git a/openapi-diff/src/modeler/AutoRest.Swagger.Tests/Resource/Swagger/new/type_changed_02.json b/openapi-diff/src/modeler/AutoRest.Swagger.Tests/Resource/Swagger/new/type_changed_02.json new file mode 100644 index 00000000..b9565036 --- /dev/null +++ b/openapi-diff/src/modeler/AutoRest.Swagger.Tests/Resource/Swagger/new/type_changed_02.json @@ -0,0 +1,48 @@ +{ + "swagger": 2.0, + "info": { + "title": "type_changed", + "version": "1.0" + }, + "host": "localhost:8000", + "schemes": [ "http", "https" ], + "consumes": [ "text/plain", "text/json" ], + "produces": [ "text/plain" ], + "paths": { + "/api/Parameters": { + "put": { + "tag": [ "Parameters" ], + "operationId": "Parameters_Put", + "produces": [ + "text/plain" + ], + "parameters": [ + { + "name": "database", + "in": "body", + "required": true, + "type": "object", + "schema": { "$ref": "#/definitions/Database" } + } + ] + } + } + }, + "definitions": { + "Database": { + "properties": { + "$ref": { + "type": "integer", + "readOnly": true, + "description": "Property named '$ref'. Unusual but valid." + }, + "b": { + "type": "integer", + "readOnly": true, + "default": 0, + "description": "This property shows the number of databases returned." + } + } + } + } +} diff --git a/openapi-diff/src/modeler/AutoRest.Swagger.Tests/Resource/Swagger/old/type_changed_02.json b/openapi-diff/src/modeler/AutoRest.Swagger.Tests/Resource/Swagger/old/type_changed_02.json new file mode 100644 index 00000000..c072fd1c --- /dev/null +++ b/openapi-diff/src/modeler/AutoRest.Swagger.Tests/Resource/Swagger/old/type_changed_02.json @@ -0,0 +1,48 @@ +{ + "swagger": 2.0, + "info": { + "title": "type_changed", + "version": "1.0" + }, + "host": "localhost:8000", + "schemes": [ "http", "https" ], + "consumes": [ "text/plain", "text/json" ], + "produces": [ "text/plain" ], + "paths": { + "/api/Parameters": { + "put": { + "tag": [ "Parameters" ], + "operationId": "Parameters_Put", + "produces": [ + "text/plain" + ], + "parameters": [ + { + "name": "database", + "in": "body", + "required": true, + "type": "object", + "schema": { "$ref": "#/definitions/Database" } + } + ] + } + } + }, + "definitions": { + "Database": { + "properties": { + "$ref": { + "type": "string", + "readOnly": true, + "description": "Property named '$ref'. Unusual but valid." + }, + "b": { + "type": "integer", + "readOnly": true, + "default": 0, + "description": "This property shows the number of databases returned." + } + } + } + } +} diff --git a/openapi-diff/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerCompareTests.cs b/openapi-diff/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerCompareTests.cs index 37620bf2..a00bafe5 100644 --- a/openapi-diff/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerCompareTests.cs +++ b/openapi-diff/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerCompareTests.cs @@ -165,6 +165,21 @@ public void TypeObjectChanged() Assert.Equal("new/type_changed_01.json#/definitions/Database/properties/a", error.NewJsonRef); } + /// + /// Verifies that if you change the type of a schema property named "$ref" (unusual but valid), it's caught. + /// + [Fact] + public void PropertyNamedRefTypeChanged() + { + var messages = CompareSwagger("type_changed_02.json").ToArray(); + var missing = messages.Where(m => m.Id == ComparisonMessages.TypeChanged.Id); + Assert.NotEmpty(missing); + var error = missing.Where(err => err.NewJsonRef.StartsWith("new/type_changed_02.json#/definitions/")).FirstOrDefault(); + Assert.NotNull(error); + Assert.Equal(Category.Error, error.Severity); + Assert.Equal("new/type_changed_02.json#/definitions/Database/properties/$ref", error.NewJsonRef); + } + /// /// Verifies that if you change the default value of a schema, it's caught. /// diff --git a/package-lock.json b/package-lock.json index 39675859..a3426785 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@azure/oad", - "version": "0.12.2", + "version": "0.12.3", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@azure/oad", - "version": "0.12.2", + "version": "0.12.3", "license": "MIT", "dependencies": { "@ts-common/fs": "0.2.0", diff --git a/package.json b/package.json index db788b65..aa539c7d 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@azure/oad", - "version": "0.12.2", + "version": "0.12.3", "author": { "name": "Microsoft Corporation", "email": "azsdkteam@microsoft.com",