From c013e616270e67889613cb57217c2ea56f6acac1 Mon Sep 17 00:00:00 2001 From: Mihaela Nadasan Date: Tue, 12 Jul 2022 20:07:39 +0300 Subject: [PATCH 1/6] BEN-2516 - make tree rebuild more verbose - use partial_rebuild which is now available --- filer/management/commands/fixfoldertrees.py | 19 ++++++++++++++++++- filer/utils/checktrees.py | 5 +---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/filer/management/commands/fixfoldertrees.py b/filer/management/commands/fixfoldertrees.py index 45f4976ab..746bc3fba 100644 --- a/filer/management/commands/fixfoldertrees.py +++ b/filer/management/commands/fixfoldertrees.py @@ -14,6 +14,11 @@ def add_arguments(self, parser): default=False, help='Performs only a check for corrupted folder trees. ' 'Prints all the corruptions it finds.') + parser.add_argument('--full-rebuild', + action='store_true', + dest='force_full_rebuild', + default=False, + help='Force a full tree rebuild.') def handle(self, *args, **options): checker = TreeChecker() @@ -39,4 +44,16 @@ def handle(self, *args, **options): else: self.stdout.write("There are no corruptions\n") else: - checker.rebuild() + self.stdout.write("Checking folder tree...\n") + checker.check_corruptions() + if checker.full_rebuild or options['force_full_rebuild']: + self.stdout.write("\nPerforming full rebuild...") + checker.manager.rebuild() + elif checker.corrupted_folders: + self.stdout.write("\nPerforming corrupted folders rebuild...") + for folder in checker.get_corrupted_root_nodes(): + self.stdout.write(f"\n\tPerforming partial rebuild for folder {folder.name}") + checker.manager.partial_rebuild(folder.tree_id) + else: + self.stdout.write("There are no corruptions, nothign to be done.\n") + self.stdout.write("\nRebuild Done.") diff --git a/filer/utils/checktrees.py b/filer/utils/checktrees.py index 836cae099..402f0daa6 100644 --- a/filer/utils/checktrees.py +++ b/filer/utils/checktrees.py @@ -107,7 +107,4 @@ def rebuild(self): self.manager.rebuild() elif self.corrupted_folders: for folder in self.get_corrupted_root_nodes(): - # since we use an older version of djang-mptt and method - # partial_rebuild does not exist ... - self.manager._rebuild_helper( - folder.pk, 1, folder.tree_id) + self.manager.partial_rebuild(folder.tree_id) From 5af765b3bc0576b0e162c4c8d25ed90acd241de7 Mon Sep 17 00:00:00 2001 From: Mihaela Nadasan Date: Tue, 12 Jul 2022 20:41:38 +0300 Subject: [PATCH 2/6] fix typo --- filer/management/commands/fixfoldertrees.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filer/management/commands/fixfoldertrees.py b/filer/management/commands/fixfoldertrees.py index 746bc3fba..59bbbfe51 100644 --- a/filer/management/commands/fixfoldertrees.py +++ b/filer/management/commands/fixfoldertrees.py @@ -55,5 +55,5 @@ def handle(self, *args, **options): self.stdout.write(f"\n\tPerforming partial rebuild for folder {folder.name}") checker.manager.partial_rebuild(folder.tree_id) else: - self.stdout.write("There are no corruptions, nothign to be done.\n") + self.stdout.write("There are no corruptions, nothing to be done.\n") self.stdout.write("\nRebuild Done.") From 7924e890f783485d41b626ad05462ab3f4c9534e Mon Sep 17 00:00:00 2001 From: Mihaela Nadasan Date: Thu, 14 Jul 2022 10:45:10 +0300 Subject: [PATCH 3/6] BEN-2516 - ignore deleted folders, same as django-mptt's rebuild if deleted folders are not filtered out, the check will always show corrupt deleted folders, but the rebuild tool does not fix them. --- filer/utils/checktrees.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/filer/utils/checktrees.py b/filer/utils/checktrees.py index 402f0daa6..05691541e 100644 --- a/filer/utils/checktrees.py +++ b/filer/utils/checktrees.py @@ -1,4 +1,5 @@ from django.db.models import Count +from filer.models import Folder class TreeCorruption(Exception): @@ -53,12 +54,13 @@ def get_corrupted_root_nodes(self): pk__in=list(self.corrupted_folders.keys())).\ values_list('tree_id', flat=True).distinct() return self.manager.filter( - parent__isnull=True, tree_id__in=corrupted_trees) + parent__isnull=True, deleted_ta__isnull=True, tree_id__in=corrupted_trees) def check_tree(self, pk, lft, tree_id, level=0): """ * checks if a certain folder tree is corrupted or not. * uses the same logic as django-mptt's rebuild tree + * ignores deleted folders, same as django-mptt's rebuild """ rght = lft + 1 child_ids = self.manager.filter(parent__pk=pk).\ @@ -68,11 +70,12 @@ def check_tree(self, pk, lft, tree_id, level=0): rght = self.check_tree(child_id, rght, tree_id, level + 1) folder = self.manager.get(pk=pk) - expected = (lft, rght, level, tree_id) - actual = (folder.lft, folder.rght, folder.level, folder.tree_id) - if expected != actual: - self.corrupted_folders.setdefault( - pk, self._build_diff_msg(expected, actual)) + if folder.deleted_at is None: + expected = (lft, rght, level, tree_id) + actual = (folder.lft, folder.rght, folder.level, folder.tree_id) + if expected != actual: + self.corrupted_folders.setdefault( + pk, self._build_diff_msg(expected, actual)) return rght + 1 def check_corruptions(self): From a913ea97df3473c81e6ca3b690fb8a7f2335c33a Mon Sep 17 00:00:00 2001 From: Mihaela Nadasan Date: Thu, 14 Jul 2022 11:00:56 +0300 Subject: [PATCH 4/6] BEN-2516 - skip check if opted for forced rebuild --- filer/management/commands/fixfoldertrees.py | 24 ++++++++++++--------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/filer/management/commands/fixfoldertrees.py b/filer/management/commands/fixfoldertrees.py index 59bbbfe51..fa164a954 100644 --- a/filer/management/commands/fixfoldertrees.py +++ b/filer/management/commands/fixfoldertrees.py @@ -14,7 +14,7 @@ def add_arguments(self, parser): default=False, help='Performs only a check for corrupted folder trees. ' 'Prints all the corruptions it finds.') - parser.add_argument('--full-rebuild', + parser.add_argument('--force-rebuild', action='store_true', dest='force_full_rebuild', default=False, @@ -44,16 +44,20 @@ def handle(self, *args, **options): else: self.stdout.write("There are no corruptions\n") else: - self.stdout.write("Checking folder tree...\n") - checker.check_corruptions() - if checker.full_rebuild or options['force_full_rebuild']: + if options['force_full_rebuild']: self.stdout.write("\nPerforming full rebuild...") checker.manager.rebuild() - elif checker.corrupted_folders: - self.stdout.write("\nPerforming corrupted folders rebuild...") - for folder in checker.get_corrupted_root_nodes(): - self.stdout.write(f"\n\tPerforming partial rebuild for folder {folder.name}") - checker.manager.partial_rebuild(folder.tree_id) else: - self.stdout.write("There are no corruptions, nothing to be done.\n") + self.stdout.write("Checking folder trees before fixing...\n") + checker.check_corruptions() + if checker.full_rebuild: + self.stdout.write("\nPerforming full rebuild...") + checker.manager.rebuild() + elif checker.corrupted_folders: + self.stdout.write("\nPerforming corrupted folders rebuild...") + for folder in checker.get_corrupted_root_nodes(): + self.stdout.write(f"\n\tPerforming partial rebuild for folder {folder.name}") + checker.manager.partial_rebuild(folder.tree_id) + else: + self.stdout.write("There are no corruptions, nothing to be done.\n") self.stdout.write("\nRebuild Done.") From 72d92fcc08e255d2e225bb2dc8bdcae1eb2f705b Mon Sep 17 00:00:00 2001 From: Mihaela Nadasan Date: Fri, 15 Jul 2022 20:05:20 +0300 Subject: [PATCH 5/6] BEN-2516 - code review items, fix typo --- filer/utils/checktrees.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filer/utils/checktrees.py b/filer/utils/checktrees.py index 05691541e..b57773941 100644 --- a/filer/utils/checktrees.py +++ b/filer/utils/checktrees.py @@ -54,7 +54,7 @@ def get_corrupted_root_nodes(self): pk__in=list(self.corrupted_folders.keys())).\ values_list('tree_id', flat=True).distinct() return self.manager.filter( - parent__isnull=True, deleted_ta__isnull=True, tree_id__in=corrupted_trees) + parent__isnull=True, deleted_at__isnull=True, tree_id__in=corrupted_trees) def check_tree(self, pk, lft, tree_id, level=0): """ From 15dc0cc5d98de4d38af12290a1aa0322d00ec5c8 Mon Sep 17 00:00:00 2001 From: Mihaela Nadasan Date: Mon, 18 Jul 2022 15:52:49 +0300 Subject: [PATCH 6/6] BEN-2516 - code review items + pep8 formatting --- filer/utils/checktrees.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/filer/utils/checktrees.py b/filer/utils/checktrees.py index b57773941..1a0b4ee0e 100644 --- a/filer/utils/checktrees.py +++ b/filer/utils/checktrees.py @@ -1,5 +1,4 @@ from django.db.models import Count -from filer.models import Folder class TreeCorruption(Exception): @@ -7,7 +6,6 @@ class TreeCorruption(Exception): class TreeChecker(object): - ordering = ['tree_id', 'lft', 'rght'] def __init__(self, folder_manager=None): @@ -20,14 +18,13 @@ def __init__(self, folder_manager=None): else: self.manager = folder_manager - def find_corruptions(self): self.check_corruptions() if (self.full_rebuild or self.corrupted_folders): raise TreeCorruption() def _build_diff_msg(self, expected, actual): - attr_idx = {'lft':0, 'rght':1, 'level':2, 'tree_id':3} + attr_idx = {'lft': 0, 'rght': 1, 'level': 2, 'tree_id': 3} expected, actual = list(expected), list(actual) diff = [] for attr, idx in list(attr_idx.items()): @@ -51,7 +48,7 @@ def get_corrupted_root_nodes(self): if not self.corruption_check_done: self.check_corruptions() corrupted_trees = self.manager.filter( - pk__in=list(self.corrupted_folders.keys())).\ + pk__in=list(self.corrupted_folders.keys())). \ values_list('tree_id', flat=True).distinct() return self.manager.filter( parent__isnull=True, deleted_at__isnull=True, tree_id__in=corrupted_trees) @@ -63,7 +60,7 @@ def check_tree(self, pk, lft, tree_id, level=0): * ignores deleted folders, same as django-mptt's rebuild """ rght = lft + 1 - child_ids = self.manager.filter(parent__pk=pk).\ + child_ids = self.manager.filter(parent__pk=pk). \ order_by(*self.ordering).values_list('pk', flat=True) for child_id in child_ids: @@ -85,15 +82,15 @@ def check_corruptions(self): * checks if there are multiple root folders with the same tree id(fixing this will require a full rebuild) """ - tree_duplicates = self.manager.filter(parent=None).\ - values_list('tree_id').\ + tree_duplicates = self.manager.filter(parent=None). \ + values_list('tree_id'). \ annotate(count=Count('id')).filter(count__gt=1) if len(tree_duplicates) > 0: self.full_rebuild = True self.corruption_check_done = True return - pks = self.manager.filter(parent=None).\ + pks = self.manager.filter(parent=None). \ order_by(*self.ordering).values_list('pk', flat=True) idx = 0 for pk in pks: