New Concrete API

[henriquemoody]

Hey, guys! The new Concrete API for Respect\Validation is almost ready then we can release our first major version (see #258). This issue was created only just to explain and discuss about what I'm working on.

There will be no changes on our Fluent API, I took care to never break the agility we have creating validation chains.

First of all I wanna be clear about the comments below, Validation is, IMHO, the best validation library ever written in PHP and I always say, @alganet is a genius for creating such an amazing library! The reason I wanna create a new Concrete API is simple, the requirements changed since the library was created, eventually code rusts and that's the normal cycle of software development.

Problems

All rules depend on AbstractRule

Yes, we have the Validatable interface, but let's be honest, we never create a rule from scratch and instead we just extend AbstractRule to create new rules.

That's a problem because all Rules are based on an implementation, if we have to change AbstractRule all rules will be changed as well.

Validatable interface has too many methods

That's the reason why we extend AbstractRule instead, this interface has 7 methods and all rule must do the same in some of them.

The methods in Validatable are really necessary to do what Validation does, I fell like all methods where created really by necessity:

1. assert()
2. check($input)
3. getName()
4. reportError($input, array $relatedExceptions = [])
5. setName($name)
6. setTemplate($template)
7. validate($input)

Decentralized validation

Each rule performs the validation to the input with no intermediate and this is a problem we are facing for more than one year (see #205 and #235).

Since we cannot intercept the input to check if it is or isn't optional, the only way to check optional inputs on all rules we have is changing all rules we have.

Currently the methods assert(), check() and __invoke() accept optional inputs as valid but the validate() method doesn't. Each rule has its own way to validate the input, so it can or cannot accept optional values and interpret whatever input as optional or valid.

NotEmpty rule

That's rule as optional value checking has been a PITA for a long time (see #244, #154, #196, #173). Also the name Empty should imply on using only the PHP empty() language constructor but it doesn't do that.

Nested exceptions

Also a problem. When using check() method it works like a charm, but wen you use assert() with key(), for example, it doesn't work properly (see #179).
I've tried several times to fix this issue on the current code-base but the fixes were always a poor workaround. Of course, working with recursion is almost always ugly, but I think we can do better on the new Concrete API.

Nested exception messages

Currently, when using assert() we can just get the full-message as string, we cannot get all messages in other format to format it as we want.

Exception message translation

We must be able to translate the exception messages in a easy way.

Cannot access Factory inside rules

Some rules depend on other rules, would be nice if we can use the same factory we already have to create them.

Proposal

I simplified the rules on a single interface with one method only:

interface RuleInterface
{
    public function apply(Context $context);
}

That's all we need, but you may ask me a few questions.

Why it's called apply()?

I'm opened to other names, I think it makes sense since we are talking about Rules.

What is the Context object?

Currently, a chain of validators use the Composite pattern and that's the only way to make nested validations, but we don't need to use the rules for that. The Context object uses the Composite pattern and every context has its own Rule, validation result and parameters.

The Context object allows us to define parameters to be used on exceptions, change its mode or even define the message to be displayed when an exception is thrown. By default, it defines the input to be used on some rules and also defines isValid, which indicates if the rule was successful applied or not (TRUE is the default value).

Why we don't have a $input parameter?

Firstly, because we don't need it every time, some rules just don't use the $input, secondly because Context already has the input and we can get it when we need it.

I cannot use a Rule as a single validation anymore?

That's true. I mean, you can but you have to create a Context object and then check if it isValid or not.

Any way, it will not make a difference if you use the Fluent API.

How about check(), assert(), validate() and __invoke() methods?

These methods are available on the Validator class, so you can still use them in the same way you already do it, like:

v::int()->positive()->assert($input);
v::int()->positive()->check($input);
v::int()->positive()->validate($input);

$rule = v::int()->positive();
$rule($input);

The only change I've made to them: the assert() and check() methods don't return any value anymore. On our current API these methods can return just TRUE or throw an exception, so it doesn't make any sense to keep this behavior, IMO.

How do you deal with optional values?

All rules are applied by Context object.

There is an Interface called RuleRequiredInterface and any rule which implements this interface will be applied no matter if the value is optional or not (rules like NotOptional, AllOf, AlwaysInvalid, AlwaysValid). Actually, it was @qrazi's idea to create this interface (see #336).

There is a trait NotOptionalTrait which performs the validation on NotOptional rule and on Context. If the value is optional and the rule doesn't implement RuleRequiredInterface, I don't even apply the rule to the Context.

What else?

• Exception messages using Markdown by default
• The Factory is now accessible on all Rules by Context object
• The RecursiveContextIterator class
• Exceptions, Rules and Contexts are created by Factory
• PHP 5.6 or higher (HHVM and PHP 7 included), only
• Replace {{name}} for {{placeholder}} on exception messages
• Full PSR-1, PSR-2, PSR-4 (and PSR-5) support

Show me the code!

Check out the new_api branch.

PS.: Just remember it's a work in progress

---

Updates:

• Result was replaced by Context
• isValid() method was removed

[henriquemoody]

I've spoken to @alganet about this new API and he likes it but I would like to know from you guys of @Respect/troubleshooters and all the community.

[augustohp]

I like the idea and the changes on internal design is much needed, I still need to digest the changes and will probably not have the time in the next two weeks.

One thing I would point as a bummer on the current branch is that it trashes the whole history of changes. I would rather pay the price of incrementally refactor the actual code and still have their history of changes.

[henriquemoody]

I totally agree with you.

I created an orphan branch in order to make clean what is in the new API and also makes easy to test only what it has. My plans are to keep the history we have.

[alganet]

I like these changes a lot. One thing is missing though, the callable API. It allows me to decouple validator itself from userland software projects:

class MyUserlandClass
{
    private $validator;
    public function setValidator(callable $validator)
    {
        $this->validator = $validator;
    }
    public function prove()
    {
        call_user_func($this->validator, $this->something);
    }
    // ...
}

$userlandRaw = new MyUserlandClass();
$userlandRaw->setValidator('is_object');
$userlandV = new MyUserlandClass();
$userlandV->setValidator(new \Respect\Validation\Rules\Object());

// Edit sample (see below)
$userlandv = new MyUserlandClass();
$userlandv->setValidator(\Respect\Validation\Validator::object());

I'm not sure how to implement this though, or even if its usage is popular, so I'm ok if it doesn't make it through a final version. (Edit: Perhaps only the Validator class could have the callable interface, that would make things much simpler.)

Changes were trashed when we moved from PSR-1 to PSR-4. They're only visible with --follow on the client side. Not sure if it is possible to see this on GitHub itself.

When we merge this into our main branch, if we add -all or equivalent at some point, many files will appear as moved (because the implementation still shares many lines in common), and then we'll keep the same behavior (deep after-move changes are visible using --follow). Maybe 1.0 would be a good time to rebase this to unify history, but I wouldn't know how to do that.

[augustohp]

Changes were trashed when we moved from PSR-1 to PSR-4. They're only visible with --follow on the client side. Not sure if it is possible to see this on GitHub itself.

I really don't care about GitHub itself, the history of each file is still there: our contributors and when changes were made.

When we merge this into our main branch, if we add -all or equivalent at some point, many files will appear as moved (because the implementation still shares many lines in common), and then we'll keep the same behavior (deep after-move changes are visible using --follow). Maybe 1.0 would be a good time to rebase this to unify history, but I wouldn't know how to do that.

Me neither, that is why I am concerned about it.

[henriquemoody]

One thing is missing though, the callable API

You're right. I just fixed it on Validator class.

When we merge this into our main branch

This branch was created only as a POC, it's an orphan branch because I didn't want any code that was not from the new Concrete API on it.

If you guys take a look on the branch's code you will see there is only the necessary code and the most important rules:

• AllOf (as AbstractComposite)
• Key (as AbstractRelated)
• Match (a better name for Regex, IMO)
• Not
• NotBlank (see commend on #173)
• NotEmpty (see commend on #173)
• NotOptional (see commend on #173)

We're not going to merge it, we really need to refactor the current code using the idea of this branch, if you guys accept this proposal, of course. In spite of the Lazy Consensus we've already agreed on, since it's a big change, I will wait more than usual to start changing our code-base until you guys accept that.

Besides, the branch still needs some refactoring, but the idea is clear already.

[nelsonsar]

I'm using this package now and a think this is a huge change and may break the coolest thing that it has: simplicity when using it.

There are design problems. But I think that you can make something less drastic.

First, I would go for the problems that I see you're describing:

1. All rules depending on AbstractRule: Use interface segregation to define new interfaces to it and new features using new interfaces or "helper" classes. Of course, I don't know all rules and never created a new one, so I may be simplist.

2. Error handling: This is a problem I'm facing too and again I don't know all the problems, but, this can be done (maybe?) by creating a new method that do things better or that make more sense to it, avoiding breaking things in a hard way. Something change getIterator method to return a sane output.

I will work in exception part this holiday anyway I'll try to make a PR soon.

[henriquemoody]

The Fluent API will not change at all, just the Concrete API and not too much, so we will still be able to do:

$validator = v::alnum()->length(1,15);
$validator->validate($input); // Or just $validator($input)

Or:

$validator = new v();
$validator->alnum();
$validator->length(1, 15);
$validator->check($input);

Or even:

$validator = new Validator();
$validator->addRule(new Alnum());
$validator->addRule(new Length(1, 15));
$validator->assert($input);

The only think we will not be able to do in the new Concrete API is to use a single rule without the Validator, like:

$alnum = new Alnum();
$alnum->validate($input); // nor check(), assert() or __invoke();

I may be wrong, but the last example above doesn't represent the majority of Respect\Validation use-cases. The problem is that for this specific case we have the more critical problems in our library, which can be summarized to decentralized validation.

For every rule we have to:

• Skip validation when value is optional
• Validate the input with validate() method
• Validate the input with __invoke() method
• Throw exception on check() when input is not valid and return true when it is valid
• Throw exception on assert() when input is not valid and return true when it is valid
• Define and retrieve the rule's name
• Be able to define the template message for exception (this changes the state of the Rule itself, which is really bad)

In the way library was designed, almost all methods of the Validatable are necessary to all rules.

Using the new Concrete API, we need the Context object. See how it works:

$result = new Context(new Alnum(), ['input' => $input], new Factory());
$result->applyRule(); // Centralized validation is here
$result->isValid;

Of course there are pros and cons, but that's the way I found to make a centralized validation, don't change the rules' state, allow a rule to define parameters (like exception templates), make nested validations (Composite) and still keep the Fluent API we have.

We can stick with the design we have and fix the exception handling, we can use Template Method to centralize the validations but we will depend even more on AbstractRule.

[nelsonsar]

Hmm, as I said I don't know much about the library itself. But, I thought something like:

interface Reportable
{
    public function reportError($input, array $relatedException = array());
}

And then

interface Validatable extends Reportable
{
    public function assert($input);
    public function check($input);
    public function validate($input);
}

Accessors (setName, getName, setTemplate) can be implemented in AbstractRule directly and overriden in child class. Later, you can change Rules to receive a name in constructor and the same thing for message.

Later you can also delegate error reporting to another class or trait (last we'll be easier) and isolate this part/responsibility. Then, remove reportError from Validatable.

All behavior that need to be extended can be extracted to another "helpers".

Some thoughts from someone that don't know much but like the lib a lot :)