Repository: ocr-d/core
Issue: workspace rename-group should also rename file IDs and file names if possible

In the current implementation, `ocrd workspace rename-group` does not rename the files themselves (and their `mets:file/@ID`), it only moves them from one directory (and `mets:fileGrp/@USE`) to another and redirects AlternativeImages.

This is not ideal, because most processors now use `ocrd_utils.make_file_id` for output file names (to avoid clashes and make file names more predictable and independent of `--page-id` runtime settings). But that function will not work with such files, because they no longer contain the fileGrp as part of their name.

Changed files:
ocrd_models/ocrd_models/ocrd_file.py, ocrd_models/ocrd_models/ocrd_mets.py, tests/test_workspace.py, tests/model/test_ocrd_file.py, ocrd/ocrd/workspace.py, tests/model/test_ocrd_mets.py

diff --git a/ocrd/ocrd/workspace.py b/ocrd/ocrd/workspace.py
index 0b69f2a5..51c46c6b 100644
--- a/ocrd/ocrd/workspace.py
+++ b/ocrd/ocrd/workspace.py
@@ -276,12 +276,24 @@ class Workspace():
             url_replacements = {}
             log.info("Moving files")
             for mets_file in self.mets.find_files(fileGrp=old, local_only=True):
-                new_url = sub(r'^%s/' % old, '%s/' % new, mets_file.url)
+                new_url = old_url = mets_file.url
+                # Directory part
+                new_url = sub(r'^%s/' % old, r'%s/' % new, new_url)
+                # File part
+                new_url = sub(r'/%s' % old, r'/%s' % new, new_url)
                 url_replacements[mets_file.url] = new_url
                 # move file from ``old`` to ``new``
                 move(mets_file.url, new_url)
                 # change the url of ``mets:file``
                 mets_file.url = new_url
+                # change the file ID and update structMap
+                # change the file ID and update structMap
+                new_id = sub(r'^%s' % old, r'%s' % new, mets_file.ID)
+                try:
+                    next(self.mets.find_files(ID=new_id))
+                    log.warning("ID %s already exists, not changing ID while renaming %s -> %s" % (new_id, old_url, new_url))
+                except StopIteration:
+                    mets_file.ID = new_id
             # change file paths in PAGE-XML imageFilename and filename attributes
             for page_file in self.mets.find_files(mimetype=MIMETYPE_PAGE, local_only=True):
                 log.info("Renaming file references in PAGE-XML %s" % page_file)
diff --git a/ocrd_models/ocrd_models/ocrd_file.py b/ocrd_models/ocrd_models/ocrd_file.py
index 4b9a912a..4637202b 100644
--- a/ocrd_models/ocrd_models/ocrd_file.py
+++ b/ocrd_models/ocrd_models/ocrd_file.py
@@ -110,7 +110,13 @@ class OcrdFile():
         """
         if ID is None:
             return
+        if self.mets is None:
+            raise Exception("OcrdFile %s has no member 'mets' pointing to parent OcrdMets" % self)
+        old_id = self.ID
         self._el.set('ID', ID)
+        # also update the references in the physical structmap
+        for pageId in self.mets.remove_physical_page_fptr(fileId=old_id):
+            self.pageId = pageId
 
     @property
     def pageId(self):
diff --git a/ocrd_models/ocrd_models/ocrd_mets.py b/ocrd_models/ocrd_models/ocrd_mets.py
index c4a05af7..9f1f184d 100644
--- a/ocrd_models/ocrd_models/ocrd_mets.py
+++ b/ocrd_models/ocrd_models/ocrd_mets.py
@@ -452,6 +452,22 @@ class OcrdMets(OcrdXmlDocument):
         if mets_div:
             mets_div[0].getparent().remove(mets_div[0])
 
+    def remove_physical_page_fptr(self, fileId):
+        """
+        Delete all ``mets:fptr[@FILEID = fileId]`` to ``mets:file[@ID == fileId]`` for :py:attr:`fileId` from all ``mets:div`` entries in the physical ``mets:structMap``.
+        Returns:
+            List of pageIds that mets:fptrs were deleted from
+        """
+        mets_fptrs = self._tree.getroot().xpath(
+            'mets:structMap[@TYPE="PHYSICAL"]/mets:div[@TYPE="physSequence"]/mets:div[@TYPE="page"]/mets:fptr[@FILEID="%s"]' % fileId,
+            namespaces=NS)
+        ret = []
+        for mets_fptr in mets_fptrs:
+            mets_div = mets_fptr.getparent()
+            ret.append(mets_div.get('ID'))
+            mets_div.remove(mets_fptr)
+        return ret
+
     def merge(self, other_mets, fileGrp_mapping=None, after_add_cb=None, **kwargs):
         """
         Add all files from other_mets.
diff --git a/tests/model/test_ocrd_file.py b/tests/model/test_ocrd_file.py
index 7ee04d01..8dbe1970 100644
--- a/tests/model/test_ocrd_file.py
+++ b/tests/model/test_ocrd_file.py
@@ -85,5 +85,13 @@ class TestOcrdFile(TestCase):
         f5 = mets.add_file('TEMP', ID='TEMP_1', mimetype='image/tiff')
         self.assertEqual(f3 == f5, True)
 
+    def test_fptr_changed_for_change_id(self):
+        mets = OcrdMets.empty_mets()
+        f1 = mets.add_file('FOO', ID='FOO_1', mimetype='image/tiff', pageId='p0001')
+        assert mets.get_physical_pages(for_fileIds=['FOO_1']) == ['p0001']
+        f1.ID = 'BAZ_1'
+        assert mets.get_physical_pages(for_fileIds=['FOO_1']) == [None]
+        assert mets.get_physical_pages(for_fileIds=['BAZ_1']) == ['p0001']
+
 if __name__ == '__main__':
     main(__file__)
diff --git a/tests/model/test_ocrd_mets.py b/tests/model/test_ocrd_mets.py
index 5937884f..986cf4e2 100644
--- a/tests/model/test_ocrd_mets.py
+++ b/tests/model/test_ocrd_mets.py
@@ -181,6 +181,13 @@ class TestOcrdMets(TestCase):
             mets.remove_physical_page('PHYS_0001')
             self.assertEqual(mets.physical_pages, ['PHYS_0002', 'PHYS_0005'])
 
+    def test_remove_physical_page_fptr(self):
+        with copy_of_directory(assets.path_to('SBB0000F29300010000/data')) as tempdir:
+            mets = OcrdMets(filename=join(tempdir, 'mets.xml'))
+            self.assertEqual(mets.get_physical_pages(for_fileIds=['FILE_0002_IMAGE']), ['PHYS_0002'])
+            mets.remove_physical_page_fptr('FILE_0002_IMAGE')
+            self.assertEqual(mets.get_physical_pages(for_fileIds=['FILE_0002_IMAGE']), [None])
+
     def test_remove_page_after_remove_file(self):
         with copy_of_directory(assets.path_to('SBB0000F29300010000/data')) as tempdir:
             mets = OcrdMets(filename=join(tempdir, 'mets.xml'))
diff --git a/tests/test_workspace.py b/tests/test_workspace.py
index b6202090..0786cb96 100644
--- a/tests/test_workspace.py
+++ b/tests/test_workspace.py
@@ -182,14 +182,13 @@ class TestWorkspace(TestCase):
             with pushd_popd(tempdir):
                 pcgts_before = page_from_file(next(workspace.mets.find_files(ID='OCR-D-GT-SEG-WORD_0001')))
                 assert pcgts_before.get_Page().imageFilename == 'OCR-D-IMG/OCR-D-IMG_0001.tif'
-                # from os import system
-                # print(system('find'))
                 workspace.rename_file_group('OCR-D-IMG', 'FOOBAR')
-                # print(system('find'))
                 pcgts_after = page_from_file(next(workspace.mets.find_files(ID='OCR-D-GT-SEG-WORD_0001')))
-                assert pcgts_after.get_Page().imageFilename == 'FOOBAR/OCR-D-IMG_0001.tif'
-                assert Path('FOOBAR/OCR-D-IMG_0001.tif').exists()
+                assert pcgts_after.get_Page().imageFilename == 'FOOBAR/FOOBAR_0001.tif'
+                assert Path('FOOBAR/FOOBAR_0001.tif').exists()
                 assert not Path('OCR-D-IMG/OCR-D-IMG_0001.tif').exists()
+                assert workspace.mets.get_physical_pages(for_fileIds=['OCR-D-IMG_0001']) == [None]
+                assert workspace.mets.get_physical_pages(for_fileIds=['FOOBAR_0001']) == ['phys_0001']
 
     def test_remove_file_group_force(self):
         with copy_of_directory(assets.path_to('SBB0000F29300010000/data')) as tempdir: