uesteibar / neuron

A GraphQL client for Elixir
https://hex.pm/packages/neuron
Other
331 stars 35 forks source link

Duplicated fragment registration #56

Closed parkerduckworth closed 4 years ago

parkerduckworth commented 4 years ago

Hey, @uesteibar !

First and foremost, thanks for the awesome lib. I've run into an issue with using multiple fragments in the same body.

Example:

query {
  aThing(id: "123546789") {
    aSubThing {
      ...fragment1
      ...fragment2
      aProperty {
        aSubProperty
      }
    }
    anotherSubThing {
      ...fragment1
      ...fragment2
      aProperty {
        aSubProperty
      }
    }
  }
}

fragment fragment1 on aType {
  aProperty
}

fragment fragment2 on anotherType {
  bProperty
}

This issue is that after registering these fragments and running the query, I receive this error message:

"message": [
        "There can be only one fragment named \"fragment1\".",
        "There can be only one fragment named \"fragment2\"."
 ]

It's clear from this error message that the fragments are likely being duplicated somehow in the Neuron.Store.

I dug into Neuron's internals to see what I could find, and threw in some print statements:

lib/neuron/fragment.ex

@doc false
  def insert_into_query(query_string) do
    stored_fragments = Store.get(:global, :fragments, []) ++ Store.get(:process, :fragments, [])
    |> IO.inspect(label: "stored_fragments")

    fragments_to_add =
      query_string
      |> find_in_query
      |> IO.inspect(label: "fragments_to_add after find_in_query")
      |> Enum.map(&List.keyfind(stored_fragments, &1, 0, &1))
      |> IO.inspect(label: "fragments_to_add after List.keyfind stored_fragments")

    fragments_to_add =
      fragments_to_add
      |> Enum.reject(&is_atom/1)
      |> IO.inspect(label: "fragments_to_add after Enum.reject")
      |> Enum.reduce(fragments_to_add, &load_missing_fragments(&1, stored_fragments, &2))
      |> IO.inspect(label: "fragments_to_add after Enum.reduce load_missing_fragments")

    missing_fragments = fragments_to_add |> Enum.filter(&is_atom/1) |> Enum.map(&Atom.to_string/1)

    if Enum.any?(missing_fragments),
      do: raise(Neuron.MissingFragmentsError, missing_fragments: missing_fragments)

    fragments_to_add
    |> Enum.map(&elem(&1, 1))
    |> Enum.map(&elem(&1, 0))
    |> Enum.reduce(query_string, &"#{&1} \n #{&2}")
  end

After compiling and re-running, this was the output:

stored_fragments: [
  fragment1: {"fragment fragment1 on aType { aProperty }", []},
  fragment2: {"fragment fragment2 on anotherType { bProperty }", []}
]

fragments_to_add after find_in_query: [: fragment2, : fragment1, : fragment1, : fragment2]

fragments_to_add after List.keyfind stored_fragments: [
  fragment2: {"fragment fragment2 on anotherType { bProperty }", []},
  fragment1: {"fragment fragment1 on aType { aProperty }", []},
  fragment1: {"fragment fragment1 on aType { aProperty }", []},
  fragment2: {"fragment fragment2 on anotherType { bProperty }", []}
]

fragments_to_add after Enum.reject: [
  fragment2: {"fragment fragment2 on anotherType { bProperty }", []},
  fragment1: {"fragment fragment1 on aType { aProperty }", []},
  fragment1: {"fragment fragment1 on aType { aProperty }", []},
  fragment2: {"fragment fragment2 on anotherType { bProperty }", []}
]

fragments_to_add after Enum.reduce load_missing_fragments: [
  fragment2: {"fragment fragment2 on anotherType { bProperty }", []},
  fragment1: {"fragment fragment1 on aType { aProperty }", []},
  fragment1: {"fragment fragment1 on aType { aProperty }", []},
  fragment2: {"fragment fragment2 on anotherType { bProperty }", []}
]

It appears that the duplication is taking place after the query string is passed to find_in_query/2.

This makes sense, considering its function definition contains a regex to find all ... fragments:

lib/neuron/fragment.ex

defp find_in_query(query_string) do
    Regex.scan(~r/(?<=\.\.\.)\w+(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)/, query_string)
    |> List.flatten()
    |> Enum.map(&String.to_atom/1)
end

The fix is a simple one: call Enum.uniq/1 after the call to Enum.map/1 to remove any duplicates! After I made this change, the problem was solved, and the query was executed correctly.

The amended function:

defp find_in_query(query_string) do
  Regex.scan(~r/(?<=\.\.\.)\w+(?=(?:[^\"]*\"[^\"]*\")*[^\"]*$)/, query_string)
  |> List.flatten()
  |> Enum.map(&String.to_atom/1)
  |> Enum.uniq
end

The output after making this change:

stored_fragments: [
  fragment1: {"fragment fragment1 on aType { aProperty }", []},
  fragment2: {"fragment fragment2 on anotherType { bProperty }", []}
]

fragments_to_add after find_in_query: [: fragment2, : fragment1]

fragments_to_add after List.keyfind stored_fragments: [
  fragment1: {"fragment fragment1 on aType { aProperty }", []},
  fragment2: {"fragment fragment2 on anotherType { bProperty }", []}
]

fragments_to_add after Enum.reject: [
  fragment1: {"fragment fragment1 on aType { aProperty }", []},
  fragment2: {"fragment fragment2 on anotherType { bProperty }", []}
]

fragments_to_add after Enum.reduce load_missing_fragments: [
  fragment1: {"fragment fragment1 on aType { aProperty }", []},
  fragment2: {"fragment fragment2 on anotherType { bProperty }", []}
]