DmitryTsepelev / graphql-ruby-fragment_cache

graphql-ruby plugin for caching parts of the response
MIT License
204 stars 34 forks source link

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

Open danielnc opened 3 years ago

danielnc commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

danielnc commented 3 years ago

@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 commented 2 years ago

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.