Skip to content

Problems with the validation of the include option #41

Open
@christophersansone

Description

@christophersansone

(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:

  1. It cannot possibly validate relationships that have a dynamic serializer, such as polymorphic or a serializer 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.

  2. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions