Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions src/linkml_reference_validator/etl/fulltext/pmc.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,16 @@ class TransientFullTextError(RuntimeError):
#: same element. Matching it alone would match every page's ``<body>``, and the
#: structural test is the only thing standing between this fetch and caching a
#: bot-check interstitial served on an HTTP 200.
#:
#: Order is a priority order, not an alphabetical one: the first match wins, so
#: the current wrapper is tried before the legacy ones. It matters for a legacy
#: page carrying several ``tsec`` sections, where only the first is returned --
#: the previous code behaved the same way, so this is a known limit rather than
#: a regression, but reordering the tuple would change which section that is.
PMC_ARTICLE_BODY_CLASSES = ("main-article-body", "article-body", "tsec")


def _find_pmc_article_body(soup: BeautifulSoup) -> Optional[Any]:
def find_pmc_article_body(soup: BeautifulSoup) -> Optional[Any]:
"""Return PMC's article-body element, or None if the page has none.

Matches on class rather than element name: the container moved from ``div``
Expand All @@ -65,9 +71,9 @@ def _find_pmc_article_body(soup: BeautifulSoup) -> Optional[Any]:
Examples:
>>> from bs4 import BeautifulSoup
>>> html = '<section class="body main-article-body"><p>Text.</p></section>'
>>> _find_pmc_article_body(BeautifulSoup(html, "html.parser")) is not None
>>> find_pmc_article_body(BeautifulSoup(html, "html.parser")) is not None
True
>>> _find_pmc_article_body(BeautifulSoup("<body><p>x</p></body>", "html.parser")) is None
>>> find_pmc_article_body(BeautifulSoup("<body><p>x</p></body>", "html.parser")) is None
True
"""
for class_name in PMC_ARTICLE_BODY_CLASSES:
Expand Down Expand Up @@ -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
Expand Down
29 changes: 23 additions & 6 deletions src/linkml_reference_validator/etl/reference_fetcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions src/linkml_reference_validator/etl/sources/pmid.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions src/linkml_reference_validator/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=(
Expand Down
24 changes: 20 additions & 4 deletions tests/test_pmc_modern_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "<p>" + ("Real article prose. " * 40) + "</p>"

Expand All @@ -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(
Expand All @@ -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():
"""``<body>`` is on every page, so matching class="body" alone would accept
an interstitial. Only the article-body classes count."""
soup = BeautifulSoup(f"<html><body>{BODY}</body></html>", "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 ``<body>``. None of its classes may
overlap the article-body list, or a bot-check page gets cached as full text.
"""
markup = (
'<html><body class="usa-page bot-check">'
"<h1>Checking your browser before accessing</h1>"
"<p>Enable JavaScript and cookies to continue.</p>"
"</body></html>"
)
soup = BeautifulSoup(markup, "html.parser")
assert find_pmc_article_body(soup) is None
59 changes: 59 additions & 0 deletions tests/test_serve_stale_html_optin.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

from linkml_reference_validator.etl.reference_fetcher import ReferenceFetcher
from linkml_reference_validator.models import (
ReferenceContent,
ReferenceValidationConfig,
)

Expand Down Expand Up @@ -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}"
81 changes: 81 additions & 0 deletions tests/test_trust_cached_entries.py
Original file line number Diff line number Diff line change
@@ -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"
)
Loading