Problems with Page.source_filename, Page.iter_source_filenames with alternatives are enabled

CHANGES.md

@@ -23,6 +23,32 @@ It has been rewritten in Typescript, and updated to use v5 of the Bootstrap CSS
   upscaling behavior in cases that do not involve upscaling. [#885][]
 - Fix bug with translation fallback of record label. [#897][]
 
+##### Dependency Tracking
+
+- Add new `_source_filenames` system field. This contains a `stringlist` of
+  the filesystem paths to all of the `.lr` files which are contributing to this
+  data record. (This only lists existing files.) [#959][]
+- Fix `Page.source_filename` so
+  that it always returns the name of an actually existing source
+  file. Previously, for a page with an _alt_ set, `source_filename`
+  was returning the name corresponding to the alt-specific `.lr` file
+  (e.g. `contents+de.lr`) even if no such file existed. [#959][]
+- Fix `Page.iter_source_filenames` so
+  that it only returns the names of actually existing source
+  files. [#959][]
+- Similarly, fix `Attachment.source_filename` and
+  `Attachment.iter_source_filenames` to only return the names of
+  existing source files. [#959][]
+- Fix `Siblings.iter_source_filenames` so that it returns all
+  sources of the previous and next pages, not just the primary sources
+  of those pages. [#959][]
+- Fix `VirtualSourceObject.iter_source_filenames`
+  to return the source filenames of the underlying record. [#959][]
+- Change `VirtualSourceObject.get_checksum` return value. It used to
+  return a concatenation of all the source filenames and their
+  checksums. Now we return the SHA1 hash of that
+  concatenation. [#959][]
+
 #### Data Modelling
 
 - Fixed pagination issue which caused child-less paginated pages to
@@ -214,6 +240,7 @@ It has been rewritten in Typescript, and updated to use v5 of the Bootstrap CSS
 [#942]: https://github.com/lektor/lektor/pull/942
 [#945]: https://github.com/lektor/lektor/pull/945
 [#952]: https://github.com/lektor/lektor/pull/952
+[#959]: https://github.com/lektor/lektor/pull/959
 
 ## 3.2.3 (2021-12-11)
 

lektor/assets.py

@@ -42,6 +42,9 @@ def __init__(self, pad, name, path=None, parent=None):
         self.name = name
         self.parent = parent
 
+    def iter_source_filenames(self):
+        yield self.source_filename
+
     @property
     def url_name(self):
         name = self.name

lektor/datamodel.py

@@ -704,6 +704,9 @@ def add_system_field(name, **opts):
 # The alt key that identifies this record
 add_system_field("_alt", type="string")
 
+# The filenames for the .lr files that contain the record's data
+add_system_field("_source_filenames", type="strings")
+
 # The alt key for the file that was actually referenced.
 add_system_field("_source_alt", type="string")
 

lektor/db.py

@@ -101,14 +101,6 @@ def _require_ctx(record):
     return ctx
 
 
-def _is_content_file(filename, alt=PRIMARY_ALT):
-    if filename == "contents.lr":
-        return True
-    if alt != PRIMARY_ALT and filename == "contents+%s.lr" % alt:
-        return True
-    return False
-
-
 class _CmpHelper:
     def __init__(self, value, reverse):
         self.value = value
@@ -518,24 +510,25 @@ def prev_page(self):
     def next_page(self):
         return self._next_page
 
-    def iter_source_filenames(self):
+    def _pages(self):
         for page in self._prev_page, self._next_page:
             if page:
-                yield page.source_filename
+                yield page
+
+    def iter_source_filenames(self):
+        return chain.from_iterable(
+            page.iter_source_filenames() for page in self._pages()
+        )
 
     def _file_infos(self, path_cache):
-        for page in self._prev_page, self._next_page:
-            if page:
-                yield path_cache.get_file_info(page.source_filename)
+        return map(path_cache.get_file_info, self.iter_source_filenames())
 
     def get_mtime(self, path_cache):
-        mtimes = [i.mtime for i in self._file_infos(path_cache)]
-        return max(mtimes) if mtimes else None
+        return max((i.mtime for i in self._file_infos(path_cache)), default=None)
 
     def get_checksum(self, path_cache):
         sums = "|".join(i.filename_and_checksum for i in self._file_infos(path_cache))
-
-        return sums or None
+        return hashlib.sha1(sums.encode("utf-8")).hexdigest() if sums else None
 
 
 def siblings_resolver(node, url_path):
@@ -563,18 +556,8 @@ def record(self):
             self["_path"], persist=self.pad.cache.is_persistent(self), alt=self.alt
         )
 
-    @property
-    def source_filename(self):
-        if self.alt != PRIMARY_ALT:
-            return os.path.join(
-                self.pad.db.to_fs_path(self["_path"]), "contents+%s.lr" % self.alt
-            )
-        return os.path.join(self.pad.db.to_fs_path(self["_path"]), "contents.lr")
-
     def iter_source_filenames(self):
-        yield self.source_filename
-        if self.alt != PRIMARY_ALT:
-            yield os.path.join(self.pad.db.to_fs_path(self["_path"]), "contents.lr")
+        return iter(self["_source_filenames"])
 
     @property
     def url_path(self):
@@ -714,14 +697,6 @@ class Attachment(Record):
 
     is_attachment = True
 
-    @property
-    def source_filename(self):
-        if self.alt != PRIMARY_ALT:
-            suffix = "+%s.lr" % self.alt
-        else:
-            suffix = ".lr"
-        return self.pad.db.to_fs_path(self["_path"]) + suffix
-
     def _is_considered_hidden(self):
         # Attachments are only considered hidden if they have been
         # configured as such.  This means that even if a record itself is
@@ -754,7 +729,8 @@ def get_fallback_record_label(self, lang):
         return self["_id"]
 
     def iter_source_filenames(self):
-        yield self.source_filename
+        for fn in super().iter_source_filenames():
+            yield fn
         yield self.attachment_filename
 
 
@@ -993,7 +969,7 @@ def __get_lektor_param_hash__(self, h):
         h.update(str(self.alt))
         h.update(str(self._include_pages))
         h.update(str(self._include_attachments))
-        h.update("(%s)" % u"|".join(self._order_by or ()).encode("utf-8"))
+        h.update("(%s)" % "|".join(self._order_by or ()).encode("utf-8"))
         h.update(str(self._limit))
         h.update(str(self._offset))
         h.update(str(self._include_hidden))
@@ -1329,47 +1305,49 @@ def load_raw_data(self, path, alt=PRIMARY_ALT, cls=None, fallback=True):
         fn_base = self.to_fs_path(path)
 
         rv = cls()
-        rv_type = None
+        is_attachment = None
+        source_filenames = []
 
         choiceiter = _iter_filename_choices(
             fn_base, [alt], self.config, fallback=fallback
         )
-        for fs_path, source_alt, is_attachment in choiceiter:
+        for fs_path, source_alt, for_attachment in choiceiter:
             # If we already determined what our return value is but the
             # type mismatches what we try now, we have to abort.  Eg:
             # a page can not become an attachment or the other way round.
-            if rv_type is not None and rv_type != is_attachment:
+            if len(source_filenames) > 0 and for_attachment != is_attachment:
                 break
 
             try:
                 with open(fs_path, "rb") as f:
-                    if rv_type is None:
-                        rv_type = is_attachment
+                    if len(source_filenames) == 0:
+                        is_attachment = for_attachment
+                    source_filenames.append(fs_path)
+                    rv.setdefault("_source_alt", source_alt)
                     for key, lines in metaformat.tokenize(f, encoding="utf-8"):
                         if key not in rv:
-                            rv[key] = u"".join(lines)
+                            rv[key] = "".join(lines)
             except IOError as e:
                 if e.errno not in (errno.ENOTDIR, errno.ENOENT):
                     raise
-                if not is_attachment or not os.path.isfile(fs_path[:-3]):
-                    continue
-                # Special case: we are loading an attachment but the meta
-                # data file does not exist.  In that case we still want to
-                # record that we're loading an attachment.
-                if is_attachment:
-                    rv_type = True
-
-            if "_source_alt" not in rv:
-                rv["_source_alt"] = source_alt
-
-        if rv_type is None:
-            return None
+
+        if len(source_filenames) == 0:
+            # No data found
+            if os.path.isfile(fn_base):
+                # Special case: we are loading an attachment
+                # but the meta data file does not exist.  In
+                # that case we still want to record that we're
+                # loading an attachment.
+                is_attachment = True
+            else:
+                return None
 
         rv["_path"] = path
         rv["_id"] = posixpath.basename(path)
         rv["_gid"] = hashlib.md5(path.encode("utf-8")).hexdigest()
         rv["_alt"] = alt
-        if rv_type:
+        rv["_source_filenames"] = "\n".join(source_filenames)  # FIXME: hackish
+        if is_attachment:
             rv["_attachment_for"] = posixpath.dirname(path)
 
         return rv
@@ -1913,7 +1891,7 @@ class Alt:
     def __init__(self, id, record):
         self.id = id
         self.record = record
-        self.exists = record is not None and os.path.isfile(record.source_filename)
+        self.exists = record is not None and record["_source_alt"] == record.alt
 
     def __repr__(self):
         return "<Alt %r%s>" % (self.id, self.exists and "*" or "")

lektor/editor.py

@@ -11,7 +11,17 @@
 from lektor.utils import secure_filename
 
 
-implied_keys = set(["_id", "_path", "_gid", "_alt", "_source_alt", "_attachment_for"])
+implied_keys = set(
+    [
+        "_id",
+        "_path",
+        "_gid",
+        "_alt",
+        "_source_alt",
+        "_source_filenames",
+        "_attachment_for",
+    ]
+)
 possibly_implied_keys = set(["_model", "_template", "_attachment_type"])
 
 

lektor/sourceobj.py

@@ -23,7 +23,17 @@ def alt(self):
 
     @property
     def source_filename(self):
-        """The primary source filename of this source object."""
+        """The primary source filename of this source object.
+
+        XXX: This is here for backword compatibility.  The default implementation
+        returns the first value returned by ``iter_source_filenames``.
+
+        (In most/all cases where one is concerned about the source files,
+        one should be interested in all the source files, not just one.)
+
+        """
+        source_filenames = self.iter_source_filenames()
+        return next(iter(source_filenames), None)
 
     is_hidden = False
     is_discoverable = True
@@ -39,9 +49,12 @@ def is_undiscoverable(self):
         return not self.is_discoverable
 
     def iter_source_filenames(self):
-        fn = self.source_filename
-        if fn is not None:
-            yield self.source_filename
+        """An iterable of the source filenames for this source object.
+
+        The first returned filename should be the "primary" one.
+        """
+        # pylint: disable=no-self-use
+        return []
 
     def iter_virtual_sources(self):
         # pylint: disable=no-self-use
@@ -163,5 +176,8 @@ def alt(self):
     def source_filename(self):
         return self.record.source_filename
 
+    def iter_source_filenames(self):
+        return self.record.iter_source_filenames()
+
     def iter_virtual_sources(self):
         yield self

tests/demo-project/content/projects/english/contents+en.lr

@@ -0,0 +1,7 @@
+name: English
+---
+website: http://en.example.org/
+---
+description: A page that only exists in English
+---
+seq: 10