diff --git a/trpcultivate_phenotypes/src/Plugin/TripalImporter/TripalCultivatePhenotypesTraitsImporter.php b/trpcultivate_phenotypes/src/Plugin/TripalImporter/TripalCultivatePhenotypesTraitsImporter.php index caaf9df2..871c7041 100644 --- a/trpcultivate_phenotypes/src/Plugin/TripalImporter/TripalCultivatePhenotypesTraitsImporter.php +++ b/trpcultivate_phenotypes/src/Plugin/TripalImporter/TripalCultivatePhenotypesTraitsImporter.php @@ -459,7 +459,7 @@ public function formValidate($form, &$form_state) { // Set failures for this validator name to an empty array to signal that // this validator has been run. $failures[$validator_name] = []; - $result = $validator->validateFile('', $file_id); + $result = $validator->validateFile($file_id); // Check if validation failed and save the results if it did. if (array_key_exists('valid', $result) && $result['valid'] === FALSE) { diff --git a/trpcultivate_phenotypes/src/Plugin/Validators/ValidDataFile.php b/trpcultivate_phenotypes/src/Plugin/Validators/ValidDataFile.php index bd75a85c..8f687ddc 100644 --- a/trpcultivate_phenotypes/src/Plugin/Validators/ValidDataFile.php +++ b/trpcultivate_phenotypes/src/Plugin/Validators/ValidDataFile.php @@ -74,16 +74,14 @@ public static function create(ContainerInterface $container, array $configuratio * Validate that the input file is a valid file. * * Checks include: - * - Parameter filename or file id is valid. + * - File ID parameter is positive non-zero integer, and cannot be null. * - Has Drupal File Id number assigned and can be loaded. * - File extension and mime type are configured by the importer. * - File exists and is not empty. * - File can be opened. * - * @param string $filename - * The full path to a file within the file system (Absolute file path). * @param int $fid - * [OPTIONAL] The unique identifier (fid) of a file that is managed by + * The unique identifier (fid) of a file that is managed by * Drupal File System. * * @return array @@ -96,84 +94,37 @@ public static function create(ContainerInterface $container, array $configuratio * - 'fid': The fid of the provided file. * - 'mime': The mime type of the input file if it is not supported. * - 'extension': The extension of the input file if not supported. - * - * @throws \Exception - * - If parameters $filename and $fid are both provided, but do not point to - * the same file object. */ - public function validateFile(string $filename, int|null $fid = NULL) { + public function validateFile(int|null $fid) { - // Parameter check, verify that the filename/file path is valid. - if (empty($filename) && is_null($fid)) { + // Parameter check, verify the file id number is not null, 0 or + // a negative value. + if (is_null($fid) || $fid <= 0) { return [ - 'case' => 'Filename is empty string', + 'case' => 'Invalid file id number', 'valid' => FALSE, 'failedItems' => [ - 'filename' => '', 'fid' => $fid, ], ]; } - // Parameter check, verify the file id number is not 0 or a negative value. - if (!is_null($fid) && $fid <= 0) { + // Load the file object by fid number. + $file_object = $this->service_EntityTypeManager + ->getStorage('file') + ->load($fid); + + // Check that the file input provided returned a file object. + if (!$file_object) { return [ - 'case' => 'Invalid file id number', + 'case' => 'File id failed to load a file object', 'valid' => FALSE, 'failedItems' => [ - 'filename' => $filename, 'fid' => $fid, ], ]; } - // Holds the file object when file is a managed file. - $file_object = NULL; - - // Load file object. - if (is_numeric($fid) && $fid > 0) { - // Load the file object by fid number. - $file_object = $this->service_EntityTypeManager - ->getStorage('file') - ->load($fid); - - // Verify that the filename (if provided) matches the filename in the file - // object returned by the file id. - if (!empty($filename) && $file_object->getFileName() != pathinfo($filename, PATHINFO_FILENAME)) { - throw new \Exception('The filename provided does not match the filename set in the file object.'); - } - } - elseif ($filename) { - // Locate the file entity by uri and load the file object using the - // returned file id number that matched. - $file_object = $this->service_EntityTypeManager - ->getStorage('file') - ->loadByProperties(['uri' => $filename]); - $file_object = reset($file_object); - } - - // Check that the file input provided returned a file object. - if (!$file_object) { - if (is_null($fid)) { - return [ - 'case' => 'Filename failed to load a file object', - 'valid' => FALSE, - 'failedItems' => [ - 'filename' => $filename, - ], - ]; - } - else { - return [ - 'case' => 'File id failed to load a file object', - 'valid' => FALSE, - 'failedItems' => [ - 'fid' => $fid, - ], - ]; - } - } - // File object loaded successfully. Any subsequent failed checks will // reference the filename and file id from the established file object. $file_filename = $file_object->getFileName(); diff --git a/trpcultivate_phenotypes/src/TripalCultivateValidator/TripalCultivatePhenotypesValidatorBase.php b/trpcultivate_phenotypes/src/TripalCultivateValidator/TripalCultivatePhenotypesValidatorBase.php index 24a0166e..ee9bf8dd 100644 --- a/trpcultivate_phenotypes/src/TripalCultivateValidator/TripalCultivatePhenotypesValidatorBase.php +++ b/trpcultivate_phenotypes/src/TripalCultivateValidator/TripalCultivatePhenotypesValidatorBase.php @@ -89,7 +89,7 @@ public function validateMetadata(array $form_values) { /** * {@inheritdoc} */ - public function validateFile(string $filename, int $fid) { + public function validateFile(int|null $fid) { $plugin_name = $this->getValidatorName(); throw new \Exception("Method validateFile() from base class called for $plugin_name. If this plugin wants to support this type of validation then they need to override it."); } diff --git a/trpcultivate_phenotypes/src/TripalCultivateValidator/TripalCultivatePhenotypesValidatorInterface.php b/trpcultivate_phenotypes/src/TripalCultivateValidator/TripalCultivatePhenotypesValidatorInterface.php index 363a3a89..959461bd 100644 --- a/trpcultivate_phenotypes/src/TripalCultivateValidator/TripalCultivatePhenotypesValidatorInterface.php +++ b/trpcultivate_phenotypes/src/TripalCultivateValidator/TripalCultivatePhenotypesValidatorInterface.php @@ -69,8 +69,6 @@ public function validateMetadata(array $form_values); * This should validate the file object (e.g. it exists, is readable) but * should not validate the contents in any way. * - * @param array $filename - * The full path and filename with extension of the file to validate. * @param int $fid * The file ID of the file object. * @@ -86,7 +84,7 @@ public function validateMetadata(array $form_values); * then this array should contain a key indicating what failed, and the * resulting value from checking its mime-type/extension. */ - public function validateFile(string $filename, int $fid); + public function validateFile(int|null $fid); /** * Validates rows within the data file submitted to an importer. diff --git a/trpcultivate_phenotypes/tests/src/Kernel/Validators/ValidatorBaseTest.php b/trpcultivate_phenotypes/tests/src/Kernel/Validators/ValidatorBaseTest.php index d7593512..b8253100 100644 --- a/trpcultivate_phenotypes/tests/src/Kernel/Validators/ValidatorBaseTest.php +++ b/trpcultivate_phenotypes/tests/src/Kernel/Validators/ValidatorBaseTest.php @@ -285,9 +285,8 @@ public function testValidatorValidateMethods() { $exception_caught = NULL; $exception_message = NULL; try { - $filename = 'public://does_not_exist.txt'; $fid = 123; - $instance->validateFile($filename, $fid); + $instance->validateFile($fid); } catch (\Exception $e) { $exception_caught = TRUE; diff --git a/trpcultivate_phenotypes/tests/src/Kernel/Validators/ValidatorValidDataFileTest.php b/trpcultivate_phenotypes/tests/src/Kernel/Validators/ValidatorValidDataFileTest.php index 4d4eaa8b..62aa8338 100644 --- a/trpcultivate_phenotypes/tests/src/Kernel/Validators/ValidatorValidDataFileTest.php +++ b/trpcultivate_phenotypes/tests/src/Kernel/Validators/ValidatorValidDataFileTest.php @@ -20,10 +20,6 @@ class ValidatorValidDataFileTest extends ChadoTestKernelBase { * * Each element is keyed by short scenario description and the value is an * array with the following keys: - * - 'test_param': a list of parameter combinations that will be passed to the - * validateRawRow() method (ie. a parameter for each `filename` and `fid`). - * - 'test_file': an array of file properties to be used in file creation. - * Keyed by: * - 'filename': the name of the file (should match filename in test_param). * - 'fid': the file id number. * - 'mime': the file MIME type. @@ -73,7 +69,6 @@ protected function setUp(): void { // Set the supported mime types for this test. $this->validator_instance->setSupportedMimeTypes([ 'tsv', - 'txt', ]); $test_file_scenario = [ @@ -96,15 +91,6 @@ protected function setUp(): void { 'filesize' => 0, ], - // An alternative file type. - 'file-alternative' => [ - 'ext' => 'txt', - 'mime' => 'text/plain', - 'content' => [ - 'string' => implode("\t", ['Header 1', 'Header 2', 'Header 3']), - ], - ], - // Not valid file. 'file-image' => [ 'ext' => 'png', @@ -136,6 +122,24 @@ protected function setUp(): void { ], 'permissions' => 'none', ], + + // Test a txt file type. + 'file-alternative' => [ + 'ext' => 'txt', + 'mime' => 'text/plain', + 'content' => [ + 'string' => implode("\t", ['Header 1', 'Header 2', 'Header 3']), + ], + ], + + // A manaaged file with file object record but refereced file is missing. + 'file-missing' => [ + 'ext' => 'tsv', + 'mime' => 'text/tab-separated-values', + 'content' => [ + 'string' => implode("\t", ['Header 1', 'Header 2', 'Header 3']), + ], + ], ]; // Array to hold the file id and file uri of the generated files @@ -152,105 +156,48 @@ protected function setUp(): void { // Reference relevant file properties that will be used // to indicate attributes of the file that failed the validation. $file_id = $file->id(); - $file_uri = $file->getFileUri(); $file_mime_type = $file->getMimeType(); $file_filename = $file->getFileName(); $file_extension = pathinfo($file_filename, PATHINFO_EXTENSION); // Create a test scenario and attach the file properties. $test_file_param[$test_scenario] = [ - 'test_param' => [ - // Test input by filename. - 'filename' => [$file_uri, NULL], - // Test input by file id (fid). - 'fid' => ['', $file_id], - ], - // Test file properties. - 'test_file' => [ - 'filename' => $file_filename, - 'fid' => $file_id, - 'mime' => $file_mime_type, - 'extension' => $file_extension, - ], + 'filename' => $file_filename, + 'fid' => $file_id, + 'mime' => $file_mime_type, + 'extension' => $file_extension, ]; - } - - // Create an unmanaged file copy of the valid test file scenario - // to use as input for validating a file without a file id (unmanaged file). - $file_valid_uri = $test_file_param['file-valid']['test_param']['filename'][0]; - // Rename the duplicate copy as unmanaged_test_data_file. - $file_unmanaged_uri = str_replace('test_data_file', 'unmanaged_test_data_file', $file_valid_uri); - // Move a copy of the file and rename it using the new filename. - copy($file_valid_uri, $file_unmanaged_uri); - - // Create test scenario using the filename of this unmanaged file. - $test_file_param['file-unmanaged'] = [ - 'test_param' => [ - 'filename' => [$file_unmanaged_uri, NULL], - ], - - 'test_file' => [ - 'filename' => $file_unmanaged_uri, - 'fid' => NULL, - 'mime' => 'text/tab-separated-values', - 'extension' => 'tsv', - ], - ]; - // Create test scenario where the filename is an empty string value. - $test_file_param['invalid-filename-parameter'] = [ - 'test_param' => [ - 'filename' => ['', NULL], - ], + // With the file for file-missing scenario created, delete the file using + // PHP delete file function to maintain the file object record. + if ($test_scenario == 'file-missing') { + $missing_file_uri = $file->getFileUri(); + unlink($missing_file_uri); + } + } - 'test_file' => [ - 'filename' => '', - 'fid' => NULL, - 'mime' => '', - 'extension' => '', - ], + // Create test scenario where the file id is null. + $test_file_param['null-fid-parameter'] = [ + 'filename' => '', + 'fid' => NULL, + 'mime' => '', + 'extension' => '', ]; // Create test scenario where the fid is zero. - $test_file_param['invalid-fid-parameter'] = [ - 'test_param' => [ - 'fid' => ['', 0], - ], - - 'test_file' => [ - 'filename' => '', - 'fid' => 0, - 'mime' => '', - 'extension' => '', - ], - ]; - - // Create test scenario where the filename does not exist. - $test_file_param['non-existent-filename'] = [ - 'test_param' => [ - 'filename' => ['public://non-existent.tsv', NULL], - ], - - 'test_file' => [ - 'filename' => 'public://non-existent.tsv', - 'fid' => NULL, - 'mime' => '', - 'extension' => '', - ], + $test_file_param['zero-fid-parameter'] = [ + 'filename' => '', + 'fid' => 0, + 'mime' => '', + 'extension' => '', ]; // Create test scenario where the file id does not exist. $test_file_param['non-existent-fid'] = [ - 'test_param' => [ - 'fid' => ['', 999], - ], - - 'test_file' => [ - 'filename' => '', - 'fid' => 999, - 'mime' => '', - 'extension' => '', - ], + 'filename' => '', + 'fid' => 999, + 'mime' => '', + 'extension' => '', ]; // Set the property to all test file input scenarios. @@ -264,11 +211,10 @@ protected function setUp(): void { * Each scenario/element is an array with the following values: * - A human-readable short description of the test scenario. * - Test scenario array key set in the $test_files property. The key - * corresponds to what parameters are provided to validation. - * - Expected validation response with the following keys. An empty array - * value indicates test for the parameter has not been performed. - * - 'filename': using filename (first parameter). - * - 'fid': using fid (file id, second parameter). + * corresponds to file properties created in setUp() method above. + * - Expected validation response with the following keys. + * - 'validation_response': the response array returned by the validator + * that contains the case title and valid status information. * - 'failed_items_key': a list of keys that reference file attributes * that cause validation to fail: * - 'filename': filename. @@ -279,63 +225,42 @@ protected function setUp(): void { public function provideFileForDataFileValidator() { return [ - // #0: Test an invalid empty string value to the filename parameter. + // #0: Test a null file id number. [ - 'invalid filename parameter', - 'invalid-filename-parameter', + 'null fid parameter', + 'null-fid-parameter', [ - 'filename' => [ - 'case' => 'Filename is empty string', + 'validation_response' => [ + 'case' => 'Invalid file id number', 'valid' => FALSE, ], - 'fid' => [], 'failed_items_key' => [ - 'filename', 'fid', ], ], ], - // #1: Test an invalid zero value to the fid parameter. + // #1: Test a zero file id number. [ - 'invalid fid parameter', - 'invalid-fid-parameter', + 'zero fid parameter', + 'zero-fid-parameter', [ - 'filename' => [], - 'fid' => [ + 'validation_response' => [ 'case' => 'Invalid file id number', 'valid' => FALSE, ], 'failed_items_key' => [ - 'filename', 'fid', ], ], ], - // #2: Test non-existent filename. + // #2: Test non-existent file id number. [ - 'filename does not exist', - 'non-existent-filename', - [ - 'filename' => [ - 'case' => 'Filename failed to load a file object', - 'valid' => FALSE, - ], - 'fid' => [], - 'failed_items_key' => [ - 'filename', - ], - ], - ], - - // #3: Test non-existent file id number. - [ - 'file id number does not exist', - 'non-existent-fid', + 'file id number does not exist', + 'non-existent-fid', [ - 'filename' => [], - 'fid' => [ + 'validation_response' => [ 'case' => 'File id failed to load a file object', 'valid' => FALSE, ], @@ -343,34 +268,27 @@ public function provideFileForDataFileValidator() { 'fid', ], ], - ], + ], - // #4: Test unmanaged filename - file does not exist in file system. + // #3: Test a valid file - primary type (tsv). [ - 'unmanaged file', - 'file-unmanaged', + 'valid tsv file', + 'file-valid', [ - 'filename' => [ - 'case' => 'Filename failed to load a file object', - 'valid' => FALSE, - ], - 'fid' => [], - 'failed_items_key' => [ - 'filename', + 'validation_response' => [ + 'case' => 'Data file is valid', + 'valid' => TRUE, ], + 'failed_items_key' => [], ], ], - // #5: Test an empty file. + // #4: Test an empty file. [ 'file is empty', 'file-empty', [ - 'filename' => [ - 'case' => 'The file has no data and is an empty file', - 'valid' => FALSE, - ], - 'fid' => [ + 'validation_response' => [ 'case' => 'The file has no data and is an empty file', 'valid' => FALSE, ], @@ -381,16 +299,28 @@ public function provideFileForDataFileValidator() { ], ], - // #6: Test file that is not the right MIME type. + // #5: Test file that is not the right MIME type. [ 'incorrect mime type', 'file-image', [ - 'filename' => [ + 'validation_response' => [ 'case' => 'Unsupported file mime type and unsupported extension', 'valid' => FALSE, ], - 'fid' => [ + 'failed_items_key' => [ + 'mime', + 'extension', + ], + ], + ], + + // #6: Test txt file type. + [ + 'txt file type', + 'file-alternative', + [ + 'validation_response' => [ 'case' => 'Unsupported file mime type and unsupported extension', 'valid' => FALSE, ], @@ -406,11 +336,7 @@ public function provideFileForDataFileValidator() { 'pretentious file', 'file-pretend', [ - 'filename' => [ - 'case' => 'Unsupported file MIME type', - 'valid' => FALSE, - ], - 'fid' => [ + 'validation_response' => [ 'case' => 'Unsupported file MIME type', 'valid' => FALSE, ], @@ -421,50 +347,28 @@ public function provideFileForDataFileValidator() { ], ], - // #8. Test a valid file - primary type (tsv). + // #8: Test a locked file - cannot read a valid file. [ - 'valid tsv file', - 'file-valid', - [ - 'filename' => [ - 'case' => 'Data file is valid', - 'valid' => TRUE, - ], - 'fid' => [ - 'case' => 'Data file is valid', - 'valid' => TRUE, - ], - 'failed_items_key' => [], - ], - ], - - // #9. Test a valid file - alternative type (txt). - [ - 'valid txt file', - 'file-alternative', + 'file is locked', + 'file-locked', [ - 'filename' => [ - 'case' => 'Data file is valid', - 'valid' => TRUE, + 'validation_response' => [ + 'case' => 'Data file cannot be opened', + 'valid' => FALSE, ], - 'fid' => [ - 'case' => 'Data file is valid', - 'valid' => TRUE, + 'failed_items_key' => [ + 'filename', + 'fid', ], - 'failed_items_key' => [], ], ], - // #10: Test a locked file - cannot read a valid file. + // #9: Test a managed file id but the file is missing. [ - 'file is locked', - 'file-locked', + 'file is missing', + 'file-missing', [ - 'filename' => [ - 'case' => 'Data file cannot be opened', - 'valid' => FALSE, - ], - 'fid' => [ + 'validation_response' => [ 'case' => 'Data file cannot be opened', 'valid' => FALSE, ], @@ -477,33 +381,6 @@ public function provideFileForDataFileValidator() { ]; } - /** - * Test data file input validator case that throws an exception. - */ - public function testDataFileExceptionCase() { - // Assign a different name than the set filename of a valid file scenario. - $filename = 'not-the-filename.tsv'; - $fid = $this->test_files['file-valid']['test_file']['fid']; - - $exception_caught = FALSE; - $exception_message = ''; - - try { - $this->validator_instance->validateFile($filename, $fid); - } - catch (\Exception $e) { - $exception_caught = TRUE; - $exception_message = $e->getMessage(); - } - - $this->assertTrue($exception_caught, 'Data File validator expects an exception thrown when filenames do not match.'); - $this->assertStringContainsString( - $exception_message, - 'The filename provided does not match the filename set in the file object.', - 'The exception message thrown by data file validator filename mismatch case does not match excepted message' - ); - } - /** * Test data file input validator. * @@ -526,26 +403,27 @@ public function testDataFileExceptionCase() { */ public function testDataFileInput(string $scenario, string $test_file_key, array $expected) { $file_input = $this->test_files[$test_file_key]; + $fid = $file_input['fid']; - foreach ($file_input['test_param'] as $input_type => $test_param) { - [$filename, $fid] = $test_param; - $validation_status = $this->validator_instance->validateFile($filename, $fid); - - // Determine the actual failed items. - // - If the validation passed, the failed item is an empty array. - // - If failed, create the item key specified by the test scenario and set - // the value using the value of the same key in the test_files property. - $failed_items = []; - foreach ($expected['failed_items_key'] as $item) { - $failed_items[$item] = $file_input['test_file'][$item]; - } + $validation_status = $this->validator_instance->validateFile($fid); + + // Determine the actual failed items. + // - If the validation passed, the failed item is an empty array. + // - If failed, create the item key specified by the test scenario and set + // the value using the value of the same key in the test_files property. + $failed_items = []; + foreach ($expected['failed_items_key'] as $file_property) { + $failed_items[$file_property] = $file_input[$file_property]; + } - $expected[$input_type]['failedItems'] = ($validation_status['valid']) ? [] : $failed_items; + $expected['validation_response']['failedItems'] = ($validation_status['valid']) ? [] : $failed_items; - foreach ($validation_status as $key => $value) { - $this->assertEquals($value, $expected[$input_type][$key], - 'The validation status key ' . $key . ' with the parameter ' . $input_type . ', does not match expected in scenario: ' . $scenario); - } + foreach ($validation_status as $key => $value) { + $this->assertEquals( + $value, + $expected['validation_response'][$key], + 'The validation status key ' . $key . ' with the fid parameter ' . $fid . ', does not match expected in scenario: ' . $scenario + ); } }