Unify attachment handling API

[jlinnosa]

Currently we have the following function signatures for adding attachments to an Email struct:

func (email *Email) AddAttachment(file string, name ...string) *Email
func (email *Email) AddAttachmentBase64(b64File, name string) *Email
func (email *Email) AddAttachmentData(data []byte, filename, mimeType string) *Email
func (email *Email) AddInline(file string, name ...string) *Email
func (email *Email) AddInlineBase64(b64File, name, mimeType string) *Email
func (email *Email) AddInlineData(data []byte, filename, mimeType string) *Email

For some reason AddAttachment and AddInline are variadic but only accept one name:

        if len(name) > 1 {
                email.Error = errors.New("Mail Error: Attach can only have a file and an optional name")
                return email
        }

Other functions take name as parameter and possibly also mimeType.

I propose we modify all these functions to accept name and mimeType. So the API would become:

func (email *Email) AddAttachment(file, name, mimeType string) *Email
func (email *Email) AddAttachmentBase64(b64Data, name, mimeType string) *Email
func (email *Email) AddAttachmentData(data []byte, name, mimeType string) *Email
func (email *Email) AddInline(file, name, mimeType string) *Email
func (email *Email) AddInlineBase64(b64Data, name, mimeType string) *Email
func (email *Email) AddInlineData(data []byte, name, mimeType string) *Email

edit. I changed argument name b64File -> b64Data so it is consistent with file and data arguments.

[jlinnosa]

I made a PR implementing these modifications.

[xhit]

Note: I don't speak english, sorry any mistake.

AddAttachment and AddInline actually receives name because it's possible call the function with the file path to attach only, and the name of file in attachment will be the name of attached file from function.

The name is only a new name that developer want to deliver.

For example, the developer can attach a file like this:

AddAttachment("filepaht/aaa.txt")

And the developer can use a new name for the file

AddAttachment("filepaht/aaa.txt","bbb.txt")

The attachment will be received like bbb.txt

And, the reason this is implemented that form is because a mandatory name will confuse some developers. Send mail with attachment is attach the file only, It's weird to see an implementation where name should change.

And about the mimetype, the package, with the name of file, can capture the mimeType. This not apply when attach a file from bytes and that's why these functions requires mimeType, but I think it's better remove the mimeType parameter and calculate the mimeType from name like other functions.

[jlinnosa]

One possibility would be to keep AddAttachment and AddInline as variadic, but accept mimeType as the second parameter for name. This doesn't change the function signature but offers all the functionality that the other functions provide.

[xhit]

I have another idea to unify all attachment.

All these methods:

func (email *Email) AddAttachment(file string, name ...string) *Email
func (email *Email) AddAttachmentBase64(b64File, name string) *Email
func (email *Email) AddAttachmentData(data []byte, filename, mimeType string) *Email
func (email *Email) AddInline(file string, name ...string) *Email
func (email *Email) AddInlineBase64(b64File, name, mimeType string) *Email
func (email *Email) AddInlineData(data []byte, filename, mimeType string) *Email

Replace with only one and use a struct to define the attachment.

The struct:

type File struct {
	Filepath string
	Name     string
	MimeType string
	B64File  string
	Data     []byte
	Inline   bool
}

The method to unify all:

func (email *Email) Attach(file File) *Email

The commit cb2f478 (non tested and WIP) is based on this idea. Feel free to test.

[xhit]

unify-attach branch is the branch to test this.

[hiendv]

Hi, @xhit
Do you have any plan to support custom ContentID (cid)?

[xhit]

@jlinnosa I finished the Unify attachment api. Please see the unify-attach. There are some changes in my last comment

The solution, if not problem with that, will be merged with a new version v3

[xhit]

@hiendv please fill another issue and I will respond on that.

[jlinnosa]

@jlinnosa I finished the Unify attachment api. Please see the unify-attach. There are some changes in my last comment

The solution, if not problem with that, will be merged with a new version v3

I'm ok with that. I looked over the changes in the PR and commented on some things.

[jlinnosa]

You could add a simple shim to implement the current API with this new function so there would be no need to bump the major version. This could be just a new feature. You could then deprecate and remove the old API when releasing v3 later.

[xhit]

Thanks, I will work on that maybe tomorrow.

[xhit]

Added in #35

Not a breaking change.

[jlinnosa]

Like I commented in the PR, please don't keep two separate implementations for the same functionality. That could be a major hindrance in the future.