10

I started using Postgres UUID type for all my models' id fields. Works great and is supported (for the most part) in Rails 4:

create_table :users, id: :uuid do |t|
  # ...
end

The problem is that Postgres will raise an error if you attempt to find a row where id is X, but X is not a properly formatted UUID string.

> User.find "3ac093e2-3a5e-4744-b49f-117b032adc6c"
ActiveRecord::RecordNotFound # good, will cause a 404
> User.find "foobar"
PG::InvalidTextRepresentation: ERROR # bad, will cause a 500

So if my user is on a page where a UUID is in the URL, and they then try to change the UUID, they'll get a 500 error instead of 404. Or perhaps they get a link to an object that no longer exists.

How can I go about avoiding this scenario in a DRY way? I can't just rescue the PG::InvalidTextRepresentation and render 404 because other things can cause this error as well.

UPDATE

I think that a regex on the format of the ID param is clean, and it raises a 404 if it doesn't match:

resources :users, id: /uuid-regex-here/

But I still have the problem of staying DRY; I don't want to put this on every single resource in my routes. I can declare multiple resources in one statement, but only if don't other options to it like member actions. So perhaps a better question is: Is there a way to set the id regex for all routes?

Community
  • 1
  • 1
tybro0103
  • 48,327
  • 33
  • 144
  • 170
  • Why are you passing foobar, though? Shouldn't your route catch that before it even lands in the hands of your model? – Denis de Bernardy Jan 24 '14 at 22:05
  • @Denis I suppose I could put a constraint on the id param ensuring it matches a UUID regex. But I would have to do that for every single resource in my routes... or did you have something else in mind? – tybro0103 Jan 24 '14 at 22:19
  • Perhaps a `before_filter` for your controllers would work. – mu is too short Jan 24 '14 at 22:27
  • @tybro0103: Speaking personally, if I forgot to make my own routes validate that the id is of the right format, or if some random call somewhere sent an id in the wrong format, I'd actually want them to cough an error. I'm not familiar enough with Rails to say exactly how you could mass-auto-validate them, but I'm guessing mu is onto something with his suggestion. – Denis de Bernardy Jan 24 '14 at 22:30
  • @Denis: The underlying problem is that the Rails guys are too lazy to put validation in the right place, they're assuming that `Model.find` will raise a RecordNotFound if the record is not found for any reason (including a type error such as `M.find('pancakes')` when the PK is an integer). In the Rails model, adding constraints to the routes would probably be the Right Thing, a filter is a quick hack. Rails is rife with unstated assumptions (try adding a route that contains an email address and see what happens) and it goes sideways when you stray from the conventional path. – mu is too short Jan 24 '14 at 22:43
  • @muistooshort: isn't there a means to add constraints, though? http://stackoverflow.com/questions/5921771/how-do-i-validate-restful-parameters-in-rails-at-the-routing-level – Denis de Bernardy Jan 24 '14 at 23:26
  • @Denis: Yes, you can add constraints to the routes but if you have a lot of routes... – mu is too short Jan 24 '14 at 23:28
  • @muistooshort: there's no way to access the routes and their constraints once they're defined, so as to process an entire list? – Denis de Bernardy Jan 24 '14 at 23:31
  • @Denis: Not in any sane way that I know about. "Convention over configuration" has its downsides and the downsides tend to be very steep and dump into a moat filled with fish hooks and rabid electric eels. The documentation for the routing stuff is, um, rather thin as well. – mu is too short Jan 24 '14 at 23:40
  • @muistooshort I don't concur about your 'underlying' problem. This isn't a string vs integer thing... '3ac093e2-44...' and 'foobar' are both strings. I understand why PG raises the error, but I think it's reasonable to have User.find('foobar') raise NotFound because the id is a string. Still, I understand what's going on, I'm just not sure cleanly fix this issue. The before_action is DRY, but a little hackish. I think a regex in the routes would be clean, but not sure how to make it dry. – tybro0103 Jan 25 '14 at 02:12
  • @Denis these are dynamic routes, where the id (or uuid in this case) is part of the url (not a query string param). So /posts/1, /posts/2, /posts/3, are all mapped to the same action and the id part is treated as a parameter. I could put a regex to ensure the id part looks like a uuid, I don't think there's a way to do that globally for all id params. – tybro0103 Jan 25 '14 at 02:13
  • AR assumes that `M.find(id)` will raise an `ActiveRecord::RecordNotFound` if `id` doesn't identify a record for any reason and that assumption is faulty. The underlying problem is that you're straying from The One True Rails Path and now you're paying for it. I suppose you could attempt to monkey patch `find` but that's nasty and won't help you with anything doesn't call `find`. – mu is too short Jan 25 '14 at 02:17
  • @muistooshort yeah I suppose. I think it's that the UUID support is new and doesn't seem fully thought-out. I think they should simply rescue from the pg type error inside of `find`. – tybro0103 Jan 25 '14 at 02:19
  • But `find` is not the only thing that can trigger that error. Monkey patch `find` if you think that's good enough. – mu is too short Jan 25 '14 at 02:22
  • @muistooshort right, which is why I don't think that's good enough :( – tybro0103 Jan 25 '14 at 02:28

2 Answers2

7

You can add a routing constraint to multiple routes at a time via constraints() do ... end.

I ended up doing this and setting a global constraint on all :id params to match it to a UUID regexp:

MyApp::Application.routes.draw do
  constraints(id: /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/i) do

    # my routes here

  end
end

This way, /posts/123 or /posts/foobar no longer match /posts/:id and 404 before ever invoking the controller action, thus avoiding the PG type error.

All of my models will use UUID for their IDs so this is clean and DRY. If I had some models with integer IDs as well, it'd be a little less clean.

tybro0103
  • 48,327
  • 33
  • 144
  • 170
  • If you're using the UUID gem, you can use their validate method instead of rolling your own https://github.com/assaf/uuid/blob/master/lib/uuid.rb#L199 – Brian Hahn Feb 04 '14 at 20:03
  • @BrianHahn Good to know. Though I'm not really rolling my own... it's just a regexp. – tybro0103 Feb 04 '14 at 20:47
  • I meant it'd be rolling your own in the context of already having the UUID gem. Your regexp seems fine when the UUID gem isn't a dependency in your project! – Brian Hahn Feb 05 '14 at 23:44
2

If you don't want to add constraints to all the routes to catch invalid UUIDs then you could kludge in a before_filter, something like this:

before_filter do
  if(params.has_key?(:id))
    uuid = params[:id].strip.downcase.gsub('-', '').gsub(/\A\{?(\h{32})\}?\z/, '\1')
    raise ActiveRecord::RecordNotFound if(uuid.blank?)
  end
end

Note that UUIDs can come in various forms (see the fine manual) so it is best to normalize them before validating them or do both normalization and validation at the same time.

You could put that into your ApplicationController if you know that all your :id parameters are supposed to be UUIDs or put the logic in an ApplicationController method and before_filter :make_sure_id_is_a_uuid in the controllers that need it.

mu is too short
  • 426,620
  • 70
  • 833
  • 800
  • This isn't bad. I'll end up going with this if I don't find a way to add the regex to the routes only once. – tybro0103 Jan 25 '14 at 02:31
  • Another monkey patching possibility would be the type conversion stuff inside the AR PostgreSQL driver, there's a big `case` statement somewhere in there. – mu is too short Jan 25 '14 at 18:09