Tsed: A String BodyParams

[tiran1984]

Information

• Version: 5.59.1
• Packages: swagger

With the method with a simple string as BodyParams, swagger will create a Model for the string variable. And I mean it should not generate a Model

Example

@Controller('/')
class MyController {
    @ContentType('text/plain')
    @Post("/insert")
    async csv(@BodyParams() csvLines: string): Promise<string> {
       ....
    }
}

Output json:

{
   "in": "body",
   "name": "body",
    "schema": {
        "type": "object",
        "properties":{}
    } 
}

Expected:

{
   "in": "body",
   "name": "body",
    "schema": {
        "type": "string"
    } 
}

[Romakita]

Hello @tiran1984
Thanks for the issue. If somebody want to fix this issue and doing an easy first PR, your are welcome. I'm currently focused on v6 release and I would like to have new contributors on this project :)

[Romakita]

Hello,

The problem is probably located here: https://github.com/TypedProject/tsed/blob/production/packages/swagger/src/class/OpenApiParamsBuilder.ts#L76

How to reproduce

In the unit test file (https://github.com/TypedProject/tsed/blob/production/packages/swagger/src/class/OpenApiParamsBuilder.spec.ts#L886), add the following code:

it("should create a swagger schema from BodyParams with String type", () => {
       // GIVEN
      class MyCtrl {
        test(@BodyParams() test: string) {}
      }

      // WHEN
      const builder = new OpenApiParamsBuilder(MyCtrl, "test").build();

      // THEN
      expect(builder.parameters).to.deep.eq([
        {
          "in": "body",
          "name": "body",
          "schema": {
            "type": "string"
          } 
       }
     ]);
});

Then run test with yarn test (or with you IDE) and see the difference.

Add log on this method here: https://github.com/TypedProject/tsed/blob/production/packages/swagger/src/class/OpenApiParamsBuilder.ts#L76 and debug ;)

See you

[rluvaton]

@Romakita I will love to contribute

[Romakita]

Hello @rluvaton
Your contribution is welcome :)

If you have any question tell me. This issue is here for new contributors. If you have any question tell me :)

Don’t forgot to assign you this issue :)

[rluvaton]

@Romakita I can't assign myself to issues 😬

[Romakita]

Ha yes....my bad ^^

[rluvaton]

Hello,

The problem is probably located here: https://github.com/TypedProject/tsed/blob/production/packages/swagger/src/class/OpenApiParamsBuilder.ts#L76

How to reproduce

In the unit test file (https://github.com/TypedProject/tsed/blob/production/packages/swagger/src/class/OpenApiParamsBuilder.spec.ts#L886), add the following code:

it("should create a swagger schema from BodyParams with String type", () => {
       // GIVEN
      class MyCtrl {
        test(@BodyParams() test: string) {}
      }

      // WHEN
      const builder = new OpenApiParamsBuilder(MyCtrl, "test").build();

      // THEN
      expect(builder.parameters).to.deep.eq([
        {
          "in": "body",
          "name": "body",
          "schema": {
            "type": "string"
          } 
       }
     ]);
});

Then run test with yarn test (or with you IDE) and see the difference.

Add log on this method here: https://github.com/TypedProject/tsed/blob/production/packages/swagger/src/class/OpenApiParamsBuilder.ts#L76 and debug ;)

See you

@Romakita Wow, this is the most helpful comment to help new contributors, thank you for that!

[Romakita]

@rluvaton your welcome ;)
I hope the debugging will be easy also ;)

[rluvaton]

@Romakita from which branch should I branch out? master or production?

[Romakita]

Production :)

[rluvaton]

@Romakita
I've got the diff

 [
         {
-          "description": ""
           "in": "body"
           "name": "body"
-          "required": false
           "schema": {
             "type": "string"
           }
         }

Should description be empty and required be false?

---

The code seems to work fine by the output of the test, I even used the problematic controller example 😬

it("should create a swagger schema from BodyParams with String type", () => {

      @Controller("/")
      class MyController {
        @ContentType("text/plain")
        @Post("/insert")
        async csv(@BodyParams() csvLines: string): Promise<string> {
          return "";
        }
      }
      const builder = new OpenApiParamsBuilder(MyController, "csv").build();

      expect(builder.parameters).to.deep.eq([
        {
          "in": "body",
          "name": "body",
          "schema": {
            "type": "string"
          }
        }
      ]);
    });

I will try another approach

[Romakita]

... Strange. Maybe the problem isn't at this level.

Ok can you create an integration test in this file: https://github.com/TypedProject/tsed/blob/production/packages/swagger/test/swagger.integration.spec.ts

Take as example any other integration test and implement the same scenario. I the problem doesn't occurs it means it's already solved by another commit :(

[Romakita]

Yes description and required is ok :)

[rluvaton]

Fixed already, can be closed

[Romakita]

Ok @rluvaton
if you have written unit test and integration test, you can push the branch and create PR :)
Sorry for the wasted time.
Romain

[rluvaton]

Ok @rluvaton
if you have written unit test and integration test, you can push the branch and create PR :)
Sorry for the wasted time.
Romain

@Romakita 👍, just a question, in the integration tests it's complaining (failing) about not adding new definition which I think it's useless, is it?

Want me to add after Calender here:
https://github.com/TypedProject/tsed/blob/2dec44625f078c4b23b32e5ec2d2023dcc6bd017/packages/swagger/test/swagger.integration.spec.ts#L88-L101

this:

               "name"
             ]
             "type": "object"
           },
-          "String": {
-            "properties": {},
-            "type": "object",
-          }
         }

for this new controller (which is one of CalendarsController.children):

@Controller("/csv")
class CsvCtrl {
  @ContentType("text/plain")
  @Post("/insert")
  async csv(@BodyParams() csvLines: string): Promise<string> {
    return "";
  }
}

Is this right? should there be this String definition?

[Romakita]

The result is not correct. String is a basic swagger type and shouldn’t generate a new entry in definitions field. It means, there is a bug somewhere in the swagger package for this scenario.

[rluvaton]

The result is not correct. String is a basic swagger type and shouldn’t generate a new entry in definitions field. It means, there is a bug somewhere in the swagger package for this scenario.

It's pretty strange because it creating it but doesn't use it

I'll try to fix it

[rluvaton]

@Romakita Is this better?

{
  "consumes": [
    "application/json"
  ]
  "definitions": {
    "Calendar": {
      "properties": {
        "id": {
          "type": "string"
        }
        "name": {
          "type": "string"
        }
      }
      "required": [
        "name"
      ]
      "type": "object"
    },
+  "CsvCtrlCsvPayload": {
+    "properties": {
+     "csvLines": {
+       "type": "string"
+     }
+    }
+    "type": "object"
+   }
  }
  "info": {
    "description": ""
    "termsOfService": ""
    "title": "Api documentation"
    "version": "1.0.0"
  }
  "paths": {
    ...
    "/rest/calendars/csv/insert": {
      "post": {
        "operationId": "CsvCtrl.csv"
        "parameters": [
          {
            "in": "body"
            "name": "body"
            "required": false
            "schema": {
+             "$ref": "#/definitions/CsvCtrlCsvPayload"
            }
          }
        ]
        "produces": [
          "text/plain"
        ]
        "responses": {
          "200": {
            "description": "Success"
          }
        }
        "tags": [
          "CsvCtrl"
        ]
      }
    }
    ...
  }
  "produces": [
    "application/json"
  ]
  "securityDefinitions": {}
  "swagger": "2.0"
  "tags": [
    {
      "name": "CalendarsController"
    }
    {
      "name": "CsvCtrl"
    }
    {
      "name": "EventCtrl"
    }
  ]
}

I've added "csvLines" to BodyParams arguement

@Controller("/csv")
class CsvCtrl {
  @ContentType("text/plain")
  @Post("/insert")
  async csv(@BodyParams("csvLines") csvLines: string): Promise<string> {
    return "";
  }
}

[Romakita]

Hello @rluvaton

This code already works and is widely covered (this is one of the basic usage of the framework):

@ContentType("text/plain")
  @Post("/insert")
  async csv(@BodyParams("csvLines") csvLines: string): Promise<string> {
    return "";
  }

But the bug occurs when the consumer send a string payload:

@ContentType("text/plain")
  @Post("/insert")
  async csv(@BodyParams() csvLines: string): Promise<string> {
    return "";
  }

In swagger.integration.spec.ts I added the following:

class CalendarsController {
  @Get("/:id")
  @Returns(200, {type: Calendar})
  async get(@PathParams("id") id: string): Promise<Calendar> {
    return new Calendar({id, name: "test"});
  }

  @Get("/")
  @ReturnsArray(200, {type: Calendar})
  async getAll(): Promise<Calendar[]> {
    return [new Calendar({id: 1, name: "name"}), new Calendar({id: 2, name: "name"})];
  }

+  @ContentType("text/plain")
+  @Post("/csv")
+  async csv(@BodyParams() csvLines: string): Promise<string> {
+    return "";
+  }
}

The expected result is:

{
      swagger: "2.0",
      tags: [
        {
          name: "CalendarsController"
        },
        {
          name: "EventCtrl"
        }
      ],
      consumes: ["application/json"],
      definitions: {
        Calendar: {
          properties: {
            id: {
              type: "string"
            },
            name: {
              type: "string"
            }
          },
          required: ["name"],
          type: "object"
        }
      },
      info: {
        description: "",
        termsOfService: "",
        title: "Api documentation",
        version: "1.0.0"
      },
      paths: {
        "/rest/calendars": {
          get: {
            operationId: "CalendarsController.getAll",
            responses: {
              "200": {
                description: "Success",
                schema: {
                  items: {
                    $ref: "#/definitions/Calendar"
                  },
                  type: "array"
                }
              }
            },
            tags: ["CalendarsController"]
          }
        },
        "/rest/calendars/events": {
          get: {
            description: "Events",
            operationId: "EventCtrl.get",
            responses: {
              "200": {
                description: "Success"
              }
            },
            tags: ["EventCtrl"]
          }
        },
        "/rest/calendars/{id}": {
          get: {
            operationId: "CalendarsController.get",
            parameters: [
              {
                in: "path",
                name: "id",
                required: true,
                type: "string"
              }
            ],
            responses: {
              "200": {
                description: "Success",
                schema: {
                  $ref: "#/definitions/Calendar"
                }
              }
            },
            tags: ["CalendarsController"]
          }
        },
+        "/rest/calendars/csv": {
+          "post": {
+            "operationId": "CalendarsController.csv",
+            "parameters": [
+              {
+                "in": "body",
+                "name": "body",
+                "required": false,
+                "schema": {
+                  "type": "string"
+                }
+              }
+            ],
+            "produces": [
+              "text/plain"
+            ],
+            "responses": {
+              "200": {
+                "description": "Success"
+              }
+            },
+           "tags": [
+              "CalendarsController"
+            ]
+          }
        }
      },
      produces: ["application/json"],
      securityDefinitions: {}
    }

But as you mentioned, the definitions field contain an unexpected String class:

-   "String": {
-            "properties": {},
-            "type": "object",
-          }

To fix it, you have to ignore all primitive types inserted in definitions field by using isPrimitive function from @tsed/core:

https://github.com/TypedProject/tsed/blob/production/packages/swagger/src/class/OpenApiModelSchemaBuilder.ts#L72

It should solve the issue :)

See you and sorry for the wasted time!
Romain

[rluvaton]

In swagger.integration.spec.ts I added the following:

In which branch you added?

[Romakita]

Sorry, I'd just tested locally to reproduce the issue and give you more details to solve the problem :)

[rluvaton]

Sorry, I'd just tested locally to reproduce the issue and give you more details to solve the problem :)

Same, I think I fixed it, I'll commit, push and create a PR

@Romakita What the schema output should be for this function? it shouldn't be possible, right? cause it only gets one variable (because the text/plain content type)

  @ContentType("text/plain")
  @Post("/csv")
  async csv(@BodyParams() csvLines: string, @BodyParams() cal: Calendar): Promise<string> {
    return "";
  }

[Romakita]

:tada: This issue has been resolved in version 5.65.6 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

[Romakita]

:tada: This issue has been resolved in version 6.0.0-beta.11 :tada:

The release is available on:

• v6.0.0-beta.11
• GitHub release

Your semantic-release bot :package::rocket: