File Tree Diff: hash the content of a page, not its HTML

Attributes rendered into the HTML change from build to build without the
page changing, so pages nobody touched were reported as modified. Hash the
text of the main node and its link and image targets instead, and version
the manifest so manifests built with different algorithms aren't compared.

Author: ericholscher

readthedocs/filetreediff/__init__.py

@@ -85,6 +85,17 @@ def get_diff(current_version: Version, base_version: Version) -> FileTreeDiff |
         if base_latest_build.id != base_version_manifest.build.id:
             outdated = True
 
+    if current_version_manifest.hash_version != base_version_manifest.hash_version:
+        # Manifests generated with different versions of the hashing algorithm
+        # can't be compared, every file would show up as modified.
+        # This resolves itself once both versions have been built again.
+        log.info(
+            "Skipping file tree diff, manifests use different hash versions.",
+            current_version_hash_version=current_version_manifest.hash_version,
+            base_version_hash_version=base_version_manifest.hash_version,
+        )
+        return None
+
     current_version_file_paths = set(current_version_manifest.files.keys())
     base_version_file_paths = set(base_version_manifest.files.keys())
 

readthedocs/filetreediff/dataclasses.py

@@ -11,6 +11,13 @@
 from readthedocs.core.resolver import Resolver
 
 
+# Version of the algorithm used to generate the ``main_content_hash`` of each file.
+# Bump it when the way we hash the content of a page changes, so manifests
+# generated with different versions of the algorithm aren't compared to each
+# other (every file would show up as modified).
+MAIN_CONTENT_HASH_VERSION = 1
+
+
 @dataclass(slots=True)
 class FileTreeDiffBuild:
     """The build associated with a file tree manifest."""
@@ -32,10 +39,17 @@ class FileTreeDiffManifest:
 
     files: dict[str, FileTreeDiffManifestFile]
     build: FileTreeDiffBuild
+    hash_version: int
 
-    def __init__(self, build_id: int, files: list[FileTreeDiffManifestFile]):
+    def __init__(
+        self,
+        build_id: int,
+        files: list[FileTreeDiffManifestFile],
+        hash_version: int = MAIN_CONTENT_HASH_VERSION,
+    ):
         self.build = FileTreeDiffBuild(id=build_id)
         self.files = {file.path: file for file in files}
+        self.hash_version = hash_version
 
     @classmethod
     def from_dict(cls, data: dict) -> "FileTreeDiffManifest":
@@ -46,11 +60,14 @@ def from_dict(cls, data: dict) -> "FileTreeDiffManifest":
         converting the object to a dictionary using the `as_dict` method.
         """
         build_id = data["build"]["id"]
+        # Manifests generated before the hash version was introduced don't have
+        # the field, they all used the same (first) version of the algorithm.
+        hash_version = data.get("hash_version", 0)
         files = [
             FileTreeDiffManifestFile(path=path, main_content_hash=file["main_content_hash"])
             for path, file in data["files"].items()
         ]
-        return cls(build_id, files)
+        return cls(build_id, files, hash_version)
 
     def as_dict(self) -> dict:
         """Convert the object to a dictionary."""

readthedocs/filetreediff/tests/test_filetreediff.py

@@ -8,6 +8,7 @@
 from readthedocs.builds.constants import BUILD_STATE_FINISHED, EXTERNAL, LATEST
 from readthedocs.builds.models import Build, Version
 from readthedocs.filetreediff import get_diff, snapshot_base_manifest
+from readthedocs.filetreediff.dataclasses import MAIN_CONTENT_HASH_VERSION
 from readthedocs.projects.models import Project
 from readthedocs.rtd_tests.storage import BuildMediaFileSystemStorageTest
 
@@ -22,11 +23,16 @@ def f(*args, **kwargs):
     return f
 
 
-def _mock_manifest(build_id: int, files: dict[str, str]):
+def _mock_manifest(
+    build_id: int,
+    files: dict[str, str],
+    hash_version: int = MAIN_CONTENT_HASH_VERSION,
+):
     return _mock_open(
         json.dumps(
             {
                 "build": {"id": build_id},
+                "hash_version": hash_version,
                 "files": {
                     path: {"main_content_hash": content_hash}
                     for path, content_hash in files.items()
@@ -122,6 +128,18 @@ def test_diff_changes(self, storage_open):
         assert [file.path for file in diff.modified] == ["tutorials/index.html"]
         assert not diff.outdated
 
+    @mock.patch.object(BuildMediaFileSystemStorageTest, "open")
+    def test_diff_manifests_with_different_hash_versions(self, storage_open):
+        files = {
+            "index.html": "hash1",
+            "tutorials/index.html": "hash2",
+        }
+        storage_open.side_effect = [
+            _mock_manifest(self.build_a.id, files, hash_version=2)(),
+            _mock_manifest(self.build_b.id, files, hash_version=1)(),
+        ]
+        assert get_diff(self.version_a, self.version_b) is None
+
     @mock.patch.object(BuildMediaFileSystemStorageTest, "open")
     def test_missing_manifest(self, storage_open):
         storage_open.side_effect = FileNotFoundError

readthedocs/search/parsers.py

@@ -337,6 +337,27 @@ def _clean_body(self, body):
 
         return body
 
+    def _get_main_content_hash(self, body):
+        """
+        Get a hash of the main content of the page.
+
+        The hash is used to detect if a page has changed between two builds
+        (see the ``readthedocs.filetreediff`` module), so it's built from the
+        parts of the page that a reader would notice: its text, and the targets
+        of its links and images.
+
+        The HTML itself isn't hashed, as its attributes hold values that change
+        from one build to the next without the page changing at all. The version
+        of the project an intersphinx link points to is one example, it's
+        rendered into the ``title`` attribute of every external reference.
+        """
+        content = [" ".join(body.text(separator=" ").split())]
+        for node in body.css("[href]"):
+            content.append("href=" + (node.attributes.get("href") or ""))
+        for node in body.css("[src]"):
+            content.append("src=" + (node.attributes.get("src") or ""))
+        return hashlib.md5("\n".join(content).encode()).hexdigest()
+
     def _is_section(self, tag):
         """
         Check if `tag` is a section (linkeable header).
@@ -479,8 +500,8 @@ def _process_content(self, page, content):
         sections = []
         main_content_hash = None
         if body:
-            main_content_hash = hashlib.md5(body.html.encode()).hexdigest()
             body = self._clean_body(body)
+            main_content_hash = self._get_main_content_hash(body)
             title = self._get_page_title(body, html) or page
             sections = self._get_sections(title=title, body=body)
         else:

readthedocs/search/tests/data/generic/out/basic.json

@@ -2,7 +2,7 @@
   {
     "path": "basic.html",
     "title": "Title of the page",
-    "main_content_hash": "006cb661925c211be260be17d64662b5",
+    "main_content_hash": "ae7bd4075a928bf3d01110ef074536df",
     "sections": [
       {
         "id": "love",

readthedocs/search/tests/data/mkdocs/out/gitbook.json

@@ -2,7 +2,7 @@
   {
     "path": "index.html",
     "title": "Mkdocs - GitBook Theme",
-    "main_content_hash": "a2050380ef001ef57754fb35b41d477d",
+    "main_content_hash": "0e7044baeb4e81314d7ca7dcc3520ccc",
     "sections": [
       {
         "id": "mkdocs-gitbook-theme",

readthedocs/search/tests/data/mkdocs/out/material.json

@@ -2,7 +2,7 @@
   {
     "path": "index.html",
     "title": "Overview",
-    "main_content_hash": "448eb15bfef60bc49a4059a34ebeaa4b",
+    "main_content_hash": "8e0837bd6812a5ad4f4ca0a8edc7bd37",
     "sections": [
       {
         "id": "",

readthedocs/search/tests/data/mkdocs/out/mkdocs-1.1.json

@@ -2,7 +2,7 @@
   {
     "path": "index.html",
     "title": "MkDocs",
-    "main_content_hash": "aca1f625e559fe49718abe801532b3ef",
+    "main_content_hash": "0c0974edad01cd3beaa61c0046f34b14",
     "sections": [
       {
         "id": "mkdocs",
@@ -39,7 +39,7 @@
   {
     "path": "404.html",
     "title": "404",
-    "main_content_hash": "2f582057ff73758c152d6925eb149099",
+    "main_content_hash": "7f5c24be74faf261f5aa35b567b838bf",
     "sections": [
       {
         "id": "404-page-not-found",
@@ -51,7 +51,7 @@
   {
     "path": "configuration.html",
     "title": "Configuration",
-    "main_content_hash": "6a5526f41cfef3da4c541f0465ac42e8",
+    "main_content_hash": "da702fc5778f6e9198781964bc734ab5",
     "sections": [
       {
         "id": "configuration",
@@ -93,7 +93,7 @@
   {
     "path": "no-title.html",
     "title": "No title - Read the Docs MkDocs Test",
-    "main_content_hash": "c02e4800b9da5e96ed35841f11072777",
+    "main_content_hash": "a7a8e577ce57e09847174516c22bffbf",
     "sections": [
       {
         "id": "",
@@ -105,7 +105,7 @@
   {
     "path": "no-main-header.html",
     "title": "I'm the header",
-    "main_content_hash": "43010c91ae89c036b157c2a3947d63b8",
+    "main_content_hash": "1bda704ab824e1a72c9d5b60dfc21090",
     "sections": [
       {
         "id": "",

readthedocs/search/tests/data/mkdocs/out/readthedocs-1.1.json

@@ -2,7 +2,7 @@
   {
     "path": "index.html",
     "title": "Read the Docs MkDocs Test Project",
-    "main_content_hash": "25c48df9b31b31eaee235206a48239a5",
+    "main_content_hash": "eba9d67d8863ea87e36e9ecd88156a71",
     "sections": [
       {
         "id": "read-the-docs-mkdocs-test-project",
@@ -29,7 +29,7 @@
   {
     "path": "404.html",
     "title": "404",
-    "main_content_hash": "5f6d687421dad7e1409dcaa3ccd2d34d",
+    "main_content_hash": "7f5c24be74faf261f5aa35b567b838bf",
     "sections": [
       {
         "id": "404-page-not-found",
@@ -41,7 +41,7 @@
   {
     "path": "versions.html",
     "title": "Versions & Themes",
-    "main_content_hash": "68a6609034e0a45adb135a60b98c12b8",
+    "main_content_hash": "8b5245086e11b33a5a52709e4a804ff6",
     "sections": [
       {
         "id": "versions-themes",

readthedocs/search/tests/data/mkdocs/out/windmill.json

@@ -2,7 +2,7 @@
   {
     "path": "index.html",
     "title": "Windmill theme",
-    "main_content_hash": "68542e8759d648c4304f0e1ca85a0547",
+    "main_content_hash": "0036900dcea4d39c40e4829c069f56ed",
     "sections": [
       {
         "id": "windmill-theme",