Text.
Text.
x
", "html.parser")) is None + >>> find_pmc_article_body(BeautifulSoup("x
", "html.parser")) is None True """ for class_name in PMC_ARTICLE_BODY_CLASSES: @@ -192,7 +198,7 @@ def _fetch_pmc_html(self, pmcid: str, config: ReferenceValidationConfig) -> Opti ) soup = BeautifulSoup(response.content, "html.parser") - article_body = _find_pmc_article_body(soup) + article_body = find_pmc_article_body(soup) if article_body: # The region is selected here, but the text comes out of the shared # extractor rather than a private copy of the paragraph walk, so diff --git a/src/linkml_reference_validator/etl/reference_fetcher.py b/src/linkml_reference_validator/etl/reference_fetcher.py index 5724d7d..af0a6d1 100644 --- a/src/linkml_reference_validator/etl/reference_fetcher.py +++ b/src/linkml_reference_validator/etl/reference_fetcher.py @@ -343,7 +343,18 @@ def fetch_with_provenance( # Check disk cache if not force_refresh: - cached = self._load_from_disk(normalized_reference_id) + cached = self._load_from_disk( + normalized_reference_id, + allow_stale=self.config.trust_cached_entries, + allow_stale_html=self.config.trust_cached_entries, + ) + if cached and self.config.trust_cached_entries: + # Trusted: serve what is on disk. No staleness re-fetch and no + # full-text retry, so nothing can shorten the body or replace it + # with a bot-check page. + return self._remember( + normalized_reference_id, FetchOutcome(content=cached) + ) if cached: # A record cached as abstract_only may predate full-text support, or # reflect a prior transient failure. Give the chain one more chance @@ -521,9 +532,12 @@ def _preserve_cached_full_text( cached.content_type, ) - # May its text be served? Asked without the bypass, so stale HTML is - # withheld here exactly as _stale_fallback withholds it, and validation - # falls back to the freshly fetched abstract. + # May its text be served? Asked with the consumer's serve policy, the + # same one _stale_fallback uses. By default stale HTML is withheld and + # validation falls back to the freshly fetched abstract; a consumer that + # has opted in gets the cached body instead. This is the path a refresh + # normally takes -- the source still answers, with an abstract -- so it + # is the one most consumers see, not _stale_fallback. servable = self._load_from_disk( normalized_reference_id, allow_stale=True, @@ -1536,8 +1550,11 @@ def _load_from_disk( ): logger.warning( "Refusing stale HTML full text for %s: it may be a repository " - "landing page. Retry when the source serves full text again to " - "repair it.", + "landing page. A re-fetch repairs it where the source still " + "serves full text -- but not for a PMC-only article, which " + "answers a Python HTTP client with a bot-check interstitial on " + "an HTTP 200. Set serve_stale_html_full_text if this cache is " + "one you review.", reference_id, ) return None diff --git a/src/linkml_reference_validator/etl/sources/pmid.py b/src/linkml_reference_validator/etl/sources/pmid.py index d2439a3..fff79e4 100644 --- a/src/linkml_reference_validator/etl/sources/pmid.py +++ b/src/linkml_reference_validator/etl/sources/pmid.py @@ -23,6 +23,7 @@ from bs4 import BeautifulSoup # type: ignore import requests # type: ignore +from linkml_reference_validator.etl.fulltext.pmc import find_pmc_article_body from linkml_reference_validator.models import ReferenceContent, ReferenceValidationConfig from linkml_reference_validator.etl.extract.html import HTMLExtractor from linkml_reference_validator.etl.extract import MIN_FULLTEXT_CHARS @@ -582,12 +583,7 @@ def _fetch_pmc_html( return None soup = BeautifulSoup(response.content, "html.parser") - # Shared with the PMC full-text provider: the container moved from a - # ``div`` to a ``section`` and changed class, so both call sites went - # blind at once (dismech#12672). - from linkml_reference_validator.etl.fulltext.pmc import _find_pmc_article_body - - article_body = _find_pmc_article_body(soup) + article_body = find_pmc_article_body(soup) if article_body: # Region selected here, text extracted by the shared extractor, for diff --git a/src/linkml_reference_validator/models.py b/src/linkml_reference_validator/models.py index 69fad13..ebc6106 100644 --- a/src/linkml_reference_validator/models.py +++ b/src/linkml_reference_validator/models.py @@ -524,6 +524,18 @@ class ReferenceValidationConfig(BaseModel): "the entry is left un-stamped so a later run still retries." ), ) + trust_cached_entries: bool = Field( + default=False, + description=( + "If True, use a cache entry as it is on disk and never re-fetch it " + "because an extractor stamp is missing or old. Off by default, so " + "caches still migrate. Set it for a committed, reviewed cache, where " + "a re-fetch is not an improvement but a risk: it can shorten a body, " + "cache a bot-check page, or replace text a curator quoted. Imperfect " + "cached text is a content problem to fix deliberately, not on every " + "validation run." + ), + ) full_text_providers: list[str] = Field( default_factory=lambda: ["pmc", "epmc_preprint", "unpaywall", "openalex"], description=( diff --git a/tests/test_pmc_modern_container.py b/tests/test_pmc_modern_container.py index 8944c5d..00ace8a 100644 --- a/tests/test_pmc_modern_container.py +++ b/tests/test_pmc_modern_container.py @@ -18,7 +18,7 @@ import pytest from bs4 import BeautifulSoup -from linkml_reference_validator.etl.fulltext.pmc import _find_pmc_article_body +from linkml_reference_validator.etl.fulltext.pmc import find_pmc_article_body BODY = "" + ("Real article prose. " * 40) + "
" @@ -34,7 +34,7 @@ ) def test_article_containers_are_found(markup, label): soup = BeautifulSoup(markup, "html.parser") - assert _find_pmc_article_body(soup) is not None, f"{label} should be recognised" + assert find_pmc_article_body(soup) is not None, f"{label} should be recognised" @pytest.mark.parametrize( @@ -49,11 +49,27 @@ def test_article_containers_are_found(markup, label): ) def test_pages_without_an_article_container_are_declined(markup, label): soup = BeautifulSoup(markup, "html.parser") - assert _find_pmc_article_body(soup) is None, f"{label} must not be accepted" + assert find_pmc_article_body(soup) is None, f"{label} must not be accepted" def test_a_bare_body_element_is_not_an_article_container(): """```` is on every page, so matching class="body" alone would accept an interstitial. Only the article-body classes count.""" soup = BeautifulSoup(f"{BODY}", "html.parser") - assert _find_pmc_article_body(soup) is None + assert find_pmc_article_body(soup) is None + + +def test_a_challenge_page_carrying_a_classed_body_is_still_declined(): + """The interstitial arrives on an HTTP 200, so this selector is the only check. + + PMC's challenge page carries a classed ````. None of its classes may + overlap the article-body list, or a bot-check page gets cached as full text. + """ + markup = ( + '' + "Enable JavaScript and cookies to continue.
" + "" + ) + soup = BeautifulSoup(markup, "html.parser") + assert find_pmc_article_body(soup) is None diff --git a/tests/test_serve_stale_html_optin.py b/tests/test_serve_stale_html_optin.py index d0a587d..7c7dadc 100644 --- a/tests/test_serve_stale_html_optin.py +++ b/tests/test_serve_stale_html_optin.py @@ -22,6 +22,7 @@ from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher from linkml_reference_validator.models import ( + ReferenceContent, ReferenceValidationConfig, ) @@ -73,3 +74,61 @@ def test_opting_in_does_not_stamp_or_rewrite_the_entry(tmp_path): text = (tmp_path / "PMID_30929739.md").read_text(encoding="utf-8") assert "extractor_version" not in text assert BODY_SENTENCE in text + + +def _fetch_with_abstract_source(tmp_path, **cfg): + """The path a refresh normally takes: the source answers, with an abstract. + + ``_stale_fallback`` needs the source to yield *nothing*. For a PMC-only + article PubMed still returns the abstract, so the refresh reaches + ``_preserve_cached_full_text`` instead: the overwrite is refused, and then a + separate question decides what may be served. That second question is the + one the flag has to answer, and it is the one dismech actually hits. + """ + _cache(tmp_path) + fetcher = ReferenceFetcher( + ReferenceValidationConfig( + cache_dir=tmp_path, email="me@example.org", fetch_full_text=False, **cfg + ) + ) + fresh = ReferenceContent( + reference_id="PMID:30929739", + content="Just the abstract.", + content_type="abstract_only", + title="A paper", + ) + + class _Source: + def fetch(self, identifier, config): + return fresh + + with patch( + "linkml_reference_validator.etl.reference_fetcher" + ".ReferenceSourceRegistry.get_source", + return_value=_Source, + ): + return fetcher.fetch("PMID:30929739") + + +def test_refused_overwrite_serves_the_abstract_by_default(tmp_path): + content = _fetch_with_abstract_source(tmp_path) + assert content is not None + assert BODY_SENTENCE not in (content.content or ""), ( + "by default the cached body stays withheld even though it was preserved" + ) + assert "Just the abstract." in (content.content or "") + + +def test_refused_overwrite_serves_the_cached_body_when_opted_in(tmp_path): + content = _fetch_with_abstract_source(tmp_path, serve_stale_html_full_text=True) + assert content is not None + assert BODY_SENTENCE in (content.content or "") + + +def test_refused_overwrite_leaves_the_file_alone_either_way(tmp_path): + """Serving is not overwriting: the entry keeps its body and stays un-stamped.""" + for flag in (False, True): + _fetch_with_abstract_source(tmp_path, serve_stale_html_full_text=flag) + text = (tmp_path / "PMID_30929739.md").read_text(encoding="utf-8") + assert BODY_SENTENCE in text, f"body lost with flag={flag}" + assert "extractor_version" not in text, f"entry stamped with flag={flag}" diff --git a/tests/test_trust_cached_entries.py b/tests/test_trust_cached_entries.py new file mode 100644 index 0000000..a1a3692 --- /dev/null +++ b/tests/test_trust_cached_entries.py @@ -0,0 +1,81 @@ +"""A committed cache can be used as it is, without a re-fetch on every run. + +Staleness re-fetching is right for a cache the tool owns and can rebuild. For a +cache that is committed and reviewed it inverts: the re-fetch is the risk, not +the repair. It can shorten a body (dismech#12879), replace an entry with a +bot-check page served on an HTTP 200 (dismech#12867), or withhold text that +nothing can recover (dismech#12672). + +`trust_cached_entries` says use the file. Off by default, so caches still +migrate for consumers that want that. +""" + +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher +from linkml_reference_validator.models import ReferenceValidationConfig + +BODY = "A sentence a curator quoted, present only in this cached body." + + +def _cache(tmp_path, content_type="full_text_html"): + (tmp_path / "PMID_20301575.md").write_text( + "---\nreference_id: PMID:20301575\ntitle: A paper\n" + f"content_type: {content_type}\n---\n\n## Content\n\n{BODY}\n", + encoding="utf-8", + ) + + +def _fetch(tmp_path, trust): + fetcher = ReferenceFetcher( + ReferenceValidationConfig( + cache_dir=tmp_path, email="me@example.org", trust_cached_entries=trust + ) + ) + calls = [] + + class _Source: + def fetch(self, identifier, config): + calls.append(identifier) + return None + + with patch( + "linkml_reference_validator.etl.reference_fetcher" + ".ReferenceSourceRegistry.get_source", + return_value=_Source, + ): + return fetcher.fetch("PMID:20301575"), calls + + +@pytest.mark.parametrize( + "content_type", ["full_text_html", "full_text_xml", "abstract_only"] +) +def test_a_trusted_entry_is_served_without_any_fetch(tmp_path, content_type): + """The point: no network call at all, so nothing can go wrong during one.""" + _cache(tmp_path, content_type) + content, calls = _fetch(tmp_path, trust=True) + + assert content is not None, f"{content_type} should be served from disk" + assert BODY in (content.content or "") + assert calls == [], f"a trusted entry must not be re-fetched, got {calls}" + + +def test_the_file_is_left_exactly_as_it_was(tmp_path): + """Nothing is rewritten, so a validation run leaves the cache untouched.""" + _cache(tmp_path) + before = (tmp_path / "PMID_20301575.md").read_text(encoding="utf-8") + _fetch(tmp_path, trust=True) + assert (tmp_path / "PMID_20301575.md").read_text(encoding="utf-8") == before + + +def test_the_default_still_re_fetches(tmp_path): + """Off by default: an unstamped entry is still treated as stale.""" + _cache(tmp_path) + _content, calls = _fetch(tmp_path, trust=False) + assert calls == ["20301575"], ( + "by default an unstamped entry is re-fetched; the flag must be opt-in" + )