Skip to content

Conversation

DonCallisto
Copy link
Contributor

@DonCallisto DonCallisto commented May 24, 2016

Q A
Doc fix? yes
New docs? no
Applies to >=2.3
Fixed tickets

You MUST specify an alias because

//Symfony\Bundle\FrameworkBundle\Validator\ConstraintValidatorFactory.php
[...]
// $name will be a FQCN and will not be present in $this->validators array
$name = $constraint->validatedBy();

if (!isset($this->validators[$name])) {
    $this->validators[$name] = new $name(); //LOOK HERE; IF I DON'T SPECIFY THIS ALIAS, I WILL END HERE: NO SERVICE!
} elseif (is_string($this->validators[$name])) {
    $this->validators[$name] = $this->container->get($this->validators[$name]);
}

[...]

So if custom validator is a service that needs to be taken from Container, there is no valid alternative to alias: it should be mandatory (and should be reported onto documentation) as you can see from this factory.

You have to specify an alias as of

```
//Symfony\Bundle\FrameworkBundle\Validator\ConstraintValidatorFactory.php
[...]
$name = $constraint->validatedBy();

if (!isset($this->validators[$name])) {
    $this->validators[$name] = new $name(); //LOOK HERE; IF I DON'T SPECIFY THIS ALIAS, I WILL END HERE; NO SERVICE THERE!
} elseif (is_string($this->validators[$name])) {
    $this->validators[$name] = $this->container->get($this->validators[$name]);
}

[...]
```
@marcoalbarelli
Copy link

+1
Also this is affecting also past versions of Symfony, not just the 3.x ones

@DonCallisto
Copy link
Contributor Author

DonCallisto commented May 24, 2016

@marcoalbarelli yes, is true: I forgot to report this.

@@ -158,7 +158,7 @@ Constraint Validators with Dependencies
If your constraint validator has dependencies, such as a database connection,
it will need to be configured as a service in the Dependency Injection
Container. This service must include the ``validator.constraint_validator``
tag and may include an ``alias`` attribute:
tag and *MUST* include an ``alias`` attribute:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add that alias must match what is returned by the validateBy method and that it must not necessarily match the service id

@marcoalbarelli
Copy link

marcoalbarelli commented May 24, 2016

Just to be on the nitpicking side of things: it should be noted that if the alias value matches the FQCN of the *Validator class the validatedBy override can be omitted

<service id="somebundle.file.validator.base_validator" class="SomeBundle\Validator\Constraints\BaseValidator">
      <tag name="validator.constraint_validator" alias="SomeBundle\Validator\Constraints\BaseValidator"/>
      <!-- rest of the service definition -->
</service>

@DonCallisto
Copy link
Contributor Author

@marcoalbarelli yes but I suppose that's not a Symfony standard anymore that way :)

@marcoalbarelli
Copy link

I didn't find any explicit constraint or standard on what should be used as an alias in "normal" services definitions, but since the alias should match a service id, which in turn has to be a valid yaml key, and since a FQCN is not a valid yaml key, I think services ids are de-facto standardized to what can be used as a valid yml key.

In this context though the term alias denotates a different thing. It doesn't match a yml key, but the output of a function.
I agree that this is a grey area and some clearing up should be performed.

Just for reference: the xsd for the service tag only says that the id property must be a string.
More strict checking is surely possible in xsd's (check the boolean type on that file for instance) but none is configured

@marcoalbarelli
Copy link

I can confirm that there is no standard in service id naming and since 3.1 any utf8 string will be considered as valid.
I just got a PR rejected for this reason

@javiereguiluz
Copy link
Member

The current docs state that alias is optional. We need a definitive decision about this (it's required/optional/convention, it's for 2.x/3.x, etc.):

alias_key

@javiereguiluz
Copy link
Member

After checking the source code of the validator component, I'm closing this in favor of #7489. But I'm not 100% sure, so I've opened symfony/symfony#21610 too. Thanks!

xabbuh added a commit that referenced this pull request Feb 28, 2017
This PR was squashed before being merged into the 2.7 branch (closes #7489).

Discussion
----------

Reworded a note about custom constraints

This replaces #6610.

Commits
-------

a9d59fd Reworded a note about custom constraints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants