Ez (or someone) asked on #merb tonight whether respond_to was the
right API for
what it does. After some discussion and pasties, I offer the
following proposed
API for content negotiation and response format selection:
First, what does respond_to do right now? I see at as performing 3
distinct
operations:
1. parse params[:format] and the accepts header to find out what
format the
client wants
2. look at all the calls to the responder to see what formats the
action
provides
3. reconcile 1 and 2 and pick a format
I propose that we split #2 and #3 into explicit steps.
Here''s a sample controller using the new API:
class ManyFormats < Application
# controllers provide :html by default, so this is implied:
provides :html
# every action in this controller also provides :xml and :txt
provides :xml, :txt
def index
# index also provides :js
provides :js
# this is smart, and based on all the things
# currently provided and all the things the client
# accepts, returns a Symbol for the response format
case request.format
when :html, :txt
render
when :xml
SomeClass.to_xml
when :js
"Object.new()"
end
end
def textonlyaction
only_provides :txt
# this wipes out :html and :xml from the controller-level
# declarations
render
end
end
Win #1: respond_to is no longer a magic black box, we have explicit
definition of
our formats and explicit flow control of our rendering
Win #2: in the case where most/all formats use templates, we get
simpler code:
class CurrentWay < Application
def index
respond_to do |fmt|
fmt.html { render }
fmt.xml { render }
fmt.txt { render }
fmt.js { render }
end
end
end
class ProposedWay < Application
def index
provides :html, :xml, :txt, :js
render
end
end
Win #2.5: Say you wrote an app without respond_to, using only HTML,
and then
decide you want to support FBML or PBJML. With the current API you
have to
modify every single action in every single controller to add a
respond_to block.
With the proposed API you add "provides :pbjml" to Application and
add your
templates and it''s done.
Added bonus: we can keep a respond_to on top of the provides()
variables for
people who like it.
Thoughts? Suggestions? I''m not too emotionally attached to this
yet, so feel
free to pick at it or shoot it down.
If it''s still deemed viable by the weekend, I''ll code it.
I very much dislike the respond_to case-statement style of handling multiple formats. If you have a class that has the same case statement in most of its methods, then it should be broken up into multiple classes. Case statements to select behavior that way are redundant in an object-oriented language. I''ve been mulling over a cleaner way of replacing respond_to with either a superclass and subclasses, or a class and modules. I''ve been trying to work that out in Rails, but if it''s a useful design it should port over to Merb pretty easily. I do like how you break out the provides() method, but I think we can get there another way with less work. I don''t mean to be cryptic or anything, but I haven''t worked out my ideas well enough to say anything really specific about them yet. Now that you''ve reminded me again about this issue I''ll see if I can get something sorted out soon. --josh On Sep 19, 2007, at 10:00 PM, Michael D. Ivey wrote:> Ez (or someone) asked on #merb tonight whether respond_to was the > right API for > what it does. After some discussion and pasties, I offer the > following proposed > API for content negotiation and response format selection: > > First, what does respond_to do right now? I see at as performing 3 > distinct > operations: > 1. parse params[:format] and the accepts header to find out what > format the > client wants > 2. look at all the calls to the responder to see what formats the > action > provides > 3. reconcile 1 and 2 and pick a format > > I propose that we split #2 and #3 into explicit steps. > > Here''s a sample controller using the new API: > > class ManyFormats < Application > # controllers provide :html by default, so this is implied: > provides :html > > # every action in this controller also provides :xml and :txt > provides :xml, :txt > > def index > # index also provides :js > provides :js > > # this is smart, and based on all the things > # currently provided and all the things the client > # accepts, returns a Symbol for the response format > case request.format > when :html, :txt > render > when :xml > SomeClass.to_xml > when :js > "Object.new()" > end > end > > def textonlyaction > only_provides :txt > # this wipes out :html and :xml from the controller-level > # declarations > render > end > end > > > Win #1: respond_to is no longer a magic black box, we have explicit > definition of > our formats and explicit flow control of our rendering > > Win #2: in the case where most/all formats use templates, we get > simpler code: > > class CurrentWay < Application > def index > respond_to do |fmt| > fmt.html { render } > fmt.xml { render } > fmt.txt { render } > fmt.js { render } > end > end > end > > class ProposedWay < Application > def index > provides :html, :xml, :txt, :js > render > end > end > > Win #2.5: Say you wrote an app without respond_to, using only HTML, > and then > decide you want to support FBML or PBJML. With the current API you > have to > modify every single action in every single controller to add a > respond_to block. > With the proposed API you add "provides :pbjml" to Application and > add your > templates and it''s done. > > Added bonus: we can keep a respond_to on top of the provides() > variables for > people who like it. > > > Thoughts? Suggestions? I''m not too emotionally attached to this > yet, so feel > free to pick at it or shoot it down. > > If it''s still deemed viable by the weekend, I''ll code it.-- Josh Susser http://blog.hasmanythrough.com
On Sep 20, 2007, at 12:54 AM, Josh Susser wrote:> I very much dislike the respond_to case-statement style of handling > multiple formats. If you have a class that has the same case > statement in most of its methods, then it should be broken up into > multiple classes. Case statements to select behavior that way are > redundant in an object-oriented language.Do you mean you dislike the current respond_to, my proposed change, or both? :) Any chance you''ll have a high-level picture of what you''re thinking before the weekend? I''d love to see it.
Sometimes it seems like actions deserve to be their own objects: we map routes to them, we''d like to know what parameters they expect, we want to say which http methods they accept, we assign them before/after filters, and we even hold collections of them (Controller.callable_actions). This responds_to issue is another example of how we''d like to treat actions as objects. We would like for the controller to look at the request, determine which format to send, and then simply call my_action.render_json or my_action.render_html. Simultaneously, our controllers are not very class like. They are basically instantiate them only to call the action. Changing the Controller class into a module would not be so hard in the current Merb - they are (more or less) just containers for actions. Has anyone else been having these thoughts? ry On 9/20/07, Michael D. Ivey <ivey at gweezlebur.com> wrote:> On Sep 20, 2007, at 12:54 AM, Josh Susser wrote: > > I very much dislike the respond_to case-statement style of handling > > multiple formats. If you have a class that has the same case > > statement in most of its methods, then it should be broken up into > > multiple classes. Case statements to select behavior that way are > > redundant in an object-oriented language. > > Do you mean you dislike the current respond_to, my proposed change, > or both? :) > > Any chance you''ll have a high-level picture of what you''re thinking > before the weekend? I''d love to see it. > _______________________________________________ > Merb-devel mailing list > Merb-devel at rubyforge.org > http://rubyforge.org/mailman/listinfo/merb-devel >
For example,
module Controllers::Products
class Index < GetAction
def initialize(params)
@products = Product.paginate(params[:page] || 1)
end
def response_json
@products.to_json
end
def response_html
render ''products/index.erb''
end
end
class Show < GetAction
extra_route '':id''
def initialize(params)
@products = Product.find(params[:id])
end
def response_html
render ''products/show.erb''
end
end
end
<%= link_to ''Products'', Controllers::Products::Index.url
%>
On 9/20/07, ry dahl <ry at tinyclouds.org> wrote:> Sometimes it seems like actions deserve to be their own objects: we
> map routes to them, we''d like to know what parameters they expect,
we
> want to say which http methods they accept, we assign them
> before/after filters, and we even hold collections of them
> (Controller.callable_actions). This responds_to issue is another
> example of how we''d like to treat actions as objects. We would
like
> for the controller to look at the request, determine which format to
> send, and then simply call my_action.render_json or
> my_action.render_html. Simultaneously, our controllers are not very
> class like. They are basically instantiate them only to call the
> action. Changing the Controller class into a module would not be so
> hard in the current Merb - they are (more or less) just containers for
> actions.
>
> Has anyone else been having these thoughts?
>
> ry
That''s kind of interesting. I agree that actions seem more like objects than methods sometimes, but here you have them done up as classes, not objects. I think the number of classes involved in this approach might be a bit much. Gets me thinking though. I''ll see if I can apply some gray matter to the problem in the next day or two... On Sep 20, 2007, at 11:13 AM, ry dahl wrote:> For example, > > module Controllers::Products > class Index < GetAction > def initialize(params) > @products = Product.paginate(params[:page] || 1) > end > > def response_json > @products.to_json > end > > def response_html > render ''products/index.erb'' > end > end > > class Show < GetAction > extra_route '':id'' > > def initialize(params) > @products = Product.find(params[:id]) > end > > def response_html > render ''products/show.erb'' > end > end > end > > <%= link_to ''Products'', Controllers::Products::Index.url %> > > > On 9/20/07, ry dahl <ry at tinyclouds.org> wrote: >> Sometimes it seems like actions deserve to be their own objects: we >> map routes to them, we''d like to know what parameters they expect, we >> want to say which http methods they accept, we assign them >> before/after filters, and we even hold collections of them >> (Controller.callable_actions). This responds_to issue is another >> example of how we''d like to treat actions as objects. We would like >> for the controller to look at the request, determine which format to >> send, and then simply call my_action.render_json or >> my_action.render_html. Simultaneously, our controllers are not very >> class like. They are basically instantiate them only to call the >> action. Changing the Controller class into a module would not be so >> hard in the current Merb - they are (more or less) just containers >> for >> actions. >> >> Has anyone else been having these thoughts? >> >> ry >-- Josh Susser http://blog.hasmanythrough.com
On 9/20/07, John Weir <john at smokinggun.com> wrote:> how would filters and common methods work?one way this could be done is by attaching a filters as class methods to the actions. for example module Controllers::Products before Index, Show do |request| raise Unauthorized unless request.session[:user].admin? end ... end would take the block argument, add do Index.class_eval { @@before_filter = block } for both Index and Show. At the time of request, the dispatcher could call that block before instantiating the action. this is just an example for entertaining the idea - not a proposal - i haven''t thought all the way through it> > For example, > > > > module Controllers::Products > > class Index < GetAction > > def initialize(params) > > @products = Product.paginate(params[:page] || 1) > > end > > > > def response_json > > @products.to_json > > end > > > > def response_html > > render ''products/index.erb'' > > end > > end > > > > class Show < GetAction > > extra_route '':id'' > > > > def initialize(params) > > @products = Product.find(params[:id]) > > end > > > > def response_html > > render ''products/show.erb'' > > end > > end > > end > > > > <%= link_to ''Products'', Controllers::Products::Index.url %> > > > > > > On 9/20/07, ry dahl <ry at tinyclouds.org> wrote: > >> Sometimes it seems like actions deserve to be their own objects: we > >> map routes to them, we''d like to know what parameters they expect, we > >> want to say which http methods they accept, we assign them > >> before/after filters, and we even hold collections of them > >> (Controller.callable_actions). This responds_to issue is another > >> example of how we''d like to treat actions as objects. We would like > >> for the controller to look at the request, determine which format to > >> send, and then simply call my_action.render_json or > >> my_action.render_html. Simultaneously, our controllers are not very > >> class like. They are basically instantiate them only to call the > >> action. Changing the Controller class into a module would not be so > >> hard in the current Merb - they are (more or less) just containers > >> for > >> actions. > >> > >> Has anyone else been having these thoughts? > >>
Merb controllers cannot be modules because modules are not instantiatable. The way merb''s thread safety works is by instantiating a new controller object for each new request/thread. -Ezra On Sep 20, 2007, at 1:13 PM, ry dahl wrote:> On 9/20/07, John Weir <john at smokinggun.com> wrote: >> how would filters and common methods work? > > one way this could be done is by attaching a filters as class methods > to the actions. for example > > module Controllers::Products > before Index, Show do |request| > raise Unauthorized unless request.session[:user].admin? > end > ... > end > > would take the block argument, add do > Index.class_eval { @@before_filter = block } > for both Index and Show. At the time of request, the dispatcher could > call that block before instantiating the action. > > this is just an example for entertaining the idea - not a proposal - i > haven''t thought all the way through it > > >>> For example, >>> >>> module Controllers::Products >>> class Index < GetAction >>> def initialize(params) >>> @products = Product.paginate(params[:page] || 1) >>> end >>> >>> def response_json >>> @products.to_json >>> end >>> >>> def response_html >>> render ''products/index.erb'' >>> end >>> end >>> >>> class Show < GetAction >>> extra_route '':id'' >>> >>> def initialize(params) >>> @products = Product.find(params[:id]) >>> end >>> >>> def response_html >>> render ''products/show.erb'' >>> end >>> end >>> end >>> >>> <%= link_to ''Products'', Controllers::Products::Index.url %> >>> >>> >>> On 9/20/07, ry dahl <ry at tinyclouds.org> wrote: >>>> Sometimes it seems like actions deserve to be their own objects: we >>>> map routes to them, we''d like to know what parameters they >>>> expect, we >>>> want to say which http methods they accept, we assign them >>>> before/after filters, and we even hold collections of them >>>> (Controller.callable_actions). This responds_to issue is another >>>> example of how we''d like to treat actions as objects. We would like >>>> for the controller to look at the request, determine which >>>> format to >>>> send, and then simply call my_action.render_json or >>>> my_action.render_html. Simultaneously, our controllers are not very >>>> class like. They are basically instantiate them only to call the >>>> action. Changing the Controller class into a module would not be so >>>> hard in the current Merb - they are (more or less) just containers >>>> for >>>> actions. >>>> >>>> Has anyone else been having these thoughts? >>>> > _______________________________________________ > Merb-devel mailing list > Merb-devel at rubyforge.org > http://rubyforge.org/mailman/listinfo/merb-devel-- Ezra Zygmuntowicz -- Founder & Ruby Hacker -- ez at engineyard.com -- Engine Yard, Serious Rails Hosting -- (866) 518-YARD (9273)
I like the "provides :xml" or "publish :xml" syntax with the
usual options
(:only, :except, :if etc.).that way you can manage all the response type
from one location.
Sometimes respond_to case-style works well when each response gets something
different but many times that''s not the case and when it
doesn''t, it sucks.
In rails I created a helper that looks at the format param so that I can do
something like render :xml => @user.to_xml if requested_as(:xml)
I personally think that the guys from make_resourceful got it almost right.
One thing that I think it''s smart is not to use the "def"
keyword. Since
actions are more than just methods. You could do something more clever.
build :index{ something here } that would either define multiple methods or
return objects.
You can still make it work with just methods, maybe something like this
class User < Application
# sets default responses and returns values
publish :html, :xml, :json, :only => [:show, :edit, :destroy], :returns
=>
{:user => [:login, :email], :head => "success"}
# run filters conditionally to request type
before :index, :show, :do => [:get_user, :increase_counter] if
requested_as(:html, :xml)
# custom responses by request types
responses_for [:index, :destroy] => {:xml => :user,
:html => render(" index.erb")}
def index
#only code in common with all the responses here.
end
end
Just trowing some ideas out there.
Diego
I imagine you could pass reponses_for an array of hashes too.
responses_for :show => {:xml => :user}, :destroy => {:xml => {:head
=>
"success"}}
responses_for :edit => {:xml => Proc.new{ render "index.erb"}}
On 9/20/07, Ezra Zygmuntowicz <ez at engineyard.com>
wrote:>
>
> Merb controllers cannot be modules because modules are not
> instantiatable. The way merb''s thread safety works is by
> instantiating a new controller object for each new request/thread.
>
> -Ezra
>
> On Sep 20, 2007, at 1:13 PM, ry dahl wrote:
>
> > On 9/20/07, John Weir <john at smokinggun.com > wrote:
> >> how would filters and common methods work?
> >
> > one way this could be done is by attaching a filters as class methods
> > to the actions. for example
> >
> > module Controllers::Products
> > before Index, Show do |request|
> > raise Unauthorized unless request.session[:user].admin?
> > end
> > ...
> > end
> >
> > would take the block argument, add do
> > Index.class_eval { @@before_filter = block }
> > for both Index and Show. At the time of request, the dispatcher could
> > call that block before instantiating the action.
> >
> > this is just an example for entertaining the idea - not a proposal - i
> > haven''t thought all the way through it
> >
> >
> >>> For example,
> >>>
> >>> module Controllers::Products
> >>> class Index < GetAction
> >>> def initialize(params)
> >>> @products = Product.paginate(params[:page] || 1)
> >>> end
> >>>
> >>> def response_json
> >>> @products.to_json
> >>> end
> >>>
> >>> def response_html
> >>> render ''products/index.erb''
> >>> end
> >>> end
> >>>
> >>> class Show < GetAction
> >>> extra_route '':id''
> >>>
> >>> def initialize(params)
> >>> @products = Product.find(params[:id])
> >>> end
> >>>
> >>> def response_html
> >>> render ''products/show.erb''
> >>> end
> >>> end
> >>> end
> >>>
> >>> <%= link_to ''Products'',
Controllers::Products::Index.url %>
> >>>
> >>>
> >>> On 9/20/07, ry dahl < ry at tinyclouds.org> wrote:
> >>>> Sometimes it seems like actions deserve to be their own
objects: we
> >>>> map routes to them, we''d like to know what
parameters they
> >>>> expect, we
> >>>> want to say which http methods they accept, we assign them
> >>>> before/after filters, and we even hold collections of them
> >>>> (Controller.callable_actions ). This responds_to issue is
another
> >>>> example of how we''d like to treat actions as
objects. We would like
> >>>> for the controller to look at the request, determine which
> >>>> format to
> >>>> send, and then simply call my_action.render_json or
> >>>> my_action.render_html. Simultaneously, our controllers are
not very
> >>>> class like. They are basically instantiate them only to
call the
> >>>> action. Changing the Controller class into a module would
not be so
> >>>> hard in the current Merb - they are (more or less) just
containers
> >>>> for
> >>>> actions.
> >>>>
> >>>> Has anyone else been having these thoughts?
> >>>>
> > _______________________________________________
> > Merb-devel mailing list
> > Merb-devel at rubyforge.org
> > http://rubyforge.org/mailman/listinfo/merb-devel
>
> -- Ezra Zygmuntowicz
> -- Founder & Ruby Hacker
> -- ez at engineyard.com
> -- Engine Yard, Serious Rails Hosting
> -- (866) 518-YARD (9273)
>
>
> _______________________________________________
> Merb-devel mailing list
> Merb-devel at rubyforge.org
> http://rubyforge.org/mailman/listinfo/merb-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://rubyforge.org/pipermail/merb-devel/attachments/20070921/c540dd96/attachment.html
On Sep 20, 2007, at 3:18 PM, Ezra Zygmuntowicz wrote:> > Merb controllers cannot be modules because modules are not > instantiatable. The way merb''s thread safety works is by > instantiating a new controller object for each new request/thread. >But under _ry''s proposed system, there is still an object to instantiate per request--it''s just the action instead of the controller that gets its own class. From a software engineering perspective, I like the direction _ry is going. The question he is raising seems to be, "What is the role of a controller?" and on a related note, "What is the role of an action?" I''ve seen very large controllers before, and they don''t always make a lot of sense as a single class. A typical controller will have a bunch of public actions followed by a bunch of protected methods. Except in very segmented areas of the website (e.g. an admin area), each protected method is generally called once by one public action-- there is not always a lot of re-use going on; rather, factoring these methods out seems to be a way of making the code in public actions more readable. In typical object-oriented design, we try to create classes that map easily to real-world things or events. In this case, it seems that an Action is a reasonable object class to consider. As _ry has mentioned, Actions have: - one or more route mappings - security measures (e.g. you can see an object, but you have to be logged in to edit it) - object state (request parameters) - multiple formats for the response - multiple HTTP request method options (GET/POST etc.) - before / after filters _ry also mentioned that we currently instantiate controllers primarily to call an action. It''s possible that we could get a small performance benefit by not having to perform a check on all of the controller''s before / after filters to see if they apply to the action. Rather, an action would just be an Action object, and it would make calls to other things in the initialize method (or ''before'' method, see below). With this architecture, we could take advantage of inheritance in the OO way (as well as map exceptions to actions the OO way): module Merb class Action def before; end def after; end end end class AdminAction < Merb::Action def before if User.find(session[:user_id]).admin? # ok else # Raise the Unauthorized controller''s NeedToLogin action (class): raise Unauthorized::NeedToLogin, "Please log in to the admin area." end super rescue ActiveRecord::RecordNotFound raise InternalServerError::StaleSession end end module Admin module Products class Show < AdminAction def initialize # ... end def before # Add additional before filter code, then # call AdminAction''s before filter super end def html_response render end def after # Do something else afterward super end end end end As a side-effect of not using before filters, we would be able to simplify the code to re-load a class in development mode (the complicated part comes when we have to remove constants so that before filters don''t get double- or triple-called). I think _ry''s ideas in this area are definitely worthy of discussion. Duane Johnson (canadaduane)
On 21 Sep 2007, at 21:22, Duane Johnson wrote:> With this architecture, we could take > advantage of inheritance in the OO way (as well as map exceptions to > actions the OO way):I think this is an interesting point! With more of my projects taking the REST view on the world, I''ll often have a base controller with common create/update/show/delete methods, that I then overide if they need modifying. By taking an approach where actions are classes, this situation might be improved. Worth thinking about ... but is quite a step away from the current "way" that it''s not going to be an easy switch. might the idea maybe be to have: app/controllers/users/index.rb class Users::Index < Users::Action def to_html render end def to_xml ''<xml/>'' end end which then renders the template at app/views/users/index.html.erb where Users::Action is the base "action" class for users (where you would put all your methods that all user actions require) and Users::Action is inherited from Application::Action (where all the app-level methods are) which is a Merb::Action I think the GetAction idea is poor and all that sort of logic should stay in the router code. I like the ideas of the matching up of the action.rb file and the template.erb, but I''m not entirly convinced, the directory structures in /controller and /views would be nicely mirrored. anyway this is all very theoretical, more things for the _ry- experimental-playground-branch ;) Regards Farms. -------------- next part -------------- An HTML attachment was scrubbed... URL: http://rubyforge.org/pipermail/merb-devel/attachments/20070922/0c81d94d/attachment.html
Has anyone had any more thoughts on what is to be done with respond_to? I''m using it quite a lot now, and it really doesn''t feel great, especially when you have quite a large action that has many responses. ? I really liked the ideas with actions as classes... and was playing with syntax, for a router and app layout ... attached should be a prettymuch barebones app with some comments if anyone is interested. My routing idea basically hands over most of the routing logic to the app developer and simply provides a frame to say "if a url matches this regex"..."run this block"... "I expect to get an action class back" this would make the router code in any framework very small/fast and easy to follow, and put all the routing power in the hands of the dev, who wouldn''t have to learn any special syntax. Regards Farms. Design & Dev Oxygen. http://www.oxdi.eu On 22 Sep 2007, at 13:34, Chris Farmiloe wrote:> > > > On 21 Sep 2007, at 21:22, Duane Johnson wrote: >> With this architecture, we could take >> advantage of inheritance in the OO way (as well as map exceptions to >> actions the OO way): > > I think this is an interesting point! > > > > With more of my projects taking the REST view on the world, I''ll > often have a base controller with common create/update/show/delete > methods, that I then overide if they need modifying. > > By taking an approach where actions are classes, this situation > might be improved. > > Worth thinking about ... but is quite a step away from the current > "way" that it''s not going to be an easy switch. > > might the idea maybe be to have: > > app/controllers/users/index.rb > > class Users::Index < Users::Action > def to_html > render > end > > def to_xml > ''<xml/>'' > end > end > > which then renders the template at > > app/views/users/index.html.erb > > > where Users::Action is the base "action" class for users (where you > would put all your methods that all user actions require) and > Users::Action is inherited from Application::Action (where all the > app-level methods are) which is a Merb::Action > > I think the GetAction idea is poor and all that sort of logic > should stay in the router code. > > I like the ideas of the matching up of the action.rb file and the > template.erb, but I''m not entirly convinced, the directory > structures in /controller and /views would be nicely mirrored. > > > anyway this is all very theoretical, more things for the _ry- > experimental-playground-branch ;) > > > > > > Regards > > Farms. > >-------------- next part -------------- An HTML attachment was scrubbed... URL: http://rubyforge.org/pipermail/merb-devel/attachments/20071004/eea5613f/attachment-0002.html -------------- next part -------------- A non-text attachment was scrubbed... Name: example.tar.bz2 Type: application/octet-stream Size: 2056 bytes Desc: not available Url : http://rubyforge.org/pipermail/merb-devel/attachments/20071004/eea5613f/attachment-0001.obj -------------- next part -------------- An HTML attachment was scrubbed... URL: http://rubyforge.org/pipermail/merb-devel/attachments/20071004/eea5613f/attachment-0003.html