Mail::Message#to_yaml - include all necessary instance variables

[urbanautomaton]

Hi all,

As noted by @sandstrom in pull request #201, the existing Mail::Message#to_yaml and #from_yaml methods do not preserve important delivery information for the message, such as the delivery_handler and transport_encoding.

Some of these fields were in fact what caused the original problem pull request #201 sought to solve. YAML refuses to serialize anonymous classes, and as delivery_handler and transport_encoding are stored as such, the default Object#to_yaml implementation fails. The existing #to_yaml method avoids this problem by not serializing any instance variables bar the header and body. While this works to a point, the deserialized Message is incapable of being delivered.

My attached commits address this by extending the solution in #201 to include all instance variables, making special cases of those storing anonymous classes. While I'm not particularly happy with this approach, it does at least work and requires no changes elsewhere in the class, which I was keen to avoid. The deserialized message retains its delivery method and handler information, and can be sent if deserialized in the right environment.

One concern I have is that my approach (which avoids the call to #ready_to_send! that the original used) may break whatever use case for #to_yaml the committer of #201 had in mind. However I think the expectation that a deserialized Message should be capable of being sent is pretty reasonable - and any transformation the original performed can be repeated on the deserialized object under my implementation.

Any feedback much appreciated - like I say I'm not wildly enamoured of this approach, but I wanted to keep my changes localised as I'm not yet fully familiar with this project.

Cheers,
Simon

[urbanautomaton]

Forgot to say - while I've opened this request on master, if the commits could be cherry-picked in to the v2.2 branch as well I would be eternally grateful, as Rails 3.0.x depends on mail v2.2.x.

[mikel]

Can you send me a pull request cherry picking these to 2.2?

[urbanautomaton]

Certainly - will do so tomorrow. Thanks for the merge!

[cpytel]

Hi @urbanautomaton it looks like a pull request was never created for the cherry pick of this into 2.2? Thanks.

[urbanautomaton]

Hi @cpytel - it was, it's #246. Sorry, should have ref'd it here when I created it.

[cpytel]

Thanks.

On Jul 12, 2011, at 3:23 PM, urbanautomaton wrote:

Hi @cpytel - it was, it's #246. Sorry, should have ref'd it here when I created it.

Reply to this email directly or view it on GitHub:
#237 (comment)

---

Chad Pytel, Founder and CEO
thoughtbot, inc.
t: 617-482-1300 x113
f: 866-217-5992
http://www.thoughtbot.com
http://www.twitter.com/thoughtbot