PHP - Saving inbound email and its attachment

Question

I have the below code, which handles inbound emails in my application, saves the actual email content (text + attachments) to my database, and the saves the actual attachment files on the server.

In my Email.php model, I have the below function which handles the incoming email. The $token is the first string of the email. For example, my To: email address 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 the above condition is met, the code 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);

}

The above approach 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);
}

Is the above design/methodology correct and seen as "best practice"?

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

I hope someone can help me by shedding some light if the above approach is okay or if anything can be improved.

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

Answer 1

It can be more efficient

It seems okay to use the create() method to insert a document record. However, for the sake of efficiency there is a method to create multiple models in a single call. Per the Laravel documentation for The create Method:

You may use the createMany method to create multiple related models:

$post = Post::find(1);
 
$post->comments()->createMany([
    ['message' => 'A new comment.'],
    ['message' => 'Another new comment.'],
]);

¹

To use that, the storeAttachments method could be updated to store the attributes to be inserted into an array and then that array could be passed to the createMany() method, something like the (untested) code below:

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 attributes to the array of attributes to be inserted
        $attributes[] = [
            'name' => $attachment->getFilename(),
            'path' => $stream->token . '/' . $filename
        ];
    }

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

The creation of $attributes could be simplified using the map() collection method, though it would require extra function calls:

public function storeAttachments(Stream $stream, InboundEmail $email)
{
    $attributes = $email->attachments()->map(function ($attachment) use ($stream) {
        //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 attributes to the array of attributes to be inserted
        return [
            'name' => $attachment->getFilename(),
            'path' => $stream->token . '/' . $filename
        ];
    });

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

Method name could be confusing

As pointed out in the previous section, multiple attachments can be created in one step using the createMany() method. This illustrates that the current method in the above code, addDocuments() only adds one document model. Someone reading the code might figure that the method adds more than one document, which does not appear to be true. A more appropriate name for that method would be addDocument().