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

Schema for unified automatic configuration validation #191

Merged
merged 8 commits into from Mar 26, 2019
Merged

Conversation

dg
Copy link
Member

@dg dg commented Mar 12, 2019

  • 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

@holantomas holantomas commented Mar 12, 2019

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
Copy link

@dakorpar dakorpar commented Mar 13, 2019

@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.

}


public static function type(string $type): self
Copy link
Member

@milo milo Mar 13, 2019

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
Copy link
Member Author

@dg dg commented Mar 14, 2019

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

@dg
Copy link
Member Author

@dg dg commented Mar 14, 2019

image

@dg dg merged commit d46798e into nette:master Mar 26, 2019
1 check was pending
@dg dg deleted the schema branch Mar 26, 2019
@ondrejmirtes
Copy link
Contributor

@ondrejmirtes ondrejmirtes commented Mar 27, 2019

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

@dg
Copy link
Member Author

@dg dg commented Mar 27, 2019

@ondrejmirtes of course!

@dg
Copy link
Member Author

@dg dg commented Mar 27, 2019

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

@trejjam
Copy link
Contributor

@trejjam trejjam commented Mar 27, 2019

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
Copy link
Contributor

@JanTvrdik JanTvrdik commented Mar 28, 2019

php compiler will yell at me

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

trejjam referenced this issue in contributte/thepay Mar 30, 2019
@trejjam
Copy link
Contributor

@trejjam trejjam commented Mar 30, 2019

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
Development

Successfully merging this pull request may close these issues.

None yet

7 participants