Issue 495: Make extensions of page image files configurable in CSV Book and Newspaper toolchains

[mjordan]

Github issue: #495

What does this Pull Request do?

Makes extensions of page image files configurable in CSV Book and Newspaper toolchains.

What's new?

In the CsvBooks.php and CsvNewspapers.php filegetter classes, replaced the hard-coded $allowed_file_extensions_for_OBJ property with a configuration option. The default values (if the option is not present in the .ini file) is the same as previously, to prevent backward breakage.

How should this be tested?

This is a 'testable' change. I have included to new test classes, tests/filegetters/CsvBooksTest.php and tests/filegetters/CsvNewspapersTest.php. In the master branch, running ./vendor/bin/phpunit results in:

OK, but incomplete, skipped, or risky tests!
Tests: 58, Assertions: 84, Skipped: 2.

In the issue-495 branch, running PHPUnit results in, showing two new tests and four new assertions:

OK, but incomplete, skipped, or risky tests!
Tests: 60, Assertions: 88, Skipped: 2.

In all new tests, I am testing for both the default value of $allowed_file_extensions_for_OBJ and a configured value.

Additional Notes

• Does this change require documentation to be updated? Yes. The new configuration option should be documented on the two toolchain pages on the wiki. Here is an example of using the new option:

FILE_GETTER
allowed_file_extensions_for_OBJ[] = jpg
allowed_file_extensions_for_OBJ[] = tif

• Does this change add any new dependencies? No
• Could this change impact execution of existing code? Yes

Interested parties

@MarcusBarnes @bondjimbond

[mjordan]

@MarcusBarnes @bondjimbond I have not included test configuration and data for this PR since it is completely testable with PHPUnit. Is this sufficient to merge, or do you want test data you can smoketest with?

[bondjimbond]

If you think PHPUnit is sufficient for testing, I'm happy with it.

[mjordan]

@MarcusBarnes do you have an opinion on this?

[MarcusBarnes]

@mjordan I've looked over the code. Since the change still provides the previous default values to prevent backward breakage and there are tests, I'm OK with merging.

[MarcusBarnes]

I'm happy to push the happy green "Merge pull request" button. Thumbs up, thumbs down?

[mjordan]

Thank you both!