Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Work on #451. #454

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Work on #451. #454

wants to merge 2 commits into from

Conversation

mjordan
Copy link
Collaborator

@mjordan mjordan commented Feb 20, 2018

Github issue: #451

What does this Pull Request do?

Adds a metadata manipulator that appends an element containing a UUID to metadata XML created by the Templated metadata parser.

What's new?

A new metadata manipulator. It takes a Twig template like

<identifier type="uuid">{{ UUID }}</identifier>

and appends it to the output of the Templated metadata parser, like this:

<?xml version="1.0"?>
<mods xmlns="http://www.loc.gov/mods/v3" xmlns:mods="http://www.loc.gov/mods/v3" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xlink="http://www.w3.org/1999/xlink">
  <titleInfo>
     <title>Small boats in Havana Harbour on a sunney day</title>
  </titleInfo>
<identifier type="uuid">fb677095-2eb5-4acf-b310-ad8c3d0491cd</identifier>
</mods>

How should this be tested?

This PR has a PHPUnit test but should also get a smoke test.

To run PHPUnit tests, in the MIK directory, run phpunit. You should get Tests: 57, Assertions: 88, Skipped: 1.

To perform a smoketest:

  1. Unzip the attached file, adjust the paths to in the .ini as required by your system
  2. check out the issue-451 branch
  3. run composer dump-autoload
  4. run ./mik -c issue-451.ini

Look at the XML files generated by MIK. They all should have a UUID element at the bottom.

issue-451.zip

Additional Notes

  • Does this change require documentation to be updated?

Yes, a wiki entry for this manipulator is at https://github.com/MarcusBarnes/mik/wiki/Metadata-manipulator:-AddUuidToTemplated.

A note about this manipulator should also be added to https://github.com/MarcusBarnes/mik/wiki/Cookbook:-Templated-Metadata-Parser.

Interested parties

@MarcusBarnes @bondjimbond

@mjordan
Copy link
Collaborator Author

mjordan commented Apr 16, 2018

Bump on this, so it doesn't get too far out of date.

@MarcusBarnes
Copy link
Owner

MarcusBarnes commented Sep 14, 2018

@mjordan I just ran a fresh install of MIK and attempted to run the unit tests. When running the tests against this pull-request I get the following error 46 times:

Error: An iterator cannot be used with foreach by reference

This does not happen when I run PHPUnit against the master branch (as of commit 2201280). @bondjimbond Are you able to confirm this PHPUnit behaviour? I'd like to confirm that this is not just an issue related to my local development environment. I get the same result if I use the version included in the MIK vendor folder and also another version I have on my system. (To run the PHPUnit tests in MIK, make sure you're in the MIK folder cd MIK/, then run php vendor/bin/phpunit).

@bondjimbond
Copy link
Collaborator

@MarcusBarnes Sorry for taking so long to respond to this. In this branch I get the same errors you do when running phpunit. In the master branch and others I don't get the errors.

@MarcusBarnes
Copy link
Owner

@bondjimbond Thanks for confirming that you see the same behaviour. I'd like to address the errors we're seeing when running the tests before merging.

@mjordan
Copy link
Collaborator Author

mjordan commented Oct 31, 2018

I'm wondering if we're hitting a PHP7 thing here.... I've seen some changes in behavior in 7.2 for example that have broken some things in MIK.

@bondjimbond
Copy link
Collaborator

@mjordan I'm running PHP 7.1.16.

@mjordan
Copy link
Collaborator Author

mjordan commented Oct 31, 2018

OK, I'll test again with 7.2.10, which is what I'm currently running.

@mjordan
Copy link
Collaborator Author

mjordan commented Nov 11, 2018

Running this now on a clean clone of MIK and the tests fail as well, even in the master branch:

PHP Warning:  is_dir() expects parameter 1 to be a valid path, object given in /usr/share/php/PHPUnit/Runner/BaseTestRunner.php on line 56
PHP Recoverable fatal error:  Object of class PHPUnit\Framework\TestSuite could not be converted to string in /usr/share/php/PHPUnit/Runner/StandardTestSuiteLoader.php on line 32

@mjordan
Copy link
Collaborator Author

mjordan commented Nov 11, 2018

Hm... is the error you guys are getting the same one reported at #465 (comment) ? Mine is:

git checkout issue-451
Switched to branch 'issue-451'
Your branch is up to date with 'origin/issue-451'.
mark@mark-ThinkPad-X1-Carbon-6th:/tmp/mik$ ./mik -c issue-451.ini 
Commencing MIK.
PHP Fatal error:  Uncaught Error: An iterator cannot be used with foreach by reference in /tmp/mik/src/fetchers/Csv.php:93
Stack trace:
#0 /tmp/mik/src/fetchers/Csv.php(128): mik\fetchers\Csv->getRecords()
#1 /tmp/mik/mik(131): mik\fetchers\Csv->getNumRecs()
#2 {main}
  thrown in /tmp/mik/src/fetchers/Csv.php on line 93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants