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

Add constraint factory #143

Merged
merged 1 commit into from
Nov 8, 2015
Merged

Add constraint factory #143

merged 1 commit into from
Nov 8, 2015

Conversation

Maks3w
Copy link
Contributor

@Maks3w Maks3w commented Apr 5, 2015

This factory can be injected and allow extend built-in constraint types with custom ones.

This factory allows extend the constraint system with custom ones.

Example:

<?php

class MyFactory extends JsonSchema\Constraints\Factory {
     public function factory($constraintName) {
           if ($constraintName === 'custom_constraint') {
               return new CustomConstraint($this->uriRetriever);
           }

           return super::factory($constraintName);
     }
}

$validator = new Validator(Constraint::CHECK_MODE_NORMAL, null, new MyFactory());

This especially usefull when you need extend FormatConstraint for to validate custom formats.

@Maks3w
Copy link
Contributor Author

Maks3w commented Jun 11, 2015

@bighappyface ping

@bighappyface
Copy link
Collaborator

+1

@alecsammon would you mind giving this a review?

* @return ConstraintInterface|ObjectConstraint
* @throws InvalidArgumentException if is not possible create the constraint instance.
*/
public function factory($constraintName)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might be clearer if called getConstraintValidator or something similar. The class is a factory, this is a getter/builder.

@alecsammon
Copy link
Contributor

Hi @Maks3w. Looks like a good PR, is essential that custom validation formats are supported by this library.

I've put quite a few comments on, most of these are to do with my personal style, so happy if you disagree and say no!

Thanks for you patience with this PR.

@Maks3w
Copy link
Contributor Author

Maks3w commented Jun 13, 2015

Done

@alecsammon
Copy link
Contributor

Hi @Maks3w - great - thank you...

@bighappyface - one question for you - if the Factory should be renamed to ConstraintFactory

Otherwise I'm happy - once answered then it's ready to merge

+1

@Maks3w
Copy link
Contributor Author

Maks3w commented Jun 13, 2015

About class name rules I have two:

  1. If there are namespaces there is no need for add suffixes or preffixes except the required by PSR-1
  2. If Utility classes will share the namespace and may could be name conflicts inside the namespace then non Utitily classes will be suffixed with the role.

@Maks3w
Copy link
Contributor Author

Maks3w commented Jun 13, 2015

Travis-CI error is not related to this changes.

@bighappyface
Copy link
Collaborator

@Maks3w would you please rebase this on master for the latest?

This factory can be injected and allow extend built-in constraint types with custom ones.
@Maks3w
Copy link
Contributor Author

Maks3w commented Jun 14, 2015

@bighappyface rebased and squashed

* @param int $checkMode
* @param UriRetriever $uriRetriever
* @param Factory $factory
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing |null

@bighappyface
Copy link
Collaborator

@Maks3w sorry for the wait on this. I'll try to get this in with #182 as the next release.

bighappyface added a commit that referenced this pull request Nov 8, 2015
@bighappyface bighappyface merged commit 09ab4df into jsonrainbow:master Nov 8, 2015
@Maks3w Maks3w deleted the feature/constraint-factory branch November 8, 2015 13:43
@AubreyHewes AubreyHewes mentioned this pull request Nov 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants