Add support for validation of XML submission files #2437
Conversation
sbelsk
force-pushed
the
submission-xml-files-validation
branch
3 times, most recently
from
6a60f38
to
c5431c2
Compare
| if (!$xml->schemaValidate($schema_file)) { | ||
| $validation_errors = libxml_get_errors(); | ||
| foreach ($validation_errors as $error) { | ||
| if ($error->level === LIBXML_ERR_ERROR || $error->level === LIBXML_ERR_FATAL) { |
There was a problem hiding this comment.
This line is what's causing the local testing of mine to fail. If I remove the LIBXML_ERR_ERROR part of the check, the submissions are successful.
My submission is from CTest 3.22.1 while the testing is 3.25.1. Otherwise the configuration files seem to be the same structure. If I submit via CTest, it fails, but copying the content over to the Docker machine and running the manual validation passes.
@williamjallen, I'm happy to demonstrate if you'd like to see my setup
There was a problem hiding this comment.
I'm unable to replicate the issue above on further testing. I'm not quite sure what happened
Forgot to reintroduce the environment variable, failures are still around. Sorry!
When attempting to validate an HTTP submission use the `storage_path` helper to turn the filename which starts at the inproess folder, `inprocess/<>.xml`, to a full path. This should not affect the path given directly to the submission:validate artisan command.
josephsnyder
force-pushed
the
submission-xml-files-validation
branch
from
c5431c2
to
d94a42b
Compare
| @@ -168,10 +169,30 @@ function ctest_parse($filehandle, $projectid, $expected_md5 = '') | |||
| } | |||
|
|
|||
| // Figure out what type of XML file this is. | |||
| $xml_info = SubmissionUtils::get_xml_type($filehandle); | |||
| try { | |||
| $xml_info = SubmissionUtils::get_xml_type($filehandle, $filename); | |||
There was a problem hiding this comment.
Given that the path passed to validate_xml() had to be modified to storage_path("app/".$filename), should this path being passed to get_xml_type() be changed as well? If so, I think these two are the only usages of the $filename function argument, so better yet, modify the value being passed to ctest_parse() from app/Jobs/ProcessSubmission.php.
There was a problem hiding this comment.
@sbelsk, the $xml_file variable within the get_xml_type only seems to be used in the error conditions at the end. if no file type and schema file are found. The content reading is all done using the $filehandle object. I'm not sure if $filehandle will ever not be valid yet.
There was a problem hiding this comment.
Makes sense. Might still be worth changing it for the logging in lines 191-193 ctest_parse() in case the XML validation fails.
This is a follow up to the submission XML schemas along with the validation artisan command (see #2335) that were recently introduced. The changes in this PR add the option to use this validation process on incoming submission files, although this option is disabled by default.
A test has been added to verify these changes, and the docs have been updated to reflect the new configuration option.