Rodrigo Rosenfeld Rosas
2012-May-11 17:58 UTC
What is the point of using :format in routes?
Today I had a strange behavior that made me suspect of jQuery at first, but then it happened that I''ve faced two gotchas, one from CoffeeScript and one from Rails itself. I have something like this: routes.rb post ''/fields/:id.:format'' => ''fields#show'', as: :field, constraints: {id: /\d+/} post ''/fields/remove/:id'' => ''fields#remove'', as: :remove_field routes.js.coffee.erb: <% h = Rails.application.routes.url_helpers { fields: false, remove_field: true }.each do |named_route, expect_id| %> <% if expect_id %> window.<%= named_route %>_path = (id, format=''.json'')-> "<%= h.send :"#{named_route}_path", ''999'' %>#{format}".replace(''999'', id) <% else %> window.<%= named_route %>_path = ''<%= h.send :"#{named_route}_path" %>'' <% end %> <% end %> It happens that my remove action is implemented like this: def remove Field[params[:id]].update deleted: true head :ok end and in my client code I had something like this: $.post remove_field_path(id, ''''), => @refreshTree() But as you have probably figured out the issue was that refreshTree was never called. That is because CoffeeScript will compare (format == null) and it happens that ('''' == null) in JavaScript: https://github.com/jashkenas/coffee-script/issues/947 I''ve already fixed my routes.js.coffee.erb to something like: path = (id, format)-> format = ''.json'' if format is undefined; ... But then I realized that I was expecting Rails request to fail in the first place since my original route was: post ''/fields/remove/:id'' instead of post ''/fields/remove/:id(.:format)'' Then, reading the guide on Routing, I''ve realized that the (.:format) is not needed. I was confused because of the last example in the generated routes.rb to restore the Rails 1 routes style: # match '':controller(/:action(/:id))(.:format)'' Shouldn''t this comment be changed to the example below to avoid such confusion? # match '':controller(/:action(/:id))'' Also, we could include some example like this to give a hint about the default (.:format) rule: # post ''/products/remove/:id'', format: false, as: :remove_product Does it make sense? Sorry for the long e-mail but I thought that a bit of context on a real case might help understand how such examples in the routes.rb could help others. Cheers, Rodrigo. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On 12/05/2012, at 5:58 AM, Rodrigo Rosenfeld Rosas <rr.rosas@gmail.com> wrote: Today I had a strange behavior that made me suspect of jQuery at first, but then it happened that I''ve faced two gotchas, one from CoffeeScript and one from Rails itself. I have something like this: routes.rb post ''/fields/:id.:format'' => ''fields#show'', as: :field, constraints: {id: /\d+/} post ''/fields/remove/:id'' => ''fields#remove'', as: :remove_field If you look at mapper.rb you''ll find the answer https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/routing/mapper.rb Basically, :format is appended to every route unless you explicitly opt out, I believe this started with 3.0? routes.js.coffee.erb: <% h = Rails.application.routes.url_helpers { fields: false, remove_field: true }.each do |named_route, expect_id| %> <% if expect_id %> window.<%= named_route %>_path = (id, format=''.json'')-> "<%= h.send :"#{named_route}_path", ''999'' %>#{format}".replace(''999'', id) <% else %> window.<%= named_route %>_path = ''<%= h.send :"#{named_route}_path" %>'' <% end %> <% end %> It happens that my remove action is implemented like this: def remove Field[params[:id]].update deleted: true head :ok end and in my client code I had something like this: $.post remove_field_path(id, ''''), => @refreshTree() But as you have probably figured out the issue was that refreshTree was never called. That is because CoffeeScript will compare (format == null) and it happens that ('''' == null) in JavaScript: https://github.com/jashkenas/coffee-script/issues/947 I''ve already fixed my routes.js.coffee.erb to something like: path = (id, format)-> format = ''.json'' if format is undefined; ... But then I realized that I was expecting Rails request to fail in the first place since my original route was: post ''/fields/remove/:id'' instead of post ''/fields/remove/:id(.:format)'' Then, reading the guide on Routing, I''ve realized that the (.:format) is not needed. I was confused because of the last example in the generated routes.rb to restore the Rails 1 routes style: # match '':controller(/:action(/:id))(.:format)'' Shouldn''t this comment be changed to the example below to avoid such confusion? # match '':controller(/:action(/:id))'' Also, we could include some example like this to give a hint about the default (.:format) rule: # post ''/products/remove/:id'', format: false, as: :remove_product Does it make sense? Sorry for the long e-mail but I thought that a bit of context on a real case might help understand how such examples in the routes.rb could help others. Cheers, Rodrigo. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Rodrigo Rosenfeld Rosas
2012-May-13 03:10 UTC
Re: What is the point of using :format in routes?
Em 12-05-2012 02:36, Michael Koziarski escreveu:> On 12/05/2012, at 5:58 AM, Rodrigo Rosenfeld Rosas <rr.rosas@gmail.com > <mailto:rr.rosas@gmail.com>> wrote: > >> Today I had a strange behavior that made me suspect of jQuery at >> first, but then it happened that I''ve faced two gotchas, one from >> CoffeeScript and one from Rails itself. >> >> I have something like this: >> >> routes.rb >> post ''/fields/:id.:format'' => ''fields#show'', as: :field, >> constraints: {id: /\d+/} >> post ''/fields/remove/:id'' => ''fields#remove'', as: :remove_field >> > > If you look at mapper.rb you''ll find the answer > > https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/routing/mapper.rb > > > Basically, :format is appended to every route unless you explicitly > opt out, I believe this started with 3.0? >Thanks, Michael, I''ve already found out the answer, but after having a surprise first :) What I was suggesting is that we changed the generated routes.rb so that the Rails 1 style route could be written as match '':controller(/:action(/:id))'' As well as mentioning and explaining the "format" option, like some of these examples: post ''/products/:id.:format'' => ''products#show'', as: :field, constraints: {id: /\d+/} # format is required post ''/products/:id'' => ''products#show'', format: true, as: :field, constraints: {id: /\d+/} # this is equivalent post ''/products/:id.json'' => ''products#show'', format: :json # it only answers to json format post ''/products/:id'' => ''products#show'', format: :json # the same with a shorter URL but still allowing you to cache the result and use the correct content-type But then I decided to test it and I was surprised that it doesn''t make any difference if you use caches_page. Also, there are more weird rules with regards to page or action caching. The routes rules don''t seem to be respected if you use caching. Consider this example: get ''/products'' => ''products#list'', format: :json try to GET /products with caches_page enabled and you won''t get a ''application/json'' content-type in the next responses because it won''t probably even read the routes rules. Also, if you GET /products.html not only it accepts this request but the content-type will be ''text/html'' instead of ''application/json'' (independent from caching). Also, consider this other example: get ''/products.json'' => ''products#list'', format: :json This will also respond to ''/products.json.html'' for example, and I don''t think it should answer to anything else but ''/products.json''. Also, the content-type declared in the routes wouldn''t be respected. Maybe I''m the only one considering the current behavior an issue, but I''d like to ask you to consider some changes in the routing rules with regards to format to make them less loose. What is your opinion on the subject? Best, Rodrigo. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On 13 May 2012, at 04:10, Rodrigo Rosenfeld Rosas wrote:> What I was suggesting is that we changed the generated routes.rb so that the Rails 1 style route could be written as > > match '':controller(/:action(/:id))''Whether you write it with or without :format it ends up being the same route - the mapper checks for the presence of the :format key in the path. The generator adds the :format to make it explicitly visible what''s happening. Some history on why there''s an optional format would probably useful here. We used to generate both formatted routes and non-formatted routes, e.g. you''d have formatted_product_path(@product, :json) and product_path(@product). Upon investigation[1][2] it was discovered that this took up significant amount of memory and was changed to an optional segment[3] in the Rails 2.3 release.> As well as mentioning and explaining the "format" option, like some of these examples: > > post ''/products/:id.:format'' => ''products#show'', as: :field, constraints: {id: /\d+/} # format is required > post ''/products/:id'' => ''products#show'', format: true, as: :field, constraints: {id: /\d+/} # this is equivalent > post ''/products/:id.json'' => ''products#show'', format: :json # it only answers to json format > post ''/products/:id'' => ''products#show'', format: :json # the same with a shorter URL but still allowing you to cache the result and use the correct content-type > > But then I decided to test it and I was surprised that it doesn''t make any difference if you use caches_page. > Also, there are more weird rules with regards to page or action caching. The routes rules don''t seem to be respected if you use caching. > > Consider this example: > > get ''/products'' => ''products#list'', format: :json > > try to GET /products with caches_page enabled and you won''t get a ''application/json'' content-type in the next responses because it won''t probably even read the routes rules.Page caching is explicitly designed to avoid hitting the Rails stack - you need to setup Apache or nginx to serve the cached files from the format-less path with the appropriate content type. Action caching should just avoid the page rendering - if you can reproduce a case where a JSON request is action cached and returns an incorrect content type when serving the cached file then please create a new issue in the tracker.> Also, if you GET /products.html not only it accepts this request but the content-type will be ''text/html'' instead of ''application/json'' (independent from caching).That''s because you''re defining a default format for the route not a constraint - either use a explicit constraint or use a regexp, e.g: get ''/products'' => ''products#list'', :format => /json/ or get ''/products'' => ''products#list'', :constraints => { :format => ''json'' } The mapper assumes non-significant keys in a route definition are defaults unless they''re regexps.> Also, consider this other example: > > get ''/products.json'' => ''products#list'', format: :json > > This will also respond to ''/products.json.html'' for example, and I don''t think it should answer to anything else but ''/products.json''. Also, the content-type declared in the routes wouldn''t be respected.That''s because :format is still being appended to the route - the mapper doesn''t interpret the path you specify other than to look for segment keys and optional segments. If you''re after a format-less path that only serves JSON then use this: get ''/products'' => ''products#list'', :format => false, :defaults => { :format => ''json'' } or if you want the extension then use this: get ''/products.json'' => ''products#list'', :format => false, :defaults => { :format => ''json'' } The only way to turn off the :format segment being added is to either insert it yourself or pass :format => false.> Maybe I''m the only one considering the current behavior an issue, but I''d like to ask you to consider some changes in the routing rules with regards to format to make them less loose.If you can come up with a route that isn''t supported or doesn''t appear to be supported please open a new issue. You don''t need to ping me on it - I normally take a look at all the routing issues that get created. Andrew White [1]: https://rails.lighthouseapp.com/projects/8994/tickets/1215 [2]: https://rails.lighthouseapp.com/projects/8994/tickets/1359 [3]: https://github.com/rails/rails/commit/fef6c32afe2276dffa0347e25808a86e7a101af1 -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Rodrigo Rosenfeld Rosas
2012-May-13 13:16 UTC
Re: What is the point of using :format in routes?
I was not saying that it wasn''t possible to write the proper routes, just that they don''t seem very consistent. I would guess that most applications won''t be API ones, so many of them won''t opt for REST and responds_to. For example, all applications I''ve been working with for the past decade will only need to respond to a single format per action. That means that if I wanted strict rules to my routes I''d have to write them as: get ''/anything'', format: false or get ''/anything.:format'' => ''anything#index'', constraints: {format: /json/}, defaults: {format: :json} I could use a block for this common option except that not all of my actions will return a JSON. Sometimes I''ll just return a "head :ok" for remove actions, for example. But it is important that Rails always returns the correct content-type so that my AJAX callbacks get interpreted correctly. The only reason I have something like ''/anything.json'' in some of my actions is because I want to cache some of them. Today is Mother''s day in Brazil and I''m going to leave in a few minutes to have lunch with my mom in a near city, but I guess I''ve already found some bugs. Consider this snippets: class TestController < ApplicationController caches_action :index def index render json: ''test'' #, content_type: ''application/json'' end def expire expire_action :index #action: :index #, format: :json head :ok end end routes.rb: get "test/index", format: false, as: :index # get "test/index" => ''test#index'', format: false, defaults: {format: :json}, as: :index get "test/expire" I''ve made several experiments that yield to strange results when it comes to content-type. Basically, I''m getting ''text/html'' instead of ''application/json''. This happens even if I pass content_type: ''application/json'' explicitly. Another bug is that expire_action doesn''t seem to work correctly either. For example, if you use the commented route the output will be: Expire fragment views/localhost:3000/assets?controller=test It doesn''t matter if I use a named route or "expire_action action: :index" or "expire_action action: :index, format: :json" if I remember correctly. Sorry, I have to leave now and I''ll only be back very late at night. Have a great Sunday, Rodrigo. Em 13-05-2012 06:10, Andrew White escreveu:> On 13 May 2012, at 04:10, Rodrigo Rosenfeld Rosas wrote: > >> What I was suggesting is that we changed the generated routes.rb so that the Rails 1 style route could be written as >> >> match '':controller(/:action(/:id))'' > Whether you write it with or without :format it ends up being the same route - the mapper checks for the presence of the :format key in the path. The generator adds the :format to make it explicitly visible what''s happening. > > Some history on why there''s an optional format would probably useful here. We used to generate both formatted routes and non-formatted routes, e.g. you''d have formatted_product_path(@product, :json) and product_path(@product). Upon investigation[1][2] it was discovered that this took up significant amount of memory and was changed to an optional segment[3] in the Rails 2.3 release. > >> As well as mentioning and explaining the "format" option, like some of these examples: >> >> post ''/products/:id.:format'' => ''products#show'', as: :field, constraints: {id: /\d+/} # format is required >> post ''/products/:id'' => ''products#show'', format: true, as: :field, constraints: {id: /\d+/} # this is equivalent >> post ''/products/:id.json'' => ''products#show'', format: :json # it only answers to json format >> post ''/products/:id'' => ''products#show'', format: :json # the same with a shorter URL but still allowing you to cache the result and use the correct content-type >> >> But then I decided to test it and I was surprised that it doesn''t make any difference if you use caches_page. >> Also, there are more weird rules with regards to page or action caching. The routes rules don''t seem to be respected if you use caching. >> >> Consider this example: >> >> get ''/products'' => ''products#list'', format: :json >> >> try to GET /products with caches_page enabled and you won''t get a ''application/json'' content-type in the next responses because it won''t probably even read the routes rules. > Page caching is explicitly designed to avoid hitting the Rails stack - you need to setup Apache or nginx to serve the cached files from the format-less path with the appropriate content type. Action caching should just avoid the page rendering - if you can reproduce a case where a JSON request is action cached and returns an incorrect content type when serving the cached file then please create a new issue in the tracker. > >> Also, if you GET /products.html not only it accepts this request but the content-type will be ''text/html'' instead of ''application/json'' (independent from caching). > That''s because you''re defining a default format for the route not a constraint - either use a explicit constraint or use a regexp, e.g: > > get ''/products'' => ''products#list'', :format => /json/ > > or > > get ''/products'' => ''products#list'', :constraints => { :format => ''json'' } > > The mapper assumes non-significant keys in a route definition are defaults unless they''re regexps. > >> Also, consider this other example: >> >> get ''/products.json'' => ''products#list'', format: :json >> >> This will also respond to ''/products.json.html'' for example, and I don''t think it should answer to anything else but ''/products.json''. Also, the content-type declared in the routes wouldn''t be respected. > That''s because :format is still being appended to the route - the mapper doesn''t interpret the path you specify other than to look for segment keys and optional segments. If you''re after a format-less path that only serves JSON then use this: > > get ''/products'' => ''products#list'', :format => false, :defaults => { :format => ''json'' } > > or if you want the extension then use this: > > get ''/products.json'' => ''products#list'', :format => false, :defaults => { :format => ''json'' } > > The only way to turn off the :format segment being added is to either insert it yourself or pass :format => false. > >> Maybe I''m the only one considering the current behavior an issue, but I''d like to ask you to consider some changes in the routing rules with regards to format to make them less loose. > If you can come up with a route that isn''t supported or doesn''t appear to be supported please open a new issue. You don''t need to ping me on it - I normally take a look at all the routing issues that get created. > > > Andrew White > > [1]: https://rails.lighthouseapp.com/projects/8994/tickets/1215 > [2]: https://rails.lighthouseapp.com/projects/8994/tickets/1359 > [3]: https://github.com/rails/rails/commit/fef6c32afe2276dffa0347e25808a86e7a101af1 >-- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On 13 May 2012, at 14:16, Rodrigo Rosenfeld Rosas wrote:> I was not saying that it wasn''t possible to write the proper routes, just that they don''t seem very consistent.I''m not getting why you think they''re inconsistent - there''s always a optional format parameter unless you tell it not to add one or you make it non-optional in the route itself.> I would guess that most applications won''t be API ones, so many of them won''t opt for REST and responds_to.There are plenty of non-API apps that utilise REST and responds_to> For example, all applications I''ve been working with for the past decade will only need to respond to a single format per action. > > That means that if I wanted strict rules to my routes I''d have to write them as: > > get ''/anything'', format: false > > or > > get ''/anything.:format'' => ''anything#index'', constraints: {format: /json/}, defaults: {format: :json} > > I could use a block for this common option except that not all of my actions will return a JSON.Just use a scope - :format works here as well: scope :format => false do resources :products end You could even use a nested scope to wrap your actions that return JSON to add the constraints/defaults.> Today is Mother''s day in Brazil and I''m going to leave in a few minutes to have lunch with my mom in a near city, but I guess I''ve already found some bugs.<example code removed>> I''ve made several experiments that yield to strange results when it comes to content-type. > > Basically, I''m getting ''text/html'' instead of ''application/json''. This happens even if I pass content_type: ''application/json'' explicitly.The test/expire route needs to be set up in the same way - this works for me: scope :format => false do defaults :format => ''json'' do get ''test/index'', :as => :index get ''test/expire'', :as => :expire end end> Another bug is that expire_action doesn''t seem to work correctly either. For example, if you use the commented route the output will be: > > Expire fragment views/localhost:3000/assets?controller=testThis is the route you get if you have the asset pipeline enabled and none of your applications route''s matches.> It doesn''t matter if I use a named route or "expire_action action: :index" or "expire_action action: :index, format: :json" if I remember correctly.There''s a slight wrinkle here - read and write action caches always adds the :format so your cache file ends with .json but expire_action is essentially a wrapper around url_for so gives a path without the :format, making the expiry fail. I''ll look at whether this needs addressing. In the meantime it''s probably best to use formatted urls - these routes work fine: constraints :format => ''json'' do get ''test/index'', :as => :index get ''test/expire'', :as => :expire end You''ll need to pass the format to the expire_action as it doesn''t infer it from the current request - otherwise you could only expire JSON caches from JSON requests. Hope you had a good Sunday. Andrew White -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Rodrigo Rosenfeld Rosas
2012-May-14 02:08 UTC
Re: What is the point of using :format in routes?
Hi Andrew, I''m finally back. And thanks, my day was really nice :) I hope you have enjoyed yours as well. Please take a look at the comments inline. Em 13-05-2012 12:23, Andrew White escreveu:> On 13 May 2012, at 14:16, Rodrigo Rosenfeld Rosas wrote: > >> I was not saying that it wasn''t possible to write the proper routes, just that they don''t seem very consistent. > I''m not getting why you think they''re inconsistent - there''s always a optional format parameter unless you tell it not to add one or you make it non-optional in the route itself.Here is the main inconsistent issue I was referring to: routes.rb: get ''test/cached'', format: false get ''test/uncached'', format: false class TestController < ApplicationController caches_action :cached def cached render json: 1 end def uncached render json: 1 end end curl -I http://localhost:3000/test/uncached 2>/dev/null | grep Content-Type Content-Type: application/json; charset=utf-8 curl -I http://localhost:3000/test/cached 2>/dev/null | grep Content-Type Content-Type: text/html; charset=utf-8 I would expect "render json: xxx" to render an ''application/json'' content-type independent from the :format state. This is true for uncached actions but cached actions seems to only take into account the value of :format. Because of that, we get a very verbose route for getting it to properly work with caches_action when we only want to support the :json format: get ''test/cached'', format: false, defaults: {format: :json} That is the other inconsistency I was talking about. I''d prefer to just read the format option as the actual format instead of the default one like in: get ''test/cached'', format: :json # that should be the same as the previous example for brevity but the above rule would allow ''/test/cached.html'' for instance. I find this an inconsistency with regards to API expected behavior, although it is consistent with the current implementation. Also, there is another problem. With this last rule, I''d need to append something like ", as: :test_cached" to be able to "expire_action :test_cached" since "expire_action action: :cached" won''t work and the full line would be too lengthy: "expire_action action: :cached, format: :json". I mean, these rules were intended to simplify rules writing of REST APIs more than actual applications that don''t need responds_to. Also, I don''t understand why we have "caches_page" and "caches_action" but "expire_page" and "expire_action". For consistency sake those should be named either caches/expires or cache/expire...>> I would guess that most applications won''t be API ones, so many of them won''t opt for REST and responds_to. > There are plenty of non-API apps that utilise REST and responds_toCould you please give me an actual example where you find responds_to to be useful for applications that don''t interact with external applications? For example, if I have a ''/products'' and ''/products.json'' where the former will render an HTML that will fill the page with the products requested by the latter, I''d probably use separate actions as they do completely different things: get ''/products'' => ''products#index'', format: false get ''/products.json'' => ''products#index_json'', format: false, defaults: {format: :json} This also makes tests easier to be written and understood.>> For example, all applications I''ve been working with for the past decade will only need to respond to a single format per action. >> >> That means that if I wanted strict rules to my routes I''d have to write them as: >> >> get ''/anything'', format: false >> >> or >> >> get ''/anything.:format'' => ''anything#index'', constraints: {format: /json/}, defaults: {format: :json} >> >> I could use a block for this common option except that not all of my actions will return a JSON. > Just use a scope - :format works here as well: > > scope :format => false do > resources :products > endThat is exactly the block-based approach I was referring to. But the problem remains. I''d still need to write a lot for my JSON routes (defaults: {format: :json}).> You could even use a nested scope to wrap your actions that return JSON to add the constraints/defaults.That would force me to write the rules in some unnatural ways. I like to group the actions not by the content-type of the response but by the involved resource instead.> ... > There''s a slight wrinkle here - read and write action caches always adds the :format so your cache file ends with .json but expire_action is essentially a wrapper around url_for so gives a path without the :format, making the expiry fail. I''ll look at whether this needs addressing.Thanks for taking a look into this. :)> In the meantime it''s probably best to use formatted urls - these routes work fine: > > constraints :format => ''json'' do > get ''test/index'', :as => :index > get ''test/expire'', :as => :expire > endIf I understand correctly, this would require me to request ''/test/expire.json'', right? But usually I''d write the expire action with "head :ok", returning a ''text/html'' or ''text/plain'' instead of ''application/json''.> You''ll need to pass the format to the expire_action as it doesn''t infer it from the current request - otherwise you could only expire JSON caches from JSON requests.Or I could just pass the named routes to "expire_action", right? The named route would already contain the format information so ''expire_action'' method wouldn''t need to guess. Please, don''t get me wrong. I''m not against making API applications easy to write, nor I''m against responds_to. I just think that the commonest case may currently need too verbose routing rules. That shortcoming could be minimized if we assumed the "format: :json" to restrict the only possible format to ''json'' instead of using it as a default. Please consider this change to routing DSL for Rails 4. Best, Rodrigo. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On 14 May 2012, at 03:08, Rodrigo Rosenfeld Rosas wrote:> I would expect "render json: xxx" to render an ''application/json'' content-type independent from the :format state. This is true for uncached actions but cached actions seems to only take into account the value of :format.That''s because the cache action tries to infer the format from the request but can''t because there''s no extension or default value for :format. It sets the content_type to the default HTML and when you call render :json it only sets the content_type if it is not already been set. I''m not sure what you expect Rails to do here - how does it know to fetch the JSON cached version if you''re relying on the content_type being set by the render? The whole point of action caching is to avoid calling render.> Because of that, we get a very verbose route for getting it to properly work with caches_action when we only want to support the :json format: > > get ''test/cached'', format: false, defaults: {format: :json} > > That is the other inconsistency I was talking about. I''d prefer to just read the format option as the actual format instead of the default one like in: > > get ''test/cached'', format: :json # that should be the same as the previous example for brevity > > but the above rule would allow ''/test/cached.html'' for instance. I find this an inconsistency with regards to API expected behavior, although it is consistent with the current implementation.As I said non-regexp keys are assumed to be defaults - use a regexp to make it only accept JSON: get ''test/caches'', :format => /json/ You''d need to have the format in the url though.> Also, there is another problem. With this last rule, I''d need to append something like ", as: :test_cached" to be able to "expire_action :test_cached" since "expire_action action: :cached" won''t work and the full line would be too lengthy: "expire_action action: :cached, format: :json".As I said you may want to expire all or some cached action formats - not just the format of the current request and you need to tell it which format somehow.> Also, I don''t understand why we have "caches_page" and "caches_action" but "expire_page" and "expire_action". For consistency sake those should be named either caches/expires or cache/expire...It makes sense when you look at the tense - caches_action is saying that the controller will cache the action (i.e in the future) whereas expire_action is actually doing it now.> Could you please give me an actual example where you find responds_to to be useful for applications that don''t interact with external applications?Pagination of results - use the JSON request to provide just the results so that they can be rendered on the client.> For example, if I have a ''/products'' and ''/products.json'' where the former will render an HTML that will fill the page with the products requested by the latter, I''d probably use separate actions as they do completely different things: > > get ''/products'' => ''products#index'', format: false > get ''/products.json'' => ''products#index_json'', format: false, defaults: {format: :json}class ProductsController responds_to :html, :json def index @products = Products.paginate :page => params[:page] responds_with(@products) end end Then use something like Jbuilder or ActiveModel::Serializers to render the JSON response and your standard ERb/Haml for the HTML response.> If I understand correctly, this would require me to request ''/test/expire.json'', right? But usually I''d write the expire action with "head :ok", returning a ''text/html'' or ''text/plain'' instead of ''application/json''.Sorry, I thought you wanted the expire action to return application/json.>> You''ll need to pass the format to the expire_action as it doesn''t infer it from the current request - otherwise you could only expire JSON caches from JSON requests. > > Or I could just pass the named routes to "expire_action", right? The named route would already contain the format information so ''expire_action'' method wouldn''t need to guess.Named url helpers still call url_for internally so you''d still end up with a path without a format extension.> That shortcoming could be minimized if we assumed the "format: :json" to restrict the only possible format to ''json'' instead of using it as a default. > > Please consider this change to routing DSL for Rails 4.We could certainly consider whether :format => :json or :format => [:html, :json] would restrict it to those formats (i.e. setup :constraints and :defaults automatically), however that would have to be with :format in the path as you can''t turn it off without :format => false. Please open an issue to consider this further. Andrew White -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Rodrigo Rosenfeld Rosas
2012-May-14 13:06 UTC
Re: What is the point of using :format in routes?
Em 14-05-2012 02:11, Andrew White escreveu:> On 14 May 2012, at 03:08, Rodrigo Rosenfeld Rosas wrote: > >> I would expect "render json: xxx" to render an ''application/json'' content-type independent from the :format state. This is true for uncached actions but cached actions seems to only take into account the value of :format. > That''s because the cache action tries to infer the format from the request but can''t because there''s no extension or default value for :format. It sets the content_type to the default HTML and when you call render :json it only sets the content_type if it is not already been set. > > I''m not sure what you expect Rails to do here - how does it know to fetch the JSON cached version if you''re relying on the content_type being set by the render? The whole point of action caching is to avoid calling render.I was expecting the relevant headers to be cached alongside with the body content of the response.>> Because of that, we get a very verbose route for getting it to properly work with caches_action when we only want to support the :json format: >> >> get ''test/cached'', format: false, defaults: {format: :json} >> >> That is the other inconsistency I was talking about. I''d prefer to just read the format option as the actual format instead of the default one like in: >> >> get ''test/cached'', format: :json # that should be the same as the previous example for brevity >> >> but the above rule would allow ''/test/cached.html'' for instance. I find this an inconsistency with regards to API expected behavior, although it is consistent with the current implementation. > As I said non-regexp keys are assumed to be defaults - use a regexp to make it only accept JSON: > > get ''test/caches'', :format => /json/ > > You''d need to have the format in the url though.Well, this is great! I don''t remember that you said that by using a regular expression it would be an enforcement instead of a default. And I don''t mind that I have to use ''/test/caches.json''. At least I know that if it is called in any other way the application will respond with 500 or any other error code. Thank you very much!>> Also, there is another problem. With this last rule, I''d need to append something like ", as: :test_cached" to be able to "expire_action :test_cached" since "expire_action action: :cached" won''t work and the full line would be too lengthy: "expire_action action: :cached, format: :json". > As I said you may want to expire all or some cached action formats - not just the format of the current request and you need to tell it which format somehow.Ah, okay, I guess I got it now.>> Also, I don''t understand why we have "caches_page" and "caches_action" but "expire_page" and "expire_action". For consistency sake those should be named either caches/expires or cache/expire... > It makes sense when you look at the tense - caches_action is saying that the controller will cache the action (i.e in the future) whereas expire_action is actually doing it now.So, shouldn''t it be the opposite? Controller will *cache* and it currently *expires*? :)>> Could you please give me an actual example where you find responds_to to be useful for applications that don''t interact with external applications? > Pagination of results - use the JSON request to provide just the results so that they can be rendered on the client. > >> For example, if I have a ''/products'' and ''/products.json'' where the former will render an HTML that will fill the page with the products requested by the latter, I''d probably use separate actions as they do completely different things: >> >> get ''/products'' => ''products#index'', format: false >> get ''/products.json'' => ''products#index_json'', format: false, defaults: {format: :json} > class ProductsController > responds_to :html, :json > > def index > @products = Products.paginate :page => params[:page] > responds_with(@products) > end > end > > Then use something like Jbuilder or ActiveModel::Serializers to render the JSON response and your standard ERb/Haml for the HTML response.Okay, makes sense.>> If I understand correctly, this would require me to request ''/test/expire.json'', right? But usually I''d write the expire action with "head :ok", returning a ''text/html'' or ''text/plain'' instead of ''application/json''. > Sorry, I thought you wanted the expire action to return application/json. > >>> You''ll need to pass the format to the expire_action as it doesn''t infer it from the current request - otherwise you could only expire JSON caches from JSON requests. >> Or I could just pass the named routes to "expire_action", right? The named route would already contain the format information so ''expire_action'' method wouldn''t need to guess. > Named url helpers still call url_for internally so you''d still end up with a path without a format extension.Yeah, I had just figured it out from a previous comment of yours in this message :)>> That shortcoming could be minimized if we assumed the "format: :json" to restrict the only possible format to ''json'' instead of using it as a default. >> >> Please consider this change to routing DSL for Rails 4. > We could certainly consider whether :format => :json or :format => [:html, :json] would restrict it to those formats (i.e. setup :constraints and :defaults automatically), however that would have to be with :format in the path as you can''t turn it off without :format => false. Please open an issue to consider this further.Thanks! I''ll try to figure it out an API that could allow an unformatted URL before opening the issue. If I can''t come to any suggestion I''ll open the issue with the API you''ve just mentioned. Wouldn''t it be possible that "get ''/products'', format: :json" responded to both ''/products'' and ''/products.json'' when a single format is given instead of an array? Of course I mean that action caching should also use the same correct content-type in the response for both URLs. Cheers, Rodrigo. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On 14 May 2012, at 14:06, Rodrigo Rosenfeld Rosas wrote:>> I''m not sure what you expect Rails to do here - how does it know to fetch the JSON cached version if you''re relying on the content_type being set by the render? The whole point of action caching is to avoid calling render. > > I was expecting the relevant headers to be cached alongside with the body content of the response.That''s a longstanding issue - you may want to look at using HTTP caching instead which does cache headers. Rails 3.2 is pre-configured with Rack::Cache - try removing the action caching code from your controller and look at the API docs for stale?, fresh_when? and expires_in. See also the Rails Guide on caching: http://guides.rubyonrails.org/caching_with_rails.html#conditional-get-support>> We could certainly consider whether :format => :json or :format => [:html, :json] would restrict it to those formats (i.e. setup :constraints and :defaults automatically), however that would have to be with :format in the path as you can''t turn it off without :format => false. Please open an issue to consider this further. > > Thanks! I''ll try to figure it out an API that could allow an unformatted URL before opening the issue. If I can''t come to any suggestion I''ll open the issue with the API you''ve just mentioned. > > Wouldn''t it be possible that "get ''/products'', format: :json" responded to both ''/products'' and ''/products.json'' when a single format is given instead of an array? Of course I mean that action caching should also use the same correct content-type in the response for both URLs.You can do that now using an empty option in your regexp: get ''/products'' => ''products#index'', :format => /|json/ This will match /products and /products.json Andrew White -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Rodrigo Rosenfeld Rosas
2012-May-15 03:02 UTC
Re: What is the point of using :format in routes?
Em 14-05-2012 18:22, Andrew White escreveu:> On 14 May 2012, at 14:06, Rodrigo Rosenfeld Rosas wrote: > >>> I''m not sure what you expect Rails to do here - how does it know to fetch the JSON cached version if you''re relying on the content_type being set by the render? The whole point of action caching is to avoid calling render. >> I was expecting the relevant headers to be cached alongside with the body content of the response. > That''s a longstanding issue - you may want to look at using HTTP caching instead which does cache headers. Rails 3.2 is pre-configured with Rack::Cache - try removing the action caching code from your controller and look at the API docs for stale?, fresh_when? and expires_in. See also the Rails Guide on caching: > > http://guides.rubyonrails.org/caching_with_rails.html#conditional-get-supportThis is different and less useful for my application. That is because I''d like to fill the cache only in the first access no matter which client did request it first. Until that action is expired, only the first client should notice the delay in the first access. All other clients should benefit from this first access as well. Would you mind explain why it is hard to store the headers alongside with the body in the caches_page mechanism?>>> We could certainly consider whether :format => :json or :format => [:html, :json] would restrict it to those formats (i.e. setup :constraints and :defaults automatically), however that would have to be with :format in the path as you can''t turn it off without :format => false. Please open an issue to consider this further. >> Thanks! I''ll try to figure it out an API that could allow an unformatted URL before opening the issue. If I can''t come to any suggestion I''ll open the issue with the API you''ve just mentioned. >> >> Wouldn''t it be possible that "get ''/products'', format: :json" responded to both ''/products'' and ''/products.json'' when a single format is given instead of an array? Of course I mean that action caching should also use the same correct content-type in the response for both URLs. > You can do that now using an empty option in your regexp: > > get ''/products'' => ''products#index'', :format => /|json/ > > This will match /products and /products.jsonYeah, but then if I accessed /products in a cached action the returned content-type would be text/html, right? Thanks for your support, Rodrigo. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On 15 May 2012, at 04:02, Rodrigo Rosenfeld Rosas wrote:> This is different and less useful for my application. That is because I''d like to fill the cache only in the first access no matter which client did request it first. Until that action is expired, only the first client should notice the delay in the first access. All other clients should benefit from this first access as well.Use Cache-Control: public to share cached resources between clients - obviously things like cookies and sessions can''t be shared.> Would you mind explain why it is hard to store the headers alongside with the body in the caches_page mechanism?The caches_page stores static files that Apache/nginx serve up - Rails doesn''t even get hit if it''s set up properly. If you meant caches_action then that''s just the way things have been historically as it''s just a wrapper around fragment caching which obviously doesn''t need headers saving. Feel free to investigate the feasibility of adding header support to action caching but be aware that controller filters will still run for a cached action (action caching is an around filter itself) so that things like authentication still work so any changes need to make sure that you don''t serve cached private data.>> You can do that now using an empty option in your regexp: >> >> get ''/products'' => ''products#index'', :format => /|json/ >> >> This will match /products and /products.json > > Yeah, but then if I accessed /products in a cached action the returned content-type would be text/html, right?You could exploit the fact that action caching still runs controller filters to set the content_type: TestApp::Application.routes.draw do get ''test/index'', :as => :index, :format => /|json/ get ''test/expire'', :as => :expire end class TestController < ApplicationController before_filter :set_content_type, :only => :index caches_action :index def index render :json => params end def expire expire_action :action => :index expire_action :action => :index, :format => :json head :ok end def set_content_type self.content_type = Mime::JSON end end This serves cached /test/index and /test/index.json as application/json and /test/expire as text/html Andrew White -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Rodrigo Rosenfeld Rosas
2012-May-15 14:13 UTC
Re: What is the point of using :format in routes?
Em 15-05-2012 02:01, Andrew White escreveu:> On 15 May 2012, at 04:02, Rodrigo Rosenfeld Rosas wrote: > >> This is different and less useful for my application. That is because I''d like to fill the cache only in the first access no matter which client did request it first. Until that action is expired, only the first client should notice the delay in the first access. All other clients should benefit from this first access as well. > Use Cache-Control: public to share cached resources between clients - obviously things like cookies and sessions can''t be shared.This would be equivalent to caches_page, while I need the authorization filters to run.>> Would you mind explain why it is hard to store the headers alongside with the body in the caches_page mechanism? > The caches_page stores static files that Apache/nginx serve up - Rails doesn''t even get hit if it''s set up properly. If you meant caches_action then that''s just the way things have been historically as it''s just a wrapper around fragment caching which obviously doesn''t need headers saving. Feel free to investigate the feasibility of adding header support to action caching but be aware that controller filters will still run for a cached action (action caching is an around filter itself) so that things like authentication still work so any changes need to make sure that you don''t serve cached private data.Thanks, I''ll try to find some free time to take a look at how caches_action works and will try to issue a pull request if that won''t demand me too much time.>>> You can do that now using an empty option in your regexp: >>> >>> get ''/products'' => ''products#index'', :format => /|json/ >>> >>> This will match /products and /products.json >> Yeah, but then if I accessed /products in a cached action the returned content-type would be text/html, right? > You could exploit the fact that action caching still runs controller filters to set the content_type: > > TestApp::Application.routes.draw do > get ''test/index'', :as => :index, :format => /|json/ > get ''test/expire'', :as => :expire > end > > class TestController< ApplicationController > before_filter :set_content_type, :only => :index > caches_action :index > > def index > render :json => params > end > > def expire > expire_action :action => :index > expire_action :action => :index, :format => :json > head :ok > end > > def set_content_type > self.content_type = Mime::JSON > end > end > > This serves cached /test/index and /test/index.json as application/json and /test/expire as text/htmlPlease, don''t get me wrong. I wasn''t trying to say that Rails won''t allow me to do what I want to. I was just asking if we could try making such common setup easier to implement in Rails and avoid duplication. If we accepted "get ''/test/index'', format: :json, as: :test_index" to just mean that, we''d only need "expire_action :test_index" which reads much better and is less error prone. Also we shouldn''t need any filters as the MIME could be extracted from this kind of routing rule. So your above example could be written as: TestApp::Application.routes.draw do get ''test/index'', :as => :index, :format => :json get ''test/expire'', :as => :expire end class TestController< ApplicationController caches_action :index def index render :json => params end def expire expire_action :index head :ok end end I''m asking if it would be possible to support this in Rails 4, for example. I''m really sorry if I''m bothering you with all those requests. This really isn''t my intention and I do value a lot all your feedback. I''m just trying to see if we could make Rails easier for non multi-format RESTful applications as well. Thank you very much, Rodrigo. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
On 15 May 2012, at 15:13, Rodrigo Rosenfeld Rosas wrote:> If we accepted "get ''/test/index'', format: :json, as: :test_index" to just mean that, we''d only need "expire_action :test_index" which reads much better and is less error prone. Also we shouldn''t need any filters as the MIME could be extracted from this kind of routing rule.If I add the functionality that :format at the route level is copied to :constraints and :defaults automatically and fix issue #6304 then that should cover your suggestion - still not sure about caching headers though.> I''m really sorry if I''m bothering you with all those requests. This really isn''t my intention and I do value a lot all your feedback.No, this is the correct place to discuss these things rather than polluting the issues tracker. Andrew White -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.
Rodrigo Rosenfeld Rosas
2012-Jun-01 13:16 UTC
Re: What is the point of using :format in routes?
Em 14-05-2012 02:11, Andrew White escreveu:> ... >> That shortcoming could be minimized if we assumed the "format: :json" to restrict the only possible format to ''json'' instead of using it as a default. >> >> Please consider this change to routing DSL for Rails 4. > We could certainly consider whether :format => :json or :format => [:html, :json] would restrict it to those formats (i.e. setup :constraints and :defaults automatically), however that would have to be with :format in the path as you can''t turn it off without :format => false. Please open an issue to consider this further.I''ve noticed an annoying issue while trying the js-routes gem. I have some routes like this: scope format: true, constraints: {format: /json/} do ... end If we change the meaning of "format: :json" I''d be able to just call "some_path", instead of "some_path(''json'')". I''ve even tried to add a defaults arguments ({defaults: :json}), but it seems the router is not smart enough for not requiring the format to be specified. So, if I opt for using js-routes I''d need to write my routes as "sections_path(''json'')", while I''d prefer to just call "sections_path()". Can you see how useful it could be to change the meaning of the format parameter when a non-boolean value is used? Thanks for considering this change. Cheers, Rodrigo. -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To post to this group, send email to rubyonrails-core@googlegroups.com. To unsubscribe from this group, send email to rubyonrails-core+unsubscribe@googlegroups.com. For more options, visit this group at http://groups.google.com/group/rubyonrails-core?hl=en.