Skip to content

Schema for unified automatic configuration validation #191

Merged
dg merged 8 commits into
nette:masterfrom
dg:schema
Mar 26, 2019
Merged

Schema for unified automatic configuration validation #191
dg merged 8 commits into
nette:masterfrom
dg:schema

Conversation

@dg

@dg dg commented Mar 12, 2019

Copy link
Copy Markdown
Member
  • BC break? no

CompilerExtension can provide a schema which is automatically used to validate and normalize configuration:

	public function getConfigSchema(): Nette\DI\Config\Schema
	{
		return Expect::structure([
			'user' => Expect::string()
			'password' => Expect::string()
			'debugger' => Expect::bool(true),
		]);
	}

These rules are used:

  • each item is optional unless you change it via Expect::bool()->required()
  • the default value is null (and [] for array, list and struct) unless you change it via Expect::bool()->default($default) or simply Expect::bool($default)
  • items are not nullable by default
  • syntax of Validators from Nette\Utils can be used, for example Expect::type('string|null') or Expect::type('string|stdClass[]') etc.

This schema describes array with exactly two items:

		return Expect::structure([
			'name' => Expect::string()->required(),
			'password' => Expect::string(),
		]);

Extra items can be allowed via otherItems($type), ie:

		return Expect::structure([
			'name' => Expect::string()->required(),
			'password' => Expect::string(),
		])->otherItems('string');

You can use constraints min() and max() to number values, but also size of strings or arrays:

		Expect::listOf('string')->min(1);

You can add additional assertions:

		Expect::string()->assert('is_file')->assert('is_readable');

Before configuration is processed, it can be normalized:

		return Expect::structure([
			'name' => Expect::string()->required(),
			'password' => Expect::string(),
		])->before(function ($input) { return $output; });

Examples of usage:

@holantomas

Copy link
Copy Markdown

Great! Only one thing. Did you think about something like Expect::string()->nullable(); instead of using enum()? If you look on database schema it's look little bit confusing(for me).

Expect::arrayOf([
			'dsn' => Expect::string()->required(), // Have to be presented, but CAN be null?
			'user' => Expect::enum(Expect::string(), null), // Have not to be presented and CAN be null
			'reflection' => Expect::string(), // Have not to be presented but CANNOT be null?
			//...
		])

Maybe this look better

Expect::arrayOf([
			'dsn' => Expect::string()->required(), // Have to be present and CANNOT be null
			'user' => Expect::string()->nullable(), // Have not be presented and CAN be null
			'reflection' => Expect::string(), // Have not to be presented but CANNOT be null
			'password' => Expect::string()->required()->nullable(), // Have to be presented but CAN be null
			//...
		])

Or am I missing something?

@dakorpar

dakorpar commented Mar 13, 2019

Copy link
Copy Markdown

@dg why not making class for config, that way we're far more strict and full autocomplete in all ides, and with php7.4 we will have typehints on properties? Wouldn't that be far better then this dynamic validation?
Since it is huge BC it can also be added abstract config class which implements \ArrayAccess with deprecation warning or something like that. Function for receiving array (from neon) is also needed. Another good way would also be with getters and setters and have that typed immidiatelly.

Comment thread src/DI/Config/Expect.php
}


public static function type(string $type): self

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about:

function type(string ...$types): self
{
    return new self(implode('|', $types));
}

To be able write:

Expect::type('string', Nette\DI\Definitions\Statement::class)

or own method for it.

@dg

dg commented Mar 14, 2019

Copy link
Copy Markdown
Member Author

@dakorpar I also thought about using the class for configuration and it's definitely a good idea.

@dg

dg commented Mar 14, 2019

Copy link
Copy Markdown
Member Author

image

@dg dg force-pushed the schema branch 2 times, most recently from 7e4108f to d9f7d5f Compare March 15, 2019 02:34
@dg dg force-pushed the master branch 2 times, most recently from c84fbc6 to 3f1cadd Compare March 15, 2019 06:42
@dg dg force-pushed the schema branch 3 times, most recently from 8d57c37 to d2d68df Compare March 15, 2019 15:42
@dg dg force-pushed the master branch 2 times, most recently from 02ef205 to 9bd6554 Compare March 17, 2019 16:04
@dg dg force-pushed the master branch 3 times, most recently from 1adcd1c to 18fdc11 Compare March 22, 2019 05:10
@dg dg merged commit d46798e into nette:master Mar 26, 2019
@dg dg deleted the schema branch March 26, 2019 12:26
@ondrejmirtes

Copy link
Copy Markdown
Contributor

Cool! Can I somehow use the system to validate what users have in their parameters section?

@dg

dg commented Mar 27, 2019

Copy link
Copy Markdown
Member Author

@ondrejmirtes of course!

@dg

dg commented Mar 27, 2019

Copy link
Copy Markdown
Member Author

@holantomas I added nullable() and require(), thanks for suggestions

@trejjam

trejjam commented Mar 27, 2019

Copy link
Copy Markdown
Contributor

Hi, thanks for this I really like it!
Is there any recomemded way how to define schema for a pre-v3 compatible nette extensions?
My point is that when I define getConfigSchema(): Nette\DI\Config\Schema method in extensions class, php compiler will yell at me that it does not know class Nette\DI\Config\Schema.

@JanTvrdik

JanTvrdik commented Mar 28, 2019

Copy link
Copy Markdown
Contributor

php compiler will yell at me

Are you sure? Afaik return types do not trigger autoloading.

trejjam referenced this pull request in contributte/thepay Mar 30, 2019
@trejjam

trejjam commented Mar 30, 2019

Copy link
Copy Markdown
Contributor

I am not sure, thanks. It was a habit from compiled languages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants