3

Consider this code

module Auth

  def sign_in(user)
    #some stuff
    session[:user_id] = user.id
  end

end

Now, I want to include this in my application controller.

ApplicationController < ActionController::Base
  include Auth
end

This makes the sign_in method available in all my controllers. Now, to make it clear that this is not a controller action, I would like to maintain the name, so my controllers read

def sign_user_in
  # Some stuff
  Auth.sign_in(@user)
end

This obviously does not work, because Rails will look for the class method in the Auth module. So the question is... Is it possible to include a module into the controller, preserve it's name or namespace, but still get access to the same scope as the controller? (in this case, the session variable).

So far the least bad way I have come up with is stop including the module and in the ApplicationController and instead pass the application controller instance along when calling an auth method like this:

def current_user(controller)
  User.find(controller.session[:user_id])
end

Calling this method from a controller using using self as an argument works.

Niels B.
  • 5,912
  • 3
  • 24
  • 44
  • 1
    IMO that would be the worst of both worlds: all the dangers of exposure with none of the single-namespace benefits. – Dave Newton May 11 '13 at 22:25
  • Single-namespace benefits? What are you talking about? I have a collection of methods that are operates on controller instances, but I don't want my application controller to grow fat. Also, I like the method context that a constant provides. I think it improves readability. I know that I can use prefixes instead and do `auth_method_name`, but I really think `Auth.method_name` looks better. JavaScript offers this thanks to it's execution context, but how does Ruby do it? If I thought there was anything beneficial about "single namespacing", I'd be programming PHP. – Niels B. May 11 '13 at 22:30
  • TL;DR. You explicitly said you wanted to access `session` from inside the mixin--but only in the mixin, not in the controller. The dangers of exposure, w/o the benefits. If you're going to separate concerns, *separate them*, and use a service object. Simply moving functionality to a mixin doesn't make the controller any less fat: moving it to an isolated class *does*. – Dave Newton May 11 '13 at 22:39
  • You are being rude. If you don't want to read my posts, close the browser tab. I want my controller to be fat at runtime, but manageable in my editor. One application controller with 20 methods that don't answer the router is not manageable to me. It is confusing. A controller method calling some method called `sign_in` in this single namespace, is not helpful either. Is it a (view) helper? Perhaps another controller action? Maybe it's inherited from ActionController::Base? Is there really nothing better than a method prefix for this? – Niels B. May 11 '13 at 22:51
  • You are using the word "rude" rather broadly. If you want the controller to be fat (which is a bad idea), then make it fat--don't *pretend* it's not fat. Why would "sign_in" need to be strewn about your entire application anyway? It's a singular, localized concern. – Dave Newton May 11 '13 at 22:55
  • Stack Overflow follows a question & answer format. Answering a question with "Too long, did not read" is rude. That being said, `sign_in` sets a value in the session. The session is accessible from inside the controller and nowhere else. Correct me if I am wrong. Auth also includes a `current_user` method which is fairly universal. Again, correct me if I am wrong. – Niels B. May 11 '13 at 23:02
  • I responded your *comment* with a TL;DR. I didn't answer your *question*. I'd argue asking "what I was talking about" and snide PHP reference was ruder than my original comment. Access to session is irrelevant to where sign_in is located; you can pass session. current_user is a separate issue, you *do* need that in multiple locations. Mixing Auth in your base controller already polluted your controller. Namespacing it is superfluous, you've exposed controller internals to Auth. Pretending you haven't on the controller side is misleading and uncommunicative. – Dave Newton May 11 '13 at 23:25
  • I **want** to expose controller internals to Auth. If Auth feels like doing a redirect_to, so be it. I just happen to feel that `Auth.current_user` reads better than `current_user` or `auth_current_user`. Because it becomes clear where `current_user` is defined and in what context. I don't want to pass stuff around. Auth is a controller extension, make no mistake about it. I just want clear readable code. – Niels B. May 11 '13 at 23:37
  • 2
    I *know* you want to, which is precisely why I think it's the worst of both worlds: a fat controller that pretends Auth is something separate when it's not. Code that looks like it's calling class methods when it's not is code that lies. – Dave Newton May 11 '13 at 23:45
  • I get your point and that's a relevant point. You would have to be aware of this to know that Auth mutates instance variables on the caller (the controller). But if you are aware of this, it is not lying at all - it's just a programming style. Whenever I call `methods` on an object and realize there are more methods than countries in the world, I become sad. I would much rather have sub objects or modules that aggregate related methods, but still operate in the same scope. Apparently the best Ruby has to offer is prefixing. – Niels B. May 12 '13 at 00:03
  • IMO modules aren't a place for code used in one place. A sub-object (auth.current_user) replaces an underscore with a period, and if in the same namespace, gains nothing real. To me this is the worst of both worlds: unfettered access to internals with a false veneer of separation. The best Ruby offers is *actual* separation, w/o misleading code and free access to internals. I don't see why sign_in isn't a single method in a single controller, and current_user isn't in the base controller , exposed as a helper. The actual auth/auth mechanism *should* be isolated in an interface implementation. – Dave Newton May 12 '13 at 00:21
  • consider using devise for authentication - https://github.com/plataformatec/devise – house9 May 12 '13 at 05:39
  • This question has nothing specifically to do with authentification, but something to do with how you can extend your application controller with mixins, each containing related functionality, while making their use explicit in the individual controllers to provide context. – Niels B. May 12 '13 at 12:45
  • Just got a notification that someone upvoted my question from 9 years ago. In the meantime I have come to realize that @DaveNewton is 100% right. I guess I felt it was nice to have concerns grouped under constants, but it's working against the language and it creates a new set of problems. :-) – Niels B. Feb 25 '22 at 13:46
  • @DaveNewton Apologies for being such a confident rookie back then. – Niels B. Feb 25 '22 at 13:54

2 Answers2

2

Try this out?

Uses an actual class for all the functionality and the controller has an instance of that class available; basically the exact same code you have above - see current_user but you only need to pass the controller instance in once, not on every method call

module Auth
  # this method is 'mixed' into controller (self)
  def initialize_authorizer
    @authorizer = ::Auth::Authorizer(self)
  end

  # this class doesn't need to be in this namespace (module), put it where ever makes sense
  class Authorizer
    def initialize(controller)
      @controller = controller
    end

    attr_reader :controller

    def sign_in(user)
      #some stuff
      controller.session[:user_id] = user.id
    end

    def current_user
      User.find(controller.session[:user_id])
    end        
  end
end

ApplicationController < ActionController::Base
  include Auth

  before_filter :initialize_authorizer
end

def sign_user_in
  # Some stuff
  @authorizer.sign_in(@user)
end
house9
  • 20,359
  • 8
  • 55
  • 61
  • I really like the idea of using an instance variable. With your solution I don't need the namespace. I can create the controller extension as a class called auth and then set it up with for use in the application controller using the before_filter. – Niels B. May 12 '13 at 14:54
0

It has been 9 years since I asked this question. In the meantime I have realized that it is not a good idea to do this as it rubs against the language.

Constants have their own self and when referencing a constant, you would expect any methods to be class methods. You would not expect them to have access to the calling object unless an a reference is explicitly passed during the method call, in which case you have a bidirectional dependency, which comes with its own set of problems. That would be a code smell and should be a cause for refactoring the software design.

Niels B.
  • 5,912
  • 3
  • 24
  • 44