refactor(lektor.assets): cache child assets on Directory

Optimization.

Since Assets are tied to a Pad, the cache lifetime will match that of
the Pad. This seems appropriate.

lektor/assets.py

@@ -1,17 +1,18 @@
 from __future__ import annotations
 
-import os
 import posixpath
 import warnings
+from collections import defaultdict
 from contextlib import suppress
 from pathlib import Path
 from pathlib import PurePosixPath
-from pathlib import PureWindowsPath
 from typing import Generator
 from typing import Iterable
 from typing import Sequence
 from typing import TYPE_CHECKING
 
+from werkzeug.utils import cached_property
+
 from lektor.sourceobj import SourceObject
 from lektor.utils import deprecated
 from lektor.utils import DeprecatedWarning
@@ -129,49 +130,44 @@ class Directory(Asset):
     """Represents a merged set of asset directories."""
 
     @property
-    def children(self) -> Generator[Asset, None, None]:
-        # XXX: this could probably be made more efficient
-        files = set()
-        for path in self._paths:
-            with suppress(OSError):
-                files.update(os.listdir(path))
-
-        for filename in files:
-            asset = self.get_child(filename)
-            if asset is not None:
-                yield asset
+    def children(self) -> Iterable[Asset]:
+        return self._children_by_name.values()
 
     def get_child(self, name: str, from_url: bool = False) -> Asset | None:
         if from_url:
             warnings.warn(_FROM_URL_DEPRECATED, stacklevel=2)
-        env = self.pad.env
+        return self._children_by_name.get(name)
 
-        if not _is_valid_path_component(name):
-            return None
-        if env.is_uninteresting_source_name(name):
-            return None
+    @cached_property
+    def _children_by_name(self) -> dict[str, Asset]:
+        return {asset.name: asset for asset in self._iter_children()}
 
-        candidates = tuple(
-            path / name for path in self._paths if path.joinpath(name).exists()
-        )
-        if len(candidates) == 0:
-            return None
-        if all(candidate.is_dir() for candidate in candidates):
-            # Merge candidate directories
-            return Directory(self.pad, parent=self, name=name, paths=candidates)
-        if not candidates[0].is_dir():
-            # If first candidate is not a directory, it shadows any other candidates
-            child_path = candidates[0]
-            asset_class = env.special_file_assets.get(child_path.suffix, File)
-            return asset_class(self.pad, parent=self, name=name, paths=(child_path,))
-
-        conflict_path = PurePosixPath(self.path, name)
-        warnings.warn(
-            f"Found both a directory and a non-directory at asset path {conflict_path}. "
-            "Ignoring all.",
-            stacklevel=2,
-        )
-        return None
+    def _iter_children(self) -> Generator[Asset, None, None]:
+        env = self.pad.env
+        candidates_by_name = defaultdict(list)
+        for path in self._paths:
+            with suppress(OSError):
+                for child in path.iterdir():
+                    if not env.is_uninteresting_source_name(child.name):
+                        candidates_by_name[child.name].append(child)
+
+        for name, candidates in candidates_by_name.items():
+            if not candidates[0].is_dir():
+                # If first candidate is not a directory, it shadows any other candidates
+                path = candidates[0]
+                asset_class = env.special_file_assets.get(path.suffix, File)
+                yield asset_class(self.pad, parent=self, name=name, paths=(path,))
+            elif all(candidate.is_dir() for candidate in candidates):
+                # Merge candidate directories
+                yield Directory(
+                    self.pad, parent=self, name=name, paths=tuple(candidates)
+                )
+            else:
+                warnings.warn(
+                    "Found both a directory and a non-directory at asset path "
+                    f"{PurePosixPath(self.path, name)}. Ignoring all.",
+                    stacklevel=3,
+                )
 
     @property
     def url_name(self) -> str:
@@ -210,15 +206,3 @@ def url_content_path(self) -> str:
 
 class File(Asset):
     """Represents a static asset file."""
-
-
-def _is_valid_path_component(comp: str) -> bool:
-    """Determine whether string is a valid component of an asset path."""
-    if comp in ("..", "."):
-        return False
-    # Check for and forbid pathsep, windows drive, reserved names, etc.
-    for path_class in (PurePosixPath, PureWindowsPath):
-        path = path_class(comp)
-        if len(path.parts) != 1 or path.anchor or path.is_reserved():
-            return False
-    return True

lektor/db.py

@@ -1689,7 +1689,7 @@ def get_root(self, alt=None):
 
     root = property(get_root)
 
-    @property
+    @cached_property
     def asset_root(self):
         """The root of the asset tree.
 

tests/test_assets.py

@@ -4,7 +4,6 @@
 
 import pytest
 
-from lektor.assets import _is_valid_path_component
 from lektor.assets import File
 from lektor.assets import get_asset
 from lektor.assets import get_asset_root
@@ -190,46 +189,6 @@ def test_asset_repr(asset, expected):
     assert repr(asset) == expected
 
 
-@pytest.mark.parametrize(
-    "comp",
-    [
-        "x",
-        "a.b",
-        "a_b",
-        "a@b",
-    ],
-)
-def test_is_valid_path_component_true(comp):
-    assert _is_valid_path_component(comp)
-
-
-@pytest.mark.parametrize(
-    "comp",
-    [
-        "",
-        ".",
-        "..",
-        "../x",
-        # abspath
-        "/x",
-        "//x",
-        r"\x",
-        r"\\x",
-        # pathsep
-        "x/y",
-        r"x\y",
-        # Windows drive
-        "c:foo",
-        "c:",
-        # Windows restricted file names
-        "nul",
-        "c:nul",
-    ],
-)
-def test_is_valid_path_component_false(comp):
-    assert not _is_valid_path_component(comp)
-
-
 @pytest.fixture
 def asset_paths(tmp_path):
     paths = tmp_path / "assets1", tmp_path / "assets2"