You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
When an XmlParser import is given a zip file (parser_fields['import_file_path'] pointing at a .zip), the job extracts the zip into importer_unzip_path but XmlParser#file_paths and #metadata_paths then glob in the wrong directory, so the parser can't find the extracted XML files and the import fails (or finds no records).
March 2020, PR Generic XML Importer #142: XmlParser introduced with the flawed File.dirname glob. Zip uploads glob the upload staging dir. Broken from day one.
September 2021, PR Fix file paths for Zip CSV imports #261: Base class real_import_file_path gains the importer_unzip_path if file? && zip? branch. XmlParser zip uploads now glob the parent of the unzip dir. Different broken location.
Trace
Post-extraction state:
parser_fields['import_file_path'] = /tmp/imports/.../bundle.zip (unchanged from upload).
importer_unzip_path = /tmp/imports/.../import_{path_string}/ (contains the extracted content).
ApplicationParser#real_import_file_path returns importer_unzip_path when file? && zip?.
XmlParser#import_file_path memoizes that directory path.
Then at app/parsers/bulkrax/xml_parser.rb lines 82-90:
file? (in app/models/concerns/bulkrax/importer_exporter_behavior.rb:47) checks parser_fields['import_file_path'] — the raw zip path, which is a file → true.
Line 85 takes the if branch → File.dirname(import_file_path) = File.dirname(importer_unzip_path) = the parent of the unzip dir.
Glob runs against the parent, not the unzip dir itself — misses the extracted XML files.
Reproduction
Upload a zip containing metadata.xml and data files via the legacy importers UI with Parser = Bulkrax::XmlParser. The import will run but find zero metadata records or files.
Why this hasn't been caught
No specs cover XmlParser with a zip upload (spec/parsers/bulkrax/xml_parser_spec.rb doesn't exercise zip paths). No zip fixtures for XML exist. The locale hint at config/locales/bulkrax.en.yml:395 advertises XML zip support, but it doesn't actually work.
Proposed fix directions
Two options, either is viable:
Update ImporterJob#unzip_imported_file to set parser_fields['import_file_path'] to the extracted location after unzipping, so file?/zip? checks see the post-extraction state. More invasive; touches the core flow.
Override XmlParser#file_paths / #metadata_paths (or #real_import_file_path) to resolve relative to importer_unzip_path directly when a zip was uploaded. Localized to XmlParser.
Pre-existing bug (at least since PR #261 in Sep 2021); the current unzip-fix branch preserves the status quo rather than fixing it in scope.
Acceptance criteria
Uploading a zip containing XML metadata results in the XML files being found and imported.
Spec coverage: spec/parsers/bulkrax/xml_parser_spec.rb exercises the zip-upload flow end-to-end with a real fixture zip.
Summary
When an
XmlParserimport is given a zip file (parser_fields['import_file_path']pointing at a.zip), the job extracts the zip intoimporter_unzip_pathbutXmlParser#file_pathsand#metadata_pathsthen glob in the wrong directory, so the parser can't find the extracted XML files and the import fails (or finds no records).Trace
Post-extraction state:
parser_fields['import_file_path']=/tmp/imports/.../bundle.zip(unchanged from upload).importer_unzip_path=/tmp/imports/.../import_{path_string}/(contains the extracted content).ApplicationParser#real_import_file_pathreturnsimporter_unzip_pathwhenfile? && zip?.XmlParser#import_file_pathmemoizes that directory path.Then at
app/parsers/bulkrax/xml_parser.rblines 82-90:file?(inapp/models/concerns/bulkrax/importer_exporter_behavior.rb:47) checksparser_fields['import_file_path']— the raw zip path, which is a file → true.ifbranch →File.dirname(import_file_path)=File.dirname(importer_unzip_path)= the parent of the unzip dir.Reproduction
Upload a zip containing
metadata.xmland data files via the legacy importers UI withParser = Bulkrax::XmlParser. The import will run but find zero metadata records or files.Why this hasn't been caught
No specs cover
XmlParserwith a zip upload (spec/parsers/bulkrax/xml_parser_spec.rbdoesn't exercise zip paths). No zip fixtures for XML exist. The locale hint atconfig/locales/bulkrax.en.yml:395advertises XML zip support, but it doesn't actually work.Proposed fix directions
Two options, either is viable:
Update
ImporterJob#unzip_imported_fileto setparser_fields['import_file_path']to the extracted location after unzipping, sofile?/zip?checks see the post-extraction state. More invasive; touches the core flow.Override
XmlParser#file_paths/#metadata_paths(or#real_import_file_path) to resolve relative toimporter_unzip_pathdirectly when a zip was uploaded. Localized toXmlParser.Pre-existing bug (at least since PR #261 in Sep 2021); the current unzip-fix branch preserves the status quo rather than fixing it in scope.
Acceptance criteria
spec/parsers/bulkrax/xml_parser_spec.rbexercises the zip-upload flow end-to-end with a real fixture zip.