[ENG-8192] Ability to force archive registrations when OSFS Folders have changed (#11194)

antkryt · web-flow · commit 3e609ac7c44f · 2025-06-30T10:28:17.000-04:00
## Purpose fix force archive for some actions ## Changes - divide revert_log_actions into separate functions - include trashed children when build a file tree - add additional retrieval logic for osf_storage_folder_created and osf_storage_file_removed - add generic retrieval fallback ## Ticket https://openscience.atlassian.net/browse/ENG-8192
diff --git a/osf/management/commands/force_archive.py b/osf/management/commands/force_archive.py
@@ -37,6 +37,7 @@
 from framework import sentry
 from framework.exceptions import HTTPError
 from osf.models import Node, NodeLog, Registration, BaseFileNode
+from osf.models.files import TrashedFileNode
 from osf.exceptions import RegistrationStuckRecoverableException, RegistrationStuckBrokenException
 from api.base.utils import waterbutler_api_url_for
 from scripts import utils as script_utils
@@ -280,48 +281,72 @@ def get_logs_to_revert(reg):
             Q(node=reg.registered_from) |
             (Q(params__source__nid=reg.registered_from._id) | Q(params__destination__nid=reg.registered_from._id))).order_by('-date')
 
+def get_file_obj_from_log(log, reg):
+    try:
+        return BaseFileNode.objects.get(_id=log.params['urls']['view'].split('/')[4])
+    except KeyError:
+        if log.action == 'osf_storage_folder_created':
+            return OsfStorageFolder.objects.get(
+                target_object_id=reg.registered_from.id,
+                name=log.params['path'].split('/')[-2]
+            )
+        elif log.action == 'osf_storage_file_removed':
+            path = log.params['path'].split('/')
+            return TrashedFileNode.objects.get(
+                target_object_id=reg.registered_from.id,
+                name=path[-1] or path[-2]  # file name or folder name
+            )
+        elif log.action in ['addon_file_moved', 'addon_file_renamed']:
+            try:
+                return BaseFileNode.objects.get(_id=log.params['source']['path'].rstrip('/').split('/')[-1])
+            except (KeyError, BaseFileNode.DoesNotExist):
+                return BaseFileNode.objects.get(_id=log.params['destination']['path'].rstrip('/').split('/')[-1])
+        else:
+            # Generic fallback
+            path = log.params.get('path', '').split('/')
+            if len(path) >= 2:
+                name = path[-1] or path[-2]  # file name or folder name
+                return BaseFileNode.objects.get(
+                    target_object_id=reg.registered_from.id,
+                    name=name
+                )
+
+        raise ValueError(f'Cannot determine file obj for log {log._id} [Registration id {reg._id}]: {log.action}')
+
+
+def handle_file_operation(file_tree, reg, file_obj, log, obj_cache):
+    logger.info(f'Reverting {log.action} {file_obj._id}:{file_obj.name} from {log.date}')
+
+    if log.action in ['osf_storage_file_added', 'osf_storage_folder_created']:
+        return modify_file_tree_recursive(reg._id, file_tree, file_obj, deleted=True, cached=bool(file_obj._id in obj_cache))
+    elif log.action == 'osf_storage_file_removed':
+        return modify_file_tree_recursive(reg._id, file_tree, file_obj, deleted=False, cached=bool(file_obj._id in obj_cache))
+    elif log.action == 'osf_storage_file_updated':
+        return modify_file_tree_recursive(reg._id, file_tree, file_obj, cached=bool(file_obj._id in obj_cache), revert=True)
+    elif log.action == 'addon_file_moved':
+        parent = BaseFileNode.objects.get(_id__in=obj_cache, name='/{}'.format(log.params['source']['materialized']).rstrip('/').split('/')[-2])
+        return modify_file_tree_recursive(reg._id, file_tree, file_obj, cached=bool(file_obj._id in obj_cache), move_under=parent)
+    elif log.action == 'addon_file_renamed':
+        return modify_file_tree_recursive(reg._id, file_tree, file_obj, cached=bool(file_obj._id in obj_cache), rename=log.params['source']['name'])
+    else:
+        raise ValueError(f'Unexpected log action: {log.action}')
+
 def revert_log_actions(file_tree, reg, obj_cache, permissible_addons):
     logs_to_revert = get_logs_to_revert(reg)
+
     if len(permissible_addons) > 1:
         logs_to_revert = logs_to_revert.exclude(action__in=PERMISSIBLE_BLACKLIST)
+
     for log in list(logs_to_revert):
-        try:
-            file_obj = BaseFileNode.objects.get(_id=log.params['urls']['view'].split('/')[4])
-        except KeyError:
-            try:
-                file_obj = BaseFileNode.objects.get(_id=log.params['source']['path'].rstrip('/').split('/')[-1])
-            except BaseFileNode.DoesNotExist:
-                # Bad log data
-                file_obj = BaseFileNode.objects.get(_id=log.params['destination']['path'].rstrip('/').split('/')[-1])
+        file_obj = get_file_obj_from_log(log, reg)
         assert file_obj.target in reg.registered_from.root.node_and_primary_descendants()
-        if log.action == 'osf_storage_file_added':
-            # Find and mark deleted
-            logger.info(f'Reverting add {file_obj._id}:{file_obj.name} from {log.date}')
-            file_tree, noop = modify_file_tree_recursive(reg._id, file_tree, file_obj, deleted=True, cached=bool(file_obj._id in obj_cache))
-        elif log.action == 'osf_storage_file_removed':
-            # Find parent and add to children, or undelete
-            logger.info(f'Reverting delete {file_obj._id}:{file_obj.name} from {log.date}')
-            file_tree, noop = modify_file_tree_recursive(reg._id, file_tree, file_obj, deleted=False, cached=bool(file_obj._id in obj_cache))
-        elif log.action == 'osf_storage_file_updated':
-            # Find file and revert version
-            logger.info(f'Reverting update {file_obj._id}:{file_obj.name} from {log.date}')
-            file_tree, noop = modify_file_tree_recursive(reg._id, file_tree, file_obj, cached=bool(file_obj._id in obj_cache), revert=True)
-        elif log.action == 'osf_storage_folder_created':
-            # Find folder and mark deleted
-            logger.info(f'Reverting folder {file_obj._id}:{file_obj.name} from {log.date}')
-            file_tree, noop = modify_file_tree_recursive(reg._id, file_tree, file_obj, deleted=True, cached=bool(file_obj._id in obj_cache))
-        elif log.action == 'addon_file_moved':
-            logger.info(f'Reverting move {file_obj._id}:{file_obj.name} from {log.date}')
-            parent = BaseFileNode.objects.get(_id__in=obj_cache, name='/{}'.format(log.params['source']['materialized']).rstrip('/').split('/')[-2])
-            file_tree, noop = modify_file_tree_recursive(reg._id, file_tree, file_obj, cached=bool(file_obj._id in obj_cache), move_under=parent)
-        elif log.action == 'addon_file_renamed':
-            logger.info('Reverting rename {}:{} -> {} from {}'.format(file_obj._id, log.params['source']['name'], file_obj.name, log.date))
-            file_tree, noop = modify_file_tree_recursive(reg._id, file_tree, file_obj, cached=bool(file_obj._id in obj_cache), rename=log.params['source']['name'])
-        else:
-            raise Exception(f'Unexpected log action: {log.action}')
+
+        file_tree, noop = handle_file_operation(file_tree, reg, file_obj, log, obj_cache)
         assert not noop, f'{reg._id}: Failed to revert action for NodeLog {log._id}'
+
         if file_obj._id not in obj_cache:
             obj_cache.add(file_obj._id)
+
     return file_tree
 
 def build_file_tree(reg, node_settings, *args, **kwargs):
@@ -337,7 +362,20 @@ def _recurse(file_obj, node):
             'version': int(file_obj.versions.latest('created').identifier) if file_obj.versions.exists() else None
         }
         if not file_obj.is_file:
-            serialized['children'] = [_recurse(child, node) for child in OsfStorageFileNode.objects.filter(target_object_id=node.id, target_content_type_id=ct_id, parent_id=file_obj.id)]
+            nonlocal reg
+            all_children = OsfStorageFileNode.objects.filter(
+                target_object_id=node.id,
+                target_content_type_id=ct_id,
+                parent_id=file_obj.id
+            ).union(
+                TrashedFileNode.objects.filter(
+                    target_object_id=node.id,
+                    target_content_type_id=ct_id,
+                    parent_id=file_obj.id,
+                    modified__gt=reg.archive_job.created,
+                )
+            )
+            serialized['children'] = [_recurse(child, node) for child in all_children]
         return serialized
 
     current_tree = _recurse(node_settings.get_root(), n)
diff --git a/osf_tests/management_commands/test_force_archive.py b/osf_tests/management_commands/test_force_archive.py
@@ -0,0 +1,194 @@
+import pytest
+from django.utils import timezone
+
+from addons.osfstorage.models import OsfStorageFile, OsfStorageFolder
+from osf.models import NodeLog, BaseFileNode
+from osf.models.files import TrashedFileNode, TrashedFolder
+from osf.management.commands.force_archive import get_file_obj_from_log, build_file_tree, DEFAULT_PERMISSIBLE_ADDONS
+from osf_tests.factories import NodeFactory, RegistrationFactory
+
+
+class TestGetFileObjFromLog:
+
+    @pytest.fixture
+    def node(self):
+        return NodeFactory(title='Test Node', category='project')
+
+    @pytest.fixture
+    def reg(self, node):
+        return RegistrationFactory(project=node, registered_date=timezone.now())
+
+    @pytest.mark.django_db
+    def test_file_added(self, node, reg):
+        file = OsfStorageFile.objects.create(target=node, name='file1.txt')
+        file.save()
+        log = NodeLog.objects.create(
+            node=node,
+            action='osf_storage_file_added',
+            params={'urls': {'view': f'/{node._id}/files/osfstorage/{file._id}/'}},
+            date=timezone.now(),
+        )
+        file_obj = get_file_obj_from_log(log, reg)
+        assert isinstance(file_obj, BaseFileNode)
+        assert file_obj == file
+
+    @pytest.mark.django_db
+    def test_file_removed(self, node, reg):
+        file = OsfStorageFile.create(target=node, name='trashed.txt')
+        file.delete()
+        log = NodeLog.objects.create(
+            node=node,
+            action='osf_storage_file_removed',
+            params={'path': '/folder1/trashed.txt'},
+            date=timezone.now(),
+        )
+        file_obj = get_file_obj_from_log(log, reg)
+        assert isinstance(file_obj, TrashedFileNode)
+        assert file_obj == file
+
+        folder = OsfStorageFolder.create(target=node, name='folder1')
+        folder.delete()
+        log = NodeLog.objects.create(
+            node=node,
+            action='osf_storage_file_removed',
+            params={'path': '/folder1/'},
+            date=timezone.now(),
+        )
+        file_obj = get_file_obj_from_log(log, reg)
+        assert isinstance(file_obj, TrashedFolder)
+        assert file_obj == folder
+
+    @pytest.mark.django_db
+    def test_folder_created(self, node, reg):
+        folder = OsfStorageFolder.create(target=node, name='folder1')
+        folder.save()
+        log = NodeLog.objects.create(
+            node=node,
+            action='osf_storage_folder_created',
+            params={'path': '/folder1/'},
+            date=timezone.now(),
+        )
+        file_obj = get_file_obj_from_log(log, reg)
+        assert isinstance(file_obj, OsfStorageFolder)
+        assert file_obj == folder
+
+    @pytest.mark.django_db
+    def test_move_rename(self, node, reg):
+        file = OsfStorageFile.create(target=node, name='file2.txt')
+        file.save()
+        log = NodeLog.objects.create(
+            node=node,
+            action='addon_file_renamed',
+            params={
+                'source': {'path': f'/{file._id}', 'name': 'file1.txt'},
+                'destination': {'path': f'/{file._id}', 'name': 'file2.txt'}
+            },
+            date=timezone.now(),
+        )
+        file_obj = get_file_obj_from_log(log, reg)
+        assert isinstance(file_obj, BaseFileNode)
+        assert file_obj == file
+
+    @pytest.mark.django_db
+    def test_generic_fallback(self, node, reg):
+        file = OsfStorageFile.create(target=node, name='fallback.txt')
+        file.save()
+        log = NodeLog.objects.create(
+            node=node,
+            action='some_other_action',
+            params={'path': '/fallback.txt'},
+            date=timezone.now(),
+        )
+        file_obj = get_file_obj_from_log(log, reg)
+        assert file_obj == file
+
+
+class TestBuildFileTree:
+
+    @pytest.fixture
+    def node(self):
+        return NodeFactory(title='Test Node', category='project')
+
+    @pytest.fixture
+    def reg(self, node):
+        return RegistrationFactory(project=node, registered_date=timezone.now())
+
+    @pytest.fixture
+    def permissible_addons(self):
+        return DEFAULT_PERMISSIBLE_ADDONS
+
+    @pytest.mark.django_db
+    def test_empty_folder(self, node, reg, permissible_addons):
+        folder = OsfStorageFolder.create(target=node, name='empty')
+        folder.save()
+
+        class DummyNodeSettings:
+            def get_root(self):
+                return folder
+
+        node_settings = DummyNodeSettings()
+        tree = build_file_tree(reg, node_settings, permissible_addons=permissible_addons)
+        assert tree['object'] == folder
+        assert tree['children'] == []
+
+    @pytest.mark.django_db
+    def test_nested_folders(self, node, reg, permissible_addons):
+        parent = OsfStorageFolder.create(target=node, name='parent')
+        parent.save()
+
+        child = OsfStorageFolder.create(target=node, name='child')
+        child.save()
+        child.move_under(parent)
+
+        file = OsfStorageFile.objects.create(target=node, name='file1.txt')
+        file.save()
+        file.move_under(child)
+
+        class DummyNodeSettings:
+            def get_root(self):
+                return parent
+
+        node_settings = DummyNodeSettings()
+        tree = build_file_tree(reg, node_settings, permissible_addons=permissible_addons)
+        assert tree['object'] == parent
+
+        child_node = next((c for c in tree['children'] if c['object'] == child), None)
+        assert child_node is not None
+        assert any(grandchild['object'] == file for grandchild in child_node['children'])
+
+    @pytest.mark.django_db
+    def test_active_and_trashed_children(self, node, reg, permissible_addons):
+        folder = OsfStorageFolder.create(target=node, name='parent')
+        folder.save()
+
+        file = OsfStorageFile.create(target=node, name='file1.txt')
+        file.save()
+        file.move_under(folder)
+
+        deleted_file = OsfStorageFile.create(target=node, name='file2.txt')
+        deleted_file.save()
+        deleted_file.move_under(folder)
+        deleted_file.delete()
+
+        class DummyNodeSettings:
+            def get_root(self):
+                return folder
+
+        node_settings = DummyNodeSettings()
+        tree = build_file_tree(reg, node_settings, permissible_addons=permissible_addons)
+        assert tree['object'] == folder
+
+        names = [child['object'].name for child in tree['children']]
+        assert 'file1.txt' in names
+        assert 'file2.txt' in names
+
+        # make sure only valid thrashed file nodes are included
+        new_reg = RegistrationFactory(project=node, registered_date=timezone.now())
+        file.delete()
+
+        tree = build_file_tree(new_reg, node_settings, permissible_addons=permissible_addons)
+        assert tree['object'] == folder
+
+        names = [child['object'].name for child in tree['children']]
+        assert 'file1.txt' in names
+        assert 'file2.txt' not in names
