Skip to content

Commit

Permalink
Refactor: API contract for policy cache
Browse files Browse the repository at this point in the history
See #801 (comment)

We'd like to support caching a few more lookups, but that's backwards-incompatible. By swapping out the cache, we could have a legacy cache (that only caches "the old way"), and a new cache that will cache _more_ things. This allows backwards-compatibility with the same API interface.
  • Loading branch information
Burgestrand committed May 8, 2024
1 parent 266cae0 commit aedc862
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 5 deletions.
12 changes: 10 additions & 2 deletions lib/pundit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
require "active_support/dependencies/autoload"
require "pundit/authorization"
require "pundit/context"
require "pundit/cache_store/null_store"
require "pundit/cache_store/hash_store"

# @api private
# To avoid name clashes with common Error naming when mixing in Pundit,
Expand Down Expand Up @@ -66,8 +68,14 @@ def self.included(base)

class << self
# @see [Pundit::Context#authorize]
def authorize(user, record, query, policy_class: nil, cache: {})
Context.new(user: user, policy_cache: cache).authorize(record, query: query, policy_class: policy_class)
def authorize(user, record, query, policy_class: nil, cache: nil)
context = if cache
Context.new(user: user, policy_cache: cache)
else
Context.new(user: user)
end

context.authorize(record, query: query, policy_class: policy_class)
end

# @see [Pundit::Context#policy_scope]
Expand Down
2 changes: 1 addition & 1 deletion lib/pundit/authorization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module Authorization
def pundit
@pundit ||= Pundit::Context.new(
user: pundit_user,
policy_cache: policies
policy_cache: Pundit::CacheStore::HashStore.new(policies)
)
end

Expand Down
16 changes: 16 additions & 0 deletions lib/pundit/cache_store/hash_store.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

module Pundit
module CacheStore
# @api private
class HashStore
def initialize(hash = {})
@store = hash
end

def fetch(key)
@store[key] ||= yield
end
end
end
end
18 changes: 18 additions & 0 deletions lib/pundit/cache_store/null_store.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# frozen_string_literal: true

module Pundit
module CacheStore
# @api private
class NullStore
@instance = new

class << self
attr_reader :instance
end

def fetch(*, **)
yield
end
end
end
end
6 changes: 4 additions & 2 deletions lib/pundit/context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module Pundit
class Context
def initialize(user:, policy_cache: {})
def initialize(user:, policy_cache: CacheStore::NullStore.instance)
@user = user
@policy_cache = policy_cache
end
Expand All @@ -27,7 +27,9 @@ def authorize(possibly_namespaced_record, query:, policy_class:)
policy = if policy_class
policy_class.new(user, record)
else
policy_cache[possibly_namespaced_record] ||= policy!(possibly_namespaced_record)
policy_cache.fetch(possibly_namespaced_record) do
policy!(possibly_namespaced_record)
end
end

raise NotAuthorizedError, query: query, record: record, policy: policy unless policy.public_send(query)
Expand Down

0 comments on commit aedc862

Please sign in to comment.