[Validator][ConstraintViolation] Implemented Serializable

[alexmarucci]

Implemented serializable and ignore the root property to avoid exception.

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

[Simperfit]

If it's a bugfix it should be in the lower branch where the bug exists.

Could you please check why the tests are not passing ?

[nicolas-grekas]

Reading the linked issue, I understand why this has been submitted ($this->root may be unserializable), but from a local pov, not sure it makes sense.
Any other idea?

[xabbuh]

moving to the 3.4 milestone as the last bugfix release for 3.3 was published today

[alexmarucci]

Another solution could be to check in FormError if the cause itself is serializable, as @ogizanagi suggest in the linked issue.
Something like:
$this->isSerializable($this->cause) ? $this->cause : null,
That would prevent the Exception to be trown.
isSerializable would be a private function inside FormError class like so;
private function isSerializable($property) { try { serialize( $property ); return true; } catch (\Exception $e) { return false; } }

What do you guys think about that solution ?

[nicolas-grekas]

Implementing isSerializable is generally not possible: some internal classes throw a fatal error instead of an exception. This would be very fragile.
Any other approach that would not require this? @ogizanagi maybe?

[ogizanagi]

Unfortunately, I don't have much more appart making ConstraintViolation implement \Serializable as here and ignore FormError::$cause, ConstraintViolation::$cause,$root in the serialization, unless scalar or implementing \Serializable.
Or implement Form::__sleep() or FormConfigInterface::__sleep().
But that's more workarounds than proper fixes :/

[xabbuh]

this is not required as symfony/validator is listed under require-dev

[xabbuh]

👍 for implementing \Serializable and ignoring the cause if it cannot be serialised

[alexmarucci]

Hey Guys,

Couldn't find the reason why tests are failing on travis. If someone has any idea is more than welcome.

[ro0NL]

Note you can already serialize the violation list using the Serializer, e.g. https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Serializer/Normalizer/ConstraintViolationListNormalizer.php

Which will work with any ConstraintViolationListInterface object, not just ConstraintViolation. But implementing serializable here doesnt hurt i guess :)

[fabpot]

Do we still need this PR after @ro0NL's comment?
If yes, this should be rebased on master as this is a new feature.

[ogizanagi]

I missed @ro0NL 's comment on this PR. I might be misinterpreting, but I fail to understand how it relates to the original issue. Yes, constraint violations can be serialized using the Symfony serializer.
But the original issue is about serializing a FormError using PHP as FormError implements \Serializable.

However, I don't know the use-case, and perhaps what's wrong here is letting users think a FormError can be safely serialized using PHP (not sure why the interface was added on this class in the first place? Probably for an older version of the profiler). Should we deprecate the \Serializable implementation in FormError?

[fabpot]

Deprecating \Serializable on FormError sounds like a great idea. I don't think we want to support serializing for errors anyway.

[xabbuh]

Aren't form errors serialised when the profile is written to the filesystem?

[ogizanagi]

@xabbuh : I don't think that's true anymore. We serialize Data (VarDumper) clones.

[nicolas-grekas]

I'm closing here as serializing is not a primary use case of the class. Using VarDumper for cloning is used internally - and should/could be used in similar userland cases IMHO - (using the Serializer component would work also).

Thanks @alexmarucci for helping this move forward.