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

[Unable to use] graphql-ruby-fragment_cache + action_policy-graphql #68

Open
danielnc opened this issue Jun 11, 2021 · 5 comments
Open
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@danielnc
Copy link

danielnc commented Jun 11, 2021

I am trying to use both fragment caching + action policy and due to lack of documentation I am unable to make it work if I have a authorized_scope: true if my field

if I have a field like this:
field :collections, [CollectionType], null: false, authorized_scope: true, cache_fragment: { context_key: :current_user } do

I get an error:

Couldn't find policy class for.... "Couldn't find implicit authorization target for Types::QueryType. Please, provide policy class explicitly using `with` option or define the `implicit_authorization_target` method."] (Array)```

If I add a explicit policy:

field :collections, [CollectionType], null: false, authorized_scope: { with: CollectionTypePolicy }, cache_fragment: { context_key: :current_user } do

I get a different error:

Couldn't infer scope type for GraphQL::Execution::Interpreter::RawValue instance

It works flawlessly for fields without authorization scope, so not sure what I need to add/change on my end or what is missing

@DmitryTsepelev
Copy link
Owner

Hi @danielnc!

When there is a cache hit, an instance of GraphQL::Execution::Interpreter::RawValue is returned to let the execution engine know that we want an early return (heads up: it's not an array of ActiveRecord objects! it's a resulting JSON). ActionPolicy has no idea what is it, that's why you get Couldn't infer scope type for GraphQL::Execution::Interpreter::RawValue instance.

authorized_scope: true fails because it cannot filter JSON.

I feel like the only way to make it work is to move scoping into the resolver method:

field :collections, [CollectionType], null: false, cache_fragment: { context_key: :current_user }

def collections
  authorized_scope(Collection, type: :relation)
end

I feel like we could fix it by performing cache checks before policy ones (because we cache only scoped data, so it should be safe). @palkan do you think it's possible?

@palkan
Copy link
Collaborator

palkan commented Jun 29, 2021

That's interesting. So, fragment cache doesn't halt the field resolution, right?

Maybe, a quick fix would be to add a passthrough policy for GraphQL::Execution::Interpreter::RawValue?

For example:

class PassthroughPolicy < ApplicationPolicy
  relation_scope(&:itself)
end

GraphQL::Execution::Interpreter::RawValue.define_singleton_method(:policy_class) { PassthroughPolicy }

@DmitryTsepelev
Copy link
Owner

So, fragment cache doesn't halt the field resolution, right?

It halts the resolution of all nested fields and returns the value as-is. Looks like policy checks are run against the returned value after that. @danielnc could you please try the snipped above? I guess we could make it a part of the gem later

@DmitryTsepelev DmitryTsepelev added enhancement New feature or request good first issue Good for newcomers labels Jul 2, 2021
@danielnc
Copy link
Author

danielnc commented Jul 2, 2021

@DmitryTsepelev / @palkan this code works for the first time when there is no cached value, for a second execution when there is a cached value, it's still throwing the same error:
Couldn't infer scope type for GraphQL::Execution::Interpreter::RawValue instance

and just a FYI, doing
authorized_scope(Collection, type: :active_record_relation)

is not the same as adding authorized_scope: true, it doesn't apply authorization scope to the field

@mretzak
Copy link
Contributor

mretzak commented Apr 14, 2022

I landed on this issue with the same problem. I was able to get around this with action policy by combining a custom scope matcher and policy. Here's the gist of it:

In an initializer:

module ActionPolicy
  module ScopeMatchers
    module GraphqlFragmentCache
      def cache_fragment_passthrough(*args, &block)
        scope_for(:cache_fragment, *args, &block)
      end
    end
  end
end

ActionPolicy::Base.extend ActionPolicy::ScopeMatchers::GraphqlFragmentCache
ActionPolicy::LookupChain.chain << lambda { |record, _|
  GraphqlFragmentCachePolicy if record.is_a?(GraphQL::Execution::Interpreter::RawValue)
}

graphql_fragment_cache_policy.rb:

class GraphqlFragmentCachePolicy < ApplicationPolicy
  scope_matcher :cache_fragment, GraphQL::Execution::Interpreter::RawValue

  cache_fragment_passthrough { |cached_collection| cached_collection }

  def show?
    true
  end
end

It's worth noting that our cache keys are granular enough to account for our various user / permission contexts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants