Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug - global id is incorrect with multiple calls #58

Closed
antulik opened this issue Jan 16, 2015 · 2 comments · Fixed by #60
Closed

bug - global id is incorrect with multiple calls #58

antulik opened this issue Jan 16, 2015 · 2 comments · Fixed by #60

Comments

@antulik
Copy link

antulik commented Jan 16, 2015

The issue as it seems global id cached on the model and if it is called more than once with different params it returns wrong result.

Check this out

> User.find(1).to_sgid.to_s == User.find(1).to_sgid(for:'asd').to_s
  User Load (1.5ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1 LIMIT 1  [["id", 1]]
  User Load (0.7ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1 LIMIT 1  [["id", 1]]
=> false

And compare to this:

> u = User.find(1)
  User Load (0.7ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1 LIMIT 1  [["id", 1]]
=> #<User:0x007febc9f74f38 ...>
> u.to_sgid.to_s == u.to_sgid(for:'asd').to_s
=> true
@antulik
Copy link
Author

antulik commented Jan 16, 2015

25cab12 commit broke that

@kaspth
Copy link
Contributor

kaspth commented Jan 16, 2015

That commit didn't break anything. At the time to_sgid could only generate the same id. A later commit introduced the options parameter, which made to_sgid able to generate different signed ids.

To determine if the caching is at all worth it, I've made a benchmark to test how slow the create call is:

require 'global_id'
require 'models/person'

require 'active_support/message_verifier'
require 'benchmark/ips'

person = Person.find(1)

GlobalID.app = 'bcx'
SignedGlobalID.verifier = ActiveSupport::MessageVerifier.new('muchSECRETsoHIDDEN')

Benchmark.ips do |x|
  x.report('gid creation') { GlobalID.create(person) }
  x.report('signed creation') { SignedGlobalID.create(person) }
end

which results in:

kaspth:globalid kasperhansen$ ruby -Ilib:test sgid-create-perf.rb
Calculating -------------------------------------
        gid creation   934.000  i/100ms
     signed creation   902.000  i/100ms
-------------------------------------------------
        gid creation     10.452k (± 4.2%) i/s -     52.304k
     signed creation     10.004k (± 4.7%) i/s -     50.512k

Which sounds like it can generate 10k signed ids per second, if I'm reading it right.

So I'm guessing we can do without the signed id cache. @jeremy, what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants