Copy constructor doesn't create new OpenApiPathItem objects when creating new instance of OpenApiPaths #1193
Copy link
Copy link
Labels
Description
Describe the bug
Copy constructors doesn't create a new instance of OpenApiPathItem and its constituent properties on new OpenApiDocument(document).
To Reproduce
I have created a breaking test to demonstrate this:
OpenAPI.NET/test/Microsoft.OpenApi.Tests/Models/OpenApiDocumentTests.cs
Lines 1341 to 1358 in d5a5f8a
| [Fact] | |
| public void CopyConstructorForAdvancedDocumentWorks() | |
| { | |
| // Arrange & Act | |
| var doc = new OpenApiDocument(AdvancedDocument); | |
| // Change value of operation id for a given url | |
| var docOpId = doc.Paths["/pets"].Operations[OperationType.Get].OperationId = "findAllMyPets"; | |
| var advancedDocOpId = AdvancedDocument.Paths["/pets"].Operations[OperationType.Get].OperationId; | |
| // Assert | |
| Assert.NotNull(doc.Info); | |
| Assert.NotNull(doc.Servers); | |
| Assert.NotNull(doc.Paths); | |
| Assert.Equal(2, doc.Paths.Count); | |
| Assert.NotNull(doc.Components); | |
| Assert.NotEqual(docOpId, advancedDocOpId); | |
| } |
This is in the branch:
is/copy-ctrs
Expected behavior
In the test, I change the operationId of the cloned OpenAPI document. I don't expect the operationId of the original OpenAPI document to also be updated.
Reactions are currently unavailable
Activity
darrelmiller commentedon Apr 2, 2023
When cloning OpenApiPaths we are using the standard Dictionary constructor which does not do a clone of the values of the key value pairs.
https://github.com/microsoft/OpenAPI.NET/blob/vnext/src/Microsoft.OpenApi/Models/OpenApiExtensibleDictionary.cs#L31
MaggieKimani1 commentedon Apr 24, 2023
Reopening this issue as its not fixed.
kevinharing commentedon Dec 1, 2023
Having this issue with cloning
OpenApiPathItemas well. As a workaround I serialize to json via the built-in functionality and use ReadFragment to deserialize it again.baywet commentedon Jul 14, 2026
This is not a bug, it's by design. Dictionary creates shallow copies, removing the derived constructor would be a breaking change at this point. And the rest of the cloning infrastructure is using offering shallow copies as well.
I've amended the doc comments in #2947 to address that.
And if we get an issue/popular request to implement deep copies across the object model, we should do that at a later stage.
Let me know if you have any additional comments or questions.