From 78e73ab63c482b35d41ad46244eee216f5fec013 Mon Sep 17 00:00:00 2001 From: Filipe Lopes Date: Sun, 6 Sep 2026 15:33:04 -0300 Subject: [PATCH 1/6] feat(graphql): add permission-aware source and concept projections --- core/__init__.py | 2 +- core/common/permissions.py | 24 +- core/common/search.py | 80 +++ core/common/views.py | 4 +- core/concepts/documents.py | 17 + core/graphql/README.md | 119 ++++ core/graphql/constants.py | 56 ++ core/graphql/extensions.py | 21 + core/graphql/indexed.py | 119 ++++ core/graphql/permissions.py | 142 ++++ core/graphql/queries.py | 625 ++++++++---------- core/graphql/schema.py | 17 +- core/graphql/search.py | 157 +++++ core/graphql/selection.py | 42 ++ core/graphql/serializers.py | 249 +++++++ core/graphql/sources.py | 68 ++ .../tests/test_concepts_from_source.py | 15 +- core/graphql/tests/test_graphql_view.py | 5 +- core/graphql/tests/test_projection.py | 182 +++++ core/graphql/tests/test_query_helpers.py | 492 +++++++++++++- core/graphql/tests/test_sources.py | 204 ++++++ core/graphql/types.py | 50 +- .../test_graphql_projection.py | 156 +++++ core/sources/documents.py | 5 + core/sources/signals.py | 11 +- 25 files changed, 2456 insertions(+), 406 deletions(-) create mode 100644 core/graphql/README.md create mode 100644 core/graphql/constants.py create mode 100644 core/graphql/extensions.py create mode 100644 core/graphql/indexed.py create mode 100644 core/graphql/permissions.py create mode 100644 core/graphql/search.py create mode 100644 core/graphql/selection.py create mode 100644 core/graphql/serializers.py create mode 100644 core/graphql/sources.py create mode 100644 core/graphql/tests/test_projection.py create mode 100644 core/graphql/tests/test_sources.py create mode 100644 core/integration_tests/test_graphql_projection.py diff --git a/core/__init__.py b/core/__init__.py index 5a22943f2..c6b40bcbb 100644 --- a/core/__init__.py +++ b/core/__init__.py @@ -4,7 +4,7 @@ __all__ = ('celery_app',) -API_VERSION = '2.3.201' +API_VERSION = '2.3.202' API_BUILD = 'dev' VERSION = API_VERSION + '-' + API_BUILD __version__ = VERSION diff --git a/core/common/permissions.py b/core/common/permissions.py index 182de1f34..c62f38cb6 100644 --- a/core/common/permissions.py +++ b/core/common/permissions.py @@ -42,16 +42,30 @@ def has_object_permission(self, request, view, obj): return False -class CanViewConceptDictionary(HasPrivateAccess): +def user_can_view_concept_dictionary(user, obj) -> bool: + """Share repository visibility without mixing user, organization and repository primary keys.""" + if obj.public_access in [ACCESS_TYPE_EDIT, ACCESS_TYPE_VIEW]: + return True + if user.is_staff: + return True + if user.is_authenticated: + if getattr(obj, 'user_id', None) == user.id: + return True + organization_id = getattr(obj, 'organization_id', None) + if getattr(obj, 'resource_type', None) == 'Organization': + organization_id = obj.id + if organization_id and user.organizations.filter(id=organization_id).exists(): + return True + return False + + +class CanViewConceptDictionary(BasePermission): """ The user can view this source """ def has_object_permission(self, request, view, obj): - if obj.public_access in [ACCESS_TYPE_EDIT, ACCESS_TYPE_VIEW]: - return True - - return super().has_object_permission(request, view, obj) + return user_can_view_concept_dictionary(request.user, obj) class CanEditConceptDictionary(HasPrivateAccess): diff --git a/core/common/search.py b/core/common/search.py index 8a7fea2ee..8e3c91a1a 100644 --- a/core/common/search.py +++ b/core/common/search.py @@ -15,6 +15,86 @@ from core.common.constants import ES_REQUEST_TIMEOUT from core.common.utils import is_url_encoded_string +from core.orgs.constants import ORG_OBJECT_TYPE +from core.users.constants import USER_OBJECT_TYPE + + +def get_document_public_visibility_criteria( # pylint: disable=too-many-arguments + user, + include_creator_private_access=False, + include_owner_private_access=False, + include_organization_memberships=False, + public_field='public_can_view', +): + """Return a shared Elasticsearch visibility criterion for owner-scoped documents. + + The base criterion is always ``public_can_view=True``. Anonymous users get only that. + Authenticated users may additionally see private documents matched by the OR of the + enabled flags below — each flag widens visibility in a specific way: + + - ``include_creator_private_access``: include private docs where ``created_by`` equals + the current user's username. Mirrors the historical REST concept/source-child rule + (a creator always sees their own private content). Used by REST list endpoints. + + - ``include_owner_private_access``: include private docs owned by the user itself + (``owner_type=USER`` and ``owner=username``). Used by GraphQL to mirror how list APIs + expose a user's own private repositories. + + - ``include_organization_memberships``: include private docs owned by any organization + the user belongs to (``owner_type=ORG`` and ``owner IN user.orgs``). Used by GraphQL + so organization members see private repos belonging to their orgs. + + Flags are independent OR-combined extensions. Staff bypass goes through + ``apply_document_public_visibility_filter`` (this helper itself does not check staff). + """ + criteria = Q('term', **{public_field: True}) + if not getattr(user, 'is_authenticated', False): + return criteria + + private_criteria = None + username = getattr(user, 'username', None) + if username and include_creator_private_access: + private_criteria = Q('term', created_by=username) + + if username and include_owner_private_access: + owner_criteria = Q('term', owner_type=USER_OBJECT_TYPE) & Q('term', owner=username.lower()) + private_criteria = owner_criteria if private_criteria is None else private_criteria | owner_criteria + + if include_organization_memberships: + organization_mnemonics = [ + mnemonic.lower() for mnemonic in user.organizations.values_list('mnemonic', flat=True) + ] + if organization_mnemonics: + org_criteria = Q('term', owner_type=ORG_OBJECT_TYPE) & Q('terms', owner=organization_mnemonics) + private_criteria = org_criteria if private_criteria is None else private_criteria | org_criteria + + if private_criteria is None: + return criteria + + return criteria | (Q('term', **{public_field: False}) & private_criteria) + + +def apply_document_public_visibility_filter( # pylint: disable=too-many-arguments + search, + user, + include_creator_private_access=False, + include_owner_private_access=False, + include_organization_memberships=False, + public_field='public_can_view', +): + """Apply a shared Elasticsearch visibility filter without changing staff searches.""" + if getattr(user, 'is_staff', False): + return search + + return search.filter( + get_document_public_visibility_criteria( + user, + include_creator_private_access=include_creator_private_access, + include_owner_private_access=include_owner_private_access, + include_organization_memberships=include_organization_memberships, + public_field=public_field, + ) + ) class CustomESFacetedSearch(FacetedSearch): diff --git a/core/common/views.py b/core/common/views.py index 69b60eb1c..ef5a633b0 100644 --- a/core/common/views.py +++ b/core/common/views.py @@ -29,7 +29,7 @@ CANONICAL_URL_REQUEST_PARAM, CHECKSUMS_PARAM, ACCESS_TYPE_NONE from core.common.exceptions import Http400 from core.common.mixins import PathWalkerMixin -from core.common.search import CustomESSearch +from core.common.search import CustomESSearch, get_document_public_visibility_criteria from core.common.serializers import RootSerializer from core.common.swagger_parameters import all_resource_query_param from core.common.throttling import ThrottleUtil @@ -704,7 +704,7 @@ def get_public_criteria(self): if self.document_model in [OrganizationDocument]: criteria |= (Q('term', public_can_view=False) & Q('term', user=username)) if self.is_concept_container_document_model() or self.is_source_child_document_model(): - criteria |= (Q('term', public_can_view=False) & Q('term', created_by=username)) + return get_document_public_visibility_criteria(user, include_creator_private_access=True) return criteria diff --git a/core/concepts/documents.py b/core/concepts/documents.py index f4fc4d7df..e03ed07b7 100644 --- a/core/concepts/documents.py +++ b/core/concepts/documents.py @@ -12,6 +12,12 @@ class Index: name = 'concepts' settings = {'number_of_shards': 1, 'number_of_replicas': 0} + # Preserve ORM semantics for direct GraphQL projections without changing REST search fields. + is_active = fields.BooleanField(attr='is_active') + parent_public_can_view = fields.BooleanField(attr='parent.public_can_view') + is_head = fields.BooleanField() + preferred_description = fields.TextField() + id = fields.TextField(attr='mnemonic') id_lowercase = fields.KeywordField(attr='mnemonic', normalizer="lowercase") id_raw = fields.KeywordField(attr='mnemonic') @@ -267,3 +273,14 @@ def get_mapped_codes(instance): else: other_mapped_codes.append(to_concept_code) return same_as_mapped_codes, other_mapped_codes, verbose_info + + @staticmethod + def prepare_is_head(instance): + """Match the versioned-object predicate used by Source.get_concepts_queryset.""" + return instance.id == instance.versioned_object_id + + @staticmethod + def prepare_preferred_description(instance): + """Store the same locale-selected description returned by GraphQL's ORM path.""" + from core.graphql.serializers import resolve_description + return resolve_description(instance) diff --git a/core/graphql/README.md b/core/graphql/README.md new file mode 100644 index 000000000..dfed162cc --- /dev/null +++ b/core/graphql/README.md @@ -0,0 +1,119 @@ +# GraphQL concepts and source MVP + +This work carries the permission architecture reviewed in [PR #838](https://github.com/OpenConceptLab/oclapi2/pull/838) +onto master `ab03e1c0`, and adds source metadata and selection-driven retrieval. The API version is `2.3.202-dev`. +Master already contains the [PR #877](https://github.com/OpenConceptLab/oclapi2/pull/877) Strawberry bump: +`strawberry-graphql==0.315.7`, with `strawberry-graphql-django==0.80.0`. These versions were retained. + +## Queries + +Open `/graphql/` for GraphiQL. Query arguments, returned fields, and summary fields include schema descriptions. +Use the existing OCL token, OIDC bearer token, or session authentication. Invalid credentials are rejected before +resolvers run; authenticated users still require the existing `graphql_api` group. Anonymous queries see public data. + +```graphql +query Dictionary($org: String!, $source: String!, $version: String) { + source(org: $org, source: $source, version: $version) { + name + description + canonicalUrl + uri + classes + datatypes + mapTypes + externalSources { name url } + summary { activeConcepts mappings } + } +} +``` + +Variables: `{"org":"CIEL","source":"CIEL"}`. For personal repositories, replace `org` with `owner` (username). +`uri` is the stored OCL relative URI, for example `/orgs/CIEL/sources/CIEL/`; releases include their version. +The canonical field is spelled `canonicalUrl`. + +```graphql +query FindConcepts($org: String, $source: String, $query: String!, $page: Int, $limit: Int) { + concepts(org: $org, source: $source, query: $query, page: $page, limit: $limit) { + totalCount + hasNextPage + versionResolved + results { conceptId display description conceptClass datatype { name } } + } +} +``` + +Variables: `{"org":"CIEL","source":"CIEL","query":"hypertension","page":1,"limit":20}`. +Omit both `org` and `source` for global search. `conceptIds` performs exact, case-sensitive mnemonic matching, +deduplicates the input, and preserves its order; it takes precedence over `query`. Supply `page` and `limit` +together; the supported result window is 10,000. Without pagination, index responses are capped at 10,000; +`totalCount` remains the total number of matches. Omitted versions use HEAD, falling back to the latest released +version only if HEAD is absent; explicit missing versions do not fall back. + +## Data access and permissions + +| Selected payload | Retrieval | +| --- | --- | +| Source `name`, `description`, `canonicalUrl`, `uri` | Source index projection, including source/version resolution | +| Concept `id`, `conceptId`, `externalId`, `display`, `description`, `conceptClass`, `datatype { name }` | Concept index projection; no ORM concept hydration | +| Only concept counts/pagination metadata | Elasticsearch request with zero result hits | +| Concept names, mappings, extras, audit metadata, datatype details | ORM hydration with selected concept columns and relations | +| Source classes, datatypes, map types, external sources, summary | Existing version-scoped model querysets; only selected aggregates execute | + +Aliases, fragments, `@skip`, `@include`, and nested `__typename` selections participate in planning. +Elasticsearch `_source` is restricted to selected fields. An empty successful direct projection is authoritative; +it does not trigger a database scan. Expected index/transport failures fall back to permission-checked ORM queries. +The older hydrated text-search path retains its empty-index database fallback. + +Counts and distinct labels use active, non-retired records. `summary.mappings` counts active, non-retired mappings. +`externalSources` is the deduplicated set of outbound target repositories, excluding the current source and linked +private targets the caller cannot view. Unresolved external URLs are taken from visible mappings. + +Repository permission checks reuse the shared REST visibility rule directly, without fabricated requests. +Both owner mnemonic and owner type scope index lookups. Global concepts also enforce parent repository visibility, +and mapping hydration independently checks target visibility. HEAD uses the same versioned-object identity as +`Source.get_concepts_queryset()`, while releases use their membership lists. + +SQL-free data retrieval does not mean SQL-free authentication: session/token lookup and organization membership +resolution can query the database. Tests verify zero SQL for anonymous public index projections. As with the +existing REST index, indexed results reflect Elasticsearch refresh and indexing propagation latency. + +## Rollout + +No database migrations or new environment variables are introduced. Refresh the source and concept indexes +before serving this GraphQL version: older concept documents lack the HEAD, activity, parent-permission and +preferred-description projection fields. Source documents add description, URI and activity fields. +Do not use incomplete indexes during the rollout; global projections filter on the new fields. + +Use the existing indexing procedure to apply the additive mappings and repopulate both models. For a deployment +that recreates indexes, use its established rebuild procedure; do not rebuild live indexes without accounting for +REST search availability. A full population command for the existing application container is: + +```sh +docker exec oclapi2-api-1 python manage.py search_index --populate --models sources.Source concepts.Concept -f --parallel +``` + +Source permission/activity propagation also refreshes the corresponding concept projection flags. Existing +REST search relevance and excluded-word semantics are preserved; unrelated search refactors from PR #838 were +not carried over. Its corrected permission sharing and documented Strawberry auth extension were retained. + +## Verification + +```sh +docker exec oclapi2-api-1 python manage.py test core.graphql.tests --keepdb --noinput -v2 +docker exec oclapi2-api-1 pylint -j2 core/graphql core/common/permissions.py core/common/search.py core/common/views.py core/sources/signals.py core/integration_tests/test_graphql_projection.py +``` + +`core.integration_tests.test_graphql_projection` requires `settings.ES_ENABLED=True`. It creates uniquely named +indexes and removes them after each test. Run it only against a test Elasticsearch service: shared fixture setup +can also exercise normal indexing hooks. It covers real index preparation, zero SQL, owner isolation, HEAD/release +selection, inactive/retired filtering, and private-parent visibility. + +For this worktree, verification used a copy at `/tmp/graphql-sources-20260906` inside the existing API container, +the dedicated database `test_graphql_sources_20260906`, and a temporary Elasticsearch container. The running app's +`/code` checkout and search indexes were not changed. Coverage uses a temporary runner that selects Python's YAML +loader because the container's C YAML loader fails under coverage instrumentation; application dependencies were +not modified to work around that test-environment issue. + +Verified results: **75 distinct tests passed**, including six tests against real Elasticsearch and the focused +REST/source-signal regressions. Coverage of `core.graphql` (excluding test files) is **98%**: 739 of 753 statements. +Pylint completed without findings. No changes were made to the application's installed dependency versions. diff --git a/core/graphql/constants.py b/core/graphql/constants.py new file mode 100644 index 000000000..2f000c988 --- /dev/null +++ b/core/graphql/constants.py @@ -0,0 +1,56 @@ +"""Shared GraphQL error metadata used by views, resolvers, and tests.""" + +from typing import Optional + +from strawberry.exceptions import GraphQLError + +AUTHENTICATION_FAILED = 'AUTHENTICATION_FAILED' +FORBIDDEN = 'FORBIDDEN' +SEARCH_UNAVAILABLE = 'SEARCH_UNAVAILABLE' +VALIDATION_ERROR = 'VALIDATION_ERROR' + +GRAPHQL_ERROR_DEFINITIONS = { + AUTHENTICATION_FAILED: { + 'message': 'Authentication failure', + 'description': 'The provided credentials are invalid for the GraphQL API.', + }, + FORBIDDEN: { + 'message': 'Forbidden', + 'description': 'The current user cannot access the requested repository.', + }, + SEARCH_UNAVAILABLE: { + 'message': 'Search unavailable', + 'description': 'Global concept search requires Elasticsearch and is temporarily unavailable.', + }, + VALIDATION_ERROR: { + 'message': 'Validation error', + 'description': 'Client supplied arguments that violate input validation rules.', + }, +} +EXPECTED_GRAPHQL_ERROR_CODES = frozenset(GRAPHQL_ERROR_DEFINITIONS.keys()) + + +def build_expected_graphql_error(code, message: Optional[str] = None): + """Return a GraphQL error with a stable code and a short client-facing description. + + Pass ``message`` to override the default human-readable message while preserving + the machine-readable ``code``. + """ + detail = GRAPHQL_ERROR_DEFINITIONS[code] + return GraphQLError( + message or detail['message'], + extensions={ + 'code': code, + 'description': detail['description'], + }, + ) + + +def build_validation_error(message: str): + """Shortcut for client-side validation failures that should not be logged as server errors.""" + return build_expected_graphql_error(VALIDATION_ERROR, message=message) + + +def get_graphql_error_code(error): + """Read the machine-readable error code attached to a GraphQL error when present.""" + return (getattr(error, 'extensions', None) or {}).get('code') diff --git a/core/graphql/extensions.py b/core/graphql/extensions.py new file mode 100644 index 000000000..a9e13dd4a --- /dev/null +++ b/core/graphql/extensions.py @@ -0,0 +1,21 @@ +"""Strawberry schema extensions used to enforce cross-cutting GraphQL policies.""" + +from typing import Iterator + +from graphql import ExecutionResult +from strawberry.extensions import SchemaExtension + +from .constants import AUTHENTICATION_FAILED, build_expected_graphql_error + + +class AuthStatusExtension(SchemaExtension): + """Reject requests with invalid credentials before any resolver runs.""" + + def on_execute(self) -> Iterator[None]: + context = self.execution_context.context + if getattr(context, 'auth_status', 'none') == 'invalid': + self.execution_context.result = ExecutionResult( + data=None, + errors=[build_expected_graphql_error(AUTHENTICATION_FAILED)], + ) + yield diff --git a/core/graphql/indexed.py b/core/graphql/indexed.py new file mode 100644 index 000000000..6b2cf3e92 --- /dev/null +++ b/core/graphql/indexed.py @@ -0,0 +1,119 @@ +"""Elasticsearch projections for payloads fully represented in the search index.""" + +import logging +from types import SimpleNamespace + +from elasticsearch import ApiError, ConnectionError as ESConnectionError, TransportError +from elasticsearch_dsl import Q + +from core.common.constants import HEAD +from core.concepts.documents import ConceptDocument +from core.sources.documents import SourceDocument + +from .permissions import apply_es_parent_visibility_filter, apply_es_visibility_filter +from .selection import index_projection +from .sources import SOURCE_INDEX_FIELDS +from .types import ConceptType, DatatypeType, SourceType + +logger = logging.getLogger(__name__) +CONCEPT_INDEX_FIELDS = { + '__typename': (), + 'datatype.__typename': ('datatype',), + 'id': (), # Elasticsearch's metadata ID is the OCL database primary key. + 'conceptId': ('id',), + 'externalId': ('external_id',), + 'display': ('name',), + 'description': ('preferred_description',), + 'conceptClass': ('concept_class',), + 'datatype.name': ('datatype',), +} + + +def search_text(search, query): + """Keep the existing GraphQL relevance clauses shared by both retrieval paths.""" + return search.query(Q('bool', should=[ + Q('match', id={'query': query, 'boost': 6, 'operator': 'AND'}), + Q('match_phrase_prefix', name={'query': query, 'boost': 4}), + Q('match', synonyms={'query': query, 'boost': 2, 'operator': 'AND'}), + ], minimum_should_match=1)) + + +def indexed_source(org, owner, source, version, user, paths): # pylint: disable=too-many-arguments + """Resolve a visible source from ES; missing/old indexes use the authorized ORM fallback.""" + from .permissions import resolve_owner + owner_value, owner_type = resolve_owner(org, owner) + fields = index_projection(paths, SOURCE_INDEX_FIELDS) + if fields is None: + return None + search = SourceDocument.search().filter('term', _mnemonic=source.lower()) + search = search.filter('term', owner=owner_value.lower()).filter('term', owner_type=owner_type) + search = search.filter('term', version=version or HEAD) + search = apply_es_visibility_filter(search, user) + search = search.source(sorted(set(fields) | {'is_active', 'version', 'mnemonic'}))[:1] + try: + hits = list(search.execute()) + except (ApiError, TransportError, ESConnectionError) as exc: + logger.warning('Source projection unavailable; using database: %s', exc) + return None + if not hits or not getattr(hits[0], 'is_active', False): + return None + hit = hits[0] + return SimpleNamespace( + mnemonic=hit.mnemonic, version=hit.version, is_head=hit.version == HEAD, + payload=SourceType(**{field: getattr(hit, field, None) for field in fields}), + ) + + +# The planner supplies request scope and payload independently. +# pylint: disable-next=too-many-arguments,too-many-locals +def indexed_concepts(paths, query, concept_ids, scope, pagination, owner, owner_type, user): + """Return selected index fields directly, without fetching or serializing ORM concepts.""" + fields = index_projection(paths, CONCEPT_INDEX_FIELDS) + if fields is None: + return None + search = ConceptDocument.search().filter('term', is_active=True).filter('term', retired=False) + if scope: + search = search.filter('term', source=scope.mnemonic.lower()) + search = search.filter('term', owner=owner.lower()).filter('term', owner_type=owner_type) + search = search.filter('term', **({'is_head': True} if scope.is_head else {'source_version': scope.version})) + else: + search = apply_es_visibility_filter(search.filter('term', is_head=True), user) + search = apply_es_parent_visibility_filter(search, user) + if concept_ids: + # Script-free ordering preserves the requested mnemonic order, with deterministic ties. + search = search.filter('terms', id_raw=concept_ids).sort('id_raw') + else: + search = search_text(search, query).sort({'_score': 'desc'}, 'id_raw') + start, end = (pagination['start'], pagination['end']) if pagination else (0, 10_000) + # ID lists need ordering before slicing. Their size is validated at the API boundary. + if not paths: + search = search[:0] + else: + search = search[0:10_000] if concept_ids else search[start:end] + search = search.source(sorted(set(fields) | ({'id'} if concept_ids else set()))) + try: + response = search.params(track_total_hits=True).execute() + except (ApiError, TransportError, ESConnectionError) as exc: + logger.warning('Concept projection unavailable; using database: %s', exc) + return None + hits = list(response) + total = response.hits.total.value + if concept_ids: + if total > 10_000: + return None # Preserve complete mnemonic ordering through the ORM. + ordering = {value: index for index, value in enumerate(concept_ids)} + hits.sort(key=lambda hit: (ordering[hit.id], int(hit.meta.id))) + hits = hits[start:end] + return [serialize_indexed_concept(hit) for hit in hits], total + + +def serialize_indexed_concept(hit): + """Construct only index-backed values; unselected relationship fields stay unloaded.""" + datatype = getattr(hit, 'datatype', None) + return ConceptType( + id=str(hit.meta.id), concept_id=getattr(hit, 'id', ''), + external_id=getattr(hit, 'external_id', None), display=getattr(hit, 'name', None) or None, + description=getattr(hit, 'preferred_description', None), concept_class=getattr(hit, 'concept_class', None), + datatype=DatatypeType(name=datatype, details=None) if datatype else None, + names=[], mappings=[], metadata=None, extras={}, + ) diff --git a/core/graphql/permissions.py b/core/graphql/permissions.py new file mode 100644 index 000000000..7d87618e9 --- /dev/null +++ b/core/graphql/permissions.py @@ -0,0 +1,142 @@ +"""Reusable permission helpers for GraphQL resolvers.""" + +from __future__ import annotations + +from typing import Optional, Tuple + +from asgiref.sync import sync_to_async +from django.db.models import Q + +from core.common.constants import ACCESS_TYPE_NONE +from core.common.permissions import user_can_view_concept_dictionary +from core.common.search import apply_document_public_visibility_filter +from core.orgs.constants import ORG_OBJECT_TYPE +from core.users.constants import USER_OBJECT_TYPE + +from .constants import FORBIDDEN, build_expected_graphql_error + +SOURCE_VERSION_CACHE_ATTR = '_graphql_source_version_cache' + + +def resolve_owner(org: Optional[str], owner: Optional[str]) -> Tuple[Optional[str], Optional[str]]: + """Collapse ``(org, owner)`` into ``(value, type)`` shared by ES filters and ownership routing. + + ``org`` takes precedence: when both are provided (callers are expected to validate this + upstream), the org form wins. Returns ``(None, None)`` when neither is supplied so callers + can short-circuit global searches. + """ + if org: + return org, ORG_OBJECT_TYPE + if owner: + return owner, USER_OBJECT_TYPE + return None, None + + +async def ensure_can_view_repo(user, source_version) -> None: + """Raise a GraphQL forbidden error when the repository is not visible to the user.""" + allowed = await sync_to_async( + user_can_view_concept_dictionary, + thread_sensitive=True, + )(user, source_version) + + if not allowed: + raise build_expected_graphql_error(FORBIDDEN) + + +def filter_global_queryset(qs, user): + """Apply the global visibility rules used by the REST concept and mapping list endpoints. + + Behaviour table: + + | User | Filter applied | + |-----------------------------------|-----------------------------------------------| + | Anonymous | ``exclude(public_access=ACCESS_TYPE_NONE)`` | + | Authenticated non-staff | ``model.apply_user_criteria`` when available, | + | | otherwise fail-closed to anonymous filter | + | Staff / superuser | No filter (full visibility) | + + The fail-closed branch matters: if a model is wired into global GraphQL queries without + implementing ``apply_user_criteria``, we still hide private rows instead of leaking them. + """ + if getattr(user, 'is_anonymous', True): + return qs.exclude(public_access=ACCESS_TYPE_NONE) + if getattr(user, 'is_staff', False): + return qs + apply_user_criteria = getattr(qs.model, 'apply_user_criteria', None) + if apply_user_criteria: + return apply_user_criteria(qs, user) + # Fail-closed: a model that does not implement apply_user_criteria must not expose private rows. + return qs.exclude(public_access=ACCESS_TYPE_NONE) + + +def filter_parent_queryset(queryset, user, prefix='parent'): + """Restrict parent/target repository access independently of a child's own access flag.""" + if getattr(user, 'is_staff', False): + return queryset + criterion = Q(**{prefix + '__isnull': True}) | ~Q(**{prefix + '__public_access': ACCESS_TYPE_NONE}) + if getattr(user, 'is_authenticated', False): + criterion |= Q(**{prefix + '__user_id': user.id}) | Q(**{prefix + '__organization__members__id': user.id}) + return queryset.filter(criterion).distinct() + + +def apply_es_visibility_filter(search, user): + """Mirror REST visibility rules in Elasticsearch so totals stay aligned with the DB.""" + return apply_document_public_visibility_filter( + search, + user, + include_owner_private_access=True, + include_organization_memberships=True, + ) + + +def apply_es_parent_visibility_filter(search, user): + """Also protect private parent repositories when a child has a public access flag.""" + return apply_document_public_visibility_filter( + search, user, include_owner_private_access=True, include_organization_memberships=True, + public_field='parent_public_can_view', + ) + + +class PermissionsMixin: + """Provide cached source resolution and shared permission helpers to resolvers.""" + + async def resolve_source_version_for_permissions( + self, + org: Optional[str], + owner: Optional[str], + source: str, + version: Optional[str], + ): + """Allow GraphQL query types to plug in their own source-version resolver.""" + raise NotImplementedError + + async def get_source_version( # pylint: disable=too-many-arguments + self, + info, + org: Optional[str], + owner: Optional[str], + source: str, + version: Optional[str], + ): + """Resolve and cache the source version for the current GraphQL request.""" + cache = getattr(info.context, SOURCE_VERSION_CACHE_ATTR, None) or {} + cache_key = (org, owner, source, version) + if cache_key in cache: + return cache[cache_key] + + source_version = await self.resolve_source_version_for_permissions(org, owner, source, version) + cache[cache_key] = source_version + setattr(info.context, SOURCE_VERSION_CACHE_ATTR, cache) + return source_version + + async def ensure_can_view_repo(self, user, source_version) -> None: + """Delegate repository permission checks to the shared helper.""" + await ensure_can_view_repo(user, source_version) + + def filter_global_queryset(self, qs, user): + """Delegate global queryset visibility rules to the shared helper.""" + return filter_global_queryset(qs, user) + + def apply_es_visibility_filter(self, search, user): + """Delegate Elasticsearch visibility rules to the shared helper.""" + return apply_es_visibility_filter(search, user) diff --git a/core/graphql/queries.py b/core/graphql/queries.py index e96b6ac7e..b7e25dbb7 100644 --- a/core/graphql/queries.py +++ b/core/graphql/queries.py @@ -1,40 +1,109 @@ +"""GraphQL resolvers (Strawberry). + +Pure helpers live in ``core/graphql/search.py`` (queryset builders, pagination, DB fallback) +and ``core/graphql/serializers.py`` (ORM → Strawberry mapping). This module hosts: + +* the Elasticsearch boundary (``concept_ids_from_es`` + orchestrator ``concepts_for_query``), +* the ``Query`` Strawberry type and its resolvers, +* permissions/validation orchestration, +* and back-compat re-exports of helpers so existing imports continue to work. +""" + from __future__ import annotations -from datetime import timezone as datetime_timezone import logging -from typing import Iterable, List, Optional, Sequence +from typing import Annotated, List, Optional import strawberry from asgiref.sync import sync_to_async -from django.db.models import Case, IntegerField, Prefetch, Q, When -from django.utils import timezone +from django.contrib.auth.models import AnonymousUser +from django.db.models import Case, IntegerField, Prefetch, When from elasticsearch import ConnectionError as ESConnectionError, TransportError -from elasticsearch_dsl import Q as ES_Q from pydash import get from strawberry.exceptions import GraphQLError from core.common.constants import HEAD from core.concepts.documents import ConceptDocument from core.concepts.models import Concept -from core.mappings.models import Mapping from core.sources.models import Source -from .types import ( - CodedDatatypeDetails, - ConceptNameType, - ConceptType, - DatatypeDetails, - DatatypeType, - MappingType, - MetadataType, - NumericDatatypeDetails, - TextDatatypeDetails, - ToSourceType, +from .constants import build_validation_error +from .permissions import ( + PermissionsMixin, + apply_es_visibility_filter, + apply_es_parent_visibility_filter, + resolve_owner, + filter_parent_queryset, ) +from .search import ( + apply_slice, + build_db_search_queryset, + build_global_head_queryset, + build_global_mapping_prefetch, + build_mapping_prefetch, + build_source_version_queryset, + concepts_for_ids, + has_next, + normalize_pagination, + with_concept_related, +) +from .serializers import ( + _to_bool, + _to_float, + build_datatype, + build_metadata, + format_datetime_for_api, + resolve_coded_datatype_details, + resolve_datatype_details, + resolve_description, + resolve_is_set_flag, + resolve_numeric_datatype_details, + resolve_text_datatype_details, + serialize_concepts, + serialize_mappings, + serialize_names, +) +from .types import ConceptType, SourceType +from .selection import child_paths, selected_paths, index_projection +from .sources import serialize_source +from .indexed import CONCEPT_INDEX_FIELDS, indexed_source, indexed_concepts, search_text logger = logging.getLogger(__name__) ES_MAX_WINDOW = 10_000 +# Back-compat re-exports for tests and callers that import these names from this module. +__all__ = [ + 'ConceptSearchResult', + 'Query', + 'resolve_source_version', + 'concept_ids_from_es', + 'concepts_for_query', + '_to_bool', + '_to_float', + 'apply_slice', + 'build_db_search_queryset', + 'build_datatype', + 'build_global_head_queryset', + 'build_global_mapping_prefetch', + 'build_mapping_prefetch', + 'build_metadata', + 'build_source_version_queryset', + 'concepts_for_ids', + 'format_datetime_for_api', + 'has_next', + 'normalize_pagination', + 'resolve_coded_datatype_details', + 'resolve_datatype_details', + 'resolve_description', + 'resolve_is_set_flag', + 'resolve_numeric_datatype_details', + 'resolve_text_datatype_details', + 'serialize_concepts', + 'serialize_mappings', + 'serialize_names', + 'with_concept_related', +] + @strawberry.type class ConceptSearchResult: @@ -46,7 +115,7 @@ class ConceptSearchResult: ) version_resolved: str = strawberry.field( name="versionResolved", - description="Exact source version used (HEAD resolves to its concrete version).", + description="Exact source version label used; HEAD remains HEAD.", ) page: Optional[int] = strawberry.field( description="Requested page (1-indexed) if pagination parameters were supplied." @@ -67,8 +136,19 @@ class ConceptSearchResult: ) -async def resolve_source_version(org: str, source: str, version: Optional[str]) -> Source: - filters = {'organization__mnemonic': org} +async def resolve_source_version( + org: Optional[str], + owner: Optional[str], + source: str, + version: Optional[str], +) -> Source: + if org: + filters = {'organization__mnemonic': org} + elif owner: + filters = {'user__username': owner} + else: + raise build_validation_error("Either org or owner must be provided to resolve a source version.") + filters['is_active'] = True target_version = version or HEAD instance = await sync_to_async(Source.get_version)(source, target_version, filters) @@ -76,284 +156,19 @@ async def resolve_source_version(org: str, source: str, version: Optional[str]) instance = await sync_to_async(Source.find_latest_released_version_by)({**filters, 'mnemonic': source}) if not instance: - raise GraphQLError( - f"Source '{source}' with version '{version or 'HEAD'}' was not found for org '{org}'." - ) + # Generic message: do not leak whether the owner exists when the source is missing. + raise GraphQLError(f"Source '{source}' with version '{version or 'HEAD'}' was not found.") return instance -def build_base_queryset(source_version: Source): - return source_version.get_concepts_queryset().filter(is_active=True, retired=False) - - -def build_mapping_prefetch(source_version: Source) -> Prefetch: - mapping_qs = ( - Mapping.objects.filter( - sources__id=source_version.id, - from_concept_id__isnull=False, - is_active=True, - retired=False, - ) - .select_related('to_source', 'to_concept', 'to_concept__parent') - .order_by('map_type', 'to_concept_code', 'to_concept__mnemonic') - .distinct() - ) - - return Prefetch('mappings_from', queryset=mapping_qs, to_attr='graphql_mappings') - - -def build_global_mapping_prefetch() -> Prefetch: - mapping_qs = ( - Mapping.objects.filter( - from_concept_id__isnull=False, - is_active=True, - retired=False, - ) - .select_related('to_source', 'to_concept', 'to_concept__parent') - .order_by('map_type', 'to_concept_code', 'to_concept__mnemonic') - .distinct() - ) - - return Prefetch('mappings_from', queryset=mapping_qs, to_attr='graphql_mappings') - - -def normalize_pagination(page: Optional[int], limit: Optional[int]) -> Optional[dict]: - if page is None or limit is None: - return None - if page < 1 or limit < 1: - raise GraphQLError('page and limit must be >= 1 when provided.') - start = (page - 1) * limit - end = start + limit - return {'page': page, 'limit': limit, 'start': start, 'end': end} - - -def has_next(total: int, pagination: Optional[dict]) -> bool: - if not pagination: - return False - return total > pagination['end'] - - -def apply_slice(qs, pagination: Optional[dict]): - if not pagination: - return qs - return qs[pagination['start']:pagination['end']] - - -def with_concept_related(qs, mapping_prefetch: Prefetch): - return qs.select_related('created_by', 'updated_by').prefetch_related('names', 'descriptions', mapping_prefetch) - - -def serialize_mappings(concept: Concept) -> List[MappingType]: - mappings = getattr(concept, 'graphql_mappings', []) or [] - result: List[MappingType] = [] - for mapping in mappings: - result.append( - MappingType( - map_type=str(mapping.map_type), - to_source=ToSourceType( - url=mapping.to_source_url, - name=mapping.to_source_name - ) if mapping.to_source_url or mapping.to_source_name else None, - to_code=mapping.get_to_concept_code(), - comment=mapping.comment, - ) - ) - return result - - -def serialize_names(concept: Concept) -> List[ConceptNameType]: - return [ - ConceptNameType( - name=name.name, - locale=name.locale, - type=name.type, - preferred=name.locale_preferred, - retired=name.retired, - ) - for name in concept.names.all() - ] - - -def resolve_description(concept: Concept) -> Optional[str]: - descriptions = list(concept.active_descriptions.all()) - if not descriptions: - return None - - def pick(predicate): - for desc in descriptions: - if predicate(desc): - return desc.description - return None - - try: - default_locale = getattr(concept.parent, 'default_locale', None) - except Source.DoesNotExist: - default_locale = None - if default_locale: - match = pick(lambda desc: desc.locale == default_locale and desc.locale_preferred) - if match: - return match - match = pick(lambda desc: desc.locale == default_locale) - if match: - return match - - match = pick(lambda desc: desc.locale_preferred) - if match: - return match - return descriptions[0].description - - -def resolve_is_set_flag(concept: Concept) -> Optional[bool]: - value = getattr(concept, 'is_set', None) - if value is None: - extras = concept.extras or {} - if 'is_set' not in extras: - return None - value = extras['is_set'] - - if isinstance(value, str): - lowered = value.strip().lower() - if lowered in {'true', '1', 'yes'}: - return True - if lowered in {'false', '0', 'no'}: - return False - return bool(value) - - -def _to_float(value) -> Optional[float]: - if value in (None, ''): - return None - try: - return float(value) - except (TypeError, ValueError): - return None - - -def _to_bool(value) -> Optional[bool]: - if value is None: - return None - if isinstance(value, bool): - return value - if isinstance(value, (int, float)): - return bool(value) - if isinstance(value, str): - lowered = value.strip().lower() - if lowered in {'true', '1', 'yes'}: - return True - if lowered in {'false', '0', 'no'}: - return False - return None - - -def resolve_numeric_datatype_details(concept: Concept) -> Optional[NumericDatatypeDetails]: - extras = concept.extras or {} - numeric_values = { - 'low_absolute': _to_float(extras.get('low_absolute')), - 'high_absolute': _to_float(extras.get('hi_absolute')), - 'low_normal': _to_float(extras.get('low_normal')), - 'high_normal': _to_float(extras.get('hi_normal')), - 'low_critical': _to_float(extras.get('low_critical')), - 'high_critical': _to_float(extras.get('hi_critical')), - } - units = extras.get('units') - if not units and not any(value is not None for value in numeric_values.values()): - return None - return NumericDatatypeDetails( - units=units, - low_absolute=numeric_values['low_absolute'], - high_absolute=numeric_values['high_absolute'], - low_normal=numeric_values['low_normal'], - high_normal=numeric_values['high_normal'], - low_critical=numeric_values['low_critical'], - high_critical=numeric_values['high_critical'], - ) - - -def resolve_coded_datatype_details(concept: Concept) -> Optional[CodedDatatypeDetails]: - extras = concept.extras or {} - allow_multiple = extras.get('allow_multiple') - if allow_multiple is None: - allow_multiple = extras.get('allow_multiple_answers') - if allow_multiple is None: - allow_multiple = extras.get('allowMultipleAnswers') - allow_multiple = _to_bool(allow_multiple) - if allow_multiple is None: - return None - return CodedDatatypeDetails(allow_multiple=allow_multiple) - - -def resolve_text_datatype_details(concept: Concept) -> Optional[TextDatatypeDetails]: - extras = concept.extras or {} - text_format = extras.get('text_format') or extras.get('textFormat') - if not text_format: - return None - return TextDatatypeDetails(text_format=text_format) - - -def resolve_datatype_details(concept: Concept) -> Optional[DatatypeDetails]: - datatype = (concept.datatype or '').strip().lower() - if datatype == 'numeric': - return resolve_numeric_datatype_details(concept) - if datatype == 'coded': - return resolve_coded_datatype_details(concept) - if datatype == 'text': - return resolve_text_datatype_details(concept) - return None - - -def format_datetime_for_api(value) -> Optional[str]: - if not value: - return None - if timezone.is_naive(value): - value = timezone.make_aware(value, datetime_timezone.utc) - return value.astimezone(datetime_timezone.utc).isoformat().replace('+00:00', 'Z') - - -def build_datatype(concept: Concept) -> Optional[DatatypeType]: - if not concept.datatype: - return None - return DatatypeType( - name=concept.datatype, - details=resolve_datatype_details(concept), - ) - - -def build_metadata(concept: Concept) -> MetadataType: - return MetadataType( - is_set=resolve_is_set_flag(concept), - is_retired=concept.retired, - created_by=getattr(concept.created_by, 'username', None), - created_at=format_datetime_for_api(concept.created_at), - updated_by=getattr(concept.updated_by, 'username', None), - updated_at=format_datetime_for_api(concept.updated_at), - ) - - -def serialize_concepts(concepts: Iterable[Concept]) -> List[ConceptType]: - output: List[ConceptType] = [] - for concept in concepts: - output.append( - ConceptType( - id=str(concept.id), - external_id=concept.external_id, - concept_id=concept.mnemonic, - display=concept.display_name, - names=serialize_names(concept), - mappings=serialize_mappings(concept), - description=resolve_description(concept), - concept_class=concept.concept_class, - datatype=build_datatype(concept), - metadata=build_metadata(concept), - ) - ) - return output - - -def concept_ids_from_es( +def concept_ids_from_es( # pylint: disable=too-many-arguments query: str, source_version: Optional[Source], pagination: Optional[dict], + owner: Optional[str] = None, + owner_type: Optional[str] = None, + user=None, ) -> Optional[tuple[list[int], int]]: trimmed = query.strip() if not trimmed: @@ -363,18 +178,24 @@ def concept_ids_from_es( search = ConceptDocument.search() if source_version: search = search.filter('term', source=source_version.mnemonic.lower()) - if source_version.is_head: - search = search.filter('term', is_latest_version=True) + if owner and owner_type: + search = search.filter('term', owner=owner.lower()).filter('term', owner_type=owner_type) + + # Always derive the effective version from the resolved Source object so that a + # HEAD-fallback (find_latest_released_version_by) does not get filtered by the + # client-supplied label, which would silently return zero hits. + effective_version = source_version.version + if effective_version == HEAD or source_version.is_head: + search = search.filter('term', is_head=True) else: - search = search.filter('term', source_version=source_version.version) - search = search.filter('term', retired=False) + search = search.filter('term', source_version=effective_version) + else: + search = search.filter('term', is_head=True) + search = apply_es_visibility_filter(search, user or AnonymousUser()) + search = apply_es_parent_visibility_filter(search, user or AnonymousUser()) + search = search.filter('term', retired=False).filter('term', is_active=True) - should_queries = [ - ES_Q('match', id={'query': trimmed, 'boost': 6, 'operator': 'AND'}), - ES_Q('match_phrase_prefix', name={'query': trimmed, 'boost': 4}), - ES_Q('match', synonyms={'query': trimmed, 'boost': 2, 'operator': 'AND'}), - ] - search = search.query(ES_Q('bool', should=should_queries, minimum_should_match=1)) + search = search_text(search, trimmed) if pagination: search = search[pagination['start']:pagination['end']] @@ -394,45 +215,25 @@ def concept_ids_from_es( return None -def fallback_db_search(base_qs, query: str): - trimmed = query.strip() - if not trimmed: - return base_qs.none() - return base_qs.filter( - Q(mnemonic__icontains=trimmed) | Q(names__name__icontains=trimmed, names__retired=False) - ).distinct() - - -async def concepts_for_ids( - base_qs, - concept_ids: Sequence[str], - pagination: Optional[dict], - mapping_prefetch: Prefetch, -) -> tuple[List[Concept], int]: - unique_ids = list(dict.fromkeys([cid for cid in concept_ids if cid])) - if not unique_ids: - raise GraphQLError('conceptIds must include at least one value when provided.') - - qs = base_qs.filter(mnemonic__in=unique_ids) - total = await sync_to_async(qs.count)() - ordering = Case( - *[When(mnemonic=value, then=pos) for pos, value in enumerate(unique_ids)], - output_field=IntegerField() - ) - qs = qs.order_by(ordering, 'mnemonic') - qs = apply_slice(qs, pagination) - qs = with_concept_related(qs, mapping_prefetch) - return await sync_to_async(list)(qs), total - - -async def concepts_for_query( +async def concepts_for_query( # pylint: disable=too-many-arguments base_qs, query: str, - source_version: Source, + source_version: Optional[Source], pagination: Optional[dict], mapping_prefetch: Prefetch, + owner: Optional[str] = None, + owner_type: Optional[str] = None, + user=None, + paths=None, ) -> tuple[List[Concept], int]: - es_result = await sync_to_async(concept_ids_from_es)(query, source_version, pagination) + es_result = await sync_to_async(concept_ids_from_es)( + query, + source_version, + pagination, + owner=owner, + owner_type=owner_type, + user=user, + ) if es_result is not None: concept_ids, total = es_result if not concept_ids: @@ -448,59 +249,121 @@ async def concepts_for_query( else: ordering = Case( *[When(id=pk, then=pos) for pos, pk in enumerate(concept_ids)], - output_field=IntegerField() + output_field=IntegerField(), ) qs = base_qs.filter(id__in=concept_ids).order_by(ordering) - qs = with_concept_related(qs, mapping_prefetch) + qs = with_concept_related(qs, mapping_prefetch, paths) return await sync_to_async(list)(qs), total - qs = fallback_db_search(base_qs, query).order_by('mnemonic') + qs = build_db_search_queryset(base_qs, query).order_by('mnemonic') total = await sync_to_async(qs.count)() qs = apply_slice(qs, pagination) - qs = with_concept_related(qs, mapping_prefetch) + qs = with_concept_related(qs, mapping_prefetch, paths) return await sync_to_async(list)(qs), total @strawberry.type -class Query: - @strawberry.field(name="concepts") - async def concepts( # pylint: disable=too-many-arguments,too-many-locals +class Query(PermissionsMixin): + async def resolve_source_version_for_permissions( + self, + org: Optional[str], + owner: Optional[str], + source: str, + version: Optional[str], + ) -> Source: + """Resolve repository versions through the shared GraphQL helper.""" + return await resolve_source_version(org, owner, source, version) + + @strawberry.field(name="concepts", description=( + "Search visible concepts. Indexed payloads are returned directly from Elasticsearch; " + "selected relationships or datatype details use the database. Anonymous callers see public data only." + )) + async def concepts( # pylint: disable=too-many-arguments,too-many-locals,too-many-branches self, info: strawberry.Info, - org: Optional[str] = None, - source: Optional[str] = None, - version: Optional[str] = None, - conceptIds: Optional[List[str]] = None, - query: Optional[str] = None, - page: Optional[int] = None, - limit: Optional[int] = None, + org: Annotated[Optional[str], + strawberry.argument(description="Organization mnemonic. Supply exactly one of org or owner with source."), + ] = None, + owner: Annotated[Optional[str], strawberry.argument(description="Username owning a personal source.")] = None, + source: Annotated[Optional[str], strawberry.argument(description="Source mnemonic, e.g. CIEL.")] = None, + version: Annotated[Optional[str], + strawberry.argument(description="Source version; defaults to HEAD, or latest release if HEAD is absent."), + ] = None, + conceptIds: Annotated[Optional[List[str]], + strawberry.argument(description="Exact concept mnemonics in result order; takes precedence over query."), + ] = None, + query: Annotated[Optional[str], + strawberry.argument(description="Free text to search concept identifiers, names and synonyms."), + ] = None, + page: Annotated[Optional[int], + strawberry.argument(description="Page number starting at 1. Supply together with limit."), + ] = None, + limit: Annotated[Optional[int], + strawberry.argument(description="Maximum results per page. Supply together with page; ES window is 10000."), + ] = None, ) -> ConceptSearchResult: - if info.context.auth_status == 'none': - raise GraphQLError('Authentication required') - - if info.context.auth_status == 'invalid': - raise GraphQLError('Authentication failure') - - concept_ids_param = conceptIds or [] + if getattr(info.context, 'auth_status', 'none') == 'invalid': + from .constants import AUTHENTICATION_FAILED, build_expected_graphql_error + raise build_expected_graphql_error(AUTHENTICATION_FAILED) + root = self or Query() + concept_ids_param = list(dict.fromkeys(value for value in (conceptIds or []) if value)) text_query = (query or '').strip() + user = getattr(info.context, 'user', AnonymousUser()) if not concept_ids_param and not text_query: - raise GraphQLError('Either conceptIds or query must be provided.') + raise build_validation_error('Either conceptIds or query must be provided.') pagination = normalize_pagination(page, limit) - if org and source: - source_version = await resolve_source_version(org, source, version) - base_qs = build_base_queryset(source_version) - mapping_prefetch = build_mapping_prefetch(source_version) + if org and owner: + raise build_validation_error('Provide either org or owner, not both.') + + if source and not org and not owner: + raise build_validation_error('Either org or owner must be provided when source is specified.') + + if version and not source: + raise build_validation_error('version requires a source.') + if (org or owner) and not source: + raise build_validation_error('source is required with org or owner.') + if len(concept_ids_param) > ES_MAX_WINDOW: + raise build_validation_error('conceptIds cannot exceed 10000 values.') + + owner_value, owner_type = resolve_owner(org, owner) + paths = selected_paths(info) + concept_paths = child_paths(paths, 'results') if paths is not None else None + if index_projection(concept_paths, CONCEPT_INDEX_FIELDS) is not None: + scope = None + if source: + scope = await sync_to_async(indexed_source)(org, owner, source, version, user, set()) + if not source or scope: + projected = await sync_to_async(indexed_concepts)( + concept_paths, text_query, concept_ids_param, scope, pagination, owner_value, owner_type, user, + ) + if projected is not None: + serialized, total = projected + return ConceptSearchResult( + org=org, source=source, version_resolved=scope.version if scope else '', + page=pagination['page'] if pagination else None, + limit=pagination['limit'] if pagination else None, + total_count=total, has_next_page=has_next(total, pagination), results=serialized, + ) + + + if (org or owner) and source: + source_version = await root.get_source_version(info, org, owner, source, version) + await root.ensure_can_view_repo(user, source_version) + base_qs = build_source_version_queryset(source_version) + mapping_prefetch = build_mapping_prefetch(source_version, user) else: # Global search across all repositories source_version = None - base_qs = Concept.objects.filter(is_active=True, retired=False) - mapping_prefetch = build_global_mapping_prefetch() + base_qs = filter_parent_queryset(root.filter_global_queryset(build_global_head_queryset(), user), user) + mapping_prefetch = build_global_mapping_prefetch(user) if concept_ids_param: - concepts, total = await concepts_for_ids(base_qs, concept_ids_param, pagination, mapping_prefetch) + concepts, total = await concepts_for_ids( + base_qs, concept_ids_param, pagination, mapping_prefetch, concept_paths, + ) else: concepts, total = await concepts_for_query( base_qs, @@ -508,9 +371,13 @@ async def concepts( # pylint: disable=too-many-arguments,too-many-locals source_version, pagination, mapping_prefetch, + owner=owner_value, + owner_type=owner_type, + user=user, + paths=concept_paths, ) - serialized = await sync_to_async(serialize_concepts)(concepts) + serialized = await sync_to_async(serialize_concepts)(concepts, concept_paths) return ConceptSearchResult( org=org, source=source, @@ -521,3 +388,31 @@ async def concepts( # pylint: disable=too-many-arguments,too-many-locals has_next_page=has_next(total, pagination), results=serialized, ) + + + @strawberry.field(description=( + "Read a visible source version. name, description, canonicalUrl and uri can use only Elasticsearch; " + "statistics query only the requested version-scoped aggregates." + )) + async def source( # pylint: disable=too-many-arguments + self, + info: strawberry.Info, + source: Annotated[str, strawberry.argument(description="Source mnemonic, e.g. CIEL.")], + org: Annotated[Optional[str], strawberry.argument(description="Owning organization mnemonic.")] = None, + owner: Annotated[Optional[str], + strawberry.argument(description="Owning username; mutually exclusive with org."), + ] = None, + version: Annotated[Optional[str], strawberry.argument(description="Version label; defaults to HEAD.")] = None, + ) -> SourceType: + """Select metadata or aggregates after validating ownership and visibility.""" + root = self or Query() + if not source or bool(org) == bool(owner): + raise build_validation_error('Provide source and exactly one of org or owner.') + user = getattr(info.context, 'user', AnonymousUser()) + paths = selected_paths(info) or set() + projected = await sync_to_async(indexed_source)(org, owner, source, version, user, paths) + if projected is not None: + return projected.payload + instance = await root.get_source_version(info, org, owner, source, version) + await root.ensure_can_view_repo(user, instance) + return await sync_to_async(serialize_source)(instance, paths, user) diff --git a/core/graphql/schema.py b/core/graphql/schema.py index 70874634c..87503f448 100644 --- a/core/graphql/schema.py +++ b/core/graphql/schema.py @@ -1,9 +1,22 @@ import strawberry from strawberry_django.optimizer import DjangoOptimizerExtension +from .constants import EXPECTED_GRAPHQL_ERROR_CODES, get_graphql_error_code +from .extensions import AuthStatusExtension from .queries import Query -schema = strawberry.Schema( + +class OCLGraphQLSchema(strawberry.Schema): + def process_errors(self, errors, execution_context=None): + # Expected business-rule failures should reach clients, but they should not be recorded as server errors. + unexpected_errors = [ + error for error in errors if get_graphql_error_code(error) not in EXPECTED_GRAPHQL_ERROR_CODES + ] + if unexpected_errors: + super().process_errors(unexpected_errors, execution_context) + + +schema = OCLGraphQLSchema( query=Query, - extensions=[DjangoOptimizerExtension], + extensions=[AuthStatusExtension, DjangoOptimizerExtension], ) diff --git a/core/graphql/search.py b/core/graphql/search.py new file mode 100644 index 000000000..abf0b8d28 --- /dev/null +++ b/core/graphql/search.py @@ -0,0 +1,157 @@ +"""Pure search helpers for GraphQL: queryset/prefetch builders, pagination and DB fallback. + +The orchestrators that talk to Elasticsearch (and decide DB-fallback) live in ``queries.py`` +so tests can patch the ES boundary in a single place. Only side-effect-free helpers go here. +""" + +from __future__ import annotations + +from typing import List, Optional, Sequence + +from asgiref.sync import sync_to_async +from django.contrib.auth.models import AnonymousUser +from django.db.models import Case, F, IntegerField, Prefetch, Q, When + +from core.concepts.models import Concept +from core.mappings.models import Mapping +from core.sources.models import Source + +from .constants import build_validation_error +from .permissions import filter_global_queryset, filter_parent_queryset + + +def build_source_version_queryset(source_version: Source): + return source_version.get_concepts_queryset().filter(is_active=True, retired=False) + + +def build_global_head_queryset(): + return Concept.objects.filter(is_active=True, retired=False, id=F('versioned_object_id')) + + +def build_mapping_prefetch(source_version: Source, user=None) -> Prefetch: + mapping_qs = ( + Mapping.objects.filter( + sources__id=source_version.id, + from_concept_id__isnull=False, + is_active=True, + retired=False, + ) + .select_related('to_source', 'to_concept', 'to_concept__parent') + .prefetch_related('to_concept__names') + .order_by('map_type', 'to_concept_code', 'to_concept__mnemonic') + .distinct() + ) + + mapping_qs = filter_parent_queryset(mapping_qs, user or AnonymousUser(), 'to_source') + mapping_qs = filter_parent_queryset(mapping_qs, user or AnonymousUser(), 'to_concept__parent') + return Prefetch('mappings_from', queryset=mapping_qs, to_attr='graphql_mappings') + + +def build_global_mapping_prefetch(user=None) -> Prefetch: + """Build the global mapping prefetch using the same visibility rules as REST list endpoints.""" + mapping_qs = ( + Mapping.objects.filter( + from_concept_id__isnull=False, + is_active=True, + retired=False, + ) + .select_related('to_source', 'to_concept', 'to_concept__parent') + .prefetch_related('to_concept__names') + .order_by('map_type', 'to_concept_code', 'to_concept__mnemonic') + .distinct() + ) + + # Mapping visibility must be filtered independently because a public concept can still reference private mappings. + mapping_qs = filter_global_queryset(mapping_qs, user or AnonymousUser()) + mapping_qs = filter_parent_queryset(mapping_qs, user or AnonymousUser()) + mapping_qs = filter_parent_queryset(mapping_qs, user or AnonymousUser(), 'to_source') + mapping_qs = filter_parent_queryset(mapping_qs, user or AnonymousUser(), 'to_concept__parent') + return Prefetch('mappings_from', queryset=mapping_qs, to_attr='graphql_mappings') + + +def normalize_pagination(page: Optional[int], limit: Optional[int]) -> Optional[dict]: + if page is None and limit is None: + return None + if page is None or limit is None: + raise build_validation_error('page and limit must be supplied together.') + if page < 1 or limit < 1: + raise build_validation_error('page and limit must be >= 1 when provided.') + start = (page - 1) * limit + end = start + limit + if end > 10_000: + raise build_validation_error('Requested page exceeds the 10000 result window.') + return {'page': page, 'limit': limit, 'start': start, 'end': end} + + +def has_next(total: int, pagination: Optional[dict]) -> bool: + if not pagination: + return False + return total > pagination['end'] + + +def apply_slice(qs, pagination: Optional[dict]): + if not pagination: + return qs + return qs[pagination['start']:pagination['end']] + + +def with_concept_related(qs, mapping_prefetch: Prefetch, paths=None): + """Hydrate only selected fields and relations; preserve the legacy helper default.""" + if paths is None: + return qs.select_related('created_by', 'updated_by', 'parent').prefetch_related( + 'names', 'descriptions', mapping_prefetch, + ) + roots = {path.split('.')[0] for path in paths} + columns = {'id', 'mnemonic'} + fields = {'externalId': 'external_id', 'conceptClass': 'concept_class', 'datatype': 'datatype', 'extras': 'extras'} + columns.update(value for key, value in fields.items() if key in roots) + relations = [] + if roots & {'display', 'description'}: + qs = qs.select_related('parent') + columns.add('parent') + if roots & {'display', 'names'}: + relations.append('names') + if 'description' in roots: + relations.append('descriptions') + if 'mappings' in roots: + relations.append(mapping_prefetch) + if 'metadata' in roots: + columns.update({'retired', 'extras', 'created_at', 'updated_at', 'created_by', 'updated_by'}) + qs = qs.select_related('created_by', 'updated_by') + if any(path.startswith('datatype.details') for path in paths): + columns.add('extras') + return qs.only(*sorted(columns)).prefetch_related(*relations) + + +async def concepts_for_ids( + base_qs, + concept_ids: Sequence[str], + pagination: Optional[dict], + mapping_prefetch: Prefetch, + paths=None, +) -> tuple[List[Concept], int]: + """Fetch concepts by mnemonic while preserving the client-provided ordering.""" + ordered_ids = list(dict.fromkeys(concept_id for concept_id in concept_ids if concept_id)) + if not ordered_ids: + raise build_validation_error('conceptIds must include at least one value when provided.') + + ordering = Case( + *[When(mnemonic=concept_id, then=pos) for pos, concept_id in enumerate(ordered_ids)], + output_field=IntegerField(), + ) + qs = base_qs.filter(mnemonic__in=ordered_ids).order_by(ordering, 'mnemonic') + total = await sync_to_async(qs.count)() + qs = apply_slice(qs, pagination) + qs = with_concept_related(qs, mapping_prefetch, paths) + return await sync_to_async(list)(qs), total + + +def build_db_search_queryset(base_qs, query: str): + """Build the database fallback used when Elasticsearch is unavailable or stale.""" + trimmed = query.strip() + if not trimmed: + return base_qs.none() + + return base_qs.filter( + Q(mnemonic__icontains=trimmed) | Q(names__name__icontains=trimmed, names__retired=False) + ).distinct() diff --git a/core/graphql/selection.py b/core/graphql/selection.py new file mode 100644 index 000000000..0b2c9025c --- /dev/null +++ b/core/graphql/selection.py @@ -0,0 +1,42 @@ +"""Plan data access from the requested fields, including fragments and directives.""" + + +def selected_paths(info): + """Return selected leaf paths; aliases never change the underlying field names.""" + fields = getattr(info, 'selected_fields', None) + if fields is None: + return None + paths = set() + + def walk(selections, prefix): + """Expand Strawberry's resolved fragments and skip disabled selections.""" + for selection in selections: + directives = getattr(selection, 'directives', {}) + if directives.get('skip', {}).get('if') or directives.get('include', {}).get('if') is False: + continue + name = getattr(selection, 'name', None) + children = getattr(selection, 'selections', []) + # Fragment spreads have a name too, but are not fields. + is_field = type(selection).__name__ == 'SelectedField' + path = prefix + (name,) if is_field else prefix + if children: + walk(children, path) + elif is_field: + paths.add('.'.join(path)) + + for field in fields: + walk(field.selections, ()) + return paths + + +def child_paths(paths, field): + """Extract a nested object's requested leaves from a selection plan.""" + prefix = field + '.' + return {path[len(prefix):] for path in paths if path.startswith(prefix)} + + +def index_projection(paths, fields): + """Return the minimum index fields, or None when any leaf needs the ORM.""" + if paths is None or not paths <= fields.keys(): + return None + return sorted({value for path in paths for value in fields[path]}) diff --git a/core/graphql/serializers.py b/core/graphql/serializers.py new file mode 100644 index 000000000..363d8d178 --- /dev/null +++ b/core/graphql/serializers.py @@ -0,0 +1,249 @@ +"""Pure-Python serializers that map ORM Concept instances into Strawberry types. + +These helpers must remain side-effect free: callers prefetch the required relations +(``names``, ``descriptions``, ``graphql_mappings``) before invoking them so the serializers +never trigger SQL. +""" + +from __future__ import annotations + +from datetime import timezone as datetime_timezone +from typing import Iterable, List, Optional + +from django.utils import timezone + +from core.concepts.models import Concept +from core.sources.models import Source + +from .types import ( + CodedDatatypeDetails, + ConceptNameType, + ConceptType, + DatatypeDetails, + DatatypeType, + MappingType, + MetadataType, + NumericDatatypeDetails, + TextDatatypeDetails, + ToSourceType, +) + + +def serialize_mappings(concept: Concept) -> List[MappingType]: + mappings = getattr(concept, 'graphql_mappings', []) or [] + result: List[MappingType] = [] + for mapping in mappings: + result.append( + MappingType( + map_type=str(mapping.map_type), + to_source=ToSourceType( + url=mapping.to_source_url, + name=mapping.to_source_name, + ) if mapping.to_source_url or mapping.to_source_name else None, + to_code=mapping.get_to_concept_code(), + to_concept_name=mapping.get_to_concept_name(), + sort_weight=mapping.sort_weight, + comment=mapping.comment, + ) + ) + return result + + +def serialize_names(concept: Concept) -> List[ConceptNameType]: + return [ + ConceptNameType( + name=name.name, + locale=name.locale, + type=name.type, + preferred=name.locale_preferred, + retired=name.retired, + ) + for name in concept.names.all() + ] + + +def resolve_description(concept: Concept) -> Optional[str]: + cached = getattr(concept, '_prefetched_objects_cache', {}) + descriptions = ( + [description for description in cached['descriptions'] if not description.retired] + if 'descriptions' in cached else list(concept.active_descriptions.all()) + ) + if not descriptions: + return None + + def pick(predicate): + for desc in descriptions: + if predicate(desc): + return desc.description + return None + + try: + default_locale = getattr(concept.parent, 'default_locale', None) + except Source.DoesNotExist: + default_locale = None + if default_locale: + match = pick(lambda desc: desc.locale == default_locale and desc.locale_preferred) + if match: + return match + match = pick(lambda desc: desc.locale == default_locale) + if match: + return match + + match = pick(lambda desc: desc.locale_preferred) + if match: + return match + return descriptions[0].description + + +def resolve_is_set_flag(concept: Concept) -> Optional[bool]: + value = getattr(concept, 'is_set', None) + if value is None: + extras = concept.extras or {} + if 'is_set' not in extras: + return None + value = extras['is_set'] + + if isinstance(value, str): + lowered = value.strip().lower() + if lowered in {'true', '1', 'yes'}: + return True + if lowered in {'false', '0', 'no'}: + return False + return bool(value) + + +def _to_float(value) -> Optional[float]: + if value in (None, ''): + return None + try: + return float(value) + except (TypeError, ValueError): + return None + + +def _to_bool(value) -> Optional[bool]: + if value is None: + return None + if isinstance(value, bool): + return value + if isinstance(value, (int, float)): + return bool(value) + if isinstance(value, str): + lowered = value.strip().lower() + if lowered in {'true', '1', 'yes'}: + return True + if lowered in {'false', '0', 'no'}: + return False + return None + + +def resolve_numeric_datatype_details(concept: Concept) -> Optional[NumericDatatypeDetails]: + extras = concept.extras or {} + numeric_values = { + 'low_absolute': _to_float(extras.get('low_absolute')), + 'high_absolute': _to_float(extras.get('hi_absolute')), + 'low_normal': _to_float(extras.get('low_normal')), + 'high_normal': _to_float(extras.get('hi_normal')), + 'low_critical': _to_float(extras.get('low_critical')), + 'high_critical': _to_float(extras.get('hi_critical')), + } + units = extras.get('units') + if not units and not any(value is not None for value in numeric_values.values()): + return None + return NumericDatatypeDetails( + units=units, + low_absolute=numeric_values['low_absolute'], + high_absolute=numeric_values['high_absolute'], + low_normal=numeric_values['low_normal'], + high_normal=numeric_values['high_normal'], + low_critical=numeric_values['low_critical'], + high_critical=numeric_values['high_critical'], + ) + + +def resolve_coded_datatype_details(concept: Concept) -> Optional[CodedDatatypeDetails]: + extras = concept.extras or {} + allow_multiple = extras.get('allow_multiple') + if allow_multiple is None: + allow_multiple = extras.get('allow_multiple_answers') + if allow_multiple is None: + allow_multiple = extras.get('allowMultipleAnswers') + allow_multiple = _to_bool(allow_multiple) + if allow_multiple is None: + return None + return CodedDatatypeDetails(allow_multiple=allow_multiple) + + +def resolve_text_datatype_details(concept: Concept) -> Optional[TextDatatypeDetails]: + extras = concept.extras or {} + text_format = extras.get('text_format') or extras.get('textFormat') + if not text_format: + return None + return TextDatatypeDetails(text_format=text_format) + + +def resolve_datatype_details(concept: Concept) -> Optional[DatatypeDetails]: + datatype = (concept.datatype or '').strip().lower() + if datatype == 'numeric': + return resolve_numeric_datatype_details(concept) + if datatype == 'coded': + return resolve_coded_datatype_details(concept) + if datatype == 'text': + return resolve_text_datatype_details(concept) + return None + + +def format_datetime_for_api(value) -> Optional[str]: + if not value: + return None + if timezone.is_naive(value): + value = timezone.make_aware(value, datetime_timezone.utc) + return value.astimezone(datetime_timezone.utc).isoformat().replace('+00:00', 'Z') + + +def build_datatype(concept: Concept) -> Optional[DatatypeType]: + if not concept.datatype: + return None + return DatatypeType( + name=concept.datatype, + details=resolve_datatype_details(concept), + ) + + +def build_metadata(concept: Concept) -> MetadataType: + return MetadataType( + is_set=resolve_is_set_flag(concept), + is_retired=concept.retired, + created_by=getattr(concept.created_by, 'username', None), + created_at=format_datetime_for_api(concept.created_at), + updated_by=getattr(concept.updated_by, 'username', None), + updated_at=format_datetime_for_api(concept.updated_at), + ) + + +def serialize_concepts(concepts: Iterable[Concept], paths=None) -> List[ConceptType]: + def requested(field): + """Detect selected scalar or nested values without evaluating model properties.""" + return paths is None or any(path == field or path.startswith(field + '.') for path in paths) + + output: List[ConceptType] = [] + for concept in concepts: + output.append( + ConceptType( + id=str(concept.id), + external_id=concept.external_id if requested('externalId') else None, + concept_id=concept.mnemonic, + display=concept.display_name if requested('display') else None, + names=serialize_names(concept) if requested('names') else [], + mappings=serialize_mappings(concept) if requested('mappings') else [], + description=resolve_description(concept) if requested('description') else None, + concept_class=concept.concept_class if requested('conceptClass') else None, + datatype=( + build_datatype(concept) if paths is None or requested('datatype.details') else + DatatypeType(name=concept.datatype, details=None) if concept.datatype else None + ) if requested('datatype') else None, + metadata=build_metadata(concept) if requested('metadata') else None, + extras=(concept.extras or {}) if requested('extras') else {}, + ) + ) + return output diff --git a/core/graphql/sources.py b/core/graphql/sources.py new file mode 100644 index 000000000..0a56114bb --- /dev/null +++ b/core/graphql/sources.py @@ -0,0 +1,68 @@ +"""Source projections reuse the repository's version-aware querysets and permissions.""" + +from core.common.permissions import user_can_view_concept_dictionary + +from .types import SourceType, ToSourceType + +SOURCE_INDEX_FIELDS = { + '__typename': (), + 'name': ('name',), + 'description': ('description',), + 'canonicalUrl': ('canonical_url',), + 'uri': ('uri',), +} + + +def serialize_source(instance, paths, user): + """Load only the aggregates requested by the client, using existing model querysets.""" + result = SourceType( + **{ + fields[0]: getattr(instance, fields[0]) + for path, fields in SOURCE_INDEX_FIELDS.items() if path in paths and fields + } + ) + if paths & {'classes', 'datatypes', 'summary.activeConcepts'}: + concepts = instance.get_concepts_queryset().filter(is_active=True, retired=False) + if 'classes' in paths: + result.classes = distinct_values(concepts, 'concept_class') + if 'datatypes' in paths: + result.datatypes = distinct_values(concepts, 'datatype') + if 'summary.activeConcepts' in paths: + result.summary.active_concepts = concepts.count() + if paths & {'mapTypes', 'summary.mappings'} or any(path.startswith('externalSources.') for path in paths): + mappings = instance.get_mappings_queryset().filter(is_active=True, retired=False) + if 'mapTypes' in paths: + result.map_types = distinct_values(mappings, 'map_type') + if 'summary.mappings' in paths: + result.summary.mappings = mappings.count() + if any(path.startswith('externalSources.') for path in paths): + result.external_sources = external_sources(mappings, instance, user) + return result + + +def distinct_values(queryset, field): + """Return stable, unique, non-empty labels without instantiating child records.""" + return list(queryset.exclude(**{field: ''}).order_by(field).values_list(field, flat=True).distinct()) + + +def external_sources(mappings, instance, user): + """Deduplicate outbound targets and hide linked repositories the caller cannot view.""" + result = {} + targets = mappings.order_by('to_source_id', 'to_source_url', 'to_concept__parent_id').distinct( + 'to_source_id', 'to_source_url', 'to_concept__parent_id', + ).select_related( + 'to_source', 'to_source__organization', 'to_source__user', + 'to_concept__parent__organization', 'to_concept__parent__user', + ) + for mapping in targets: + target = mapping.get_to_source() + if target: + if target.mnemonic == instance.mnemonic and target.parent == instance.parent: + continue + if not user_can_view_concept_dictionary(user, target): + continue + url = mapping.to_source_url or (target.uri if target else None) + name = target.name if target else mapping.to_source_name + if url or name: + result[(url or '', name or '')] = ToSourceType(url=url, name=name) + return [result[key] for key in sorted(result)] diff --git a/core/graphql/tests/test_concepts_from_source.py b/core/graphql/tests/test_concepts_from_source.py index c4caf4013..acea4c97c 100644 --- a/core/graphql/tests/test_concepts_from_source.py +++ b/core/graphql/tests/test_concepts_from_source.py @@ -39,6 +39,10 @@ class ConceptsFromSourceQueryTests(OCLTestCase): maxDiff = None def setUp(self): + # This suite exercises ORM hydration/fallback; index projections have independent tests. + projection = mock.patch('core.graphql.queries.indexed_concepts', return_value=None) + projection.start() + self.addCleanup(projection.stop) self._old_async_flag = os.environ.get('DJANGO_ALLOW_ASYNC_UNSAFE') os.environ['DJANGO_ALLOW_ASYNC_UNSAFE'] = 'true' self.super_user = bootstrap_super_user() @@ -82,6 +86,7 @@ def setUp(self): from_concept=self.concept1, to_concept=self.concept2, map_type='Same As', + sort_weight=1.5, comment='primary link', created_by=self.audit_user, updated_by=self.audit_user, @@ -136,7 +141,8 @@ def test_fetch_concepts_by_ids_with_pagination(self): results { conceptId display - mappings { mapType toSource { url name } toCode comment } + mappings { mapType toSource { url name } toCode toConceptName sortWeight comment } + extras } } } @@ -161,6 +167,9 @@ def test_fetch_concepts_by_ids_with_pagination(self): self.assertEqual(len(payload['results']), 1) self.assertEqual(payload['results'][0]['conceptId'], self.concept1.mnemonic) self.assertEqual(payload['results'][0]['mappings'][0]['toCode'], self.concept2.mnemonic) + self.assertEqual(payload['results'][0]['mappings'][0]['toConceptName'], self.concept2.display_name) + self.assertEqual(payload['results'][0]['mappings'][0]['sortWeight'], self.mapping.sort_weight) + self.assertEqual(payload['results'][0]['extras'], self.concept1.extras) def test_concepts_include_metadata_fields(self): query = """ @@ -438,7 +447,9 @@ def test_fetch_concepts_for_specific_version(self): self.assertEqual(payload['versionResolved'], self.release_version.version) self.assertEqual(payload['results'][0]['conceptId'], self.concept1.mnemonic) - def test_fetch_concepts_global_search(self): + @mock.patch('core.graphql.queries.concept_ids_from_es') + def test_fetch_concepts_global_search(self, mock_es): + mock_es.return_value = ([self.concept1.id], 1) query = """ query GlobalConcepts($query: String!) { concepts(query: $query) { diff --git a/core/graphql/tests/test_graphql_view.py b/core/graphql/tests/test_graphql_view.py index 510f1b5c3..7f790ea41 100644 --- a/core/graphql/tests/test_graphql_view.py +++ b/core/graphql/tests/test_graphql_view.py @@ -9,6 +9,7 @@ from rest_framework.exceptions import AuthenticationFailed from core.common.tests import OCLTestCase +from core.graphql.constants import AUTHENTICATION_FAILED from core.graphql.tests.conftest import bootstrap_super_user, create_user_with_token @@ -94,7 +95,7 @@ def authenticate(self, request): token_type='Bearer', authentication_backend_class=ModelBackend, ) - ): + ), patch('strawberry.schema.base.StrawberryLogger.error') as error_logger: response = self._post_graphql( headers={"HTTP_AUTHORIZATION": "Bearer invalid-oidc-token"}, query=query @@ -104,3 +105,5 @@ def authenticate(self, request): self.assertEqual(response.status_code, 200) self.assertIn('errors', payload) self.assertIn('Authentication failure', payload['errors'][0]['message']) + self.assertEqual(payload['errors'][0]['extensions']['code'], AUTHENTICATION_FAILED) + error_logger.assert_not_called() diff --git a/core/graphql/tests/test_projection.py b/core/graphql/tests/test_projection.py new file mode 100644 index 000000000..4ee6b4fef --- /dev/null +++ b/core/graphql/tests/test_projection.py @@ -0,0 +1,182 @@ +"""Selection planning contracts and SQL-free index retrieval through the public schema.""" + +from types import SimpleNamespace +from unittest.mock import patch + +from asgiref.sync import async_to_sync +from django.contrib.auth.models import AnonymousUser +from django.test import SimpleTestCase +from elasticsearch import ConnectionError as ESConnectionError +from elasticsearch_dsl import Search +from elasticsearch_dsl.response import Response + +from core.graphql.schema import schema +from core.graphql.selection import index_projection + + +class ProjectionTests(SimpleTestCase): + """SimpleTestCase rejects SQL, including accidentally materialized ORM relationships.""" + + def execute(self, query, variables=None, auth_status='none'): + """Execute as an anonymous user without allowing database access.""" + return async_to_sync(schema.execute)( + query, variable_values=variables, + context_value=SimpleNamespace(user=AnonymousUser(), auth_status=auth_status), + ) + + @staticmethod + def response(index, fields, total=None): + """Use real Elasticsearch response wrappers to validate projection serialization.""" + hits = [{'_id': str(pos + 10), '_index': index, '_source': item} for pos, item in enumerate(fields)] + return Response(Search(), {'hits': {'total': {'value': len(hits) if total is None else total}, 'hits': hits}}) + + def test_source_metadata_has_no_sql_and_projects_selected_fields(self): + """Minimal source reads use one ES request with owner, version and visibility filters.""" + response = self.response('sources', [{ + 'name': 'CIEL', 'description': 'Clinical dictionary', 'canonical_url': 'https://ciel.org', + 'uri': '/orgs/CIEL/sources/CIEL/', 'is_active': True, 'version': 'HEAD', 'mnemonic': 'CIEL', + }]) + with patch('elasticsearch_dsl.Search.execute', autospec=True, return_value=response) as execute: + result = self.execute('''{ source(org: "CIEL", source: "CIEL") { + name description canonicalUrl uri + } }''') + self.assertIsNone(result.errors) + self.assertEqual(result.data['source']['uri'], '/orgs/CIEL/sources/CIEL/') + body = execute.call_args.args[0].to_dict() + self.assertEqual(set(body['_source']), { + 'name', 'description', 'canonical_url', 'uri', 'is_active', 'version', 'mnemonic', + }) + filters = str(body['query']) + for expected in ('public_can_view', 'owner_type', 'Organization', 'ciel', 'HEAD'): + self.assertIn(expected, filters) + + def test_concept_fragments_aliases_and_directives_remain_sql_free(self): + """Skipped heavy fields do not force ORM loading, including named fragments.""" + response = self.response('concepts', [{ + 'id': '123', 'name': 'Hypertension', 'datatype': 'Numeric', + 'concept_class': 'Diagnosis', 'preferred_description': 'Preferred definition', + }]) + query = '''query($heavy: Boolean!, $light: Boolean!) { + found: concepts(query: "hypertension") { totalCount results { + ...Light + ... on ConceptType { description } + names @include(if: $heavy) { name } + mappings @skip(if: $light) { mapType } + } } + } + fragment Light on ConceptType { code: conceptId label: display datatype { name } conceptClass } + ''' + with patch('elasticsearch_dsl.Search.execute', autospec=True, return_value=response) as execute: + result = self.execute(query, {'heavy': False, 'light': True}) + self.assertIsNone(result.errors) + self.assertEqual(result.data['found']['results'][0], { + 'code': '123', 'label': 'Hypertension', 'datatype': {'name': 'Numeric'}, + 'conceptClass': 'Diagnosis', 'description': 'Preferred definition', + }) + body = execute.call_args.args[0].to_dict() + self.assertEqual(set(body['_source']), {'id', 'name', 'datatype', 'concept_class', 'preferred_description'}) + self.assertNotIn('extras', body['_source']) + self.assertIn('is_head', str(body['query'])) + + def test_id_order_pagination_and_total(self): + """Ordering follows exact mnemonic input, with duplicates removed before slicing.""" + response = self.response('concepts', [{'id': 'A'}, {'id': 'B'}, {'id': 'C'}]) + with patch('elasticsearch_dsl.Search.execute', return_value=response): + result = self.execute('''{ concepts(conceptIds: ["C", "B", "C", "A"], page: 2, limit: 1) { + totalCount hasNextPage results { conceptId } + } }''') + self.assertIsNone(result.errors) + self.assertEqual(result.data['concepts'], { + 'totalCount': 3, 'hasNextPage': True, 'results': [{'conceptId': 'B'}], + }) + + def test_zero_hits_are_authoritative_for_index_projection(self): + """An empty successful ES response does not trigger database search.""" + with patch('elasticsearch_dsl.Search.execute', return_value=self.response('concepts', [])): + result = self.execute('{ concepts(query: "absent") { totalCount results { display } } }') + self.assertIsNone(result.errors) + self.assertEqual(result.data['concepts'], {'totalCount': 0, 'results': []}) + + def test_two_aliases_plan_fields_independently(self): + """Selections for one alias never get dropped or borrowed from another.""" + with patch('elasticsearch_dsl.Search.execute', autospec=True, return_value=self.response('concepts', [])) as es: + result = self.execute('''{ + a: concepts(query: "a") { results { conceptId } } + b: concepts(query: "b") { results { display } } + }''') + self.assertIsNone(result.errors) + self.assertEqual([call.args[0].to_dict()['_source'] for call in es.call_args_list], [['id'], ['name']]) + + def test_invalid_auth_stops_all_queries(self): + """Schema-level auth failure precedes both source and concept data access.""" + with patch('elasticsearch_dsl.Search.execute') as es: + result = self.execute('{ source(org: "CIEL", source: "CIEL") { name } }', auth_status='invalid') + self.assertEqual(result.errors[0].extensions['code'], 'AUTHENTICATION_FAILED') + es.assert_not_called() + + def test_invalid_scope_and_pagination_stop_before_search(self): + """Ambiguous ownership, incomplete pagination and excessive windows are rejected early.""" + cases = [ + 'source(org: "O", owner: "U", source: "S") { name }', + 'source(source: "S") { name }', + 'concepts(query: "x", org: "O") { totalCount }', + 'concepts(query: "x", version: "v1") { totalCount }', + 'concepts(query: "x", page: 1) { totalCount }', + 'concepts(query: "x", limit: 1) { totalCount }', + 'concepts(query: "x", page: 0, limit: 1) { totalCount }', + 'concepts(query: "x", page: 2, limit: 10000) { totalCount }', + ] + for body in cases: + with self.subTest(body=body), patch('elasticsearch_dsl.Search.execute') as es: + result = self.execute('{' + body + '}') + self.assertEqual(result.errors[0].extensions['code'], 'VALIDATION_ERROR') + es.assert_not_called() + + def test_planner_rejects_relationships_and_details(self): + """Only a fully covered payload can use the direct projection path.""" + self.assertIsNone(index_projection({'datatype.details.units'}, {'datatype.name': ('datatype',)})) + self.assertIsNone(index_projection(None, {})) + self.assertEqual(index_projection(set(), {}), []) + + def test_es_connection_failure_returns_fallback_signal(self): + """Only expected transport failures request the permission-checked ORM fallback.""" + from core.graphql.indexed import indexed_concepts, indexed_source + with patch('elasticsearch_dsl.Search.execute', side_effect=ESConnectionError('offline')): + self.assertIsNone(indexed_concepts({'display'}, 'x', [], None, None, None, None, AnonymousUser())) + self.assertIsNone(indexed_source('O', None, 'S', None, AnonymousUser(), {'name'})) + + def test_introspection_documents_queries_and_arguments(self): + """GraphiQL exposes query, argument, source and summary descriptions.""" + sdl = schema.as_str() + for fragment in ('canonicalUrl', 'externalSources', 'activeConcepts', 'SourceSummaryType', + 'Owning organization mnemonic.', 'Source mnemonic, e.g. CIEL.', + 'Free text to search concept identifiers'): + self.assertIn(fragment, sdl) + + def test_typename_only_nested_selection_preserves_nullable_objects(self): + """A datatype selected only for __typename still needs its indexed value.""" + response = self.response('concepts', [{'datatype': 'Numeric'}]) + with patch('elasticsearch_dsl.Search.execute', return_value=response): + result = self.execute('{ concepts(query: "x") { results { datatype { __typename } } } }') + self.assertIsNone(result.errors) + self.assertEqual(result.data['concepts']['results'], [{'datatype': {'__typename': 'DatatypeType'}}]) + + def test_count_only_does_not_fetch_result_documents(self): + """Count-only queries request zero hits instead of loading an unused result page.""" + response = self.response('concepts', [], total=500) + with patch('elasticsearch_dsl.Search.execute', autospec=True, return_value=response) as es: + result = self.execute('{ concepts(query: "x") { totalCount } }') + self.assertIsNone(result.errors) + self.assertEqual(result.data['concepts']['totalCount'], 500) + self.assertEqual(es.call_args.args[0].to_dict()['size'], 0) + + def test_shared_search_rule_preserves_rest_creator_access(self): + """REST keeps creator visibility while GraphQL explicitly opts into owner/membership rules.""" + from core.common.search import get_document_public_visibility_criteria + from elasticsearch_dsl import Q + user = SimpleNamespace(is_authenticated=True, username='Creator') + criterion = get_document_public_visibility_criteria(user, include_creator_private_access=True) + expected = Q('term', public_can_view=True) | ( + Q('term', public_can_view=False) & Q('term', created_by='Creator') + ) + self.assertEqual(criterion.to_dict(), expected.to_dict()) diff --git a/core/graphql/tests/test_query_helpers.py b/core/graphql/tests/test_query_helpers.py index e0148904b..166ce657a 100644 --- a/core/graphql/tests/test_query_helpers.py +++ b/core/graphql/tests/test_query_helpers.py @@ -11,7 +11,7 @@ from strawberry.django.views import AsyncGraphQLView from strawberry.exceptions import GraphQLError -from core.common.constants import HEAD +from core.common.constants import ACCESS_TYPE_NONE, ACCESS_TYPE_VIEW, HEAD from core.common.tests import OCLTestCase from core.concepts.models import Concept from core.concepts.tests.factories import ( @@ -24,14 +24,14 @@ _to_bool, _to_float, apply_slice, - build_base_queryset, + build_global_head_queryset, + build_source_version_queryset, build_datatype, build_global_mapping_prefetch, build_mapping_prefetch, concept_ids_from_es, concepts_for_ids, concepts_for_query, - fallback_db_search, format_datetime_for_api, has_next, normalize_pagination, @@ -47,6 +47,11 @@ serialize_names, with_concept_related, ) +from core.graphql.constants import ( + AUTHENTICATION_FAILED, + FORBIDDEN, + build_expected_graphql_error, +) from core.graphql.schema import schema from core.graphql.tests.conftest import bootstrap_super_user, create_user_with_token from core.graphql.views import AuthenticatedGraphQLView @@ -156,6 +161,14 @@ def test_get_context_handles_session_and_token_states(self): context = async_to_sync(view.get_context)(no_group_token_request) self.assertIsInstance(context.user, AnonymousUser) self.assertEqual(context.auth_status, 'invalid') + def test_schema_process_errors_skips_expected_business_errors(self): + with patch('strawberry.schema.base.StrawberryLogger.error') as error_logger: + schema.process_errors([build_expected_graphql_error(AUTHENTICATION_FAILED)]) + error_logger.assert_not_called() + + with patch('strawberry.schema.base.StrawberryLogger.error') as error_logger: + schema.process_errors([GraphQLError('unexpected boom')]) + error_logger.assert_called_once() class QueryHelperTests(OCLTestCase): @@ -250,7 +263,7 @@ def test_resolve_source_version_and_base_queries(self): ) with patch('core.graphql.queries.Source.get_version', return_value=self.source): success = async_to_sync(resolve_source_version)( - self.organization.mnemonic, self.source.mnemonic, None + self.organization.mnemonic, None, self.source.mnemonic, None ) self.assertEqual(success, self.source) @@ -258,15 +271,15 @@ def test_resolve_source_version_and_base_queries(self): 'core.graphql.queries.Source.find_latest_released_version_by', return_value=fallback_only ): resolved = async_to_sync(resolve_source_version)( - self.organization.mnemonic, fallback_only.mnemonic, None + self.organization.mnemonic, None, fallback_only.mnemonic, None ) self.assertEqual(resolved, fallback_only) with self.assertRaises(GraphQLError): async_to_sync(resolve_source_version)( - self.organization.mnemonic, 'missing-source', 'v-does-not-exist' + self.organization.mnemonic, None, 'missing-source', 'v-does-not-exist' ) - base_qs = build_base_queryset(self.source) + base_qs = build_source_version_queryset(self.source) mapping_prefetch = build_mapping_prefetch(self.source) global_prefetch = build_global_mapping_prefetch() self.assertIsNotNone(mapping_prefetch) @@ -282,12 +295,43 @@ def test_resolve_source_version_and_base_queries(self): related_qs = with_concept_related(base_qs, mapping_prefetch) self.assertGreaterEqual(related_qs.count(), 2) + def test_build_global_mapping_prefetch_filters_private_mappings(self): + private_mapping = MappingFactory( + parent=self.source, + from_concept=self.concept1, + to_concept=self.concept2, + public_access=ACCESS_TYPE_NONE, + created_by=self.audit_user, + updated_by=self.audit_user, + ) + anonymous_qs = with_concept_related( + build_global_head_queryset(), + build_global_mapping_prefetch(AnonymousUser()), + ).filter(id=self.concept1.id) + anonymous_concept = list(anonymous_qs)[0] + self.assertTrue(all( + mapping.public_access != ACCESS_TYPE_NONE for mapping in anonymous_concept.graphql_mappings + )) + + member = UserProfileFactory( + username='graphql-mapping-member', + created_by=self.super_user, + updated_by=self.super_user, + ) + self.organization.members.add(member) + member_qs = with_concept_related( + build_global_head_queryset(), + build_global_mapping_prefetch(member), + ).filter(id=self.concept1.id) + member_concept = list(member_qs)[0] + self.assertTrue(any(mapping.id == private_mapping.id for mapping in member_concept.graphql_mappings)) + def test_resolve_source_version_error_path_and_pagination_defaults(self): with patch('core.graphql.queries.Source.get_version', return_value=None), patch( 'core.graphql.queries.Source.find_latest_released_version_by', return_value=None ): with self.assertRaises(GraphQLError): - async_to_sync(resolve_source_version)('ORG', 'SRC', None) + async_to_sync(resolve_source_version)('ORG', None, 'SRC', None) self.assertIsNone(normalize_pagination(None, None)) self.assertFalse(has_next(10, None)) @@ -300,6 +344,9 @@ def test_serializers_and_resolvers(self): self.concept1.graphql_mappings = [self.mapping] serialized = serialize_concepts([self.concept1])[0] self.assertEqual(serialized.mappings[0].to_code, self.concept2.mnemonic) + self.assertEqual(serialized.mappings[0].to_concept_name, self.concept2.display_name) + self.assertEqual(serialized.mappings[0].sort_weight, self.mapping.sort_weight) + self.assertEqual(serialized.extras, self.concept1.extras) self.assertEqual(serialized.metadata.created_by, self.audit_user.username) self.assertEqual(serialized.description, 'FR description') @@ -419,6 +466,9 @@ def __getitem__(self, key): def params(self, **_kwargs): return self + def extra(self, **_kwargs): + return self + def execute(self): return FakeResponse(self._items, self._total) @@ -433,11 +483,49 @@ def execute(self): with patch('core.graphql.queries.ConceptDocument.search', side_effect=Exception('boom')): self.assertIsNone(concept_ids_from_es('text', self.source, None)) - def test_fallback_and_concepts_queries(self): - base_qs = build_base_queryset(self.source) - self.assertEqual(fallback_db_search(base_qs, ' ').count(), 0) - self.assertIn(self.concept1.id, list(fallback_db_search(base_qs, 'UTIL').values_list('id', flat=True))) + def test_concept_ids_from_es_applies_global_visibility_filter(self): + class RecordingResponse: + def __init__(self): + self.hits = SimpleNamespace(total=SimpleNamespace(value=0)) + + def __iter__(self): + return iter([]) + + class RecordingSearch: + def __init__(self): + self.filters = [] + + def filter(self, *args, **kwargs): + self.filters.append((args, kwargs)) + return self + + def query(self, *_args, **_kwargs): + return self + + def __getitem__(self, _key): + return self + + def params(self, **_kwargs): + return self + + def extra(self, **_kwargs): + return self + def execute(self): + return RecordingResponse() + + anonymous_search = RecordingSearch() + with patch('core.graphql.queries.ConceptDocument.search', return_value=anonymous_search): + concept_ids_from_es('shared', None, None, user=AnonymousUser()) + self.assertTrue( + any( + len(args) == 1 and not kwargs and 'public_can_view' in str(args[0]) + for args, kwargs in anonymous_search.filters + ) + ) + + def test_concepts_queries_behavior(self): + base_qs = build_source_version_queryset(self.source) mapping_prefetch = build_mapping_prefetch(self.source) with self.assertRaises(GraphQLError): async_to_sync(concepts_for_ids)(base_qs, [], normalize_pagination(1, 1), mapping_prefetch) @@ -464,6 +552,17 @@ def test_fallback_and_concepts_queries(self): ) self.assertGreaterEqual(total, 1) + with patch('core.graphql.queries.concept_ids_from_es', return_value=None): + global_concepts, _ = async_to_sync(concepts_for_query)( + build_global_head_queryset(), + 'UTIL', + None, + normalize_pagination(1, 1), + build_global_mapping_prefetch(AnonymousUser()), + user=AnonymousUser(), + ) + self.assertEqual(len(global_concepts), 1) + with patch('core.graphql.queries.concept_ids_from_es', return_value=([], 2)): concepts, total = async_to_sync(concepts_for_query)( base_qs, 'UTIL', self.source, None, mapping_prefetch @@ -472,15 +571,16 @@ def test_fallback_and_concepts_queries(self): self.assertEqual(concepts, []) def test_query_concepts_auth_and_results(self): - info_none = SimpleNamespace(context=SimpleNamespace(auth_status='none')) + info_none = SimpleNamespace(context=SimpleNamespace(auth_status='none', user=AnonymousUser())) with self.assertRaises(GraphQLError): async_to_sync(Query().concepts)(info_none) - info_invalid = SimpleNamespace(context=SimpleNamespace(auth_status='invalid')) - with self.assertRaises(GraphQLError): + info_invalid = SimpleNamespace(context=SimpleNamespace(auth_status='invalid', user=AnonymousUser())) + with self.assertRaises(GraphQLError) as invalid: async_to_sync(Query().concepts)(info_invalid, query='test') + self.assertEqual(invalid.exception.extensions['code'], AUTHENTICATION_FAILED) - info_valid = SimpleNamespace(context=SimpleNamespace(auth_status='valid')) + info_valid = SimpleNamespace(context=SimpleNamespace(auth_status='valid', user=self.audit_user)) with self.assertRaises(GraphQLError): async_to_sync(Query().concepts)(info_valid) @@ -499,12 +599,8 @@ def test_query_concepts_auth_and_results(self): self.assertEqual(result_ids.limit, 1) with patch('core.graphql.queries.concept_ids_from_es', return_value=None): - result_query = async_to_sync(Query().concepts)( - info_valid, - query='UTIL', - ) - self.assertGreaterEqual(result_query.total_count, 1) - self.assertFalse(result_query.has_next_page) + global_fallback = async_to_sync(Query().concepts)(info_valid, query='UTIL') + self.assertGreaterEqual(global_fallback.total_count, 1) with patch('core.graphql.queries.concept_ids_from_es', return_value=([], 2)), patch( 'core.graphql.queries.resolve_source_version', return_value=self.source @@ -513,7 +609,355 @@ def test_query_concepts_auth_and_results(self): self.assertEqual(result_es_empty.total_count, 2) self.assertEqual(result_es_empty.results, []) - with patch('core.graphql.queries.resolve_source_version', return_value=self.source): - result_global = async_to_sync(Query().concepts)(info_valid, query='UTIL') + with patch('core.graphql.queries.concept_ids_from_es', return_value=([self.concept1.id], 1)): + result_global = async_to_sync(Query().concepts)( + info_valid, + query='UTIL', + ) self.assertIsNone(result_global.org) self.assertIsNone(result_global.source) + + def test_query_concepts_enforces_repo_permissions_and_filters_global_results(self): # pylint: disable=too-many-locals + private_org = OrganizationFactory( + mnemonic='PRIVATE', + created_by=self.super_user, + updated_by=self.super_user, + ) + private_source = OrganizationSourceFactory( + organization=private_org, + mnemonic='PRIVATE-SRC', + public_access=ACCESS_TYPE_NONE, + created_by=self.super_user, + updated_by=self.super_user, + ) + private_concept = ConceptFactory( + parent=private_source, + mnemonic='PRIVATE-CONCEPT', + public_access=ACCESS_TYPE_NONE, + created_by=self.audit_user, + updated_by=self.audit_user, + ) + ConceptNameFactory( + concept=private_concept, + name='Shared Visibility', + locale='en', + locale_preferred=True, + ) + + public_org = OrganizationFactory( + mnemonic='PUBLIC', + created_by=self.super_user, + updated_by=self.super_user, + ) + public_source = OrganizationSourceFactory( + organization=public_org, + mnemonic='PUBLIC-SRC', + public_access=ACCESS_TYPE_VIEW, + created_by=self.super_user, + updated_by=self.super_user, + ) + public_concept = ConceptFactory( + parent=public_source, + mnemonic='PUBLIC-CONCEPT', + public_access=ACCESS_TYPE_VIEW, + created_by=self.audit_user, + updated_by=self.audit_user, + ) + ConceptNameFactory( + concept=public_concept, + name='Shared Visibility', + locale='en', + locale_preferred=True, + ) + + outsider = UserProfileFactory( + username='graphql-outsider', + created_by=self.super_user, + updated_by=self.super_user, + ) + member = UserProfileFactory( + username='graphql-member', + created_by=self.super_user, + updated_by=self.super_user, + ) + private_org.members.add(member) + + anonymous_info = SimpleNamespace(context=SimpleNamespace(auth_status='none', user=AnonymousUser())) + outsider_info = SimpleNamespace(context=SimpleNamespace(auth_status='valid', user=outsider)) + member_info = SimpleNamespace(context=SimpleNamespace(auth_status='valid', user=member)) + invalid_info = SimpleNamespace(context=SimpleNamespace(auth_status='invalid', user=AnonymousUser())) + + with self.assertRaises(GraphQLError) as forbidden: + async_to_sync(Query().concepts)( + outsider_info, + org=private_org.mnemonic, + source=private_source.mnemonic, + conceptIds=[private_concept.mnemonic], + ) + self.assertEqual(str(forbidden.exception), 'Forbidden') + self.assertEqual(forbidden.exception.extensions['code'], FORBIDDEN) + + with self.assertRaises(GraphQLError) as invalid_private: + async_to_sync(Query().concepts)( + invalid_info, + org=private_org.mnemonic, + source=private_source.mnemonic, + conceptIds=[private_concept.mnemonic], + ) + self.assertEqual(str(invalid_private.exception), 'Authentication failure') + self.assertEqual(invalid_private.exception.extensions['code'], AUTHENTICATION_FAILED) + + with self.assertRaises(GraphQLError) as invalid_public: + async_to_sync(Query().concepts)( + invalid_info, + org=public_org.mnemonic, + source=public_source.mnemonic, + conceptIds=[public_concept.mnemonic], + ) + self.assertEqual(str(invalid_public.exception), 'Authentication failure') + self.assertEqual(invalid_public.exception.extensions['code'], AUTHENTICATION_FAILED) + + public_repo_result = async_to_sync(Query().concepts)( + anonymous_info, + org=public_org.mnemonic, + source=public_source.mnemonic, + conceptIds=[public_concept.mnemonic], + ) + self.assertEqual(public_repo_result.total_count, 1) + self.assertEqual(public_repo_result.results[0].concept_id, public_concept.mnemonic) + + with patch( + 'core.graphql.queries.concept_ids_from_es', + return_value=([public_concept.id], 1), + ): + anonymous_global = async_to_sync(Query().concepts)(anonymous_info, query='Shared Visibility') + self.assertEqual( + [concept.concept_id for concept in anonymous_global.results], + [public_concept.mnemonic], + ) + + with patch( + 'core.graphql.queries.concept_ids_from_es', + return_value=([private_concept.id, public_concept.id], 2), + ): + member_global = async_to_sync(Query().concepts)(member_info, query='Shared Visibility') + self.assertEqual( + {concept.concept_id for concept in member_global.results}, + {private_concept.mnemonic, public_concept.mnemonic}, + ) + + # ------------------------------------------------------------------ + # Regression tests for blockers caught in code review + # ------------------------------------------------------------------ + + def test_concept_ids_from_es_uses_resolved_version_not_client_label(self): + """B1 regression: when a client asks for an unreleased version and the resolver falls back + to ``find_latest_released_version_by``, the ES filter must follow the resolved Source.version, + not the original client label (which would produce zero hits).""" + + + class RecordingResponse: + def __init__(self): + self.hits = SimpleNamespace(total=SimpleNamespace(value=0)) + + def __iter__(self): + return iter([]) + + class RecordingSearch: + def __init__(self): + self.filters = [] + + def filter(self, *args, **kwargs): + self.filters.append((args, kwargs)) + return self + + def query(self, *_args, **_kwargs): + return self + + def __getitem__(self, _key): + return self + + def params(self, **_kwargs): + return self + + def extra(self, **_kwargs): + return self + + def execute(self): + return RecordingResponse() + + resolved_source = SimpleNamespace( + mnemonic='SRC', + version='v2.0', # what the resolver actually returned + is_head=False, + ) + recording = RecordingSearch() + with patch('core.graphql.queries.ConceptDocument.search', return_value=recording): + concept_ids_from_es('text', resolved_source, None) + + # The source_version filter must use the *resolved* version label, not whatever the + # client originally typed in (the old code used `version_label or HEAD`). + version_filters = [ + (args, kwargs) for args, kwargs in recording.filters + if args == ('term',) and kwargs.get('source_version') == 'v2.0' + ] + self.assertEqual(len(version_filters), 1) + # And it must NOT have applied the is_latest_version=True filter when the resolved + # version is a concrete (non-HEAD) release. + is_latest_filters = [ + (args, kwargs) for args, kwargs in recording.filters + if kwargs.get('is_head') is True + ] + self.assertEqual(is_latest_filters, []) + + def test_concept_ids_from_es_uses_is_latest_for_head_source(self): + """B1 sibling: HEAD sources must filter by the same head identity predicate as the ORM.""" + + class RecordingResponse: + def __init__(self): + self.hits = SimpleNamespace(total=SimpleNamespace(value=0)) + + def __iter__(self): + return iter([]) + + class RecordingSearch: + def __init__(self): + self.filters = [] + + def filter(self, *args, **kwargs): + self.filters.append((args, kwargs)) + return self + + def query(self, *_args, **_kwargs): + return self + + def __getitem__(self, _key): + return self + + def params(self, **_kwargs): + return self + + def extra(self, **_kwargs): + return self + + def execute(self): + return RecordingResponse() + + head_source = SimpleNamespace(mnemonic='SRC', version=HEAD, is_head=True) + recording = RecordingSearch() + with patch('core.graphql.queries.ConceptDocument.search', return_value=recording): + concept_ids_from_es('text', head_source, None) + + is_latest_filters = [ + (args, kwargs) for args, kwargs in recording.filters + if kwargs.get('is_head') is True + ] + self.assertEqual(len(is_latest_filters), 1) + + def test_filter_global_queryset_fails_closed_without_apply_user_criteria(self): + """S1 regression: an authenticated non-staff user must not see ACCESS_TYPE_NONE rows when + the queryset model does not implement ``apply_user_criteria``.""" + from core.graphql.permissions import filter_global_queryset + + class FakeModel: + # Intentionally no apply_user_criteria + pass + + class FakeQuerySet: + def __init__(self): + self.model = FakeModel + self.excluded = None + + def exclude(self, **kwargs): + self.excluded = kwargs + return self + + qs = FakeQuerySet() + non_staff_user = SimpleNamespace(is_anonymous=False, is_staff=False) + filter_global_queryset(qs, non_staff_user) + self.assertEqual(qs.excluded, {'public_access': ACCESS_TYPE_NONE}) + + def test_filter_global_queryset_uses_apply_user_criteria_when_available(self): + """S1 sibling: when the model exposes ``apply_user_criteria`` we delegate to it.""" + from core.graphql.permissions import filter_global_queryset + + sentinel = object() + + class FakeModel: + @staticmethod + def apply_user_criteria(qs, user): # pylint: disable=unused-argument + return sentinel + + class FakeQuerySet: + model = FakeModel + + def exclude(self, **_kwargs): # pragma: no cover - must not be called + raise AssertionError('exclude must not be called when apply_user_criteria exists') + + non_staff_user = SimpleNamespace(is_anonymous=False, is_staff=False) + self.assertIs(filter_global_queryset(FakeQuerySet(), non_staff_user), sentinel) + + def test_filter_global_queryset_staff_sees_everything(self): + """S1 sibling: staff users bypass visibility filters entirely.""" + from core.graphql.permissions import filter_global_queryset + + class FakeQuerySet: + model = object # never reached + + def exclude(self, **_kwargs): # pragma: no cover + raise AssertionError('staff path must not filter') + + staff_user = SimpleNamespace(is_anonymous=False, is_staff=True) + qs = FakeQuerySet() + self.assertIs(filter_global_queryset(qs, staff_user), qs) + + def test_validation_errors_carry_validation_error_code(self): + """All client-side validation failures must surface a stable VALIDATION_ERROR code so + clients can branch on it and the schema's process_errors can suppress server-error logs.""" + from core.graphql.constants import VALIDATION_ERROR + + info_valid = SimpleNamespace(context=SimpleNamespace(auth_status='valid', user=self.audit_user)) + + # 1. Neither conceptIds nor query + with self.assertRaises(GraphQLError) as missing_args: + async_to_sync(Query().concepts)(info_valid) + self.assertEqual(missing_args.exception.extensions['code'], VALIDATION_ERROR) + + # 2. Both org and owner + with self.assertRaises(GraphQLError) as both_owners: + async_to_sync(Query().concepts)( + info_valid, org='X', owner='Y', source='S', query='q' + ) + self.assertEqual(both_owners.exception.extensions['code'], VALIDATION_ERROR) + + # 3. Source without org/owner + with self.assertRaises(GraphQLError) as orphan_source: + async_to_sync(Query().concepts)(info_valid, source='S', query='q') + self.assertEqual(orphan_source.exception.extensions['code'], VALIDATION_ERROR) + + # 4. Pagination out of range + with self.assertRaises(GraphQLError) as bad_page: + async_to_sync(Query().concepts)(info_valid, query='q', page=0, limit=1) + self.assertEqual(bad_page.exception.extensions['code'], VALIDATION_ERROR) + + def test_validation_errors_are_suppressed_from_server_error_log(self): + """VALIDATION_ERROR codes must be in EXPECTED_GRAPHQL_ERROR_CODES so schema.process_errors + does not log them as unexpected server errors.""" + from core.graphql.constants import VALIDATION_ERROR, build_validation_error, EXPECTED_GRAPHQL_ERROR_CODES + + self.assertIn(VALIDATION_ERROR, EXPECTED_GRAPHQL_ERROR_CODES) + + with patch('strawberry.schema.base.StrawberryLogger.error') as error_logger: + schema.process_errors([build_validation_error('bad input')]) + error_logger.assert_not_called() + + def test_resolve_source_version_error_does_not_leak_owner(self): + """S7 regression: when the source is not found, the error must not differentiate between + a missing repo and a missing owner.""" + with patch('core.graphql.queries.Source.get_version', return_value=None), patch( + 'core.graphql.queries.Source.find_latest_released_version_by', return_value=None + ): + with self.assertRaises(GraphQLError) as missing: + async_to_sync(resolve_source_version)('ORG', None, 'SRC', None) + # Message must be the generic form — no "for org 'ORG'" suffix. + self.assertEqual(str(missing.exception), "Source 'SRC' with version 'HEAD' was not found.") diff --git a/core/graphql/tests/test_sources.py b/core/graphql/tests/test_sources.py new file mode 100644 index 000000000..236b1bbba --- /dev/null +++ b/core/graphql/tests/test_sources.py @@ -0,0 +1,204 @@ +"""Source MVP aggregates and permissions through the executable Strawberry schema.""" + +from types import SimpleNamespace +from unittest.mock import patch + +from asgiref.sync import async_to_sync +from django.contrib.auth.models import AnonymousUser + +from core.common.constants import ACCESS_TYPE_NONE, HEAD +from core.common.tests import OCLTestCase +from core.concepts.tests.factories import ConceptFactory +from core.graphql.schema import schema +from core.mappings.tests.factories import MappingFactory +from core.orgs.tests.factories import OrganizationFactory +from core.sources.tests.factories import OrganizationSourceFactory, UserSourceFactory +from core.users.tests.factories import UserProfileFactory + + +class SourceQueryTests(OCLTestCase): + """Use real version-scoped ORM queries while isolating the optional ES shortcut.""" + + def setUp(self): + """Create public and private sources with representative versioned contents.""" + super().setUp() + projection = patch('core.graphql.queries.indexed_source', return_value=None) + projection.start() + self.addCleanup(projection.stop) + self.org = OrganizationFactory(mnemonic='GRAPHQL') + self.source = OrganizationSourceFactory( + organization=self.org, mnemonic='Dictionary', name='Dictionary', canonical_url='https://example.org/codes', + ) + self.concept = ConceptFactory(parent=self.source, concept_class='Diagnosis', datatype='Numeric') + ConceptFactory(parent=self.source, concept_class='Test', datatype='Text') + ConceptFactory(parent=self.source, concept_class='Ignored', datatype='Ignored', retired=True) + self.target = OrganizationSourceFactory(name='External', mnemonic='External') + self.mapping = MappingFactory( + parent=self.source, from_concept=self.concept, to_source=self.target, map_type='SAME-AS', + ) + MappingFactory(parent=self.source, from_concept=self.concept, to_source=self.target, map_type='SAME-AS') + MappingFactory(parent=self.source, from_concept=self.concept, retired=True, map_type='RETIRED') + + def execute(self, fields, user=None, source=None, version=None, owner=None): # pylint: disable=too-many-arguments + """Execute source queries with an explicit principal and fresh request cache.""" + source = source or self.source + return async_to_sync(schema.execute)( + 'query($org: String, $owner: String, $source: String!, $version: String) {' + ' source(org: $org, owner: $owner, source: $source, version: $version) {' + fields + '} }', + variable_values={ + 'org': None if owner else source.organization.mnemonic, 'owner': owner, + 'source': source.mnemonic, 'version': version, + }, + context_value=SimpleNamespace(user=user or AnonymousUser(), auth_status='valid' if user else 'none'), + ) + + def test_full_mvp_counts_unique_labels_and_targets(self): + """Retired records and duplicate labels/targets do not inflate the MVP output.""" + result = self.execute('name description canonicalUrl uri mapTypes externalSources { name url } ' + 'classes datatypes summary { activeConcepts mappings }') + self.assertIsNone(result.errors) + data = result.data['source'] + self.assertEqual(data['summary'], {'activeConcepts': 2, 'mappings': 2}) + self.assertEqual(data['classes'], ['Diagnosis', 'Test']) + self.assertEqual(data['datatypes'], ['Numeric', 'Text']) + self.assertEqual(data['mapTypes'], ['SAME-AS']) + self.assertEqual(data['externalSources'], [{'name': self.target.name, 'url': self.target.uri}]) + self.assertEqual(data['canonicalUrl'], self.source.canonical_url) + self.assertEqual(data['uri'], self.source.uri) + + def test_summary_selection_does_not_query_unrequested_children(self): + """Requesting only concept count never touches mappings or their targets.""" + with patch('core.sources.models.Source.get_mappings_queryset', side_effect=AssertionError('mappings')): + result = self.execute('summary { activeConcepts }') + self.assertIsNone(result.errors) + self.assertEqual(result.data['source']['summary'], {'activeConcepts': 2}) + + def test_metadata_fallback_does_not_load_any_children(self): + """ES failures still leave metadata-only ORM queries small.""" + with patch('core.sources.models.Source.get_mappings_queryset', side_effect=AssertionError('mappings')), \ + patch('core.sources.models.Source.get_concepts_queryset', side_effect=AssertionError('concepts')): + result = self.execute('name uri') + self.assertIsNone(result.errors) + + def test_release_summary_uses_membership_not_head_records(self): + """A release uses its own concept and mapping membership.""" + release = OrganizationSourceFactory( + organization=self.org, mnemonic=self.source.mnemonic, version='v1', released=True, + ) + self.concept.sources.add(release) + self.mapping.sources.add(release) + result = self.execute('uri classes summary { activeConcepts mappings }', version='v1') + self.assertIsNone(result.errors) + self.assertEqual(result.data['source']['summary'], {'activeConcepts': 1, 'mappings': 1}) + self.assertEqual(result.data['source']['uri'], release.uri) + + def test_private_source_permissions_cover_anonymous_outsider_member_and_staff(self): + """Only organization members and staff can read a private organization's source.""" + self.source.public_access = ACCESS_TYPE_NONE + self.source.save() + outsider = UserProfileFactory() + member = UserProfileFactory() + self.org.members.add(member) + staff = UserProfileFactory(is_staff=True) + for user, allowed in ((None, False), (outsider, False), (member, True), (staff, True)): + with self.subTest(user=getattr(user, 'username', 'anonymous')): + result = self.execute('name summary { mappings }', user=user) + if allowed: + self.assertIsNone(result.errors) + else: + self.assertEqual(result.errors[0].extensions['code'], 'FORBIDDEN') + self.assertIsNone(result.data) + + def test_personal_private_source_owner_has_access(self): + """Private personal repositories use user ownership rather than organization IDs.""" + owner = UserProfileFactory() + private = UserSourceFactory(user=owner, public_access=ACCESS_TYPE_NONE) + for user, allowed in ((owner, True), (UserProfileFactory(), False), (None, False)): + result = self.execute('name', source=private, user=user, owner=owner.username) + self.assertEqual(result.errors is None, allowed) + + def test_external_sources_do_not_disclose_private_targets(self): + """A public mapping cannot expose metadata about a linked private target repository.""" + self.target.public_access = ACCESS_TYPE_NONE + self.target.save() + result = self.execute('externalSources { name url }') + self.assertIsNone(result.errors) + self.assertEqual(result.data['source']['externalSources'], []) + + def test_missing_source_and_explicit_version_fail(self): + """Missing explicit versions are not silently replaced with HEAD.""" + result = self.execute('name', version='missing') + self.assertIsNotNone(result.errors) + self.assertIn('was not found', result.errors[0].message) + + def test_empty_repository_has_empty_aggregates(self): + """Empty sources return zero counts and lists, never fabricated labels.""" + empty = OrganizationSourceFactory(organization=self.org, version=HEAD) + result = self.execute('classes datatypes mapTypes summary { activeConcepts mappings }', source=empty) + self.assertIsNone(result.errors) + self.assertEqual(result.data['source'], { + 'classes': [], 'datatypes': [], 'mapTypes': [], 'summary': {'activeConcepts': 0, 'mappings': 0}, + }) + + def test_external_targets_resolved_through_concept_also_obey_permissions(self): + """Target concepts must not bypass repository visibility when to_source is absent.""" + target_concept = ConceptFactory(parent=self.target) + MappingFactory(parent=self.source, from_concept=self.concept, to_concept=target_concept, to_source=None) + self.target.public_access = ACCESS_TYPE_NONE + self.target.save() + result = self.execute('externalSources { name url }') + self.assertIsNone(result.errors) + self.assertEqual(result.data['source']['externalSources'], []) + + def test_typename_only_summary_and_datatype_details(self): + """Introspection-style selections still instantiate requested nested objects.""" + result = self.execute('summary { __typename }') + self.assertIsNone(result.errors) + self.assertEqual(result.data['source'], {'summary': {'__typename': 'SourceSummaryType'}}) + self.concept.extras = {'units': 'mg'} + self.concept.save() + with patch('core.graphql.queries.indexed_concepts', return_value=None): + result = async_to_sync(schema.execute)( + '{ concepts(org: "GRAPHQL", source: "Dictionary", conceptIds: ["' + self.concept.mnemonic + '"]) ' + '{ results { datatype { details { __typename } } metadata { __typename } } } }', + context_value=SimpleNamespace(user=AnonymousUser(), auth_status='none'), + ) + self.assertIsNone(result.errors) + self.assertEqual(result.data['concepts']['results'][0]['datatype']['details'], { + '__typename': 'NumericDatatypeDetails', + }) + + def test_parent_permission_changes_update_projection_fields(self): + """Source ACL propagation refreshes the indexed parent flag as well as the child flag.""" + self.source.public_access = ACCESS_TYPE_NONE + self.source._should_update_public_access = True # pylint: disable=protected-access + with patch.object(type(self.source), 'batch_index') as index: + self.source.save() + concept_update = next(call for call in index.call_args_list if call.args[1].Index.name == 'concepts') + self.assertEqual(concept_update.kwargs['partial_doc'], { + 'public_can_view': False, 'parent_public_can_view': False, + }) + + def test_parent_deactivation_updates_indexed_concept_flag(self): + """Deactivated parent repositories cannot leave active concept projections behind.""" + self.source.is_active = False + self.source._should_update_is_active = True # pylint: disable=protected-access + with patch.object(type(self.source), 'batch_index') as index: + self.source.save() + concept_update = next(call for call in index.call_args_list if call.args[1].Index.name == 'concepts') + self.assertEqual(concept_update.kwargs['partial_doc'], {'is_active': False}) + + def test_repository_and_owner_id_collisions_do_not_grant_private_access(self): + """Numeric IDs from distinct ownership tables must not confer access to another owner's source.""" + from core.common.permissions import CanViewConceptDictionary, user_can_view_concept_dictionary + member = UserProfileFactory() + self.org.members.add(member) + # Simulate independent sequences colliding, without changing fixture primary keys. + repository = SimpleNamespace( + public_access=ACCESS_TYPE_NONE, id=self.org.id, user_id=member.id + 100, + organization_id=None, parent_id=self.org.id, resource_type='Source', + ) + self.assertFalse(user_can_view_concept_dictionary(member, repository)) + self.assertFalse(CanViewConceptDictionary().has_object_permission( + SimpleNamespace(user=member), None, repository, + )) diff --git a/core/graphql/types.py b/core/graphql/types.py index 82fe181e2..089b1a544 100644 --- a/core/graphql/types.py +++ b/core/graphql/types.py @@ -3,6 +3,7 @@ from typing import Annotated, List, Optional, Union import strawberry +from strawberry.scalars import JSON @strawberry.type @@ -34,6 +35,14 @@ class MappingType: name="toCode", description="Identifier of the target concept in the mapped source.", ) + to_concept_name: Optional[str] = strawberry.field( + name="toConceptName", + description="Display name of the target concept when available.", + ) + sort_weight: Optional[float] = strawberry.field( + name="sortWeight", + description="Numeric weight used to order mappings within the same mapping type.", + ) comment: Optional[str] = strawberry.field(description="Optional notes attached to the mapping.") @@ -130,7 +139,7 @@ class MetadataType: class ConceptType: id: strawberry.ID = strawberry.field( name="id", - description="CIEL concept identifier (mirrors the numeric ID used internally).", + description="OCL concept record identifier (the database primary key).", ) external_id: Optional[str] = strawberry.field( name="externalId", @@ -162,3 +171,42 @@ class ConceptType: metadata: MetadataType = strawberry.field( description="Operational metadata such as status and audit fields." ) + extras: JSON = strawberry.field(description="Additional custom metadata attached to the concept.") + + +@strawberry.type(description="Counts of active, non-retired records in the selected source version.") +class SourceSummaryType: + """Minimal version-scoped repository summary.""" + + active_concepts: int = strawberry.field(description="Number of active, non-retired concepts.", default=0) + mappings: int = strawberry.field(description="Number of active, non-retired mappings.", default=0) + + +@strawberry.type(description="Source metadata and statistics for one OCL repository version.") +class SourceType: + """Selectable source metadata with explicitly requested aggregates.""" + + name: Optional[str] = strawberry.field(description="Human-readable source name.", default=None) + description: Optional[str] = strawberry.field(description="Source description.", default=None) + canonical_url: Optional[str] = strawberry.field(description="Canonical source URL.", default=None) + uri: Optional[str] = strawberry.field( + description="OCL relative URI, e.g. /orgs/CIEL/sources/CIEL/; includes the version for releases.", + default=None, + ) + map_types: List[str] = strawberry.field( + description="Distinct map types used by active, non-retired mappings in this version.", default_factory=list, + ) + external_sources: List[ToSourceType] = strawberry.field( + description="Visible external target sources referenced by active, non-retired outbound mappings.", + default_factory=list, + ) + classes: List[str] = strawberry.field( + description="Distinct concept classes used by active, non-retired concepts in this version.", + default_factory=list, + ) + datatypes: List[str] = strawberry.field( + description="Distinct datatypes used by active, non-retired concepts in this version.", default_factory=list, + ) + summary: SourceSummaryType = strawberry.field( + description="Counts computed only for the selected summary fields.", default_factory=SourceSummaryType, + ) diff --git a/core/integration_tests/test_graphql_projection.py b/core/integration_tests/test_graphql_projection.py new file mode 100644 index 000000000..3ec25f7de --- /dev/null +++ b/core/integration_tests/test_graphql_projection.py @@ -0,0 +1,156 @@ +"""Real Elasticsearch verification for GraphQL projection semantics and SQL avoidance.""" + +from types import SimpleNamespace +from unittest import skipUnless +from unittest.mock import patch +from uuid import uuid4 + +from asgiref.sync import async_to_sync +from django.conf import settings +from django.contrib.auth.models import AnonymousUser +from elasticsearch_dsl.connections import connections + +from core.common.constants import ACCESS_TYPE_NONE +from core.common.tests import OCLTestCase +from core.concepts.documents import ConceptDocument +from core.concepts.tests.factories import ConceptDescriptionFactory, ConceptFactory, ConceptNameFactory +from core.graphql.schema import schema +from core.sources.documents import SourceDocument +from core.sources.tests.factories import OrganizationSourceFactory +from core.users.tests.factories import UserProfileFactory + + +@skipUnless(getattr(settings, 'ES_ENABLED', False), 'Requires Elasticsearch') +class GraphQLProjectionIntegrationTests(OCLTestCase): + """Use temporary index names and real indexed ORM fixtures, without touching shared indexes.""" + + def setUp(self): + """Prepare dedicated indexes; redirect GraphQL searches only for this test.""" + super().setUp() + self.connection = connections.get_connection() + suffix = uuid4().hex + self.indexes = {} + for document in (ConceptDocument, SourceDocument): + index = document._index.clone(f'graphql-test-{document.Index.name}-{suffix}') # pylint: disable=protected-access + index.create() + self.addCleanup(index.delete) + self.indexes[document] = index._name # pylint: disable=protected-access + search = document.search().index().index(index._name) # pylint: disable=protected-access + patcher = patch.object(document, 'search', side_effect=lambda search=search: search) + patcher.start() + self.addCleanup(patcher.stop) + self.source = OrganizationSourceFactory(mnemonic='CASE-Sensitive', name='Clinical dictionary') + self.concept = ConceptFactory(parent=self.source, mnemonic='AbC', datatype='Numeric', concept_class='Diagnosis') + ConceptNameFactory(concept=self.concept, name='Hypertension', locale='en', locale_preferred=True) + ConceptDescriptionFactory(concept=self.concept, name='Preferred definition', locale='en', locale_preferred=True) + self.index(self.source, SourceDocument) + self.index(self.concept, ConceptDocument) + + def index(self, instance, document): + """Index the actual Document preparation output, including the additive projection fields.""" + self.connection.index( + index=self.indexes[document], id=instance.id, document=document().prepare(instance), refresh=True, + ) + + def execute(self, query, user=None): + """Use a fresh schema context so membership caches cannot cross principals.""" + return async_to_sync(schema.execute)( + query, context_value=SimpleNamespace(user=user or AnonymousUser(), auth_status='valid' if user else 'none'), + ) + + def test_source_and_scoped_concepts_execute_without_sql(self): + """Real source and concept lookups require no SQL after anonymous context creation.""" + query = '''{ + source(org: "%s", source: "%s") { name description canonicalUrl uri } + concepts(org: "%s", source: "%s", query: "Hypertension") { + versionResolved totalCount results { conceptId display description datatype { name } conceptClass } + } + }''' % (self.source.organization.mnemonic, self.source.mnemonic, + self.source.organization.mnemonic, self.source.mnemonic) + with self.assertNumQueries(0): + result = self.execute(query) + self.assertIsNone(result.errors) + self.assertEqual(result.data['source']['name'], self.source.name) + self.assertEqual(result.data['source']['uri'], self.source.uri) + self.assertEqual(result.data['concepts']['totalCount'], 1) + self.assertEqual(result.data['concepts']['results'], [{ + 'conceptId': 'AbC', 'display': 'Hypertension', 'description': 'Preferred definition', + 'datatype': {'name': 'Numeric'}, 'conceptClass': 'Diagnosis', + }]) + + def test_global_projection_uses_head_and_excludes_retired_and_inactive(self): + """Global counts omit historical versions and inactive/retired documents.""" + for fields in ({'retired': True}, {'is_active': False}): + excluded = ConceptFactory(parent=self.source, **fields) + ConceptNameFactory(concept=excluded, name='Hypertension', locale='en', locale_preferred=True) + self.index(excluded, ConceptDocument) + self.index(self.concept.get_latest_version(), ConceptDocument) + with self.assertNumQueries(0): + result = self.execute('{ concepts(query: "Hypertension") { totalCount results { conceptId } } }') + self.assertIsNone(result.errors) + self.assertEqual(result.data['concepts']['totalCount'], 1) + + def test_private_parent_is_hidden_even_if_child_flag_is_public(self): + """A public child flag cannot disclose records from a private repository.""" + self.source.public_access = ACCESS_TYPE_NONE + self.source.save() + self.index(self.source, SourceDocument) + self.index(self.concept, ConceptDocument) + with self.assertNumQueries(0): + result = self.execute('{ concepts(query: "Hypertension") { totalCount results { display } } }') + self.assertIsNone(result.errors) + self.assertEqual(result.data['concepts']['totalCount'], 0) + member = UserProfileFactory() + self.source.organization.members.add(member) + result = self.execute('{ concepts(query: "Hypertension") { totalCount results { display } } }', member) + self.assertIsNone(result.errors) + self.assertEqual(result.data['concepts']['totalCount'], 1) + + def test_two_owners_with_same_source_mnemonic_do_not_mix(self): + """Repository identity includes owner and owner type, not only the source mnemonic.""" + other = OrganizationSourceFactory(mnemonic=self.source.mnemonic) + concept = ConceptFactory(parent=other, mnemonic='Other') + self.index(concept, ConceptDocument) + query = ('{ concepts(org: "%s", source: "%s", conceptIds: ["AbC", "Other"]) ' + '{ totalCount results { conceptId } } }') % ( + self.source.organization.mnemonic, self.source.mnemonic, + ) + with self.assertNumQueries(0): + result = self.execute(query) + self.assertIsNone(result.errors) + self.assertEqual(result.data['concepts']['results'], [{'conceptId': 'AbC'}]) + + def test_release_projection_matches_membership(self): + """Explicit releases search source_version membership instead of HEAD flags.""" + release = OrganizationSourceFactory( + organization=self.source.organization, mnemonic=self.source.mnemonic, version='v1', released=True, + ) + historical = self.concept.get_latest_version() + historical.sources.add(release) + self.index(release, SourceDocument) + self.index(historical, ConceptDocument) + query = ('{ concepts(org: "%s", source: "%s", version: "v1", conceptIds: ["AbC"]) ' + '{ versionResolved results { id } } }') % ( + self.source.organization.mnemonic, self.source.mnemonic, + ) + with self.assertNumQueries(0): + result = self.execute(query) + self.assertIsNone(result.errors) + self.assertEqual(result.data['concepts']['results'], [{'id': str(historical.id)}]) + + def test_hydrated_payload_uses_same_head_and_selected_columns(self): + """Requesting names hydrates the matching HEAD without selecting unused concept extras.""" + from django.db import connection + from django.test.utils import CaptureQueriesContext + query = ('{ concepts(org: "%s", source: "%s", query: "Hypertension") ' + '{ totalCount results { id names { name } } } }') % ( + self.source.organization.mnemonic, self.source.mnemonic, + ) + with CaptureQueriesContext(connection) as queries: + result = self.execute(query) + self.assertIsNone(result.errors) + self.assertEqual(result.data['concepts']['totalCount'], 1) + self.assertEqual(result.data['concepts']['results'][0]['id'], str(self.concept.id)) + self.assertEqual(result.data['concepts']['results'][0]['names'], [{'name': 'Hypertension'}]) + self.assertTrue(queries.captured_queries) + self.assertFalse(any('"concepts"."extras"' in query['sql'] for query in queries.captured_queries)) diff --git a/core/sources/documents.py b/core/sources/documents.py index d8f79d533..5718242df 100644 --- a/core/sources/documents.py +++ b/core/sources/documents.py @@ -14,6 +14,11 @@ class Index: name = 'sources' settings = {'number_of_shards': 1, 'number_of_replicas': 0} + # Additive projection fields; existing indexes require a source reindex. + description = fields.TextField(attr='description') + uri = fields.KeywordField(attr='uri') + is_active = fields.BooleanField(attr='is_active') + locale = fields.ListField(fields.KeywordField()) last_update = fields.DateField(attr='updated_at') updated_by = fields.KeywordField(attr='updated_by.username') diff --git a/core/sources/signals.py b/core/sources/signals.py index 1d581f329..1dfc2ad72 100644 --- a/core/sources/signals.py +++ b/core/sources/signals.py @@ -10,8 +10,11 @@ def propagate_parent_attributes(sender, instance=None, created=False, **kwargs): if created: instance.record_create_event() if not created and instance: + from core.concepts.documents import ConceptDocument if get(instance, '_should_update_is_active'): instance.concepts_set.exclude(is_active=instance.is_active).update(is_active=instance.is_active) + # Direct GraphQL hits use this flag without reloading the concept from the database. + instance.batch_index(instance.concepts_set, ConceptDocument, partial_doc={'is_active': instance.is_active}) instance.mappings_set.exclude(is_active=instance.is_active).update(is_active=instance.is_active) if get(instance, '_should_update_public_access'): @@ -21,9 +24,11 @@ def propagate_parent_attributes(sender, instance=None, created=False, **kwargs): public_access=instance.public_access).update(public_access=instance.public_access) partial_doc = {'public_can_view': instance.public_can_view} - if updated_concepts: - from core.concepts.documents import ConceptDocument - instance.batch_index(instance.concepts_set, ConceptDocument, partial_doc=partial_doc) + if updated_concepts or instance.concepts_set.exists(): + instance.batch_index( + instance.concepts_set, ConceptDocument, + partial_doc={**partial_doc, 'parent_public_can_view': instance.public_can_view}, + ) if updated_mappings: from core.mappings.documents import MappingDocument instance.batch_index(instance.mappings_set, MappingDocument, partial_doc=partial_doc) From 091bfb0377c8cae4b768318006475cd11f2b8d73 Mon Sep 17 00:00:00 2001 From: Filipe Lopes Date: Mon, 7 Sep 2026 17:37:13 -0300 Subject: [PATCH 2/6] feat(graphql): support display name for concept projections # * Update ConceptDocument to use `display_name` field instead of `name` for GraphQL projections * Ensure GraphQL resolvers and tests correctly utilize the `display_name` field in the Elasticsearch index * This change allows clients to retrieve the human-readable display name instead of the internal name when fetching concepts via GraphQL --- core/concepts/documents.py | 1 + core/graphql/README.md | 4 ++-- core/graphql/indexed.py | 4 ++-- core/graphql/sources.py | 8 ++++---- core/graphql/tests/test_projection.py | 10 ++++++---- core/graphql/tests/test_sources.py | 8 ++++---- core/graphql/types.py | 10 +++++++++- core/integration_tests/test_graphql_projection.py | 8 ++++---- 8 files changed, 32 insertions(+), 21 deletions(-) diff --git a/core/concepts/documents.py b/core/concepts/documents.py index e03ed07b7..e04d8f19c 100644 --- a/core/concepts/documents.py +++ b/core/concepts/documents.py @@ -17,6 +17,7 @@ class Index: parent_public_can_view = fields.BooleanField(attr='parent.public_can_view') is_head = fields.BooleanField() preferred_description = fields.TextField() + display_name = fields.TextField(attr='display_name') id = fields.TextField(attr='mnemonic') id_lowercase = fields.KeywordField(attr='mnemonic', normalizer="lowercase") diff --git a/core/graphql/README.md b/core/graphql/README.md index dfed162cc..85d13bdec 100644 --- a/core/graphql/README.md +++ b/core/graphql/README.md @@ -21,7 +21,7 @@ query Dictionary($org: String!, $source: String!, $version: String) { classes datatypes mapTypes - externalSources { name url } + externalSources { name uri } summary { activeConcepts mappings } } } @@ -66,7 +66,7 @@ The older hydrated text-search path retains its empty-index database fallback. Counts and distinct labels use active, non-retired records. `summary.mappings` counts active, non-retired mappings. `externalSources` is the deduplicated set of outbound target repositories, excluding the current source and linked -private targets the caller cannot view. Unresolved external URLs are taken from visible mappings. +private targets the caller cannot view. Unresolved external URIs are taken from visible mappings. Repository permission checks reuse the shared REST visibility rule directly, without fabricated requests. Both owner mnemonic and owner type scope index lookups. Global concepts also enforce parent repository visibility, diff --git a/core/graphql/indexed.py b/core/graphql/indexed.py index 6b2cf3e92..65e67c611 100644 --- a/core/graphql/indexed.py +++ b/core/graphql/indexed.py @@ -22,7 +22,7 @@ 'id': (), # Elasticsearch's metadata ID is the OCL database primary key. 'conceptId': ('id',), 'externalId': ('external_id',), - 'display': ('name',), + 'display': ('display_name',), 'description': ('preferred_description',), 'conceptClass': ('concept_class',), 'datatype.name': ('datatype',), @@ -112,7 +112,7 @@ def serialize_indexed_concept(hit): datatype = getattr(hit, 'datatype', None) return ConceptType( id=str(hit.meta.id), concept_id=getattr(hit, 'id', ''), - external_id=getattr(hit, 'external_id', None), display=getattr(hit, 'name', None) or None, + external_id=getattr(hit, 'external_id', None), display=getattr(hit, 'display_name', None), description=getattr(hit, 'preferred_description', None), concept_class=getattr(hit, 'concept_class', None), datatype=DatatypeType(name=datatype, details=None) if datatype else None, names=[], mappings=[], metadata=None, extras={}, diff --git a/core/graphql/sources.py b/core/graphql/sources.py index 0a56114bb..4ec871c00 100644 --- a/core/graphql/sources.py +++ b/core/graphql/sources.py @@ -2,7 +2,7 @@ from core.common.permissions import user_can_view_concept_dictionary -from .types import SourceType, ToSourceType +from .types import ExternalSourceType, SourceType SOURCE_INDEX_FIELDS = { '__typename': (), @@ -61,8 +61,8 @@ def external_sources(mappings, instance, user): continue if not user_can_view_concept_dictionary(user, target): continue - url = mapping.to_source_url or (target.uri if target else None) + uri = mapping.to_source_url or (target.uri if target else None) name = target.name if target else mapping.to_source_name - if url or name: - result[(url or '', name or '')] = ToSourceType(url=url, name=name) + if uri or name: + result[(uri or '', name or '')] = ExternalSourceType(uri=uri, name=name) return [result[key] for key in sorted(result)] diff --git a/core/graphql/tests/test_projection.py b/core/graphql/tests/test_projection.py index 4ee6b4fef..1b3ff3280 100644 --- a/core/graphql/tests/test_projection.py +++ b/core/graphql/tests/test_projection.py @@ -53,7 +53,7 @@ def test_source_metadata_has_no_sql_and_projects_selected_fields(self): def test_concept_fragments_aliases_and_directives_remain_sql_free(self): """Skipped heavy fields do not force ORM loading, including named fragments.""" response = self.response('concepts', [{ - 'id': '123', 'name': 'Hypertension', 'datatype': 'Numeric', + 'id': '123', 'display_name': 'Hypertension-test', 'datatype': 'Numeric', 'concept_class': 'Diagnosis', 'preferred_description': 'Preferred definition', }]) query = '''query($heavy: Boolean!, $light: Boolean!) { @@ -70,11 +70,13 @@ def test_concept_fragments_aliases_and_directives_remain_sql_free(self): result = self.execute(query, {'heavy': False, 'light': True}) self.assertIsNone(result.errors) self.assertEqual(result.data['found']['results'][0], { - 'code': '123', 'label': 'Hypertension', 'datatype': {'name': 'Numeric'}, + 'code': '123', 'label': 'Hypertension-test', 'datatype': {'name': 'Numeric'}, 'conceptClass': 'Diagnosis', 'description': 'Preferred definition', }) body = execute.call_args.args[0].to_dict() - self.assertEqual(set(body['_source']), {'id', 'name', 'datatype', 'concept_class', 'preferred_description'}) + self.assertEqual(set(body['_source']), { + 'id', 'display_name', 'datatype', 'concept_class', 'preferred_description', + }) self.assertNotIn('extras', body['_source']) self.assertIn('is_head', str(body['query'])) @@ -105,7 +107,7 @@ def test_two_aliases_plan_fields_independently(self): b: concepts(query: "b") { results { display } } }''') self.assertIsNone(result.errors) - self.assertEqual([call.args[0].to_dict()['_source'] for call in es.call_args_list], [['id'], ['name']]) + self.assertEqual([call.args[0].to_dict()['_source'] for call in es.call_args_list], [['id'], ['display_name']]) def test_invalid_auth_stops_all_queries(self): """Schema-level auth failure precedes both source and concept data access.""" diff --git a/core/graphql/tests/test_sources.py b/core/graphql/tests/test_sources.py index 236b1bbba..e4f2c9b3c 100644 --- a/core/graphql/tests/test_sources.py +++ b/core/graphql/tests/test_sources.py @@ -54,7 +54,7 @@ def execute(self, fields, user=None, source=None, version=None, owner=None): # def test_full_mvp_counts_unique_labels_and_targets(self): """Retired records and duplicate labels/targets do not inflate the MVP output.""" - result = self.execute('name description canonicalUrl uri mapTypes externalSources { name url } ' + result = self.execute('name description canonicalUrl uri mapTypes externalSources { name uri } ' 'classes datatypes summary { activeConcepts mappings }') self.assertIsNone(result.errors) data = result.data['source'] @@ -62,7 +62,7 @@ def test_full_mvp_counts_unique_labels_and_targets(self): self.assertEqual(data['classes'], ['Diagnosis', 'Test']) self.assertEqual(data['datatypes'], ['Numeric', 'Text']) self.assertEqual(data['mapTypes'], ['SAME-AS']) - self.assertEqual(data['externalSources'], [{'name': self.target.name, 'url': self.target.uri}]) + self.assertEqual(data['externalSources'], [{'name': self.target.name, 'uri': self.target.uri}]) self.assertEqual(data['canonicalUrl'], self.source.canonical_url) self.assertEqual(data['uri'], self.source.uri) @@ -121,7 +121,7 @@ def test_external_sources_do_not_disclose_private_targets(self): """A public mapping cannot expose metadata about a linked private target repository.""" self.target.public_access = ACCESS_TYPE_NONE self.target.save() - result = self.execute('externalSources { name url }') + result = self.execute('externalSources { name uri }') self.assertIsNone(result.errors) self.assertEqual(result.data['source']['externalSources'], []) @@ -146,7 +146,7 @@ def test_external_targets_resolved_through_concept_also_obey_permissions(self): MappingFactory(parent=self.source, from_concept=self.concept, to_concept=target_concept, to_source=None) self.target.public_access = ACCESS_TYPE_NONE self.target.save() - result = self.execute('externalSources { name url }') + result = self.execute('externalSources { name uri }') self.assertIsNone(result.errors) self.assertEqual(result.data['source']['externalSources'], []) diff --git a/core/graphql/types.py b/core/graphql/types.py index 089b1a544..b9d43d523 100644 --- a/core/graphql/types.py +++ b/core/graphql/types.py @@ -21,6 +21,14 @@ class ToSourceType: name: Optional[str] = strawberry.field(description="Human-readable name for the target source.") +@strawberry.type +class ExternalSourceType: + """GraphQL metadata for a source referenced by an outbound mapping.""" + + uri: Optional[str] = strawberry.field(description="URI identifying the target source.") + name: Optional[str] = strawberry.field(description="Human-readable name for the target source.") + + @strawberry.type class MappingType: map_type: str = strawberry.field( @@ -196,7 +204,7 @@ class SourceType: map_types: List[str] = strawberry.field( description="Distinct map types used by active, non-retired mappings in this version.", default_factory=list, ) - external_sources: List[ToSourceType] = strawberry.field( + external_sources: List[ExternalSourceType] = strawberry.field( description="Visible external target sources referenced by active, non-retired outbound mappings.", default_factory=list, ) diff --git a/core/integration_tests/test_graphql_projection.py b/core/integration_tests/test_graphql_projection.py index 3ec25f7de..4514f9af5 100644 --- a/core/integration_tests/test_graphql_projection.py +++ b/core/integration_tests/test_graphql_projection.py @@ -41,7 +41,7 @@ def setUp(self): self.addCleanup(patcher.stop) self.source = OrganizationSourceFactory(mnemonic='CASE-Sensitive', name='Clinical dictionary') self.concept = ConceptFactory(parent=self.source, mnemonic='AbC', datatype='Numeric', concept_class='Diagnosis') - ConceptNameFactory(concept=self.concept, name='Hypertension', locale='en', locale_preferred=True) + ConceptNameFactory(concept=self.concept, name='Hypertension-test', locale='en', locale_preferred=True) ConceptDescriptionFactory(concept=self.concept, name='Preferred definition', locale='en', locale_preferred=True) self.index(self.source, SourceDocument) self.index(self.concept, ConceptDocument) @@ -74,7 +74,7 @@ def test_source_and_scoped_concepts_execute_without_sql(self): self.assertEqual(result.data['source']['uri'], self.source.uri) self.assertEqual(result.data['concepts']['totalCount'], 1) self.assertEqual(result.data['concepts']['results'], [{ - 'conceptId': 'AbC', 'display': 'Hypertension', 'description': 'Preferred definition', + 'conceptId': 'AbC', 'display': 'Hypertension-test', 'description': 'Preferred definition', 'datatype': {'name': 'Numeric'}, 'conceptClass': 'Diagnosis', }]) @@ -82,7 +82,7 @@ def test_global_projection_uses_head_and_excludes_retired_and_inactive(self): """Global counts omit historical versions and inactive/retired documents.""" for fields in ({'retired': True}, {'is_active': False}): excluded = ConceptFactory(parent=self.source, **fields) - ConceptNameFactory(concept=excluded, name='Hypertension', locale='en', locale_preferred=True) + ConceptNameFactory(concept=excluded, name='Hypertension-test', locale='en', locale_preferred=True) self.index(excluded, ConceptDocument) self.index(self.concept.get_latest_version(), ConceptDocument) with self.assertNumQueries(0): @@ -151,6 +151,6 @@ def test_hydrated_payload_uses_same_head_and_selected_columns(self): self.assertIsNone(result.errors) self.assertEqual(result.data['concepts']['totalCount'], 1) self.assertEqual(result.data['concepts']['results'][0]['id'], str(self.concept.id)) - self.assertEqual(result.data['concepts']['results'][0]['names'], [{'name': 'Hypertension'}]) + self.assertEqual(result.data['concepts']['results'][0]['names'], [{'name': 'Hypertension-test'}]) self.assertTrue(queries.captured_queries) self.assertFalse(any('"concepts"."extras"' in query['sql'] for query in queries.captured_queries)) From e392d95fa39542d23b3a1c0e19e95ca0f2818050 Mon Sep 17 00:00:00 2001 From: Filipe Lopes Date: Tue, 8 Sep 2026 10:11:01 -0300 Subject: [PATCH 3/6] feat(graphql): support display name for concept projections * Add `display_name` field to `ConceptDocument` projections * Refactor source projection URI to be rebuilt instead of stored to ensure data integrity and accurate URL generation --- core/common/search.py | 7 ++-- core/concepts/documents.py | 8 ----- core/graphql/README.md | 22 +++++++----- core/graphql/indexed.py | 31 ++++++++++++---- core/graphql/permissions.py | 8 ----- core/graphql/queries.py | 2 -- core/graphql/sources.py | 16 +++++++-- core/graphql/tests/test_projection.py | 36 ++++++++++++++----- core/graphql/tests/test_sources.py | 6 ++-- .../test_graphql_projection.py | 24 ++++++++++--- core/sources/documents.py | 4 +-- core/sources/signals.py | 7 ++-- 12 files changed, 104 insertions(+), 67 deletions(-) diff --git a/core/common/search.py b/core/common/search.py index 8e3c91a1a..c9c73467a 100644 --- a/core/common/search.py +++ b/core/common/search.py @@ -24,7 +24,6 @@ def get_document_public_visibility_criteria( # pylint: disable=too-many-argumen include_creator_private_access=False, include_owner_private_access=False, include_organization_memberships=False, - public_field='public_can_view', ): """Return a shared Elasticsearch visibility criterion for owner-scoped documents. @@ -47,7 +46,7 @@ def get_document_public_visibility_criteria( # pylint: disable=too-many-argumen Flags are independent OR-combined extensions. Staff bypass goes through ``apply_document_public_visibility_filter`` (this helper itself does not check staff). """ - criteria = Q('term', **{public_field: True}) + criteria = Q('term', public_can_view=True) if not getattr(user, 'is_authenticated', False): return criteria @@ -71,7 +70,7 @@ def get_document_public_visibility_criteria( # pylint: disable=too-many-argumen if private_criteria is None: return criteria - return criteria | (Q('term', **{public_field: False}) & private_criteria) + return criteria | (Q('term', public_can_view=False) & private_criteria) def apply_document_public_visibility_filter( # pylint: disable=too-many-arguments @@ -80,7 +79,6 @@ def apply_document_public_visibility_filter( # pylint: disable=too-many-argumen include_creator_private_access=False, include_owner_private_access=False, include_organization_memberships=False, - public_field='public_can_view', ): """Apply a shared Elasticsearch visibility filter without changing staff searches.""" if getattr(user, 'is_staff', False): @@ -92,7 +90,6 @@ def apply_document_public_visibility_filter( # pylint: disable=too-many-argumen include_creator_private_access=include_creator_private_access, include_owner_private_access=include_owner_private_access, include_organization_memberships=include_organization_memberships, - public_field=public_field, ) ) diff --git a/core/concepts/documents.py b/core/concepts/documents.py index e04d8f19c..bb24a0d4b 100644 --- a/core/concepts/documents.py +++ b/core/concepts/documents.py @@ -14,9 +14,7 @@ class Index: # Preserve ORM semantics for direct GraphQL projections without changing REST search fields. is_active = fields.BooleanField(attr='is_active') - parent_public_can_view = fields.BooleanField(attr='parent.public_can_view') is_head = fields.BooleanField() - preferred_description = fields.TextField() display_name = fields.TextField(attr='display_name') id = fields.TextField(attr='mnemonic') @@ -279,9 +277,3 @@ def get_mapped_codes(instance): def prepare_is_head(instance): """Match the versioned-object predicate used by Source.get_concepts_queryset.""" return instance.id == instance.versioned_object_id - - @staticmethod - def prepare_preferred_description(instance): - """Store the same locale-selected description returned by GraphQL's ORM path.""" - from core.graphql.serializers import resolve_description - return resolve_description(instance) diff --git a/core/graphql/README.md b/core/graphql/README.md index 85d13bdec..0b887aa75 100644 --- a/core/graphql/README.md +++ b/core/graphql/README.md @@ -53,8 +53,9 @@ version only if HEAD is absent; explicit missing versions do not fall back. | Selected payload | Retrieval | | --- | --- | -| Source `name`, `description`, `canonicalUrl`, `uri` | Source index projection, including source/version resolution | -| Concept `id`, `conceptId`, `externalId`, `display`, `description`, `conceptClass`, `datatype { name }` | Concept index projection; no ORM concept hydration | +| Source `name`, `canonicalUrl`, `uri` | Source index projection, including source/version resolution. `uri` is rebuilt from owner, owner type, mnemonic and version rather than stored | +| Concept `id`, `conceptId`, `externalId`, `display`, `conceptClass`, `datatype { name }` | Concept index projection; no ORM concept hydration | +| Source `description`, concept `description` | Not indexed; selecting either routes that request through the ORM | | Only concept counts/pagination metadata | Elasticsearch request with zero result hits | | Concept names, mappings, extras, audit metadata, datatype details | ORM hydration with selected concept columns and relations | | Source classes, datatypes, map types, external sources, summary | Existing version-scoped model querysets; only selected aggregates execute | @@ -69,8 +70,9 @@ Counts and distinct labels use active, non-retired records. `summary.mappings` c private targets the caller cannot view. Unresolved external URIs are taken from visible mappings. Repository permission checks reuse the shared REST visibility rule directly, without fabricated requests. -Both owner mnemonic and owner type scope index lookups. Global concepts also enforce parent repository visibility, -and mapping hydration independently checks target visibility. HEAD uses the same versioned-object identity as +Both owner mnemonic and owner type scope index lookups. Concept visibility relies on the indexed +`public_can_view` flag that `core/sources/signals.py` already propagates from the parent repository, and +mapping hydration independently checks target visibility. HEAD uses the same versioned-object identity as `Source.get_concepts_queryset()`, while releases use their membership lists. SQL-free data retrieval does not mean SQL-free authentication: session/token lookup and organization membership @@ -80,9 +82,10 @@ existing REST index, indexed results reflect Elasticsearch refresh and indexing ## Rollout No database migrations or new environment variables are introduced. Refresh the source and concept indexes -before serving this GraphQL version: older concept documents lack the HEAD, activity, parent-permission and -preferred-description projection fields. Source documents add description, URI and activity fields. -Do not use incomplete indexes during the rollout; global projections filter on the new fields. +before serving this GraphQL version: older concept documents lack the `is_active`, `is_head` and `display_name` +projection fields, and older source documents lack `is_active`. Do not use incomplete indexes during the +rollout; concept projections filter on `is_active` and `is_head`, so an unrefreshed index returns zero +concepts without raising an error. Use the existing indexing procedure to apply the additive mappings and repopulate both models. For a deployment that recreates indexes, use its established rebuild procedure; do not rebuild live indexes without accounting for @@ -92,7 +95,7 @@ REST search availability. A full population command for the existing application docker exec oclapi2-api-1 python manage.py search_index --populate --models sources.Source concepts.Concept -f --parallel ``` -Source permission/activity propagation also refreshes the corresponding concept projection flags. Existing +Source permission/activity propagation refreshes the corresponding concept projection flags. Existing REST search relevance and excluded-word semantics are preserved; unrelated search refactors from PR #838 were not carried over. Its corrected permission sharing and documented Strawberry auth extension were retained. @@ -106,7 +109,8 @@ docker exec oclapi2-api-1 pylint -j2 core/graphql core/common/permissions.py cor `core.integration_tests.test_graphql_projection` requires `settings.ES_ENABLED=True`. It creates uniquely named indexes and removes them after each test. Run it only against a test Elasticsearch service: shared fixture setup can also exercise normal indexing hooks. It covers real index preparation, zero SQL, owner isolation, HEAD/release -selection, inactive/retired filtering, and private-parent visibility. +selection, inactive/retired filtering, private-repository visibility, the rebuilt source URI, and the +database fallback for `description`. For this worktree, verification used a copy at `/tmp/graphql-sources-20260906` inside the existing API container, the dedicated database `test_graphql_sources_20260906`, and a temporary Elasticsearch container. The running app's diff --git a/core/graphql/indexed.py b/core/graphql/indexed.py index 65e67c611..f74fee833 100644 --- a/core/graphql/indexed.py +++ b/core/graphql/indexed.py @@ -3,14 +3,17 @@ import logging from types import SimpleNamespace +from django.urls import reverse from elasticsearch import ApiError, ConnectionError as ESConnectionError, TransportError from elasticsearch_dsl import Q from core.common.constants import HEAD +from core.common.utils import encode_string, is_url_encoded_string from core.concepts.documents import ConceptDocument from core.sources.documents import SourceDocument +from core.users.constants import USER_OBJECT_TYPE -from .permissions import apply_es_parent_visibility_filter, apply_es_visibility_filter +from .permissions import apply_es_visibility_filter from .selection import index_projection from .sources import SOURCE_INDEX_FIELDS from .types import ConceptType, DatatypeType, SourceType @@ -23,7 +26,6 @@ 'conceptId': ('id',), 'externalId': ('external_id',), 'display': ('display_name',), - 'description': ('preferred_description',), 'conceptClass': ('concept_class',), 'datatype.name': ('datatype',), } @@ -49,7 +51,7 @@ def indexed_source(org, owner, source, version, user, paths): # pylint: disable search = search.filter('term', owner=owner_value.lower()).filter('term', owner_type=owner_type) search = search.filter('term', version=version or HEAD) search = apply_es_visibility_filter(search, user) - search = search.source(sorted(set(fields) | {'is_active', 'version', 'mnemonic'}))[:1] + search = search.source(sorted(set(fields) | {'is_active', 'version', 'mnemonic', 'owner', 'owner_type'}))[:1] try: hits = list(search.execute()) except (ApiError, TransportError, ESConnectionError) as exc: @@ -58,12 +60,28 @@ def indexed_source(org, owner, source, version, user, paths): # pylint: disable if not hits or not getattr(hits[0], 'is_active', False): return None hit = hits[0] + payload = SourceType(**{field: getattr(hit, field, None) for field in fields}) + if 'uri' in paths: + payload.uri = source_uri(hit) return SimpleNamespace( - mnemonic=hit.mnemonic, version=hit.version, is_head=hit.version == HEAD, - payload=SourceType(**{field: getattr(hit, field, None) for field in fields}), + mnemonic=hit.mnemonic, version=hit.version, is_head=hit.version == HEAD, payload=payload, ) +def source_uri(hit): + """Rebuild the relative URI with the same URL machinery and version encoding the ORM stores. + + Reversing the real routes keeps percent-encoding identical to ``calculate_uri`` for versions + that contain reserved characters, which a plain string join would silently get wrong. + """ + owner_kwarg = 'user' if hit.owner_type == USER_OBJECT_TYPE else 'org' + kwargs = {owner_kwarg: hit.owner, 'source': hit.mnemonic} + if hit.version == HEAD: + return reverse('source-detail', kwargs=kwargs) + version = hit.version if is_url_encoded_string(hit.version) else encode_string(hit.version, safe=' ') + return reverse('source-version-detail', kwargs={**kwargs, 'version': version}) + + # The planner supplies request scope and payload independently. # pylint: disable-next=too-many-arguments,too-many-locals def indexed_concepts(paths, query, concept_ids, scope, pagination, owner, owner_type, user): @@ -78,7 +96,6 @@ def indexed_concepts(paths, query, concept_ids, scope, pagination, owner, owner_ search = search.filter('term', **({'is_head': True} if scope.is_head else {'source_version': scope.version})) else: search = apply_es_visibility_filter(search.filter('term', is_head=True), user) - search = apply_es_parent_visibility_filter(search, user) if concept_ids: # Script-free ordering preserves the requested mnemonic order, with deterministic ties. search = search.filter('terms', id_raw=concept_ids).sort('id_raw') @@ -113,7 +130,7 @@ def serialize_indexed_concept(hit): return ConceptType( id=str(hit.meta.id), concept_id=getattr(hit, 'id', ''), external_id=getattr(hit, 'external_id', None), display=getattr(hit, 'display_name', None), - description=getattr(hit, 'preferred_description', None), concept_class=getattr(hit, 'concept_class', None), + description=None, concept_class=getattr(hit, 'concept_class', None), datatype=DatatypeType(name=datatype, details=None) if datatype else None, names=[], mappings=[], metadata=None, extras={}, ) diff --git a/core/graphql/permissions.py b/core/graphql/permissions.py index 7d87618e9..17a41287d 100644 --- a/core/graphql/permissions.py +++ b/core/graphql/permissions.py @@ -89,14 +89,6 @@ def apply_es_visibility_filter(search, user): ) -def apply_es_parent_visibility_filter(search, user): - """Also protect private parent repositories when a child has a public access flag.""" - return apply_document_public_visibility_filter( - search, user, include_owner_private_access=True, include_organization_memberships=True, - public_field='parent_public_can_view', - ) - - class PermissionsMixin: """Provide cached source resolution and shared permission helpers to resolvers.""" diff --git a/core/graphql/queries.py b/core/graphql/queries.py index b7e25dbb7..24c865626 100644 --- a/core/graphql/queries.py +++ b/core/graphql/queries.py @@ -31,7 +31,6 @@ from .permissions import ( PermissionsMixin, apply_es_visibility_filter, - apply_es_parent_visibility_filter, resolve_owner, filter_parent_queryset, ) @@ -192,7 +191,6 @@ def concept_ids_from_es( # pylint: disable=too-many-arguments else: search = search.filter('term', is_head=True) search = apply_es_visibility_filter(search, user or AnonymousUser()) - search = apply_es_parent_visibility_filter(search, user or AnonymousUser()) search = search.filter('term', retired=False).filter('term', is_active=True) search = search_text(search, trimmed) diff --git a/core/graphql/sources.py b/core/graphql/sources.py index 4ec871c00..a805fda84 100644 --- a/core/graphql/sources.py +++ b/core/graphql/sources.py @@ -4,21 +4,31 @@ from .types import ExternalSourceType, SourceType -SOURCE_INDEX_FIELDS = { - '__typename': (), +# GraphQL path to model attribute, used when the ORM serializes a source. +SOURCE_FIELDS = { 'name': ('name',), 'description': ('description',), 'canonicalUrl': ('canonical_url',), 'uri': ('uri',), } +# Payloads the source index can answer on its own. ``description`` is deliberately absent: it is +# not stored in the index, so selecting it routes the whole request through the ORM. ``uri`` maps +# to no stored field because it is rebuilt from ownership, mnemonic and version. +SOURCE_INDEX_FIELDS = { + '__typename': (), + 'name': ('name',), + 'canonicalUrl': ('canonical_url',), + 'uri': (), +} + def serialize_source(instance, paths, user): """Load only the aggregates requested by the client, using existing model querysets.""" result = SourceType( **{ fields[0]: getattr(instance, fields[0]) - for path, fields in SOURCE_INDEX_FIELDS.items() if path in paths and fields + for path, fields in SOURCE_FIELDS.items() if path in paths } ) if paths & {'classes', 'datatypes', 'summary.activeConcepts'}: diff --git a/core/graphql/tests/test_projection.py b/core/graphql/tests/test_projection.py index 1b3ff3280..17b87e44c 100644 --- a/core/graphql/tests/test_projection.py +++ b/core/graphql/tests/test_projection.py @@ -10,6 +10,7 @@ from elasticsearch_dsl import Search from elasticsearch_dsl.response import Response +from core.graphql.indexed import source_uri from core.graphql.schema import schema from core.graphql.selection import index_projection @@ -33,33 +34,52 @@ def response(index, fields, total=None): def test_source_metadata_has_no_sql_and_projects_selected_fields(self): """Minimal source reads use one ES request with owner, version and visibility filters.""" response = self.response('sources', [{ - 'name': 'CIEL', 'description': 'Clinical dictionary', 'canonical_url': 'https://ciel.org', - 'uri': '/orgs/CIEL/sources/CIEL/', 'is_active': True, 'version': 'HEAD', 'mnemonic': 'CIEL', + 'name': 'CIEL', 'canonical_url': 'https://ciel.org', 'is_active': True, + 'version': 'HEAD', 'mnemonic': 'CIEL', 'owner': 'CIEL', 'owner_type': 'Organization', }]) with patch('elasticsearch_dsl.Search.execute', autospec=True, return_value=response) as execute: result = self.execute('''{ source(org: "CIEL", source: "CIEL") { - name description canonicalUrl uri + name canonicalUrl uri } }''') self.assertIsNone(result.errors) + # The URI is rebuilt from ownership, mnemonic and version rather than read from the index. self.assertEqual(result.data['source']['uri'], '/orgs/CIEL/sources/CIEL/') body = execute.call_args.args[0].to_dict() self.assertEqual(set(body['_source']), { - 'name', 'description', 'canonical_url', 'uri', 'is_active', 'version', 'mnemonic', + 'name', 'canonical_url', 'is_active', 'version', 'mnemonic', 'owner', 'owner_type', }) filters = str(body['query']) for expected in ('public_can_view', 'owner_type', 'Organization', 'ciel', 'HEAD'): self.assertIn(expected, filters) + def test_rebuilt_source_uri_matches_the_stored_encoding(self): + """The URI is not indexed, so its reconstruction must reproduce ORM percent-encoding.""" + def hit(owner, owner_type, mnemonic, version): + return SimpleNamespace(owner=owner, owner_type=owner_type, mnemonic=mnemonic, version=version) + + self.assertEqual(source_uri(hit('CIEL', 'Organization', 'CIEL', 'HEAD')), '/orgs/CIEL/sources/CIEL/') + self.assertEqual(source_uri(hit('jane', 'User', 'S1', 'HEAD')), '/users/jane/sources/S1/') + # Reserved characters in a version label are double-encoded, exactly as calculate_uri stores them. + self.assertEqual( + source_uri(hit('OpenMRS-OCL-Squad', 'Organization', 'Bridge-5', 'WHO-ICD11@2026-01')), + '/orgs/OpenMRS-OCL-Squad/sources/Bridge-5/WHO-ICD11%25402026-01/', + ) + # An already-encoded label must not be encoded a second time. + self.assertEqual( + source_uri(hit('OCL', 'Organization', 'S1', 'v1%402026')), + '/orgs/OCL/sources/S1/v1%25402026/', + ) + def test_concept_fragments_aliases_and_directives_remain_sql_free(self): """Skipped heavy fields do not force ORM loading, including named fragments.""" response = self.response('concepts', [{ 'id': '123', 'display_name': 'Hypertension-test', 'datatype': 'Numeric', - 'concept_class': 'Diagnosis', 'preferred_description': 'Preferred definition', + 'concept_class': 'Diagnosis', }]) query = '''query($heavy: Boolean!, $light: Boolean!) { found: concepts(query: "hypertension") { totalCount results { ...Light - ... on ConceptType { description } + ... on ConceptType { externalId } names @include(if: $heavy) { name } mappings @skip(if: $light) { mapType } } } @@ -71,11 +91,11 @@ def test_concept_fragments_aliases_and_directives_remain_sql_free(self): self.assertIsNone(result.errors) self.assertEqual(result.data['found']['results'][0], { 'code': '123', 'label': 'Hypertension-test', 'datatype': {'name': 'Numeric'}, - 'conceptClass': 'Diagnosis', 'description': 'Preferred definition', + 'conceptClass': 'Diagnosis', 'externalId': None, }) body = execute.call_args.args[0].to_dict() self.assertEqual(set(body['_source']), { - 'id', 'display_name', 'datatype', 'concept_class', 'preferred_description', + 'id', 'display_name', 'datatype', 'concept_class', 'external_id', }) self.assertNotIn('extras', body['_source']) self.assertIn('is_head', str(body['query'])) diff --git a/core/graphql/tests/test_sources.py b/core/graphql/tests/test_sources.py index e4f2c9b3c..0ef2822e3 100644 --- a/core/graphql/tests/test_sources.py +++ b/core/graphql/tests/test_sources.py @@ -169,15 +169,13 @@ def test_typename_only_summary_and_datatype_details(self): }) def test_parent_permission_changes_update_projection_fields(self): - """Source ACL propagation refreshes the indexed parent flag as well as the child flag.""" + """Source ACL propagation refreshes the indexed child visibility flag.""" self.source.public_access = ACCESS_TYPE_NONE self.source._should_update_public_access = True # pylint: disable=protected-access with patch.object(type(self.source), 'batch_index') as index: self.source.save() concept_update = next(call for call in index.call_args_list if call.args[1].Index.name == 'concepts') - self.assertEqual(concept_update.kwargs['partial_doc'], { - 'public_can_view': False, 'parent_public_can_view': False, - }) + self.assertEqual(concept_update.kwargs['partial_doc'], {'public_can_view': False}) def test_parent_deactivation_updates_indexed_concept_flag(self): """Deactivated parent repositories cannot leave active concept projections behind.""" diff --git a/core/integration_tests/test_graphql_projection.py b/core/integration_tests/test_graphql_projection.py index 4514f9af5..0877a6619 100644 --- a/core/integration_tests/test_graphql_projection.py +++ b/core/integration_tests/test_graphql_projection.py @@ -61,9 +61,9 @@ def execute(self, query, user=None): def test_source_and_scoped_concepts_execute_without_sql(self): """Real source and concept lookups require no SQL after anonymous context creation.""" query = '''{ - source(org: "%s", source: "%s") { name description canonicalUrl uri } + source(org: "%s", source: "%s") { name canonicalUrl uri } concepts(org: "%s", source: "%s", query: "Hypertension") { - versionResolved totalCount results { conceptId display description datatype { name } conceptClass } + versionResolved totalCount results { conceptId display datatype { name } conceptClass } } }''' % (self.source.organization.mnemonic, self.source.mnemonic, self.source.organization.mnemonic, self.source.mnemonic) @@ -71,13 +71,24 @@ def test_source_and_scoped_concepts_execute_without_sql(self): result = self.execute(query) self.assertIsNone(result.errors) self.assertEqual(result.data['source']['name'], self.source.name) + # The rebuilt URI must match the one the ORM stores, including owner and mnemonic casing. self.assertEqual(result.data['source']['uri'], self.source.uri) self.assertEqual(result.data['concepts']['totalCount'], 1) self.assertEqual(result.data['concepts']['results'], [{ - 'conceptId': 'AbC', 'display': 'Hypertension-test', 'description': 'Preferred definition', + 'conceptId': 'AbC', 'display': 'Hypertension-test', 'datatype': {'name': 'Numeric'}, 'conceptClass': 'Diagnosis', }]) + def test_description_selection_falls_back_to_the_database(self): + """description is not indexed, so selecting it routes the whole request through the ORM.""" + result = self.execute('''{ + concepts(org: "%s", source: "%s", query: "Hypertension") { results { conceptId description } } + }''' % (self.source.organization.mnemonic, self.source.mnemonic)) + self.assertIsNone(result.errors) + self.assertEqual( + result.data['concepts']['results'], [{'conceptId': 'AbC', 'description': 'Preferred definition'}], + ) + def test_global_projection_uses_head_and_excludes_retired_and_inactive(self): """Global counts omit historical versions and inactive/retired documents.""" for fields in ({'retired': True}, {'is_active': False}): @@ -90,11 +101,14 @@ def test_global_projection_uses_head_and_excludes_retired_and_inactive(self): self.assertIsNone(result.errors) self.assertEqual(result.data['concepts']['totalCount'], 1) - def test_private_parent_is_hidden_even_if_child_flag_is_public(self): - """A public child flag cannot disclose records from a private repository.""" + def test_private_parent_hides_its_concepts(self): + """Concepts of a private repository stay hidden from anonymous global search.""" self.source.public_access = ACCESS_TYPE_NONE self.source.save() self.index(self.source, SourceDocument) + # Propagation copies the repository access onto children before they are reindexed; + # the concept projection is filtered by that copied flag alone. + self.concept.public_access = ACCESS_TYPE_NONE self.index(self.concept, ConceptDocument) with self.assertNumQueries(0): result = self.execute('{ concepts(query: "Hypertension") { totalCount results { display } } }') diff --git a/core/sources/documents.py b/core/sources/documents.py index 5718242df..4b1aacb14 100644 --- a/core/sources/documents.py +++ b/core/sources/documents.py @@ -14,9 +14,7 @@ class Index: name = 'sources' settings = {'number_of_shards': 1, 'number_of_replicas': 0} - # Additive projection fields; existing indexes require a source reindex. - description = fields.TextField(attr='description') - uri = fields.KeywordField(attr='uri') + # Additive projection field; existing indexes require a source reindex. is_active = fields.BooleanField(attr='is_active') locale = fields.ListField(fields.KeywordField()) diff --git a/core/sources/signals.py b/core/sources/signals.py index 1dfc2ad72..840258c45 100644 --- a/core/sources/signals.py +++ b/core/sources/signals.py @@ -24,11 +24,8 @@ def propagate_parent_attributes(sender, instance=None, created=False, **kwargs): public_access=instance.public_access).update(public_access=instance.public_access) partial_doc = {'public_can_view': instance.public_can_view} - if updated_concepts or instance.concepts_set.exists(): - instance.batch_index( - instance.concepts_set, ConceptDocument, - partial_doc={**partial_doc, 'parent_public_can_view': instance.public_can_view}, - ) + if updated_concepts: + instance.batch_index(instance.concepts_set, ConceptDocument, partial_doc=partial_doc) if updated_mappings: from core.mappings.documents import MappingDocument instance.batch_index(instance.mappings_set, MappingDocument, partial_doc=partial_doc) From 2c19af73f7f1b634ff98dad71fb3a7c7c274361e Mon Sep 17 00:00:00 2001 From: Filipe Lopes Date: Thu, 10 Sep 2026 18:18:25 -0300 Subject: [PATCH 4/6] feat(graphql): update source referencing mappings to use external source type * Replaced the internal `ToSourceType` with `ExternalSourceType` in GraphQL types, allowing the system to handle different kinds of external sources. * Updated serializers and queries to use the new `ExternalSourceType` definition, improving type consistency across the GraphQL API. --- core/graphql/serializers.py | 6 +++--- core/graphql/tests/test_concepts_from_source.py | 2 +- core/graphql/types.py | 8 +------- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/core/graphql/serializers.py b/core/graphql/serializers.py index 363d8d178..2184f0928 100644 --- a/core/graphql/serializers.py +++ b/core/graphql/serializers.py @@ -21,11 +21,11 @@ ConceptType, DatatypeDetails, DatatypeType, + ExternalSourceType, MappingType, MetadataType, NumericDatatypeDetails, TextDatatypeDetails, - ToSourceType, ) @@ -36,8 +36,8 @@ def serialize_mappings(concept: Concept) -> List[MappingType]: result.append( MappingType( map_type=str(mapping.map_type), - to_source=ToSourceType( - url=mapping.to_source_url, + to_source=ExternalSourceType( + uri=mapping.to_source_url, name=mapping.to_source_name, ) if mapping.to_source_url or mapping.to_source_name else None, to_code=mapping.get_to_concept_code(), diff --git a/core/graphql/tests/test_concepts_from_source.py b/core/graphql/tests/test_concepts_from_source.py index acea4c97c..b10571205 100644 --- a/core/graphql/tests/test_concepts_from_source.py +++ b/core/graphql/tests/test_concepts_from_source.py @@ -141,7 +141,7 @@ def test_fetch_concepts_by_ids_with_pagination(self): results { conceptId display - mappings { mapType toSource { url name } toCode toConceptName sortWeight comment } + mappings { mapType toSource { uri name } toCode toConceptName sortWeight comment } extras } } diff --git a/core/graphql/types.py b/core/graphql/types.py index b9d43d523..799ba490c 100644 --- a/core/graphql/types.py +++ b/core/graphql/types.py @@ -15,12 +15,6 @@ class ConceptNameType: retired: bool = strawberry.field(description="Indicates whether this name is retired.") -@strawberry.type -class ToSourceType: - url: Optional[str] = strawberry.field(description="URL pointing to the target source.") - name: Optional[str] = strawberry.field(description="Human-readable name for the target source.") - - @strawberry.type class ExternalSourceType: """GraphQL metadata for a source referenced by an outbound mapping.""" @@ -35,7 +29,7 @@ class MappingType: name="mapType", description="Mapping type (e.g. SAME-AS, NARROWER-THAN).", ) - to_source: Optional[ToSourceType] = strawberry.field( + to_source: Optional[ExternalSourceType] = strawberry.field( name="toSource", description="Metadata about the source/collection the mapping points to.", ) From a95c4b58b3fa7070c04cda9fad72d580fc359ce6 Mon Sep 17 00:00:00 2001 From: Filipe Lopes Date: Fri, 11 Sep 2026 08:19:13 -0300 Subject: [PATCH 5/6] refactor(concepts): improve document preparation for display names and versioning --- core/concepts/documents.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/core/concepts/documents.py b/core/concepts/documents.py index bb24a0d4b..5db97f0e9 100644 --- a/core/concepts/documents.py +++ b/core/concepts/documents.py @@ -12,10 +12,12 @@ class Index: name = 'concepts' settings = {'number_of_shards': 1, 'number_of_replicas': 0} - # Preserve ORM semantics for direct GraphQL projections without changing REST search fields. + # New fields for direct GraphQL projections; they mirror ORM semantics without altering the + # existing REST search fields. `display_name` is the raw preferred-locale name -- `name` and + # `_name` below are search-normalized ('-' -> '_' / lowercased) and can't be used for display. is_active = fields.BooleanField(attr='is_active') - is_head = fields.BooleanField() - display_name = fields.TextField(attr='display_name') + is_head = fields.BooleanField() # resolves via the VersionedModel.is_head property + display_name = fields.TextField() # populated in prepare(), reusing preferred_locale id = fields.TextField(attr='mnemonic') id_lowercase = fields.KeywordField(attr='mnemonic', normalizer="lowercase") @@ -223,6 +225,7 @@ def prepare(self, instance): preferred_locale = instance.preferred_locale name = get(preferred_locale, 'name') or '' + data['display_name'] = name data['_name'] = name.lower() data['name'] = name.replace('-', '_') synonyms = [n for n in instance.active_names.all() if n.name and n.name != name] @@ -272,8 +275,3 @@ def get_mapped_codes(instance): else: other_mapped_codes.append(to_concept_code) return same_as_mapped_codes, other_mapped_codes, verbose_info - - @staticmethod - def prepare_is_head(instance): - """Match the versioned-object predicate used by Source.get_concepts_queryset.""" - return instance.id == instance.versioned_object_id From b0dfae6bacdfeffd1119b41ab29734f6afe3b534 Mon Sep 17 00:00:00 2001 From: Filipe Lopes Date: Fri, 11 Sep 2026 09:06:56 -0300 Subject: [PATCH 6/6] refactor(concepts): index display names using search-normalized fields * Remove dedicated display_name field from ConceptDocument and rely on search-normalized fields (name and _name) * Introduce a function to reconstruct the display name by combining the information from both indexed fields upon retrieval from Elasticsearch --- core/concepts/documents.py | 6 +----- core/graphql/indexed.py | 30 +++++++++++++++++++++++++-- core/graphql/tests/test_projection.py | 9 ++++---- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/core/concepts/documents.py b/core/concepts/documents.py index 5db97f0e9..ae63b68de 100644 --- a/core/concepts/documents.py +++ b/core/concepts/documents.py @@ -12,12 +12,9 @@ class Index: name = 'concepts' settings = {'number_of_shards': 1, 'number_of_replicas': 0} - # New fields for direct GraphQL projections; they mirror ORM semantics without altering the - # existing REST search fields. `display_name` is the raw preferred-locale name -- `name` and - # `_name` below are search-normalized ('-' -> '_' / lowercased) and can't be used for display. + is_active = fields.BooleanField(attr='is_active') is_head = fields.BooleanField() # resolves via the VersionedModel.is_head property - display_name = fields.TextField() # populated in prepare(), reusing preferred_locale id = fields.TextField(attr='mnemonic') id_lowercase = fields.KeywordField(attr='mnemonic', normalizer="lowercase") @@ -225,7 +222,6 @@ def prepare(self, instance): preferred_locale = instance.preferred_locale name = get(preferred_locale, 'name') or '' - data['display_name'] = name data['_name'] = name.lower() data['name'] = name.replace('-', '_') synonyms = [n for n in instance.active_names.all() if n.name and n.name != name] diff --git a/core/graphql/indexed.py b/core/graphql/indexed.py index f74fee833..a36fbd432 100644 --- a/core/graphql/indexed.py +++ b/core/graphql/indexed.py @@ -25,7 +25,7 @@ 'id': (), # Elasticsearch's metadata ID is the OCL database primary key. 'conceptId': ('id',), 'externalId': ('external_id',), - 'display': ('display_name',), + 'display': ('name', '_name'), 'conceptClass': ('concept_class',), 'datatype.name': ('datatype',), } @@ -124,12 +124,38 @@ def indexed_concepts(paths, query, concept_ids, scope, pagination, owner, owner_ return [serialize_indexed_concept(hit) for hit in hits], total +def restore_display_name(name, lower_name): + """Rebuild the concept display value from the two search-normalized name fields. + + `ConceptDocument.prepare` stores the preferred-locale name twice: `name` keeps the original + casing but rewrites '-' as '_' (so hyphenated clinical terms stay a single token), while + `_name` keeps the original hyphens but is lowercased. Neither is the display value on its + own, so we take the casing from `name` and the hyphens from `_name`, position by position. + + Only names actually containing an underscore need this. A term whose original text already + held an underscore is indistinguishable in `name` alone, but `_name` disambiguates it; the + naive fallback (every '_' back to '-') applies only when `_name` is missing or its length + diverges, which we accept as an unlikely, low-impact risk rather than indexing a third field. + """ + if not name or '_' not in name: + return name + if not lower_name or len(lower_name) != len(name): + return name.replace('_', '-') + return ''.join( + '-' if char == '_' and lower_name[index] == '-' else char + for index, char in enumerate(name) + ) + + def serialize_indexed_concept(hit): """Construct only index-backed values; unselected relationship fields stay unloaded.""" datatype = getattr(hit, 'datatype', None) + # `_name` is read from the source dict: attribute access on a Hit reserves the '_' prefix. + source = hit.to_dict() return ConceptType( id=str(hit.meta.id), concept_id=getattr(hit, 'id', ''), - external_id=getattr(hit, 'external_id', None), display=getattr(hit, 'display_name', None), + external_id=getattr(hit, 'external_id', None), + display=restore_display_name(source.get('name'), source.get('_name')), description=None, concept_class=getattr(hit, 'concept_class', None), datatype=DatatypeType(name=datatype, details=None) if datatype else None, names=[], mappings=[], metadata=None, extras={}, diff --git a/core/graphql/tests/test_projection.py b/core/graphql/tests/test_projection.py index 17b87e44c..801df189f 100644 --- a/core/graphql/tests/test_projection.py +++ b/core/graphql/tests/test_projection.py @@ -73,8 +73,9 @@ def hit(owner, owner_type, mnemonic, version): def test_concept_fragments_aliases_and_directives_remain_sql_free(self): """Skipped heavy fields do not force ORM loading, including named fragments.""" response = self.response('concepts', [{ - 'id': '123', 'display_name': 'Hypertension-test', 'datatype': 'Numeric', - 'concept_class': 'Diagnosis', + # The index stores the search-normalized pair; `display` is rebuilt from them. + 'id': '123', 'name': 'Hypertension_test', '_name': 'hypertension-test', + 'datatype': 'Numeric', 'concept_class': 'Diagnosis', }]) query = '''query($heavy: Boolean!, $light: Boolean!) { found: concepts(query: "hypertension") { totalCount results { @@ -95,7 +96,7 @@ def test_concept_fragments_aliases_and_directives_remain_sql_free(self): }) body = execute.call_args.args[0].to_dict() self.assertEqual(set(body['_source']), { - 'id', 'display_name', 'datatype', 'concept_class', 'external_id', + 'id', 'name', '_name', 'datatype', 'concept_class', 'external_id', }) self.assertNotIn('extras', body['_source']) self.assertIn('is_head', str(body['query'])) @@ -127,7 +128,7 @@ def test_two_aliases_plan_fields_independently(self): b: concepts(query: "b") { results { display } } }''') self.assertIsNone(result.errors) - self.assertEqual([call.args[0].to_dict()['_source'] for call in es.call_args_list], [['id'], ['display_name']]) + self.assertEqual([call.args[0].to_dict()['_source'] for call in es.call_args_list], [['id'], ['_name', 'name']]) def test_invalid_auth_stops_all_queries(self): """Schema-level auth failure precedes both source and concept data access."""