From a05ff6c7d951fec424735fbe0ba52eca011b96f6 Mon Sep 17 00:00:00 2001 From: Graham Hukill Date: Mon, 10 Aug 2026 11:34:09 -0400 Subject: [PATCH 1/4] Refactor LibGuidesAPIClient to helpers Why these changes are being introduced: With the proposed introduction of a new ReseaarchDatabases transformation class that may also use the LibGuides API, it makes sense to have it refactored out of libguides.py. How this addresses that need: The LibGuidesAPIClient is refactored to helpers.py. Side effects of this change: * None Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/TIMX-655 --- tests/sources/json/test_libguides.py | 2 +- transmogrifier/helpers.py | 92 +++++++++++++++++++++++ transmogrifier/sources/json/libguides.py | 93 +----------------------- 3 files changed, 94 insertions(+), 93 deletions(-) diff --git a/tests/sources/json/test_libguides.py b/tests/sources/json/test_libguides.py index fcb2e441..2fdd1721 100644 --- a/tests/sources/json/test_libguides.py +++ b/tests/sources/json/test_libguides.py @@ -401,7 +401,7 @@ def test_libguides_api_client_fetch_guides_expands_sub_pages_into_rows(): mock_response.json.return_value = mock_api_response with patch( - "transmogrifier.sources.json.libguides.requests.get", + "transmogrifier.helpers.requests.get", return_value=mock_response, ): df = client.fetch_guides("fake-token") diff --git a/transmogrifier/helpers.py b/transmogrifier/helpers.py index da3d661f..4ded49f1 100644 --- a/transmogrifier/helpers.py +++ b/transmogrifier/helpers.py @@ -1,7 +1,12 @@ import logging +import re from datetime import UTC, datetime +import pandas as pd +import requests + import transmogrifier.models as timdex +from transmogrifier import config from transmogrifier.config import DATE_FORMATS logger = logging.getLogger(__name__) @@ -131,3 +136,90 @@ def validate_date_range( end_date, ) return False + + +class LibGuidesAPIClient: + """Client for LibGuides API communication and data retrieval. + + This class retrieves metadata about all LibGuides via an API, retrieving data that is + not found in the OAI-PMH XML records or the websites themselves. This valuable data + is used during transformation to identify records for exclusion, occasionally + provide friendlier URLs, and other data augmentation. + + This class is instantiated as a singleton object in this module. Once instantiated, + it is attached to the Libguides transformer instance. This allows class methods + on the transformer to access cached data from this singleton object, ultimately + resulting in only a single API call per multiple record transformation run. + + This class relies on two environment variables: + - LIBGUIDES_CLIENT_ID + - LIBGUIDES_API_TOKEN + """ + + def __init__(self) -> None: + if not config.LIBGUIDES_CLIENT_ID: + raise RuntimeError("Required env var 'LIBGUIDES_CLIENT_ID' is not set") + if not config.LIBGUIDES_API_TOKEN: + raise RuntimeError("Required env var 'LIBGUIDES_API_TOKEN' is not set") + + self.client_id = str(config.LIBGUIDES_CLIENT_ID) + self.client_secret = config.LIBGUIDES_API_TOKEN + self._api_guides_df: pd.DataFrame | None = None + + @property + def api_guides_df(self) -> pd.DataFrame: + if self._api_guides_df is None: + self._api_guides_df = self.fetch_guides(self.get_api_token()) + return self._api_guides_df + + def get_api_token(self) -> str: + data = { + "grant_type": "client_credentials", + "client_id": self.client_id, + "client_secret": self.client_secret, + } + response = requests.post( + config.LIBGUIDES_TOKEN_URL, headers={}, data=data, timeout=60 + ) + response.raise_for_status() + payload = response.json() + return payload.get("access_token") + + def fetch_guides(self, token: str) -> pd.DataFrame: + """Retrieve metadata for all LibGuides. + + Each guide may contain a 'pages' key with a list of sub-page dicts. These + sub-pages are expanded into their own rows in the returned DataFrame, inheriting + any columns from the parent guide that the sub-page does not have. + """ + logger.debug("Retrieving all guides from Libguides API.") + headers = {"Authorization": f"Bearer {token}"} + response = requests.get(config.LIBGUIDES_GUIDES_URL, headers=headers, timeout=60) + response.raise_for_status() + guides = response.json() + + all_rows: list[dict] = [] + for guide in guides: + pages = guide.get("pages", []) + all_rows.append(guide) + for page in pages: + # inherit parent columns, then overlay page-specific columns + page_row = {**guide, **page} + all_rows.append(page_row) + + return pd.DataFrame(all_rows) + + def get_guide_by_url(self, url: str) -> pd.Series: + """Get metadata for a single guide via a URL.""" + # strip GET parameter preview=...; duplicate for base URL + url = re.sub(r"([&?])preview=.*", "", url) + url = url.removesuffix("/") + + matches = self.api_guides_df[ + (self.api_guides_df.url.str.lower() == url.lower()) + | (self.api_guides_df.friendly_url.str.lower() == url.lower()) + ] + if len(matches) == 1: + return matches.iloc[0] + + raise ValueError(f"Found {len(matches)} guide ids for URL: {url}, expecting one.") diff --git a/transmogrifier/sources/json/libguides.py b/transmogrifier/sources/json/libguides.py index accfb521..bfb32ad9 100644 --- a/transmogrifier/sources/json/libguides.py +++ b/transmogrifier/sources/json/libguides.py @@ -6,18 +6,12 @@ from urllib.parse import urlparse import pandas as pd -import requests from bs4 import BeautifulSoup, Tag from dateutil.parser import parse as date_parser import transmogrifier.models as timdex -from transmogrifier.config import ( - LIBGUIDES_API_TOKEN, - LIBGUIDES_CLIENT_ID, - LIBGUIDES_GUIDES_URL, - LIBGUIDES_TOKEN_URL, -) from transmogrifier.exceptions import SkippedRecordEvent +from transmogrifier.helpers import LibGuidesAPIClient from transmogrifier.sources.jsontransformer import JSONTransformer from transmogrifier.sources.transformer import JSON @@ -41,91 +35,6 @@ ] -class LibGuidesAPIClient: - """Client for LibGuides API communication and data retrieval. - - This class retrieves metadata about all LibGuides via an API, retrieving data that is - not found in the OAI-PMH XML records or the websites themselves. This valuable data - is used during transformation to identify records for exclusion, occasionally - provide friendlier URLs, and other data augmentation. - - This class is instantiated as a singleton object in this module. Once instantiated, - it is attached to the Libguides transformer instance. This allows class methods - on the transformer to access cached data from this singleton object, ultimately - resulting in only a single API call per multiple record transformation run. - - This class relies on two environment variables: - - LIBGUIDES_CLIENT_ID - - LIBGUIDES_API_TOKEN - """ - - def __init__(self) -> None: - if not LIBGUIDES_CLIENT_ID: - raise RuntimeError("Required env var 'LIBGUIDES_CLIENT_ID' is not set") - if not LIBGUIDES_API_TOKEN: - raise RuntimeError("Required env var 'LIBGUIDES_API_TOKEN' is not set") - - self.client_id = str(LIBGUIDES_CLIENT_ID) - self.client_secret = LIBGUIDES_API_TOKEN - self._api_guides_df: pd.DataFrame | None = None - - @property - def api_guides_df(self) -> pd.DataFrame: - if self._api_guides_df is None: - self._api_guides_df = self.fetch_guides(self.get_api_token()) - return self._api_guides_df - - def get_api_token(self) -> str: - data = { - "grant_type": "client_credentials", - "client_id": self.client_id, - "client_secret": self.client_secret, - } - response = requests.post(LIBGUIDES_TOKEN_URL, headers={}, data=data, timeout=60) - response.raise_for_status() - payload = response.json() - return payload.get("access_token") - - def fetch_guides(self, token: str) -> pd.DataFrame: - """Retrieve metadata for all LibGuides. - - Each guide may contain a 'pages' key with a list of sub-page dicts. These - sub-pages are expanded into their own rows in the returned DataFrame, inheriting - any columns from the parent guide that the sub-page does not have. - """ - logger.debug("Retrieving all guides from Libguides API.") - headers = {"Authorization": f"Bearer {token}"} - response = requests.get(LIBGUIDES_GUIDES_URL, headers=headers, timeout=60) - response.raise_for_status() - guides = response.json() - - all_rows: list[dict] = [] - for guide in guides: - pages = guide.get("pages", []) - all_rows.append(guide) - for page in pages: - # inherit parent columns, then overlay page-specific columns - page_row = {**guide, **page} - all_rows.append(page_row) - - return pd.DataFrame(all_rows) - - def get_guide_by_url(self, url: str) -> pd.Series: - """Get metadata for a single guide via a URL.""" - # strip GET parameter preview=...; duplicate for base URL - url = re.sub(r"([&?])preview=.*", "", url) - url = url.removesuffix("/") - - matches = self.api_guides_df[ - (self.api_guides_df.url.str.lower() == url.lower()) - | (self.api_guides_df.friendly_url.str.lower() == url.lower()) - ] - if len(matches) == 1: - return matches.iloc[0] - - raise ValueError(f"Found {len(matches)} guide ids for URL: {url}, expecting one.") - - # instantiate a LibGuidesAPIClient singleton libguides_api_client = LibGuidesAPIClient() From 1f5622df9bcefe6f84ca85932bb64fbb9c2c0a8a Mon Sep 17 00:00:00 2001 From: Graham Hukill Date: Mon, 10 Aug 2026 13:25:49 -0400 Subject: [PATCH 2/4] New ResearchDatabases transformer class Why these changes are being introduced: It turns out that the Springshare OAI endpoint that we harvest AZ items (research databases) has *never* produced deletes. Without those deleted records / tombstones in OAI, we were not successfully removing records from TIMDEX. Transmogrifier is currently the place in the TIMDEX ETL ecosystem where records are first written to the dataset, and most commonly, if they are records to index or delete. There may come a time when explicit pre-transform work is performed to establish source records in the TIMDEX dataset, then we transform them, but Transmogrifier has historically been responsible for that double duty and continues to be at this time. Ultimately, we need to identify AZ items that are no longer publicly available and establish `action=delete` records in the TIMDEX dataset to have them removed. The OAI harvester is less per-source opinionated than Transmogrifier, making it a poor choice for this. How this addresses that need: A new ResearchDatabases transformer class has been created. Formerly, the TIMDEX source `researchdatabases` used the Springshare transformer class as a naive OAIDC XML transformation. This new class changes nothing about the metadata transformation, but allows for a place to identify records that were formerly indexed in TIMDEX but are no longer publicly accessible. There is precedence here in the libguides source, which also performs some additional work via the Springshare API. With that scaffolding already present, it was a relatively simple addition to have researchdatabases do a bit of extra work beyond the OAI XML records provided. The ResearchDatabases transformer does something unique: while yielding the OAI XML records provided by the harvester like normal, it also queries the Springshare API and identifies records we've seen before but are no longer public. For these a *synthetic* OAI XML record is injected into the records yielded by this class for transformation. Those are handled by pre-existing logic and ultimately get written to the TIMDEX dataset with `action=delete`. Side effects of this change: * The researchdatabase source now requires the Springshare API credentials that formerly only the libguides source required. * If AZ items are deleted or hidden, they should get removed from TIMDEX now. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/TIMX-655 --- transmogrifier/config.py | 3 +- transmogrifier/helpers.py | 21 +++++ .../sources/xml/researchdatabases.py | 94 +++++++++++++++++++ transmogrifier/sources/xml/springshare.py | 10 +- transmogrifier/sources/xmltransformer.py | 5 +- 5 files changed, 123 insertions(+), 10 deletions(-) create mode 100644 transmogrifier/sources/xml/researchdatabases.py diff --git a/transmogrifier/config.py b/transmogrifier/config.py index 880cce94..208d9f9c 100644 --- a/transmogrifier/config.py +++ b/transmogrifier/config.py @@ -127,7 +127,7 @@ "researchdatabases": { "name": "Research Databases", "base-url": "https://libguides.mit.edu/", - "transform-class": "transmogrifier.sources.xml.springshare.SpringshareOaiDc", + "transform-class": "transmogrifier.sources.xml.researchdatabases.ResearchDatabases", # noqa: E501 }, "whoas": { "name": "Woods Hole Open Access Server", @@ -149,6 +149,7 @@ ) LIBGUIDES_API_TOKEN = os.getenv("LIBGUIDES_API_TOKEN") LIBGUIDES_CLIENT_ID = os.getenv("LIBGUIDES_CLIENT_ID") +LAST_AZ_IDENTIFIERS_PATH = os.getenv("LAST_AZ_IDENTIFIERS_PATH") def configure_logger( diff --git a/transmogrifier/helpers.py b/transmogrifier/helpers.py index 4ded49f1..f0930623 100644 --- a/transmogrifier/helpers.py +++ b/transmogrifier/helpers.py @@ -223,3 +223,24 @@ def get_guide_by_url(self, url: str) -> pd.Series: return matches.iloc[0] raise ValueError(f"Found {len(matches)} guide ids for URL: {url}, expecting one.") + + def fetch_az(self, token: str) -> pd.DataFrame: + """Retrieve AZ items from API.""" + headers = {"Authorization": f"Bearer {token}"} + response = requests.get( + "https://lgapi-us.libapps.com/1.2/az?expand=pages", + headers=headers, + timeout=60, + ) + response.raise_for_status() + return pd.DataFrame(response.json()) + + def get_current_az_identifiers(self) -> list[str]: + """Get list of identifiers for non-hidden / public AZ items. + + When filtering to enable_hidden = 0, the count matches the OAI-PMH full harvest + for AZ items. + """ + az_df = self.fetch_az(self.get_api_token()) + non_hidden_az_df = az_df[az_df.enable_hidden == "0"] + return list(non_hidden_az_df.id) diff --git a/transmogrifier/sources/xml/researchdatabases.py b/transmogrifier/sources/xml/researchdatabases.py new file mode 100644 index 00000000..da506245 --- /dev/null +++ b/transmogrifier/sources/xml/researchdatabases.py @@ -0,0 +1,94 @@ +import logging +from collections.abc import Iterator +from datetime import UTC, datetime + +import smart_open # type: ignore[import-untyped] +from bs4 import BeautifulSoup, Tag +from lxml import etree + +from transmogrifier.config import LAST_AZ_IDENTIFIERS_PATH +from transmogrifier.helpers import LibGuidesAPIClient +from transmogrifier.sources.xml.springshare import SpringshareOaiDc + +logger = logging.getLogger(__name__) + + +class ResearchDatabases(SpringshareOaiDc): + @classmethod + def parse_source_file(cls, source_file: str) -> Iterator[Tag]: + """Yield records from harvested OAI + API gathered records for possible delete.""" + yield from cls._yield_oai_xml_records_for_indexing(source_file) + yield from cls._yield_api_records_for_deleting() + + @classmethod + def _yield_oai_xml_records_for_indexing(cls, source_file: str) -> Iterator[Tag]: + """Yield OAI records from extracted XML file. + + This functionality is a direct port from XMLTransformer, but allows for a custom + self.parse_source_file in this transformer class. + """ + with smart_open.open(source_file, "rb") as file: + for _, element in etree.iterparse( + file, + tag="{*}record", + encoding="utf-8", + recover=True, + ): + record_string = etree.tostring(element, encoding="utf-8") + record = cls.parse_bs4_in_isolated_thread(record_string) + yield record + element.clear() + + @classmethod + def _yield_api_records_for_deleting(cls) -> Iterator[Tag]: + """Yield synthetic OAI records that will prompt deletes in TIMDEX. + + This method will yield stubbed OAI records *as-if* the Springshare OAI endpoint + produced deletes. This is achieved by querying the Springshare API, retrieving + a list of public/non-hidden AZ items, and comparing to a managed list of last + known public/non-hidden AZ items. Any items that are no longer public should + be removed from TIMDEX, which these synthetic OAI XML records archive. + """ + if LAST_AZ_IDENTIFIERS_PATH is None: + raise RuntimeError( + "Env var 'LAST_AZ_IDENTIFIERS_PATH' must be set " + "for the 'researchdatabases' transformation'" + ) + + client = LibGuidesAPIClient() + + # retrieve current AZ identifiers from API + az_current_identifiers = client.get_current_az_identifiers() + + # retrieve previous AZ identifiers from file + with smart_open.open(LAST_AZ_IDENTIFIERS_PATH) as f: + az_previous_identifiers = f.read().splitlines() + + # isolate identifiers from previous list not in current list + deleted_identifiers = set(az_previous_identifiers).difference( + az_current_identifiers + ) + logger.info( + f"{len(deleted_identifiers)} identifiers identified for deletion: " + f"{list(deleted_identifiers)}" + ) + + # yield fake OAI records that mark a record for delete + now_date = datetime.now(tz=UTC).strftime("%Y-%m-%dT%H:%M:%SZ") + for deleted_identifier in deleted_identifiers: + oai_delete_record = f""" + +
+ oai:libguides.com:az/{deleted_identifier} + {now_date} + az +
+ This is a synthetic delete record created by Transmogrifier. + + + """ + yield BeautifulSoup(oai_delete_record, "xml") + + # update AZ identifiers list with current AZ items + with smart_open.open(LAST_AZ_IDENTIFIERS_PATH, "w") as f: + f.writelines(f"{identifier}\n" for identifier in az_current_identifiers) diff --git a/transmogrifier/sources/xml/springshare.py b/transmogrifier/sources/xml/springshare.py index 3717daf9..26de3733 100644 --- a/transmogrifier/sources/xml/springshare.py +++ b/transmogrifier/sources/xml/springshare.py @@ -70,11 +70,11 @@ def get_links(self, source_record: Tag) -> list[timdex.Link] | None: url=str(identifier.string), ) ) - - logger.debug( - "Record ID %s has links that cannot be generated: missing dc:identifier", - source_record_id, - ) + else: + logger.debug( + "Record ID %s has links that cannot be generated: missing dc:identifier", + source_record_id, + ) return links or None def get_source_link( diff --git a/transmogrifier/sources/xmltransformer.py b/transmogrifier/sources/xmltransformer.py index 21e87dc9..e97e584b 100644 --- a/transmogrifier/sources/xmltransformer.py +++ b/transmogrifier/sources/xmltransformer.py @@ -1,7 +1,7 @@ from __future__ import annotations import threading -from typing import TYPE_CHECKING, final +from typing import TYPE_CHECKING import smart_open # type: ignore[import-untyped] from bs4 import BeautifulSoup, Tag # type: ignore[import-untyped] @@ -16,14 +16,11 @@ class XMLTransformer(Transformer): """XML transformer class.""" - @final @classmethod def parse_source_file(cls, source_file: str) -> Iterator[Tag]: """ Parse XML file and return source records as bs4 Tags via an iterator. - May not be overridden. - Args: source_file: A file containing source records to be transformed. """ From b1a382f94dc04aec0a913b2a77a1e2216274da02 Mon Sep 17 00:00:00 2001 From: Graham Hukill Date: Tue, 11 Aug 2026 10:35:01 -0400 Subject: [PATCH 3/4] Testing for new ResearchDatabases transformer --- ...-08-11-full-extracted-records-to-index.xml | 80 ++++++++++ tests/sources/xml/test_researchdatabases.py | 142 ++++++++++++++++++ tests/test_cli.py | 4 +- 3 files changed, 224 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/researchdatabases/researchdatabases-2026-08-11-full-extracted-records-to-index.xml create mode 100644 tests/sources/xml/test_researchdatabases.py diff --git a/tests/fixtures/researchdatabases/researchdatabases-2026-08-11-full-extracted-records-to-index.xml b/tests/fixtures/researchdatabases/researchdatabases-2026-08-11-full-extracted-records-to-index.xml new file mode 100644 index 00000000..262ebb45 --- /dev/null +++ b/tests/fixtures/researchdatabases/researchdatabases-2026-08-11-full-extracted-records-to-index.xml @@ -0,0 +1,80 @@ + + + 2026-08-11T09:30:27Z + + https://libguides.mit.edu/oai.php + + + +
+ oai:libguides.com:az/65257807 + 2025-12-01T14:59:03Z + az +
+ + + + + + + + + + The most comprehensive index to articles in Linguistics and Language Development and use.

]]> + 2022-01-28 22:15:37 + https://libguides.mit.edu/llba + + + + +
+ oai:libguides.com:az/65257808 + 2025-12-01T14:59:03Z + az +
+ + + + + + + + + Indexes major astronomy and astrophysics journals; includes abstracts for most entries and full text scans of tens of thousands of articles.

]]> + 2022-01-28 22:15:37 + https://libguides.mit.edu/ads + + + + +
+ oai:libguides.com:az/65257809 + 2025-12-01T14:59:03Z + az +
+ + + + + + + + Coverage: 1990 - present
A full-text collection of newspapers, journals and magazines published by ethnic and minority presses.

]]> + 2022-01-28 22:15:37 + https://libguides.mit.edu/ethnic + + + + + diff --git a/tests/sources/xml/test_researchdatabases.py b/tests/sources/xml/test_researchdatabases.py new file mode 100644 index 00000000..b76caf89 --- /dev/null +++ b/tests/sources/xml/test_researchdatabases.py @@ -0,0 +1,142 @@ +# ruff: noqa: PLR2004, SLF001 + +from unittest.mock import patch + +import pytest +from bs4 import Tag + +from transmogrifier.helpers import LibGuidesAPIClient +from transmogrifier.sources.xml.researchdatabases import ResearchDatabases + + +@pytest.fixture +def last_az_identifiers(tmp_path) -> str: + path = tmp_path / "last_az_identifiers.txt" + with open(path, "w") as f: + f.write("1234") + return str(path) + + +@pytest.fixture(autouse=True) +def _test_env_libguides(last_az_identifiers): + with ( + patch("transmogrifier.config.LIBGUIDES_CLIENT_ID", "123"), + patch("transmogrifier.config.LIBGUIDES_API_TOKEN", "aaabbbdddccc"), + patch( + "transmogrifier.sources.xml.researchdatabases.LAST_AZ_IDENTIFIERS_PATH", + last_az_identifiers, + ), + ): + yield + + +@pytest.fixture +def mocked_current_az_identifiers(): + """Mock current AZ identifiers from LibGuides API, none matching previous ones.""" + with patch.object( + LibGuidesAPIClient, + "get_current_az_identifiers", + return_value=["7777", "8888", "9999"], + ): + yield + + +@pytest.fixture +def researchdatabases_transformer(): + + return ResearchDatabases.load( + "researchdatabases", + ( + "tests/fixtures/researchdatabases/researchdatabases-" + "2026-08-11-full-extracted-records-to-index.xml" + ), + ) + + +def test_researchdatabases_yields_oai_records_pass(researchdatabases_transformer): + + oai_records = list( + researchdatabases_transformer._yield_oai_xml_records_for_indexing( + researchdatabases_transformer.source_file + ) + ) + + assert len(oai_records) == 3 + assert isinstance(oai_records[0], Tag) + + +def test_researchdatabases_yields_deleted_records_success( + researchdatabases_transformer, + mocked_current_az_identifiers, +): + deleted_records = list( + researchdatabases_transformer._yield_api_records_for_deleting() + ) + assert len(deleted_records) == 1 + assert isinstance(deleted_records[0], Tag) + + +def test_researchdatabases_synthetic_deleted_record( + researchdatabases_transformer, + mocked_current_az_identifiers, +): + """Assert that Transformer injects synthetic OAI delete records. + + Example record: + + + +
+ oai:libguides.com:az/1234 + 2026-08-11T14:13:38Z + az +
+ This is a synthetic delete record created by Transmogrifier. + + + """ + deleted_record = next(researchdatabases_transformer._yield_api_records_for_deleting()) + + header = deleted_record.find("record").find("header") + + assert header.get("status") == "deleted" + assert header.find("identifier").string == "oai:libguides.com:az/1234" + assert ( + deleted_record.find("note").string + == "This is a synthetic delete record created by Transmogrifier." + ) + + +def test_researchdatabases_last_az_identifiers_file_updated( + researchdatabases_transformer, mocked_current_az_identifiers, last_az_identifiers +): + """Assert the last AZ identifiers list is updated.""" + _ = list(researchdatabases_transformer._yield_api_records_for_deleting()) + with open(last_az_identifiers) as f: + assert f.read() == """7777\n8888\n9999\n""" + + +def test_researchdatabases_identifiers_text_file_missing_error( + tmp_path, + researchdatabases_transformer, + mocked_current_az_identifiers, +): + """Assert that missing AZ identifiers list will bubble up exception.""" + bad_path = tmp_path / "does-not-exist.txt" + with ( + patch( + "transmogrifier.sources.xml.researchdatabases.LAST_AZ_IDENTIFIERS_PATH", + bad_path, + ), + pytest.raises(FileNotFoundError, match=r"does-not-exist.txt"), + ): + list(researchdatabases_transformer._yield_api_records_for_deleting()) + + +def test_research_databases_custom_parse_source_file_yields_oai_and_synthetic_records( + researchdatabases_transformer, + mocked_current_az_identifiers, +): + records = list(researchdatabases_transformer.source_records) + + assert len(records) == 4 # 3 OAI + 1 synthetic delete diff --git a/tests/test_cli.py b/tests/test_cli.py index 8533fbcf..712eb89e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -215,9 +215,9 @@ def test_transform_no_memory_fault_for_threaded_bs4_parsing(monkeypatch, tmp_pat "run", "transform", "-s", - "researchdatabases", + "libguides", "-i", - "tests/fixtures/dataset/libguides-2025-01-09-full-extracted-records-to-index.xml", + "tests/fixtures/libguides/libguides-2026-02-20-full-extracted-records-to-index.jsonl", "-o", f"{tmp_path}/dataset", ], From 54b7b370cb390496df206000376dd433ea361054 Mon Sep 17 00:00:00 2001 From: Graham Hukill Date: Tue, 11 Aug 2026 10:57:18 -0400 Subject: [PATCH 4/4] researchdatabases deletes optional via LAST_AZ_IDENTIFIERS_PATH Why these changes are being introduced: There were some edge cases with the reliance on the env var LAST_AZ_IDENTIFIERS_PATH for identifying deletes in researchdatabases: - env var is not set - file does not exist - desire to NOT check for deletes How this addresses that need: Now, ResearchDatabases transformer only queries the Springshare API if the env var LAST_AZ_IDENTIFIERS_PATH is set. If the file does not exist it will create it. This keeps the source fully backwards compatible, but when the env var is set, it is utilized and deletes are detected. Side effects of this change: * researchdatabases source remains fully backwards compatbile until env var LAST_AZ_IDENTIFIERS_PATH is set. Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/TIMX-655 --- README.md | 19 +++++++++ tests/sources/xml/test_researchdatabases.py | 39 ++++++++++++++----- .../sources/xml/researchdatabases.py | 29 +++++++++++--- 3 files changed, 71 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index f1705811..eaeec20d 100644 --- a/README.md +++ b/README.md @@ -59,8 +59,27 @@ WORKSPACE=### Set to `dev` for local development, this will be set to `stage` an WARNING_ONLY_LOGGERS=### Comma-seperated list of logger names to set as WARNING only, e.g. 'botocore,charset_normalizer,smart_open' LIBGUIDES_API_TOKEN=### Libguides API token [required for libguides source] LIBGUIDES_CLIENT_ID=### Libguides account id [required for libguides source] +LAST_AZ_IDENTIFIERS_PATH=### S3 or local filepath of the last known public AZ item identifiers [enables synthetic delete records for researchdatabases source] ``` +#### `LAST_AZ_IDENTIFIERS_PATH` + +The Springshare OAI-PMH endpoint does not emit deletes for AZ (research database) items +that have been unpublished or hidden. When `LAST_AZ_IDENTIFIERS_PATH` is set, the +`researchdatabases` transformation queries the LibGuides API for the current set of +public AZ items, compares it to the identifiers stored at this path, and yields synthetic +delete records for any identifiers that have dropped out. The file is then rewritten with +the current identifiers for the next run. + +Notes: + +- If the env var is not set, no deletes are determined and the LibGuides API is not +queried. This makes the behavior opt-in and backwards compatible with runs that predate +it. +- If the env var is set but the file does not yet exist, that run cannot determine any +deletes, but it will create the file so that subsequent runs can. +- This also requires `LIBGUIDES_API_TOKEN` and `LIBGUIDES_CLIENT_ID` to be set. + ## CLI commands ### `transform` diff --git a/tests/sources/xml/test_researchdatabases.py b/tests/sources/xml/test_researchdatabases.py index b76caf89..2d18f251 100644 --- a/tests/sources/xml/test_researchdatabases.py +++ b/tests/sources/xml/test_researchdatabases.py @@ -116,21 +116,40 @@ def test_researchdatabases_last_az_identifiers_file_updated( assert f.read() == """7777\n8888\n9999\n""" -def test_researchdatabases_identifiers_text_file_missing_error( +def test_researchdatabases_env_var_not_set_skips_deletes( + caplog, + researchdatabases_transformer, + mocked_current_az_identifiers, +): + """Assert that deletes are opt-in via the 'LAST_AZ_IDENTIFIERS_PATH' env var.""" + caplog.set_level("WARNING") + with patch( + "transmogrifier.sources.xml.researchdatabases.LAST_AZ_IDENTIFIERS_PATH", + None, + ): + assert list(researchdatabases_transformer._yield_api_records_for_deleting()) == [] + + assert "Env var 'LAST_AZ_IDENTIFIERS_PATH' is not set" in caplog.text + + +def test_researchdatabases_identifiers_text_file_missing_creates_file( + caplog, tmp_path, researchdatabases_transformer, mocked_current_az_identifiers, ): - """Assert that missing AZ identifiers list will bubble up exception.""" - bad_path = tmp_path / "does-not-exist.txt" - with ( - patch( - "transmogrifier.sources.xml.researchdatabases.LAST_AZ_IDENTIFIERS_PATH", - bad_path, - ), - pytest.raises(FileNotFoundError, match=r"does-not-exist.txt"), + """Assert that a missing AZ identifiers list yields no deletes, but creates file.""" + caplog.set_level("WARNING") + cold_start_path = tmp_path / "does-not-exist.txt" + with patch( + "transmogrifier.sources.xml.researchdatabases.LAST_AZ_IDENTIFIERS_PATH", + str(cold_start_path), ): - list(researchdatabases_transformer._yield_api_records_for_deleting()) + assert list(researchdatabases_transformer._yield_api_records_for_deleting()) == [] + + assert "but file does not exist, this run will create it" in caplog.text + with open(cold_start_path) as f: + assert f.read() == "7777\n8888\n9999\n" def test_research_databases_custom_parse_source_file_yields_oai_and_synthetic_records( diff --git a/transmogrifier/sources/xml/researchdatabases.py b/transmogrifier/sources/xml/researchdatabases.py index da506245..17e4984e 100644 --- a/transmogrifier/sources/xml/researchdatabases.py +++ b/transmogrifier/sources/xml/researchdatabases.py @@ -16,7 +16,11 @@ class ResearchDatabases(SpringshareOaiDc): @classmethod def parse_source_file(cls, source_file: str) -> Iterator[Tag]: - """Yield records from harvested OAI + API gathered records for possible delete.""" + """Yield records from harvested OAI + API gathered records for possible delete. + + Synthetic delete records are only yielded if the env var + 'LAST_AZ_IDENTIFIERS_PATH' is set. + """ yield from cls._yield_oai_xml_records_for_indexing(source_file) yield from cls._yield_api_records_for_deleting() @@ -48,12 +52,18 @@ def _yield_api_records_for_deleting(cls) -> Iterator[Tag]: a list of public/non-hidden AZ items, and comparing to a managed list of last known public/non-hidden AZ items. Any items that are no longer public should be removed from TIMDEX, which these synthetic OAI XML records archive. + + Determining deletes is opt-in via the env var 'LAST_AZ_IDENTIFIERS_PATH'. If + unset, no synthetic delete records are yielded and the Springshare API is not + queried. If set but the file does not yet exist, the file will be created for + future use. """ if LAST_AZ_IDENTIFIERS_PATH is None: - raise RuntimeError( - "Env var 'LAST_AZ_IDENTIFIERS_PATH' must be set " - "for the 'researchdatabases' transformation'" + logger.warning( + "Env var 'LAST_AZ_IDENTIFIERS_PATH' is not set, synthetic delete " + "records will not be determined and yielded." ) + return client = LibGuidesAPIClient() @@ -61,8 +71,15 @@ def _yield_api_records_for_deleting(cls) -> Iterator[Tag]: az_current_identifiers = client.get_current_az_identifiers() # retrieve previous AZ identifiers from file - with smart_open.open(LAST_AZ_IDENTIFIERS_PATH) as f: - az_previous_identifiers = f.read().splitlines() + try: + with smart_open.open(LAST_AZ_IDENTIFIERS_PATH) as f: + az_previous_identifiers = f.read().splitlines() + except OSError: + logger.warning( + "Env var 'LAST_AZ_IDENTIFIERS_PATH' is set, but file does not exist, " + "this run will create it." + ) + az_previous_identifiers = [] # isolate identifiers from previous list not in current list deleted_identifiers = set(az_previous_identifiers).difference(