Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Error with queued exports when filesystems are different #4033

Closed
1 task done
adamgoucher opened this issue Nov 21, 2023 · 4 comments · Fixed by #4034
Closed
1 task done

[Bug]: Error with queued exports when filesystems are different #4033

adamgoucher opened this issue Nov 21, 2023 · 4 comments · Fixed by #4034
Labels

Comments

@adamgoucher
Copy link
Contributor

Is the bug applicable and reproducable to the latest version of the package and hasn't it been reported before?

  • Yes, it's still reproducable

What version of Laravel Excel are you using?

3.1.50

What version of Laravel are you using?

10.32.1

What version of PHP are you using?

8.2

Describe your issue

#3863 which was fixed in #3876 deals with Imports. I am experiencing the equivalent of it with exporting.

We're changing some infrastructure around and so running a mixed environment with the report job gets picked up on one box (aws linux 2), but actually dealt with on another (ubuntu) and thus have different filesystems.

How can the issue be reproduced?

Using 2 different boxes...

  • create a queued report on box a
  • have box b actually execute the report

What should be the expected behaviour?

The path it tries to write to on box b is the path it stored in the queued job payload on box a, which isn't necessarily the correct thing.

adamgoucher added a commit to adamgoucher/Laravel-Excel that referenced this issue Nov 21, 2023
patrickbrouwers pushed a commit that referenced this issue Nov 29, 2023
* check if local path is valid before writing

fixes #4033

* missing import

* fixing extra spaces on empty line
@axyr
Copy link

axyr commented Dec 15, 2023

For the time being, when this fixed is not yet merged, I made an EventListener to ensure the temporary path exists:

<?php

namespace App\Shared\Excel;

use Illuminate\Queue\Events\JobProcessing;
use Illuminate\Support\Arr;
use Maatwebsite\Excel\Files\RemoteTemporaryFile;
use Maatwebsite\Excel\Files\TemporaryFile;
use Maatwebsite\Excel\Files\TemporaryFileFactory;
use Maatwebsite\Excel\Jobs\QueueExport;
use ReflectionObject;

// EventServiceProvider:
// \Illuminate\Queue\Events\JobProcessing::class => [
//      EnsureExcelTemporaryFileExists::class,
// ],

class EnsureExcelTemporaryFileExists
{
    public function handle(JobProcessing $jobProcessing): void
    {
        if ($export = $this->getQueueExport($jobProcessing)) {

            $temporaryFile = $this->getTemporaryFile($export);

            $this->ensureTemporaryFileExists($temporaryFile);
        }
    }

    private function getQueueExport(JobProcessing $jobProcessing): ?QueueExport
    {
        $payload = $jobProcessing->job->payload();

        if ($payload['data']['commandName'] === QueueExport::class) {
            return unserialize($payload['data']['command'], [QueueExport::class]);
        }

        return null;
    }

    private function getTemporaryFile(QueueExport $export): TemporaryFile
    {
        return (new ReflectionObject($export))
            ->getProperty('temporaryFile')
            ->getValue($export);
    }

    private function ensureTemporaryFileExists(TemporaryFile $temporaryFile): void
    {
        if ($temporaryFile instanceof RemoteTemporaryFile && !$temporaryFile->existsLocally()) {
            resolve(TemporaryFileFactory::class)
                ->makeLocal(Arr::last(explode('/', $temporaryFile->getLocalPath())));
        }
    }
}

@patrickbrouwers
Copy link
Member

It is merged right?

@axyr
Copy link

axyr commented Dec 15, 2023

Maybe I am a bit noob when it comes to tagging, but when using "maatwebsite/excel": "^3.1" it is pulling 3.1.50, but this change is not there, so I assume it on the feature branch, but not on the latest tag?

@patrickbrouwers
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants