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

Error when marshalling a Contentful resource object #53

Closed
jsdalton opened this issue Jan 29, 2015 · 5 comments
Closed

Error when marshalling a Contentful resource object #53

jsdalton opened this issue Jan 29, 2015 · 5 comments
Assignees

Comments

@jsdalton
Copy link
Member

So this sounds like a pretty typical use case of this gem, but we've run into an issue where the built in Ruby Marshal class thrown an error when a Contentful::Resource object is dumped.

In a nutshell, we're using Contentful in a Ruby project and we're trying to cache the response we get from our call to the Contentful::Client. The Rails cache stores all in some form or another attempt to marshal an object (i.e. dump the object to a byte string) and save that value in the cache, and then load the object back when the cache result is fetched.

So the error we're hitting is coming up on almost every attempt to a marshal a Contentful::Resource, e.g. given a response from a client:

response.class
=> Contentful::Array
Marshal.dump(response)
TypeError: can't dump anonymous class #Module:0x007ff8de3d49e0
irb(main):109:0> response.items[0].class
=> Contentful::Entry
irb(main):110:0> Marshal.dump(response.items[0])
TypeError: can't dump anonymous class #Module:0x007ff8de3d49e0

This also happens with :dynamic_entries set to :auto.

As you can imagine, caching the results of a remote API call locally is a pretty big use case, so I'd be curious to see if this issue has come up elsewhere or if it's on the roadmap already. Also there's a good chance "I'm doing it wrong" and that there's another piece of the response that can be successfully dumped and loaded in a cache for these purposes.

Any insights would be great. Thanks! Also let me know if this issue would be better serviced via the support email channel.

@pxlpnk
Copy link
Contributor

pxlpnk commented Jan 29, 2015

We generate all classes and methods dynamically on the fly from the JSON response, which makes it hard to create an object that can be marshalled at the current point.

It is on our radar to make this possible in a nice way, but there is no timeframe yet when this is going to happen.
In the meanwhile you can check out the contentful_rails project if it suits your needs.
A another possibility would be to use custom classes.

Let me know what you think also on how caching in the gem could be implemented.
This is the right channel for this kind of issues. Code related issues should end up here, product related issues are better handled through the support channel.

@pxlpnk pxlpnk self-assigned this Jan 29, 2015
@jsdalton
Copy link
Member Author

Thanks for the response.

Good news is I did some further exploration and I discovered that the source of the error was actually the Rails logger instance I was passing in to the client on creation. Without that logger the marshal dump/load process appears to be working just fine.

I'll probably code up my own workaround to excluding the logger when the client is marshaled but just wanted to share this finding with you guys, as it appears for the most part everything in your library supports marshaling.

Thanks again!

@pxlpnk
Copy link
Contributor

pxlpnk commented Jan 30, 2015

This is interesting, I will have a look and try to reproduce this.
Thank you for your input.

@plexus
Copy link

plexus commented Mar 16, 2017

I know this is an old thread, but I just ran into this today, trying to implement Contentful caching for a client.

We're using ContentfulModel, and these can be marshalled and cached fine, but they might have associations that are dynamic.

It turns out there's actually a fairly easy fix. DynamicEntry.create does this

module Contentful
  class DynamicEntry < Entry
    # ...
    def self.create(content_type)   
      # ...
      Class.new DynamicEntry do
        # ...
      end
    end
  end
end

This class is anonymous, and so Ruby will refuse to Marshal it. You can get around this by assigning it to a constant.

    def self.create
      #...
      klz = Class.new DynamicEntry do
         # ...
      end
      const_set(:"#{content_type.id.camelize}Class", klz)
    end

Now it marshals and unmarhals fine.

In the current 2.x line of development this DynamicEntry is gone though, so I'm not sure how to proceed.

  • get the client on 2.0, assuming it works or can be made to work there?
  • submit this as a patch for the 1.2.x line, to maybe go into a 1.3.0 release?
  • monkey patch and be done with it?

@dlitvakb
Copy link
Contributor

Hey @plexus,

Thanks for the heads up regarding the 1.x possible fix.

Currently, I would argue in favor of moving to 2.0 (which will be released next Tuesday), in which Entries are no longer anonymous classes and simplify much of their behavior. There is a very small caveat regarding include resolution strategies in cases you have circular references, as there is no longer an entry-level cache, but you can tweak the settings to improve performance on those cases. I've explained it in further detail here: #124 (comment)

Basically the move to 2.0 has little to no impact in most codebases, as the only external API that's changing is Contentful::Link#resolve and Contentful::Array#next_page, in which by survey I found very little usage of, making it safe to assume that the upgrade will be seamless.

To use it in contentful_model though, right now you'd have to fork it in order to be able to upgrade the version lock. But, I'll be releasing a new version of it within the next week or two upgrading it to use the new SDK. I still have not a specific plan on how to replace some bits of behavior, but I have a pretty good idea overall how to revamp it to take advantage of the new niceties that the new SDK provides.

In the meantime, if you can overcome the issue by monkey patching it, I think that's a good compromise. As I think this should be a very short-term situation we'll be facing.

Hope this helps and clarifies the short-term panorama.

Cheers

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

No branches or pull requests

4 participants