raptor
raptor copied to clipboard
garybernhardt
How should applications be structured (Pt. 2)?
I read through the original discussion of this, and there were lots of good things there. I want to use DCI, and separate my application like Uncle Bob's "lost architecture" talk mentioned (which I've also seen before reading the previous discussion).
I've got an app that looks like this:
module MyApp
App = Raptor::App.new(self) do
root :render => "root", :present => :home_page
path "user" do
index :to => 'Repository::User.all'
new
create :to => 'MyApp::Context::CreateUser.execute', :redirect => :index
end
end
end
I've got a context that looks like this:
module MyApp
module Context
class CreateUser
def self.execute(user, params)
raise Raptor::ValidationError if params['email'].empty? || params['password'].empty?
user.email = params['email']
user.password = params['password']
user.extend(UnregisteredUser).register
{:user => @user}¬
end
protected
module UnregisteredUser
def register
Arden::Repository.for(:user).create(self)
end
end
end
end
end
So questions:
- Is there a way to get validation errors into the presenter? I'm guessing I'd want to use some kind of DI injector for this, but I'm at a loss on how to write it. It didn't look like Raptor::ValidationError had any way of storing errors or anything like that.
- I'm also thinking that using Raptor::ValidationError directly inside my context class is a bad idea. Raptor is a delivery mechanism, so I should be making a buffer layer to handle the translation of my app and the delivery mechanism (raptor, in this case)
- All in all, I think it's a little murky how things get passed around in the system. A little more documentation would help here, I think, explaining specifically what the conventions should be (this seems to be a largely convention based system).
I think the big "how to design an app with raptor" question is how to properly pass data around the layers. DI seems like the preferred way, but it's unclear to me how to write an Injectables class that does this.
DCI
I (and I think @garybernhardt) feel that DCI is a big mistake (at least, as I understand it right now). Whilst it's probably better than Rails, including modules into things at runtime makes me want a shower. I don't think we'll ever be doing explicit support for DCI in raptor.
Answers to Questions
Is there a way to get validation errors into the presenter? I'm guessing I'd want to use some kind of DI injector for this, but I'm at a loss on how to write it. It didn't look like Raptor::ValidationError had any way of storing errors or anything like that.
Firstly: Raptor::ValidationError is named badly, it should really be something like Raptor::Routes::ValidationError. It's used for checking that your routes are somewhat sane at startup time, and nothing else. You're not meant to use it externally.
As of right now, there isn't a good way to do this. I think (eventually), we'll keep the injector around when doing internal redirects, and allow you to inject exception into your presenter/views for when you rescue from an exception using raptor. Then your raptor ORM wrapper would store validation errors inside exceptions. This probably comes after doing some more internal design work though (right now the injector gets passed nearly everywhere, and I think we're going to move into it being stored in a thread-local variable).
I'm also thinking that using Raptor::ValidationError directly inside my context class is a bad idea. Raptor is a delivery mechanism, so I should be making a buffer layer to handle the translation of my app and the delivery mechanism (raptor, in this case).
ValidationError is purely meant for route validations, nothing else right now (this conversation is pointing out the naming flaw there rather well).
However, to answer the general point, one of raptor's design goals (for me anyway), is to minimize the amount of translation you have to do between your app and the web (and to push all that translation inside the router). This is pretty ambitious, and I have no idea how well it will pan out.
All in all, I think it's a little murky how things get passed around in the system. A little more documentation would help here, I think, explaining specifically what the conventions should be (this seems to be a largely convention based system).
I think everybody agrees with that. However, raptor isn't baked yet, and we're not that confident on the design (though I'm getting more confident, apart from a couple of (as yet) unsolved issues). The way I'm attempting to increase confidence here is by writing apps in it (which I am doing right now). I think once the design is more baked, we'll be adding significantly more docs.
injecting modules into things at runtime is all kinds of bad.
I hear you on that. I think doing DCI with decorators is just as fine though. I'm in experimenting stage, and I don't think that using a decorator class as opposed to a pure module is all that much different from a conceptual standpoint. Conceptually, adding behavior to a class is the preferred way, but if that's a large performance hit (as I gathered from the other thread), I don't have a strong resistance to using decorators. It's an implementation detail, and largely irrelevant to this discussion.
Hooking up your application to raptor should be basically the same as if you do pure DCI, or if you use Uncle Bob's Interactors (which I think is just a step below DCI).
I don't think we'll ever be doing explicit support for DCI in raptor.
And I don't think you should be. DCI is an architecture pattern that should be outside of your delivery mechanism chain. There's still a lot of interesting work to be done on how to build web apps with it.
allow you to inject exception into your presenter/views for when you rescue from an exception using raptor. Then your raptor ORM wrapper would store validation errors inside exceptions.
I like this method.
is to minimize the amount of translation you have to do between your app and the web (and to push all that translation inside the router)
Don't you get into Sinatra territory there? That basically sounds like a "route controller" hybrid like sinatra and padrino have. I could be thinking in a different direction though.
The way I'm attempting to increase confidence here is by writing apps in it (which I am doing right now)
Yep, this system is really interesting to me. I don't like how rails does things at all, in how it doesn't really facilitate serving your application. It strongly encourages you to write "rails apps", and I'm seeing that in our application at work.
I'm porting over a php application to ruby for experimentation on "doing it the right way" in a completely separated architecture, and it seems like the way raptor does things can facilitate this easily.
Balls, looking through the code suggests that @garybernhardt did mean for users to use ValidationError externally. That's something we've (iirc) never discussed though, and I don't know what his thoughts on it are. Please to be ignoring my failings above.
Don't you get into Sinatra territory there? That basically sounds like a "route controller" hybrid like sinatra and padrino have. I could be thinking in a different direction though.
I probably was quite jumbled when I said that. Raptor (as it stands currently) is all router. There isn't anything else. You never call it, it always calls you (except for ValidationError, but I think that is only semi-baked right now). The router is designed to do all of your HTTP interactions, so you can focus on your application as it stands. We're definitely not looking towards Sinatra territory (Sinatra is very lightweight compared to Raptor, and requires you to explicitly write "controller" actions).
looking through the code suggests that @garybernhardt did mean for users to use ValidationError externally.
I think that's where I got the idea. :)
Raptor (as it stands currently) is all router. There isn't anything else.
Isn't it basically MVP+router? Seems like the M is the :to part of the route definition (M is your application, basically), and the VP part is hooked up in the route internally as well by convention.
This is why Raptor is very interesting to me, because there is clear separation between the delivery mechanism and your application, and it seems to be encouraged to design your app this way. With rails, sinatra, etc, it's very easy to mix concerns.
To me, MVC and MVP are delivery mechanism patterns, not application design patterns. This is where DCI and Interactors come into play. Kudos on the work done so far.
The ValidationError thing was hacked in when I added the seven standard routes. The problem is that the router needs to know when to re-render things. For example, the create action renders new on a ValidationError:
def create(params={})
route(:create, "POST", "/",
{:redirect => :show,
ValidationError => :new,
:to => "#{record_module}.create"}.merge(params))
end
Without a ValidationError exception that Raptor knows about, I don't see a way for the built-in routes to have that behavior. :(
More generally, @zombor, I'm glad that you find the separation between delivery and application so clear! That's exactly what prompted us to start the project: frustration with Rails forcing coupling everywhere.
Any ideas about the validation stuff? ValidationError can definitely go if there's a better way to do it, and especially if that way makes feeding errors to presenters easier.
This kinda comes back to design thread part 1, I guess. It's all about responses from the services: some services need to say "I failed for this reason", and that needs to get to the presenter. We never decided between exceptions and plain old objects for that during the last thread.
Any ideas about the validation stuff? ValidationError can definitely go if there's a better way to do it, and especially if that way makes feeding errors to presenters easier.
What I do in rails right now is have the context throw an error exception(like Error::NotFound or Error::Validation), then the controller rescues them and displays the correct view with the exception data. The error exceptions contain the error hash, and any other information that is needed to display the error. Maybe somehow work that in like you did ValidationError in the route definition so you could define what kinds of exceptions go where.
I definitely don't think it's a good idea for Raptor to define these exceptions and except the application to use them like I did in my example above.
I think exceptions are clearer to handle, otherwise you end up with hashes/objects that have {:status => :success} in them.
You could have ValidationError capable of accepting arbitrary objects, and add it to the injector so that the route we're redirecting to can get at it?
path "articles" do
update :ValidationError => :edit
edit
end
The worry here is that you now (probably) want the injector to be capable of injecting a Maybe like thing to the view, so that you can render the :new action without a ValidationException coming in.
We can already do this with optional arguments in the injector (you can inject a method with a default argument, and if there's no injectable available, it'll use the default arg), but we can't do it in the view (at least not via the method_missing thing that's currently proposed). If the view allowed you to inject things via inject(:exception, fallback), this'd probably be fine (as per @garybernhardt's first option in #55). I've written up some other options for fallbacks in #55.
One way around that is to require all validation exceptions go into the presenter at construction time. I'm not sure I like this very much though.
Basically what I think I want is something that will take the response of the delegator and put it into the presenter somehow. The response would either be an exception, or something that was returned by the delegator call.
Another thing that I thought of was sessions. I assume we should just rack stuff for this. However, since we don't want to be using delivery mechanism stuff in our applications, we'd need some kind of intermediate layer between the app and raptor that handled this stuff. This sounds an awful lot like a controller to me. Maybe I'm thinking about it wrong though? Thoughts?
So right now, you can bring in the delegate result by having your initialize method on the presenter take an arg of "subject". Once that pull req is merged, you can ask for "exception"as well in the presenter's constructor.
If the presenter can have the exception injected, is it necessary to inject the exception directly into the view with a default? Isn't translating from the injected exception to view-consumable data the job of the presenter?
I'm of the opinion that the template (what you call the view, i think) should get everything from the presenter (and nowhere else). It provides a clear separation between view logic and display.
This is why I prefer to use mustache for views, but that's neither here nor there.
Yep, I agree. Actually, if we're serious about all data in templates coming from presenters, then templates shouldn't have anything directly injected into them at all. Thoughts, @tcrayford?
Not directly injecting into templates seems tempting, but I think it messes up with stuff like "I want to show a logout button when the user is logged in". If you want this behaviour in a layout, you have to inject the fact that the user is logged in into every presenter that is on a page that'll show logout, which sucks (and each of those presenters is just going to expose the logged-in-ness to the layout).
Well, the classical OO answer to that would be for those presenters to subclass (or mix in—whatever) the logout button presenter. Problem? ;)
Yep, when using mustache, I have a layout class that contains all this logic, and my pages extend this.
In classical MVC, I'd pass that info in via the controller. Not sure how I'd do it in a system like this. I don't think I understand how Injectors work completely. Can they grab state from other areas of the system (like session, etc)?
Yes—an injectable can have stuff injected into it. Your application can define injectables that have lower-level objects injected into them. It's not clear how understandable this will be. Right now it's more confusing than it needs to be because the custom injectable API is ugly.
If we have subclassing of presenters, we're still going to end up with a lot of presenters like this:
class ArticlePresenter < UserLoggedInPresenter
attr_reader :subject
def initialize(user_logged_in, subject)
super(user_logged_in)
@subject = subject
end
end
You remove a single attr_reader, but still have to pass around the fact that the user is logged in.
HERETICAL IDEA
Remove presenters, have views/templates that are all directly mustache (and encourage inheritance of views), screw other templating languages.
I'm a huge fan of that :)
When initially looking into raptor I learned about Tilt, but was very disappointed that mustache wasn't available. It's basically a really nice presenter pattern library with a strict template language.
I actually had a crack at implementing mustache with raptor's current templating design, but threw it out because we couldn't do things like content_for :title. I assume mustache users just override title in the layout subclass for this stuff, right?
When I asked about mustache on Twitter, I got a lot of pretty negative opinions. The specific objections escape me. It does seem to fit in well with Raptor, though...
Yeah. I don't really like content_for personally. I feel like it's just a hack around the fact there's no real class behind the template.
Tom, you are blowing my entire mind right now by advocating inheritance. ;)
I don't think I'm strongly advocating it, just saying we could do it, and it does solve this problem rather neatly.
I think a lot of the objections are because it doesn't work like erb. You can't use native ruby things in the template like people are used to (for links, etc). I initially had our new project on mustache, but I got overridden and we switched to erb with presenters because of this fact.
I don't think (and hope) that my objections with mustache aren't those of the typical rails dev. I dislike a bunch of the design the ruby impl of mustache has. E.g. Why do my presenters inherit from View? There's a very obvious muddling of responsibilities there, the View both renders and presents the data. I think this is due to rubyists disliking introducing new classes (but I'm not sure).
However, we don't need that to be user-visible, we can just pile our own jank on top of mustache to hide our users from it's clunkiness.
Yeah, I'd definitely want to avoid forcing Raptor users to write classes inheriting from a Mustache class.
On another topic related to some of the the original posts here, I was able to write some basic error validation and get it displaying in the view:
module MyApp
App = Raptor::App.new(self) do
root :render => "root", :present => :home_page
path "user" do
index :to => 'Repository::User.all'
new
create :to => 'MyApp::Context::CreateUser.execute', :redirect => :index, Error::Validation => :new
end
end
end
module MyApp
module Context
class CreateUser
def self.execute(user, params)
if params['email'].empty? || params['password'].empty?
raise Error::Validation.new(
{:email => params['email'].empty?, :password => params['password'].empty?}
)
end
user.email = params['email']
user.password = params['password']
user.extend(UnregisteredUser).register
{:status => :success, :user => @user}
end
protected
module UnregisteredUser
def register
Arden::Repository.for(:user).create(self)
end
end
end
end
end
module Error
class Validation < Exception
attr_reader :errors
def initialize(errors)
@errors = errors
end
end
module MyApp
module Presenters
class HomePage
end
class User
def initialize(subject, exception = nil)
@subject = subject
@exception = exception
end
let(:email) { @subject.email }
let(:email_error) {
return nil if @exception.nil?
@exception.errors[:email]
}
let(:password_error) {
return nil if @exception.nil?
@exception.errors[:password]
}
end
class UserList
takes :subject
let(:all) { @subject }
end
end
end
This crude implementation seems to work well, and could be expanded to something a lot better without much trouble.
E.g. Why do my presenters inherit from View? There's a very obvious muddling of responsibilities there, the View both renders and presents the data.
Yeah, that's one nice thing I like about the php version of mustache. You can pass in any kind of presenter class into the constructor and it will use that for it's data. I don't think the ruby version can do that.
@tcrayford, when you tried the mustache thing did it actually work at all? Anything we can look at?
btw, I made repos for my experiment application:
https://github.com/zombor/Raptor-Demo-Delivery https://github.com/zombor/Raptor-Demo-App
Delivery is the raptor app, and only includes delivery mechanism files. App is the business logic files of the application.
It currently only does a user registration, and a user logic (stores the user id in the session).
Sometime since I posted this, I wrote a standalone mustache renderer. It lets you use PORO's as your view classes:
https://gist.github.com/zombor/4727040
This eliminates the crappy need to extend and bind your view classes to any kind of renderer, erb, mustache, or anything else. This could probably be adapted easily to use erb as the rendering engine.