Respect / Validation

The most awesome validation engine ever created for PHP
https://respect-validation.readthedocs.io
MIT License
5.78k stars 774 forks source link

Version 2.0 #258

Closed henriquemoody closed 3 years ago

henriquemoody commented 9 years ago

We are trying to follow Semantic Versioning standards and there are a lot of issues all over the place, most of them require backward compatibility break. This issue is just the first step to make Respect\Validation 2.0 happens. Surely we need all contributors and users to make it works, so feel free to commend on this issue if you have any ideas.

Keep on mind this issue is not related to related to new rules or bug fixes which doesn't break BC, this is about improvements and big changes

:construction_worker: This issue is a work in progress! :construction_worker:

henriquemoody commented 9 years ago

It come up in #364: what is the best version name?

It seems better to be 2.0 instead of 1.0 since we have too many changes, specially because it won't have the current Concrete API we have on our current codebase.

What you think? Specially you, @Respect/troubleshooters.

userabuser commented 9 years ago

@henriquemoody If you are following SemVer and introduce a backwards incompatible change to the public API, then you should go to directly 1.0 from current stable version.

Point 8 of SemVer:

Major version X (X.y.z | X > 0) MUST be incremented if any backwards incompatible changes are introduced to the public API. It MAY include minor and patch level changes. Patch and minor version MUST be reset to 0 when major version is incremented.

henriquemoody commented 9 years ago

Thanks, @userabuser! That's what I thought.

henriquemoody commented 9 years ago

@augustohp and @alganet, we've discuss the version name in #364, any other thoughts about it?

I'm :+1: to keep the 0.x versions, and don't create a 1.x with the codebase we have and I'm :+1: to name the first major version as 1.x with the new API, despite what was commented by @userabuser.

augustohp commented 9 years ago

I'm :+1: to keep the 0.x versions, and don't create a 1.x with the codebase we have and I'm :+1: to name the first major version as 1.x with the new API, despite what was commented by @userabuser.

I would go directly into 2.0, at least we give notice to people on how different the 2.0 api is form 0.* tags.

henriquemoody commented 9 years ago

I have the same feeling about it.

henriquemoody commented 9 years ago

I renamed milestone 1.0 to 2.0.

Also, I renamed milestone 0.10 to 1.0, as suggested by @augustohp on #364 about creating an MAJOR version with the current codebase.

If anyone else disagree on that just say something before the next 72 hours.

// CC: @Respect/troubleshooters

awps commented 6 years ago

@henriquemoody @augustohp Any news. on this? I see that there is no progress for almost 3 years.
This library will get the v2.0 tag in one day or it's abandoned?

Ask the community for help if you can't maintain it anymore.

augustohp commented 6 years ago

I think this issue is dead, but not the project. The code works but I confess I am not aware of any urgent matter that needs attention, if anything @awps let us know.

henriquemoody commented 6 years ago

The help it's been asked all long, as any open source project, I haven't seen many pull requests coming up, or even ideas for solutions, mostly users complaining about bugs, functionalities and rules that are not implemented, many times support for using the library even with all the documentation we have. There is even some pull requests with "Help Wanted" label and nothing happens. Some times, when I don't fix the pull requests myself and ask for changes it takes a very long time to process feedback. Check the history of the repository list, you'll see what I'm talking about.

This library has so many cool features, but also has many bugs, some of them are really hard to fix.

I like this library a lot, and I have worked a lot on it, unfortunately I don't have as much time as I used to have and it just looks abandoned. That's why when you say "community" I can't quite understand of what you are talking about.

If you want to help, it will be more than appreciated, if you don't see how, because perhaps it's not clear enough and we should improve on that, we will be more than happy to see that happening here!

awps commented 6 years ago

@henriquemoody I have not used the library but I want to use it in an upcoming project. After browsing the issues I saw this topic and made me think that the library is not maintained. My bad, I should check the commits first to realize that the last one was 2 months ago. πŸ‘

Anyway, I think that this library still needs a major release and I hope for at least:

Unfortunately, my time is limited as well, but I would like to contribute if we come to a decision.

augustohp commented 6 years ago

Proper docblock comments so the IDE's can understand what's going on.

Rule classes are pretty straightforward and your IDE should auto-complete things when using them directly. Through the static validation API, IDE hints are present. Can you further elaborate what you expect that isn't happening?

Custom error messages. Means that we would be able to translate them according to how it's done in our CMS or Framework.

Not many people use this but you can customize messages or add an adapter to translate them.

Anyway, I think that this library still needs a major release and I hope for at least:

Although there are things we want to do and discuss, we (and a lot of people) have been using this for production environments for a couple of years without problems. If this works for you, I am sure it will be helpful - if not, for real deal breakers we will be here. πŸ˜‰

nickl- commented 6 years ago

Go Go Go Version 2.0

\o/

shyandsy commented 5 years ago

when the 2.0 will be release?

henriquemoody commented 5 years ago

Hey, @shyandsy! I wish I had the answer now, but I don't.

There is quite some progress with #921, once that's done the version will be released!

nickl- commented 5 years ago

Go @henriquemoody πŸ‘ Go @henriquemoody πŸ‘ Go @henriquemoody πŸ‘

@henriquemoody @henriquemoody you the man if you can't do it no ono can

Goooooooo @henriquemoody

πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯ πŸ’₯

sunchayn commented 5 years ago

Is there any chances to implement a new structure and API for the project with Backward Compatibility in mind ? For example by using facades or utilities classes. I think that the current API has a great potential to became messy, confusing and hard to read very quickly. I think about something like this

Validation::rules([
    'name' => 'notOptional|between:4,8|alpha',
    'phone' => 'notOptional|phone',
    'state' => '!intType',
])
->when('country equals USA', [
    'state' => 'subdivisionCode:US'
])
->messages([
    'name' => [
      'notOptional' => 'Name is required',
    ],
])->validate($_POST);

this is a rough idea and we can evolve it.

Also, I think that we should make some rules a part of Validator class or some general construction class I'm talking about rules like : when, each, key... IMO they should't be part of or documented among validation rules.

Also, I thing adding a recipes page will be useful too as I mentioned earlier the current API is a bit confusing #971

I don't know if these are suitable for v2.0 or should be discussed in future major releases

alganet commented 5 years ago

@mazentouati my 2 cents as the original developer:

The Validation::something() is already a façade! It turns the OO api into a lisp-style fluent language. The intended canonical API is the dependency injected one:

// Fluent, since 5.4 which wasn't available at 1.0
(new Rules\AllOf(new Rules\StringLength(1,15), new Rules\StringType()))->validate('something');

// Traditional DI-style API
$fifteenLength = new Rules\Length(1, 15);
$aString = new Rules\StringType();
$fifteenString = new Rules\AllOf($aString, $fifteenLength);
$fifteenString->validate($something);

The idea of having multiple façades to reuse these rules in different contexts is fine for me, but I would avoid putting it into the façade we already have. Maybe we can specialize it by popular use cases, like forms:

FormValidator::fields([
    'name' => ['notOptional', 'between' => [4,8], 'alpha'],
    'phone' => ['notOptional', 'phone'],
    'state' => ['not' => ['intType']],
])
->etc();

The array syntax above is JSON-compatible (it can be serialized and deserialized easily across languages) and it does not require a custom parser for the | and : tokens.

The FormValidator class though can have its own custom methods and sugar separate from the main API.


About the releases, 2.0 is probably OK as it is. Adding a façade could be in 2.1 or later and it will still keep backwards compatibility with 2.0. If we can keep momentum, future releases could take considerably less time.

henriquemoody commented 5 years ago

The API from Validation provides a lot of flexibility, and it can get messy when your validations are too big, but in that case you are probably already abstracting your validations. Check this out:

It you wanna customize the API, you can create a library that uses Validation under to hood and implements a different DSL on top of it. I've seem a couple:

Also, there are some libraries that wrap validation adapting it to their needs, for example:

It's been years since I released the last MINOR version of Validation, so I wouldn't like to do much more refactoring until 2.0 is released. Anyways, what you described as rules can be a rule on its own that can be released in version 2.0 or any 2.x version! (Same way I'd like to have a annotation rule)

If you're interested on doing that I may be able to help you and also pointing out some challenges you may face!

sunchayn commented 5 years ago

@henriquemoody I think it's a good a idea to link some of these adapters into the repository. As I thinking about creating an adapter for Laravel I did not check yet if there's already one. Yeah it can be a rule but as I mentioned in my previous comment thing like this and when, each, etc. shouldn't be consider rules in my opinion. As they are kind of utilities and helper that use the genuine rules under the hood. Creating a road map after the release of v2.0 would be cool.

StarveTheEgo commented 5 years ago

I see setFactory method is deleted from Validator And i see all the rules now depend on only one, default, Factory instance. And i do not find it nice Especially for Concrete API

To describe the problem, i think i can just provide some abstract examples:

Example 1:

// set Factory up somewhere once
Factory::setDefaultInstance(
    (new Factory())
        ->withRuleNamespace('My\\Validation\\Rules')
        ->withExceptionNamespace('My\\Validation\\Exceptions');
// ... then somewhere else, far from this one
$validator = new Validator();

Thats hard to determine what Factory do i use right now:

  1. In IDE i cannot Go-to-definition of my Factory instance
  2. This is a thing to remember ('we are setting up factory for all the validators somewhere'); Such things to remember are not good practice

Example 2:

// build factory somewhere, let it be something very rough, **just for example**:
class SomeClass {
    public function getProjectValidationFactory() {
        return (new Factory())
        ->withRuleNamespace('My\\Validation\\Rules')
        ->withExceptionNamespace('My\\Validation\\Exceptions');
    }
}

// then somewhere else
$validator_factory = $some_class_instance->getProjectValidationFactory();
// option A:
$validator = new Validator();
$validator->setFactory($validator_factory);
// or even something like
// option B:
$validator = $validator_factory::getValidator();
  1. Here i can always Go-to-definition to my Factory object in IDE
  2. Here i can visually, even without IDE, see that i depend on specific factory

I think it would have more sense if we would be able to set Factory method for each validator instance (not rule, even tho Validator extends AllOf rule)

StarveTheEgo commented 5 years ago

Another option would be make rules to explicitly tell what kind of exception they will throw So it will allow us to get rid of Factory usage for Exceptions

StarveTheEgo commented 5 years ago

Also i am not sure that built-in validation Exceptions must be final, because it makes me copy entire exception class when i just want to make, for example, custom alias (or enhance) for some rule (using AbstractEnvelope). Meanwhile AllOfException is not final only because ValidatorException uses it, - looks inconsistent

I vote for removing final keyword from rules exceptions

StarveTheEgo commented 5 years ago

This issue is quite old, so i probably need to summon you manually, in case notifications are not working (i am still new to GitHub, sorry) @henriquemoody

StarveTheEgo commented 5 years ago

LanguageCode rule has this string: ['', 'qaa-qtz'], // Reserved for local use I would like to know what is it for :)

henriquemoody commented 5 years ago

Hey, @StarveTheEgo, thanks for taking the time to give feedback on this issue, that gave me some food for thought! :panda_face:

About the Factory, indeed, I think it's better to define it to the Validator too, and I hate that Singleton. The reason why I implemented it that way is that it's difficult to ensure that we have the same Factory all over the chain. I want to use the Factory to create exceptions because that also allows better customization and that there will be always an exception available.

The "AllOfException", as the same as the "AllOf" rule, is not final yet because I'm working on it. In fact that the only thing that is preventing #921 to get closed. All the exceptions should be final, their templates are in a property, so even if you extend, or customize that, you would still need to copy them. If that's not the case, I would very much like that you create another issue so we can make their API more flexible!

About ['', 'qaa-qtz'], // Reserved for local use: http://www.loc.gov/standards/iso639-2/php/langcodes-keyword.php?SearchTerm=qtz&SearchType=ALL


The notifications are enabled, it's just that I've been a bit busy. However, since it's such a big issue, I suggest you create new ones to talk about more specific subjects, or else it gets difficult to keep the conversation.