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

Does not reload code changes when using namespaces #16

Closed
gururuby opened this issue Jun 19, 2018 · 7 comments
Closed

Does not reload code changes when using namespaces #16

gururuby opened this issue Jun 19, 2018 · 7 comments
Labels
bug Something isn't working

Comments

@gururuby
Copy link

Hi, I have a controller(Subscriptions) inside the Dashboard module with action manage, eg (app/controllers/dashboard/subscriptions_controller.rb)

module Dashboard
  class SubscriptionsController < BaseController
     def manage
        authorize! Subscription.new
     end
  end
end

And my policies structure app/policies/dashboard/subscription_policy.rb

module Dashboard
  class SubscriptionPolicy < ApplicationPolicy
    def manage?
       true
    end
  end
end

When I change the policy(manage?) to false, then the changes do not apply, but everything works fine when I do not use namespaces, and I take subscription_policy to the root of the app / policies directory

Ruby 2.5.1
Rails 5.2.0
Action Policy 0.2.0

@palkan palkan added the investigate Something is probably wrong label Jun 19, 2018
@jacobwalkr
Copy link

Could this be related to #14?

@gururuby
Copy link
Author

@jacobwalkr may be. There are additions if I call authorize! and specify policy directly via the with option, changes in the policy are applied without restarting the server

@ycherniavskyi
Copy link

ycherniavskyi commented Jun 29, 2018

Got the same reload issue.
After some investigation found the reason - NamespaceCache class, which hold Hash with first-time loaded policy class and clear it only on init.

Seams that in development this cache must be disabled or must be clearing on each request.

@palkan palkan added bug Something isn't working and removed investigate Something is probably wrong labels Jun 29, 2018
@palkan
Copy link
Owner

palkan commented Jun 29, 2018

@ycherniavskyi

Thanks for the investigation!

Seams that in development this cache must be disabled or must be clearing on each request.

We should add a configuration to disable this cache, and disable it by default in development env.

We can also embrace app.reload.before_class_unload callback for Rails 5 and clear the cache.

If anyone is willing to propose a PR – you're welcome! If not – I'll take care of it a little bit later.

@ycherniavskyi
Copy link

@palkan could you please explain a bit the exact purpose of NamespaceCache class. Why namespaced class has the cache but class without namespaced hasn't.

Also as I see from the source, found namespaced policy also cache in the object and in the thread. Will not it be easy and simple just remove NamespaceCache mechanism 🙂?

@palkan
Copy link
Owner

palkan commented Jun 29, 2018

Why namespaced class has the cache but class without namespaced hasn't.

Namespaced lookup is more expensive. I've added a benchmark which shows that it could be 4x slower without cache:

Comparison:
               cache:   178745.5 i/s
             cache B:   169435.4 i/s - same-ish: difference falls within error
          no cache A:    95312.7 i/s - same-ish: difference falls within error
          no cache B:    43121.0 i/s - 4.15x  slower

Also as I see from the source, found namespaced policy also cache in the object and in the thread. Will not it be easy and simple just remove NamespaceCache mechanism 🙂?

NamespaceCache caches a policy class for a given namespace and a policy name, we use it every time we want to initialize a policy within the matching context.
Per-thread and instance caches store policy instances, they only re-used within a single request. We still have to resolve a policy class.

@palkan palkan added this to In progress in ActionPolicy Development Jul 3, 2018
@palkan
Copy link
Owner

palkan commented Jul 3, 2018

Fixed by 1d4a2e7 and released in 0.2.3.

@palkan palkan closed this as completed Jul 3, 2018
ActionPolicy Development automation moved this from In progress to Done Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

No branches or pull requests

4 participants