xogroup / felicity

Javascript object constructors and sample data based on Joi schema.
Other
111 stars 9 forks source link

Examples[childSchema.type] is not a constructor #116

Closed danielo515 closed 7 years ago

danielo515 commented 7 years ago

Context

What are you trying to achieve or the steps to reproduce ?

I'm trying to generate an example from a joi schema. Something as simple as this

const Felicity = require('felicity');
const ProbeSchema = require('./path/to/schema');

console.log(Felicity.example(ProbeSchema));

But I'm getting the following error:

/node_modules/felicity/lib/helpers.js:615
                const child = new Examples[childSchema.type](childSchemaRaw, childOptions);

What result did you expect ?

I expect an auto generated example, of course 😄

What result did you observe ?

Instead I get an error.

I think this may be related with #80 Is felicity unable to generate examples from extended Joi instances?

Regards

danielo515 commented 7 years ago

Not sure if this deserves a mention, but if I try this:

'use strict';

const Felicity = require('felicity');
const ProbeSchema = require('./path');

const FelicityProbeConstructor = Felicity.entityFor(ProbeSchema);
const probeInstance = new FelicityProbeConstructor();

console.log(probeInstance.example());

I get an even larger error:


/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/helpers.js:615
                const child = new Examples[childSchema.type](childSchemaRaw, childOptions);
                              ^

TypeError: Examples[childSchema.type] is not a constructor
    at Object.keys.forEach (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/helpers.js:615:31)
    at Array.forEach (native)
    at ObjectExample._generate (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/helpers.js:591:53)
    at ObjectExample.generate (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/helpers.js:36:21)
    at valueGenerator (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/helpers.js:878:20)
    at Object.exampleGenerator [as example] (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/exampleGenerator.js:10:27)
    at Object.<anonymous> (/Users/danielo/GIT/case-supermodels/.bin/populateDb.js:9:22)
    at Module._compile (module.js:571:32)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:488:32)
agent:case-supermodels danielo$ node .bin/populateDb.js
/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/helpers.js:615
                const child = new Examples[childSchema.type](childSchemaRaw, childOptions);
                              ^

TypeError: Examples[childSchema.type] is not a constructor
    at Object.keys.forEach (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/helpers.js:615:31)
    at Array.forEach (native)
    at ObjectExample._generate (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/helpers.js:591:53)
    at ObjectExample.generate (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/helpers.js:36:21)
    at valueGenerator (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/helpers.js:878:20)
    at exampleGenerator (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/exampleGenerator.js:10:27)
    at getExample (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/index.js:52:16)
    at Constructor.example (/Users/danielo/GIT/case-supermodels/node_modules/felicity/lib/index.js:91:20)
    at Object.<anonymous> (/Users/danielo/GIT/case-supermodels/.bin/populateDb.js:9:27)
    at Module._compile (module.js:571:32)
WesTyler commented 7 years ago

Is felicity unable to generate examples from extended Joi instances?

Unfortunately for now, yes. What extension/extended types are your schema using? Just out of curiosity ;)

If you do have some extended Joi.something() in your schema, that would explain both of the issues you're seeing on example and entityFor. If you are using an extended Joi but only base schema types, that's a different problem :P

danielo515 commented 7 years ago

I'm using both. For example I have one Joi.string().integer() and Joi.timestamp().

What is exactly the barrier? By my experience joi extend is just adding types based on existing ones.

WesTyler commented 7 years ago

The barrier for Felicity is knowing how to generate random data that matches the type. Or for entityFor, knowing what the "empty" state for the type looks like (null for most, [], {}, etc).

I'm toying around with a Joi-like extension point where you can provide the extended type and a function that generates valid data for that type. I haven't tried building it out yet though. Hopefully soon.

danielo515 commented 7 years ago

Isn't that kind of information reported by describe ? If not, then I think it should be describe responsibility to provide such interface to extensions. If I remember correctly extensions can provide a describe function. Could that be used by Felicity taking in account that the provided description is correct ?

WesTyler commented 7 years ago

Yeah, Felicity is driven completely by describe. But, it has to know how to translate that output.

For example, using the joi-date-extensions extended Joi. You hand Felicity.example Joi.date().format('MM-DD-YY') and you expect to get something like 05-26-17 back. But all Felicity has to go on is a flag that says momentFormat: [ 'MM-DD-YY' ]. So yes, it knows the base type (date) but not how to handle the specific constraints that make a value truly "valid".

With the track I'm thinking about, you would extend Felicity by providing something like (completely making this format up on the spot):

{
    schema: Joi.date().format(),
    example: (randomBaseType) => {
        const moment = new Moment(randomBaseType);
        const targetFormat = Array.isArray(schemaDescription.flags.momentFormat) ?
            pickRandomFromArray(schemaDescription.flags.momentFormat) :
            schemaDescription.flags.momentFormat;

        return moment.format(targetFormat);
    }
}
danielo515 commented 7 years ago

How does felicity create random values based on regular expressions? What if each extension includes a regular expression on its description that can be used by felicity to generate random values?

Another alternative would be to allow a special property on the description specific for felicity. Since Felicity is the only project providing this great functionalities to Joi I feel that Joi should create some specific hooks for it. I really thank that just one extension should be provided, the one that is given to Joi. If you have to make sure that you inject to Felicity all the same extensions that were injected to Joi we will suffer a lot, I promise you. If all you get is a Joi instance, you have no idea of how was it extended, and you will end on a code-split situation, where you have to maintain the Joi extension, the felicity extension and make sure you inject both on every place.

WesTyler commented 7 years ago

How does felicity create random values based on regular expressions

With the randexp library :)

allow a special property on the description specific for felicity

Nah, not going to pollute Joi in support of Felicity. We are making the describe behavior more complete and thorough, but I won't try to put something specific in there for Felicity.

I really thank that just one extension should be provided

Yeah, that would be ideal! Haha. If you think of a way to make it happen, I'm all ears. I've been thinking this through for the better part of a year now.

If you have to make sure that you inject to Felicity all the same extensions that were injected to Joi we will suffer a lot, I promise you

I agree completely. It's not an ideal situation, but it would allow you to do what you were trying to do in opening this issue in the first place. If I can get a workable solution in then it will allow me to get a more complete picture of the problem set in order to devise a more complete and ideal version 2 (or 3) implementation. :D

danielo515 commented 7 years ago

I have been thinking about this. I don't have the solution, but at least I have a couple of comments.

I don't know why this error happens. I can understand that felicity will not be able to create a valid example for custom extension, but at least it should not break this way. There are other libraries that use joi descriptions to create examples ( hapi-swagger is an example). The worst that happened to me it's that instead of a valid example they just write the type. In my case, I have a custom type which is URL, based on string, what happy-swagger prints it's url:"string" . I think it will be fantastic if felicty has this kind of fallback.

Nah, not going to pollute Joi in support of Felicity. We are making the describe behavior more complete and thorough, but I won't try to put something specific in there for Felicity.

This will only apply for the extensions. As far as I know (because it is poorly documented) the extensions have the chance to add stuff to their own description. The extensions that want to support felicity could include certain information there specific for felicity. I don't see the problem here because the extension author will be forced to create a felicity extension anyway, better do it on a single place.

WesTyler commented 7 years ago

I think it will be fantastic if felicty has this kind of fallback.

I agree. Working on that currently. Hoping to have a patch up in the next few days ;)

This will only apply for the extensions.

Ah, gotcha. Yeah, I'm totally fine with that approach. I thought you meant adding a describe property to Joi proper.

danielo515 commented 7 years ago

I thought you meant adding a describe property to Joi proper.

Sorry, not sure to understand 😕

I mean something like:

Joi.customType().describe()
{ type: 'string', invalids: [ '' ] , generate: (something) => somethingelse }

The problem is that describe is supposed to return something serializable, while a function it is not. But since it is not documented, I don't see the problem 😄

WesTyler commented 7 years ago

We are going to be adding support for .default() functions to the describe output in Joi v11, so in general having functions in the describe output is totally fine. I like where this is going :D :)

WesTyler commented 7 years ago

@danielo515 - I opened up a PR for a basic fallback implementation for extensions. If you don't mind, would you check out that branch and see if it fixes the TypeError you were seeing with your extension?

If it does, or if I don't hear back in a couple of days, I'll go ahead and publish as a patch while I work on proper extensions support.

danielo515 commented 7 years ago

I'll try to rest it tomorrow and report back.

Many thanks

danielo515 commented 7 years ago

Hello @WesTyler

I'm still getting the same error:


const ProbeSchema = joi.object({url: joi.url()}) // url is part of my extension, and is of type url

 console.log(Felicity.example(ProbeSchema));
TypeError: Examples[childSchema.type] is not a constructor
    at Object.keys.forEach (/Users/danielo/programmingTest/felicity/lib/valueGenerator.js:616:31)
    at Array.forEach (native)
    at ObjectExample._generate (/Users/danielo/programmingTest/felicity/lib/valueGenerator.js:592:53)
    at ObjectExample.generate (/Users/danielo/programmingTest/felicity/lib/valueGenerator.js:37:21)
    at valueGenerator (/Users/danielo/programmingTest/felicity/lib/valueGenerator.js:827:20)
    at Object.exampleGenerator [as example] (/Users/danielo/programmingTest/felicity/lib/exampleGenerator.js:9:27)
    at repl:1:22
    at ContextifyScript.Script.runInThisContext (vm.js:23:33)
    at REPLServer.defaultEval (repl.js:336:29)
    at bound (domain.js:280:14)
WesTyler commented 7 years ago

Eff.

Any chance you could send me a gist with your extension? I haven't been able to write an extension that fails on this branch...

danielo515 commented 7 years ago

Sure. Expect it tomorrow. It may be a problem with how I author extensions

WesTyler commented 7 years ago

No matter how you author an extension, if it works with Joi it shouldn't throw TypeErrors here :)

Thanks for working with me on this!

danielo515 commented 7 years ago

Here you have an example that fails:

https://gist.github.com/danielo515/32e78441e900440f7350db81bca8fa0a

Many thanks Regards

WesTyler commented 7 years ago

Ok great, I'll try to get something like that worked into the tests. Thanks for sharing that over!

WesTyler commented 7 years ago

Ahhhh hahaha okay this is easy. I made the change in the wrong spot, so my fallbacks didn't work for children :P . My bad. Should have an update on this branch shortly.

WesTyler commented 7 years ago

Should be good to go now.

danielo515 commented 7 years ago

Hello @WesTyler

I tried the new version of Felicity with my base example and it now works. However, I think you are not taking full advantage of what the describe method provides to you.

If you take a look at the description output of my example, it provides an array of valid values. However, instead of picking one, you are returning a random string. I can understand that it can be tricky to manage some situations, but this one (where a valids array is present) should be an easy one, and an oportunity to make many joi extensions compatible with Felicity with no effort. For example, if you are an author extension that just validates an string with a list of possible values, then you just have to provide a valid description and you're done.

Regards

WesTyler commented 7 years ago

I think Felicity will currently use a value in the valids array sometimes. The only time it will only use the values in valids is if the allowOnly flag is set as well.

const allowOnly = Joi.object().keys({
    strict: Joi.string().valid(['option1', 'option2'])
});
const allowNull = Joi.object().keys({
    permissive: Joi.string().allow(null)
});

Felicity.example(allowOnly);
/*
strict will "randomly" be either option1 or option2
{
    strict: 'option1'
}
*/
Felicity.example(allowNull);
/*
If we always use a random value from "valids" (Previous behavior through v2.0.0)
permissive will always be null. Not ideal.
{
    permissive: null
}
*/
/*
If we sometimes use a random value from "valids" (current behavior as of v2.1.0)
permissive will only sometimes be null.
{
    permissive: 'ahjhkegi739ugq9'
}
*/
WesTyler commented 7 years ago

Perhaps I can default Felicity to always use valids if present, but have an option to override that for specific examples like my use case for not always wanting null when we .allow(null)

WesTyler commented 7 years ago

Actually, I like that better. I'll open up a separate ticket for that switch in default behavior.

Is there anything remaining on this issue for super-basic extension support? If not, I'll merge that PR and close this. More extensive extension support per discussions above will follow, this resolution will be to simply prevent TypeErrors.

danielo515 commented 7 years ago

Hello @WesTyler You're right this issue is resolved. We can continue discussing about the valids thing in the other ticket.

Regards