diff --git a/backend/app/api/v1/endpoint_modules/ogm_webhook.py b/backend/app/api/v1/endpoint_modules/ogm_webhook.py index 4ca36d6..95bfc2f 100644 --- a/backend/app/api/v1/endpoint_modules/ogm_webhook.py +++ b/backend/app/api/v1/endpoint_modules/ogm_webhook.py @@ -17,7 +17,14 @@ router = APIRouter() EVENT_DRIVEN_WATCH_MODES = {"webhook", "both", "nightly"} -REPOSITORY_DISCOVERY_ACTIONS = {"created", "publicized", "renamed", "transferred", "unarchived"} +REPOSITORY_DISCOVERY_ACTIONS = { + "created", + "publicized", + "renamed", + "transferred", + "archived", + "unarchived", +} def _verify_github_signature(body: bytes, signature_header: Optional[str], secret: str) -> bool: diff --git a/backend/app/services/ogm_harvest/harvest.py b/backend/app/services/ogm_harvest/harvest.py index a61f455..16564e0 100644 --- a/backend/app/services/ogm_harvest/harvest.py +++ b/backend/app/services/ogm_harvest/harvest.py @@ -27,10 +27,17 @@ async def harvest_repo( Returns a dict with run_id, repo, commit, stats, and missing_count. """ repo = OGMHarvestRepository() + await repo.ensure_repo(repo_name) + configured = await repo.get_repo(repo_name) + if configured and ( + not configured.get("ogm_enabled", True) + or (configured.get("ogm_tags") or {}).get("ogm_archived", False) + ): + return {"ogm_repo_name": repo_name, "status": "skipped", "reason": "disabled or archived"} + syncer = OGMRepoSync(base_dir=checkout_base_dir) importer = OGMResourceImporter(repo=repo) - await repo.upsert_repo(ogm_repo_name=repo_name) await repo.mark_repo_harvest_started(repo_name) run_id = await repo.create_harvest_run(ogm_repo_name=repo_name, ogm_trigger=trigger) run_started_at = datetime.utcnow() diff --git a/backend/app/services/ogm_harvest/repository.py b/backend/app/services/ogm_harvest/repository.py index 31ff877..169d12e 100644 --- a/backend/app/services/ogm_harvest/repository.py +++ b/backend/app/services/ogm_harvest/repository.py @@ -193,6 +193,13 @@ async def get_repo(self, ogm_repo_name: str) -> Optional[Dict[str, Any]]: ) return dict(row) if row else None + async def ensure_repo(self, ogm_repo_name: str) -> None: + """Create an unknown repo without overwriting existing discovery/admin settings.""" + stmt = pg_insert(ogm_repos).values(ogm_repo_name=ogm_repo_name) + await database.execute( + stmt.on_conflict_do_nothing(index_elements=[ogm_repos.c.ogm_repo_name]) + ) + async def upsert_repo( self, ogm_repo_name: str, diff --git a/backend/app/tasks/ogm_harvest.py b/backend/app/tasks/ogm_harvest.py index 4f5ed33..37b788a 100644 --- a/backend/app/tasks/ogm_harvest.py +++ b/backend/app/tasks/ogm_harvest.py @@ -71,7 +71,7 @@ async def _ogm_harvest_all_async(trigger: str) -> Dict[str, Any]: repos = await repo.list_repos() selected = [] for r in repos: - if not r.get("ogm_enabled", True): + if not r.get("ogm_enabled", True) or (r.get("ogm_tags") or {}).get("ogm_archived"): continue mode = (r.get("ogm_watch_mode") or "").lower() if trigger.lower() in SCHEDULED_OGM_TRIGGERS and mode not in SCHEDULED_OGM_WATCH_MODES: diff --git a/backend/scripts/ogm_harvester.py b/backend/scripts/ogm_harvester.py index dbfa10a..52e4691 100644 --- a/backend/scripts/ogm_harvester.py +++ b/backend/scripts/ogm_harvester.py @@ -82,7 +82,11 @@ def docs_to_index(self) -> Generator[Tuple[Dict, str], None, None]: self.logger.warning(f"OGM path does not exist: {self.ogm_path}") return + # Old checkouts remain on disk after GitHub repositories are archived. + active_repos = set(self.repositories()) for json_file in ogm_path.rglob("*.json"): + if json_file.relative_to(ogm_path).parts[0] not in active_repos: + continue # Skip layers.json files if json_file.name == "layers.json": self.logger.debug(f"skipping {json_file}; not a geoblacklight JSON document") diff --git a/backend/scripts/populate_ogm_repos.py b/backend/scripts/populate_ogm_repos.py index 135d42d..3a3ca3d 100644 --- a/backend/scripts/populate_ogm_repos.py +++ b/backend/scripts/populate_ogm_repos.py @@ -217,7 +217,7 @@ def main(): parser.add_argument( "--include-archived", action="store_true", - help="Include archived repos (default: skip archived)", + help="Deprecated compatibility flag; archived repos are always recorded as disabled", ) args = parser.parse_args() @@ -229,8 +229,7 @@ def main(): sync_db_url = _sync_database_url(database_url) repos = list_org_repos(args.org, token, per_page=args.per_page) - if not args.include_archived: - repos = [r for r in repos if not r.get("archived", False)] + # Reconcile archived repos too, so previously enabled entries become disabled. if args.limit: repos = repos[: args.limit] diff --git a/backend/scripts/trigger_ogm_nightly_sync.py b/backend/scripts/trigger_ogm_nightly_sync.py index 9ba2a35..ac45a38 100755 --- a/backend/scripts/trigger_ogm_nightly_sync.py +++ b/backend/scripts/trigger_ogm_nightly_sync.py @@ -52,7 +52,7 @@ def main() -> None: parser.add_argument( "--include-archived", action="store_true", - help="Include archived repos when refreshing ogm_repos", + help="Deprecated compatibility flag; archived repos are always recorded as disabled", ) parser.add_argument( "--skip-harvest", @@ -69,8 +69,7 @@ def main() -> None: sync_db_url = _sync_database_url(database_url) repos = list_org_repos(args.org, token, per_page=args.per_page) - if not args.include_archived: - repos = [repo for repo in repos if not repo.get("archived", False)] + # Reconcile archived repos too, so previously enabled entries become disabled. if args.limit is not None: repos = repos[: args.limit] diff --git a/backend/tests/api/v1/test_ogm_webhook.py b/backend/tests/api/v1/test_ogm_webhook.py index f4cc792..05d4f19 100644 --- a/backend/tests/api/v1/test_ogm_webhook.py +++ b/backend/tests/api/v1/test_ogm_webhook.py @@ -218,3 +218,19 @@ def test_push_to_unknown_aardvark_repo_discovers_and_queues(monkeypatch): assert response.json()["queued"] == "edu.new" assert queued == [{"repo_name": "edu.new", "trigger": "push"}] assert FakeRepo.upserts[0]["ogm_repo_name"] == "edu.new" + + +def test_repository_archived_disables_existing_repo_without_queuing(monkeypatch): + queued = [] + FakeRepo.rows = {"edu.umn": {"ogm_enabled": True, "ogm_watch_mode": "both"}} + monkeypatch.setenv("OGM_WEBHOOK_SECRET", SECRET) + monkeypatch.setattr(ogm_webhook, "OGMHarvestRepository", FakeRepo) + monkeypatch.setattr(ogm_webhook, "repo_has_metadata_aardvark", lambda *args: True) + monkeypatch.setattr(ogm_webhook.ogm_harvest_repo, "delay", lambda **kw: queued.append(kw)) + response = _post_webhook( + {"action": "archived", "repository": _repo_payload("edu.umn", archived=True)}, "repository" + ) + assert response.status_code == 200 + assert FakeRepo.rows["edu.umn"]["ogm_enabled"] is False + assert FakeRepo.rows["edu.umn"]["ogm_tags"]["ogm_archived"] is True + assert queued == [] diff --git a/backend/tests/scripts/test_ogm_harvester.py b/backend/tests/scripts/test_ogm_harvester.py new file mode 100644 index 0000000..b513117 --- /dev/null +++ b/backend/tests/scripts/test_ogm_harvester.py @@ -0,0 +1,23 @@ +import json + +from scripts.ogm_harvester import OGMHarvester + + +def test_docs_to_index_excludes_archived_checkouts(tmp_path, monkeypatch): + for repo in ("edu.umn", "geobtaa"): + directory = tmp_path / repo / "metadata-aardvark" + directory.mkdir(parents=True) + (directory / "record.json").write_text( + json.dumps( + { + "id": "same-migrated-record", + "gbl_mdVersion_s": "Aardvark", + "source": repo, + } + ) + ) + harvester = OGMHarvester(ogm_path=str(tmp_path)) + monkeypatch.setattr(harvester, "repositories", lambda: ["geobtaa"]) + records = list(harvester.docs_to_index()) + assert len(records) == 1 + assert records[0][0]["source"] == "geobtaa" diff --git a/backend/tests/scripts/test_populate_ogm_repos.py b/backend/tests/scripts/test_populate_ogm_repos.py index 4187777..dce38e8 100644 --- a/backend/tests/scripts/test_populate_ogm_repos.py +++ b/backend/tests/scripts/test_populate_ogm_repos.py @@ -1,5 +1,7 @@ import json +import pytest + from scripts import populate_ogm_repos @@ -62,3 +64,39 @@ def fake_get(url, headers, params, timeout): ) assert calls[0]["headers"]["Authorization"] == "Bearer bad-token" assert "Authorization" not in calls[1]["headers"] + + +@pytest.mark.parametrize("script_name", ["populate_ogm_repos", "trigger_ogm_nightly_sync"]) +@pytest.mark.parametrize("extra_args", [[], ["--include-archived"]]) +def test_refresh_disables_archived_repos_and_enables_geobtaa(monkeypatch, script_name, extra_args): + from importlib import import_module + + script = import_module(f"scripts.{script_name}") + monkeypatch.setenv("DATABASE_URL", "postgresql://unused/test") + args = [script_name, *extra_args] + if script_name == "trigger_ogm_nightly_sync": + args.append("--skip-harvest") + monkeypatch.setattr("sys.argv", args) + monkeypatch.setattr( + script, + "list_org_repos", + lambda *a, **kw: [ + {"name": "edu.umn", "archived": True}, + {"name": "geobtaa", "archived": False}, + ], + ) + monkeypatch.setattr(script, "repo_has_metadata_aardvark", lambda *a: True) + rows = [] + + def capture(url, values, dry_run=False): + rows.extend(values) + return len(values), 0 + + monkeypatch.setattr(script, "upsert_rows", capture) + script.main() + assert [(r["ogm_repo_name"], r["ogm_enabled"]) for r in rows] == [ + ("edu.umn", False), + ("geobtaa", True), + ] + assert rows[0]["ogm_watch_mode"] == "manual" + assert rows[0]["ogm_tags"]["ogm_archived"] is True diff --git a/backend/tests/services/test_ogm_search_index.py b/backend/tests/services/test_ogm_search_index.py index 10504a5..d705196 100644 --- a/backend/tests/services/test_ogm_search_index.py +++ b/backend/tests/services/test_ogm_search_index.py @@ -37,7 +37,7 @@ async def test_harvest_requires_search_sync_before_reporting_success(monkeypatch repo = MagicMock() for name in ( - "upsert_repo", + "ensure_repo", "mark_repo_harvest_started", "cancel_other_running_runs", "update_harvest_run", @@ -45,6 +45,7 @@ async def test_harvest_requires_search_sync_before_reporting_success(monkeypatch "mark_repo_harvest_completed", ): setattr(repo, name, AsyncMock()) + repo.get_repo = AsyncMock(return_value={"ogm_enabled": True}) repo.create_harvest_run = AsyncMock(return_value=1) importer = MagicMock(changed_thumbnail_resource_ids=set()) importer.upsert_stream = AsyncMock(return_value={"imported": 1, "errors": 0}) @@ -72,3 +73,40 @@ async def test_harvest_requires_search_sync_before_reporting_success(monkeypatch assert result["stats"]["search_index"] == {"indexed": 1, "errors": 0} assert repo.finalize_harvest_run.await_args.kwargs["ogm_status"] == "success" search.assert_awaited_once_with("repo") + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "settings", + [ + {"ogm_enabled": False}, + {"ogm_enabled": True, "ogm_tags": {"ogm_archived": True}}, + ], +) +async def test_harvest_skips_disabled_or_archived_repo(monkeypatch, settings): + from app.services.ogm_harvest import harvest + + repo = MagicMock() + repo.ensure_repo = AsyncMock() + repo.get_repo = AsyncMock(return_value=settings) + syncer = MagicMock() + monkeypatch.setattr(harvest, "OGMHarvestRepository", lambda: repo) + monkeypatch.setattr(harvest, "OGMRepoSync", syncer) + result = await harvest.harvest_repo("edu.umn", trigger="push") + assert result["status"] == "skipped" + syncer.assert_not_called() + repo.upsert_repo.assert_not_called() + repo.mark_repo_harvest_started.assert_not_called() + + +@pytest.mark.asyncio +async def test_ensure_repo_does_not_overwrite_catalog_settings(monkeypatch): + from sqlalchemy.dialects import postgresql + + from app.services.ogm_harvest import repository + + execute = AsyncMock() + monkeypatch.setattr(repository.database, "execute", execute) + await repository.OGMHarvestRepository().ensure_repo("geobtaa") + statement = str(execute.await_args.args[0].compile(dialect=postgresql.dialect())) + assert "ON CONFLICT (ogm_repo_name) DO NOTHING" in statement diff --git a/backend/tests/tasks/test_ogm_harvest_tasks.py b/backend/tests/tasks/test_ogm_harvest_tasks.py index 6f9ac54..cf3fe65 100644 --- a/backend/tests/tasks/test_ogm_harvest_tasks.py +++ b/backend/tests/tasks/test_ogm_harvest_tasks.py @@ -19,6 +19,12 @@ async def test_ogm_harvest_all_nightly_trigger_selects_scheduled_modes(monkeypat {"ogm_repo_name": "scheduled-repo", "ogm_enabled": True, "ogm_watch_mode": "scheduled"}, {"ogm_repo_name": "both-repo", "ogm_enabled": True, "ogm_watch_mode": "both"}, {"ogm_repo_name": "manual-repo", "ogm_enabled": True, "ogm_watch_mode": "manual"}, + { + "ogm_repo_name": "archived-repo", + "ogm_enabled": True, + "ogm_watch_mode": "both", + "ogm_tags": {"ogm_archived": True}, + }, {"ogm_repo_name": "disabled-repo", "ogm_enabled": False, "ogm_watch_mode": "nightly"}, ] ) diff --git a/docs/harvester.md b/docs/harvester.md index 92f0f86..980d7e0 100644 --- a/docs/harvester.md +++ b/docs/harvester.md @@ -80,3 +80,13 @@ representations that may contain an older asset URL. This refresh can be disable with `OGM_THUMBNAIL_REFRESH_ENABLED=false`; a refresh failure is recorded in the harvest statistics and does not turn an otherwise successful metadata harvest into a failure. + +Repository discovery always reconciles archived GitHub repositories into the +catalog as disabled with manual watch mode, including repositories that were +previously enabled. The legacy `--include-archived` flag is accepted for +compatibility but is no longer needed. Archived BTAA repositories are replaced +by the active `geobtaa` repository. Harvest jobs check the saved enabled/archive +status before pulling and preserve catalog settings when starting a run. +The standalone harvester also excludes archived checkouts when streaming records. +Existing records are retained until refreshed from their current source; disabling +a repository does not delete its previously harvested records.