nette / schema

📐 Validating data structures against a given Schema.
https://doc.nette.org/schema
Other
886 stars 26 forks source link

Optional structure with required fields #17

Closed josefsabl closed 3 years ago

josefsabl commented 4 years ago

Let's say I validate the data structure of a blog post. This blog post has a body and an author. And as we support anonymous comments, the user is NOT mandatory. The author is a structure as well and if it IS PROVIDED it MUST have an e-mail.

I have a schema like this:

- comment: structure
  - body: string (required)
  - author: structure (not required)
    - email: string (required)
    - name: string (not required)

At the moment I am unable to validate the data with Nette/Schema like that because if I mark the email as required I still get a validation exception even if the author (not required) is not provided at all.

My validation schema code is like:

Expect::structure([
    'body' => Expect::string()->required(),
    'author' => Expect::structure([
        'email' => Expect::string()->required(),
        'name' => Expect::string(),
    ]),
]);

And for data like this:

- body: Hello World!

I get The mandatory option 'author > email' is missing.. I get the same result even if I explicitly set the structure as not required like this: 'author' => Expect::structure([ ... ])->setRequired(false).

This example is obviously all made up to make things clear but I've hit this issue three times in different places. One of them being writing MongoDB extension for DI and trying to validate the driverOptions (see the context, structure inside structure, but context not mandatory)

Is this a bug? Is it a feature? Or am I missing something?

We are obviously able to work around this by explicitly checking the values in all sorts of places but it is a shame we have to pollute our codebase, especially when we streamlined it pretty much by implementing Nette\Schema in the first place :-)

mabar commented 4 years ago

It's a feature. Imagine instead of array there's a object with required attribute. How could you instantiate it if required attribute is missing? Combine structure with null via Expect::anyOf() or simply make structure ->nullable() Eventually, dependencies between fields could be created via callback defined on structure

josefsabl commented 4 years ago

Imagine instead of array there's a object with required attribute. How could you instantiate it if required attribute is missing?

I can imagine pretty well because that is exactly my case :-) The problem is I don't want to instantiate it if it actually is missing in the input.

Imagine I have a code like:

$post->setBody($validatedInput->body)
if ($validatedInput->author) {
  $post->setAuthor(new Author($validatedInput->author->email));
}

So input like this:

- body: Hello world!

Should result in:

$post->setBody($validatedInput->body);

Something like this:

- body: Hello world!
- author:
  - email: foo@bar.baz

Should result in:

$post->setBody($validatedInput->body);
$post->setAuthor(new Author($validatedInput->author->email))

But I cannot manage to validate it with Schema.

If I set the email as required. It doesn't matter that the author is not there at all and I get an exception.

If I don't set the email as required it obviously is not required and I cannot be sure if it is there when author is present.

To make things worse, the author node is always there with its properties set to null even if it is not present in the input.

I am not sure what did you mean exactly, but structure cannot be set as nullable, can it?

I am already taking a look here to see if it solves the problem. I'd love to use Nette/Schema, the idea is great. I am however not sure if this use case matches the purpose and features of Nette/Schema.

josefsabl commented 4 years ago

This is what does exactly what I need using the Garden Schema.

\Garden\Schema\Schema::parse([
  'body:string',
  'author:o?' => [
    'email:string'
  ]
])

I still like the Nette Schema's interface better but I think there is something wrongly designed (imho, no offence) or I am still missing something :-)

mabar commented 4 years ago

I would expect that to work, but never tried

Expect::anyOf(
  Expect::structure([...]),
  Expect::null()
)
josefsabl commented 4 years ago

Yeah, this should probably work, didn't get this suggestion for the first time. I believe this to be a bit hackish and inconsistent though, you don't do

Expect::anyOf(
  Expect::string(),
  Expect::null()
)

But rather:

Expect::string()->required(false);

And as we generate the schema programatically in some places this further complicates things.