Add DecimalTypeObject to test-integration (#143) #237

meanstigerkim · 2021-09-11T14:26:11Z

Decimal Type was tested.
Is it right to do it this way?
It comes up as a double type with json.

ghost · 2021-09-11T14:26:23Z

All CLA requirements met.

justinyoo

@minbumm Thanks for your PR!

I've left a few comments with regards to mainly to take care of the request payload. In addition to that, I was wondering where your test codes are. Would you please add it?

justinyoo · 2021-09-14T03:33:05Z

...soft.Azure.WebJobs.Extensions.OpenApi.TestApp/Get_ApplicationJson_DecimalType_HttpTrigger.cs

+
+namespace Microsoft.Azure.WebJobs.Extensions.OpenApi.TestApp
+{
+    public class Get_ApplicationJson_DecimalType_HttpTrigger


Can you change both file name and class name to Post_ApplicationJson_DecimalType_HttpTrigger?

justinyoo · 2021-09-14T03:33:30Z

...soft.Azure.WebJobs.Extensions.OpenApi.TestApp/Get_ApplicationJson_DecimalType_HttpTrigger.cs

+    {
+        [FunctionName(nameof(Get_ApplicationJson_DecimalType_HttpTrigger))]
+        [OpenApiOperation(operationId: nameof(Get_ApplicationJson_DecimalType_HttpTrigger.Get_ApplicationJson_DecimalType), tags: new[] { "dataType" })]
+        [OpenApiResponseWithBody(statusCode: HttpStatusCode.OK, contentType: "application/json", bodyType: typeof(DecimalTypeObjectModel), Description = "The OK response")]


We would need the OpenApiRequestBody(...) attribute for the request payload.

...soft.Azure.WebJobs.Extensions.OpenApi.TestApp/Get_ApplicationJson_DecimalType_HttpTrigger.cs

justinyoo · 2021-09-14T03:39:14Z

Decimal Type was tested.
Is it right to do it this way?
It comes up as a double type with json.

decimal data type is not declared on the spec, it's considered as number with the format of double.

meanstigerkim · 2021-09-17T06:44:15Z

Add Model : test-integration2/TestApps/Models/DecimalTypeObjectModel.cs
Add HttpTrigger to test : test-integration2/TestApps/Post_ApplicationJson_DecimalType_HttpTrigger.cs
I add HttpTrigger about Decimal Type to the TestApp.

…ntTypeSchema -> modified(propertyType ->propertyFormat)

Deserve82

I wrote some reviews, please check your http trigger app with these reviews.

Deserve82 · 2021-09-23T14:40:00Z

...oft.Azure.WebJobs.Extensions.OpenApi.TestApp/Post_ApplicationJson_DecimalType_HttpTrigger.cs

@@ -18,6 +18,7 @@ public class Post_ApplicationJson_DecimalType_HttpTrigger
    {
        [FunctionName(nameof(Post_ApplicationJson_DecimalType_HttpTrigger))]
        [OpenApiOperation(operationId: nameof(Post_ApplicationJson_DecimalType_HttpTrigger.Post_ApplicationJson_DecimalType), tags: new[] { "demical" })]
+        [OpenApiRequestBody(contentType: "application/octet-stream", bodyType: typeof(decimal), Required = true, Description = "The OK response")]


contentType does not match with your test DataRow.
I think contentType should be "text/plain", not application/octet-stream

Thanks, I'll give it a try

Deserve82 · 2021-09-23T14:44:59Z

...crosoft.Azure.WebJobs.Extensions.OpenApi.Document.Tests/Post_AppicationJson_Decimal_Tests.cs

+        }
+
+        [DataTestMethod]
+        [DataRow("/post-applicationjson-decimal", "post", "text/plain", "number", "decimal")]


Here is a problem. Now data row has "decimal" value in propertyFormat, but the format of your app does not have any "decimal" format. I think it's better to change "decimal" to "double"

Thanks, I'll give it a try

Deserve82 · 2021-09-23T14:48:15Z

...crosoft.Azure.WebJobs.Extensions.OpenApi.Document.Tests/Post_AppicationJson_Decimal_Tests.cs

+        }
+
+        [DataTestMethod]
+        [DataRow("/post-applicationjson-decimal", "post", "200", "application/json", "decimalObjectModel")]


Same problem with "decimalObjectModel". you wrote your model type name "decimalTypeObjectModel". Please change this.

thanks i fixed it

Deserve82 · 2021-09-23T14:48:40Z

...crosoft.Azure.WebJobs.Extensions.OpenApi.Document.Tests/Post_AppicationJson_Decimal_Tests.cs

+        }
+
+        [DataTestMethod]
+        [DataRow("decimaleObjectModel", "object")]


Same problem with "decimalObjectModel". you wrote your model type name "decimalTypeObjectModel". Please change this.

thanks i fixed it

Deserve82 · 2021-09-23T14:49:37Z

...crosoft.Azure.WebJobs.Extensions.OpenApi.Document.Tests/Post_AppicationJson_Decimal_Tests.cs

+        }
+
+        [DataTestMethod]
+        [DataRow("decimalObjectModel", "decimalValue", "number", "decimal")]


Same problem with "decimalObjectModel". you wrote your model type name "decimalTypeObjectModel". Please change this.

thanks i fixed it

Given_OpenApiDocument_Then_It_Should_Return_ComponentSchemaProperty ->propertyFormat

…m/azure-functions-openapi-extension into feature/test-integration2

Given_OpenApiDocument_Then_It_Should_Return_ComponentSchemaProperty propertyFormat decimal -> double

)

Azure#247)

this is integration tests for string visitor type.

…#243)

This reverts commit af42aaf.

meanstigerkim · 2021-10-05T02:05:58Z

I made the mistake of doing the revert.

…#237

Add DecimalTypeObject to test-integration

2dc3505

meanstigerkim changed the title ~~Add DecimalTypeObject to test-integration #143~~ Add DecimalTypeObject to test-integration (#143) Sep 11, 2021

This was referenced Sep 11, 2021

Add DoubleObject to test-integration (#143) #238

Closed

Add DoubleObject to test-integration (#143) #239

Merged

justinyoo added the oca-pr label Sep 14, 2021

justinyoo requested changes Sep 14, 2021

View reviewed changes

justinyoo added the enhancement New feature or request label Sep 14, 2021

meanstigerkim added 2 commits September 17, 2021 14:10

GET->POST Chage

c530163

Add OpenApiRequestBody

2a550b1

github-actions bot added the oca-issue label Sep 17, 2021

Given_OpenApiDocument_Then_It_Should_Return_OperationRequestBodyConte…

5b921c6

…ntTypeSchema -> modified(propertyType ->propertyFormat)

meanstigerkim marked this pull request as draft September 23, 2021 00:55

meanstigerkim marked this pull request as ready for review September 23, 2021 01:32

Deserve82 suggested changes Sep 23, 2021

View reviewed changes

meanstigerkim and others added 13 commits September 24, 2021 07:51

I modified it based on the advice I got from Git from Deserve82

9900771

fix bug

aad319e

Given_OpenApiDocument_Then_It_Should_Return_ComponentSchemaProperty ->propertyFormat

fix bug(text/plain) -> application/octet-stream

ec859f8

fix bug(text/plain) -> application/octet-stream

3b6f60f

Merge branch 'feature/test-integration2' of https://github.com/minbum…

fa45f53

…m/azure-functions-openapi-extension into feature/test-integration2

Merge branch 'feature/test-integration2' of https://github.com/minbum…

b70e1fb

…m/azure-functions-openapi-extension into feature/test-integration2

Merge branch 'feature/test-integration2' of https://github.com/minbum…

32a7d4d

…m/azure-functions-openapi-extension into feature/test-integration2

fix bug

ea84da4

Given_OpenApiDocument_Then_It_Should_Return_ComponentSchemaProperty propertyFormat decimal -> double

fixed that fails to use a model with a property named Aliases (Azure#218

503462d

)

Feature/Azure#156 list of recursive models (Azure#233)

b2596d5

Add integration testing for Boolean Object Type (Azure#143) (Azure#236)

5e5acff

Support "Base Object (root of all classes)" type (Azure#234)

f862a32

Tidy up files

3a96a38

cjakobsen and others added 17 commits October 5, 2021 10:20

Fix: Change links to moved docs (Azure#242)

09cb79b

Update docs

bb7319a

Add DoubleObject to test-integration (Azure#143) (Azure#239)

e01dd8a

Update type extension methods to detect arrays and dictionaries better (

3e6dd62

Azure#247)

Update issue templates

7ecff10

Add issue template

f857a5b

Change the issue template name

cebbd7a

Add PR template

2135d96

Add integration testing for String Object Type (Azure#250)

8b9d84b

this is integration tests for string visitor type.

Feature/Azure#139 OpenAPI query string/path parameter examples (Azure…

0f625c7

…#243)

HTTP/HTTPS in Swagger UI + oauth2+redirect.html (Azure#253)

e612070

Fix "could not resolve reference error" Azure#160 (Azure#246)

39820ca

Add authorisation feature to OpenApiHttpTriggerContext (Azure#255)

ef9169c

Add integration testing for jObject Type (Azure#252)

2180949

Formatting

7db0d2f

Update docs for 0.9.0-preview (Azure#256)

af42aaf

Revert "Update docs for 0.9.0-preview (Azure#256)"

6dc3cb6

This reverts commit af42aaf.

meanstigerkim closed this Oct 5, 2021

meanstigerkim pushed a commit to meanstigerkim/azure-functions-openapi-extension that referenced this pull request Oct 5, 2021

rewrite - Add DecimalTypeObject to test-integration (Azure#143) Azure…

72c9227

…#237

justinyoo added invalid and removed enhancement New feature or request labels Oct 16, 2021

Add DecimalTypeObject to test-integration (#143) #237

Add DecimalTypeObject to test-integration (#143) #237

Uh oh!

Conversation

meanstigerkim commented Sep 11, 2021

Uh oh!

ghost commented Sep 11, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

justinyoo 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!

Uh oh!

justinyoo commented Sep 14, 2021

Uh oh!

meanstigerkim commented Sep 17, 2021

Uh oh!

Deserve82 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!

Deserve82 Sep 23, 2021 • 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!

Deserve82 Sep 23, 2021 • 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!

Deserve82 Sep 23, 2021 • 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!

Deserve82 Sep 23, 2021 • 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!

meanstigerkim commented Oct 5, 2021

Uh oh!

Uh oh!

ghost commented Sep 11, 2021 •

edited by ghost

Loading

Deserve82 Sep 23, 2021 •

edited

Loading

Deserve82 Sep 23, 2021 •

edited

Loading

Deserve82 Sep 23, 2021 •

edited

Loading

Deserve82 Sep 23, 2021 •

edited

Loading