Proposal: Deferred Installation of Routes

[Bauerpauer]

If we want to provide built-in controllers (session handling, authentication, etc), we either need to not require them by default, or require them but have their routes NOT get installed until needed. If we go w/ the "not require them" route, we still have an issue where the controller's can't be inherited from. This is a proposal for a way around that:

class Harbor::Controller

  # Save the routes for later
  def self.routes(&block)
    @routes_block = block
  end

  # Execute the routes block, installing the routes into Harbor::Router
  def self.install_routes
    class_eval(&@routes_block) if @routes_block
  end

  # Always install the deferred routes when you inherit from a "Base" controller
  def self.inherited(klass)
    klass.install_routes
  end

  def self.activate!
    install_routes
  end

end

class Harbor::Session

  def upgrade(request)
    # Do some abstract authentication things here instead of this..
    (user = User.first(email_address: request['email_address'])) && user.password == request['password']
  end

end

class Harbor::Controllers::Sessions < Harbor::Controller

  def self.activate!
    super
    # Things?
  end

  routes do

    redirect "/login", "/sessions/new"

    new { } # Magically renders sessions/new because this is the sessions controller, the "new" action, and nothing was rendered inside the block

    create do
      if request.session.upgrade(request)
        redirect_to_referrer
      else
        render "sessions/new"
      end
    end

    delete { request.session.abandon!; redirect "/sessions/new" }

  end

end

class TestApp < Harbor::Application

  def initialize
    Harbor::Session.configure do |options|
      # Configure it...
    end

    # User Harbor's built-in Session Abstraction
    Harbor::Controllers::Sessions.activate!
  end

end

[sam]

We should be able to override a route by just defining it twice. Wouldn't that resolve this issue?

[Bauerpauer]

Hmm, I suppose works just fine. It'd suck for some third-party lib to come in and rip up your routes on accident though, just by requiring a controller. I'm not sure I dig that. Maybe it'll never be an issue in practice, though.

[sam]

Same as normal though, require your actual application last. So you always win.

[sam]

This seems kinda muddy so I'm going to close it. Controllers can't be inherited from according to pull request #50, but if we want to enable that (and I'm not arguing against it, it seems sane enough to me), we should make that a separate, specific ticket.

[fgrehm]

@sam @Bauerpauer if we wanna support controller inheritance, we might introduce something like Harbor::AbstractController to have routes registration (and possibly action filters) deferred until the controller is inherited, but we should not allow absolute paths registration or things will get messed up :P

[sam]

@fgrehm Why's that? Don't routes just overwrite if you define them twice? Last-in-wins?

[fgrehm]

@sam Yup, but depending on the require order the "last-won" might change ;)

[fgrehm]

@sam and about the lazy registration, we need to "calculate" relative routes based on the controller being inherited or we would have the same routes for all controllers that inherits from the super class

please let me know if I'm not clear ;-)

[sam]

@fgrehm Right, well for auto-loading I guess we'd have to have dependencies. Given foo.rb with:

class Foo < Harbor::Controller
  get "bar" do
    response.puts "Hello World!"
  end
end

And in baz.rb you've got:

class Baz < Foo
  get "bar" do
    response.puts "Overwritten Baby Yeah!"
  end
end

Then baz.rb would need a dependency on foo.rb so that if foo.rb were reloaded, baz.rb would be undefined and reloaded as well?

As far as "calculating" the routes: I'd actually expect we not do that. IOW, if we inherit, and define the same path, then we're overwriting the existing route, not adding a new one to our own namespace. Because why bother with the inheritance then? (Other to grab non-route methods I guess.)

[fgrehm]

@sam I told you that inheriting controllers can confuse people :-P there is actually a problem with your example as get "bar" on Baz won't override Foo's get "bar" because one will be registered as "foo/bar" and the other as "baz/bar" ;-) that's one of the things I want to avoid with the "AbstractController" (or whatever name we give it) :-D

and about "inheriting" / "calculating" routes, it would provide the support for some kind of "BaseResourceController" / "CrudController" along the lines of inherited_resources (not as part of harbor core itself of course ;-)

[sam]

Yeah... we could just push off inherited Controllers for now unless you really want to see them in there. I think we could get along without them easily enough. If you want to overwrite a route, then redefine it in the same Controller or another. If we had a PortAuthority for Harbor-v1 for example, it doesn't seem all that critical that you'd be able to inherit from Admin::Users. Just re-open it.

The only negative is perhaps one of aesthetics, but I don't think re-opening and overwriting is necessarily an "ugly" thing to do.

[fgrehm]

I don't have any problem to drop inherited controllers, and to work around "aesthetics issues" we can always include modules on the fly :-P