Skip to content
This repository has been archived by the owner on Jan 30, 2020. It is now read-only.

[RFC] Replace Input with a chain of callbacks. #87

Open
Maks3w opened this issue Jan 20, 2016 · 7 comments
Open

[RFC] Replace Input with a chain of callbacks. #87

Maks3w opened this issue Jan 20, 2016 · 7 comments

Comments

@Maks3w
Copy link
Member

Maks3w commented Jan 20, 2016

I started to think the best way of build a custom pipe of filters and validators where you can filter after validation, etc.

I propose replace Zend\InputFilter\Input with a custom chain of methods like A+ promises.

For resume A+ promises have the following main properties along others I won't detail here:

  • has the shape of then(callback success = null, callback error = null)
  • success chain is broken when an exception is raised
  • if error chain return a value then the next success callback continue with the value provided.

So basically the chain do two kind of things. Transformations of the input value and reject the chain if is invalid.

$chain = new Chain();
$chain
  ->then(
    // Tipical filter function
    function($inputValue) use ($context) {
      return $filteredValue;
    }
  )
  ->then(
    // Tipical validation function
    function($inputValue) use ($context) {
      if ( ! isValid($inputValue) {
        throw new ValidationException(); // Break the chain
      }

      // Note MUST return a value, not the isValid boolean.
      // It can return any other object like DateTime or a database Entity
      return $inputValue;
    }
  )

  ->then($hydrate)
  ->then($validate)
  ->then($filter)

  ->then(
    null,
    function ($error) use ($context) {
      if  ($error === null) {
        // chain was invoked without `$inputValue` (i.e. optional field)
        return $fallbackValue;
      }

      throw $error;
    }
  )
  ->then(
    null,
    function ($error) use ($context) {
      // Use the fallback value when input is invalid
      return $fallbackValue ? : throw $error;
    }
  ),
  ->then(
    null,
    function ($error) use ($context) {
      // A fixed error message when input is invalid
      throw new ValidationException("fixed error message");
    }
  )
;

$context = mixed;
$chain->resolve($inputValue);

Options:

  • Reject promise using a reject method callback instead throwing exceptions.

    ->then($reject($errror) {}, $inputValue);

Benefits of this design:

  • Complete customization of the pipe.
  • Avoid create two times the same object, one for validate and another one for normalize (like DateTime)

Thoughts?

/cc @weierophinney, @Ocramius, @bakura10, @zendframework/community-review-team

@zf2timo
Copy link

zf2timo commented Jan 20, 2016

In which context do you want to create and execute the chain. I think this could lead to heavy objects, because there are so much dependencies (Hydrator, Validator, Filter, ...).

And in my opinion the code is really hard to read.

But aside of this, it could be a great feature.

@Maks3w
Copy link
Member Author

Maks3w commented Jan 20, 2016

Chain can be created from static methods and added to input filters

$inputFilter->addInput("input name", Chain $chain);
$result = $inputFilter->resolve($inputValuesArray);

$result->isValid();
$result->getValues();

About the code legibility this is as easy as use local vars or refactor dependent components for be API compatible.

$removeWhitespaces = ...;
$validateIsFormattedAsADate = ...;

$chain
  ->then($removeWhitespaces)
  ->then($validateIsFormattedAsADate)
  ->then(null, $setFallbackValue)
;

About the dependencies, there is no dependencies, just callables.

@Maks3w
Copy link
Member Author

Maks3w commented Jan 20, 2016

Note this way of make chains could replace or remove ValidatorChain and FilterChain classes

@stefanotorresi
Copy link
Contributor

while I like the idea of replacing *Chain classes with pipes of callbacks, I'm not keen on using A+ promises terminology for something that is not async at all. It may be confusing for people not familiar with such concepts, and it feels like mixing apples and oranges.

@Maks3w
Copy link
Member Author

Maks3w commented Feb 19, 2016

Well, who said this things could not be async in the future or with alternative PHP engines.

@stefanotorresi
Copy link
Contributor

If that was the actual intention, I'd rather have the input filter wrapped in a promise via a dedicated implementation like reactphp/promise, or introduce a new zend component for that purpose, but then again... why would you do that?
A promise-like interface has many aspects that are not that useful in this particular case, all you want to do is just pass an array through a stack of callbacks... Why not rethink the current Chain class to be more generic and simple instead, and avoid the design overhead of things like a "fake-promise" resolution and rejection?

@weierophinney
Copy link
Member

This repository has been closed and moved to laminas/laminas-inputfilter; a new issue has been opened at laminas/laminas-inputfilter#6.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants