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

New dry-system-powered application and slices structure #1019

Open
wants to merge 63 commits into
base: unstable
from

Conversation

@timriley
Copy link
Member

timriley commented Nov 3, 2019

Hello! Here in this PR we have a “native” dry-system integration as part of the Hanami core application structure. By “native” I mean an approach that lets us take advantage of as many dry-system features as possible, while also exposing those features to application authors for use within their apps.

The key aspect of this arrangement is one of a single application and multiple slices. The application acts as both the boot coordinator as well as the “core” of the overall system. Each slice represents a discrete system of functionality. Slices gain access to the application’s facilities, and they’ll eventually be able to import other slices too. The end result is that we end up with the slices and application forming a graph of the application’s high-level areas of functionality.

Here’s the kind of application structure that will now be possible (please excused the hurried diagram):

hanami-application-graph

This is made possible thanks to features of dry-system, which in this new Hanami arrangement, we use to the fullest extent. This kind of application structure has also been proven in production over multiple applications and multiple years (within Icelab, and both @solnic and @flash-gordon
maintain similar apps too).

Now here’s how it all works. There’s a lot to get through here, but I’ll try hit on all the important elements:

Hanami::Application

This class now represents the application in the description above. It’s meant to be subclassed once (i.e. YourApp::Application < Hanami::Application), which is similar to Hanami v1 AFAIK.

What’s different here is that the application _class _represents a stateful, bootable app. The instance of this class is just the rack application (we’ll get to that later). So hanami application exposes a few lifecycle methods:

  • .init, which prepares the container and detects the slices
  • .init_bootable and .start_bootable, which init or started individual bootable components (registered via .register_bootable)
  • .boot, which finalizes the application container, boots all the slices, and registers an instance of the rack app

The application container is managed internally as a separate object. It’s defined automatically, and configured to ensure it behaves as we require. When it comes to configuration, I want to make it so that all the important settings are defined on Hanami::Configuration so regular users don’t need to worry about container internals. I’ve done it for some things alredy, but some more will need to be done before this is fully ready.

The container is available as YourApp::Application.container and is also assigned to YourApp::Container for convenient access.

The container’s auto-injection mixin is also assigned to YourApp::Deps for easy inclusion into classes.

The result is that you get all the container facilities without having to worry about setting it up yourself (or even having to see a bunch of boilerplate files inside your app).

The application also offers a small subset of the container’s methods for direct access (the ones that will most commonly be used when interacting with the app): .keys, .resolve, and .[]

Hanami::Slice

Unlike Application, slices are regular classes and instantiated normally. They’re built automatically, detected from the directories inside slices/.

Each slice behaves in a similar way to the application. It:

  • defines and configures a container
  • imports the application container so all its keys are available under the “application.” namespace (“application.logger”, etc.)
  • makes it accessible as .container
  • assigns it to SliceName::Container for convenient access
  • and assigns the auto-injection mixin to SliceName::Deps

Slices have the same lifecycle methods as the application (.init and .boot), support having their own bootable components, and also provide the same convenience methods that delegate to the container (.keys, .resolve, and .[])

Resulting code structure

With all of this in place, we can now write our application code as we would with dry-system normally, e.g.

require "slice_name/some_slice_mapper"



module SliceName
  class DoSomethinginclude Deps["article_repo"]

    def call(params)
      params = SomeSliceMapper.(params)
      article_repo.do_somethiong_with(params)
    end
  end
end

Some aspects of this I want to point out:

  1. The auto-injection mixin is now available without any extra work, thanks to the hanami application and slice init process I described above
  2. We’re relying on dry-system auto-injection to populate the containers, which means that if we’re referring to other constants within our app, we need to “require what we require” to ensure they’re available when the file is loaded.
  3. Because all these files need to be requirable (both for manual requires and for dry-system's auto-registration, at least in its current form), we need their paths on the file system to match their module/class hierarchy, so e.g. SliceName::DoSomething needs to be at slices/slice_name/lib/slice_name/do_something.rb

Router enhancements

There’s an enhancement to the router to go along with the application/slices structure. It provides:

  • Endpoint resolution based on container registrations (i.e. root to: "home.show" will load the object registered at "actions.home.show")
  • An enhancement to #mount (when called with a symbol, e.g. mount :admin, at: "/admin") such that any endpoints resolved within that block are done so using that slice’s own container
  • A new “context” option for the router (I’ll have a PR on hanami/router for this) so that an optional argument can be passed to the router/mount blocks. For Hanami apps, either the application object or the mounted slice object will be passed as the block arguments.
  • Support for declaring middleware inline. This makes it possible to use a middleware only within a specific subset of the application routes.

The end result is that this kind of thing is now possible:

Hanami.application.routes do |application|
  use Rack::Lint

  use Rack::Static,
    urls: ["/assets"],
    root: "public"

  # Cleanly access application resources inside routes!
  use Rack::Session::Cookie,
    key: "decaf_sucks.session",
    secret: application[:settings].session_secret,
    expire_after: 10 * 365 * 24 * 60 * 60 # 10 years

  mount :main, at: "/" do
    root to: "home"
  end

  mount :admin, at: "/admin" do |slice|
    # Only authenticate /admin!
    use Rack::Auth::Basic, "Decaf Sucks" do |username, password|
      password == slice[:settings].admin_password
    end

    root to: "home"
  end
end

On naming

It wouldn't be a typical PR from me if I didn't talk about naming ;)

I renamed:

  • Hanami.application_class back to Hanami.application, and
  • Hanami.application back to Hanami.app

IIRC these were the names Luca started with at the beginning of the v2 rewrites. They make a lot more sense, I think, now that Hanami.application (i.e. YourApp::Application) is the main application (i.e. the application object with its own boot lifecycle). I'm not 100% sure about the .app name, since it's really more of a "rack-mountable web app" rather than just an "app", but OTOH at least it's short.

Also, regarding the auto-injection mixin, I decided to call it Deps instead our historically used Import for a few reasons:

  • The Import module is actually exporting (the dependencies) rather than importing them, if you think about it hard enough (@flash-gordon convinced me of this). In the end, I think it would be good to avoid the possibility of confusion here entirely) 
  • An "Import" is actually a pretty commonly used concept within applications, and it is nice not to have the naming clash (it's certainly something we've had to deal with in our apps)
  • In comparison to this, Deps is nice and short and easy to type, and not a verb, so there's no confusion there, and it still reads nice and succinctly. It's also gets people thinking about "dependencies" while they're building out their classes.

Other small bits of note

I upgraded hanami/boot.rb to keep it working like it was before, i.e. just a simple require "hamami/boot" will find and boot the Hanami application. Right now I’m doing this naively, simply by looking for config/application.rb to exist relative to the current working directory. Later we could consider making this a bit more sophisticated like how bundler searches for a relevant Gemfile.

Speaking of Bundler though, I actually extracted that application detection into an hanami/setup.rb file, with a name to mirror the bundler/setup we see so often in Ruby code. This actually works nicely given that in certain cases you may not want to fully boot your Hanami application. So now you can just init it only, e.g.

require "hanami/setup"
Hanami.init

Demos

You can see this whole structure in action here:

  • This PR to Luca’s soundeck, a very minimal native hanami v2 app
  • My decaf_sucks app, which started off as a hybrid dry-system/hanami/rom-rb app, but I’ve updated various parts as required to work with this hanami v2 application/slices structure

TODO

I’ve gone far enough with this work that I think it’s ready for an early review. Before we can merge this PR, however, I still want to do the following:

  • Get the existing tests passing (of course!)
  • Make application root configurable
  • Make more the application/slice containers configurable through Hanami::Configuration settings.
  • Along with this, also provide a place in the configuration where a user can just pass a block that can be eval’ed within the various containers, as the ultimate escape hatch.
  • Add support for slices to import other slices (I’ve already built this in other work, I just need to get it enabled here)
  • Get the PRs to hanami-rotuer and hanami-controller merged (hanami/router#192 & hanami/controller#299)
  • Enhance hanami-controller so that it provides a way to have the Hanami application configuration applied automatically to a base class used for all other actions in the app (e.g. YourApp::Action < Hanami::Action[:application])
  • Ensure dry-system will lazy load components when asked for them via .key?

I think that’s about it.

Then once this is merged I’d like to consider the following:

  • Work out a formal way of handling application-level settings (maybe it's as simple as keeping Dotenv.load, maybe it's something more sophisticated, this is TBD)
  • Reconcile the rack logging/filtering so it fits properly with the filtering in Hanami's own logger
  • Looking at a way for a more streamlined use of auto-injection, via class-level macros instead of module inclusion. I think this would just “feel” better and probably make this idea more palatable to a wider range of people.
  • Ensure it’s at least possible to build non-web Hanami v2 apps, i.e. they should run fine without routes, and we should avoid requiring web-specific files or libraries unless they’re really going to be used
  • Make it possible to declare which slices should/should not be booted in a given circumstance (e.g. it's not uncommon to have subsystems that need to exist for rake tasks or background jobs only, and should not be booted when running the web-facing part of an app)
  • Customise dry-system importer to not load redundant *.application.* registrations on slices imported by other slices

Next steps

There's a lot to digest here, but what I'd really like in terms of feedback right now is whether people are happy with the high-level structures here. If we're good with that, I think it'd be good to merge this into unstable and then have smaller iterative loops in the individual details (as @jodosha has suggested to me in a review of a previous iteration of this work).

timriley added 24 commits Sep 25, 2019
This feels like a better way to approach it than having the Hanami::Container class have to know about it internally.
- Try and load an existing Container class, if one exists
- If not, set up a plain Dry::System::Container subclass
- And then configure that directly (rather than relying on behaviour already configured in the Hanami::Application::Container subclass that we were using before)
- Rename "Import" auto-injector module to "Deps"
- Try and load an existing Deps module, if one exists
- Add `Hanami::Application.init` method to do most basic loading of application environment (defining container and deps module, detecting slices)
- Change container/slices accessors to be simple attr readers instead of lazy loaders
This uses the method names I eventually want to use on Dry::System::Container directly
This was from the first pass at converting hanami to using a container
Implement Hanami.boot and update "magic" hanami/boot file to require key files (config/application, config/routes) from conventional places before attempting to boot the app.
This just converts the "#" to a ".", which fits perfectly fine with our container identifiers
It's questionable whether we want to keep this here, though
Now we can:

require "hanami/setup"

to attempt to load the conventional config files (config/{application,routes}).

From here, we can choose to:

Hanami.init

to detect the slices and init the application (without fully booting)

And we can continue to:

require "hanami/boot"

which requires "hanami/setup" internally and then fully boots the application via `Hanami.boot`.
I've been wanting to do this enough while debugging this setup that I think it'll be helpful
timriley added 2 commits Nov 5, 2019
@timriley timriley force-pushed the enhancement/application-and-slices branch from 4b292d6 to 0ceab40 Nov 5, 2019
@timriley timriley force-pushed the enhancement/application-and-slices branch from 0ceab40 to ebd4a7b Nov 5, 2019
timriley added 3 commits Nov 5, 2019
@jodosha
jodosha approved these changes Nov 5, 2019
Copy link
Member

jodosha left a comment

@timriley Thanks for your hard work and all the patience. I left feedbacks, please consider them, where it makes sense.


We should move this forward. We can address further feedbacks in focused PRs against unstable.

lib/hanami/application.rb Outdated Show resolved Hide resolved
# TODO: do we need anything more sophisticated than this? This is how
# Dry::System::Container determines its root by default, anyway.
Dir.pwd
end

This comment has been minimized.

Copy link
@jodosha

jodosha Nov 5, 2019

Member

Given the root path is important for third party devs, I'm wondering if this is needed at all: do we just need Hanami.root? Or if we need it here because of dry-system, we can refer to Hanami.root like this?

def root
  Hanami.root
end

As @solnic pointed, we can make that Hanami.root using Dir.pwd by default and override it via configuration.

def mount(app, at:, host: nil, &block)
if app.is_a?(Symbol)
# TODO: I wonder if this is the actual behaviour we want...
raise "Slices can only be mounted from top-level of routes" unless @context.respond_to?(:slices)

This comment has been minimized.

Copy link
@jodosha

jodosha Nov 5, 2019

Member

@timriley What is this for?

def initialize(
command_name:,
out: $stdout,
inflector: Dry::Inflector.new,

This comment has been minimized.

Copy link
@jodosha

jodosha Nov 5, 2019

Member

@timriley Shouldn't this be the same instance of the inflector from the app? In this way it will inherit custom rules defined by end users.

This comment has been minimized.

Copy link
@timriley

timriley Nov 7, 2019

Author Member

@jodosha Yep, absolutely we should be passing the same inflector as configured for the app.

What you're seeing here is just the default param for initialising command classes before they've been provided the app object.

In actual usage, this won't be used, because with the way I've set up the CLI, the #with_application method is called to "prepare" each already-initialized command to be used with the application. And I've just pushed b075fdd which ensures the application's inflector is passed through at that point too.

@@ -0,0 +1 @@
# TODO: put actual components here

This comment has been minimized.

Copy link
@jodosha

jodosha Nov 5, 2019

Member

@timriley What are these components?

This comment has been minimized.

Copy link
@timriley

timriley Nov 5, 2019

Author Member

@jodosha This was me planning ahead for Hanami to vend its own dry-system bootable components, like these: https://github.com/icelab/snowpack/blob/master/lib/snowpack/components/persistence.rb

In this new dry-system-powered world, this is how we'll want Hanami to provide configurable bootable components for various things (managing the rom-rb boot definitely comes to mind, I'm not sure what else there might be for now).

What I might do for now, though, is just remove this (it was a hangover from when I copied over the Snowpack structures) and we can reintroduce it later when we need it.

lib/hanami/configuration.rb Show resolved Hide resolved
def self.new(*args)
ctx = super

# if ctx.rom

This comment has been minimized.

Copy link
@jodosha

jodosha Nov 5, 2019

Member

@timriley I would love to see it. 👍

This comment has been minimized.

Copy link
@timriley

timriley Nov 7, 2019

Author Member

@jodosha 100%. As soon as we work out how rom/hanami-model boots inside this new application structure (which, tbh, we can start on as soon as this is merged), we can have a whole range of nice rom conveniences within the console :)

That said, your comment here is a reminder I should just pull out this commented code, for now :)

end
end

class SliceDelegator < SimpleDelegator

This comment has been minimized.

Copy link
@jodosha

jodosha Nov 5, 2019

Member

@timriley I'm not sure if this is valid for this case, but for proxy objects it's often advisable to use BasicObject as superclass to reduce to the minimum the clash between methods available to Object subclasses vs BasicObject.

If you think it's the case, please consider to use Hanami::Utils::BasicObject, which has some few extra features that are worth to have. On top of all: #inspect and "pp" compat. Given we're talking about an interactive console, those features are really valuable.

This comment has been minimized.

Copy link
@timriley

timriley Nov 7, 2019

Author Member

@jodosha Thanks for the heads up about Hanami::Utils::BasicObject, that sounds like a handy convenience.

However, in this case, I think we actually want the SimpleDelegator-provided behaviour, passing through all known method calls to the underlying slice object, just with that one added enhancement via the #method_missing implementation (though I'm sure there are a few other shortcuts we can add in addition to this, later on).

require "hanami"

begin
root = Dir.pwd

This comment has been minimized.

Copy link
@jodosha

jodosha Nov 5, 2019

Member

As @solnic pointed above, is there the need here as well to make this configurable?

This comment has been minimized.

Copy link
@timriley

timriley Nov 7, 2019

Author Member

@jodosha We can't make this configurable because at this point, we're yet to even have loaded the application class! All we can do at this point is make a best guess as to where that file may reside, based on established conventions (i.e. "try load config/application.rb").

Like I mentioned in my original comment, this could be made more sophisticated by copying Bundler's require "bundler/setup" behaviour (where it tries recursing up directories until it finds the file that it wants). However, I think that should definitely be a after-this-PR thing.

I will make one change to this file now, though, where it looks up the location of the routes file from the application config, once it's loaded that.

This comment has been minimized.

Copy link
@timriley

timriley Nov 7, 2019

Author Member

FWIW, the alternative to require "hanami/setup", of course, is something like require_relative "config/application" (or whichever relative path is appropriate for the current file), but I think the require "hanami/setup" file is a much nicer looking thing, and a much easier to remember thing, if we can get the behaviour right :)

timriley added 4 commits Nov 7, 2019
This means we can respect the configuration setting pointing to the routes file path
timriley added 16 commits Nov 7, 2019
This seems like the more appropriate place than the boot methods
This makes the console helpers work properly
This will allow for slices to be registered manually, outside of the standard slice loading that happens during application init

Also, store slices in a hash, keyed by their name, for easier lookup later
This includes deleting the already_booted_spec, since we no longer raise an error when attempting to boot a previously booted application, instead just making that a no op.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.