TmpFile must have ext for tsv or more.

[yupmin]

Requirements

Please take note of our contributing guidelines: https://laravel-excel-docs.dev/docs/3.0/getting-started/contributing
Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.

Mark the following tasks as done:

• Checked the codebase to ensure that your feature doesn't already exist.
• Checked the pull requests to ensure that another person hasn't already submitted the feature or fix.
• Adjusted the Documentation.
• Added tests to ensure against regression.

Description of the Change

Why Should This Be Added?

I am not good at English. Please understand.

I Want to import tsv file 'title.akas.tsv' on imdb sample data
But tsv is processing like csv, so tsv extension file can't passing.

'title.akas.tsv' mime type detected as 'application/octet-stream'. maybe this file contains 'utf-8' chareset string and multi country language.
in 'vendor/phpoffice/phpspreadsheet/src/PhpSpreadsheet/Reader/Csv.php', 'canRead' function is incorrent at sometime. 'mime_content_type' in 'canRead' is unsafe.

so. tmpfile must have extension.

Benefits

tsv or csv extension file import is very well.

Possible Drawbacks

It looks absent.

Verification Process

None

Applicable Issues

[GlennM]

Hi @yupmin Thanks for your pull request.

Could you have a look at the CI, as it's currently failing?

[yupmin]

@GlennM is it right?

[patrickbrouwers]

@yupmin please also provide a Test for this.

[patrickbrouwers]

@yupmin changes are not needed. You only have to use WithCustomCsvSettings concern to specify the delimiter. See example: https://github.com/Maatwebsite/Laravel-Excel/blob/3.1/tests/ExcelTest.php#L252

[topclaudy]

Changes are required. The mime type of my CSV (downloaded automatically by a script) is reported as "text/x-Algol68" (which is not a supported mime type). The delimiter is comma (,). But when I try to import it, the mime type is wrongly guessed because the 'TmpFile' doesn't have any extension.

[patrickbrouwers]

@topclaudy just make sure to pass the reader type explicitly: https://laravel-excel.maatwebsite.nl/3.1/imports/basics.html#specifying-a-reader-type

[topclaudy]

Still happening (I tried \Maatwebsite\Excel\Excel::CSV). The exception message is "File could not be read". The problem is with the phpoffice/phpspreadsheet/src/PhpSpreadsheet/Reader/Csv:canRead function which returns false on the mime type check.

[patrickbrouwers]

Not sure what we can do about that @topclaudy here , as that's a PhpSpreadsheet method. You can better open an issue over there, I think :)

[topclaudy]

More details... the error spawns from your ReaderFactory

Maatwebsite\Excel\Exceptions\UnreadableFileException {#1844
#message: "File could not be read"
#code: 0
#file: "./vendor/maatwebsite/excel/src/Factories/ReaderFactory.php"
#line: 22
trace: {
./vendor/maatwebsite/excel/src/Factories/ReaderFactory.php:22 { …}
./vendor/maatwebsite/excel/src/Reader.php:273 { …}
./vendor/maatwebsite/excel/src/Reader.php:80 { …}
./vendor/maatwebsite/excel/src/Excel.php:129 { …}
./vendor/laravel/framework/src/Illuminate/Support/Facades/Facade.php:223 { …}
./app/Quotes/Importer/CAShortSale.php:28 {
›
› Excel::import($this, $filename, 'local', \Maatwebsite\Excel\Excel::CSV);
› }
arguments: {
$method: "import"
$args: array:4 [ …4]
}
}
./app/Console/Commands/Quotes/ImportShortSale.php:47 { …}
App\Console\Commands\Quotes\ImportShortSale->handle() {}
./vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:29 { …}
./vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:87 { …}
./vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:31 { …}
./vendor/laravel/framework/src/Illuminate/Container/Container.php:572 { …}
./vendor/laravel/framework/src/Illuminate/Console/Command.php:183 { …}
./vendor/symfony/console/Command/Command.php:255 { …}
./vendor/laravel/framework/src/Illuminate/Console/Command.php:170 { …}
./vendor/symfony/console/Application.php:886 { …}
./vendor/symfony/console/Application.php:262 { …}
./vendor/symfony/console/Application.php:145 { …}
./vendor/laravel/framework/src/Illuminate/Console/Application.php:89 { …}
./vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php:122 { …}
./artisan:35 { …}
}
}

[topclaudy]

Already reported here PHPOffice/PhpSpreadsheet#429

[topclaudy]

The issue could be solved by accepting the PR for the correct extension in the temp file.

[patrickbrouwers]

I'll have another look at it soon.

[yupmin]

PHPOffice/PhpSpreadsheet#429 (comment)

phpspreadsheet package should be changed.

/src/PhpSpreadsheet/Reader/Csv.php

    public function canRead($pFilename)
    {
        ...
        // Trust file extension if any
        if (strtolower(pathinfo($pFilename, PATHINFO_EXTENSION)) === 'csv' ||
            strtolower(pathinfo($pFilename, PATHINFO_EXTENSION)) === 'tsv'
        ) {
            return true;
        }
       ...

And laravel-excel package should be changed in this way.

    protected function getTmpFile($filePath): string
    {
         $ext = strtolower(pathinfo($filePath, PATHINFO_EXTENSION));

         return $this->tmpPath . DIRECTORY_SEPARATOR . str_random(16) . '.' . $ext;
    }

or tsv reader is needed.

[patrickbrouwers]

@yupmin we cannot change PhpSpreadsheet code, you'll have to create a PR there.

[patrickbrouwers]

This should no longer be an issue with the next release

[ebsp]

@patrickbrouwers is it released yet? And if so, how is it fixed?

[GlennM]

@patrickbrouwers is it released yet? And if so, how is it fixed?

I think this been released in version 3.1.6. A test was added to tests/ExcelTest.php:

    /**
     * @test
     */
    public function can_store_tsv_export_with_default_settings()
    {
        $export = new EmptyExport;
        $response = $this->SUT->store($export, 'filename.tsv');
        $this->assertTrue($response);
        $this->assertFileExists(__DIR__ . '/Data/Disks/Local/filename.tsv');
    }