Skip to content

[Validator] Added missing error codes and turned codes into UUIDs #12388

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

Closed
wants to merge 22 commits into from

Conversation

webmozart
Copy link
Contributor

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets -
License MIT
Doc PR -

With the current implementation of error codes, checking which error occurred is unnecessarily complex:

if ($violation->getConstraint() instanceof Length && Length::TOO_SHORT_ERROR === $violation->getCode()) {
    // ...
}

Also, the code is completely missing for some constraints. This is fixed now. By using UUIDs, the check is reduced to:

if (Length::TOO_SHORT_ERROR === $violation->getCode()) {
    // ...
}

Also, APIs can simply output the error code and the name of the error without needing to point to the constraint as well.

Before:

[
    {
        "code": "1",
        "name": "TOO_SHORT_ERROR",
        "message": "This value is too short. ...",
        "constraint": "Symfony\Component\Validator\Constraints\Length"
    }
]

After:

[
    {
        "code": "9ff3fdc4-b214-49db-8718-39c315e33d45",
        "name": "TOO_SHORT_ERROR",
        "message": "This value is too short. ..."
    }
]

This makes it possible to implement a service on symfony.com which looks up error codes, e.g.

symfony.com/error?code=9ff3fdc4-b214-49db-8718-39c315e33d45

Such a URL could redirect directly to the documentation of the appropriate constraint. We could even support user-submitted error codes which redirect to the documentation of that constraint.

@Tobion
Copy link
Contributor

Tobion commented Nov 3, 2014

I think there are still constraints without error codes like Null and False.

@webmozart
Copy link
Contributor Author

@Tobion #12389 :)

@fabpot
Copy link
Member

fabpot commented Nov 3, 2014

👍

@aeoris
Copy link
Contributor

aeoris commented Nov 3, 2014

👍

Just the way I like it... thanks @webmozart!

@webmozart
Copy link
Contributor Author

This was lost in 2.7. However, it shouldn't be a problem to merge this into 2.8 as the error code values are internal details.

jaytaph and others added 21 commits June 20, 2015 19:03
…r into StringUtil (issei-m)

This PR was squashed before being merged into the 2.8 branch (closes symfony#14660).

Discussion
----------

[Form] moved data trimming logic of TrimListener into StringUtil

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

I introduced this PR at symfony#14146, but I didn't make it on time for 2.7 feature freeze. So I'm opening the new PR for 2.8.

Following is my first introduction:

> In the small job which needs not to build a form (e.g. just process a value gotten from Request), I want to reuse the trim logic.

@dosten
@Tobion
@xabbuh
@stof
@webmozart
Thanks for all your feedback. I have fixed all issues mentioned on symfony#14146. Please re-consider it.

Commits
-------

f42c777 [Form] moved data trimming logic of TrimListener into StringUtil
… (jaytaph)

This PR was merged into the 2.8 branch.

Discussion
----------

[Console][Table] allow multiple render() calls.

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Fixed tickets  | ~
| Tests pass?   | yes
| License       | MIT

This is a cherry-pick of symfony#14983.

Commits
-------

b93bcc1 Fixed colspan issues with multiple render() calls
…definition for validator mapping (jakzal)

This PR was merged into the 2.8 branch.

Discussion
----------

[FrameworkBundle] Add a doctrine cache service definition for validator mapping

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | symfony/symfony-docs#5409

Following symfony#12975, this PR only registers a new service so it's possible to use the new doctrine based cache implementation instead of the deprecated one. To use it, the end user would need to configure it in his `config.yml`:

```yaml
framework:
    validation:
        cache: validator.mapping.cache.doctrine.apc
```

In 3.0 we'll be able to replace the deprecated definition by aliasing `validator.mapping.cache.apc` to `validator.mapping.cache.doctrine.apc`.

I thought of automatic wrapping of services which implement doctrine interface, but decided it would be too magic.

I'm not convinced if APC is a good default anymore and hope for some discussion. I've used it as it's also used in serializer, and probably translation (see symfony#13986). Since there's a built in opcache in more recent PHP versions, and apcu doesn't seem to be stable, there are better choices. Perhaps a better default would be a filesystem cache (not better performing, but it works anywhere).

Commits
-------

0642911 [FrameworkBundle] Add a doctrine cache service definition for validator mapping
…dosten)

This PR was merged into the 2.8 branch.

Discussion
----------

[DependencyInjection] Deprecate scope concept

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| License       | MIT

This PR mark as deprecated the concept of scopes in the DI container. See symfony#10557 (comment).
Also adds a new `shared` flag to the service definitions in replacement of the `prototype` scope.

Commits
-------

6c4a676 Add "shared" flag and deprecate scopes concept
…e is set (WouterJ)

This PR was merged into the 2.8 branch.

Discussion
----------

[2.8] Make service not shared when prototype scope is set

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Deprecating the scopes was introducing a BC break in Symfony 2.8 when using the prototype scope. As `$shared` wasn't updated if the scope was set to `prototype`, people will always get an error because of [the `CheckDefinitionValidityPass`](https://github.com/symfony/DependencyInjection/blob/04d5d925a987f35854f51b1c468c0ca81e1d61b9/Compiler/CheckDefinitionValidityPass.php#L54-L57).

Commits
-------

b7030cc Make service not shared when prototype scope is set
…return trigger_error($e, E_USER_ERROR);` (nicolas-grekas)

This PR was merged into the 2.8 branch.

Discussion
----------

[Debug] Allow throwing from __toString() with `return trigger_error($e, E_USER_ERROR);`

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Commits
-------

f360758 [Debug] Allow throwing from __toString() with `return trigger_error($e, E_USER_ERROR);`
Also include the service id in the deprecation message.
…ssages (xabbuh)

This PR was merged into the 2.8 branch.

Discussion
----------

[2.8][DependencyInjection] improve deprecation messages

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Also include the service id in the deprecation message.

Commits
-------

d144dbb [DependencyInjection] improve deprecation messages
The `server:start` command will report an error message when a lock file
does exist.

However, this means that you cannot restart the web server process if
the previously running process terminated accidentally or if it was
terminated by the user without executing the `server:stop` command (e.g.
by using the system's `kill` command or the task manager).

This commit adds a `--force` option that makes it possible to launch the
web server process even if a lock file does exist.
… (twifty)

This PR was submitted for the 2.7 branch but it was merged into the 2.8 branch instead (closes symfony#14238).

Discussion
----------

[config] added remove option to ignoreExtraKeys

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

I have added a remove option to the `ignoreExtraKeys()` method of `Config/Definition/Builder/ArrayNodeDefinition`. The current implementation removes all such keys from the resulting array. This can at times be an inconvenience as we have to write extra methods to remove them before the validation then add them back again when done. Passing a boolean to the method now leaves them in place.

Commits
-------

1df4ebe added remove option to ignoreExtraKeys
…r startup (xabbuh)

This PR was merged into the 2.8 branch.

Discussion
----------

[FrameworkBundle] add option to force web server startup

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

The `server:start` command will report an error message when a lock file
does exist.

However, this means that you cannot restart the web server process if
the previously running process terminated accidentally or if it was
terminated by the user without executing the `server:stop` command (e.g.
by using the system's `kill` command or the task manager).

This commit adds a `--force` option that makes it possible to launch the
web server process even if a lock file does exist.

Commits
-------

1583fad add option to force web server startup
…ard paths (kbond)

This PR was submitted for the 2.7 branch but it was merged into the 2.8 branch instead (closes symfony#14764).

Discussion
----------

[TwigBundle] Warmup twig templates in non-standard paths

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#12507
| License       | MIT
| Doc PR        | symfony/symfony-docs#5391

Commits
-------

96cce38 Warmup twig templates in non-standard paths (closes symfony#12507)
fabpot added a commit that referenced this pull request Jul 1, 2015
… into UUIDs (webmozart)

This PR was merged into the 2.8 branch.

Discussion
----------

[Validator] Added missing error codes and turned codes into UUIDs

Reopened #12388 on the 2.8 branch.

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

With the current implementation of error codes, checking which error occurred is unnecessarily complex:

```php
if ($violation->getConstraint() instanceof Length && Length::TOO_SHORT_ERROR === $violation->getCode()) {
    // ...
}
```

Also, the code is completely missing for some constraints. This is fixed now. By using UUIDs, the check is reduced to:

```php
if (Length::TOO_SHORT_ERROR === $violation->getCode()) {
    // ...
}
```

Also, APIs can simply output the error code and the name of the error without needing to point to the constraint as well.

Before:

```json
[
    {
        "code": "1",
        "name": "TOO_SHORT_ERROR",
        "message": "This value is too short. ...",
        "constraint": "Symfony\\Component\\Validator\\Constraints\\Length"
    }
]
```

After:

```json
[
    {
        "code": "9ff3fdc4-b214-49db-8718-39c315e33d45",
        "name": "TOO_SHORT_ERROR",
        "message": "This value is too short. ..."
    }
]
```

This makes it possible to implement a service on symfony.com which looks up error codes, e.g.

symfony.com/error?code=9ff3fdc4-b214-49db-8718-39c315e33d45

Such a URL could redirect directly to the documentation of the appropriate constraint. We could even support user-submitted error codes which redirect to the documentation of that constraint.

Commits
-------

8874e88 [Validator] Added missing error codes and turned codes into UUIDs
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.