Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

resolve_type second parameter is just some other resolved object? #1149

Open
marcinkoziej opened this issue Jan 28, 2022 · 2 comments
Open

Comments

@marcinkoziej
Copy link
Contributor

marcinkoziej commented Jan 28, 2022

Hi!
Reporting a potential bug:

Environment

Erlang/OTP 24 [erts-12.0.3] [source] [64-bit] [smp:4:4] [ds:4:4:10] [async-threads:1] [jit]
Elixir 1.12.0 (compiled with Erlang/OTP 23)
mix deps |grep absinthe

  • absinthe 1.7.0 (Hex package) (mix)
    locked at 1.7.0 (absinthe) 566a5b55
  • absinthe_phoenix 2.0.2 (Hex package) (mix)
    locked at 2.0.2 (absinthe_phoenix) d3691892
  • absinthe_plug 1.5.8 (Hex package) (mix)
    locked at 1.5.8 (absinthe_plug) bbb04176
    Client framework: GrapqiQL playground

Expected behavior

When resolving a list of interfaces, I expect that resolve_type(object, env) receives a second argument that contains information about parent/location of object in the graph - consistently for all items.

Actual behavior

I have a problem when resolving type of a list of objects.

I have a following graph:

(  campaign  ) -has-many-> (  targets ) -has-many-> (target emails)
  / can be \                 / can be \
public     private        public     private 
campaign   campaign       target     target 

If you have a permission to see campaign (interface) as private campaign (object), you can also see the target (iface) as private target (object).
Target emails are only visible on private target.

In target interface definition, i want to check what the parent (campaign) was resolved to either :public_campaign or :private_campaign (the :parent_type field)

  interface :target do
    field :id, non_null(:string)
    field :name, non_null(:string)
   # .... 
    resolve_type(fn
      _, %{parent_type: %{identifier: :private_campaign}} ->  :private_target
      _, %{parent_type: %{identifier: :public_campaign}} -> :public_target
      _, %{parent_type: _other} -> :public_target 
    end)
  end

When doing a query like this:

campaign(id: 123) {
  ... on PrivateCampaign {
    targets {
      ... on PrivateTarget {
        emails {
          email
        } 
      }
    }
  }
}

When resolving all items in the target list for particular campaign, only the FIRST target is resolved as :private_target - and the resolve_type is given the object where I can match on: %{parent_type: %{identifier: :private_campaign}}.
The SECOND target receives something else as second argument - the %{parent_type: %{identifier: :target_email}}, which seems to be a child of FIRST target, not parent of SECOND one!
This is quite confusing - is this a bug, or is it expected? If expected, how use a resolve_type supposed to use the second argument? What would be the use case?

Relevant Schema/Middleware Code

campaign type:

  interface :campaign do
    @desc "Campaign id"
    field :id, non_null(:integer)
    @desc "External ID (if set)"
    field :external_id, :integer

    field :targets, list_of(:target) do
      resolve(&Resolvers.Campaign.targets/3)
    end

    resolve_type(fn
      %{org_id: org_id}, %{context: %{auth: %Auth{staffer: %{org_id: org_id}}}} ->
        :private_campaign
      _, _ ->
        :public_campaign
    end)
  end

  object :public_campaign do
    interface(:campaign)
    import_fields(:campaign)
  end

  object :private_campaign do
    interface(:campaign)
    import_fields(:campaign)
    # few extra private fields
  end

targets:

  object :target_email do
    field :email, non_null(:string)
    field :email_status, non_null(:email_status)
  end

  interface :target do
    field :id, non_null(:string)
    field :name, non_null(:string)
    # more fields... 

    resolve_type(fn
      _, %{parent_type: p%{identifier: :private_campaign}} ->
        :private_target
      _, %{parent_type: %{identifier: :public_campaign}} -> :public_target
      _, %{parent_type: _pt} -> :public_target
    end)
  end

  object :public_target do
    interface(:target)
    import_fields(:target)
  end

  object :private_target do
    interface(:target)
    import_fields(:target)

    field :emails, non_null(list_of(:target_email))
  end

@marcinkoziej
Copy link
Contributor Author

marcinkoziej commented Jan 28, 2022

Additional debugging shows that for every item, resolve_type is called with a different path in second argument (each path element has {name, partent_type.identifier}):

item 1: Target { "Marcin Koziej", "e1f8d74e-be4e-4329-ad23-2e2972575d64"}

path: [
  {"targets", :private_campaign}
  0,
  {"campaigns", :private_org}
  {"org", :query}
  {:op, "A"}
]

item 2: Target { "Paweł Koziej", "cef7d183-0f3d-4598-a670-2a13414c5e77"}
path: [
  {"email", :target_email},
  0,
  {"emails", :private_target},
  0,
  {"targets", :private_campaign},
  0,
  {"campaigns", :private_org},
  {"org", :query},
  {:op, "A"}
]

item 3: Target {"Lolek Koziej", "e6078b59-e6d9-42da-9e55-36a1508bd407"}
path: [
  {"externalId", :public_target},
  1,
  {"targets", :private_campaign},
  0,
  {"campaigns", :private_org},
  {"org", :query},
  {:op, "A"}
]

So the calls to resolve_type pass Execution context with very weird paths:

For item 0: Campaigns[0] -> Target[0] (resolution 1)
For item 1: Campaigns[0] -> Target[0] -> Email[0] (resolution 2)
For item 2: Campaigns[0] -> Target[1] -> externalId (resolution 3)

(not that for Target with index 2, it is not even in the path - Target with index 1 is)

@benwilson512
Copy link
Contributor

Hey @marcinkoziej, that is definitely a bit strange. Traditionally the reason we pass in the env has more to do with making the schema available and the context available, I don't know that the other values in that struct have been validated in this situation. Possibly there is some data leaking over from the execution flow. Let me look into it.

marcinkoziej added a commit to marcinkoziej/absinthe that referenced this issue Nov 16, 2022
Addresses issue absinthe-graphql#1149.
module `Absinthe.Phase.Document.Execution.Resolution` walks the result
tree and tracks `path` using a function parameter. At the same time, a
`Resolution` struct is passed, which `path` field is not consistently
tracked.

The problem manifests itself in bogus Resolution path passed onto
`resolve_type` function as described in function absinthe-graphql#1149.

The reason for not updating the resolution.path in the walk process can
be explained by comment of @benwilson512: `Traditionally the reason we
pass in the env has more to do with making the schema available and the
context available, I don't know that the other values in that struct
have been validated in this situation.` Perhaps path parameter was used
in the beginning and later resolution paramter was added, but not used
to track current tree walk in it's path field.

Nevertheless, the resolution is passed on to user code - into
`resolve_type(value, resolution)` callback, which might use path to
understand the context of value (eg. what is the type of parent?).

This PR fixes the bookkeeping of `resolution.path` field. I have not
inspected the use of other field, I am not sure which ones would change
during the value walk (`parent_type` maybe?).

One could consider to just use `resolution.path` instead of `path`
parameter - so there is not double bookkeeping of this value. This is up
to maintainers to decide, I did not make such a change.
marcinkoziej added a commit to marcinkoziej/absinthe that referenced this issue Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants