Skip to content

Commit

Permalink
[Validator] Renamed @Validation constraint to @set
Browse files Browse the repository at this point in the history
  • Loading branch information
@fabpot
Bernhard Schussek authored and fabpot committed Jan 3, 2011
1 parent ba422e8 commit 708c780
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 6 deletions.
2 changes: 1 addition & 1 deletion ...nent/Validator/Constraints/Validation.php → ...y/Component/Validator/Constraints/Set.php
View file
Expand Up @@ -11,7 +11,7 @@
* with this source code in the file LICENSE.
*/

class Validation
class Set
{
public $constraints;

Expand Down
8 changes: 4 additions & 4 deletions src/Symfony/Component/Validator/Mapping/Loader/AnnotationLoader.php
View file
Expand Up @@ -14,7 +14,7 @@
use Symfony\Component\Validator\Exception\MappingException;
use Symfony\Component\Validator\Mapping\ClassMetadata;
use Doctrine\Common\Annotations\AnnotationReader;

This comment has been minimized.

Sign in to view

Copy link
@hhamon

hhamon Feb 27, 2011

Contributor

The Validator component should not rely on Doctrine API... Why don't copy the Annotation reader class from Doctrine in the Validator component ?

This comment has been minimized.

Sign in to view

Copy link
@beberlei

beberlei Feb 27, 2011

Contributor

This is the AnnotationLoader, which uses Doctrine Annotations. There is XML and YAML for those that don't want to have the Doctrine dependency.

This comment has been minimized.

Sign in to view

Copy link
@Seldaek

Seldaek Feb 27, 2011

Member

Maybe it'd make sense to move the annotation parser out of doctrine as a standalone lib? Sounds like a common-enough use case that could be useful to many.

This comment has been minimized.

Sign in to view

Copy link
@beberlei

beberlei Feb 27, 2011

Contributor

Its inside common, that are about 10-20 files. I don't see why this should be necessary, maybe subtree split it.

This comment has been minimized.

Sign in to view

Copy link
@Seldaek

Seldaek Feb 27, 2011

Member

It's not about the amount of files, and I do realize it's in common, it's just imo about sending a message that this is a standalone annotation parsing lib vs. being a part of the doctrine ecosystem, which many associate mostly with the ORM/ODM, and not reusable bits.

Also I have been bitten in the past by deleting "useless" stuff from my vendor dir, including doctrine-common, just because I didn't think of that dependency, even though I know it's there, so I would expect others to do the same mistake. Anyways it's not a big issue, just explaining my POV.

This comment has been minimized.

Sign in to view

Copy link
@hhamon

hhamon Feb 27, 2011

Contributor

That completely makes sense. By definiton a Component is a standalone API that can be used out of the framework. And that the case for me. I just want the Form component without Symfony and I would like to use annotations to define validation rules. I don't want to use XML or YAML files to externalize validation rules because it sucks. That's why the Form component must not rely on Doctrine and should embed its own annotations parser implementation.

This comment has been minimized.

Sign in to view

Copy link
@beberlei

beberlei Feb 27, 2011

Contributor

NIH syndrome? No thank you.

This comment has been minimized.

Sign in to view

Copy link
@hhamon

hhamon Feb 27, 2011

Contributor

The goal is not to reinvent the wheel as the Doctrine annotations loader does the job but as Jordi said, it should be better to externalize the annotations parser component in its own repository maybe as it can benefit for everyone.

This comment has been minimized.

Sign in to view

Copy link
@Seldaek

Seldaek Feb 27, 2011

Member

I think what he means is more copying the Doctrine AnnotationReader stuff into the component, not rewriting it. Of course that introduces some extra work to keep the two in sync, but it's not quite NIH :)

This comment has been minimized.

Sign in to view

Copy link
@hhamon

hhamon Feb 27, 2011

Contributor

Anyway, the problem here is just the use of an external library that avoids the Form component to become standalone.

This comment has been minimized.

Sign in to view

Copy link
@stof

stof Feb 27, 2011

Member

It is standalone when you use XML or YAML. Components can have optionnal dependencies IMHO. But I agree it should be said clearly in the doc of the component.

Copying the Doctrine reader just sucks for autoloading when using Doctrine Common as these classes would be registered twice (one in Doctrine Common and one in Form) unless you don't register it automatically but then the issue is the same than requiring the external lib: the end-user has to know about the dependency.

This comment has been minimized.

Sign in to view

Copy link
@kriswallsmith

kriswallsmith Feb 27, 2011

Contributor

-1 for copying the dependency. Symfont2 relies on the user to install the necessary dependencies.

Not to nit-pick, but YAML requires the YAML component, so XML is the only natively supported driver.
use Symfony\Component\Validator\Constraints\Validation;
use Symfony\Component\Validator\Constraints\Set;
use Symfony\Component\Validator\Constraints\GroupSequence;
use Symfony\Component\Validator\Constraint;

Expand Down Expand Up @@ -46,7 +46,7 @@ public function loadClassMetadata(ClassMetadata $metadata)
$loaded = false;

foreach ($this->reader->getClassAnnotations($reflClass) as $constraint) {
if ($constraint instanceof Validation) {
if ($constraint instanceof Set) {
foreach ($constraint->constraints as $constraint) {
$metadata->addConstraint($constraint);
}
Expand All @@ -62,7 +62,7 @@ public function loadClassMetadata(ClassMetadata $metadata)
foreach ($reflClass->getProperties() as $property) {
if ($property->getDeclaringClass()->getName() == $className) {
foreach ($this->reader->getPropertyAnnotations($property) as $constraint) {
if ($constraint instanceof Validation) {
if ($constraint instanceof Set) {
foreach ($constraint->constraints as $constraint) {
$metadata->addPropertyConstraint($property->getName(), $constraint);
}
Expand All @@ -81,7 +81,7 @@ public function loadClassMetadata(ClassMetadata $metadata)
// TODO: clean this up
$name = lcfirst(substr($method->getName(), 0, 3)=='get' ? substr($method->getName(), 3) : substr($method->getName(), 2));

if ($constraint instanceof Validation) {
if ($constraint instanceof Set) {
foreach ($constraint->constraints as $constraint) {
$metadata->addGetterConstraint($name, $constraint);
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Symfony/Tests/Component/Validator/Fixtures/Entity.php
View file
Expand Up @@ -10,7 +10,7 @@
* @Symfony\Tests\Component\Validator\Fixtures\ConstraintA
* @validation:Min(3)
* @validation:Choice({"A", "B"})
* @validation:Validation({
* @validation:Set({
* @validation:All({@validation:NotNull, @validation:Min(3)}),
* @validation:All(constraints={@validation:NotNull, @validation:Min(3)})
* })
Expand Down

0 comments on commit 708c780

Please sign in to comment.