Change attachment struct to support file data instead of filename

[sbrink]

I propose that we change the current Attachement struct before releasing version 1.0.

Why?

At the moment an attachment struct looks like this:

defstruct filename: nil, content_type: nil, path: nil

This only allows uploads from the file system, not dynamically generated contents. An use case for generated contents would be .ics files. You want to append a generated .ics file for adding an event to the user's calendar.

Furthermore every adapter has to implement the correct encoding - this is redundant. Here's an example I found from a patch for the bamboo_stmp adapter:

defp add_attachment_body(body, attachment) do
  file = File.read!(attachment)
   "#{body}\r\n#{Base.encode64(file)}\r\n"
 end

Proposal:

We change the struct definition to:

defstruct filename: nil, content_type: nil, data: nil

We could:

1. Split the new function into new_from_data and new_from_attachment
2. Use pattern matching in the new function

The new new method does the job of file reading and encoding.

What do you think?

[gitviola]

+1

In most of my use cases I would use dynamically generated data that I wouldn't want to store on the hard disk before attaching them to outgoing emails. I think it makes sense to change the Attachment struct before releasing version 1.0 as @sbrink proposes.

[paulcsmith]

Thank you so much for the suggestion.

I like this idea. I like option 1 of using two different functions, but I think new_from_file might make more sense since they are both attachments.

I'm not sure about doing the Base64 encoding outside the adapter because there may be adapters that don't need that.

Would one of you be willing to work on a PR for this?

[andrewtimberlake]

I agree with this. I made a change in my adapter to accept {:binary, data} in the path to allow for this (https://github.com/andrewtimberlake/bamboo_sparkpost/blob/feature/attachments/lib/bamboo/adapters/sparkpost_adapter.ex#L173)

[sbrink]

I had no time last week. @schurig and I are working on it now.

[paulcsmith]

You rock. Thank you!

[gitviola]

Hi everyone!

@sbrink and I were working on a version this afternoon

• supports |> put_attachment %Attachment{filename: "event.ics", data: "raw content..", content_type: "text/calendar"}
• sets Attachment.data when using |> put_attachment "/hi.jpg" or |> put_attachment %Plug{..}
• supports attachments in the mailgun and test adapter
• updates the mandrill adapter to use the attachment.data

Still missing

• tests for the mailgun adapter (attachment support)

---

We're currently implementing it into our Phoenix app. Here are the changes: https://github.com/workshops-de/bamboo/commit/develop

@paulcsmith what do you think?

[paulcsmith]

@schurig It's hard to tell what changed, but from what I can see it looks great! Can you open up a PR so it's easier to see what changed and easier to leave comments. Thanks a ton!

[gitviola]

@paulcsmith this one can be closed I think 🎉 🌮

[paulcsmith]

You're right, thank you!