Add support for UnsupportedStatementVersionException from php-xapi/exception

[Lctrs]

Goal of this exception is to be catched by the SerializerListener in php-xapi/lrs-bundle to throw a BadRequestHttpException with a specific message.

[Lctrs]

Why this ?

It seems logical to me that our serializer implementations throw an exception if they try to deserialize a statement at a version they don't support.

This will potentially prevent the Symfony Serializer to throw more obscure exceptions, while this exception make the root of the problem pretty clear.

If we make this check after the SerializationListener has done its job in our LrsBundle, we can potentially never reach this check because of the said possible problems that can occurs during the deserialization process because of the version mismatch.

WDYT @xabbuh ?

[xabbuh]

Sounds like a good idea to me. But what do you think if we move this exception to the php-xapi/exception package? It will probably prove to be useful in other contexts too (and I don't think we need a specific implementation here).

[xabbuh]

We will also need to update the changelog.

[Lctrs]

Why not. It might be useful in the repository part too.

[Lctrs]

What parent exception should it extend then @xabbuh ?

[xabbuh]

Do we want it to extend another extension?

[Lctrs]

Having it extending \InvalidArgumentException seems the more logical to me.

[xabbuh]

One thing that still bothers me: We currently only cover the deserialization part. What if at some point the spec introduces breaking changes which we would need to cover in the model package? From the outside the object might still be a Statement instance, but it may be composed in other ways. Making the new generic UnsupportedStatementVersionException extend the StatementDeserializationException will block us from throwing a similar one during normalization. I think we should somehow reflect this in the exception's class name.

[Lctrs]

See php-xapi/exception#1 :)

[xabbuh]

I thought about this once more and I think we should not mix low-level exceptions (like the ones from the serializer package) with high-level exceptions (the exceptions from the exceptions package). Instead, it's the responsibility of an LRS or the client library to map low-level to high-level exceptions. Therefore, I think we should still merge this PR, but with a slightly different exception name (by the way, php-xapi/exception#1 is merged ;)).

[Lctrs]

I don't see the value of this.

In the LrsBundle, we don't map low-level to high-level exceptions but we catch our exceptions - whatever library they are coming from - and throw the good HttpException from Symfony according to the spec.

[xabbuh]

Good point, I didn't think of some of other other exceptions. Can you then update the exception namespace to use our new one and also add php-xapi/exception to the required packages?

[Lctrs]

@xabbuh Done.

[xabbuh]

Thank you @Lctrs.