Handle spaces in file names in the import scripts

[dltj]

No description provided.

[demiankatz]

Thanks, @dltj! See below for a couple of minor stylistic suggestions, and a question that may reveal my general ignorance. ;-)

[demiankatz]

This seems to have gotten indented an extra two spaces for some reason...

[demiankatz]

Also, since this script is largely the work of others and my bash expertise is limited, I may just be confused... but I thought the purpose of some of this code was to pass multiple matching files to the import script at once in a space-delimited list. How do we get around confusion between space-delimited files and files containing spaces? Is the find command smart enough to quote things appropriately for us? Do you need me to try to set up any edge case test scenarios to confirm proper functionality, or have you already done so?

[dltj]

The changes are mostly dealing with the output of the find command for filenames that have spaces in them. If one puts a filename with spaces (properly escaped) on the command line itself, it works okay. The -print0 option from the find command separates file entries by null characters...that was already there. The rest of the changes deal with handling that correctly. The script had some notable pitfalls when handling filenames with spaces...notably the use of the for control flow command. Needing to double-quote variable names that deal with values with spaces is also a notable pitfall.

This is working in production for me now. I'm not sure how we'd set up a test with 'bash' and hook it up to the PHP test harness.

[demiankatz]

If we wanted an automated test of this, we could create a directory containing MARC records with unusual filenames, and then run a batch import of this directory as part of the build.xml MARC indexing process. Then we would need a Mink test to confirm that the records actually exist in the index. A bit roundabout and complicated, and very possibly not worth the effort, but that's how I would approach it if I had to. (I'm perfectly content to just do some manual testing of my own when time permits, unless you're eager to automate something in the meantime).

[dltj]

Let's go the manual testing route, and I'll post a message to Slack to get others to look at this. Normally I'd be up for learning new tools, but it is feeling like such a crunch time at the moment.

[demiankatz]

Okay, I tried some manual testing, and nothing seems to be working for me... but maybe I'm operating on incorrect assumptions. Here's what I did:

1.) I created two test directories under $VUFIND_LOCAL_DIR/harvest: $VUFIND_LOCAL_DIR/harvest/marc and $VUFIND_LOCAL_DIR/harvest/marc records, so I could run tests with or without a space in the harvest directory name.

2.) I copied the same two sample records from the test data into both of these directories. I copied testbug2.mrc as is (for a "normal" filename example) and I copied journals.mrc as journal data.mrc to introduce a filename containing a space.

Here's what happens when I try to run the batch import on these two directories:

"marc" example:

dkatz@dkatz-VirtualBox:/usr/local/vufind$ harvest/batch-import-marc.sh marc
Now Importing: /testbug2.mrc
DEBUG [main] (Boot.java:713) - Adding jars files in directory: /usr/local/vufind/import/lib_local
DEBUG [main] (Boot.java:713) - Adding jars files in directory: /usr/local/vufind/solr/vendor/modules/analysis-extras/lib
 INFO [main] (ValueIndexerFactory.java:119) - Using directory: /usr/local/vufind/import/index_java as location of java sources
 INFO [main] (PropertyUtils.java:337) - Opening file (instead of 1 other options): /usr/local/vufind/local/import/import.properties
DEBUG [main] (SolrCoreLoader.java:48) - Pinging Solr at URL:  http://localhost:8983/solr/biblio/admin/ping
DEBUG [main] (SolrCoreLoader.java:65) -     ...
DEBUG [main] (SolrCoreLoader.java:54) -       "status":"OK"
DEBUG [main] (SolrCoreLoader.java:96) - Found Solrj class org.apache.solr.client.solrj.impl.HttpSolrClient$Builder
 INFO [main] (IndexDriver.java:166) - Reading and compiling index specifications: marc.properties, marc_local.properties
 INFO [main] (IndexDriver.java:257) - Opening index spec file: /usr/local/vufind/import/marc.properties
 INFO [main] (IndexDriver.java:257) - Opening index spec file: /usr/local/vufind/import/marc_local.properties
 INFO [main] (IndexDriver.java:100) - Opening input files: [/testbug2.mrc]
FATAL [main] (Boot.java:240) - ERROR: Error while invoking main method in specified class: org.solrmarc.driver.IndexDriver
java.lang.IllegalArgumentException: /testbug2.mrc (No such file or directory)
	at org.solrmarc.driver.IndexDriver.configureReader(IndexDriver.java:235)
	at org.solrmarc.driver.IndexDriver.execute(IndexDriver.java:101)
	at org.solrmarc.driver.IndexDriver.main(IndexDriver.java:72)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.solrmarc.driver.Boot.invokeMain(Boot.java:229)
	at org.solrmarc.driver.ConfigDriver.main(ConfigDriver.java:136)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.solrmarc.driver.Boot.invokeMain(Boot.java:229)
	at org.solrmarc.driver.Boot.main(Boot.java:79)
Caused by: java.io.FileNotFoundException: /testbug2.mrc (No such file or directory)
	at java.base/java.io.FileInputStream.open0(Native Method)
	at java.base/java.io.FileInputStream.open(FileInputStream.java:213)
	at java.base/java.io.FileInputStream.<init>(FileInputStream.java:152)
	at java.base/java.io.FileInputStream.<init>(FileInputStream.java:106)
	at org.marc4j.MarcReaderFactory.makeReader(MarcReaderFactory.java:65)
	at org.marc4j.MarcReaderFactory.makeReader(MarcReaderFactory.java:43)
	at org.solrmarc.driver.IndexDriver.configureReader(IndexDriver.java:231)
	... 10 more

"marc records" example:

dkatz@dkatz-VirtualBox:/usr/local/vufind$ harvest/batch-import-marc.sh "marc records"
harvest/batch-import-marc.sh: line 94: [: /usr/local/vufind/local/harvest/marc: binary operator expected
harvest/batch-import-marc.sh: line 103: [: /usr/local/vufind/local/harvest/marc: binary operator expected
harvest/batch-import-marc.sh: line 110: [: /usr/local/vufind/local/harvest/marc: binary operator expected
find: ‘records’: No such file or directory
harvest/batch-import-marc.sh: line 131: /usr/local/vufind/local/harvest/marc records/log/testbug2.mrc.log: No such file or directory
harvest/batch-import-marc.sh: line 136: /usr/local/vufind/local/harvest/marc records/log/testbug2.mrc.log: No such file or directory
Now Importing: /testbug2.mrc

Are you seeing different results?

Also, I notice that you have targeted the release-10.1 branch here; if you feel strongly that this is a bug fix, I'm not entirely opposed to going forward on that path... but since the changes are significant, maybe it would be more conservative to target this against dev for inclusion in release 11.0.