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

Tokens serialization #1250

Merged
merged 1 commit into from
Mar 23, 2019
Merged

Conversation

dks17
Copy link
Contributor

@dks17 dks17 commented Jan 20, 2019

This is custom coder for tokens column with known object's structure (only Hash). This is based on JSON coder with some edge cases (nil, blank string).

PG registers jsonb as alias type for json and uses own text coder where also uses JSON coder and few other date/time formats for encoding/decoding.

Should help to serialize/deserialize tokens properly.

This should fix:
#1079 #121
and may:
#1240 #1224

Need help testing the PR before it will be merged.

UPDATE

  • Updated dump method.
  • Deleted code from load method that recognize case when json is blank string (it is unlikely case).

@MaicolBen
Copy link
Collaborator

@dks17 Awesome!! Can you explain what you did?

@dks17
Copy link
Contributor Author

dks17 commented Jan 21, 2019

@MaicolBen Wrote small description about it at the top message.

module DeviseTokenAuth::TokensSerialization
# Serialization hash to json
def self.dump(object)
object.each_value(&:compact!) unless object.nil?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we care about storing null values we can delete this line.
See last_token below:

"{"nlw47IfMSJxHl8v2bvhrOw":{"token":"$2a$10$3yWVUhPtnn/vigTztah2fuioMMEB6R8aZunkFOjzguV1ZCZ8kga16","expiry":1549026407,"last_token":null,"updated_at":"2019-01-18T13:06:32.876Z"}}"

# Serialization hash to json
def self.dump(object)
object.each_value(&:compact!) unless object.nil?
JSON.generate(object)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Serialize nil to null:

JSON.generate(nil)
=> "null"

Copy link
Collaborator

@MaicolBen MaicolBen left a comment

Choose a reason for hiding this comment

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

@dks17 Thank you for explaining this, I tested it in one app but we can check in the issue's thread if it's happening after merged

@kdubel
Copy link

kdubel commented May 15, 2019

Thanks for this fix! I had v1.1.0 of gem and the problem existed, updated to your fix and everything works fine. @MaicolBen maybe it's a good idea to bump version as searching for this issue takes some time?

@softwaregravy
Copy link

I'm tracing through the threads on this issue as well. I agree it would be great to get this out.

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.

4 participants