[Routing] Array constraints for path segments

Hi,

I would like to know if anyone see any disadvantages in allowing array in segments constraints in addition to regexps. For example,

Photo::TYPES = %w(small big)
get 'photos/:type', to: 'photos#index', constraints: { type: Photo::TYPES }

my use case is that I usually have a model with a constant of allowed values and I would like to use it directly instead of the alternative below.

get 'photos/:type', to: 'photos#index', constraints: { type: Regexp.new(Photo::TYPES.join("|")) }

If you think this can be accepted into Rails I will start to work on it.

Thanks.

--

Filipe Giusti

It seems like this doesn’t belong in the routing layer.

Take the default routing for the SHOW action, for example:

get 'photos/:id', to: 'photos#show

This assigns the input to the parameter :id whether or not the id actually exists in the database. When the controller does Photo.find in the show action, it will throw an ActiveRecord::RecordNotFound exception and return a 404 error.

In your case, this would mean checking that :type is valid in the controller (or better, model) and raising an exception if it’s invalid. Then you can return a 404 or other error from the controller.

1 Like

This is already in Rails 4 - it was added in this commit:

https://github.com/rails/rails/commit/90d2802b71a6e89aedfe40564a37bd35f777e541

Andrew White https://twitter.com/pixeltrix

However in your case:

  Add support for other types of routing constraints · rails/rails@90d2802 · GitHub

won't help since the path requires regexp constraints - it only helps with request based constraints like :subdomain or :port - which addresses Grayson's concern as it's only for accepting an array of valid values for things like host, subdomain, port etc.

Andrew White https://twitter.com/pixeltrix

https://gist.github.com/smartjeck/5895283

Thanks Grayson, what you said really makes sense. No point in patching Rails for that.

Andy, thanks for pointing that out, the Rails decision to only accept request constraints makes a lot of sense.

There is still one advantage on allowing it to be in the route constraints, that you won’t be able to generate invalid routes using the helpers.

I think the biggest problem for this is that it might encourage abuse of the feature. While this might make sense for a few items (like small, medium and large photo; or all, pending and completed todo items - in these case I think it makes more sense to define them as routes than handling them in the controller), I think it would be a very bad idea to pass it an array of country codes as an example.

What I would like to do in that case though, would be to do something like:

[:all, :pending, :completed].each do |filter|

get “items/#{filter}”, to: “items#index”, as: “#{filter}_items”

​end

Of course in here there needs to be a way for the router to communicate type of filter to the controller. So what I would like to have is actually this:

[:all, :pending, :completed].each do |filter|

get “items/#{filter}”, to: “items#index”, as: “#{filter}_items”, extra_params: { filter: filter }

​end

​This would produce more or less the same result as what the OP wanted, but generates nicer URL helpers (IMO), much more explicit and provides a more general mechanism that can be used in other cases.

(I thought about using defaults, but that wouldn’t work because it can be override in the query string. These also seem to be a requirements hash for resource routes which might do this, but it doesn’t look like it exists for non-resource routes.)

However, these extra params would be used for incoming routes but not for URL generation which you could argue is a weird semantic.

The alternative would be to pass these to different (non-restful) actions on the controller and within the controller they could pass it onto a common private method.

Godfrey

Godfrey, I agree with you that allowing it would encourage abuse. However I’m not sure if we should restrain what Rails can provide because people might abuse it. I don’t know what’s the philosophy of Rails regarding that.

Anyway, I don’t fell that your suggestion is a weird semantic, so I guess both that or the regexp creation for constraints that I ended up using are good alternatives.

If no one else has an argument pro accepting arrays, I don’t think it’s needed right now.