search

laminas / laminas-validator

Validation classes for a wide range of domains, and the ability to chain validators to create complex validation criteria
https://docs.laminas.dev/laminas-validator/
BSD 3-Clause "New" or "Revised" License
132 stars 55 forks source link

Road to ZF3 #48

Closed michalbundyra closed 2 months ago

michalbundyra commented 4 years ago

Hi,

I've done a massive PR to this (https://github.com/zendframework/zf2/pull/5067) but it's a massive work and a big hell to review.

I suggest that we:

  1. Move validators to their specifc components (for instance moving Db validators to the Zend\Db component), and deprecating the validators here (for version 2.5).
  2. Remove them in v3, and start the refactoring work. Here are various things I have done in the past:
    • Make validators stateless (each validator returns a ValidationResult, that can be translated using another component for instance).
    • Using the new service manager of ZF3.
    • Homogeneize names (in ZF2, we didn't have time to use the "underscore_separated" convention for option name, which make it very incoherent with how the framework work).
    • Various code cleaning, optimization for PHP 5.5

Originally posted by @bakura10 at https://github.com/zendframework/zend-validator/issues/1

michalbundyra commented 4 years ago

:+1: for stateless

Originally posted by @snapshotpl at https://github.com/zendframework/zend-validator/issues/1#issuecomment-106086440

michalbundyra commented 4 years ago

I suggest to split the proposal in invidual issues for to have clean discussion threads.

:+1: For place validator inside of each component.

Originally posted by @Maks3w at https://github.com/zendframework/zend-validator/issues/1#issuecomment-106247122

michalbundyra commented 4 years ago

Move validators to their specifc components (for instance moving Db validators to the Zend\Db component), and deprecating the validators here (for version 2.5).

First, v2.5 is imminent; I just need to ensure all the components are passing tests on current master, and then I'll be tagging their 2.5 versions, followed by the refactoring of the ZF2 repo to be a metapackage (see zendframework/zf2#7542 and my own tree that extends that). As such, deprecation would have to be following 2.5.

That said, I'm a bit leery of spreading the validators amongst all the various component repositories:

The second can be solved, of course, by modifying the current "suggest" section of the composer.json to refer to the validators instead of the dependency; however, it still provides overhead in consuming the validators for the end-user, as they both need to install the additional package(s) and then configure the validators in the plugin manager.

We've seen already that having validators split between zend-i18n and zend-validator causes problems when folks are looking for common validators such as alpha, alnum, DateTime, IsFloat, and IsInt; they expect them to be in zend-validator, and have no idea that they might be elsewhere.

Originally posted by @weierophinney at https://github.com/zendframework/zend-validator/issues/1#issuecomment-106575295

michalbundyra commented 4 years ago

Also, as someone who uses Zend\Validator standalone, I don't want to bring in the whole world. If anything I'd like basic i18n ones to be in Zend\Validator.

Originally posted by @akrabat at https://github.com/zendframework/zend-validator/issues/1#issuecomment-106576380

michalbundyra commented 4 years ago

I'm definitely not against bringing back validators to this package. But on the other hand it's just moving the problem : instead of introducing a validator dependency to crypt or db, we introduce a dependency to crypt and db in validator. Not sure which one is better.

Envoyé de mon iPhone

Le 28 mai 2015 à 21:43, Rob Allen notifications@github.com a écrit :

Also, as someone who uses Zend\Validator standalone, I don't want to bring in the whole world. If anything I'd like basic i18n ones to be in Zend\Validator.

— Reply to this email directly or view it on GitHub.

Originally posted by @bakura10 at https://github.com/zendframework/zend-validator/issues/1#issuecomment-106581725

michalbundyra commented 4 years ago

instead of introducing a validator dependency to crypt or db, we introduce a dependency to crypt and db in validator

Not the way we have it currently. The only hard dependency is zend-stdlib at the moment. We have 8 "suggest" components, each indicating they should be installed if you need to use specific validators. (Those same are added as dev dependencies in order to test.)

In production, you use composer install --no-dev, which means you'll not get any extra dependencies.

Originally posted by @weierophinney at https://github.com/zendframework/zend-validator/issues/1#issuecomment-106583393

michalbundyra commented 4 years ago

So whats the difference if we move the validators to their own components? :D I definitely agree that i18n should be in the validators package because they actually don't rely on any class in i18n package.

But for db and crypt they definitely should be in Zend/db and Zend/crypt as they rely on those. No?

Envoyé de mon iPhone

Le 28 mai 2015 à 22:12, weierophinney notifications@github.com a écrit :

instead of introducing a validator dependency to crypt or db, we introduce a dependency to crypt and db in validator

Not the way we have it currently. The only hard dependency is zend-stdlib at the moment. We have 8 "suggest" components, each indicating they should be installed if you need to use specific validators. (Those same are added as dev dependencies in order to test.)

In production, you use composer install --no-dev, which means you'll not get any extra dependencies.

— Reply to this email directly or view it on GitHub.

Originally posted by @bakura10 at https://github.com/zendframework/zend-validator/issues/1#issuecomment-106613510

michalbundyra commented 4 years ago

My only argument would be to choose one over the other. Like @bakura10 said it's somewhat of moving the problem from A to B. There's really no difference between those two scenarios:

The result is the same. In both scenarios the dependency is a suggest only.

It IS important though, that we choose one of the two. The way it is currently handled, some validators aren't within the zend-validator package. And truthfully, no one knows this until the ServiceManager throws the error that the validator can't be found ;) Not even myself. Quite frankly, I don't give a damn as to what packages are "suggested" towards me by composer. Suggestions don't automatically imply "requirements" if I want to use feature X or Y. And my strong guess is that I am not the only one having this interpretation of suggested packages.

Personally my vote would likely go to have all validators within zend-validator. Given the package name this feels more intuitive. And it would likely be less painful in teaching the beginner end of developers in how to be using them.

Michaël Gallego notifications@github.com schrieb am Fr., 29. Mai 2015 um 00:04 Uhr:

So whats the difference if we move the validators to their own components? :D I definitely agree that i18n should be in the validators package because they actually don't rely on any class in i18n package.

But for db and crypt they definitely should be in Zend/db and Zend/crypt as they rely on those. No?

Envoyé de mon iPhone

Le 28 mai 2015 à 22:12, weierophinney notifications@github.com a écrit :

instead of introducing a validator dependency to crypt or db, we introduce a dependency to crypt and db in validator

Not the way we have it currently. The only hard dependency is zend-stdlib at the moment. We have 8 "suggest" components, each indicating they should be installed if you need to use specific validators. (Those same are added as dev dependencies in order to test.)

In production, you use composer install --no-dev, which means you'll not get any extra dependencies.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/zendframework/zend-validator/issues/1#issuecomment-106613510 .

Originally posted by @manuakasam at https://github.com/zendframework/zend-validator/issues/1#issuecomment-106718989

michalbundyra commented 4 years ago

Only a screenshot: zend-validator :-1:

Originally posted by @froschdesign at https://github.com/zendframework/zend-validator/issues/1#issuecomment-106729996

michalbundyra commented 4 years ago

@bakura10 Could you open a different issue for each different idea? This will allow to have different discussion threads about the same thing (I hope)

By the way please reword this issue for only discuss the validator split thing.

Originally posted by @Maks3w at https://github.com/zendframework/zend-validator/issues/1#issuecomment-108002380

michalbundyra commented 4 years ago

I think we mix two different scopes sometimes. The framework scope and the library scope.

As framework we want to have a single point where to list and discover available validators. As library we want to be decoupled and provide the basis so other developers implement his/her custom validators.

This is my list of benefits of split validators in different components:

This is the list of problems if validators are split:

From my point of view the "problems" described must be resolved by the framework and not by the component at may reveal the same issues a developer has when create a custom component and expect to be used in a zendframework project.

Possible alternative solutions:

Originally posted by @Maks3w at https://github.com/zendframework/zend-validator/issues/1#issuecomment-108008355

michalbundyra commented 4 years ago

Foo-Validators are more coupled with Foo releases so component and subcomponent can be released at the same time without to wait for a Zend\Validator release

This is definitely the point that I didn't think about that definitely should push us to separate validators into separate repositories. Otherwise we are going to have the same problem as what we had in ZF2 (although at a smaller scope).

This would mean that if we update Zend\Crypt to v3 for instance (with a BC change somewhere) we'll be force to make Zend\Validator v3 also because of the dependency.

Originally posted by @bakura10 at https://github.com/zendframework/zend-validator/issues/1#issuecomment-108071837

michalbundyra commented 4 years ago

Hmm.. yeah I see that point, too.

Originally posted by @akrabat at https://github.com/zendframework/zend-validator/issues/1#issuecomment-108073327

michalbundyra commented 4 years ago

So whats the difference if we move the validators to their own components?

In a word: Discovery.

Most of the assumptions I've seen in this thread are that people are using the component in the context of Zend Framework. One of the points of splitting the components into their own repositories, however, is to emphasize that ZF is a component library foundation, on which a framework is built. This emphasis is to encourage developers to use ZF components even if they are not using the framework.

If you come to zend-validator from that direction, would you ever consider looking in zend-db for validators? or zend-i18n? or zend-crypt? Chances are that you would have no idea that those packages also deliver validators, and, when you are unable to find the validator you need in zend-validator, you move on to a different library entirely.

That's the fundamental argument I've been trying to make.

Foo-Validators are more coupled with Foo releases so component and subcomponent can be released at the same time without to wait for a Zend\Validator release This is definitely the point that I didn't think about that definitely should push us to separate validators into separate repositories. Otherwise we are going to have the same problem as what we had in ZF2 (although at a smaller scope).

This would mean that if we update Zend\Crypt to v3 for instance (with a BC change somewhere) we'll be force to make Zend\Validator v3 also because of the dependency.

Untrue. zend-validator will suggest the components on which it depends, with the versions it is tested against. The require-dev section will reference the major.minor pair for semantic versioning as well, meaning you can easily determine that zend-validator is still only supporting the v2 versions of zend-crypt.

We can, in a minor release of zend-validator up the version requirement for these specific validators.

As an example, let's say zend-validator is currently on v2.6, and suggests ~2.5 for zend-crypt if you are using the crypt-specific validators. We then release a v3 of zend-crypt. If you are using zend-validator, and require zend-crypt, you are still pulling in a v2 series zend-crypt, as zend-validator does not support v3 of zend-crypt yet. For v2.7, we make the necessary changes to support zend-crypt v3, and suggest ~3.0 for zend-crypt; a user who updates their application will now have zend-validator ~2.7 and zend-crypt at ~3.0.

The only place this will be a problem is if elsewhere in their application they want/need zend-crypt v3 while they're still on ~2.6 of zend-validator. In that particular case, the developer can always opt to write their own implementation(s) of the zend-crypt-based validator(s).

We have two paths, then:

Both have pros and cons, and lead to issues for developers.

Personally, with Apigility, I forgot that we had validators elsewhere when we wrote the zf-content-validation module. If we consider that even veteran ZF users can miss/forget this fact, what hope do new users have of finding them? As such, I recommend strongly that we keep all validators and all filters in the same packages.

Originally posted by @weierophinney at https://github.com/zendframework/zend-validator/issues/1#issuecomment-117207353

michalbundyra commented 4 years ago

With regards to the other points, I'm quite happy with this pitch. :+1:

In particular, making them stateless, and having a "validation result" object makes a ton of sense, and promotes far better re-usability. Considering that most ZF developers interact with validators primarily through InputFilters (or Forms, but Forms just delegate to InputFilters), the change will be essentially transparent for most developers as well.

Originally posted by @weierophinney at https://github.com/zendframework/zend-validator/issues/1#issuecomment-117207906

michalbundyra commented 4 years ago

Personally, with Apigility, I forgot that we had validators elsewhere when we wrote the zf-content-validation module. If we consider that even veteran ZF users can miss/forget this fact, what hope do new users have of finding them? As such, I recommend strongly that we keep all validators and all filters in the same packages.

This is because the discovery problem exists, will continue to exist and won't be fixed ever.

For me put all validators in the same repo is not an option.

Apigility is expected to have his own validators and them are not expected to be included in this repo because may to be too specific even while them could be reused.

My suggest is:

If not possible then:

As I said in the above posts. Discover is not an issue of the component, it's an issue of the Framework and should be fixed at Framework level (providing an index in ZF2 repo)

This would mean that if we update Zend\Crypt to v3 for instance (with a BC change somewhere) we'll be force to make Zend\Validator v3 also because of the dependency.

Untrue. zend-validator will suggest the components on which it depends, with the versions it is tested against. The require-dev section will reference the major.minor pair for semantic versioning as well, meaning you can easily determine that zend-validator is still only supporting the v2 versions of zend-crypt.

We can, in a minor release of zend-validator up the version requirement for these specific validators.

  1. This is a discussion about the global release process and this thread is not the best place for to discuss them.
  2. I think in the following scenario:
Root: {
 Foo: ^1,
 Baz: ^1: {
     Foo: ^1 // Same dependency as root package.
}

What happens if Baz publish a security patch 1.0.1 and raise his dependency to Foo: ^2

Root: {
 Foo: ^1,
 Baz: ^1: {
     Foo: ^2 // Version conflicts with root constraints.
}

This means the security patch (and any other non BC release) cannot be deployed without the developer raise his dependency to Foo: ^2

For me BC releases implies anyone can be trust in to have the last non BC version without to change any line of code.

Originally posted by @Maks3w at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122333713

michalbundyra commented 4 years ago

@bakura10 are you sure that point A is a good idea? in my opinion we can move db validators into the different project es. zend-db-validator-interop.. :) or similar..

Originally posted by @gianarb at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122366742

michalbundyra commented 4 years ago

Promote ValidatorPluginManager to ZF2 repo with the whole list of validators maintained by the project.

This is the thing I strongly disagree. As I said, we NEED to find a way to aggregate config for non-modules. As said several times, we should either have a concept of "mini-module" for component framework, or make each component a module, and allow syntax such as:

return [
   'modules' => [
      'Zend\*'
   ]
];

Of course, this implies that we refactor a bit the module manager to make it more lightweight, or having more aggressive caching. But I feel very bad to install a some sort of meta package just for populating the various plugin managers (as it iwll also means that any addition to any package will require us to also update this meta package).

(PS : and yes, I agree with splitting, Zend\Db validators should not belong to this package, they are to me the same concept as a Aws validator inside an Aws module, it should definitely sits in another module). Zend\Validator should focus on the basic architecture and provide dependency-free vlaidators

Originally posted by @bakura10 at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122374788

michalbundyra commented 4 years ago

@bakura10 I don't see your argument. Are you proposing some kind of autodiscover or selfregister protocol?

Originally posted by @Maks3w at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122375967

michalbundyra commented 4 years ago

Exactly. Currently to do all the wiring up inside the framework, we are using this: https://github.com/zendframework/zend-mvc/blob/master/src/Service/ServiceListenerFactory.php which is ugly because of all the hidden dependencies.

The kind of meta package you are suggesting is I think a wrong idea because it will introduce back a single package that has dependencies over each system that use a plugin manager, to wire the various plugins of each component and goes against the new philosophy of components.

I've already thought of various experiments to make each component registering their validators/filters/hydrators/whatever into the plugin manager, but I think the simplest soltuion is simply to introducing a concept of "light module" (but the problem is that it will be a bit consistent... why only the official components would use that?) or going to the logic : component become module.

But definitely, a solution needs to be found in this area :)

Originally posted by @bakura10 at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122385905

michalbundyra commented 4 years ago

Do we all agree discover feature is outside of the scope of the Validator component itself?

Originally posted by @Maks3w at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122388588

michalbundyra commented 4 years ago

Not completely, as it's a piece to making the splitting of validators/filters more easier. Ideally, people should be able to install Zend\Validator, then Zend\Db, and being able to have Zend\Db validators available in the plugin manager without having to wiring up more things.

Originally posted by @bakura10 at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122389248

michalbundyra commented 4 years ago

My point is zend-validator should have only code about definition and common tools for validation.

All those stuff about "integration", "discover" or whatever you want to name is out of the library and should be managed outside.

Zend\Validator is good as a standalone project. Wrappers about how to plug into ZF2 services should be done outside, same as if someone want to wrap the library to a SF bundle or AnotherServiceContainerProject

Outside means:

Finally, the roadmap shouln't be blocked pending about to design the autoregister protocol proposed by bakura. If we agree with the rest with the points we should start to work on them and manage a static index of validators (outside of this repo) until a best option is present.

Originally posted by @Maks3w at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122392654

michalbundyra commented 4 years ago

So you bascially suggest that we actually completely remove the ValidatorPluginManager from this library then? (otherwise your point does not make sense ^^).

Originally posted by @bakura10 at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122412857

michalbundyra commented 4 years ago

yep

Originally posted by @Maks3w at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122414637

michalbundyra commented 4 years ago

So you bascially suggest that we actually completely remove the ValidatorPluginManager from this library then? (otherwise your point does not make sense ^^).

Today is a very important day! We have ideally removed two dependencies:

from Zend\Validator! wow :+1:

Originally posted by @gianarb at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122438761

michalbundyra commented 4 years ago

This is pretty awesome! :tada:

Originally posted by @fntlnz at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122441628

michalbundyra commented 4 years ago

Ok !

I will therefore try to work on the refactor (based on my previous work and those new feedbacks!) asap :).

Originally posted by @bakura10 at https://github.com/zendframework/zend-validator/issues/1#issuecomment-122602331

michalbundyra commented 4 years ago

Before you go too far on this, I need to weigh in here.

I thoroughly disagree with both proposals: both moving component-specific validators into their components, and removing the plugin manager.

The arguments made so far center primarily around one concept: separation of concerns. Removal of component-specific validators means that the zend-validator component is both slimmer code-wise, as well as dependency-wise. Removal of the plugin manager means losing a component. In both cases, the idea is to push this stuff elsewhere: component-specific validators go into their components, and things like plugin management become a "framework concern."

My argument against the change is similarly simple to state: it puts too much onus on the consumer.

@Maks3w has alluded to one aspect already: discoverability of validators. He maintains that discoverability should be pushed into the framework, or into framework-specific packages (e.g., ZF2 modules, Symfony bundles, etc.). I think that's a cop-out: it means that if you come across the zend-validator component, it's not immediately usable. You have to either do further discovery to find the correct consumable package, or you end up having to figure out how to wire it yourself. Either way, you're left with something you cannot use easily and simply.

Second, it pushes the dependency problem elsewhere. Right now, any validator that has additional dependencies leads to a new suggest line in composer.json, which, if they were properly worded, would read something like:

"zendframework/zend-db": "~2.0 if you wish to use the various DB-centric validators"

These same dependencies are mirrored in our require-dev section to enable testing.

If we move the validators to their own components, we get multiple new problems:

Next, let's look at removal of the plugin manager.

One key usage of validators within zend-validator is via ValidatorChain. This can be used by specifying either validator instances, or validator names; when the latter are used, we pull from a plugin manager. This makes for simpler usage:

$chain->attachByName('IsInt')
    ->attachByName('Between', ['min' => 100, 'max' => 1000']);

This could just as easily be done with instances, obviously:

$chain->attach(new IsInt())
    ->attach(new Between(100, 1000));

However, the most common way to use validators is via a configuration-based tool such as zend-inputfilter or zend-form. In those cases, you're using something along these lines:

'currency' => [
    'validators' => [
        [ 'type' => 'IsInt' ],
        [ 'type' => 'Between', 'options' => ['min' => 100, 'max' => 1000]],
    ],
]

Configuration-driven usage implies factories of some sort, and for ZF2, that has traditionally meant a plugin manager.

Getting rid of the plugin manager makes consumption harder:

We solved this partially in zend-feed, by introducing a component-specific plugin manager, and a default implementation that is standalone. This allows the most common, documented use case to "just work," while reducing a dependency. For those who want the more powerful features of zend-servicemanager, an implementation is present to allow doing that as well. I strongly urge that instead of removing the plugin manager entirely, we go this route instead, as it's the most common, most consumed use case.

One key point I want to make is that with the switch to component-based repos instead of a monolithic repo, one big goal is to try and increase standalone usage of components. We cannot assume users are using the full-stack, nor that they are familiar with ZF and all of its components. The simpler we can make the standalone consumption story, the more likely we'll get such adoption.

My point is that while it may look and feel more semantically correct to move the validators into different components, or to add a layer on top in order to achieve a proper separation of concerns, such architectural changes are detrimental to actual consumers and potential users of the component. We should be looking at how the components are currently used, how people might discover and use them, and write code that facilitates those paths as succinctly and easily as possible. Pragmatic architecture, please.

Originally posted by @weierophinney at https://github.com/zendframework/zend-validator/issues/1#issuecomment-123035264

michalbundyra commented 4 years ago

For the record, I mostly care about getting the PluginManager instance out of the InputFilter instance. The factory should use the PluginManager to build all the things and give me back an InputFilter that's ready to go.

Also, I want the fully features PluginManager, so I'm fine with a ServiceManager dependency as it's really common to have custom validators.

Originally posted by @akrabat at https://github.com/zendframework/zend-validator/issues/1#issuecomment-123040816

gsteel commented 2 months ago

I'm closing this because it's pretty old 😂