Fixes to enable custom structmaps to be more expressive #1404

ross-spencer · 2019-04-11T07:22:17Z

Fixes to enable custom structmaps to be more expressive

Previously a user would need to submit a custom mets:structMap in
Archivematica using a FILEID attribute for a file path which would
be replaced during METS generation. The FILEID is only capable of
storing a limited set of values and so here we replace that
capability and ask users to use the CONTENTIDS attribute of a custom
structMap which can encode a larger range of values to ensure that
the final METS document remains valid.

Unit tests have been created to support maintenance of this feature
on an ongoing basis.

Validation of structMap input is completed during transfer and ingest
and this validation was previously been done by an external command
called xmllint. Given the opportunity to make the validation of METS
documents in Archivematica more consistent these calls in
Archivematica jobs to xmllint have been replaced with more idiomatic
calls to Python lxml validation methods.

Validation of mets:structMap files is now completed against
METS v1.12.

Other small fixes in this commit include the ability for the custom
structMap identifier to be updated dynamically where previously it
would always be structMap_2 which would be incompatible if other
automatically generated structMaps were included in the METS. Users
should note that they can also label a custom structMap with
something more meaningful to them using their own label and this will
not be replaced.

From the original commit history

This PR is separated into multiple commits and it is recommended to tackle them one by one for the code reviewer:

We implement new unit tests to demonstrate the existing capability.
We enhance the create_mets_v2.py code to support mode dynamic testing and add those tests.
Make sure FILEIDs are all handled.
We fix custom structmap numbering.
We add the fixes to embed_custom_structmap to enable more complex structures.
Add mets validation as structmap should be valid in its own right.
We do some minor refactoring to create_mets_v2.py to separate responsibilities somewhat.
We replace the verification shell script calling xmllint with Python.
We didn't correctly anticipate all tests required to accurately reflect a transfer layout, this commit deals with that.
Finally, we missed the sanitized names use-case where user submitted names might not anticipate AM's handling of that, so we deal with that here.

Feature: Draft documentation
Sample-data for testing can be found here: artefactual/archivematica-sampledata#52

Connected to archivematica/Issues#283
Connected to archivematica/Issues#633
Connected to archivematica/Issues#245
Connected to IISH#5
Connected to IISH#4

cole

This code looks good! A few questions inline.

I have one overall concern. If I understand ADR-0001 correctly, we'd like to (over time) remove most of the XML generation code from create_mets_v2, and replace it with metsrw usage. It seems like this PR does not advance that goal. It doesn't make that task too much harder, but it seems like if we're touching this script in a significant way, it's a good opportunity to move in that direction. I do understand though that metsrw has some serious issues, that may prevent it from being used at all here, so maybe we need to just understand that while we have that goal, we can't advance it here (I don't know the urgency of this change, etc).

cole · 2019-04-15T16:22:15Z

src/MCPClient/lib/clientScripts/verify_mets.py

+                return
+            if not os.path.isfile(mets_xsd):
+                raise (VerifyMETSException("METS asset is unavailable"))
+            xmlschema = etree.XMLSchema(etree.parse(mets_xsd))


Just to confirm, does metsrw not yet work for this case?

cole · 2019-04-15T16:26:15Z

src/MCPClient/tests/test_create_aip_mets.py

@@ -1,7 +1,9 @@
 # -*- coding: utf8
+from __future__ import unicode_literals


cole · 2019-04-15T16:32:40Z

src/MCPClient/tests/test_create_aip_mets.py

+        return sum([len(files) for _, dir_, files in os.walk(path)])
+
+    @staticmethod
+    def validate_mets(mets_xsd, mets_structmap):


Again, just to confirm, metsrw can't be used here?

cole · 2019-04-15T17:18:37Z

src/MCPClient/lib/clientScripts/create_mets_v2.py

+    that they don't have to anticipate the Archivematica normalization process.
+    """
+    return os.path.join(
+        "", *[sanitizeName(name.encode("utf8")) for name in path.split(os.path.sep)]


The idea here is that these files will already have been sanitized, so we're just matching up to the current state on the filesystem/database, correct?

Sorry i missed this @cole, yep! That's right. We'll have to keep this in mind when you complete your work on that microservice job.

cole · 2019-04-15T17:46:57Z

src/MCPClient/lib/clientScripts/create_mets_v2.py

-                tree.getroot()
-            )  # TDOD - not root to return, but sub element structMap
-            # print etree.tostring(root)
+            root = tree.getroot()


Just to confirm, does metsrw not yet work for this case?

ross-spencer · 2019-04-16T09:19:18Z

Hi @cole thanks for the quick input. For metsrw I investigated the possibility, and came up with this as the primary issue for not using it: archivematica/Issues#636 and getting to this stage in the PR used the majority of the budgeted time as much as I would love to work my way through it.

In prepping the way for metsrw:

We have removed a shell script calling xmllint so brought scripts into line using lxml.
We use the same validation call in the microservices as we do in the unit tests.
Created unit tests for the new feature to help with refactoring to use metsrw in future.

I approached it as taking one step forward.

cole · 2019-04-16T16:46:12Z

@ross-spencer fair enough! Moving create_mets_v2 to metsrw is a big job, maybe it's wishful thinking that we can tackle it incrementally. Thanks for the detailed issues you've created (archivematica/Issues#636 and related).

ross-spencer · 2019-04-16T17:20:30Z

@cole yeah, but I'm looking forward to making it happen though 🤓 . If there's an appetite to have lxml and metsrw mixing with each other for a few releases, then that's useful to know too.

Previously a user would need to submit a custom mets:structMap in Archivematica using a FILEID attribute for a file path which would be replaced during METS generation. The FILEID is only capable of storing a limited set of values and so here we replace that capability and ask users to use the CONTENTIDS attribute of a custom structMap which can encode a larger range of values to ensure that the final METS document remains valid. Unit tests have been created to support maintenance of this feature on an ongoing basis. Validation of structMap input is completed during transfer and ingest and this validation was previously been done by an external command called xmllint. Given the opportunity to make the validation of METS documents in Archivematica more consistent these calls in Archivematica jobs to xmllint have been replaced with more idiomatic calls to Python lxml validation methods. Validation of mets:structMap files is now completed against METS v1.12. Other small fixes in this commit include the ability for the custom structMap identifier to be updated dynamically where previously it would always be structMap_2 which would be incompatible if other automatically generated structMaps were included in the METS. Users should note that they can also label a custom structMap with something more meaningful to them using their own label and this will not be replaced.

ross-spencer force-pushed the dev/issue-283-structmap-fixes branch 9 times, most recently from d3bc39a to 5476b50 Compare April 12, 2019 21:55

ross-spencer mentioned this pull request Apr 13, 2019

Provide various custom structMap examples artefactual/archivematica-sampledata#52

Merged

ross-spencer force-pushed the dev/issue-283-structmap-fixes branch 2 times, most recently from 247309f to 51b3793 Compare April 13, 2019 18:01

ross-spencer changed the title ~~WIP: Custom structmap fixes~~ Fixes to enable custom structmaps to be more expressive Apr 15, 2019

ross-spencer requested a review from a team April 15, 2019 14:39

cole reviewed Apr 15, 2019

View reviewed changes

cole approved these changes Apr 16, 2019

View reviewed changes

ross-spencer force-pushed the dev/issue-283-structmap-fixes branch from 51b3793 to 2a539c5 Compare April 17, 2019 08:35

ross-spencer mentioned this pull request Apr 17, 2019

Problem: Archivematica core contains old METS.xsd archivematica/Issues#245

Closed

ross-spencer merged commit 2a539c5 into qa/1.x Apr 17, 2019

ross-spencer deleted the dev/issue-283-structmap-fixes branch April 17, 2019 08:47

ross-spencer mentioned this pull request Apr 17, 2019

Enable use of CONTENTIDS for nested paths in Struct Maps #1301

Closed

replaceafill mentioned this pull request Jan 27, 2020

Problem: Can't start a transfer if transfer name has diacritics archivematica/Issues#1051

Closed

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes to enable custom structmaps to be more expressive #1404

Fixes to enable custom structmaps to be more expressive #1404

ross-spencer commented Apr 11, 2019 •

edited

Loading

cole left a comment

cole Apr 15, 2019

cole Apr 15, 2019

cole Apr 15, 2019

cole Apr 15, 2019

ross-spencer Apr 17, 2019

cole Apr 15, 2019

ross-spencer commented Apr 16, 2019

cole commented Apr 16, 2019

ross-spencer commented Apr 16, 2019

		@@ -1,7 +1,9 @@
		# -*- coding: utf8
		from __future__ import unicode_literals

Fixes to enable custom structmaps to be more expressive #1404

Fixes to enable custom structmaps to be more expressive #1404

Conversation

ross-spencer commented Apr 11, 2019 • edited Loading

cole left a comment

Choose a reason for hiding this comment

cole Apr 15, 2019

Choose a reason for hiding this comment

cole Apr 15, 2019

Choose a reason for hiding this comment

cole Apr 15, 2019

Choose a reason for hiding this comment

cole Apr 15, 2019

Choose a reason for hiding this comment

ross-spencer Apr 17, 2019

Choose a reason for hiding this comment

cole Apr 15, 2019

Choose a reason for hiding this comment

ross-spencer commented Apr 16, 2019

cole commented Apr 16, 2019

ross-spencer commented Apr 16, 2019

ross-spencer commented Apr 11, 2019 •

edited

Loading