add the write logic for the OpenApiAny types#31
add the write logic for the OpenApiAny types#31xuzhg merged 2 commits intomicrosoft:masterfrom xuzhg:AddWriteSpecExtensionMethods
Conversation
|
@xuzhg, |
| // </copyright> | ||
| //--------------------------------------------------------------------- | ||
|
|
||
| namespace Microsoft.OpenApi |
There was a problem hiding this comment.
Microsoft.OpenApi [](start = 10, length = 17)
Microsoft.OpenApi.Any
There was a problem hiding this comment.
Ok, I will change all the class in "Any" folder.
| @@ -6,22 +6,7 @@ | |||
|
|
|||
| namespace Microsoft.OpenApi | |||
There was a problem hiding this comment.
Microsoft.OpenApi [](start = 10, length = 17)
Microsoft.OpenApi.Any
There was a problem hiding this comment.
See the above comment
| Password | ||
| } | ||
|
|
||
| public interface IOpenApiPrimitive : IOpenApiAny |
There was a problem hiding this comment.
IOpenApiPrimitive [](start = 21, length = 17)
Why do we need both abstract class and interface?
There was a problem hiding this comment.
interface is used for the know type for the primitive.
the abstract class reserved as the generic type for primitive. So, each "primitive" has a separate base "generic" type implemented the IOpenApiPrimitive interface. Hope it can answer the question. :)
There was a problem hiding this comment.
Understood. I see how you are using it in the Extensions now.
| /// <summary> | ||
| /// Extensions methods for writing the <see cref="IOpenApiAny"/> | ||
| /// </summary> | ||
| public static class WriterOpenApiAnyExtensions |
There was a problem hiding this comment.
WriterOpenApiAnyExtensions [](start = 24, length = 26)
OpenApiWriterAnyExtensions
|
|
||
| namespace Microsoft.OpenApi.Tests | ||
| { | ||
| public class WriterOpenApiAnyExtensionsTests |
There was a problem hiding this comment.
WriterOpenApiAnyExtensionsTests [](start = 17, length = 31)
OpenApiWriterAnyExtensionsTests
I wonder if this Refers to: src/Microsoft.OpenApi/Any/OpenApiPrimitive.cs:13 in b7addc6. [](commit_id = b7addc6, deletion_comment = False) |
| }; | ||
|
|
||
| [Fact] | ||
| public void WriteOpenApiObjectWorksJson() |
There was a problem hiding this comment.
WriteOpenApiObjectWorksJson [](start = 20, length = 27)
WriteOpenApiObjectAsJsonWorks
| } | ||
|
|
||
| [Fact] | ||
| public void WriteOpenApiArrayWorksJson() |
There was a problem hiding this comment.
WriteOpenApiArrayWorksJson [](start = 20, length = 26)
WriteOpenApiArrayAsJsonWorks
| Assert.Equal(expect, json); | ||
| } | ||
|
|
||
| private static string WriteToJson(IOpenApiAny any) |
There was a problem hiding this comment.
WriteToJson [](start = 30, length = 11)
WriteAsJson
|
Can you add a few more unit tests? One including a variety of primitive types and maybe one including null. |
|
@PerthCharern More test cases added. Would you please take a look? Thanks. |
…or write any type
| } | ||
| else | ||
| { | ||
| // add "\r\n" at the head because the expect string has a newline at starting. |
There was a problem hiding this comment.
// add "\r\n" at the head because the expect string has a newline at starting. [](start = 16, length = 78)
Instead of this arbitrary newline, is it better to just make the expected string start with { (instead of newline)?
There was a problem hiding this comment.
I do think the below:
string expect = @"
[
false,
{
....
]
},
""stringValue2""
]";looks better than:
string expect = @"[
false,
{
....
]
},
""stringValue2""
]";What do you think?
There was a problem hiding this comment.
That's fine. Not a big deal. It looks kinda funny with the indentation being off, but I see why you need that.
|
I'm ok with merging this PR, because I think the primitives are useful. However, I'm not convinced the that OpenApiArray/OpenApiObject/OpenApiAny are the right way of handling examples and extensions. |
…9483-9ee135a0c31b feat: Add support for dataValue and serializedValue in OpenApiExample
4 participants
Add the write logic for the OpenApiAny types.
Add 2 test cases to test the write logic
Modify some test cases copyright.