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

FormTypeValidatorExtension uses wrong entity #11036

Closed
tobias-93 opened this issue Jun 2, 2014 · 15 comments
Closed

FormTypeValidatorExtension uses wrong entity #11036

tobias-93 opened this issue Jun 2, 2014 · 15 comments

Comments

@tobias-93
Copy link
Contributor

The variable $validator of Symfony\Component\Form\Extension\Validator\Type\FormTypeValidatorExtension uses Symfony\Component\Validator\ValidatorInterface, which is moved to Symfony\Component\Validator\Validator\ValidatorInterface since Symfony 2.5 . Please change this in this entity.

Regards, Tobias Feijten

@stof
Copy link
Member

stof commented Jun 2, 2014

It is not moved. There is a new interface in Symfony 2.5, while the old one is still there.

However, it is true that the FormTypeValidatorExtension should be updated to accept both implementations

@cjunge-work
Copy link

I've just updated to Symfony 2.5, and using the new validator with forms is throwing an error related to the expected interface being wrong. I have set my config to api: 2.5.

As stated above, the Validator is being created using the new classes (implementing Symfony\Component\Validator\Validator\ValidatorInterface) while the Form extension is expecting the old interface. This makes the out-of-the-box experience unusable.

By changing the config to api: auto it works correctly.

@stof
Copy link
Member

stof commented Jun 3, 2014

@cjunge-work given that most bundles still support Symfony 2.3 as it is the LTS, switching to api: 2.5 is not the best idea yet. It is much better to run in BC mode (i.e. supporting both the 2.4 and 2.5 apis), which is done by api: auto on PHP 5.3.9+

@tobias-93
Copy link
Contributor Author

@stof Same goes for Symfony\Component\Form\Extension\Validator\EventListener\ValidationListener, by the way

@cordoval
Copy link
Contributor

this is messed up then:

 Matthias\SymfonyServiceDefinitionValidator\Exception\InvalidServiceDefinitionsException]
Service definition validation errors (2):

form.type_guesser.validator: Argument for type-hint "Symfony\Component\Validator\MetadataFactor
yInterface" points to a service of class "Symfony\Component\Validator\ValidatorInterface"
validator: Class "Symfony\Component\Validator\ValidatorBuilderInterface" does not exist

related matthiasnoback/symfony-service-definition-validator#12

@matthiasnoback
Copy link

Indeed, the service definitions are invalid here (see the issue that @cordoval pointed to). The form.type_guesser.validator needs an instance of MetadataFactoryInterface for which the validator service is used, but that service is defined as being of class ValidatorInterface (which itself does not extend from MetadataFactoryInterface, though the actual Validator class does).

I think this should be fixed for consistency sake. I will make a pull request for this later.

@tobias-93
Copy link
Contributor Author

@stof Any update on this work yet? @cordoval I don't think your pull request will fix this ticket, but it will fix the problem you have.

@marfillaster
Copy link
Contributor

Also to add, using auto api causes errors of different fields to pile up onto a single field, usually the last. 2.4 api doesn't have this issue.

@sebastianblum
Copy link

in my opinion, from the validator is only the validate method used.
and this method is available in the validator until 2.4

public function validate($value, $groups = null, $traverse = false, $deep = false);
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/ValidatorInterface.php#L46

and in the new validator since 2.5

public function validate($value, $constraints = null, $groups = null);
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Validator/Validator/ValidatorInterface.php#L44

So I think we can solve this issue with a code like this:

<?php
public function __construct($validator)
{
    if ($validator instanceof \Symfony\Component\Validator\ValidatorInterface  // 2.4
        || $validator instanceof \Symfony\Component\Validator\Validator\ValidatorInterface // 2.5
    ) {
        $this->validator = $validator;
    } else {
        throw new \InvalidArgumentException('Validator should be instanceof ValidatorInterface');
     }
    $this->violationMapper = new ViolationMapper();
}

The instance of checks is not a nice solution, but I works.

@webmozart do you have a better solution?

I can create a pull request and with tests, if the core members think this could be a solution for the problem.
greetings from munich, sebastian

@sebastianblum
Copy link

@stof
Copy link
Member

stof commented Jul 8, 2014

@sebastianblum doing instanceof checks in the constructor is the right solution IMO. Can you send a PR fixing it ?

@sebastianblum
Copy link

Yes, I will do it for the FormTypeValidatorExtension and theValidationListener

@tobias-93
Copy link
Contributor Author

@sebastianblum I think #11049 can be solved this way as well. Could you do this for ConstraintValidator and ConstraintValidatorInterface as well?

@sebastianblum
Copy link

Hello!

I created a pull request #11350

can you please check my commit and discuss it.

If it is ok, I will try to fix #11049

fabpot added a commit that referenced this issue Jul 15, 2014
…11036 (Sebastian Blum)

This PR was submitted for the master branch but it was merged into the 2.5 branch instead (closes #11350).

Discussion
----------

[2.5][Form] solved dependency to ValidatorInterface, fix #11036

| Q             | A
| ------------- | ---
| Bug fix?      | [yes]
| New feature?  | [no]
| BC breaks?    | [no]
| Deprecations? | [no]
| Tests pass?   | [yes]
| Fixed tickets | #11036, #11345
| License       | MIT
| Doc PR        |

Since Symfony 2.5

The problem was that the form component has a hardcoded depencency to the deprecated validator component (api Version 2.4)
The pull request fixes the dependency to the validator component and supports now both implementations, apiVersion 2.5 and apiVersion 2.4 of the validator component.

@symfony Core Members
please review the changes https://github.com/sebastianblum/symfony/blob/0a1e9c208f8730219bebf89f6696b246a0c88da7/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php
I'm not sure if it was the right solution

Commits
-------

705d67b [2.5][Form] solved dependency to ValidatorInterface, fix #11036
@fabpot fabpot closed this as completed Jul 15, 2014
@tobias-93
Copy link
Contributor Author

@sebastianblum Thanks!

fabpot added a commit that referenced this issue Jul 16, 2014
…11036 (Sebastian Blum)

This PR was squashed before being merged into the 2.6-dev branch (closes #11350).

Discussion
----------

[2.5][Form] solved dependency to ValidatorInterface, fix #11036

| Q             | A
| ------------- | ---
| Bug fix?      | [yes]
| New feature?  | [no]
| BC breaks?    | [no]
| Deprecations? | [no]
| Tests pass?   | [yes]
| Fixed tickets | #11036, #11345
| License       | MIT
| Doc PR        |

Since Symfony 2.5

The problem was that the form component has a hardcoded depencency to the deprecated validator component (api Version 2.4)
The pull request fixes the dependency to the validator component and supports now both implementations, apiVersion 2.5 and apiVersion 2.4 of the validator component.

@symfony Core Members
please review the changes https://github.com/sebastianblum/symfony/blob/0a1e9c208f8730219bebf89f6696b246a0c88da7/src/Symfony/Component/Form/Extension/Validator/ValidatorExtension.php
I'm not sure if it was the right solution

Commits
-------

ab765c9 [2.5][Form] solved dependency to ValidatorInterface, fix #11036
fabpot added a commit that referenced this issue Jul 16, 2014
* 2.5:
  [Process] Adjust PR #11264, make it Windows compatible and fix CS
  [Process] Fix unit tests on Windows platform
  bumped Symfony version to 2.5.3
  bumped Symfony version to 2.4.9
  bumped Symfony version to 2.3.19
  updated VERSION for 2.5.2
  updated CHANGELOG for 2.5.2
  updated VERSION for 2.4.8
  updated CHANGELOG for 2.4.8
  [2.5][Form] solved dependency to ValidatorInterface, fix #11036
  updated VERSION for 2.3.18
  update CONTRIBUTORS for 2.3.18
  updated CHANGELOG for 2.3.18
  [Process] Use correct test for empty string in UnixPipes

Conflicts:
	src/Symfony/Component/HttpKernel/Kernel.php
	src/Symfony/Component/Process/ProcessPipes.php
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants