New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Documentation and TS Types missed for FastifyInstance#setSchemaController #3480
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
cc @fastify/typescript |
types/instance.d.ts
Outdated
/** | ||
* Set the schema controller for all routes. | ||
*/ | ||
setSchemaController(schemaControllerOpts: (payload: unknown, statusCode: number) => void): FastifyInstance<RawServer, RawRequest, RawReply, Logger>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure the type here is correct?
fastify/test/schema-feature.test.js
Lines 1242 to 1606 in 1c8e4ed
test('setSchemaController per instance', t => { | |
t.plan(7) | |
const fastify = Fastify({}) | |
fastify.register(async (instance1) => { | |
instance1.setSchemaController({ | |
bucket: function factoryBucket (storeInit) { | |
t.pass('instance1 has created the bucket') | |
return { | |
add (schema) { t.fail('add is not called') }, | |
getSchema (id) { t.fail('getSchema is not called') }, | |
getSchemas () { t.fail('getSchemas is not called') } | |
} | |
} | |
}) | |
}) | |
fastify.register(async (instance2) => { | |
const bSchema = { $id: 'b', type: 'string' } | |
instance2.setSchemaController({ | |
bucket: function factoryBucket (storeInit) { | |
t.pass('instance2 has created the bucket') | |
const map = {} | |
return { | |
add (schema) { | |
t.equal(schema.$id, bSchema.$id, 'add is called') | |
map[schema.$id] = schema | |
}, | |
getSchema (id) { | |
t.pass('getSchema is called') | |
return map[id] | |
}, | |
getSchemas () { | |
t.pass('getSchemas is called') | |
} | |
} | |
} | |
}) | |
instance2.addSchema(bSchema) | |
instance2.addHook('onReady', function (done) { | |
instance2.getSchemas() | |
t.same(instance2.getSchema('b'), bSchema, 'the schema are loaded') | |
done() | |
}) | |
}) | |
fastify.ready(err => { t.error(err) }) | |
}) | |
test('setSchemaController: Inherits correctly parent schemas with a customized validator instance', async t => { | |
t.plan(5) | |
const customAjv = new Ajv({ coerceTypes: false }) | |
const server = Fastify() | |
const someSchema = { | |
$id: 'some', | |
type: 'array', | |
items: { | |
type: 'string' | |
} | |
} | |
const errorResponseSchema = { | |
$id: 'error_response', | |
type: 'object', | |
properties: { | |
statusCode: { | |
type: 'integer' | |
}, | |
message: { | |
type: 'string' | |
} | |
} | |
} | |
server.addSchema(someSchema) | |
server.addSchema(errorResponseSchema) | |
server.register((instance, _, done) => { | |
instance.setSchemaController({ | |
compilersFactory: { | |
buildValidator: function (externalSchemas) { | |
const schemaKeys = Object.keys(externalSchemas) | |
t.equal(schemaKeys.length, 2, 'Contains same number of schemas') | |
t.hasStrict([someSchema, errorResponseSchema], Object.values(externalSchemas), 'Contains expected schemas') | |
for (const key of schemaKeys) { | |
if (customAjv.getSchema(key) == null) { | |
customAjv.addSchema(externalSchemas[key], key) | |
} | |
} | |
return function validatorCompiler ({ schema }) { | |
return customAjv.compile(schema) | |
} | |
} | |
} | |
}) | |
instance.get( | |
'/', | |
{ | |
schema: { | |
querystring: { | |
msg: { | |
$ref: 'some#' | |
} | |
}, | |
response: { | |
'4xx': { | |
$ref: 'error_response#' | |
} | |
} | |
} | |
}, | |
(req, reply) => { | |
reply.send({ noop: 'noop' }) | |
} | |
) | |
done() | |
}) | |
const res = await server.inject({ | |
method: 'GET', | |
url: '/', | |
query: { | |
msg: 'string' | |
} | |
}) | |
const json = res.json() | |
t.equal(json.message, 'querystring.msg should be array') | |
t.equal(json.statusCode, 400) | |
t.equal(res.statusCode, 400, 'Should not coearce the string into array') | |
}) | |
test('setSchemaController: Inherits buildSerializer from parent if not present within the instance', async t => { | |
t.plan(6) | |
const customAjv = new Ajv({ coerceTypes: false }) | |
const someSchema = { | |
$id: 'some', | |
type: 'array', | |
items: { | |
type: 'string' | |
} | |
} | |
const errorResponseSchema = { | |
$id: 'error_response', | |
type: 'object', | |
properties: { | |
statusCode: { | |
type: 'integer' | |
}, | |
message: { | |
type: 'string' | |
} | |
} | |
} | |
let rootSerializerCalled = 0 | |
let rootValidatorCalled = 0 | |
let childValidatorCalled = 0 | |
const rootBuildSerializer = function (externalSchemas) { | |
rootSerializerCalled++ | |
return function serializer () { | |
return data => { | |
return JSON.stringify({ | |
statusCode: data.statusCode, | |
message: data.message | |
}) | |
} | |
} | |
} | |
const rootBuildValidator = function (externalSchemas) { | |
rootValidatorCalled++ | |
return function validatorCompiler ({ schema }) { | |
return customAjv.compile(schema) | |
} | |
} | |
const server = Fastify({ | |
schemaController: { | |
compilersFactory: { | |
buildValidator: rootBuildValidator, | |
buildSerializer: rootBuildSerializer | |
} | |
} | |
}) | |
server.addSchema(someSchema) | |
server.addSchema(errorResponseSchema) | |
server.register((instance, _, done) => { | |
instance.setSchemaController({ | |
compilersFactory: { | |
buildValidator: function (externalSchemas) { | |
childValidatorCalled++ | |
const schemaKeys = Object.keys(externalSchemas) | |
for (const key of schemaKeys) { | |
if (customAjv.getSchema(key) == null) { | |
customAjv.addSchema(externalSchemas[key], key) | |
} | |
} | |
return function validatorCompiler ({ schema }) { | |
return customAjv.compile(schema) | |
} | |
} | |
} | |
}) | |
instance.get( | |
'/', | |
{ | |
schema: { | |
querystring: { | |
msg: { | |
$ref: 'some#' | |
} | |
}, | |
response: { | |
'4xx': { | |
$ref: 'error_response#' | |
} | |
} | |
} | |
}, | |
(req, reply) => { | |
reply.send({ noop: 'noop' }) | |
} | |
) | |
done() | |
}) | |
const res = await server.inject({ | |
method: 'GET', | |
url: '/', | |
query: { | |
msg: 'string' | |
} | |
}) | |
const json = res.json() | |
t.equal(json.statusCode, 400) | |
t.equal(json.message, 'querystring.msg should be array') | |
t.equal(rootSerializerCalled, 1, 'Should be called from the child') | |
t.equal(rootValidatorCalled, 0, 'Should not be called from the child') | |
t.equal(childValidatorCalled, 1, 'Should be called from the child') | |
t.equal(res.statusCode, 400, 'Should not coerce the string into array') | |
}) | |
test('setSchemaController: Inherits buildValidator from parent if not present within the instance', async t => { | |
t.plan(6) | |
const customAjv = new Ajv({ coerceTypes: false }) | |
const someSchema = { | |
$id: 'some', | |
type: 'array', | |
items: { | |
type: 'string' | |
} | |
} | |
const errorResponseSchema = { | |
$id: 'error_response', | |
type: 'object', | |
properties: { | |
statusCode: { | |
type: 'integer' | |
}, | |
message: { | |
type: 'string' | |
} | |
} | |
} | |
let rootSerializerCalled = 0 | |
let rootValidatorCalled = 0 | |
let childSerializerCalled = 0 | |
const rootBuildSerializer = function (externalSchemas) { | |
rootSerializerCalled++ | |
return function serializer () { | |
return data => JSON.stringify(data) | |
} | |
} | |
const rootBuildValidator = function (externalSchemas) { | |
rootValidatorCalled++ | |
const schemaKeys = Object.keys(externalSchemas) | |
for (const key of schemaKeys) { | |
if (customAjv.getSchema(key) == null) { | |
customAjv.addSchema(externalSchemas[key], key) | |
} | |
} | |
return function validatorCompiler ({ schema }) { | |
return customAjv.compile(schema) | |
} | |
} | |
const server = Fastify({ | |
schemaController: { | |
compilersFactory: { | |
buildValidator: rootBuildValidator, | |
buildSerializer: rootBuildSerializer | |
} | |
} | |
}) | |
server.register((instance, _, done) => { | |
instance.register((subInstance, _, subDone) => { | |
subInstance.setSchemaController({ | |
compilersFactory: { | |
buildSerializer: function (externalSchemas) { | |
childSerializerCalled++ | |
return function serializerCompiler () { | |
return data => { | |
return JSON.stringify({ | |
statusCode: data.statusCode, | |
message: data.message | |
}) | |
} | |
} | |
} | |
} | |
}) | |
subInstance.get( | |
'/', | |
{ | |
schema: { | |
querystring: { | |
msg: { | |
$ref: 'some#' | |
} | |
}, | |
response: { | |
'4xx': { | |
$ref: 'error_response#' | |
} | |
} | |
} | |
}, | |
(req, reply) => { | |
reply.send({ noop: 'noop' }) | |
} | |
) | |
subDone() | |
}) | |
done() | |
}) | |
server.addSchema(someSchema) | |
server.addSchema(errorResponseSchema) | |
const res = await server.inject({ | |
method: 'GET', | |
url: '/', | |
query: { | |
msg: ['string'] | |
} | |
}) | |
const json = res.json() | |
t.equal(json.statusCode, 400) | |
t.equal(json.message, 'querystring.msg should be array') | |
t.equal(rootSerializerCalled, 0, 'Should be called from the child') | |
t.equal(rootValidatorCalled, 1, 'Should not be called from the child') | |
t.equal(childSerializerCalled, 1, 'Should be called from the child') | |
t.equal(res.statusCode, 400, 'Should not coearce the string into array') | |
}) |
I think it should be something similar to
Lines 133 to 143 in 0439d5f
schemaController?: { | |
bucket?: (parentSchemas?: unknown) => { | |
addSchema(schema: unknown): FastifyInstance; | |
getSchema(schemaId: string): unknown; | |
getSchemas(): Record<string, unknown>; | |
}; | |
compilersFactory?: { | |
buildValidator?: ValidatorCompiler; | |
buildSerializer?: (externalSchemas: unknown, serializerOptsServerOption: FastifyServerOptions["serializerOpts"]) => FastifySerializerCompiler<unknown>; | |
}; | |
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was in doubt with the return being void, I was hoping to find the Schema Controller type, but all that there is the Fastify Server Instance type. Perhaps i could make this a type using what we wave in the line 133 too ?
and then to test in this case it would return the schemaController ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the actual implementation.
Lines 631 to 639 in e632a9b
function setSchemaController (schemaControllerOpts) { | |
throwIfAlreadyStarted('Cannot call "setSchemaController" when fastify instance is already started!') | |
const old = this[kSchemaController] | |
const schemaController = SchemaController.buildSchemaController(old, Object.assign({}, old.opts, schemaControllerOpts)) | |
this[kSchemaController] = schemaController | |
this.getSchema = schemaController.getSchema.bind(schemaController) | |
this.getSchemas = schemaController.getSchemas.bind(schemaController) | |
return this | |
} |
The type should be
interface SchemaControllerOptions {
bucket?: (parentSchemas?: unknown) => {
addSchema(schema: unknown): FastifyInstance;
getSchema(schemaId: string): unknown;
getSchemas(): Record<string, unknown>;
};
compilersFactory?: {
buildValidator?: ValidatorCompiler;
buildSerializer?: (externalSchemas: unknown, serializerOptsServerOption: FastifyServerOptions["serializerOpts"]) => FastifySerializerCompiler<unknown>;
};
}
setSchemaController(schemaControllerOpts: SchemaControllerOptions): FastifyInstance<RawServer, RawRequest, RawReply, Logger>;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've done the changes i think now the code is right but i wanted to make the Type tests better I looked on the web but i think I could do some more in the test part, also I wanted to implement the compilersFactory type test but I really don't have any ideia how to do it :( I could do the same way as I did on the bucket ? If you have any links to help me get better on this test part I would LOVE to learn more about this kind of testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Checklist
npm run test
andnpm run benchmark
and the Code of conduct
related to #3366
I'm Really happy to contribute! if something is wrong/ tests are bad feel I will make it right as fast as I can