Make Open API Any reader works in the reader project by xuzhg · Pull Request #54 · microsoft/OpenAPI.NET

xuzhg · 2017-11-02T01:04:19Z

Make Open API Any <IOpenApiAny> reader works in the reader project

msftclas · 2017-11-02T01:04:23Z

@xuzhg,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

xuzhg · 2017-11-02T01:09:25Z

src/Microsoft.OpenApi/ExtensionMethods.cs

@@ -0,0 +1,58 @@
+// ------------------------------------------------------------
+//  Copyright (c) Microsoft Corporation.  All rights reserved.


This C# file will be moved into "Models" folder later.
Because I have other PR which has the same file, in order to avoid conflict in the later merge, I add it in the root folder temporary.

xuzhg · 2017-11-02T01:09:57Z

src/Microsoft.OpenApi.Readers/YamlReaders/OpenApiV3Deserializer.cs

        public static PatternFieldMap<OpenApiInfo> InfoPatternFields = new PatternFieldMap<OpenApiInfo>
        {
-            { (s)=> s.StartsWith("x-"), (o,k,n)=> o.Extensions.Add(k, new OpenApiString(n.GetScalarValue())) }
+            { (s)=> s.StartsWith("x-"), (o,k,n)=> o.AddExtension(k, n.CreateAny()) }


I didn't change others in this PR, but will be do later

xuzhg · 2017-11-02T01:12:13Z

src/Microsoft.OpenApi.Readers/YamlReaders/ParseNodes/ValueNode.cs

+                return new OpenApiBoolean(false);
+            }
+
+            // TODO: add more codes to identify each primitive types


do you know the suitable way to identify each primitive type?

We can add this incrementally. I think we are on the right track.

xuzhg · 2017-11-02T01:13:29Z

test/Microsoft.OpenApi.Readers.Tests/Samples/CompleteHeader.yaml

+        name: Darrel Miller
+        url: http://www.bizcoder.com
+        email: [email protected]
+    x-list:


It looks YamlReader can't process "object" embedded in the "list". Do you know it's by design or not?

It's not by design. YAML spec supports this and YAML is technically a superset of JSON, so anything that works in JSON must be supported in YAML.

Yes. I tested it. Yaml reader supports the following:

{ "a": "a1", "b": [ "false", { "c": "c1" } ] }

"b" list has the "Boolean" and "Object" element

xuzhg · 2017-11-02T18:16:29Z

@PerthCharern @darrelmiller ping...

PerthCharern · 2017-11-02T20:28:37Z

src/Microsoft.OpenApi/ExtensionMethods.cs

+        /// <param name="extension">The extensible Open API element. </param>
+        /// <param name="name">The extension name.</param>
+        /// <param name="any">The extension value.</param>
+        public static void AddExtension<T>(this T extension, string name, IOpenApiAny any)


extension [](start = 50, length = 9)

call this element

PerthCharern · 2017-11-02T20:29:45Z

The change to .csproj file is missing

PerthCharern · 2017-11-02T20:32:08Z

src/Microsoft.OpenApi/ExtensionMethods.cs

+
+            if (!name.StartsWith(OpenApiConstants.OpenApiDocExtensionFieldNamePrefix))
+            {
+                throw new OpenApiException(SRResource.ExtensionFieldNameMustBeginWithXMinus);


ExtensionFieldNameMustBeginWithXMinus [](start = 54, length = 37)

I would call it either XDash or XHyphen.

PerthCharern · 2017-11-02T20:34:45Z

src/Microsoft.OpenApi.Readers/YamlReaders/ParseNodes/ParseNode.cs

+        /// <returns></returns>
+        public virtual IOpenApiAny CreateAny()
+        {
+            throw new NotImplementedException();


NotImplementedException [](start = 22, length = 23)

Technically NotSupportedException.

NotImplementedException is supposed to be for something temporary (i.e. sth you haven't implemented YET)

xuzhg · 2017-11-02T20:35:15Z

The change to .csproj file is missing

What do you mean?

PerthCharern · 2017-11-02T20:36:45Z

src/Microsoft.OpenApi.Readers/YamlReaders/ParseNodes/ValueNode.cs

+        {
+            string value = GetScalarValue();
+
+            if (String.IsNullOrWhiteSpace(value))


IsNullOrWhiteSpace [](start = 23, length = 18)

I think let's be conservative here, and only check null to get OpenApiNull.

White space might actually mean something...

PerthCharern · 2017-11-02T20:45:31Z

src/Microsoft.OpenApi.Readers/YamlReaders/OpenApiV3Deserializer.cs

        public static PatternFieldMap<OpenApiInfo> InfoPatternFields = new PatternFieldMap<OpenApiInfo>
        {
-            { (s)=> s.StartsWith("x-"), (o,k,n)=> o.Extensions.Add(k, new OpenApiString(n.GetScalarValue())) }
+            { (s)=> s.StartsWith("x-"), (o,k,n)=> o.AddExtension(k, n.CreateAny()) }


k [](start = 43, length = 1)

wondering what k stands for. Maybe p for property is better?

I will change it later

PerthCharern

PerthCharern · 2017-11-02T20:53:44Z

src/Microsoft.OpenApi/Properties/SRResource.Designer.cs


        /// <summary>
-        ///   Looks up a localized string similar to The filed name of extension doesn&apos;t begin with x-..
+        ///   Looks up a localized string similar to The filed name &apos;{0}&apos; of extension doesn&apos;t begin with x-..


'{0}' of extension doesn't [](start = 68, length = 42)

Does apostrophe cause issue in this file?

It's auto generated file.

Chore/upstream sync

msftclas added the cla-not-required label Nov 2, 2017

xuzhg requested review from PerthCharern and darrelmiller November 2, 2017 01:06

xuzhg mentioned this pull request Nov 2, 2017

Add a prototype extension methods to covert string to OpenApiAny object #27

Closed

xuzhg commented Nov 2, 2017

View reviewed changes

PerthCharern reviewed Nov 2, 2017

View reviewed changes

Make Open API Any reader works in the reader project

540c834

PerthCharern reviewed Nov 2, 2017

View reviewed changes

xuzhg added 2 commits November 2, 2017 13:41

resolve the comments to change parameter name and error message name

3a717f1

put the name into the error message for better understanding

6ff388e

PerthCharern reviewed Nov 2, 2017

View reviewed changes

PerthCharern approved these changes Nov 2, 2017

View reviewed changes

PerthCharern reviewed Nov 2, 2017

View reviewed changes

xuzhg merged commit b653b90 into microsoft:master Nov 2, 2017

xuzhg deleted the ReadAny branch November 2, 2017 21:08

baywet added a commit that referenced this pull request Nov 11, 2025

Merge pull request #54 from BinkyLabs/chore/upstream-sync

dcf7956

Chore/upstream sync

		@@ -0,0 +1,58 @@
		// ------------------------------------------------------------
		// Copyright (c) Microsoft Corporation. All rights reserved.

Comments

Conversation

xuzhg commented Nov 2, 2017

Uh oh!

msftclas commented Nov 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xuzhg commented Nov 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PerthCharern commented Nov 2, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xuzhg commented Nov 2, 2017

Uh oh!

PerthCharern Nov 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PerthCharern left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

PerthCharern Nov 2, 2017 •

edited

Loading