Auto-generate stable fixture UUIDs on PostgreSQL #12016

Merged
merged 1 commit into from Apr 10, 2014
@roderickvd

Fixes #11524. This PR builds on that one by @ruckc:

  • Added tests.
  • Supports polymorphic associations by inspecting the polymorphic foreign key type.
@roderickvd

Build should be restarted because of minitest problems on travis. Test suite passes fine here.

@tenderlove
Ruby on Rails member

I'd rather not optionally depend on uuid tools. Why don't we monkey patch SecureRandom to add a stable uuid method, and I can work on pushing that method upstream (to Ruby):

>> require 'digest/md5'
=> true
>> def uuid key
>>   bytes = Digest::MD5.digest(key).unpack 'NnnnnN'
>>   bytes[2] = (bytes[2] & 0x0FFF) | 0x4000
>>   bytes[3] = (bytes[3] & 0x3FFF) | 0x8000
>>   "%08x-%04x-%04x-%04x-%04x%08x" % bytes
>> end
=> :uuid
>> uuid "hello world"
=> "5eb63bbb-e01e-4ed0-93cb-22bb8f5acdc3"
>> uuid "hello world"
=> "5eb63bbb-e01e-4ed0-93cb-22bb8f5acdc3"
>> uuid "hello world!"
=> "fc3ff98e-8c6a-4d30-87d5-15c0473f8677"
>> 

This code was mostly stolen from here.

@roderickvd

Aye, fewer dependencies. Got something cooking that I'll push to my branch shortly.

What about the API -- uuid_v3(namespace, name) for MD5 and uuid_v5(namespace, name) for SHA-1? The stock uuid could in turn be aliased as uuid_v4. Note that we can't do without namespaces if we want to stay RFC 4122 compliant.

I thought about doing something like uuid(version, namespace, name) but that doesn't read well without introducing symbols.

@roderickvd

I've removed the dependency in 615b0d9 and rebased in a7fe3f7. This tags along nicely with the ReflectionProxy that @tenderlove introduced. Let me know if there's anything else.

@robin850 robin850 and 2 others commented on an outdated diff Sep 28, 2013
...vesupport/lib/active_support/core_ext/securerandom.rb
+ UUID_DNS_NAMESPACE = "k\xA7\xB8\x10\x9D\xAD\x11\xD1\x80\xB4\x00\xC0O\xD40\xC8" #:nodoc:
+ UUID_URL_NAMESPACE = "k\xA7\xB8\x11\x9D\xAD\x11\xD1\x80\xB4\x00\xC0O\xD40\xC8" #:nodoc:
+ UUID_OID_NAMESPACE = "k\xA7\xB8\x12\x9D\xAD\x11\xD1\x80\xB4\x00\xC0O\xD40\xC8" #:nodoc:
+ UUID_X500_NAMESPACE = "k\xA7\xB8\x14\x9D\xAD\x11\xD1\x80\xB4\x00\xC0O\xD40\xC8" #:nodoc:
+
+ # ::uuid generates a v5 non-random UUID (Universally Unique IDentifier)
+ #
+ # p SecureRandom.uuid_from_hash(Digest::MD5, SecureRandom::UUID_DNS_NAMESPACE, 'www.widgets.com') #=> "3d813cbb-47fb-32ba-91df-831e1593ac29"
+ # p SecureRandom.uuid_from_hash(Digest::MD5, SecureRandom::UUID_URL_NAMESPACE, 'http://www.widgets.com') #=> "86df55fb-428e-3843-8583-ba3c05f290bc"
+ # p SecureRandom.uuid_from_hash(Digest::SHA1, SecureRandom::UUID_OID_NAMESPACE, '1.2.3') #=> "42d5e23b-3a02-5135-9135-52d1102f1f00"
+ # p SecureRandom.uuid_from_hash(Digest::SHA1, SecureRandom::UUID_X500_NAMESPACE, 'cn=John Doe, ou=People, o=Acme, c=US') #=> "fd5b2ddf-bcfe-58b6-90d6-db50f74db527"
+ #
+ # Using Digest::MD5 generates version 3 UUIDs; Digest::SHA1 generates version 5 UUIDs.
+ # ::uuid_from_hash always generates the same UUID for a given name and namespace combination.
+ #
+ # See RFC 4122 for details of UUID.
@robin850
Ruby on Rails member
robin850 added a line comment Sep 28, 2013

For the documentation, I think it's a bit redundant to put the method's name. I would simply say "Generates a v5 non-random ...". Could you update this with other methods as well please ? Also, we could put a link to RFC 4122, what do you think ? Very nice PR though. Thank you!

@roderickvd
roderickvd added a line comment Sep 29, 2013
@zzak
Ruby on Rails member
zzak added a line comment Sep 29, 2013

I find these examples too redundant, also we can easily change SecureRandom docs to reflect new style.

@roderickvd
roderickvd added a line comment Sep 29, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@roderickvd

I improved the legibility of the SecureRandom documentation as per @robin850's suggestions. Any chance this could get slipped into 4.0.1? Let me know if I should squash!

@rafaelfranca
Ruby on Rails member

I don't think so. This don't look like a bug fix, seems more like a new feature so it'd only go in 4.1.0

@roderickvd

Yet issue #11524 has been there for three months now -- ever since 4.0 was released, the UUID support for fixtures has been missing, causing them to break on UUID columns out of the box. I'd say that's a bug, not a missing feature.

@ruckc
@rafaelfranca
Ruby on Rails member

Automatically generated tests are also broken for strings primary keys. I still think this should not be backported since it is a new feature not a bug fix.

@roderickvd
@roderickvd

2 months ago @rafaelfranca considered this for a 4.1.0 release. Now I see it has been scheduled for 4.2.0. 😔

@rafaelfranca
Ruby on Rails member

I still can add this to 4.1.0, but you will need to rebase and squash you commits. Could you?

@rafaelfranca rafaelfranca was assigned Jan 2, 2014
@roderickvd
@roderickvd

Squashed and rebased in 355396a.

@senny
Ruby on Rails member

@rafaelfranca can you take another look?

@rafaelfranca rafaelfranca commented on the diff Apr 3, 2014
activerecord/CHANGELOG.md
@@ -752,6 +758,10 @@
*thedarkone*
+* Test that PostgreSQL adapter includes `usec` when quoting `DateTime` objects
@rafaelfranca
Ruby on Rails member
rafaelfranca added a line comment Apr 3, 2014

This changelog entry seems wrong but we can remove when merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rafaelfranca rafaelfranca commented on the diff Apr 3, 2014
activerecord/lib/active_record/fixtures.rb
@@ -657,7 +662,8 @@ def table_rows
row[association.foreign_type] = $1
end
- row[fk_name] = ActiveRecord::FixtureSet.identify(value)
+ fk_type = association.send(:active_record).columns_hash[association.foreign_key].type
@rafaelfranca
Ruby on Rails member
rafaelfranca added a line comment Apr 3, 2014

Calling send here is weird. If we need to use this method I think is better to make it public and mark it as nodoc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rafaelfranca
Ruby on Rails member

@senny just these two minor comments. We can even merge this and work on these comments after it or we can wait @roderickvd upgrade the PR.

@roderickvd

Great! Unfortunately I won't have access to my Git repository the coming two months, because I'm at sea catching pirates (arr!).

Hopefully my work still merges correctly.

@rafaelfranca
Ruby on Rails member

@roderickvd Awesome!

@senny do you want to merge or should I?

@senny
Ruby on Rails member

@rafaelfranca please go ahead.

@rafaelfranca
Ruby on Rails member

Just to update this thread, I'm merging this.

@rafaelfranca rafaelfranca merged commit 9330631 into rails:master Apr 10, 2014

1 check passed

Details default The Travis CI build passed
@blowmage

Thank you for this!

@rafaelfranca
Ruby on Rails member

❤️

@yabawock

Any chance of getting this backported for 4.1?

@robin850
Ruby on Rails member

@yabawock : Unfortunately, this can't be backported as this is a new feature (see this comment).

@roderickvd roderickvd deleted the roderickvd:uuid_fixes branch Jul 11, 2014
@jalberto

Any suggested workaround for Rails 4.1.x?

I understand why this is defined as "feature" but it feels like a bug, as it block me using Rails 4.1 uuid feature out of the box.

@ralph

Suggested workaround is: Use factories.

Just kidding, you can use fixtures like so:

one:
  id: "80dfbd1f-aa6b-463c-a0ca-56d8ea205a73"
  user_id: "353ccafe-d9a1-45ee-8250-2c33bbac2a31"
  body: "Lorem Ipsum"

Just make sure you put valid uuids into the quotes. You won't be able to use advanced features like referencing associated records by name, but anyway.

@jalberto

LOL for a second I believed it!

I suspected it will not be possible to use reference by name.

I really think some fixtures features are broke in 4.1 because of this.

@barberousse

My workaround right now is to use fixtures anyway and then use glue in seed.db to connect associations. e.g.

users = User.find_each 
profiles = Profile.find_each

# In this example there are as many profiles as users, but its not hard
# to do weirder things
User.count.times do 
  profile = Profile.next
  user = User.next
  user.profile = profile
end

I've done things like this for hundreds of instances and it all gets done pretty quick.

@AndrewSwerlick

I just developed a workaround that almost allows for reference by name. Basically I make use of the ability to put erb into yaml to generate stable uuid by hashing the fixture label.

First install the gem uuidtools

then put this code into your test_helper.rb at the bottom (outside of the class definition)

module FixtureFileHelpers
  def uuid(label)
    UUIDTools::UUID.sha1_create(UUIDTools::UUID_DNS_NAMESPACE, label)
  end
end
ActiveRecord::FixtureSet.context_class.send :include, FixtureFileHelpers

now inside your fixtures you can explicitly set an id like this

one:
  id: <%= uuid "one" %>

Then you can reference this id in another fixture file like this

child:
  parent_id: <%= uuid "one" %>

It's more verbose than I'd like, but it gives you the same basic functionality

(Note I got this idea by looking that the pull request that actually fixes the issue. Its very similar, though the pull request basically hooks it into to active records internals to make it work automagically)

@seuros
Ruby on Rails member

You don't need the gem, you can use SecureRandom.uuid

@AndrewSwerlick

Can you? From what I saw of the pull request, it looks like SecureRandom was monkey patched so it includes methods that generate stable uuid from a given string. From the docs here I don't see a way to generate stable uuids, just random ones. The stable part is important since otherwise it's impossible to ensure you get the same uuid for the parent_id and the object's id.

@barberousse

That looks pretty good @AndrewSwerlick , definitely gonna apply the solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment