Add a transform method for schema #144

Roms1383 · 2019-01-20T03:22:57Z

Add a transform option to convert route.schema on documentation generation in dynamic mode.

Add transform options in opts to be able to convert schema properties if required

Update documentation for transform option

This version will be able to generate documentation if fastify-swagger merge the PR fastify/fastify-swagger#144 for the new transform option

Roms1383 · 2019-01-20T03:42:08Z

As a side note, I had the same errors locally as in travis-ci when running TypeScript tsc --project ./tsconfig.json the first time after install : I had to install @types/node to fix it.

Add mention about the mode and fix notes

mcollina · 2019-01-20T11:21:30Z

I’m not sure why, but can you add that as a devDependency?

We’ll need CI to pass to land this.

mcollina · 2019-01-20T11:22:13Z

Can you add a unit test? Also, can you update the typings?

Roms1383 · 2019-01-20T11:36:56Z

Done @mcollina, waiting for CI

Roms1383 · 2019-01-20T11:37:20Z

Ah, forgot the typings sorry...

mcollina · 2019-01-20T11:42:12Z

index.d.ts

+    /**
+     * Overwrite the route schema
+     * @default false
+     */


I don’t think the default is false. Typings also have tests.

I'm gonna fix it and currently looking at for the unit-tests

Where do you usually test dynamic mode ?

Sorry for being so long about the unit-tests, I'm not familiar with tap.
Also I don't know to which extend you need to test the property.

Roms1383 · 2019-01-20T13:18:06Z

Ok it should be good by now :)
I had to add the devDependencies for the unit-tests : if you think that's not ok, please provide me with feedback.

Roms1383 · 2019-01-20T15:25:43Z

As a side note I noticed that it doesn't play well with response but I don't know if it comes from Joi schema or Fastify internals. From what I remember, error got generated located in genResponse when trying to create description property.

Roms1383 · 2019-01-20T15:38:49Z

Anyway my guess is that the user who implements transform method has the responsibility to make it work and can even filter which one to transform or not so that shouldn't be blocking.

mcollina

LGTM

mcollina · 2019-01-21T09:59:27Z

This seems a pretty innocent feature to have. I don't really understand your points, would you mind opening an issue or a follow up PR?

Roms1383 · 2019-01-21T15:11:55Z

@mcollina Actually after testing and looking at documentation, to make the story short that's OK 😃

In details, my test was something like :

const params = ...
const body = ...
const querystring = ...
const response = {
  200: Joi.object().keys({ message: Joi.string().required() }).required()
}

Looking at fastify documentation I understand why adding a joi object for schema's response generate an issue : as stated here the response will anyway get used with fast-json-stringify which is the bug I got 👍

not ok Converting circular structure to JSON
    found:
      name: TypeError
      stack: |-
        TypeError: Converting circular structure to JSON
            at stringify (/Users/romain/Development/sandbox/fastify-swagger/node_modules/fast-json-stable-stringify/index.js:42:19)
            at stringify (/Users/romain/Development/sandbox/fastify-swagger/node_modules/fast-json-stable-stringify/index.js:50:25)
            at stringify (/Users/romain/Development/sandbox/fastify-swagger/node_modules/fast-json-stable-stringify/index.js:50:25)
            at module.exports (/Users/romain/Development/sandbox/fastify-swagger/node_modules/fast-json-stable-stringify/index.js:58:7)
            at Ajv._addSchema (/Users/romain/Development/sandbox/fastify-swagger/node_modules/ajv/lib/ajv.js:293:30)
            at Ajv.compile (/Users/romain/Development/sandbox/fastify-swagger/node_modules/ajv/lib/ajv.js:111:24)
            at isValidSchema (/Users/romain/Development/sandbox/fastify-swagger/node_modules/fast-json-stringify/index.js:917:7)
            at build (/Users/romain/Development/sandbox/fastify-swagger/node_modules/fast-json-stringify/index.js:39:3)
            at getValidatorForStatusCodeSchema (/Users/romain/Development/sandbox/fastify-swagger/node_modules/fastify/lib/validation.js:13:10)
            at /Users/romain/Development/sandbox/fastify-swagger/node_modules/fastify/lib/validation.js:19:21
            at Array.reduce (<anonymous>)
            at getResponseSchema (/Users/romain/Development/sandbox/fastify-swagger/node_modules/fastify/lib/validation.js:18:22)
            at build (/Users/romain/Development/sandbox/fastify-swagger/node_modules/fastify/lib/validation.js:53:31)
            at afterRouteAdded (/Users/romain/Development/sandbox/fastify-swagger/node_modules/fastify/fastify.js:636:9)
            at /Users/romain/Development/sandbox/fastify-swagger/node_modules/fastify/fastify.js:596:7
            at Object._encapsulateThreeParam (/Users/romain/Development/sandbox/fastify-swagger/node_modules/avvio/boot.js:389:7)
      message: Converting circular structure to JSON
    at:
      line: 62
      column: 7
      file: test/transform.js
      function: fastify.ready.err
    stack: |
      fastify.ready.err (test/transform.js:62:7)
      Object._encapsulateThreeParam (node_modules/avvio/boot.js:380:13)
      Boot.callWithCbOrNextTick (node_modules/avvio/boot.js:311:5)
      release (node_modules/fastq/queue.js:127:16)
      Object.resume (node_modules/fastq/queue.js:61:7)
      Boot.Plugin.loadPlugin.call (node_modules/avvio/boot.js:146:20)
      toLoad.finish (node_modules/avvio/plugin.js:169:7)
      done (node_modules/avvio/plugin.js:127:5)
      check (node_modules/avvio/plugin.js:138:7)
    source: |
      t.error(err)

Last but not least it's actually mentioned here that :

(...) maybe you want to change the validation library. Perhaps you like Joi. In this case, you can use it to validate the url parameters, body, and query string!

And there's no mention of validating schema's response, so it makes sense.
Thanks !

* Add transform in opts Add transform options in opts to be able to convert schema properties if required * Update documentation Update documentation for transform option * Lint code * Update documentation Add mention about the mode and fix notes * Add @types/node as a dev dependency * Update typings * Remove wrong default * Add basic unit-test * Fix unit-tests

This version will be able to generate documentation if fastify-swagger merge the PR fastify/fastify-swagger#144 for the new transform option

Roms1383 added 2 commits January 19, 2019 21:02

Add transform in opts

332f9c1

Add transform options in opts to be able to convert schema properties if required

Update documentation

b7bbf21

Update documentation for transform option

Roms1383 mentioned this pull request Jan 20, 2019

Add a transform method for schema #143

Closed

Roms1383 added a commit to Roms1383/robust-database that referenced this pull request Jan 20, 2019

✨ Generate documentation

aa3a727

This version will be able to generate documentation if fastify-swagger merge the PR fastify/fastify-swagger#144 for the new transform option

Lint code

c11e316

Update documentation

9a276a0

Add mention about the mode and fix notes

Add @types/node as a dev dependency

614f2e8

Update typings

d6bacde

mcollina reviewed Jan 20, 2019

View reviewed changes

Roms1383 added 3 commits January 20, 2019 18:46

Remove wrong default

1d806b6

Add basic unit-test

bc69735

Fix unit-tests

31f25a5

mcollina approved these changes Jan 21, 2019

View reviewed changes

mcollina merged commit 20aab51 into fastify:master Jan 21, 2019

Roms1383 mentioned this pull request Jan 21, 2019

Plugin not working with JOI #32

Closed

Roms1383 added a commit to Roms1383/robust-database that referenced this pull request Feb 8, 2019

✨ Generate documentation

a1f4f1b

This version will be able to generate documentation if fastify-swagger merge the PR fastify/fastify-swagger#144 for the new transform option

alexanderniebuhr mentioned this pull request Mar 3, 2021

fix: joi transform #374

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a transform method for schema #144

Add a transform method for schema #144

Roms1383 commented Jan 20, 2019

Roms1383 commented Jan 20, 2019

mcollina commented Jan 20, 2019

mcollina commented Jan 20, 2019

Roms1383 commented Jan 20, 2019

Roms1383 commented Jan 20, 2019

mcollina Jan 20, 2019

Roms1383 Jan 20, 2019 •

edited

Roms1383 Jan 20, 2019

Roms1383 Jan 20, 2019

Roms1383 commented Jan 20, 2019

Roms1383 commented Jan 20, 2019

Roms1383 commented Jan 20, 2019

mcollina left a comment

mcollina commented Jan 21, 2019

Roms1383 commented Jan 21, 2019

Add a transform method for schema #144

Add a transform method for schema #144

Conversation

Roms1383 commented Jan 20, 2019

Roms1383 commented Jan 20, 2019

mcollina commented Jan 20, 2019

mcollina commented Jan 20, 2019

Roms1383 commented Jan 20, 2019

Roms1383 commented Jan 20, 2019

mcollina Jan 20, 2019

Choose a reason for hiding this comment

Roms1383 Jan 20, 2019 • edited

Choose a reason for hiding this comment

Roms1383 Jan 20, 2019

Choose a reason for hiding this comment

Roms1383 Jan 20, 2019

Choose a reason for hiding this comment

Roms1383 commented Jan 20, 2019

Roms1383 commented Jan 20, 2019

Roms1383 commented Jan 20, 2019

mcollina left a comment

Choose a reason for hiding this comment

mcollina commented Jan 21, 2019

Roms1383 commented Jan 21, 2019

Roms1383 Jan 20, 2019 •

edited