From 54bb506e10cf248a60005f450129ddf595499b13 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@stuvel.eu>
Date: Thu, 14 Sep 2017 17:43:23 +0200
Subject: [PATCH] Orphan finder: also interpret 24-char hex strings as
 ObjectIDs

This is necessary as some dynamic node properties have ObjectIDs saved
as strings.
---
 pillar/cli/maintenance.py  |  7 +++++++
 tests/test_orphan_files.py | 20 +++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/pillar/cli/maintenance.py b/pillar/cli/maintenance.py
index 4449b5eb..b106d5ab 100644
--- a/pillar/cli/maintenance.py
+++ b/pillar/cli/maintenance.py
@@ -553,6 +553,7 @@ def _find_orphan_files() -> typing.Set[bson.ObjectId]:
 
     Returns an iterable of all orphan file IDs.
     """
+    from pillar.api.utils import str2id
 
     log.debug('Finding orphan files')
 
@@ -570,6 +571,12 @@ def _find_orphan_files() -> typing.Set[bson.ObjectId]:
     def find_object_ids(something: typing.Any) -> typing.Iterable[bson.ObjectId]:
         if isinstance(something, bson.ObjectId):
             yield something
+        elif isinstance(something, str) and len(something) == 24:
+            try:
+                yield bson.ObjectId(something)
+            except (bson.objectid.InvalidId, TypeError):
+                # It apparently wasn't an ObjectID after all.
+                pass
         elif isinstance(something, (list, set, tuple)):
             for item in something:
                 yield from find_object_ids(item)
diff --git a/tests/test_orphan_files.py b/tests/test_orphan_files.py
index b3002992..09d31a07 100644
--- a/tests/test_orphan_files.py
+++ b/tests/test_orphan_files.py
@@ -25,7 +25,7 @@ class OrphanFilesTest(AbstractPillarTest):
         project_ids = (public1, public2, private1, private2)
         file_ids = collections.defaultdict(list)
         for pidx, pid in enumerate(project_ids):
-            for filenum in range(5):
+            for filenum in range(6):
                 generated_file_id = ObjectId(f'{pidx}{filenum}' + 22 * 'a')
                 file_id, _ = self.ensure_file_exists({
                     '_id': generated_file_id,
@@ -88,6 +88,24 @@ class OrphanFilesTest(AbstractPillarTest):
                 'random': {'field': [fids[2]]}
             })
             # fids[3] is an orphan.
+            # fids[5] is converted to a string and then used.
+            self.create_node({
+                '_id': ObjectId(),
+                'project': pid,
+                'picture': ObjectId('572761f39837730efe8e1210'),
+                'description': '',
+                'node_type': 'totally-unknown',
+                'user': ObjectId(24 * 'a'),
+                'properties': {
+                    'status': 'published',
+                    'content_type': 'image',
+                    'file': str(fids[5]),
+                },
+                'name': 'Image random field',
+                '_updated': datetime.datetime(2016, 5, 2, 14, 19, 58, 0, tzinfo=tz_util.utc),
+                '_created': datetime.datetime(2016, 5, 2, 14, 19, 37, 0, tzinfo=tz_util.utc),
+                '_etag': '6b8589b42c880e3626f43f3e82a5c5b946742687'
+            })
 
         from pillar.cli.maintenance import _find_orphan_files