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

Encode signed global id metadata with a hash. #31

Merged
merged 2 commits into from
Aug 24, 2014
Merged

Encode signed global id metadata with a hash. #31

merged 2 commits into from
Aug 24, 2014

Conversation

kaspth
Copy link
Contributor

@kaspth kaspth commented Aug 22, 2014

Based on @jeremy's message here: #29 (comment)

Let's us accommodate new metadata without breaking old messages.

This change also flips some assert_equal calls with wrong ordered arguments.

Let's us accommodate new metadata without breaking old messages.

This change also flips some `assert_equal` calls with wrong ordered arguments.
gid, parsed_purpose = pick_verifier(options).verify(sgid)
gid if pick_purpose(options) == parsed_purpose
metadata = pick_verifier(options).verify(sgid)
metadata['gid'] if pick_purpose(options) == metadata['purpose']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the verifier uses a different serializer, like YAML or Marshal, these will be Symbol keys. They happen to be Strings since we're using a JSON serializer in tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we do about that mismatch? Introduce a uniform serializer, which calls with_indifferent_access?

Den 22/08/2014 kl. 16.28 skrev Jeremy Kemper notifications@github.com:

In lib/global_id/signed_global_id.rb:

@@ -29,8 +29,8 @@ def pick_purpose(options)

 private
   def verify(sgid, options)
  •    gid, parsed_purpose = pick_verifier(options).verify(sgid)
    
  •    gid if pick_purpose(options) == parsed_purpose
    
  •    metadata = pick_verifier(options).verify(sgid)
    
  •    metadata['gid'] if pick_purpose(options) == metadata['purpose']
    
    If the verifier uses a different serializer, like YAML or Marshal, these will be Symbol keys. They happen to be Strings since we're using a JSON serializer in tests.


Reply to this email directly or view it on GitHub.

Kasper

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably use String keys in #to_h

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah, of course!

jeremy added a commit that referenced this pull request Aug 24, 2014
Encode signed global id metadata with a hash.
@jeremy jeremy merged commit bf5b899 into rails:master Aug 24, 2014
@kaspth kaspth deleted the hash-metadata branch August 25, 2014 07:40
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 this pull request may close these issues.

2 participants