Skip to content
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

Dependency error #588

Closed
gergoerdosi opened this issue Mar 5, 2015 · 13 comments
Closed

Dependency error #588

gergoerdosi opened this issue Mar 5, 2015 · 13 comments
Assignees
Labels
support Questions, discussions, and general support

Comments

@gergoerdosi
Copy link
Contributor

There are two keys, if one of them is empty, the other should not be empty. I created this schema:

var schema = Joi.object().keys({
    a: Joi.string().when('b', { is: '', then: Joi.string().min(1) }),
    b: Joi.string().when('a', { is: '', then: Joi.string().min(1) })
});

However it raises an error:

/node_modules/joi/node_modules/hoek/lib/index.js:678
    throw new Error(msgs.join(' ') || 'Unknown error');
          ^
Error: item added into group b created a dependencies error
    at Object.exports.assert (/node_modules/joi/node_modules/hoek/lib/index.js:678:11)
    at internals.Topo.add (/node_modules/joi/node_modules/topo/lib/index.js:51:10)
    at internals.Object.keys (/node_modules/joi/lib/object.js:275:14)
    at Object.<anonymous> (/index.js:3:27)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)

Is this a bug or should I create the schema in a different way?

@Marsup Marsup added the support Questions, discussions, and general support label Mar 5, 2015
@Marsup Marsup self-assigned this Mar 5, 2015
@Marsup
Copy link
Collaborator

Marsup commented Mar 5, 2015

This looks like a circular dependency. To check a we need to check b which needs to check a...
Can't you use xor for that ?

@gergoerdosi
Copy link
Contributor Author

@Marsup It's actually or, not xor. The final schema is a bit more complicated, both keys can exist and have a value. Also, if one key presents and is not empty, then the other key doesn't even have to present. This is how I imagined the final schema:

var schema = Joi.object().keys({
    a: Joi.string().when('b', { is: '', then: Joi.string().min(1) }),
    b: Joi.string().when('a', { is: '', then: Joi.string().min(1) })
}).or('a', 'b');

I tried to answer a Stack Overflow question when ran into this issue. You can find more details there: http://stackoverflow.com/questions/28864812/using-joi-require-one-of-two-fields-to-be-non-empty

@Marsup
Copy link
Collaborator

Marsup commented Mar 5, 2015

This works but it's ugly :

var schema = Joi.alternatives().try(
    Joi.object().keys({
        a: Joi.string().min(1),
        b: Joi.string().only('')
    }),
    Joi.object().keys({
        a: Joi.string().only(''),
        b: Joi.string().min(1)
    })
);

Maybe we should let joi lazily evaluate the conditions without checking for circular deps (ie. remove Topo). @hueniverse I understand why it's useful in hapi, but what kind of mistake does it prevent in joi's context ?

@hueniverse
Copy link
Contributor

You need it because values can be modified so the order in which you check is important.

@Marsup Marsup closed this as completed Dec 18, 2015
@elyobo
Copy link

elyobo commented May 20, 2016

This is a pretty unfriendly workaround for the case where one of two fields must be present; it gets even worse if one of three or more fields are required, and exponentially worse if you have multiple sets of fields (e.g. a or b is required and c or d is required).

I can see that allowing the values to be modified does make this problematic though :-/

@Marsup
Copy link
Collaborator

Marsup commented May 21, 2016

You have the or to do that. And empty() happened a little while after that which should make this case much easier.

@elyobo
Copy link

elyobo commented May 22, 2016

Thanks @Marsup, I did see or earlier and thought that it was the solution, but its default behaviour is to treat empty strings as present which doesn't work well for my requirements (validating forms, where everything is defined, but may be ''). I hadn't seen empty though and it looks like a combination of or and empty should do the trick.

Hueniverse pointed out above that the values can be modified and so ordering is important; how is this worked around in or then? I assume it applies to the values before modification?

@elyobo
Copy link

elyobo commented May 22, 2016

empty + or seems to work for the "require this field or that field" but treating undefined as the only "empty" seems to be pretty heavily baked in (e.g. additional validation is only skipped for undefined values).

I think the only practical approach here is to preprocess the data to validate and convert '' to undefined, which then makes everything more or less work as expected (or works, no need to mess with .allow('') to make strings optional, additional validation is skipped for optional and undefined fields), e.g.

const Joi = require('joi');
const preprocess = value => JSON.parse(JSON.stringify(
    value, (k, v) => v === '' ? undefined: v
));
const schema = Joi.object().keys({
  a: Joi.string().email(),
  b: Joi.string().email()}
).or('a', 'b');

// Errors
console.log(Joi.validate(preprocess({}), schema));
console.log(Joi.validate(preprocess({a: ''}), schema));
console.log(Joi.validate(preprocess({b: ''}), schema));

// OK
console.log(Joi.validate(preprocess({a: 'foo@bar.com', b: ''}), schema));
console.log(Joi.validate(preprocess({b: 'foo@bar.com', a: ''}), schema));

@Marsup
Copy link
Collaborator

Marsup commented May 22, 2016

It is exactly what empty does, I don't know what you're asking.

@elyobo
Copy link

elyobo commented May 22, 2016

Perhaps I misunderstood you? I thought you were referring to any.empty, which doesn't do that at all. Was it another empty that you were referring to?

Anyway, I didn't mean to turn this closed ticked into a support request. Joi is clearly more targeted at API usage than form validation and so it requires a bit of work to make it behave nicely for it and that's fair enough.

@Marsup
Copy link
Collaborator

Marsup commented May 22, 2016

It doesn't do what ? It converts anything that matches the empty schema to undefined.

@elyobo
Copy link

elyobo commented May 22, 2016 via email

@elyobo
Copy link

elyobo commented May 22, 2016

Thanks, I was able to get .empty working.

My mistake was thinking I could apply it to the result of object.keys() and have it apply everywhere, rather than having to call it on each individual schema, e.g. the following does not work.

const schema = Joi.object().keys({
  a: Joi.string().email(),
  b: Joi.string().email(),
}).or('a', 'b').empty('');

But this does.

const email = Joi.string().email().empty('');
const schema = Joi.object().keys({ a: email, b: email }).or('a', 'b');

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
support Questions, discussions, and general support
Projects
None yet
Development

No branches or pull requests

4 participants