From 7fe443f2b657d1a67d71c1c8f337a6902fcff798 Mon Sep 17 00:00:00 2001 From: Joseph Rhoads Date: Mon, 28 Sep 2026 21:49:43 +0200 Subject: [PATCH 01/11] Throttle anonymous client registration at 5/hour (#576) Add a ClientRegistrationThrottle (AnonRateThrottle subclass with a class-level rate) on ClientRegistrationView so settings.py stays unchanged. Cover the wiring and 429-after-five-requests path with a unit test that never sends email. Co-authored-by: Cursor Agent Co-authored-by: Joseph Rhoads --- rorapi/common/views.py | 10 +++++++ .../tests_unit/tests_register_throttle.py | 29 +++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 rorapi/tests/tests_unit/tests_register_throttle.py diff --git a/rorapi/common/views.py b/rorapi/common/views.py index 439a348a..68aa99d0 100644 --- a/rorapi/common/views.py +++ b/rorapi/common/views.py @@ -5,6 +5,7 @@ from django.views import View from django.shortcuts import redirect from rest_framework.permissions import BasePermission +from rest_framework.throttling import AnonRateThrottle from rest_framework.views import APIView from rest_framework.parsers import FormParser, MultiPartParser from rorapi.settings import DATA @@ -43,7 +44,16 @@ from rorapi.v2.models import Client from rorapi.v2.serializers import ClientSerializer + +class ClientRegistrationThrottle(AnonRateThrottle): + """Tight anonymous limit for client-ID registration (settings.py left unchanged).""" + + rate = "5/hour" + + class ClientRegistrationView(APIView): + throttle_classes = [ClientRegistrationThrottle] + def post(self, request, version='v2'): serializer = ClientSerializer(data=request.data) if serializer.is_valid(): diff --git a/rorapi/tests/tests_unit/tests_register_throttle.py b/rorapi/tests/tests_unit/tests_register_throttle.py new file mode 100644 index 00000000..ca3d5c4f --- /dev/null +++ b/rorapi/tests/tests_unit/tests_register_throttle.py @@ -0,0 +1,29 @@ +from django.core.cache import cache +from django.test import SimpleTestCase +from rest_framework.exceptions import Throttled +from rest_framework.test import APIRequestFactory + +from rorapi.common.views import ClientRegistrationThrottle, ClientRegistrationView + +factory = APIRequestFactory() + + +class ClientRegistrationThrottleTests(SimpleTestCase): + def setUp(self): + # DRF SimpleRateThrottle binds django.core.cache.cache at import time. + ClientRegistrationThrottle.cache = cache + cache.clear() + self.view = ClientRegistrationView() + + def test_view_uses_class_rate_throttle(self): + self.assertEqual(ClientRegistrationThrottle.rate, "5/hour") + self.assertEqual( + ClientRegistrationView.throttle_classes, [ClientRegistrationThrottle] + ) + + def test_allows_five_anonymous_requests_then_throttles(self): + request = self.view.initialize_request(factory.post("/v2/register")) + for _ in range(5): + self.view.check_throttles(request) + with self.assertRaises(Throttled): + self.view.check_throttles(request) From 8a9b84345c9a2130754c6b7680b15ffc39643931 Mon Sep 17 00:00:00 2001 From: Joseph Rhoads Date: Mon, 28 Sep 2026 22:09:27 +0200 Subject: [PATCH 02/11] Remove unused GRID, LaunchDarkly, and legacy command code (#575) Co-authored-by: Cursor Agent --- ARCHITECTURE.md | 8 +- README.md | 44 +--- rorapi/common/features.py | 6 - rorapi/common/queries.py | 15 +- .../management/commands/legacyconvertgrid.py | 209 ------------------ .../management/commands/legacydownloadgrid.py | 36 --- rorapi/management/commands/legacyindexgrid.py | 87 -------- rorapi/management/commands/legacyseeschema.py | 20 -- rorapi/management/commands/legacyupgrade.py | 18 -- rorapi/settings.py | 94 +------- rorapi/v2/models.py | 14 -- rorapi/v2/tests.py | 12 - 12 files changed, 9 insertions(+), 554 deletions(-) delete mode 100644 rorapi/common/features.py delete mode 100644 rorapi/management/commands/legacyconvertgrid.py delete mode 100644 rorapi/management/commands/legacydownloadgrid.py delete mode 100644 rorapi/management/commands/legacyindexgrid.py delete mode 100644 rorapi/management/commands/legacyseeschema.py delete mode 100644 rorapi/management/commands/legacyupgrade.py delete mode 100644 rorapi/v2/tests.py diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 34558c4f..5e00ec6e 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -67,7 +67,6 @@ The ROR API provides: | App server | Phusion Passenger + Nginx (`vendor/docker/webapp.conf`) | | Container | Docker (`Dockerfile` based on `phusion/passenger-python312:3.2.0`) | | Observability | Sentry (`sentry-sdk` 1.45.1), django-prometheus 2.4.1 | -| Feature flags | LaunchDarkly (`rorapi/common/features.py`; `launchdarkly-server-sdk` 7.6.1) | | Email | django-ses 4.8.0 (client ID registration emails) | | External packages | `update_address` (Geonames enrichment), `jsonschema` 3.2.0, `rapidfuzz` 3.6.1, `boto3` (unpinned), `pandas` 2.2.3 | @@ -107,7 +106,7 @@ ror-api/ │ │ ├── record_template.json │ │ ├── ror_schema_v2_1.json # Vendored JSON schema for write validation │ │ └── index_template_es7.json # ES index template + mappings -│ ├── management/commands/ # CLI indexing and legacy GRID tools +│ ├── management/commands/ # CLI indexing and data setup │ ├── migrations/ # Django migrations (Client model) │ └── tests/ # Unit, integration, functional, affiliation suites └── vendor/docker/ # Nginx, env, Terraform var templates for deploy @@ -333,7 +332,6 @@ Loaded from environment and optional root `.env` file (`python-dotenv`). | `ROUTE_USER`, `TOKEN` | Admin API authentication | | `ROR_BASE_URL` | Base URL configuration | | `SENTRY_DSN` | Error reporting | -| `LAUNCH_DARKLY_KEY` | Feature flags | | `SINGLE_SEARCH_DEFAULT` | Default affiliation matcher (`True`/`False`) | | `ENABLE_BEHAVIORAL_LIMITING` | Rate limiting toggle (edge behavior) | | `SECRET_KEY` | Django secret (falls back to a hardcoded default if unset; `DEBUG` is always `False`) | @@ -400,9 +398,7 @@ Deploy mechanism: GitHub Action updates `_ror-api-*.auto.tfvars` in the `new-dep ## Legacy Code -Commands prefixed with `legacy*` (GRID conversion, old upgrade paths) are **non-functional** — referenced data was moved to ror-data. GRID-based generation ended March 2022. Do not extend or rely on these unless explicitly reviving historical tooling. - -`settings.py` still contains commented GRID/ROR_DUMP version history for reference. `GRID_REMOVED_IDS` is an empty list retained for a check in `retrieve_organization`. +GRID-based generation ended March 2022. The `legacy*` management commands, the LaunchDarkly call site, and the empty `GRID_REMOVED_IDS` check have been removed. `settings.py` still contains a short comment noting that ROR is no longer based on GRID. Historical GRID/ROR dump files live in [ror-data](https://github.com/ror-community/ror-data). --- diff --git a/README.md b/README.md index b517d280..a29e935a 100644 --- a/README.md +++ b/README.md @@ -107,49 +107,9 @@ The API uses the v2 schema only. Use `-s 2` when indexing a data dump. A v2 form python manage.py setup v1.32-2023-09-14-ror-data -s 2 -t -## LEGACY: Converting GRID data to ROR (process used prior to Mar 2022) +## GRID history (prior to Mar 2022) -Steps used prior to Mar 2022: -- Convert latest GRID dataset to ROR (including assigning ROR IDs) -- Generate ROR data dump -- Index ROR data dump into Elastic Search - -As of Mar 2022 ROR is no longer based on GRID. Record additions/updates and data deployment is now managed in https://github.com/ror-community/ror-records using the ```indexror``` command described above. - -Steps below no longer work, as data files have been moved to [ror-data](https://github.com/ror-community/ror-data). This information is being maintained for historical purposes. - -Management commands used in this process no longer work and are pre-pended with "legacy". - - -To import GRID data, you need a system where `setup` has been run successfully. Then first update the `GRID` variable in `settings.py`, e.g. - -``` -GRID = { - 'VERSION': '2020-03-15', - 'URL': 'https://digitalscience.figshare.com/ndownloader/files/22091379' -} -``` - -And, also in `settings.py`, set the `ROR_DUMP` variable, e.g. - -``` -ROR_DUMP = {'VERSION': '2020-04-02'} -``` - -Then run this command: `./manage.py upgrade`. - -You should see this in the console: - -``` -Downloading GRID version 2020-03-15 -Converting GRID dataset to ROR schema -ROR dataset created -ROR dataset ZIP archive created -``` - -This will create a new `data/ror-2020-03-15` folder, containing a `ror.json` and `ror.zip`. To finish the process, add the new folder to git and push to the GitHub repo. - -To install the updated ROR data, run `./manage.py setup`. +Before March 2022, ROR records were derived from GRID: convert the GRID dataset, generate a dump, and index it. That pipeline and its management commands have been removed. Record additions/updates and data deployment are now managed in https://github.com/ror-community/ror-records using the `indexror` command described above. Historical GRID/ROR dump files live in [ror-data](https://github.com/ror-community/ror-data). ## Create new record file (v2 only) diff --git a/rorapi/common/features.py b/rorapi/common/features.py deleted file mode 100644 index 680402f5..00000000 --- a/rorapi/common/features.py +++ /dev/null @@ -1,6 +0,0 @@ -import ldclient -from ldclient.config import Config -from rorapi.settings import LAUNCH_DARKLY_KEY - -ldclient.set_config(Config(LAUNCH_DARKLY_KEY)) -launch_darkly_client = ldclient.get() \ No newline at end of file diff --git a/rorapi/common/queries.py b/rorapi/common/queries.py index bd4d02aa..e3c4a4a5 100644 --- a/rorapi/common/queries.py +++ b/rorapi/common/queries.py @@ -11,7 +11,7 @@ Organization as OrganizationV2, ListResult as ListResultV2 ) -from rorapi.settings import GRID_REMOVED_IDS, ROR_API, ES_VARS +from rorapi.settings import ROR_API, ES_VARS from rorapi.common.es_utils import ESQueryBuilder from urllib.parse import unquote @@ -280,19 +280,6 @@ def search_organizations(params): def retrieve_organization(ror_id): """Retrieves the organization of the given ROR ID""" - if any(ror_id in ror_id_url for ror_id_url in GRID_REMOVED_IDS): - return ( - Errors( - [ - "ROR ID '{}' was removed by GRID during the time period (Jan 2019-Mar 2022) " - "that ROR was synced with GRID. We are currently working with the ROR Curation Advisory Board " - "to restore these records and expect to complete this work in 2022".format( - ror_id - ) - ] - ), - None, - ) search = build_retrieve_query(ror_id) results = search.execute() total = results.hits.total.value diff --git a/rorapi/management/commands/legacyconvertgrid.py b/rorapi/management/commands/legacyconvertgrid.py deleted file mode 100644 index cbddbff4..00000000 --- a/rorapi/management/commands/legacyconvertgrid.py +++ /dev/null @@ -1,209 +0,0 @@ -import base32_crockford -import json -import os.path -import random -import zipfile -import re -from rorapi.settings import ES, ES_VARS, ROR_API, GRID, ROR_DUMP - -from django.core.management.base import BaseCommand - -# Previously used to convert latest GRID dataset configured in settings.py -# to ROR and assign ROR IDs to each GRID org -# As of Mar 2022 ROR is no longer based on GRID -# New records are now created in https://github.com/ror-community/ror-records and pushed to S3 -# Individual record files in S3 are indexed with indexror.py -# Entire dataset zip files in https://github.com/ror-community/ror-data -# can be indexed with setup.py, which uses indexrordump.py - -def generate_ror_id(): - """Generates random ROR ID. - - The checksum calculation is copied from - https://github.com/datacite/base32-url/blob/master/lib/base32/url.rb - to maintain the compatibility with previously generated ROR IDs. - """ - - n = random.randint(0, 200000000) - n_encoded = base32_crockford.encode(n).lower().zfill(6) - checksum = str(98 - ((n * 100) % 97)).zfill(2) - return '{}0{}{}'.format(ROR_API['ID_PREFIX'], n_encoded, checksum) - - -def get_ror_id(grid_id, es): - """Maps GRID ID to ROR ID. - - If given GRID ID was indexed previously, corresponding ROR ID is obtained - from the index. Otherwise, new ROR ID is generated. - """ - - s = ES.search(ES_VARS['INDEX'], - body={'query': { - 'term': { - 'external_ids.GRID.all': grid_id - } - }}) - if s['hits']['total'] == 1: - return s['hits']['hits'][0]['_id'] - return generate_ror_id() - - -def geonames_city(geonames_city): - geonames = ["geonames_admin1", "geonames_admin2"] - geonames_attributes = ["id", "name", "ascii_name", "code"] - nuts = ["nuts_level1", "nuts_level2", "nuts_level3"] - nuts_attributes = ["code", "name"] - geonames_city_hsh = {} - for k, v in geonames_city.items(): - if (k in geonames): - if isinstance(v, dict): - geonames_city_hsh[k] = { - i: v.get(i, None) - for i in geonames_attributes - } - elif v is None: - geonames_city_hsh[k] = {i: None for i in geonames_attributes} - elif (k in nuts): - if isinstance(v, dict): - geonames_city_hsh[k] = { - i: v.get(i, None) - for i in nuts_attributes - } - elif v is None: - geonames_city_hsh[k] = {i: None for i in nuts_attributes} - else: - geonames_city_hsh[k] = v - return geonames_city_hsh - - -def addresses(location): - line = "" - address = ["line_1", "line_2", "line_3"] - combine_lines = address + ["country", "country_code"] - geonames_admin = ["id", "code", "name", "ascii_name"] - nuts = ["code", "name"] - new_addresses = [] - hsh = {} - hsh["line"] = None - for h in location: - for k, v in h.items(): - if not (k in combine_lines) and (k != "geonames_city"): - v = v if v != "" else None - hsh[k] = v - elif k == "geonames_city": - if isinstance(v, dict): - hsh[k] = geonames_city(v) - elif v is None: - hsh[k] = {} - elif (k in combine_lines): - n = [] - for i in address: - if not (h[i] is None): - n.append(h[i]) - line = " ".join(n) - line = re.sub(' +', ' ', line) - if (len(line) == 1 and line == " "): - line = line.strip() - line = line if len(line) > 0 else None - hsh["line"] = line - new_addresses.append(hsh) - return new_addresses - - -def convert_organization(grid_org, es): - """Converts the organization metadata from GRID schema to ROR schema.""" - return { - 'id': - get_ror_id(grid_org['id'], ES), - 'name': - grid_org['name'], - 'types': - grid_org['types'], - 'links': - grid_org['links'], - 'aliases': - grid_org['aliases'], - 'acronyms': - grid_org['acronyms'], - 'status': - grid_org['status'], - 'wikipedia_url': - grid_org['wikipedia_url'], - 'labels': - grid_org['labels'], - 'email_address': - grid_org['email_address'], - 'ip_addresses': - grid_org['ip_addresses'], - 'established': - grid_org['established'], - 'country': { - 'country_code': grid_org['addresses'][0]['country_code'], - 'country_name': grid_org['addresses'][0]['country'] - }, - 'relationships': - grid_org["relationships"], - 'addresses': - addresses(grid_org["addresses"]), - 'external_ids': - getExternalIds( - dict(grid_org.get('external_ids', {}), - GRID={ - 'preferred': grid_org['id'], - 'all': grid_org['id'] - })) - } - - -def getExternalIds(external_ids): - if 'ROR' in external_ids: del external_ids['ROR'] - return external_ids - - -def get_ids(data): - ids = {} - for d in data: - ids[d['external_ids']['GRID']['all']] = d['id'] - return ids - - -def get_grid(record, ids): - if record['relationships']: - for r in record['relationships']: - r['id'] = ids[r['id']] - - return record - - -class Command(BaseCommand): - help = 'Converts GRID dataset to ROR schema' - - def handle(self, *args, **options): - os.makedirs(ROR_DUMP['DIR'], exist_ok=True) - # make sure we are not overwriting an existing ROR JSON file - # with new ROR identifiers - if zipfile.is_zipfile(ROR_DUMP['ROR_ZIP_PATH']): - self.stdout.write('ROR dataset already exists') - return - - if not os.path.isfile(ROR_DUMP['ROR_JSON_PATH']): - with open(GRID['GRID_JSON_PATH'], 'r') as it: - grid_data = json.load(it) - - self.stdout.write('Converting GRID dataset to ROR schema') - intermediate_ror_data = [ - convert_organization(org, ES) - for org in grid_data['institutes'] if org['status'] == 'active' - ] - ids = get_ids(intermediate_ror_data) - ror_data = [get_grid(rec, ids) for rec in intermediate_ror_data] - with open(ROR_DUMP['ROR_JSON_PATH'], 'w') as outfile: - json.dump(ror_data, outfile, indent=4) - self.stdout.write('ROR dataset created') - - # generate zip archive - with zipfile.ZipFile(ROR_DUMP['ROR_ZIP_PATH'], 'w') as zipArchive: - zipArchive.write(ROR_DUMP['ROR_JSON_PATH'], - arcname='ror.json', - compress_type=zipfile.ZIP_DEFLATED) - self.stdout.write('ROR dataset ZIP archive created') diff --git a/rorapi/management/commands/legacydownloadgrid.py b/rorapi/management/commands/legacydownloadgrid.py deleted file mode 100644 index 0c75bdea..00000000 --- a/rorapi/management/commands/legacydownloadgrid.py +++ /dev/null @@ -1,36 +0,0 @@ -import os -import requests -import zipfile - -from django.core.management.base import BaseCommand -from rorapi.settings import GRID - -# Previously used to download latest GRID dataset configured in settings.py -# which was used to generate a new ROR datasets -# As of Mar 2022 ROR is no longer based on GRID -# New records are now created in https://github.com/ror-community/ror-records and pushed to S3 -# Individual record files in S3 are indexed with indexror.py -# Entire dataset zip files in https://github.com/ror-community/ror-data -# can be indexed with setup.py, which uses indexrordump.py - -class Command(BaseCommand): - help = 'Downloads GRID dataset' - - def handle(self, *args, **options): - os.makedirs(GRID['DIR'], exist_ok=True) - - # make sure we are not overwriting an existing ROR JSON file - # with new ROR identifiers - if zipfile.is_zipfile(GRID['GRID_ZIP_PATH']): - self.stdout.write('Already downloaded GRID version {}'.format( - GRID['VERSION'])) - return - - self.stdout.write('Downloading GRID version {}'.format( - GRID['VERSION'])) - r = requests.get(GRID['URL']) - with open(GRID['GRID_ZIP_PATH'], 'wb') as f: - f.write(r.content) - - with zipfile.ZipFile(GRID['GRID_ZIP_PATH'], 'r') as zip_ref: - zip_ref.extractall(GRID['DIR']) diff --git a/rorapi/management/commands/legacyindexgrid.py b/rorapi/management/commands/legacyindexgrid.py deleted file mode 100644 index 0bb49306..00000000 --- a/rorapi/management/commands/legacyindexgrid.py +++ /dev/null @@ -1,87 +0,0 @@ -import json -import re -import zipfile -from rorapi.settings import ES, ES_VARS, LEGACY_ROR_DUMP - -from django.core.management.base import BaseCommand -from elasticsearch import TransportError - - -def get_nested_names(org): - yield org['name'] - for label in org['labels']: - yield label['label'] - for alias in org['aliases']: - yield alias - for acronym in org['acronyms']: - yield acronym - - -def get_nested_ids(org): - yield org['id'] - yield re.sub('https://', '', org['id']) - yield re.sub('https://ror.org/', '', org['id']) - for ext_name, ext_id in org['external_ids'].items(): - if ext_name == 'GRID': - yield ext_id['all'] - else: - for eid in ext_id['all']: - yield eid - - -class Command(BaseCommand): - help = 'Indexes ROR dataset' - - def handle(self, *args, **options): - with zipfile.ZipFile(LEGACY_ROR_DUMP['ROR_ZIP_PATH'], 'r') as zip_ref: - zip_ref.extractall(LEGACY_ROR_DUMP['DIR']) - - with open(LEGACY_ROR_DUMP['ROR_JSON_PATH'], 'r') as it: - dataset = json.load(it) - - self.stdout.write('Indexing ROR dataset') - - index = ES_VARS['INDEX'] - backup_index = '{}-tmp'.format(index) - ES.reindex(body={ - 'source': { - 'index': index - }, - 'dest': { - 'index': backup_index - } - }) - - try: - for i in range(0, len(dataset), ES_VARS['BULK_SIZE']): - body = [] - for org in dataset[i:i + ES_VARS['BULK_SIZE']]: - body.append({ - 'index': { - '_index': index, - '_type': 'org', - '_id': org['id'] - } - }) - org['names_ids'] = [{ - 'name': n - } for n in get_nested_names(org)] - org['names_ids'] += [{ - 'id': n - } for n in get_nested_ids(org)] - body.append(org) - ES.bulk(body) - except TransportError as e: - self.stdout.write(str(e)) - ES.reindex(body={ - 'source': { - 'index': backup_index - }, - 'dest': { - 'index': index - } - }) - - if ES.indices.exists(backup_index): - ES.indices.delete(backup_index) - self.stdout.write('ROR dataset ' + LEGACY_ROR_DUMP['VERSION'] + ' indexed') diff --git a/rorapi/management/commands/legacyseeschema.py b/rorapi/management/commands/legacyseeschema.py deleted file mode 100644 index 0e016a3a..00000000 --- a/rorapi/management/commands/legacyseeschema.py +++ /dev/null @@ -1,20 +0,0 @@ -import json -from rorapi.settings import ES, ES_VARS - -from django.core.management.base import BaseCommand - - -class Command(BaseCommand): - help = 'Create ROR API index' - - def handle(self, *args, **options): - index = ES_VARS['INDEX'] - if ES.indices.exists(index): - raw_data = ES.indices.get_mapping( index ) - schema = raw_data[ index ]["mappings"]["org"] - print (json.dumps(schema, indent=4)) - else: - with open(ES_VARS['INDEX_TEMPLATE'], 'r') as it: - template = json.load(it) - ES.indices.create(index=index, body=template) - self.stdout.write('Created index {}'.format(index)) diff --git a/rorapi/management/commands/legacyupgrade.py b/rorapi/management/commands/legacyupgrade.py deleted file mode 100644 index 3ea802bb..00000000 --- a/rorapi/management/commands/legacyupgrade.py +++ /dev/null @@ -1,18 +0,0 @@ -from django.core.management.base import BaseCommand -from .downloadgrid import Command as DownloadGridCommand -from .convertgrid import Command as ConvertGridCommand - -# Previously used to generate ROR dataset -# based on the latest GRID dataset configured in settings.py -# As of Mar 2022 ROR is no longer based on GRID -# New records are now created in https://github.com/ror-community/ror-records and pushed to S3 -# Individual record files in S3 are indexed with indexror.py -# Entire dataset zip files in https://github.com/ror-community/ror-data -# can be indexed with setup.py, which uses indexrordump.py - -class Command(BaseCommand): - help = 'Generate up-to-date ror.zip from GRID data' - - def handle(self, *args, **options): - DownloadGridCommand().handle(args, options) - ConvertGridCommand().handle(args, options) diff --git a/rorapi/settings.py b/rorapi/settings.py index dd558a4e..a874037e 100644 --- a/rorapi/settings.py +++ b/rorapi/settings.py @@ -185,93 +185,11 @@ timeout=240, connection_class=RequestsHttpConnection) -# ROR DUMP grid-2018-11-14 -# GRID = { -# 'VERSION': '2018-11-14', -# 'URL': 'https://ndownloader.figshare.com/files/13575374' -# } - -# ROR DUMP grid-2019-02-17 -# GRID = { -# 'VERSION': '2019-02-17', -# 'URL': 'https://digitalscience.figshare.com/ndownloader/files/14399291' -# } - -# ROR DUMP ror-2019-09-19 -# GRID = { -# 'VERSION': '2019-05-06', -# 'URL': 'https://digitalscience.figshare.com/ndownloader/files/15167609' -# } - -# ROR DUMP ror-2019-11-07 -# GRID = { -# 'VERSION': '2019-10-06', -# 'URL': 'https://digitalscience.figshare.com/ndownloader/files/17948195' -# } - -# ROR DUMP ror-2019-12-18 -# GRID = { -# 'VERSION': '2019-12-10', -# 'URL': 'https://digitalscience.figshare.com/ndownloader/files/20151785' -# } - -# ROR DUMP ror-2020-03-15 -# GRID = { -# 'VERSION': '2020-03-15', -# 'URL': 'https://digitalscience.figshare.com/ndownloader/files/22091379' -# } - -# ROR DUMP ror-2020-07-06 -# GRID = { -# 'VERSION': '2020-06-29', -# 'URL': 'https://digitalscience.figshare.com/ndownloader/files/23552738' -# } - -# ROR DUMP ror-2020-10-19 -#GRID = { -# 'VERSION': '2020-10-06', -# 'URL': 'https://digitalscience.figshare.com/ndownloader/files/25039403' -# } - -# ROR DUMP 2020-12-21 and 2021-03-17 -#GRID = { -# 'VERSION': '2020-12-09', -# 'URL': 'https://digitalscience.figshare.com/ndownloader/files/25791104' - -# ROR DUMP 2021-04-06 -#GRID = { -# 'VERSION': '2021-03-25', -# 'URL': 'https://digitalscience.figshare.com/ndownloader/files/27251693' -#} - -# ROR DUMP 2021-09-23 -GRID = { - 'VERSION': '2021-09-16', - 'URL': 'no url' -} -# The latest GRID update, 2021-09-16, was shared via google drive. - -# GRID and LEGACY_ROR_DUMP vars were previously used to -# generate ROR dataset based on the latest GRID dataset -# Directories and files that these vars point to have been moved -# to https://github.com/ror-community/ror-data -# Scripts preprended with 'legacy' no longer work -# As of Mar 2022 ROR is no longer based on GRID -# New records are now created in https://github.com/ror-community/ror-records and pushed to S3 -# Individual record files in S3 are indexed with indexror.py +# As of Mar 2022 ROR is no longer based on GRID. +# New records are created in https://github.com/ror-community/ror-records and pushed to S3. +# Individual record files in S3 are indexed with indexror.py. # Entire dataset zip files in https://github.com/ror-community/ror-data -# can be indexed with setup.py, which uses indexrordump.py - -GRID['DIR'] = os.path.join(BASE_DIR, 'rorapi', 'data', - 'grid-{}'.format(GRID['VERSION'])) -GRID['GRID_ZIP_PATH'] = os.path.join(GRID['DIR'], 'grid.zip') -GRID['GRID_JSON_PATH'] = os.path.join(GRID['DIR'], 'grid.json') - -LEGACY_ROR_DUMP = {'VERSION': '2021-09-23'} -LEGACY_ROR_DUMP['DIR'] = os.path.join(BASE_DIR, 'rorapi', 'data', - 'ror-{}'.format(LEGACY_ROR_DUMP['VERSION'])) -LEGACY_ROR_DUMP['ROR_ZIP_PATH'] = os.path.join(LEGACY_ROR_DUMP['DIR'], 'ror.zip') -LEGACY_ROR_DUMP['ROR_JSON_PATH'] = os.path.join(LEGACY_ROR_DUMP['DIR'], 'ror.json') +# can be indexed with setup.py, which uses indexrordump.py. ROR_DUMP = {} ROR_DUMP['PROD_REPO_URL'] = 'https://api.github.com/repos/ror-community/ror-data' @@ -296,10 +214,6 @@ DATA['DIR'] = os.path.join(BASE_DIR, 'rorapi', 'data') ROR_API = {'PAGE_SIZE': 20, 'ID_PREFIX': 'https://ror.org/'} -GRID_REMOVED_IDS = [] - -LAUNCH_DARKLY_KEY = os.environ.get('LAUNCH_DARKLY_KEY') - # Toggle for behavior-based rate limiting ENABLE_BEHAVIORAL_LIMITING = os.getenv("ENABLE_BEHAVIORAL_LIMITING", "False") == "True" diff --git a/rorapi/v2/models.py b/rorapi/v2/models.py index b937d7dd..9cfdb136 100644 --- a/rorapi/v2/models.py +++ b/rorapi/v2/models.py @@ -1,4 +1,3 @@ -from geonamescache.mappers import country import random import string from django.db import models @@ -13,19 +12,6 @@ def __init__(self, data): self.title = continent_code_to_name(data.key) self.count = data.doc_count -class CountryBucket: - """A model class for country aggregation bucket""" - - def __init__(self, data): - self.id = data.key.lower() - mapper = country(from_key="iso", to_key="name") - try: - self.title = mapper(data.key) - except AttributeError: - # if we have a country code with no name mapping, skip it to prevent 500 - pass - self.count = data.doc_count - class Aggregations: """Aggregations model class""" diff --git a/rorapi/v2/tests.py b/rorapi/v2/tests.py deleted file mode 100644 index 02b18058..00000000 --- a/rorapi/v2/tests.py +++ /dev/null @@ -1,12 +0,0 @@ -from django.test import TestCase -from rorapi.v2.models import Client - -class ClientTests(TestCase): - def test_client_registration(self): - client = Client.objects.create(email='test@example.com') - self.assertIsNotNone(client.client_id) - - def test_validate_client_id(self): - response = self.client.get('/validate-client-id/INVALID_ID/') - self.assertEqual(response.status_code, 200) - self.assertFalse(response.json()['valid']) From bef5d03ffd6cbd206f86086694208dfa48570c35 Mon Sep 17 00:00:00 2001 From: Joseph Rhoads Date: Tue, 29 Sep 2026 10:47:31 +0200 Subject: [PATCH 03/11] Drop unused Django contrib apps and middleware (#591) Co-authored-by: Cursor Agent --- rorapi/settings.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/rorapi/settings.py b/rorapi/settings.py index a874037e..dea38ed7 100644 --- a/rorapi/settings.py +++ b/rorapi/settings.py @@ -55,11 +55,7 @@ # Application definition INSTALLED_APPS = [ - 'django.contrib.admin', - 'django.contrib.auth', 'django.contrib.contenttypes', - 'django.contrib.sessions', - 'django.contrib.messages', 'django.contrib.staticfiles', 'rest_framework', 'django_prometheus', @@ -72,12 +68,7 @@ 'corsheaders.middleware.CorsMiddleware', 'rorapi.middleware.cors.AlwaysAllowOriginMiddleware', 'django.middleware.security.SecurityMiddleware', - 'django.contrib.sessions.middleware.SessionMiddleware', 'django.middleware.common.CommonMiddleware', - 'django.middleware.csrf.CsrfViewMiddleware', - 'django.contrib.auth.middleware.AuthenticationMiddleware', - 'django.contrib.messages.middleware.MessageMiddleware', - 'django.middleware.clickjacking.XFrameOptionsMiddleware', 'django_prometheus.middleware.PrometheusAfterMiddleware' ] @@ -105,6 +96,9 @@ REST_FRAMEWORK = { 'DEFAULT_RENDERER_CLASSES': ('rest_framework.renderers.JSONRenderer', ), + 'DEFAULT_AUTHENTICATION_CLASSES': [], + 'UNAUTHENTICATED_USER': None, + 'UNAUTHENTICATED_TOKEN': None, 'DEFAULT_VERSIONING_CLASS': 'rest_framework.versioning.URLPathVersioning', 'DEFAULT_VERSION': 'v2', 'ALLOWED_VERSIONS': ['v2'], From 7e5268461078f8db7e9d2b3e830786af60011ee4 Mon Sep 17 00:00:00 2001 From: Joseph Rhoads Date: Tue, 29 Sep 2026 17:55:51 +0200 Subject: [PATCH 04/11] Make v2 search case- and accent-insensitive (#592) Co-authored-by: Cursor Agent --- rorapi/common/queries.py | 26 ++++-- .../tests_integration/tests_search_v2.py | 78 ++++++++++++++++++ rorapi/tests/tests_unit/tests_queries_v2.py | 40 +++++++++- rorapi/v2/index_template_es7.json | 79 +++++++++++++++---- 4 files changed, 194 insertions(+), 29 deletions(-) diff --git a/rorapi/common/queries.py b/rorapi/common/queries.py index e3c4a4a5..1577c0e0 100644 --- a/rorapi/common/queries.py +++ b/rorapi/common/queries.py @@ -1,5 +1,6 @@ import re import json +import unicodedata from titlecase import titlecase from collections import defaultdict @@ -187,6 +188,14 @@ def validate(params): return Errors(errors) if errors else None +def nfc(value): + """Normalizes a string to Unicode NFC so decomposed (NFD) input matches + the precomposed characters indexed in Elasticsearch""" + if isinstance(value, str): + return unicodedata.normalize("NFC", value) + return value + + def build_search_query(params): """Builds search query from API parameters""" @@ -198,20 +207,21 @@ def build_search_query(params): del params["all_status"] if "query.advanced" in params: - qb.add_string_query_advanced(params.get("query.advanced")) + qb.add_string_query_advanced(nfc(params.get("query.advanced"))) elif "query" in params: - ror_id = get_ror_id(params.get("query")) + query = nfc(params.get("query")) + ror_id = get_ror_id(query) if ror_id is not None: qb.add_id_query(ror_id) else: - qb.add_string_query(params.get("query")) + qb.add_string_query(query) else: qb.add_match_all_query() if "filter" in params or (not "all_status" in params): filters = [ f.split(":") - for f in filter_string_to_list(params.get("filter", "")) + for f in filter_string_to_list(nfc(params.get("filter", ""))) if f ] # normalize filter values based on casing conventions used in ROR records @@ -247,10 +257,10 @@ def build_search_query(params): qb.add_aggregations( [ - ("types", "types"), - ("countries", "locations.geonames_details.country_code"), - ("continents", "locations.geonames_details.continent_code"), - ("statuses", "status"), + ("types", "types.raw"), + ("countries", "locations.geonames_details.country_code.raw"), + ("continents", "locations.geonames_details.continent_code.raw"), + ("statuses", "status.raw"), ] ) diff --git a/rorapi/tests/tests_integration/tests_search_v2.py b/rorapi/tests/tests_integration/tests_search_v2.py index 462841be..c8279de2 100644 --- a/rorapi/tests/tests_integration/tests_search_v2.py +++ b/rorapi/tests/tests_integration/tests_search_v2.py @@ -133,3 +133,81 @@ def test_extra_word(self): }).json() self.assertTrue(items['number_of_results'] > 0) self.assertEqual(items['items'][0]['id'], 'https://ror.org/00fbnyb24') + + +class CaseAndAccentInsensitiveTestCase(SimpleTestCase): + """Case- and diacritic-insensitive matching (ror-roadmap#175, #398). + + Requires an index created from the current index_template_es7.json. + """ + + def search(self, **params): + return requests.get(BASE_URL, params).json() + + def assert_same_results(self, variants, param='query.advanced'): + results = [self.search(**{param: v}) for v in variants] + baseline = results[0] + self.assertTrue(baseline['number_of_results'] > 0, variants[0]) + baseline_ids = {i['id'] for i in baseline['items']} + for variant, result in zip(variants[1:], results[1:]): + self.assertEqual(result['number_of_results'], + baseline['number_of_results'], variant) + self.assertEqual({i['id'] for i in result['items']}, + baseline_ids, variant) + + def test_keyword_field_case(self): + self.assert_same_results([ + 'locations.geonames_details.name:Denver', + 'locations.geonames_details.name:denver', + 'locations.geonames_details.name:DENVER', + ]) + + def test_relationship_type_case(self): + self.assert_same_results([ + 'relationships.type:child', + 'relationships.type:Child', + ]) + + def test_bare_term_case(self): + self.assert_same_results(['Stellenbosch', 'stellenbosch']) + + def test_keyword_field_diacritics(self): + self.assert_same_results([ + 'locations.geonames_details.name:Huế', + 'locations.geonames_details.name:Hue', + 'locations.geonames_details.name:hue', + ]) + self.assert_same_results([ + 'locations.geonames_details.name:Montréal', + 'locations.geonames_details.name:Montreal', + ]) + + def test_nfd_and_nfc_input(self): + nfc = 'locations.geonames_details.name:Hu\u1ebf' + nfd = 'locations.geonames_details.name:Hu\u0065\u0302\u0301' + self.assertNotEqual(nfc, nfd) + self.assert_same_results([nfc, nfd]) + + def test_text_field_diacritics(self): + self.assert_same_results([ + 'names.value:"Université de Montréal"', + 'names.value:"Universite de Montreal"', + 'names.value:"universite de montreal"', + ]) + + def test_filter_case(self): + self.assert_same_results(['Denver'], param='query') + upper = requests.get(BASE_URL, {'filter': 'types:Education'}).json() + lower = requests.get(BASE_URL, {'filter': 'types:education'}).json() + self.assertTrue(upper['number_of_results'] > 0) + self.assertEqual(upper['number_of_results'], + lower['number_of_results']) + + def test_aggregation_keys_keep_original_casing(self): + # Bucket titles are derived from the raw (un-normalized) keys, e.g. the + # country name is looked up from the upper-case ISO code. + meta = self.search(**{'query.advanced': 'locations.geonames_details.name:denver'})['meta'] + countries = {b['id']: b['title'] for b in meta['countries']} + self.assertEqual(countries.get('us'), 'United States') + continents = {b['id']: b['title'] for b in meta['continents']} + self.assertEqual(continents.get('na'), 'North America') diff --git a/rorapi/tests/tests_unit/tests_queries_v2.py b/rorapi/tests/tests_unit/tests_queries_v2.py index 130ff931..251b5424 100644 --- a/rorapi/tests/tests_unit/tests_queries_v2.py +++ b/rorapi/tests/tests_unit/tests_queries_v2.py @@ -168,10 +168,10 @@ class BuildSearchQueryTestCase(SimpleTestCase): def setUp(self): self.default_query = \ - {'aggs': {'types': {'terms': {'field': 'types', 'size': 10, 'min_doc_count': 1}}, - 'countries': {'terms': {'field': 'locations.geonames_details.country_code', 'size': 10, 'min_doc_count': 1}}, - 'continents': {'terms': {'field': 'locations.geonames_details.continent_code', 'size': 10, 'min_doc_count': 1}}, - 'statuses': {'terms': {'field': 'status', 'size': 10, 'min_doc_count': 1}}}, + {'aggs': {'types': {'terms': {'field': 'types.raw', 'size': 10, 'min_doc_count': 1}}, + 'countries': {'terms': {'field': 'locations.geonames_details.country_code.raw', 'size': 10, 'min_doc_count': 1}}, + 'continents': {'terms': {'field': 'locations.geonames_details.continent_code.raw', 'size': 10, 'min_doc_count': 1}}, + 'statuses': {'terms': {'field': 'status.raw', 'size': 10, 'min_doc_count': 1}}}, 'track_total_hits': True, 'from': 0, 'size': 20} def test_empty_query_default(self): @@ -284,6 +284,38 @@ def test_query_advanced(self): query = build_search_query({'query.advanced': 'query terms'}) self.assertEqual(query.to_dict(), expected) + def test_query_advanced_nfc_normalization(self): + nfd = 'locations.geonames_details.name:Hu\u0065\u0302\u0301' + nfc_form = 'locations.geonames_details.name:Hu\u1ebf' + self.assertNotEqual(nfd, nfc_form) + query = build_search_query({'query.advanced': nfd}).to_dict() + self.assertEqual( + query['query']['bool']['must'][0]['query_string']['query'], nfc_form) + + def test_query_nfc_normalization(self): + query = build_search_query({'query': 'Montre\u0301al'}).to_dict() + self.assertEqual( + query['query']['bool']['must'][0]['nested']['query']['query_string']['query'], + 'Montr\u00e9al') + + def test_filter_nfc_normalization(self): + query = build_search_query({ + 'filter': 'locations.geonames_details.country_name:Cura\u0063\u0327ao', + 'all_status': '' + }).to_dict() + self.assertIn( + {'terms': {'locations.geonames_details.country_name': ('Cura\u00e7ao',)}}, + query['query']['bool']['filter']) + + def test_aggregations_use_raw_subfields(self): + aggs = build_search_query({}).to_dict()['aggs'] + self.assertEqual(aggs['types']['terms']['field'], 'types.raw') + self.assertEqual(aggs['countries']['terms']['field'], + 'locations.geonames_details.country_code.raw') + self.assertEqual(aggs['continents']['terms']['field'], + 'locations.geonames_details.continent_code.raw') + self.assertEqual(aggs['statuses']['terms']['field'], 'status.raw') + def test_query_advanced_all_status(self): expected = {'query': { 'bool': { diff --git a/rorapi/v2/index_template_es7.json b/rorapi/v2/index_template_es7.json index a16c10db..152ab18d 100644 --- a/rorapi/v2/index_template_es7.json +++ b/rorapi/v2/index_template_es7.json @@ -19,6 +19,15 @@ "type": "asciifolding", "preserve_original": true } + }, + "normalizer": { + "folding_normalizer": { + "type": "custom", + "filter": [ + "lowercase", + "asciifolding" + ] + } } } }, @@ -61,7 +70,8 @@ "type": "keyword" }, "type": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer" }, "preferred": { "type": "keyword" @@ -78,7 +88,8 @@ "analyzer": "simple" }, "type": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer" } } }, @@ -90,22 +101,38 @@ "geonames_details": { "properties": { "continent_code": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer", + "fields": { + "raw": { + "type": "keyword" + } + } }, "continent_name": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer" }, "country_code": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer", + "fields": { + "raw": { + "type": "keyword" + } + } }, "country_name": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer" }, "country_subdivision_code": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer" }, "country_subdivision_name": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer" }, "lat": { "type": "float" @@ -114,7 +141,8 @@ "type": "float" }, "name": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer" } } } @@ -133,23 +161,33 @@ "analyzer": "string_lowercase", "fielddata": true } - } + }, + "analyzer": "string_lowercase" }, "lang": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer" }, "types": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer" } } }, "types": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer", + "fields": { + "raw": { + "type": "keyword" + } + } }, "relationships": { "properties": { "type": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer" }, "label": { "type": "text", @@ -162,7 +200,8 @@ "analyzer": "string_lowercase", "fielddata": true } - } + }, + "analyzer": "string_lowercase" }, "id": { "type": "keyword" @@ -170,7 +209,13 @@ } }, "status": { - "type": "keyword" + "type": "keyword", + "normalizer": "folding_normalizer", + "fields": { + "raw": { + "type": "keyword" + } + } }, "names_ids": { "type": "nested", @@ -223,4 +268,4 @@ } } } -} \ No newline at end of file +} From bea10296f980efad0408d60febfeb0e18ef7d494 Mon Sep 17 00:00:00 2001 From: Joseph Rhoads Date: Wed, 30 Sep 2026 18:00:08 +0200 Subject: [PATCH 05/11] Replace application prints and bare excepts with logging (#593) Co-authored-by: Cursor Agent --- rorapi/common/create_update.py | 10 ++- rorapi/common/csv_bulk.py | 14 ++-- rorapi/common/csv_create.py | 12 ++-- rorapi/common/csv_update.py | 84 +++++++++++++----------- rorapi/common/csv_utils.py | 44 +++++++------ rorapi/common/record_utils.py | 8 ++- rorapi/common/views.py | 12 ++-- rorapi/management/commands/getrordump.py | 4 +- rorapi/management/commands/setup.py | 4 +- rorapi/settings.py | 20 +++++- 10 files changed, 129 insertions(+), 83 deletions(-) diff --git a/rorapi/common/create_update.py b/rorapi/common/create_update.py index f965fba0..05bc3373 100644 --- a/rorapi/common/create_update.py +++ b/rorapi/common/create_update.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import copy import functools import json @@ -60,10 +64,10 @@ def update_locations(locations): for location in locations: if 'geonames_id' in location: try: - print(location['geonames_id']) + logger.info(location['geonames_id']) updated_location = ua.new_geonames_v2(str(location['geonames_id'])) updated_locations.append(updated_location['location']) - except: + except Exception: error = "Error retrieving Geonames data for ID {}. Please check that this is a valid Geonames ID".format(location['geonames_id']) return error, updated_locations @@ -90,7 +94,7 @@ def new_record_from_json(json_input, version): new_record['locations'] = updated_locations new_record = add_created_last_mod(new_record) new_ror_id = check_ror_id() - print("new ror id: " + new_ror_id) + logger.info("new ror id: " + new_ror_id) new_record['id'] = new_ror_id error, valid_data = validate_record(sort_list_fields(new_record), get_v2_schema()) return error, valid_data diff --git a/rorapi/common/csv_bulk.py b/rorapi/common/csv_bulk.py index c6dbe8fe..d5ee30be 100644 --- a/rorapi/common/csv_bulk.py +++ b/rorapi/common/csv_bulk.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import csv import json import io @@ -43,7 +47,7 @@ def save_report_file(report, report_fields, csv_file, dir_name, validate_only): f.write(chunk) def process_csv(csv_file, version, validate_only): - print("Processing CSV") + logger.info("Processing CSV") dir_name = datetime.now().strftime("%Y-%m-%d_%H_%M_%S") + "-ror-records" success_msg = None error = None @@ -53,15 +57,15 @@ def process_csv(csv_file, version, validate_only): updated_count = 0 new_count = 0 read_file = csv_file.read().decode('utf-8') - print(read_file) + logger.info(read_file) reader = csv.DictReader(io.StringIO(read_file)) row_num = 2 for row in reader: html_url = None ror_id = None updated = False - print("Row data") - print(row) + logger.info("Row data") + logger.info(row) if row['html_url']: html_url = row['html_url'] if row['id']: @@ -80,7 +84,7 @@ def process_csv(csv_file, version, validate_only): ror_id = v2_record['id'] serializer = OrganizationSerializerV2(v2_record) json_obj = json.loads(JSONRenderer().render(serializer.data)) - print(json_obj) + logger.info(json_obj) if not validate_only: #create file file = save_record_file(ror_id, updated, json_obj, dir_name) diff --git a/rorapi/common/csv_create.py b/rorapi/common/csv_create.py index 10ff03b2..154b90bc 100644 --- a/rorapi/common/csv_create.py +++ b/rorapi/common/csv_create.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import copy from rorapi.common.record_utils import * from rorapi.common.csv_utils import * @@ -74,8 +78,8 @@ def new_record_from_csv(csv_data, version): "lang": lang_code } temp_names.append(name_obj) - print("temp names 1:") - print(temp_names) + logger.info("temp names 1:") + logger.info(temp_names) name_vals = [n['value'] for n in temp_names] dup_names = [] for n in name_vals: @@ -99,8 +103,8 @@ def new_record_from_csv(csv_data, version): if name_obj not in temp_names: temp_names = [t for t in temp_names if t not in name_lang_dups] temp_names.append(name_obj) - print("temp names 2:") - print(temp_names) + logger.info("temp names 2:") + logger.info(temp_names) v2_data['names'] = temp_names #status diff --git a/rorapi/common/csv_update.py b/rorapi/common/csv_update.py index 2299e2a9..16cb78da 100644 --- a/rorapi/common/csv_update.py +++ b/rorapi/common/csv_update.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import copy from rorapi.common.record_utils import * from rorapi.v2.record_constants import * @@ -12,29 +16,29 @@ def update_record_from_csv(csv_data, version): errors = [] updated_record = None - print("updating record from csv") + logger.info("updating record from csv") existing_org_errors, existing_org = retrieve_organization(csv_data['id']) - print(existing_org) + logger.info(existing_org) if existing_org is None: errors.append("No existing record found for ROR ID '{}'".format(csv_data['id'])) else: row_validation_errors = validate_csv_row_update_syntax(csv_data) if row_validation_errors: errors.extend(row_validation_errors) - print("row validation errors:") - print(errors) + logger.info("row validation errors:") + logger.info(errors) else: serializer = OrganizationSerializerV2(existing_org) existing_record = serializer.data - print(existing_record) + logger.info(existing_record) update_data = {} #domains if csv_data['domains']: actions_values = get_actions_values(csv_data['domains']) temp_domains = copy.deepcopy(existing_record['domains']) - print("initial temp domains:") - print(temp_domains) + logger.info("initial temp domains:") + logger.info(temp_domains) if UPDATE_ACTIONS['DELETE'] in actions_values: delete_values = actions_values[UPDATE_ACTIONS['DELETE']] if delete_values is None: @@ -45,23 +49,23 @@ def update_record_from_csv(csv_data, version): errors.append("Attempting to delete domain(s) that don't exist: {}".format(d)) temp_domains = [d for d in temp_domains if d not in delete_values] - print("temp domains delete") - print(temp_domains) + logger.info("temp domains delete") + logger.info(temp_domains) if UPDATE_ACTIONS['ADD'] in actions_values: add_values = actions_values[UPDATE_ACTIONS['ADD']] for a in add_values: if a in temp_domains: errors.append("Attempting to add domain(s) that already exist: {}".format(a)) - print(add_values) + logger.info(add_values) temp_domains.extend(add_values) - print("temp domains add") - print(temp_domains) + logger.info("temp domains add") + logger.info(temp_domains) if UPDATE_ACTIONS['REPLACE'] in actions_values: temp_domains = actions_values[UPDATE_ACTIONS['REPLACE']] - print("temp domains replace") - print(temp_domains) - print("final temp domains:") - print(temp_domains) + logger.info("temp domains replace") + logger.info(temp_domains) + logger.info("final temp domains:") + logger.info(temp_domains) update_data['domains'] = temp_domains #established @@ -180,18 +184,18 @@ def update_record_from_csv(csv_data, version): "value": r } temp_links.append(link_obj) - print("final temp links:") - print(temp_links) + logger.info("final temp links:") + logger.info(temp_links) update_data['links'] = temp_links #locations if csv_data['locations.geonames_id']: actions_values = get_actions_values(csv_data['locations.geonames_id']) temp_locations = copy.deepcopy(existing_record['locations']) - print("initial temp locations:") - print(temp_locations) + logger.info("initial temp locations:") + logger.info(temp_locations) existing_geonames_ids = [tl['geonames_id'] for tl in temp_locations] - print(existing_geonames_ids) + logger.info(existing_geonames_ids) if UPDATE_ACTIONS['DELETE'] in actions_values: delete_values = [int(d) for d in actions_values[UPDATE_ACTIONS['DELETE']]] for d in delete_values: @@ -219,8 +223,8 @@ def update_record_from_csv(csv_data, version): "geonames_details": {} } temp_locations.append(location_obj) - print("final temp locations:") - print(temp_locations) + logger.info("final temp locations:") + logger.info(temp_locations) update_data['locations'] = temp_locations #names @@ -228,20 +232,20 @@ def update_record_from_csv(csv_data, version): for k,v in V2_NAME_TYPES.items(): if csv_data['names.types.' + v]: updated_name_types.append(v) - print("updated name types") - print(updated_name_types) + logger.info("updated name types") + logger.info(updated_name_types) if updated_name_types: temp_names = copy.deepcopy(existing_record['names']) for t in updated_name_types: - print("updating name type " + t) + logger.info("updating name type " + t) if csv_data['names.types.' + t]: actions_values = get_actions_values(csv_data['names.types.' + t]) for k, v in actions_values.items(): if v: vals_obj_list = [] for val in v: - print("val is") - print(val) + logger.info("val is") + logger.info(val) vals_obj = { "value": None, "lang": None @@ -263,12 +267,12 @@ def update_record_from_csv(csv_data, version): if vals_obj["value"]: vals_obj_list.append(vals_obj) actions_values[k] = vals_obj_list - print("updated actions values") - print(actions_values) + logger.info("updated actions values") + logger.info(actions_values) if UPDATE_ACTIONS['DELETE'] in actions_values: - print("delete in actions") + logger.info("delete in actions") delete_values = actions_values[UPDATE_ACTIONS['DELETE']] - print(delete_values) + logger.info(delete_values) if delete_values is None: temp_names = [tn for tn in temp_names if t not in tn['types']] else: @@ -299,8 +303,8 @@ def update_record_from_csv(csv_data, version): else: name_vals_match = [tn for tn in temp_names if (tn['value'] == a['value'] and tn['lang'] == a['lang'])] if name_vals_match: - print("name vals match") - print(name_vals_match) + logger.info("name vals match") + logger.info(name_vals_match) for nvm in name_vals_match: # if value and lang exist but not type, add type only if len(nvm['types']) > 0: @@ -346,8 +350,8 @@ def update_record_from_csv(csv_data, version): } temp_names.append(name_obj) - print("final temp names:") - print(temp_names) + logger.info("final temp names:") + logger.info(temp_names) update_data['names'] = temp_names #status @@ -362,8 +366,8 @@ def update_record_from_csv(csv_data, version): if csv_data['types']: actions_values = get_actions_values(csv_data['types']) temp_types = copy.deepcopy(existing_record['types']) - print("initial temp types:") - print(temp_types) + logger.info("initial temp types:") + logger.info(temp_types) if UPDATE_ACTIONS['DELETE'] in actions_values: delete_values = [av.lower() for av in actions_values[UPDATE_ACTIONS['DELETE']]] for d in delete_values: @@ -380,8 +384,8 @@ def update_record_from_csv(csv_data, version): temp_types.extend(add_values) if UPDATE_ACTIONS['REPLACE'] in actions_values: temp_types = [av.lower() for av in actions_values[UPDATE_ACTIONS['REPLACE']]] - print("final temp types:") - print(temp_types) + logger.info("final temp types:") + logger.info(temp_types) update_data['types'] = temp_types if not errors: diff --git a/rorapi/common/csv_utils.py b/rorapi/common/csv_utils.py index 8c4d7062..d0e8e1f1 100644 --- a/rorapi/common/csv_utils.py +++ b/rorapi/common/csv_utils.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import csv import io import re @@ -43,27 +47,27 @@ def get_actions_values(csv_field): - print("getting actions values:") + logger.info("getting actions values:") actions_values = {} if csv_field.lower() == UPDATE_ACTIONS["DELETE"]: actions_values[UPDATE_ACTIONS["DELETE"]] = None elif UPDATE_DELIMITER in csv_field: for ua in list(UPDATE_ACTIONS.values()): - print(ua) + logger.info(ua) if ua + UPDATE_DELIMITER in csv_field: - print("doing regex:") + logger.info("doing regex:") regex = r"(" + re.escape( ua + UPDATE_DELIMITER) + r")(.*?)(?=$|(add|delete|replace)==)" result = re.search(regex, csv_field) - print(result[0]) + logger.info(result[0]) temp_val = result[0].replace(ua + UPDATE_DELIMITER, '') - print("temp val:") - print(temp_val) + logger.info("temp val:") + logger.info(temp_val) actions_values[ua] = [v.strip() for v in temp_val.split(';') if v] else: actions_values[UPDATE_ACTIONS["REPLACE"]] = [v.strip() for v in csv_field.split(';') if v] - print(actions_values) + logger.info(actions_values) return actions_values def validate_csv(csv_file): @@ -80,28 +84,28 @@ def validate_csv(csv_file): for field in CSV_REQUIRED_FIELDS_ACTIONS.keys(): if field not in csv_fields: missing_fields.append(field) - print(missing_fields) + logger.info(missing_fields) if missing_fields: errors.append(f'CSV file is missing columns: {", ".join(missing_fields)}') else: errors.append("CSV file contains no data rows") except IOError as e: errors.append(f"Error parsing CSV file: {e}") - print(errors) + logger.info(errors) return errors def validate_csv_row_update_syntax(csv_data): - print("validating row") + logger.info("validating row") errors = [] for k, v in csv_data.items(): if UPDATE_DELIMITER in v: - print("field:") - print(k) - print("value:") - print(v) + logger.info("field:") + logger.info(k) + logger.info("value:") + logger.info(v) actions_values = get_actions_values(v) - print("actions values:") - print(actions_values) + logger.info("actions values:") + logger.info(actions_values) update_actions = list(actions_values.keys()) if not update_actions: errors.append("Update delimiter '{}' found in '{}' field but no valid update action found in value {}".format(UPDATE_DELIMITER, k, v)) @@ -111,10 +115,10 @@ def validate_csv_row_update_syntax(csv_data): if not (UPDATE_ACTIONS['ADD'] and UPDATE_ACTIONS['DELETE']) in update_actions: errors.append("Invalid combination of update actions '{}' found in '{}' field.".format(", ".join(update_actions), k)) disallowed_actions = [ua for ua in update_actions if ua not in CSV_REQUIRED_FIELDS_ACTIONS[k]] - print("allowed actions:") - print(CSV_REQUIRED_FIELDS_ACTIONS[k]) - print("disallowed actions:") - print(disallowed_actions) + logger.info("allowed actions:") + logger.info(CSV_REQUIRED_FIELDS_ACTIONS[k]) + logger.info("disallowed actions:") + logger.info(disallowed_actions) if disallowed_actions: errors.append("Invalid update action(s) '{}' found in {} field. Allowed actions for this field are '{}'".format(", ".join(disallowed_actions), k, ", ".join(CSV_REQUIRED_FIELDS_ACTIONS[k]))) if v.strip() == UPDATE_ACTIONS['DELETE'].lower() and k in NO_DELETE_FIELDS: diff --git a/rorapi/common/record_utils.py b/rorapi/common/record_utils.py index 23327857..a8e2fee5 100644 --- a/rorapi/common/record_utils.py +++ b/rorapi/common/record_utils.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import jsonschema import requests from iso639 import Lang @@ -24,8 +28,8 @@ def get_file_from_url(url): def validate_record(data, schema): try: - print("validating data:") - print(data) + logger.info("validating data:") + logger.info(data) jsonschema.validate(data, schema) except jsonschema.ValidationError as e: return "Validation error: " + e.message, None diff --git a/rorapi/common/views.py b/rorapi/common/views.py index 68aa99d0..0bbc8c98 100644 --- a/rorapi/common/views.py +++ b/rorapi/common/views.py @@ -1,3 +1,7 @@ +import logging + +logger = logging.getLogger(__name__) + import csv from rest_framework import viewsets, routers, status from rest_framework.response import Response @@ -160,7 +164,7 @@ class OrganizationViewSet(viewsets.ViewSet): def list(self, request, version=REST_FRAMEWORK["DEFAULT_VERSION"]): params = request.GET.dict() if "query.name" in params or "query.names" in params: - print("redirecting") + logger.info("redirecting") param_name = "query.name" if "query.name" in params else "query.names" params["query"] = params[param_name] del params[param_name] @@ -276,7 +280,7 @@ class GenerateId(APIView): def get(self, request, version=REST_FRAMEWORK["DEFAULT_VERSION"]): id = check_ror_id() - print("Generated ID: {}".format(id)) + logger.info("Generated ID: {}".format(id)) return Response({"id": id}) class IndexData(APIView): @@ -319,7 +323,7 @@ def post(self, request, version=REST_FRAMEWORK["DEFAULT_VERSION"]): errors = Errors(["File upload required. 'file' field is missing."]) else: mime_type = magic.from_buffer(file_object.read(2048)) - print(mime_type) + logger.info(mime_type) if "ASCII text" in mime_type or "UTF-8 text" in mime_type or "UTF-8 Unicode text" in mime_type or "CSV text" in mime_type: file_object.seek(0) csv_validation_errors = validate_csv(file_object) @@ -338,7 +342,7 @@ def post(self, request, version=REST_FRAMEWORK["DEFAULT_VERSION"]): else: errors = Errors(["Could not process request. No data included in request."]) if errors is not None: - print(errors.__dict__) + logger.info(errors.__dict__) return Response( ErrorsSerializer(errors).data, status=status.HTTP_400_BAD_REQUEST ) diff --git a/rorapi/management/commands/getrordump.py b/rorapi/management/commands/getrordump.py index cb75d74b..2e2d5702 100644 --- a/rorapi/management/commands/getrordump.py +++ b/rorapi/management/commands/getrordump.py @@ -27,7 +27,7 @@ def get_ror_dump_sha(filename, use_test_data, github_headers): if filename in file['name']: sha = file['sha'] return sha - except: + except Exception: return None def get_ror_dump_zip(self, filename, use_test_data, github_headers): @@ -52,7 +52,7 @@ def get_ror_dump_zip(self, filename, use_test_data, github_headers): if dir_names: raise SystemExit(f"Dump zip has extra directory and cannot be indexed") return zip_file.name - except: + except Exception: raise SystemExit(f"Something went wrong saving zip file") class Command(BaseCommand): diff --git a/rorapi/management/commands/setup.py b/rorapi/management/commands/setup.py index fc9f8bfb..018a9339 100644 --- a/rorapi/management/commands/setup.py +++ b/rorapi/management/commands/setup.py @@ -111,9 +111,9 @@ def handle(self, *args, **options): filename = options['filename'] use_test_data = options['testdata'] if use_test_data: - print("Using ror-data-test repo") + logger.info("Using ror-data-test repo") else: - print("Using ror-data repo") + logger.info("Using ror-data repo") try: sha = get_ror_dump_sha(filename, use_test_data) diff --git a/rorapi/settings.py b/rorapi/settings.py index dea38ed7..8931bbb0 100644 --- a/rorapi/settings.py +++ b/rorapi/settings.py @@ -10,6 +10,7 @@ https://docs.djangoproject.com/en/5.2/ref/settings/ """ +import logging import os import sys import json @@ -203,7 +204,8 @@ #DATA['CLIENT'] = localboto3.client('s3') #DATA['OBJECT'] = DATA['CLIENT'].list_objects_v2(Bucket = DATA['DATA_STORE']) else: - print("Please set the DATA_STORE environment variable or run this codebase through docker compose") + logging.getLogger(__name__).warning( + "Please set the DATA_STORE environment variable or run this codebase through docker compose") DATA['DIR'] = os.path.join(BASE_DIR, 'rorapi', 'data') ROR_API = {'PAGE_SIZE': 20, 'ID_PREFIX': 'https://ror.org/'} @@ -220,3 +222,19 @@ AWS_ACCESS_KEY_ID = os.environ.get('AWS_ACCESS_KEY_ID') AWS_SECRET_ACCESS_KEY = os.environ.get('AWS_SECRET_ACCESS_KEY') AWS_SES_REGION_NAME = os.environ.get('AWS_REGION', 'eu-west-1') + +LOGGING = { + 'version': 1, + 'disable_existing_loggers': False, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + }, + }, + 'loggers': { + 'rorapi': { + 'handlers': ['console'], + 'level': 'INFO', + }, + }, +} From bebf5b8ba378f54a289dcfc8f442bac1394a4eee Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 21:53:59 +0200 Subject: [PATCH 06/11] Bump actions/setup-python from 6 to 7 (#583) Signed-off-by: dependabot[bot] --- .github/workflows/pull-request.yml | 2 +- .github/workflows/run_tests.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pull-request.yml b/.github/workflows/pull-request.yml index f3bc27cb..6f765f91 100644 --- a/.github/workflows/pull-request.yml +++ b/.github/workflows/pull-request.yml @@ -18,7 +18,7 @@ jobs: - name: Checkout ror-api code uses: actions/checkout@v2 - name: Set up Python environment - uses: actions/setup-python@v6 + uses: actions/setup-python@v7 with: python-version: "3.12" - name: Install ruff diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 142a0833..392cabdf 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -43,7 +43,7 @@ jobs: with: path: ror-api - name: Set up Python environment - uses: actions/setup-python@v6 + uses: actions/setup-python@v7 with: python-version: "3.12" cache: "pip" From f7c2a10e6d2dc5933829a36795aea250e9177bf7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 22:02:56 +0200 Subject: [PATCH 07/11] Bump docker/setup-buildx-action from 1 to 4 (#579) Signed-off-by: dependabot[bot] --- .github/workflows/dev.yml | 2 +- .github/workflows/release.yml | 2 +- .github/workflows/staging.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 7fcc925f..3230acb5 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -19,7 +19,7 @@ jobs: - name: Checkout uses: actions/checkout@v2 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v4 - name: Cache Docker layers uses: actions/cache@v4 with: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4dd3cdd2..1add6ccc 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -13,7 +13,7 @@ jobs: - name: Checkout uses: actions/checkout@v2 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v4 - name: Cache Docker layers uses: actions/cache@v4 with: diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index e52b0ada..87786bc1 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -15,7 +15,7 @@ jobs: - name: Checkout uses: actions/checkout@v2 - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v1 + uses: docker/setup-buildx-action@v4 - name: Cache Docker layers uses: actions/cache@v4 with: From 71fa945db0f8134dd125b25fd6086eff96bad7a3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 22:03:22 +0200 Subject: [PATCH 08/11] Bump docker/login-action from 1 to 4 (#580) Signed-off-by: dependabot[bot] --- .github/workflows/dev.yml | 2 +- .github/workflows/release.yml | 2 +- .github/workflows/staging.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 3230acb5..2bc55da7 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -28,7 +28,7 @@ jobs: restore-keys: | ${{ runner.os }}-buildx- - name: Login to DockerHub - uses: docker/login-action@v1 + uses: docker/login-action@v4 with: username: ${{ secrets.DOCKERHUB_RORAPI_USERNAME }} password: ${{ secrets.DOCKERHUB_RORAPI_TOKEN }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1add6ccc..0d995a33 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,7 +22,7 @@ jobs: restore-keys: | ${{ runner.os }}-buildx- - name: Login to DockerHub - uses: docker/login-action@v1 + uses: docker/login-action@v4 with: username: ${{ secrets.DOCKERHUB_RORAPI_USERNAME }} password: ${{ secrets.DOCKERHUB_RORAPI_TOKEN }} diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 87786bc1..70e37ef8 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -24,7 +24,7 @@ jobs: restore-keys: | ${{ runner.os }}-buildx- - name: Login to DockerHub - uses: docker/login-action@v1 + uses: docker/login-action@v4 with: username: ${{ secrets.DOCKERHUB_RORAPI_USERNAME }} password: ${{ secrets.DOCKERHUB_RORAPI_TOKEN }} From 6ee5810af109812a2249076c2acc921706db7f41 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 22:10:16 +0200 Subject: [PATCH 09/11] Bump docker/build-push-action from 2 to 7 (#578) Bumps [docker/build-push-action](https://github.com/docker/build-push-action) from 2 to 7. - [Release notes](https://github.com/docker/build-push-action/releases) - [Commits](https://github.com/docker/build-push-action/compare/v2...v7) --- updated-dependencies: - dependency-name: docker/build-push-action dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/dev.yml | 2 +- .github/workflows/release.yml | 2 +- .github/workflows/staging.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 2bc55da7..cb812ede 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -33,7 +33,7 @@ jobs: username: ${{ secrets.DOCKERHUB_RORAPI_USERNAME }} password: ${{ secrets.DOCKERHUB_RORAPI_TOKEN }} - name: Build and push - uses: docker/build-push-action@v2 + uses: docker/build-push-action@v7 with: context: . file: ./Dockerfile diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0d995a33..1d9d1680 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -31,7 +31,7 @@ jobs: echo "GIT_TAG=$(git tag --points-at HEAD)" >> $GITHUB_OUTPUT id: set_git_vars - name: Build and push - uses: docker/build-push-action@v2 + uses: docker/build-push-action@v7 with: context: . file: ./Dockerfile diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index 70e37ef8..b24f3952 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -29,7 +29,7 @@ jobs: username: ${{ secrets.DOCKERHUB_RORAPI_USERNAME }} password: ${{ secrets.DOCKERHUB_RORAPI_TOKEN }} - name: Build and push - uses: docker/build-push-action@v2 + uses: docker/build-push-action@v7 with: context: . file: ./Dockerfile From f571e3660175d7250f394d60854b9f41857b1c87 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed, 30 Sep 2026 22:34:50 +0200 Subject: [PATCH 10/11] Bump ad-m/github-push-action from 0.6.0 to 1.3.0 (#584) Bumps [ad-m/github-push-action](https://github.com/ad-m/github-push-action) from 0.6.0 to 1.3.0. - [Release notes](https://github.com/ad-m/github-push-action/releases) - [Commits](https://github.com/ad-m/github-push-action/compare/v0.6.0...v1.3.0) --- updated-dependencies: - dependency-name: ad-m/github-push-action dependency-version: 1.3.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/dev.yml | 2 +- .github/workflows/release.yml | 2 +- .github/workflows/staging.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index cb812ede..e848a26f 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -84,7 +84,7 @@ jobs: git add ror/services/api/environments/dev/_ror-api-dev.auto.tfvars git commit -m "Adding ror-api git variables for commit ${{ steps.extract_variables.outputs.GIT_SHA }}" - name: Push changes - uses: ad-m/github-push-action@v0.6.0 + uses: ad-m/github-push-action@v1.3.0 with: github_token: ${{ secrets.PERSONAL_ACCESS_TOKEN }} repository: "ror-community/new-deployment" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1d9d1680..c5781abd 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -80,7 +80,7 @@ jobs: git add ror/services/api/environments/prod/_ror-api.auto.tfvars git commit -m "Adding ror-api git variables for commit ${{ steps.extract_variables.outputs.GIT_SHA }}" - name: Push changes - uses: ad-m/github-push-action@v0.6.0 + uses: ad-m/github-push-action@v1.3.0 with: github_token: ${{ secrets.PERSONAL_ACCESS_TOKEN }} repository: 'ror-community/new-deployment' diff --git a/.github/workflows/staging.yml b/.github/workflows/staging.yml index b24f3952..80babe6f 100644 --- a/.github/workflows/staging.yml +++ b/.github/workflows/staging.yml @@ -80,7 +80,7 @@ jobs: git add ror/services/api/environments/staging/_ror-api-staging.auto.tfvars git commit -m "Adding ror-api git variables for commit ${{ steps.extract_variables.outputs.GIT_SHA }}" - name: Push changes - uses: ad-m/github-push-action@v0.6.0 + uses: ad-m/github-push-action@v1.3.0 with: github_token: ${{ secrets.PERSONAL_ACCESS_TOKEN }} repository: 'ror-community/new-deployment' From cee31f0e2e0d46955543197c9f49de6751acde6f Mon Sep 17 00:00:00 2001 From: Joseph Rhoads Date: Thu, 1 Oct 2026 14:37:49 +0200 Subject: [PATCH 11/11] Fix advanced search for spaced external IDs and clarify v1 FundRef paths (#596) Co-authored-by: Cursor Agent --- rorapi/common/queries.py | 60 +++++++++++++++++-- .../tests_integration/tests_search_v2.py | 34 +++++++++++ rorapi/tests/tests_unit/tests_queries_v2.py | 38 +++++++++++- 3 files changed, 127 insertions(+), 5 deletions(-) diff --git a/rorapi/common/queries.py b/rorapi/common/queries.py index 1577c0e0..1302b226 100644 --- a/rorapi/common/queries.py +++ b/rorapi/common/queries.py @@ -58,6 +58,25 @@ # _exists_: check if field has non-null value, ex _exists_:wikipedia_url ALLOWED_ENDINGS = ("_exists_", "\\", "\\*") +# Keyword fields whose values often contain spaces (e.g. ISNI). Unquoted +# whitespace is split by Elasticsearch query_string (default_operator AND), +# so we auto-quote those values before building the query. +_EXTERNAL_ID_VALUE_PATTERN = re.compile( + r"(?Pexternal_ids\.(?:all|preferred)):" + r"(?P(?P\"(?:\\.|[^\"\\])*\")|" + r"(?P.*?))" + r"(?=(?:\s+(?:AND|OR|NOT)\b|\s*\)|$))", + re.IGNORECASE | re.DOTALL, +) +_LEGACY_EXTERNAL_ID_FIELD = re.compile( + r"^external_ids\.[^.]+\.(all|preferred)$", re.IGNORECASE +) +_V2_EXTERNAL_ID_HINT = ( + "for schema v2 use external_ids.type, external_ids.all, and/or " + "external_ids.preferred " + "(e.g. external_ids.type:fundref AND external_ids.all:100000908)" +) + def get_ror_id(string): """Extracts ROR id from a string and transforms it into canonical form""" @@ -81,6 +100,40 @@ def adv_query_string_to_list(query_string): return field_list +def quote_spaced_external_id_values(query_string): + """Quote unquoted external_ids.all / .preferred values that contain spaces. + + Elasticsearch query_string treats unquoted whitespace as term separators. + ISNI (and similar) IDs are stored as single keyword tokens with spaces, so + ``external_ids.all:0000 0001 2375 2908`` must become + ``external_ids.all:"0000 0001 2375 2908"``. Already-quoted values and + values without whitespace are left unchanged. + """ + if not isinstance(query_string, str) or not query_string: + return query_string + + def repl(match): + field = match.group("field") + if match.group("quoted") is not None: + return "{}:{}".format(field, match.group("quoted")) + value = match.group("unquoted") or "" + core = value.rstrip() + trailing_ws = value[len(core) :] + if not core or not any(c.isspace() for c in core): + return "{}:{}{}".format(field, core, trailing_ws) + return '{}:"{}"{}'.format(field, core, trailing_ws) + + return _EXTERNAL_ID_VALUE_PATTERN.sub(repl, query_string) + + +def illegal_field_error_message(field): + """Build the illegal-field error, with a v2 hint for legacy ID paths.""" + message = "string '{}' contains an illegal field name".format(field) + if _LEGACY_EXTERNAL_ID_FIELD.match(field): + message = "{}; {}".format(message, _V2_EXTERNAL_ID_HINT) + return message + + def check_status_adv_q(adv_q_string): status_in_q = False adv_query_fields = adv_query_string_to_list(adv_q_string) @@ -158,9 +211,7 @@ def validate(params): and not f.endswith(tuple(ALLOWED_ENDINGS)) ) ] - errors.extend( - ["string '{}' contains an illegal field name".format(f) for f in illegal_fields] - ) + errors.extend([illegal_field_error_message(f) for f in illegal_fields]) filters = filter_string_to_list(params.get("filter", "")) invalid_filters = [f for f in filters if ":" not in f] @@ -207,7 +258,8 @@ def build_search_query(params): del params["all_status"] if "query.advanced" in params: - qb.add_string_query_advanced(nfc(params.get("query.advanced"))) + advanced = quote_spaced_external_id_values(nfc(params.get("query.advanced"))) + qb.add_string_query_advanced(advanced) elif "query" in params: query = nfc(params.get("query")) ror_id = get_ror_id(query) diff --git a/rorapi/tests/tests_integration/tests_search_v2.py b/rorapi/tests/tests_integration/tests_search_v2.py index c8279de2..6650b8d7 100644 --- a/rorapi/tests/tests_integration/tests_search_v2.py +++ b/rorapi/tests/tests_integration/tests_search_v2.py @@ -211,3 +211,37 @@ def test_aggregation_keys_keep_original_casing(self): self.assertEqual(countries.get('us'), 'United States') continents = {b['id']: b['title'] for b in meta['continents']} self.assertEqual(continents.get('na'), 'North America') + + +class ExternalIdAdvancedSearchTestCase(SimpleTestCase): + """External ID advanced search (ror-roadmap#70, #71).""" + + def test_spaced_isni_without_quotes(self): + # query_string would otherwise split on spaces (roadmap#71) + unquoted = requests.get(BASE_URL, { + 'query.advanced': 'external_ids.all:0000 0001 2375 2908' + }).json() + quoted = requests.get(BASE_URL, { + 'query.advanced': 'external_ids.all:"0000 0001 2375 2908"' + }).json() + self.assertTrue(quoted['number_of_results'] > 0) + self.assertEqual(unquoted['number_of_results'], + quoted['number_of_results']) + self.assertEqual(unquoted['items'][0]['id'], + 'https://ror.org/019496w77') + + def test_fundref_via_v2_fields(self): + items = requests.get(BASE_URL, { + 'query.advanced': + 'external_ids.type:fundref AND external_ids.all:100000908' + }).json() + self.assertEqual(items['number_of_results'], 1) + self.assertEqual(items['items'][0]['id'], 'https://ror.org/02g8xhs57') + + def test_legacy_fundref_path_error_hint(self): + items = requests.get(BASE_URL, { + 'query.advanced': 'external_ids.FundRef.all:100000908' + }).json() + self.assertIn('errors', items) + self.assertTrue(any('external_ids.all' in e for e in items['errors'])) + self.assertTrue(any('schema v2' in e for e in items['errors'])) diff --git a/rorapi/tests/tests_unit/tests_queries_v2.py b/rorapi/tests/tests_unit/tests_queries_v2.py index 251b5424..b8d87ea1 100644 --- a/rorapi/tests/tests_unit/tests_queries_v2.py +++ b/rorapi/tests/tests_unit/tests_queries_v2.py @@ -4,7 +4,8 @@ from django.test import SimpleTestCase from rorapi.common.queries import get_ror_id, validate, build_search_query, \ - build_retrieve_query, search_organizations, retrieve_organization + build_retrieve_query, search_organizations, retrieve_organization, \ + quote_spaced_external_id_values from rorapi.settings import ES_VARS from .utils import IterableAttrDict @@ -66,6 +67,16 @@ def test_illegal_field(self): self.assertEqual(len(error.errors), 1) self.assertTrue(any(['illegal' in e for e in error.errors])) + def test_legacy_external_id_field_hint(self): + error = validate({ + 'query.advanced': 'external_ids.FundRef.all:100000908' + }) + self.assertEqual(len(error.errors), 1) + self.assertIn('illegal field name', error.errors[0]) + self.assertIn('external_ids.type', error.errors[0]) + self.assertIn('external_ids.all', error.errors[0]) + self.assertIn('schema v2', error.errors[0]) + def test_invalid_filter(self): error = validate({ @@ -284,6 +295,31 @@ def test_query_advanced(self): query = build_search_query({'query.advanced': 'query terms'}) self.assertEqual(query.to_dict(), expected) + def test_quote_spaced_external_id_values_helper(self): + self.assertEqual( + quote_spaced_external_id_values( + 'external_ids.all:0000 0001 2375 2908'), + 'external_ids.all:"0000 0001 2375 2908"') + self.assertEqual( + quote_spaced_external_id_values( + 'external_ids.all:"0000 0001 2375 2908"'), + 'external_ids.all:"0000 0001 2375 2908"') + self.assertEqual( + quote_spaced_external_id_values('external_ids.all:100000908'), + 'external_ids.all:100000908') + self.assertEqual( + quote_spaced_external_id_values( + 'external_ids.all:0000 0001 2375 2908 AND status:active'), + 'external_ids.all:"0000 0001 2375 2908" AND status:active') + + def test_query_advanced_quotes_spaced_external_ids(self): + query = build_search_query({ + 'query.advanced': 'external_ids.all:0000 0001 2375 2908' + }).to_dict() + self.assertEqual( + query['query']['bool']['must'][0]['query_string']['query'], + 'external_ids.all:"0000 0001 2375 2908"') + def test_query_advanced_nfc_normalization(self): nfd = 'locations.geonames_details.name:Hu\u0065\u0302\u0301' nfc_form = 'locations.geonames_details.name:Hu\u1ebf'