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

[data-mapper] Validate behavior end of life? #1184

Closed
cristianoc72 opened this issue May 11, 2016 · 2 comments
Closed

[data-mapper] Validate behavior end of life? #1184

cristianoc72 opened this issue May 11, 2016 · 2 comments

Comments

@cristianoc72
Copy link
Member

I'm opening this issue only for a discussion.

Even if I'm very fond to Validate behavior, because it was my first contribution to Propel project, imho we should re-think to it, inside the new data mapper architecture.

Since data mapper relies on POPOs, Validate behavior has no more sense for me, in fact there'll be no more validate() method inside the entities. For NestedSet and Sortable we have a new manager class, to apply these behaviors to the entities, but for Validate, the Symfony Validator class itself can do the job without adding any wrapper. Furthermore, I spend the same time to write rules into the schema.xml or in a separated yaml file, so we have no advantage on using it.
So, in my opinion we should remove the Validate behavior at all 😭 or, at least, leave it only for the ActiveRecord way.

The alternative could be to use Validate behavior as a facility to auto-generate a yaml file, to use with Symfony Validator, based on schema data types.
E.g. the following schema

<database>
    <entity name="Person">
        <field name="Name" type="varchar" size="100" required="true" />
        <behavior name="validate" />
    </entity>
</database>

could generate the following validation.yaml

Person:
    properties:
        name:
            - NotBlank: ~
            - Length:
                max: 100

Anyway, this violates the "separation of concerns" principle: it isn't an ORM business.

So, let's discuss/vote:

  1. remove the Validate behavior
  2. keep it only for ActiveRecord
  3. refactor it to generate yaml file
@marcj
Copy link
Member

marcj commented May 11, 2016

I'm for removing it 👍 In fact I already removed two behaviors with the last data-mapper commit: validate and versionable (but not for a the sake of completely removal, but rather with the thought to just move it to a separat package/repository, so the core is lighter) However, it's good to have less behavior in the core as this is currently our main delay for the data-mapper. I also agree that validation should be made with other libraries.

@mpscholten
Copy link
Member

👍 also for removing it. Once the data mapper is ready, we can still decide to rebuild it or atleast add documentation on best practises around validating propel models with external libraries.

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

No branches or pull requests

3 participants