From d65206c7fed5e54bb6dd9e1a5834abcbb3163619 Mon Sep 17 00:00:00 2001 From: Brady Wetherington Date: Mon, 20 Jan 2025 13:49:42 +0000 Subject: [PATCH] Move tests to UI-side --- app/Http/Controllers/Api/ImportController.php | 32 +++++++++++++++++- app/Importer/Importer.php | 18 ---------- .../Importing/Api/ImportAssetsTest.php | 26 --------------- tests/Feature/Importing/Ui/ImportTest.php | 33 +++++++++++++++++++ 4 files changed, 64 insertions(+), 45 deletions(-) diff --git a/app/Http/Controllers/Api/ImportController.php b/app/Http/Controllers/Api/ImportController.php index ebf8b550b2..b2954d38bb 100644 --- a/app/Http/Controllers/Api/ImportController.php +++ b/app/Http/Controllers/Api/ImportController.php @@ -9,12 +9,14 @@ use App\Http\Transformers\ImportsTransformer; use App\Models\Asset; use App\Models\Company; use App\Models\Import; +use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\Artisan; use Illuminate\Database\Eloquent\JsonEncodingException; use Illuminate\Support\Facades\Request; use Illuminate\Support\Facades\Session; use Illuminate\Support\Facades\Storage; use League\Csv\Reader; +use Onnov\DetectEncoding\EncodingDetector; use Symfony\Component\HttpFoundation\File\Exception\FileException; use Illuminate\Support\Facades\Log; use Illuminate\Http\JsonResponse; @@ -45,6 +47,9 @@ class ImportController extends Controller $path = config('app.private_uploads').'/imports'; $results = []; $import = new Import; + $detector = new EncodingDetector(); + + \Log::error("Okay, do we have files? ".count($files)); foreach ($files as $file) { if (! in_array($file->getMimeType(), [ 'application/vnd.ms-excel', @@ -55,7 +60,7 @@ class ImportController extends Controller 'text/comma-separated-values', 'text/tsv', ])) { $results['error'] = 'File type must be CSV. Uploaded file is '.$file->getMimeType(); - + \Log::error("Bad mime type"); return response()->json(Helper::formatStandardApiResponse('error', null, $results['error']), 422); } @@ -63,11 +68,34 @@ class ImportController extends Controller if (! ini_get('auto_detect_line_endings')) { ini_set('auto_detect_line_endings', '1'); } + $file_contents = $file->getContent(); //TODO - this *does* load the whole file in RAM, but we need that to be able to 'iconv' it? + $encoding = $detector->getEncoding($file_contents); + $reader = null; + if (strcasecmp($encoding, 'UTF-8') != 0) { + \Log::error("Weird encoding detected: $encoding"); + $transliterated = iconv($encoding, 'UTF-8', $file_contents); + if ($transliterated !== false) { + \Log::error("Transliteration was successful."); + $tmpname = tempnam(sys_get_temp_dir(), ''); + $tmpresults = file_put_contents($tmpname, $transliterated); + if ($tmpresults !== false) { + \Log::error("Temporary file written to $tmpname"); + $transliterated = null; //save on memory? + $newfile = new UploadedFile($tmpname, $file->getClientOriginalName(), null, null, true); //WARNING: this is enabling 'test mode' - which is gross, but otherwise the file won't be treated as 'uploaded' + if ($newfile->isValid()) { + \Log::error("new UploadedFile was created"); + $file = $newfile; + } + } + } + } $reader = Reader::createFromFileObject($file->openFile('r')); //file pointer leak? + $file_contents = null; //try to save on memory, I guess? try { $import->header_row = $reader->fetchOne(0); } catch (JsonEncodingException $e) { + \Log::error("cant load header row?"); return response()->json( Helper::formatStandardApiResponse( 'error', @@ -94,6 +122,7 @@ class ImportController extends Controller } } if (count($duplicate_headers) > 0) { + \Log::error("Duplicate headers?"); return response()->json(Helper::formatStandardApiResponse('error', null, implode('; ', $duplicate_headers)),422); } @@ -101,6 +130,7 @@ class ImportController extends Controller // Grab the first row to display via ajax as the user picks fields $import->first_row = $reader->fetchOne(1); } catch (JsonEncodingException $e) { + \Log::error("JSON eocding reception?"); return response()->json( Helper::formatStandardApiResponse( 'error', diff --git a/app/Importer/Importer.php b/app/Importer/Importer.php index 48cdc7266c..907c8b72c5 100644 --- a/app/Importer/Importer.php +++ b/app/Importer/Importer.php @@ -13,7 +13,6 @@ use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\DB; use League\Csv\Reader; use Illuminate\Support\Facades\Log; -use Onnov\DetectEncoding\EncodingDetector; abstract class Importer { @@ -125,28 +124,11 @@ abstract class Importer if (! ini_get('auto_detect_line_endings')) { ini_set('auto_detect_line_endings', '1'); } - $detector = new EncodingDetector(); - // By default the importer passes a url to the file. // However, for testing we also support passing a string directly if (is_file($file)) { - $file_contents = file_get_contents($file); // TODO - this loads up the file in memory! Which could be 'big' and thus, this could be 'bad' - } else { - $file_contents = $file; - } - $encoding = $detector->getEncoding($file_contents); - \Log::debug("DETECTED ENCODING IS: $encoding"); - $file_contents = null; //try to save some memory? - if (is_file($file)) { - if ($encoding && strcasecmp($encoding, 'UTF-8') != 0) { - $file = "php://filter/convert.iconv.$encoding.utf-8/resource=".$file; - } $this->csv = Reader::createFromPath($file); } else { - //we already have the string, so do the conversion directly here? - if ($encoding && strcasecmp($encoding, 'UTF-8') != 0) { - $file = iconv($encoding, 'UTF-8', $file); - } $this->csv = Reader::createFromString($file); } $this->tempPassword = substr(str_shuffle('0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ'), 0, 40); diff --git a/tests/Feature/Importing/Api/ImportAssetsTest.php b/tests/Feature/Importing/Api/ImportAssetsTest.php index fdea4ca47a..0f54b22e92 100644 --- a/tests/Feature/Importing/Api/ImportAssetsTest.php +++ b/tests/Feature/Importing/Api/ImportAssetsTest.php @@ -141,32 +141,6 @@ class ImportAssetsTest extends ImportDataTestCase implements TestsPermissionsReq } - #[Test] - public function importInternationalAsset(): void - { - $evil_string = 'blähÅÄÖ'; //'це; //first one is cyrllic? so is second. - $evil_string = 'це'; //cyrliccic - windows-1251 (ONE) - //copypasta the thing? well, the important bits? - $importFileBuilder = ImportFileBuilder::new(['itemName' => $evil_string]); //not 'name' - $row = $importFileBuilder->firstRow(); - $import = Import::factory()->asset()->create(['file_path' => $importFileBuilder->saveToImportsDirectory(null, 'WINDOWS-1251')]); - - $this->actingAsForApi(User::factory()->superuser()->create()); - $this->importFileResponse(['import' => $import->id]) - ->assertOk() - ->assertExactJson([ - 'payload' => null, - 'status' => 'success', - 'messages' => ['redirect_url' => route('hardware.index')] - ]); - - $newAsset = Asset::query() - ->with(['location', 'supplier', 'company', 'assignedAssets', 'defaultLoc', 'assetStatus', 'model.category', 'model.manufacturer']) - ->where('serial', $row['serialNumber']) - ->sole(); - - $this->assertEquals($evil_string, $newAsset->name); - } #[Test] public function willIgnoreUnknownColumnsWhenFileContainsUnknownColumns(): void { diff --git a/tests/Feature/Importing/Ui/ImportTest.php b/tests/Feature/Importing/Ui/ImportTest.php index 3493f47aff..4b811c4872 100644 --- a/tests/Feature/Importing/Ui/ImportTest.php +++ b/tests/Feature/Importing/Ui/ImportTest.php @@ -3,6 +3,8 @@ namespace Tests\Feature\Importing\Ui; use App\Models\User; +use Illuminate\Http\UploadedFile; +use PHPUnit\Framework\Attributes\Test; use Tests\TestCase; class ImportTest extends TestCase @@ -13,4 +15,35 @@ class ImportTest extends TestCase ->get(route('imports.index')) ->assertOk(); } + + public function testStoreInternationalAsset(): void + { + $evil_string = 'це'; //cyrillic - windows-1251 (ONE) + $csv_contents = "a,b,c\n$evil_string,$evil_string,$evil_string\n"; + + // now, deliberately transform our UTF-8 into Windows-1251 so we can test out the character-set detection + $transliterated_contents = iconv('UTF-8', 'WINDOWS-1251', $csv_contents); + //\Log::error("RAW TRANSLITERATED CONTENTS: $transliterated_contents"); // should show 'unicode missing glyph' symbol in logs. + + $this->actingAsForApi(User::factory()->superuser()->create()); + $results = $this->post(route('api.imports.store'), ['files' => [UploadedFile::fake()->createWithContent("myname.csv", $transliterated_contents)]]) + ->assertOk() + ->assertJsonStructure([ + "files" => [ + [ + "created_at", + "field_map", + "file_path", + "filesize", + "first_row", + "header_row", + "id", + "import_type", + "name", + ] + ] + ]); + \Log::error(print_r($results, true)); + $this->assertEquals($evil_string, $results->json()['files'][0]['first_row'][0]); + } }