Add logical metadata file validation, fixes #98 #101
Conversation
djjuhasz
commented
Dec 17, 2024
djjuhasz
commented
- Add the logical metadata file to the expected SIP structure for AIP type SIPs
- Validate the logical metadata file against the PREMIS v3 XSD
- Move the logical metadata file to the "metadata/" directory when transforming the SIP for delivery to preservation
Refs #98 Add the logical metadata file to the expected SIP structure for the AIP SIP types. Update activities related to SIP structure: identify SIP, validate files, validate structure, and verify manifest.
djjuhasz
changed the title
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #101 +/- ##
==========================================
+ Coverage 61.22% 61.86% +0.64%
==========================================
Files 32 35 +3
Lines 2174 2347 +173
==========================================
+ Hits 1331 1452 +121
- Misses 742 780 +38
- Partials 101 115 +14 ☔ View full report in Codecov by Sentry. |
djjuhasz
force-pushed
the
dev/issue-98-verify-logical-md-file-exists
branch
2 times, most recently
from
2b01003 to
9e9f87c
Compare
| }, | ||
| { | ||
| name: "Returns a file not found failure", | ||
| validator: newFakeValidator().WithErr(errors.New("file not found")), |
There was a problem hiding this comment.
What are the pros and cons of having a mock object return a file not found error versus testing with a non-existent file?
There was a problem hiding this comment.
@mcantelon I think the main pro of using a mock validator is that the tests are not dependent on xmllint being installed in the test environment. Unfortunately I negated that advantage by using the real "xmllint" validator in the first test, and sure enough that test failed in CI until I added xmllint to the CI runner. 🤔 The mock validator is also faster (though I'm sure the difference isn't noticeable in these small test cases), and allows more control over the return values.
I think the main con of using a mock validator is if there is some integration issue between the ValidatePREMIS activity and xmlvalidate.XMLLintValidator then it won't be caught by these unit tests. Ideally integration issues would be caught by integration tests, but we don't have any integration tests for this project. :(
There was a problem hiding this comment.
Good points! Yeah, integration tests definitely seem like they'd be handy (but also potentially a lot of work to create/maintain at this stage).
There was a problem hiding this comment.
I've decided to remove the xmllint dependency in the first test here. The deciding factor for me is that the tests should not fail if a developer runs them locally without xmllint installed.
Refs #98. Add an activity to validate the logical metadata PREMIS XML file against a local copy of the PREMIS v3 XSD. To avoid downloading the PREMIS XSD every time the activity runs I've embedded a local copy of the XSD in the worker Go binary, and write the contents to a file on disk so it can be loaded by xmllint.
Refs #98. Move the logical metadata file to the metadata directory when transforming the SIP.
djjuhasz
force-pushed
the
dev/issue-98-verify-logical-md-file-exists
branch
from
978519f to
3d770f1
Compare
