Code review needed - saving inbound emails and attachments to server

Posted 1 month ago by oliverbusk

I have below code, which is basically handling inbound emails in my application, save the actual email content (text + attachments) to my database, and further save the actual attachment file on my server.

In my Email.php model, I have below function that handles the incoming email. The $token is the first string of the email. For example, my To: email can look like this: [email protected] where avdksokiell is my token.

So as you can see, I fire off the Email model as below:

Mailbox::to('{token}@myapp.com', Email::class);

When above condition is met, below will be run:

public function __invoke(InboundEmail $email, $token)
{
    //Get the correct stream.
    $stream = Stream::where('token', $token)->firstOrFail();

    //Persist the token to the database. {token}@in.myapp.com
    $email->stream_id = $stream->id;

    //Save the email.
    $email->save();

    //If any attachments, persist them to the database.
    $this->storeAttachments($stream, $email);

}

Above works, because I have a relationship set up between Stream and Email:

Stream.php:

/**
 * A stream can have many e-mails
 */
public function emails()
{
    return $this->hasMany(Email::class);
}

Email.php:

/**
 * An email belongs to a Stream.
 */
public function stream()
{
    return $this->belongsTo(Stream::class);
}

Furthermore, in order to save the actual attachment from the mail, I have created a method called storeAttachments(Stream $stream, InboundEmail $email) function in Email that looks like below:

/**
 * A method to store incoming attachments from email.
 *
 */
public function storeAttachments(Stream $stream, InboundEmail $email)
{
    $attributes = [];
    foreach ($email->attachments() as $attachment) {
        //Set an unique filename
        $filename = uniqid() . '.' . File::extension($attachment->getFilename());
        //Store the file on the server
        $store = Storage::put($stream->token . '/' . $filename, $attachment->getContent());
        //Add file information, so we can persist it to the database.
        $attributes['name'] = $attachment->getFilename();
        $attributes['path'] = $stream->token . '/' . $filename;

        //Persist it to the database.
        $stream->addDocuments($attributes);
    }
}

Because my users can choose to:

  1. Upload files from a web frontend
  2. Send files by email into my app

I also have a Document model, that stores all the files:

I also have a relationship set up here:

Stream.php:

/**
 * A stream can have many documents
 */
public function documents()
{
    return $this->hasMany(Document::class);
}

Document.php:

//A document belongs to a Stream.
public function stream()
{
    return $this->belongsTo(Stream::class);
}

Now, for handling the saving files to the database, I have below method in my Stream model:

/**
 * Add document(s) to the stream
 *
 * @return Illuminate\Database\Eloquent\Model
 */
public function addDocuments(array $attributes)
{
    return $this->documents()->create($attributes);
}

I am a bit unsure if above design/methodology is correct and seen as "best practice".

One thing I notice is, that whenever I handle an incoming mail in the __invoke function, I run a query to get the stream_id by looking up the token because the relationships are using stream_id.

Hope someone can help me shed some light if above is OK or if anything can be improved.

I'll be happy to share more code if needed.

Please sign in or create an account to participate in this conversation.

Reply to

Use Markdown with GitHub-flavored code blocks.