Skip to content

Conversation

relaxcore
Copy link

@relaxcore relaxcore commented Jan 21, 2021

What is the current behavior?

The Deserialization module use as_json to represent input params as JSON. But it change all objects (uploaded files, rails models, etc) to strings. Same as use to_s on them.

Example:

params = { data: { attributes: { file: ::File.new(any_existing_filepath) } } }

params.as_json.deep_stringify_keys.dig('data', 'attributes', 'file').class # => String
params.to_h.deep_stringify_keys.dig('data', 'attributes', 'file').class # => File

What is the new behavior?

File attributes in deserialized data have the correct type

Checklist

Please make sure the following requirements are complete:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes /
    features)
  • All automated checks pass (CI/CD)

@relaxcore relaxcore force-pushed the fix/object_as_value branch 3 times, most recently from fa76ad2 to 3e1b68a Compare January 21, 2021 11:46
@stas
Copy link
Owner

stas commented Jan 21, 2021

@relaxcore this looks good, except for the part with with_indifferent_access (I'm trying to stay away from Rails stuff).

I imagine you're using the jsonapi_deserialize as part of a multipart request, please correct me if I'm wrong?!

Thank you!

@relaxcore relaxcore force-pushed the fix/object_as_value branch from 3e1b68a to d2709d6 Compare January 21, 2021 16:49
@relaxcore
Copy link
Author

relaxcore commented Jan 21, 2021

@stas you're right 😄
Just replaced #with_indifferent_access method with #deep_stringify_keys. Should be okay as well

@stas
Copy link
Owner

stas commented Jan 21, 2021

@relaxcore that's still ActiveSupport 🍯

Let's just leave it without :)

@relaxcore
Copy link
Author

relaxcore commented Jan 21, 2021

@stas it won't work in this way just after #to_h. As I understood, keys should be a strings, not symbols

{ data: {} }.as_json.keys.first.class # => String
{ data: {} }.to_h.keys.first.class # => Symbol

BTW, #as_json is also ActiveSupport 😸

@stas
Copy link
Owner

stas commented Jan 21, 2021

@relaxcore consider using #transform_keys(&:to_s):
https://devdocs.io/ruby~3/hash#method-i-transform_keys

@relaxcore
Copy link
Author

relaxcore commented Jan 21, 2021

@stas that's also not a good idea, cause it won't "stringify" nested keys, like attributes, etc. With current #as_json logic all nested keys are strings

@stas
Copy link
Owner

stas commented Jan 25, 2021

@relaxcore sorry for the delay. Before we look into technical solutions here, would you be kind to explain a bit what's the use-case here.

From my side it's hard to understand how you ended up deserializing a JSONAPI request with a file upload, since it usually requires an application/json content-type. Are you sending multipart requests and trying to stick to JSONAPI format?

@relaxcore
Copy link
Author

relaxcore commented Jan 27, 2021

@stas Everything is pretty simple and as you said. Just have one multipart request and find such behavior

@stas
Copy link
Owner

stas commented Feb 14, 2021

@relaxcore sorry for the delay here.

Please let me know if you're still interested in wrapping this up.
What I'd like to suggest here is to make the support for deep_stringify_keys pluggable, in a similar way it's done here:
https://github.com/stas/jsonapi.rb/blob/master/lib/jsonapi/deserialization.rb#L1-L9

It looks like dry-transformer can be used here
https://github.com/dry-rb/dry-transformer/blob/master/lib/dry/transformer/hash.rb

Please let me know what you think. Thanks again for sending the PR.

@stas stas added this to the v1.8 milestone Sep 1, 2021
@alakra
Copy link

alakra commented Mar 8, 2023

Bump. Any updates on this?

@relaxcore relaxcore closed this Apr 24, 2023
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.

3 participants