Dataloader loading incorrect association when struct Id is null

[arunr14]

Encountered a scenario where dataloader is associating the incorrect child object to a parent object. I was able to work around it by explicitly specifying the foreign key relationship in the resolver.

Environment

• Elixir version : Elixir 1.16.2 (compiled with Erlang/OTP 26)
• Absinthe version: Absinthe 1.6.8
• Client Framework and version : Apollo

ISSUE

I have a graphql object called task with a field checklist that is resolved using dataloader. I have two ecto schemas called task and recurringTask with both of them having a field checklist_id. I have a custom resolver that queries both tables, combines the data and returns a struct %task{ :id, :recurring_id, :checklist_id}. In the returned list, it is possible for some entries to have id = null. When this happens dataloader associates the entry with and incorrect checklist object i.e. task.checklist_id != task.checklist.id. Other entries in the list have task.checklist_id = task.checklist.id as expected. However, for entries with id = null dataloader seems to associate a random checklist object from the ones it has fetched.

I was able to workaround this issue by explicitly specifying the foreign key in the resolver.

Schema/Code

ECTO schema

schema "tasks" do
   <other fields>
    belongs_to :checklist, Tasks.Checklist
    
schema "recurring_tasks" do
    <other fields>
    belongs_to :checklist, Tasks.Checklist

GRAPH QL Definitions

field :tasks, non_null_list(:task) do
      resolve &Queries.list_tasks/3
end

object :task do
    ecto_fields Tasks.Task
    field :checklist, :checklist, resolve: dataloader(Tasks)
end

WORKAROUND:

object :task do
    ecto_fields Tasks.Task
    field :checklist, :checklist, resolve: fn parent, _, %{context: %{loader: loader}} -> 
      loader
      |> Dataloader.load(Tasks, {:one, Tasks.Checklist}, id: parent.checklist_id)
      |> on_load(fn loader ->
        loader
        |> Dataloader.get(Tasks, {:one, Tasks.Checklist}, id: parent.checklist_id)
        |> (&{:ok, &1}).()
    end)
  end
end

[fuelen]

I noticed the same is true when the struct doesn't even have an ID.

[fuelen]

If having a primary key is a strong requirement, then dataloader must early raise an exception. Or generate an internal ID by itself

[szymonkozak]

@fuelen I have opened a PR that adds such raise: https://github.com/absinthe-graphql/dataloader/pull/177

@benwilson512 What do you think about that?