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

Her::Model::Attributes.initialize_collection does not accept string keys #396

Open
ditsara opened this issue Feb 4, 2016 · 3 comments
Open

Comments

@ditsara
Copy link

ditsara commented Feb 4, 2016

When I use Faraday::HttpCache in my middleware, cached responses return this error:

/Users/ditsara/.rvm/gems/ruby-2.3.0@gq-commerce/gems/her-0.8.1/lib/her/model/attributes.rb:35

The problem is Faraday::HttpCache returns parsed_data with string keys instead of symbols, so klass.extract_array in her/model/parse.rb:171 returns nil because it's looking for :data (but not 'data').

I was able to work around the problem by adding:

def self.extract_array(request_data)
  super(request_data.with_indifferent_access)
end

to my class (ActiveSupport::HashWithIndifferentAccess). Should extract_array be modified to accept a string key? I can submit a PR if necessary.

relevant code snippets

My middleware:

  c.use Faraday::HttpCache,
    store: Rails.cache,
    shared_cache: false,
    logger: Rails.logger

  # Request
  c.use FaradayMiddleware::EncodeJson

  # Response
  c.use Her::Middleware::DefaultParseJSON

  # Adapter
  c.use Faraday::Adapter::NetHttp

her/model/attributes.rb:34

      def self.initialize_collection(klass, parsed_data={})
        collection_data = klass.extract_array(parsed_data).map do |item_data|
        ...

her/model/parse.rb:171

        def extract_array(request_data)
          if request_data[:data].is_a?(Hash) && (active_model_serializers_format? || json_api_format?)
            request_data[:data][pluralized_parsed_root_element]
          else
            request_data[:data]
          end
        end
@coreyward
Copy link

Does Faraday::HttpCache rely on intercepting the request before it's made? It would seem that it should be caching raw HTTP responses before the keys have been symbolized in the first place. scratches head

@ditsara
Copy link
Author

ditsara commented Feb 19, 2016

I believe that it needs to, so it can add the proper Cache-Control headers if it has previously stored the etag. How would I test? By placing Faraday::HttpCache below NetHttp in the middleware stack?

@coreyward
Copy link

Ah, so after looking at Faraday::HttpCache and the Her docs a little better, I think the issue is that HttpCache is using serialization, and Her is very adamant about the input it receives. The documentation references using FaradayMiddleware::Caching, which doesn't seem to attempt any of this serialization malarky. That might be worth trying instead.

If you would prefer to stick with HttpCache, you could probably just write a custom serializer that piggybacks on JSON and has a call to stringify_keys on the load. I would avoid the Hash#with_indifferent_access for the simple reason that you're handing Her an object that it isn't expecting and you could run into a bug with it.

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

No branches or pull requests

2 participants