Description
(Currently there is a bug that is fixed in both #13 and #32, but there is more to the story. Creating this issue to discuss further.)
The validate_includes! method is designed to check the include
option prior to serialization to see if there are any items that do not actually have a relationship defined. For example:
class FooSerializer
include FastJsonapi::ObjectSerializer
attribute :name
end
FooSerializer.new(model, { include: :bar })
This example would not pass the validation, because it is attempting to include bar
, which has not been defined as a relationship.
However, there are two problems with this:
-
It cannot possibly validate relationships that have a dynamic serializer, such as
polymorphic
or aserializer
Proc (Improved relationship serializer options #32). The best it can do is ignore these cases. Prior to the fixes in includes : disable static checks when using polymorphic relations #13 and Improved relationship serializer options #32, it would incorrectly fail the validation. -
This validation occurs for every single serialization. You can pass in the same set of includes 100K times, and it will still iterate through each item and validate it. This is extra work with performance implications for every serialization that occurs, for little benefit.
First, I think we should decide whether we want the serializer to fail in this situation. The alternative is to simply ignore any items in the list that do not apply.
If we want to ignore, then I believe all we need to do is remove this validation.
If we want it to fail, then I believe a better approach would simply be to fail when it actually attempts to serialize that included relationship, rather than to do a pre-flight validation. I think we can change get_included_records to fail when it encounters an invalid item. If we did it that way, then it would resolve the problems with the existing implementation: it could validate the polymorphic relationships with zero performance impact.