Performance: Switch browse action to list archive files only once per archive.

borgmatic/actions/browse/archive.py

@@ -45,12 +45,9 @@ def get_repository_archives(config, repository):
         )
 
 
-def get_archive_files(config, repository, archive_name, list_path=None):
+def get_archive_files(config, repository, archive_name):
     with borgmatic.logger.Log_prefix(repository.get('label', repository['path'])):
-        if list_path:
-            logger.info(f'Listing archive {archive_name} at path {list_path}')
-        else:
-            logger.info(f'Listing archive {archive_name}')
+        logger.info(f'Listing archive {archive_name}')
 
         global_arguments = argparse.Namespace()
         local_path = config.get('local_path', 'borg')
@@ -60,10 +57,8 @@ def get_archive_files(config, repository, archive_name, list_path=None):
 
         return (
             (
-                path_data['type']
-                if os.path.join(list_path, base_path) == path_data['path']
-                else 'd',
-                base_path,
+                path_data['type'],
+                path_data['path'],
                 path_data.get('linktarget'),
             )
             for path_data in borgmatic.borg.list.capture_archive_listing(
@@ -72,12 +67,9 @@ def get_archive_files(config, repository, archive_name, list_path=None):
                 config,
                 local_borg_version,
                 global_arguments,
-                list_paths=(rf're:^{list_path}/[^/]+',) if list_path else (r're:^[^/]+',),
                 local_path=local_path,
                 remote_path=remote_path,
             )
-            for base_path in (os.path.relpath(path_data['path'], list_path).split(os.path.sep)[0],)
-            if base_path not in seen and not seen.add(base_path)
         )
 
 

borgmatic/actions/browse/carousel.py

@@ -43,6 +43,7 @@ def make_next_panel(focused_panel, option_id):
                 focused_panel.config,
                 focused_panel.repository,
                 focused_panel.archive_name,
+                path_loaded=focused_panel.path_loaded,
                 path_components=(*focused_panel.path_components, option_id),
             )
         elif option.prompt.startswith(
@@ -118,7 +119,7 @@ class Carousel(textual.containers.Horizontal):
         self.focused_panel.styles.display = 'none'
         self.focused_panel = next_panel
         self.focused_panel.focus()
-        self.refresh(recompose=True)
+        self.mount(self.focused_panel)
 
     def on_option_list_option_highlighted(self, event):
         '''
@@ -133,7 +134,9 @@ class Carousel(textual.containers.Horizontal):
         An option has been selected, so advance to the next panel—unless the option selected is
         "..", in which case go to the previous panel.
         '''
-        if event.option_list != self.focused_panel or event.option_id == 'loading-indicator':  # pragma: no cover
+        if (
+            event.option_list != self.focused_panel or event.option_id == 'loading-indicator'
+        ):  # pragma: no cover
             return
 
         if event.option_id == '..':

borgmatic/actions/browse/loading.py

@@ -7,6 +7,10 @@ import textual.widgets.option_list
 LOADING_DOT_INTERVAL_SECONDS = 0.3
 
 
+import logging
+logger = logging.getLogger('__name__')
+
+
 def update_inline_loading_indicator(widget):
     if isinstance(widget, textual.widgets.OptionList):
         with contextlib.suppress(textual.widgets.option_list.OptionDoesNotExist):

borgmatic/actions/browse/panels.py

@@ -1,3 +1,5 @@
+import contextlib
+import logging
 import os
 
 import textual.binding
@@ -7,6 +9,10 @@ import borgmatic.actions.browse.loading
 import borgmatic.actions.browse.paths
 import borgmatic.actions.browse.workers
 
+
+logger = logging.getLogger('__name__')
+
+
 OPTION_LIST_BINDINGS = (
     *textual.widgets.OptionList.BINDINGS,
     textual.binding.Binding(
@@ -90,10 +96,59 @@ class Archives_list(textual.widgets.OptionList):
             self.highlighted_option_changed = True
 
 
+def add_archive_path(
+    directory_list,
+    config,
+    repository,
+    archive_name,
+    archive_path,
+):
+    archive_path_components = archive_path.file_path.split(os.path.sep)
+
+    if directory_list.path_components:
+        # If the loaded path doesn't match this directory list's own path, then we don't care about
+        # it for purposes of displaying this particular directory.
+        if tuple(archive_path_components[: len(directory_list.path_components)]) != directory_list.path_components:
+            return
+
+        # Strip off the portion of the archive path that matches the directory list's own path.
+        archive_path_components = archive_path_components[len(directory_list.path_components):]
+
+    base_path = archive_path_components[0]
+
+    # If the option is already in the list, bail.
+    with contextlib.suppress(textual.widgets.option_list.OptionDoesNotExist):
+        directory_list.get_option(option_id=base_path)
+
+        return
+
+    pieces = (
+        borgmatic.actions.browse.paths.PATH_TYPE_ICONS.get(
+            archive_path.path_type if len(archive_path_components) == 1 else 'd', '❓'
+        ),
+        base_path,
+    ) + (('→', archive_path.link_target) if archive_path.link_target else ())
+    highlighted_option = directory_list.highlighted_option
+    sorted_options = sorted(
+        [
+            *directory_list.options,
+            textual.widgets.option_list.Option(' '.join(pieces), id=base_path),
+        ],
+        key=lambda option: ((option.id == 'loading-indicator'), option.prompt),
+    )
+
+    directory_list.set_options(sorted_options)
+    directory_list.highlighted = (
+        directory_list.get_option_index(highlighted_option.id)
+        if highlighted_option and directory_list.highlighted_option_changed
+        else 0
+    )
+
+
 class Directory_list(textual.widgets.OptionList):
     BINDINGS = OPTION_LIST_BINDINGS
 
-    def __init__(self, config, repository, archive_name, path_components=None):
+    def __init__(self, config, repository, archive_name, path_loaded=None, path_components=None):
         self.config = config
         self.repository = repository
         self.archive_name = archive_name
@@ -101,6 +156,7 @@ class Directory_list(textual.widgets.OptionList):
         self.highlighted_option_changed = False
 
         super().__init__(classes='panel')
+
         self.border_title = ' '.join(
             (
                 borgmatic.actions.browse.paths.PATH_TYPE_ICONS[
@@ -112,17 +168,52 @@ class Directory_list(textual.widgets.OptionList):
             )
         )
 
-        timer = borgmatic.actions.browse.loading.add_inline_loading_indicator(self)
+        # FIXME: This isn't working when loading is still underway??? I don't see a ".."
+        if self.path_components:
+            self.add_option(
+                textual.widgets.option_list.Option(
+                    f'{borgmatic.actions.browse.paths.PATH_TYPE_ICONS[borgmatic.actions.browse.paths.Path_type.DIRECTORY.value]} ..',
+                    id='..',
+                ),
+            )
 
-        borgmatic.actions.browse.workers.add_archive_files(
-            self.app,
+        self.path_loaded = path_loaded or borgmatic.actions.browse.workers.Archive_path_loaded(
+            self, 'archive path loaded'
+        )
+
+        if not self.path_loaded.complete:
+            # FIXME: After going back to a previous panel, the loading indicator is no longer animating.
+            self.timer = borgmatic.actions.browse.loading.add_inline_loading_indicator(self)
+
+        if self.path_components:
+            for archive_path in borgmatic.actions.browse.workers.get_paths(
+                self.path_loaded.path_hierarchy, self.path_components
+            ):
+                self.on_archive_path_loaded(archive_path)
+        else:
+            borgmatic.actions.browse.workers.load_archive_files(
+                self.app,
+                directory_list=self,
+                config=self.config,
+                repository=self.repository,
+                archive_name=self.archive_name,
+            )
+
+    def on_mount(self):
+        self.path_loaded.subscribe(self, self.on_archive_path_loaded)
+
+    def on_archive_path_loaded(self, data):
+        if data is borgmatic.actions.browse.workers.LOADING_DONE:
+            self.timer.stop()
+            self.remove_option('loading-indicator')
+            return
+
+        add_archive_path(
             directory_list=self,
             config=self.config,
             repository=self.repository,
             archive_name=self.archive_name,
-            list_path=os.path.sep.join(self.path_components),
-            root_directory=not bool(self.path_components),
-            timer=timer,
+            archive_path=data,
         )
 
     def on_option_list_option_highlighted(self, event):

borgmatic/actions/browse/workers.py

@@ -1,10 +1,19 @@
+import collections
+import contextlib
+import logging
+import os
+
 import rich.syntax
 import textual
+import textual.signal
 import textual.widgets.option_list
 
 import borgmatic.actions.browse.archive
 
 
+logger = logging.getLogger('__name__')
+
+
 @textual.work(thread=True)
 def add_repository_archives(browse_app, archives_list, config, repository, timer):
     archives_data = borgmatic.actions.browse.archive.get_repository_archives(config, repository)
@@ -36,50 +45,62 @@ def add_repository_archives(browse_app, archives_list, config, repository, timer
     browse_app.call_from_thread(timer.stop)
 
 
+Archive_path = collections.namedtuple(
+    'Archive_path',
+    ('path_type', 'file_path', 'link_target'),
+)
+
+
+def record_path(archive_path, hierarchy, path_components):
+    if len(path_components) == 1:
+        hierarchy[path_components[0]] = {} if archive_path.path_type == 'd' else archive_path
+        return
+
+    record_path(archive_path, hierarchy.setdefault(path_components[0], {}), path_components[1:])
+
+
+def get_paths(hierarchy, path_components, full_path_components=None):
+    if full_path_components is None:
+        full_path_components = path_components
+
+    if len(path_components) == 1:
+        return (
+            archive_path
+            if isinstance(archive_path, Archive_path)
+            else Archive_path('d', os.path.join(*full_path_components, component), '')
+            for component, archive_path in hierarchy[path_components[0]].items()
+        )
+
+    return get_paths(hierarchy[path_components[0]], path_components[1:], full_path_components)
+
+
+LOADING_DONE = object()
+
+
+class Archive_path_loaded(textual.signal.Signal):
+    def __init__(self, owner, name):
+        self.path_hierarchy = {}
+        self.complete = False
+
+        super().__init__(owner, name)
+
+    def publish(self, data):
+        super().publish(data)
+
+        if data is LOADING_DONE:
+            self.complete = True
+        else:
+            record_path(data, self.path_hierarchy, data.file_path.split(os.path.sep))
+
+
 @textual.work(thread=True)
-def add_archive_files(
-    browse_app, directory_list, config, repository, archive_name, list_path, root_directory, timer
-):
-    file_type_paths = borgmatic.actions.browse.archive.get_archive_files(
-        config, repository, archive_name, list_path
-    )
-    loading_option = directory_list.get_option('loading-indicator')
+def load_archive_files(browse_app, directory_list, config, repository, archive_name):
+    for path_type, file_path, link_target in borgmatic.actions.browse.archive.get_archive_files(
+        config, repository, archive_name
+    ):
+        directory_list.path_loaded.publish(Archive_path(path_type, file_path, link_target))
 
-    if not root_directory:
-        browse_app.call_from_thread(directory_list.remove_option, 'loading-indicator')
-        browse_app.call_from_thread(
-            directory_list.add_options,
-            (
-                textual.widgets.option_list.Option(
-                    f'{borgmatic.actions.browse.paths.PATH_TYPE_ICONS[borgmatic.actions.browse.paths.Path_type.DIRECTORY.value]} ..',
-                    id='..',
-                ),
-                loading_option,
-            ),
-        )
-
-    for path_type, file_path, link_target in file_type_paths:
-        pieces = (
-            borgmatic.actions.browse.paths.PATH_TYPE_ICONS.get(path_type, '❓'),
-            file_path,
-        ) + (('→', link_target) if link_target else ())
-        highlighted_option = directory_list.highlighted_option
-        sorted_options = sorted(
-            [
-                *directory_list.options,
-                textual.widgets.option_list.Option(' '.join(pieces), id=file_path),
-            ],
-            key=lambda option: ((option.id == 'loading-indicator'), option.prompt),
-        )
-        browse_app.call_from_thread(directory_list.set_options, sorted_options)
-        directory_list.highlighted = (
-            directory_list.get_option_index(highlighted_option.id)
-            if highlighted_option and directory_list.highlighted_option_changed
-            else 0
-        )
-
-    browse_app.call_from_thread(timer.stop)
-    browse_app.call_from_thread(directory_list.remove_option, 'loading-indicator')
+    directory_list.path_loaded.publish(LOADING_DONE)
 
 
 @textual.work(thread=True)

tests/integration/actions/browse/test_carousel.py

@@ -96,7 +96,10 @@ def test_make_next_panel_with_non_root_directory_list_and_selected_directory_opt
         flexmock()
     )
     focused_panel = borgmatic.actions.browse.panels.Directory_list(
-        config, config['repositories'][0], 'archive', path_components=('etc',),
+        config,
+        config['repositories'][0],
+        'archive',
+        path_components=('etc',),
     )
     flexmock(focused_panel).should_receive('get_option').and_return(flexmock(prompt='📁 borgmatic'))
 
@@ -123,7 +126,9 @@ def test_make_next_panel_with_root_directory_list_and_selected_file_option_retur
     focused_panel = borgmatic.actions.browse.panels.Directory_list(
         config, config['repositories'][0], 'archive'
     )
-    flexmock(focused_panel).should_receive('get_option').and_return(flexmock(prompt='📄 config.yaml'))
+    flexmock(focused_panel).should_receive('get_option').and_return(
+        flexmock(prompt='📄 config.yaml')
+    )
 
     directory_list = module.make_next_panel(focused_panel=focused_panel, option_id='config.yaml')
 
@@ -146,9 +151,14 @@ def test_make_next_panel_with_non_root_directory_list_and_selected_file_option_r
         flexmock()
     )
     focused_panel = borgmatic.actions.browse.panels.Directory_list(
-        config, config['repositories'][0], 'archive', path_components=('etc', 'borgmatic'),
+        config,
+        config['repositories'][0],
+        'archive',
+        path_components=('etc', 'borgmatic'),
+    )
+    flexmock(focused_panel).should_receive('get_option').and_return(
+        flexmock(prompt='📄 config.yaml')
     )
-    flexmock(focused_panel).should_receive('get_option').and_return(flexmock(prompt='📄 config.yaml'))
 
     directory_list = module.make_next_panel(focused_panel=focused_panel, option_id='config.yaml')
 
@@ -174,7 +184,9 @@ def test_make_next_panel_with_directory_list_and_unsupported_selected_option_ret
     focused_panel = borgmatic.actions.browse.panels.Directory_list(
         config, config['repositories'][0], 'archive'
     )
-    flexmock(focused_panel).should_receive('get_option').and_return(flexmock(prompt=f'{icon} config.yaml'))
+    flexmock(focused_panel).should_receive('get_option').and_return(
+        flexmock(prompt=f'{icon} config.yaml')
+    )
 
     assert module.make_next_panel(focused_panel=focused_panel, option_id='config.yaml') is None