From b039290d10f2e486577f45742a7fe77adfee85c0 Mon Sep 17 00:00:00 2001 From: Baris Akcam Date: Fri, 25 Sep 2026 14:38:38 +0200 Subject: [PATCH 01/11] feat(selection): retain definitions named by the selection query Add @retainedDefinitions, a directive on the selection query that keeps scalars, directives and enums the selected fields do not reference, and narrows a retained enum to the values it lists. An empty list keeps every definition of that kind; an absent argument leaves today's behaviour, so a query without the directive filters exactly as before. The directive is defined by s2dm rather than by the model, so it is taken off the document before the document is validated against the schema. Removing an enum value that a retained default or directive argument needs is refused. Instance tag exclude entries are not treated as references: an entry asserts that one instance is absent rather than consuming a value, so an entry naming a removed value is dropped as vacuous. Entries naming no removed value are left alone, so a misspelled entry still fails at expansion. --- src/s2dm/api/routes/query_validate.py | 4 +- src/s2dm/constants/directive.py | 1 + .../exporters/utils/retained_definitions.py | 383 ++++++++++++++++++ src/s2dm/exporters/utils/schema_loader.py | 17 + .../data/retained_definitions_schema.graphql | 35 ++ tests/test_retained_definitions.py | 147 +++++++ 6 files changed, 586 insertions(+), 1 deletion(-) create mode 100644 src/s2dm/exporters/utils/retained_definitions.py create mode 100644 tests/data/retained_definitions_schema.graphql create mode 100644 tests/test_retained_definitions.py diff --git a/src/s2dm/api/routes/query_validate.py b/src/s2dm/api/routes/query_validate.py index 8c17cac7..677a7b43 100644 --- a/src/s2dm/api/routes/query_validate.py +++ b/src/s2dm/api/routes/query_validate.py @@ -9,6 +9,7 @@ from s2dm.api.models.query_validate import ValidateQueryRequest from s2dm.api.services.response_service import execute_and_respond from s2dm.api.services.schema_service import path_for_content, process_schema_input, validate_schema_or_raise +from s2dm.exporters.utils.retained_definitions import extract_retained_definitions, validate_retained_definitions from s2dm.exporters.utils.schema_loader import load_schema router = APIRouter(responses=COMMON_RESPONSES) @@ -27,7 +28,8 @@ def process_request() -> list[str]: query_path = path_for_content(request.selection_query, "selection_query", ".graphql") query_text = query_path.read_text(encoding="utf-8") - query_document = parse(query_text) + query_document, retained = extract_retained_definitions(parse(query_text)) + validate_retained_definitions(schema, retained) validation_errors = validate(schema, query_document) diff --git a/src/s2dm/constants/directive.py b/src/s2dm/constants/directive.py index 516f9c10..9165b283 100644 --- a/src/s2dm/constants/directive.py +++ b/src/s2dm/constants/directive.py @@ -14,6 +14,7 @@ class Directive(str, Enum): REFERENCE = "reference" VSPEC = "vspec" MODL = "modl" + RETAINED_DEFINITIONS = "retainedDefinitions" class BuiltInDirective(str, Enum): diff --git a/src/s2dm/exporters/utils/retained_definitions.py b/src/s2dm/exporters/utils/retained_definitions.py new file mode 100644 index 00000000..e5927672 --- /dev/null +++ b/src/s2dm/exporters/utils/retained_definitions.py @@ -0,0 +1,383 @@ +"""Extraction and validation of the @retainedDefinitions selection query directive.""" + +from dataclasses import dataclass +from typing import Any, cast + +from graphql import ( + DocumentNode, + GraphQLEnumType, + GraphQLInputObjectType, + GraphQLInterfaceType, + GraphQLObjectType, + GraphQLScalarType, + GraphQLSchema, + get_named_type, +) +from graphql.language.ast import EnumValueNode, ListValueNode, OperationDefinitionNode, StringValueNode +from graphql.utilities import value_from_ast_untyped + +from s2dm import log +from s2dm.constants.directive import Directive, DirectiveArgument +from s2dm.exporters.utils.directive import has_given_directive + +DIRECTIVE_NAME = Directive.RETAINED_DEFINITIONS.value +ALL = "__all__" + + +@dataclass(frozen=True) +class RetainedDefinitions: + """Definitions a selection query asks to retain beyond the ones its fields reference. + + A value of None means the argument was absent and the existing dependency-based behavior + applies. A value of ALL means an empty list was given, which retains every definition of + that kind. + """ + + enums: dict[str, list[str] | str] | str | None = None + scalars: list[str] | str | None = None + directives: list[str] | str | None = None + + @property + def is_empty(self) -> bool: + return self.enums is None and self.scalars is None and self.directives is None + + +def _name_list(value: Any, argument: str) -> list[str] | str: + """Read a [String!] argument, where an empty list stands for every definition of that kind.""" + if not isinstance(value, list): + raise ValueError(f"@{DIRECTIVE_NAME}: '{argument}' must be a list of names") + if not value: + return ALL + non_strings = [entry for entry in value if not isinstance(entry, str)] + if non_strings: + raise ValueError(f"@{DIRECTIVE_NAME}: '{argument}' must contain only names") + return list(dict.fromkeys(value)) + + +def _enum_selection(value: Any) -> dict[str, list[str] | str] | str: + """Read the enums argument, which maps an enum name to the values to retain.""" + if isinstance(value, list): + if value: + raise ValueError(f"@{DIRECTIVE_NAME}: 'enums' must map enum names to values") + return ALL + if not isinstance(value, dict): + raise ValueError(f"@{DIRECTIVE_NAME}: 'enums' must map enum names to values") + if not value: + return ALL + + selection: dict[str, list[str] | str] = {} + for enum_name, values in value.items(): + selection[enum_name] = _name_list(values, f"enums.{enum_name}") + return selection + + +def _read_directive_arguments(node: Any) -> RetainedDefinitions: + arguments = {argument.name.value: value_from_ast_untyped(argument.value) for argument in node.arguments} + + unknown = sorted(set(arguments) - {"enums", "scalars", "directives"}) + if unknown: + raise ValueError(f"@{DIRECTIVE_NAME}: unknown argument(s) {unknown}") + + return RetainedDefinitions( + enums=_enum_selection(arguments["enums"]) if "enums" in arguments else None, + scalars=_name_list(arguments["scalars"], "scalars") if "scalars" in arguments else None, + directives=_name_list(arguments["directives"], "directives") if "directives" in arguments else None, + ) + + +def extract_retained_definitions(document: DocumentNode) -> tuple[DocumentNode, RetainedDefinitions]: + """Remove @retainedDefinitions from every operation and read it from the first query operation. + + The directive is defined by S2DM rather than by the model, so it is taken out of the document + before the document is validated against the schema. Later operations are stripped as well, + since only the first query operation is read. + + Args: + document: The parsed selection query document. + + Returns: + The document without the directive, and the definitions it asked to retain. + """ + retained = RetainedDefinitions() + seen_first_query = False + definitions = [] + + for definition in document.definitions: + if not isinstance(definition, OperationDefinitionNode): + definitions.append(definition) + continue + + applied = [d for d in definition.directives if d.name.value == Directive.RETAINED_DEFINITIONS] + is_query = definition.operation.value == "query" + + if applied and is_query and not seen_first_query: + if len(applied) > 1: + raise ValueError(f"@{DIRECTIVE_NAME} is applied more than once on one operation") + retained = _read_directive_arguments(applied[0]) + elif applied: + log.warning(f"Ignoring @{DIRECTIVE_NAME} outside the first query operation") + + seen_first_query = seen_first_query or is_query + + if applied: + remaining = tuple(d for d in definition.directives if d.name.value != Directive.RETAINED_DEFINITIONS) + definition = OperationDefinitionNode( + operation=definition.operation, + name=definition.name, + variable_definitions=definition.variable_definitions, + directives=remaining, + selection_set=definition.selection_set, + loc=definition.loc, + ) + definitions.append(definition) + + return DocumentNode(definitions=tuple(definitions), loc=document.loc), retained + + +def validate_retained_definitions(schema: GraphQLSchema, retained: RetainedDefinitions) -> None: + """Check every name, kind and enum value in the selection against the source model. + + Args: + schema: The unfiltered schema the selection is written against. + retained: The definitions the selection query asked to retain. + + Raises: + ValueError: If any name is missing, is of the wrong kind, or names an absent enum value. + """ + errors: list[str] = [] + + def check_kind(names: list[str] | str, kind: type, label: str) -> None: + if isinstance(names, str): + return + for name in names: + type_definition = schema.type_map.get(name) + if type_definition is None: + errors.append(f"'{name}' is not defined in the model") + elif not isinstance(type_definition, kind): + errors.append(f"'{name}' is not {label}") + + check_kind(retained.scalars or [], GraphQLScalarType, "a scalar") + + if isinstance(retained.enums, dict): + check_kind(list(retained.enums), GraphQLEnumType, "an enum") + for enum_name, values in retained.enums.items(): + enum_type = schema.type_map.get(enum_name) + if not isinstance(enum_type, GraphQLEnumType) or isinstance(values, str): + continue + missing = [value for value in values if value not in enum_type.values] + if missing: + errors.append(f"enum '{enum_name}' has no value(s) {sorted(missing)}") + + if isinstance(retained.directives, list): + defined = {directive.name for directive in schema.directives} + errors.extend( + f"directive '@{name}' is not defined in the model" for name in retained.directives if name not in defined + ) + + if errors: + raise ValueError(f"@{DIRECTIVE_NAME} validation failed:\n" + "\n".join(f" - {error}" for error in errors)) + + +def retained_type_names(schema: GraphQLSchema, retained: RetainedDefinitions) -> list[str]: + """Names of the scalar and enum types the selection asks to retain regardless of references.""" + names: list[str] = [] + + if retained.scalars == ALL: + names += [name for name, t in schema.type_map.items() if isinstance(t, GraphQLScalarType)] + elif retained.scalars: + names += list(retained.scalars) + + if retained.enums == ALL: + names += [name for name, t in schema.type_map.items() if isinstance(t, GraphQLEnumType)] + elif isinstance(retained.enums, dict): + names += list(retained.enums) + + return [name for name in names if not name.startswith("__")] + + +def retained_directive_names(schema: GraphQLSchema, retained: RetainedDefinitions) -> list[str]: + """Names of the directives the selection asks to retain regardless of use.""" + if retained.directives == ALL: + return [directive.name for directive in schema.directives] + return list(retained.directives or []) + + +def _removed_values(schema: GraphQLSchema, retained: RetainedDefinitions) -> dict[str, set[str]]: + if not isinstance(retained.enums, dict): + return {} + + removed: dict[str, set[str]] = {} + for enum_name, values in retained.enums.items(): + enum_type = schema.type_map.get(enum_name) + if not isinstance(enum_type, GraphQLEnumType) or isinstance(values, str): + continue + dropped = {name for name in enum_type.values if name not in values} + if dropped: + removed[enum_name] = dropped + return removed + + +def _reject_values_in_use(schema: GraphQLSchema, removed: dict[str, set[str]]) -> None: + """Refuse a selection that removes a value a retained default or directive argument needs.""" + conflicts: list[str] = [] + + def check_default(owner: str, value_type: Any, default: Any) -> None: + if not isinstance(value_type, GraphQLEnumType) or not isinstance(default, str): + return + if default in removed.get(value_type.name, set()): + conflicts.append(f"{owner} defaults to '{default}'") + + for type_name, type_definition in schema.type_map.items(): + if type_name.startswith("__"): + continue + if isinstance(type_definition, GraphQLObjectType | GraphQLInterfaceType): + for field_name, field in type_definition.fields.items(): + for argument_name, argument in field.args.items(): + owner = f"{type_name}.{field_name}({argument_name}:)" + check_default(owner, get_named_type(argument.type), argument.default_value) + elif isinstance(type_definition, GraphQLInputObjectType): + for field_name, input_field in type_definition.fields.items(): + owner = f"{type_name}.{field_name}" + check_default(owner, get_named_type(input_field.type), input_field.default_value) + + argument_types = { + directive.name: {name: get_named_type(argument.type) for name, argument in directive.args.items()} + for directive in schema.directives + } + for directive in schema.directives: + for argument_name, argument in directive.args.items(): + owner = f"@{directive.name}({argument_name}:)" + check_default(owner, get_named_type(argument.type), argument.default_value) + + for owner, node in _directive_carriers(schema): + for applied in node.directives: + for argument in applied.arguments: + value_type = argument_types.get(applied.name.value, {}).get(argument.name.value) + if not isinstance(value_type, GraphQLEnumType) or not isinstance(argument.value, EnumValueNode): + continue + if argument.value.value in removed.get(value_type.name, set()): + conflicts.append(f"@{applied.name.value}({argument.name.value}:) on {owner}") + + if conflicts: + raise ValueError( + f"@{DIRECTIVE_NAME} removes enum values that are still in use:\n" + + "\n".join(f" - {conflict}" for conflict in sorted(conflicts)) + ) + + +def _directive_carriers(schema: GraphQLSchema) -> list[tuple[str, Any]]: + """Every AST node in the schema that can carry applied directives, with a label for messages.""" + carriers: list[tuple[str, Any]] = [] + for type_name, type_definition in schema.type_map.items(): + if type_name.startswith("__"): + continue + if type_definition.ast_node is not None: + carriers.append((type_name, type_definition.ast_node)) + fields = getattr(type_definition, "fields", None) + if not fields: + continue + for field_name, field in fields.items(): + if field.ast_node is not None: + carriers.append((f"{type_name}.{field_name}", field.ast_node)) + return carriers + + +def _tag_dimensions(source_type: Any) -> list[str] | None: + """Enum type name per instance tag dimension, in the order the instances are built in.""" + if isinstance(source_type, GraphQLEnumType): + return [source_type.name] + if not isinstance(source_type, GraphQLObjectType): + return None + + dimensions: list[str] = [] + for field in source_type.fields.values(): + enum_type = get_named_type(field.type) + if not isinstance(enum_type, GraphQLEnumType): + return None + dimensions.append(enum_type.name) + return dimensions + + +def _exclude_nodes(schema: GraphQLSchema, source_name: str) -> list[Any]: + """Every AST node carrying an @instanceTag exclude list for the given instance tag source.""" + nodes: list[Any] = [] + source_type = schema.type_map.get(source_name) + if source_type is not None and source_type.ast_node is not None: + nodes.append(source_type.ast_node) + + for type_definition in schema.type_map.values(): + fields = getattr(type_definition, "fields", None) + if not fields: + continue + for field in fields.values(): + if field.ast_node is None or get_named_type(field.type).name != source_name: + continue + nodes.append(field.ast_node) + return nodes + + +def _prune_instance_tag_excludes(schema: GraphQLSchema, removed: dict[str, set[str]]) -> None: + """Drop the exclude entries that name a removed value, since they can no longer match. + + An exclude entry asserts that one instance is absent rather than consuming a value, so + removing a value it names leaves the entry vacuous rather than broken. + """ + for source_name, source_type in list(schema.type_map.items()): + if source_name.startswith("__") or not isinstance(source_type, GraphQLObjectType | GraphQLEnumType): + continue + if not has_given_directive(source_type, Directive.INSTANCE_TAG): + continue + dimensions = _tag_dimensions(source_type) + if dimensions is None: + continue + + for node in _exclude_nodes(schema, source_name): + for applied in node.directives: + if applied.name.value != Directive.INSTANCE_TAG: + continue + for argument in applied.arguments: + if argument.name.value != DirectiveArgument.EXCLUDE: + continue + if not isinstance(argument.value, ListValueNode): + continue + kept = tuple( + entry for entry in argument.value.values if not _entry_is_vacuous(entry, dimensions, removed) + ) + if len(kept) != len(argument.value.values): + log.debug(f"Dropping vacuous @{Directive.INSTANCE_TAG.value} exclude entries on {source_name}") + argument.value = ListValueNode(values=kept, loc=argument.value.loc) + + +def _entry_is_vacuous(entry: Any, dimensions: list[str], removed: dict[str, set[str]]) -> bool: + if not isinstance(entry, StringValueNode): + return False + segments = entry.value.split(".") + if len(segments) != len(dimensions): + return False + pairs = zip(segments, dimensions, strict=True) + return any(segment in removed.get(enum_name, set()) for segment, enum_name in pairs) + + +def apply_enum_value_selection(schema: GraphQLSchema, retained: RetainedDefinitions) -> None: + """Narrow the retained enums to their selected values and drop the exclude entries that follow. + + Args: + schema: The schema being filtered, modified in place. + retained: The definitions the selection query asked to retain. + + Raises: + ValueError: If a removed value is still needed by a retained default or directive argument. + """ + removed = _removed_values(schema, retained) + if not removed: + return + + _reject_values_in_use(schema, removed) + + for enum_name, dropped in removed.items(): + enum_type = cast(GraphQLEnumType, schema.type_map[enum_name]) + for value_name in dropped: + del enum_type.values[value_name] + log.debug(f"Retained {len(enum_type.values)} of {len(enum_type.values) + len(dropped)} {enum_name} values") + + _prune_instance_tag_excludes(schema, removed) diff --git a/src/s2dm/exporters/utils/schema_loader.py b/src/s2dm/exporters/utils/schema_loader.py index 37935f42..37af9dc9 100644 --- a/src/s2dm/exporters/utils/schema_loader.py +++ b/src/s2dm/exporters/utils/schema_loader.py @@ -53,6 +53,13 @@ from s2dm.exporters.utils.instance_tag import expand_instances_in_schema, is_valid_instance_tag_field from s2dm.exporters.utils.naming import apply_naming_to_schema, convert_name, load_naming_config from s2dm.exporters.utils.naming_config import ContextType, ElementType, NamingConventionConfig, get_case_for_element +from s2dm.exporters.utils.retained_definitions import ( + apply_enum_value_selection, + extract_retained_definitions, + retained_directive_names, + retained_type_names, + validate_retained_definitions, +) from s2dm.exporters.utils.violations import ConstraintViolation, Severity from s2dm.ledger import Ledger, annotate_schema_with_ledger from s2dm.tools.constraint_checker import ConstraintChecker @@ -574,6 +581,9 @@ def prune_schema_using_query_selection( if not schema.query_type: raise ValueError("Schema has no query type defined") + document, retained = extract_retained_definitions(document) + validate_retained_definitions(schema, retained) + _validate_schema(schema, document) fields_to_keep: dict[str, set[str]] = {} @@ -717,6 +727,11 @@ def collect_selections(type_name: str, selection_set: SelectionSetNode) -> None: query_operation = query_operations[0] collect_selections(schema.query_type.name, query_operation.selection_set) + for type_name in retained_type_names(schema, retained): + keep_type(type_name) + for directive_name in retained_directive_names(schema, retained): + keep_directive(directive_name) + while pending_types: type_name = pending_types.pop() for directive_name in directives_on_type(type_name): @@ -744,6 +759,8 @@ def collect_selections(type_name: str, selection_set: SelectionSetNode) -> None: schema.directives = tuple(directive for directive in schema.directives if directive.name in directives_used) + apply_enum_value_selection(schema, retained) + log.debug(f"Composed filtered schema with {len(fields_to_keep)} object types") return schema diff --git a/tests/data/retained_definitions_schema.graphql b/tests/data/retained_definitions_schema.graphql new file mode 100644 index 00000000..c2c1325e --- /dev/null +++ b/tests/data/retained_definitions_schema.graphql @@ -0,0 +1,35 @@ +directive @instanceTag(exclude: [String!]) on OBJECT | FIELD_DEFINITION | ENUM +directive @range(min: Float, max: Float) on FIELD_DEFINITION +directive @confidential on FIELD_DEFINITION + +scalar DateTime + +enum RowEnum { ROW1 ROW2 } +enum PositionEnum { LEFT MIDDLE RIGHT } +enum SeatMaterial { CLOTH LEATHER VINYL } +enum VelocityUnit { KILOM_PER_HR MI_PER_HR M_PER_SEC } + +type CabinPosition @instanceTag { + row: RowEnum! + position: PositionEnum! +} + +type Seat { + cabinPosition: CabinPosition @instanceTag(exclude: ["ROW1.MIDDLE", "ROW2.LEFT"]) + isOccupied: Boolean + material: SeatMaterial + lastAdjusted: DateTime +} + +type Cabin { + seats: [Seat] +} + +type Vehicle { + cabin: Cabin + averageSpeed(unit: VelocityUnit = KILOM_PER_HR): Float @range(min: 0, max: 300) +} + +type Query { + vehicle: Vehicle +} diff --git a/tests/test_retained_definitions.py b/tests/test_retained_definitions.py new file mode 100644 index 00000000..584ddf5a --- /dev/null +++ b/tests/test_retained_definitions.py @@ -0,0 +1,147 @@ +"""Tests for the @retainedDefinitions selection query directive.""" + +from pathlib import Path +from typing import cast + +import pytest +from graphql import GraphQLEnumType, GraphQLObjectType, GraphQLSchema, parse +from graphql import print_schema as print_graphql_schema + +from s2dm.exporters.utils.schema_loader import ( + load_and_process_schema, + load_schema, + prune_schema_using_query_selection, +) + +SCHEMA_PATH = Path("tests/data/retained_definitions_schema.graphql") + + +def prune(query: str, expanded_instances: bool = False) -> GraphQLSchema: + schema = load_schema([SCHEMA_PATH]) + return prune_schema_using_query_selection(schema, parse(query), expanded_instances) + + +SEATS = "vehicle { cabin { seats { isOccupied } } }" + + +class TestWithoutTheDirective: + def test_unreferenced_definitions_are_dropped(self) -> None: + schema = prune(f"query Selection {{ {SEATS} }}") + + assert "DateTime" not in schema.type_map + assert "SeatMaterial" not in schema.type_map + assert "confidential" not in {directive.name for directive in schema.directives} + + def test_a_directive_on_a_pruned_field_is_still_retained(self) -> None: + """Existing behavior: directives are collected from a type before its fields are deleted.""" + schema = prune(f"query Selection {{ {SEATS} }}") + + assert "range" in {directive.name for directive in schema.directives} + + +class TestRetention: + def test_unreferenced_scalar_is_retained(self) -> None: + schema = prune(f'query Selection @retainedDefinitions(scalars: ["DateTime"]) {{ {SEATS} }}') + + assert "DateTime" in schema.type_map + + def test_unapplied_directive_is_retained(self) -> None: + schema = prune(f'query Selection @retainedDefinitions(directives: ["confidential"]) {{ {SEATS} }}') + + assert "confidential" in {directive.name for directive in schema.directives} + + def test_empty_list_retains_every_definition_of_that_kind(self) -> None: + schema = prune(f"query Selection @retainedDefinitions(scalars: [], enums: []) {{ {SEATS} }}") + + assert "DateTime" in schema.type_map + assert "SeatMaterial" in schema.type_map + assert "VelocityUnit" in schema.type_map + + def test_unreferenced_enum_is_retained_and_narrowed(self) -> None: + schema = prune( + f"query Selection @retainedDefinitions(enums: {{ SeatMaterial: [CLOTH, LEATHER] }}) {{ {SEATS} }}" + ) + + enum_type = schema.type_map["SeatMaterial"] + assert isinstance(enum_type, GraphQLEnumType) + assert list(enum_type.values) == ["CLOTH", "LEATHER"] + + def test_empty_value_list_keeps_every_value(self) -> None: + schema = prune(f"query Selection @retainedDefinitions(enums: {{ SeatMaterial: [] }}) {{ {SEATS} }}") + + enum_type = schema.type_map["SeatMaterial"] + assert isinstance(enum_type, GraphQLEnumType) + assert list(enum_type.values) == ["CLOTH", "LEATHER", "VINYL"] + + def test_the_directive_does_not_reach_the_filtered_schema(self) -> None: + schema = prune(f'query Selection @retainedDefinitions(scalars: ["DateTime"]) {{ {SEATS} }}') + + assert "retainedDefinitions" not in print_graphql_schema(schema) + + +class TestValuesInUse: + def test_a_retained_default_refuses_its_value_being_removed(self) -> None: + query = ( + "query Selection @retainedDefinitions(enums: { VelocityUnit: [MI_PER_HR] }) { vehicle { averageSpeed } }" + ) + + with pytest.raises(ValueError, match="Vehicle.averageSpeed"): + prune(query) + + def test_a_pruned_default_does_not_hold_its_value(self) -> None: + query = f"query Selection @retainedDefinitions(enums: {{ VelocityUnit: [MI_PER_HR] }}) {{ {SEATS} }}" + + schema = prune(query) + + enum_type = schema.type_map["VelocityUnit"] + assert isinstance(enum_type, GraphQLEnumType) + assert list(enum_type.values) == ["MI_PER_HR"] + + +class TestValidation: + @pytest.mark.parametrize( + ("selection", "message"), + [ + ('scalars: ["Timestamp"]', "'Timestamp' is not defined in the model"), + ("enums: { Vehicle: [] }", "'Vehicle' is not an enum"), + ("enums: { SeatMaterial: [SUEDE] }", "has no value"), + ('directives: ["constraint"]', "is not defined in the model"), + ], + ) + def test_invalid_selections_are_reported(self, selection: str, message: str) -> None: + with pytest.raises(ValueError, match=message): + prune(f"query Selection @retainedDefinitions({selection}) {{ {SEATS} }}") + + +def expand(tmp_path: Path, query: str, schema_path: Path = SCHEMA_PATH) -> dict[str, list[str]]: + """Filter and expand the fixture, returning the generated seat instances by row.""" + query_file = tmp_path / "query.graphql" + query_file.write_text(query) + annotated_schema, _, _ = load_and_process_schema([schema_path], None, query_file, None, True) + return { + name.split("_")[1]: sorted(cast(GraphQLObjectType, type_definition).fields) + for name, type_definition in annotated_schema.schema.type_map.items() + if name.startswith("Seat_") and name.endswith("_Position") + } + + +class TestInstanceTagDimensions: + def test_the_model_excludes_two_instances(self, tmp_path: Path) -> None: + instances = expand(tmp_path, f"query Selection {{ {SEATS} }}") + + assert instances == {"ROW1": ["LEFT", "RIGHT"], "ROW2": ["MIDDLE", "RIGHT"]} + + def test_narrowing_a_dimension_reduces_the_instances(self, tmp_path: Path) -> None: + query = f"query Selection @retainedDefinitions(enums: {{ PositionEnum: [LEFT, RIGHT] }}) {{ {SEATS} }}" + + instances = expand(tmp_path, query) + + assert instances == {"ROW1": ["LEFT", "RIGHT"], "ROW2": ["RIGHT"]} + + def test_a_misspelled_exclude_entry_still_fails(self, tmp_path: Path) -> None: + schema_path = tmp_path / "schema.graphql" + schema_path.write_text(SCHEMA_PATH.read_text().replace('"ROW1.MIDDLE"', '"ROW1.MIDLE"')) + query = f"query Selection @retainedDefinitions(enums: {{ PositionEnum: [LEFT, RIGHT] }}) {{ {SEATS} }}" + + with pytest.raises(ValueError, match="match no instance"): + expand(tmp_path, query, schema_path) From 83d0399bde4d44258c5a219ba186ad19c52f36bc Mon Sep 17 00:00:00 2001 From: Baris Akcam Date: Fri, 25 Sep 2026 14:50:41 +0200 Subject: [PATCH 02/11] docs(selection): document retaining unreferenced definitions Cover @retainedDefinitions in the selection query section: the three forms each argument takes, selecting enum values, and what narrowing an instance tag dimension does to the generated instances. --- docs-gen/content/docs/tools/cli.md | 57 ++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/docs-gen/content/docs/tools/cli.md b/docs-gen/content/docs/tools/cli.md index 4af3296c..bd8a6f2e 100644 --- a/docs-gen/content/docs/tools/cli.md +++ b/docs-gen/content/docs/tools/cli.md @@ -2605,6 +2605,63 @@ The filtered schema will include: **Note:** The query must be valid against the schema. Root fields in the query (e.g., `vehicle`) must exist in the `Query` type of the schema. +#### Retaining Unreferenced Definitions + +Filtering keeps only the definitions the selected fields reach, so a scalar nobody selected, or a directive that does not appear on the retained slice, is dropped. The `@retainedDefinitions` directive on the selection query keeps them anyway, and selects which values of an enum to keep. + +```graphql +query Selection + @retainedDefinitions( + enums: { SeatMaterial: [CLOTH, LEATHER] } + scalars: ["DateTime"] + directives: ["cardinality"] + ) { + vehicle { + cabin { + seats { + isOccupied + } + } + } +} +``` + +Each argument accepts three forms: + +- Absent: only referenced definitions are kept, so a query without the directive filters exactly as it did before. +- An empty list, such as `scalars: []`: every definition of that kind is kept. +- A list of names: those definitions are kept in addition to the referenced ones. + +Selecting a directive retains its definition only. It does not apply the directive to any field or type. + +##### Enum Values + +An entry in `enums` maps an enum name to the values to keep. An empty list keeps every value, which is how to retain an enum that nothing references. + +```graphql +enums: { + FuelType: [] # kept with all of its values + VehicleStatus: [ACTIVE, INACTIVE] # the remaining values are dropped +} +``` + +Removing a value that a retained default or directive argument still needs is refused: + +```text +@retainedDefinitions removes enum values that are still in use: + - Vehicle.averageSpeed(unit:) defaults to 'KILOM_PER_HR' +``` + +That default belongs to a selected field. Had `averageSpeed` not been selected, the field and its default would have been filtered out and the value could be removed. + +##### Enum Values and Instance Tags + +An instance tag dimension is an enum, so selecting its values changes how many instances are generated. A seat list expanded over `RowEnum` and `PositionEnum` produces one branch per combination, and narrowing `PositionEnum` to two values produces two branches at that level. + +When a value is removed, the `@instanceTag(exclude: ...)` entries naming it are dropped as well, since they can no longer match an instance. Entries naming no removed value continue to apply. Given a model that excludes `ROW1.MIDDLE` and `ROW2.LEFT`, a selection of `PositionEnum: [LEFT, RIGHT]` produces `ROW1` with `LEFT` and `RIGHT`, and `ROW2` with `RIGHT` alone. + +**Note:** `@retainedDefinitions` is defined by s2dm rather than by the model, and is removed from the query before the query is validated against the schema. A GraphQL tool that does not know about it reports an unknown directive for a selection query that uses it. + ### Root Type Filtering All export commands and the compose command support the `--root-type` flag to filter the schema to only a specific type and its transitive dependencies. From 9f82f2c65c59ed38a8800b863249fdf51f088eb6 Mon Sep 17 00:00:00 2001 From: Baris Akcam Date: Fri, 25 Sep 2026 15:26:16 +0200 Subject: [PATCH 03/11] feat(selection): warn when an instance tag expands to no instances A selection that removes the last surviving combination left the container type empty with nothing said. The generated JSON Schema then carries no properties alongside additionalProperties false, which admits only the empty object, so the filtered schema rejects the data it was derived from. Warn rather than fail: the same state is reachable by excluding every instance in the model, and that has always been allowed. --- src/s2dm/exporters/utils/instance_tag.py | 5 +++++ tests/test_retained_definitions.py | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/s2dm/exporters/utils/instance_tag.py b/src/s2dm/exporters/utils/instance_tag.py index 23609b95..b5664ebe 100644 --- a/src/s2dm/exporters/utils/instance_tag.py +++ b/src/s2dm/exporters/utils/instance_tag.py @@ -411,6 +411,11 @@ def case_for(element: ElementType, context: ContextType | None) -> CaseFormat | field_metadata: dict[tuple[str, str], FieldMetadata] = {} for expandable in expandable_fields: instances = included_instances(expandable, instance_tag_case) + if not instances: + log.warning( + f"'{expandable.parent_type.name}.{expandable.field_name}' expands to no instances; " + f"every combination is excluded or filtered out" + ) top_type, intermediate_types = _build_instance_types( expandable.base_type, list(expandable.instance_tag_dict), instances, expandable.leaf_nullable ) diff --git a/tests/test_retained_definitions.py b/tests/test_retained_definitions.py index 584ddf5a..c8c62fab 100644 --- a/tests/test_retained_definitions.py +++ b/tests/test_retained_definitions.py @@ -7,6 +7,7 @@ from graphql import GraphQLEnumType, GraphQLObjectType, GraphQLSchema, parse from graphql import print_schema as print_graphql_schema +from s2dm import log from s2dm.exporters.utils.schema_loader import ( load_and_process_schema, load_schema, @@ -145,3 +146,19 @@ def test_a_misspelled_exclude_entry_still_fails(self, tmp_path: Path) -> None: with pytest.raises(ValueError, match="match no instance"): expand(tmp_path, query, schema_path) + + def test_a_selection_that_excludes_every_instance_warns( + self, tmp_path: Path, caplog: pytest.LogCaptureFixture + ) -> None: + schema_path = tmp_path / "schema.graphql" + schema_path.write_text(SCHEMA_PATH.read_text().replace('"ROW2.LEFT"', '"ROW2.MIDDLE"')) + query = f"query Selection @retainedDefinitions(enums: {{ PositionEnum: [MIDDLE] }}) {{ {SEATS} }}" + + log.addHandler(caplog.handler) + try: + instances = expand(tmp_path, query, schema_path) + finally: + log.removeHandler(caplog.handler) + + assert instances == {} + assert "Cabin.seats' expands to no instances" in caplog.text From fd3d829a271c59000d68488dd7e58d41802b54af Mon Sep 17 00:00:00 2001 From: Baris Akcam Date: Mon, 28 Sep 2026 11:47:46 +0200 Subject: [PATCH 04/11] refactor(selection)!: rename the directive to @pick and keep enums whole Enums can no longer be narrowed to a subset of their values. Selecting values let a query change how an instance tag unfolds, which is what @instanceTag(exclude: ...) is for and belongs to the model rather than to the query. Picking an enum now keeps it whole, so filtering never changes what an enum means. The enums argument becomes a plain list of names like the other two, which retires the EnumSelection scalar. Nothing can remove a value any more, so the rule refusing removals that a retained default still needed, and the rule dropping instance tag exclude entries left vacuous by a removal, both go with it. The warning for an instance tag that expands to no instances goes too: a selection can no longer empty the product, and reaching that state by excluding every instance in the model has always been allowed. --- docs-gen/content/docs/tools/cli.md | 47 +-- src/s2dm/api/routes/query_validate.py | 6 +- src/s2dm/constants/directive.py | 2 +- src/s2dm/exporters/utils/instance_tag.py | 5 - src/s2dm/exporters/utils/pick.py | 160 ++++++++ .../exporters/utils/retained_definitions.py | 383 ------------------ src/s2dm/exporters/utils/schema_loader.py | 21 +- ...ons_schema.graphql => pick_schema.graphql} | 9 - tests/test_pick.py | 92 +++++ tests/test_retained_definitions.py | 164 -------- 10 files changed, 275 insertions(+), 614 deletions(-) create mode 100644 src/s2dm/exporters/utils/pick.py delete mode 100644 src/s2dm/exporters/utils/retained_definitions.py rename tests/data/{retained_definitions_schema.graphql => pick_schema.graphql} (61%) create mode 100644 tests/test_pick.py delete mode 100644 tests/test_retained_definitions.py diff --git a/docs-gen/content/docs/tools/cli.md b/docs-gen/content/docs/tools/cli.md index bd8a6f2e..698ef611 100644 --- a/docs-gen/content/docs/tools/cli.md +++ b/docs-gen/content/docs/tools/cli.md @@ -2605,23 +2605,20 @@ The filtered schema will include: **Note:** The query must be valid against the schema. Root fields in the query (e.g., `vehicle`) must exist in the `Query` type of the schema. -#### Retaining Unreferenced Definitions +#### Picking Unreferenced Definitions -Filtering keeps only the definitions the selected fields reach, so a scalar nobody selected, or a directive that does not appear on the retained slice, is dropped. The `@retainedDefinitions` directive on the selection query keeps them anyway, and selects which values of an enum to keep. +Filtering keeps only the definitions the selected fields reach, so a scalar nobody selected, or a directive that does not appear on the retained slice, is dropped. The `@pick` directive on the selection query keeps them anyway. ```graphql query Selection - @retainedDefinitions( - enums: { SeatMaterial: [CLOTH, LEATHER] } + @pick( + enums: ["FuelType", "VehicleStatus"] scalars: ["DateTime"] - directives: ["cardinality"] + directives: ["constraint"] ) { vehicle { - cabin { - seats { - isOccupied - } - } + id + model } } ``` @@ -2632,35 +2629,11 @@ Each argument accepts three forms: - An empty list, such as `scalars: []`: every definition of that kind is kept. - A list of names: those definitions are kept in addition to the referenced ones. -Selecting a directive retains its definition only. It does not apply the directive to any field or type. - -##### Enum Values - -An entry in `enums` maps an enum name to the values to keep. An empty list keeps every value, which is how to retain an enum that nothing references. - -```graphql -enums: { - FuelType: [] # kept with all of its values - VehicleStatus: [ACTIVE, INACTIVE] # the remaining values are dropped -} -``` - -Removing a value that a retained default or directive argument still needs is refused: - -```text -@retainedDefinitions removes enum values that are still in use: - - Vehicle.averageSpeed(unit:) defaults to 'KILOM_PER_HR' -``` - -That default belongs to a selected field. Had `averageSpeed` not been selected, the field and its default would have been filtered out and the value could be removed. - -##### Enum Values and Instance Tags - -An instance tag dimension is an enum, so selecting its values changes how many instances are generated. A seat list expanded over `RowEnum` and `PositionEnum` produces one branch per combination, and narrowing `PositionEnum` to two values produces two branches at that level. +An enum is always kept whole. There is no way to select a subset of its values, so filtering never changes what an enum means. Use `@instanceTag(exclude: ...)` in the model to steer which instances an instance tag unfolds into. -When a value is removed, the `@instanceTag(exclude: ...)` entries naming it are dropped as well, since they can no longer match an instance. Entries naming no removed value continue to apply. Given a model that excludes `ROW1.MIDDLE` and `ROW2.LEFT`, a selection of `PositionEnum: [LEFT, RIGHT]` produces `ROW1` with `LEFT` and `RIGHT`, and `ROW2` with `RIGHT` alone. +Selecting a directive keeps its definition only. It does not apply the directive to any field or type. -**Note:** `@retainedDefinitions` is defined by s2dm rather than by the model, and is removed from the query before the query is validated against the schema. A GraphQL tool that does not know about it reports an unknown directive for a selection query that uses it. +**Note:** `@pick` is defined by s2dm rather than by the model, and is removed from the query before the query is validated against the schema. A GraphQL tool that does not know about it reports an unknown directive for a selection query that uses it. ### Root Type Filtering diff --git a/src/s2dm/api/routes/query_validate.py b/src/s2dm/api/routes/query_validate.py index 677a7b43..064c8a21 100644 --- a/src/s2dm/api/routes/query_validate.py +++ b/src/s2dm/api/routes/query_validate.py @@ -9,7 +9,7 @@ from s2dm.api.models.query_validate import ValidateQueryRequest from s2dm.api.services.response_service import execute_and_respond from s2dm.api.services.schema_service import path_for_content, process_schema_input, validate_schema_or_raise -from s2dm.exporters.utils.retained_definitions import extract_retained_definitions, validate_retained_definitions +from s2dm.exporters.utils.pick import extract_picked_definitions, validate_picked_definitions from s2dm.exporters.utils.schema_loader import load_schema router = APIRouter(responses=COMMON_RESPONSES) @@ -28,8 +28,8 @@ def process_request() -> list[str]: query_path = path_for_content(request.selection_query, "selection_query", ".graphql") query_text = query_path.read_text(encoding="utf-8") - query_document, retained = extract_retained_definitions(parse(query_text)) - validate_retained_definitions(schema, retained) + query_document, picked = extract_picked_definitions(parse(query_text)) + validate_picked_definitions(schema, picked) validation_errors = validate(schema, query_document) diff --git a/src/s2dm/constants/directive.py b/src/s2dm/constants/directive.py index 9165b283..2faecb1d 100644 --- a/src/s2dm/constants/directive.py +++ b/src/s2dm/constants/directive.py @@ -14,7 +14,7 @@ class Directive(str, Enum): REFERENCE = "reference" VSPEC = "vspec" MODL = "modl" - RETAINED_DEFINITIONS = "retainedDefinitions" + PICK = "pick" class BuiltInDirective(str, Enum): diff --git a/src/s2dm/exporters/utils/instance_tag.py b/src/s2dm/exporters/utils/instance_tag.py index b5664ebe..23609b95 100644 --- a/src/s2dm/exporters/utils/instance_tag.py +++ b/src/s2dm/exporters/utils/instance_tag.py @@ -411,11 +411,6 @@ def case_for(element: ElementType, context: ContextType | None) -> CaseFormat | field_metadata: dict[tuple[str, str], FieldMetadata] = {} for expandable in expandable_fields: instances = included_instances(expandable, instance_tag_case) - if not instances: - log.warning( - f"'{expandable.parent_type.name}.{expandable.field_name}' expands to no instances; " - f"every combination is excluded or filtered out" - ) top_type, intermediate_types = _build_instance_types( expandable.base_type, list(expandable.instance_tag_dict), instances, expandable.leaf_nullable ) diff --git a/src/s2dm/exporters/utils/pick.py b/src/s2dm/exporters/utils/pick.py new file mode 100644 index 00000000..e6b1befd --- /dev/null +++ b/src/s2dm/exporters/utils/pick.py @@ -0,0 +1,160 @@ +"""Extraction and validation of the @pick selection query directive.""" + +from dataclasses import dataclass +from typing import Any + +from graphql import ( + DocumentNode, + GraphQLEnumType, + GraphQLScalarType, + GraphQLSchema, +) +from graphql.language.ast import OperationDefinitionNode +from graphql.utilities import value_from_ast_untyped + +from s2dm import log +from s2dm.constants.directive import Directive + +DIRECTIVE_NAME = Directive.PICK.value +ALL = "__all__" +ARGUMENTS = ("enums", "scalars", "directives") + + +@dataclass(frozen=True) +class PickedDefinitions: + """Definitions a selection query asks to keep beyond the ones its fields reference. + + None means the argument was absent, so the existing dependency-based behavior applies. + ALL means an empty list was given, which keeps every definition of that kind. Enums are + always kept whole; there is no way to select a subset of their values. + """ + + enums: list[str] | str | None = None + scalars: list[str] | str | None = None + directives: list[str] | str | None = None + + +def _name_list(value: Any, argument: str) -> list[str] | str: + """Read a [String!] argument, where an empty list stands for every definition of that kind.""" + if not isinstance(value, list): + raise ValueError(f"@{DIRECTIVE_NAME}: '{argument}' must be a list of names") + if not value: + return ALL + if any(not isinstance(entry, str) for entry in value): + raise ValueError(f"@{DIRECTIVE_NAME}: '{argument}' must contain only names") + return list(dict.fromkeys(value)) + + +def _read_directive_arguments(node: Any) -> PickedDefinitions: + arguments = {argument.name.value: value_from_ast_untyped(argument.value) for argument in node.arguments} + + unknown = sorted(set(arguments) - set(ARGUMENTS)) + if unknown: + raise ValueError(f"@{DIRECTIVE_NAME}: unknown argument(s) {unknown}") + + picked = {name: _name_list(arguments[name], name) for name in ARGUMENTS if name in arguments} + return PickedDefinitions(**picked) + + +def extract_picked_definitions(document: DocumentNode) -> tuple[DocumentNode, PickedDefinitions]: + """Remove @pick from every operation and read it from the first query operation. + + The directive is defined by S2DM rather than by the model, so it is taken out of the document + before the document is validated against the schema. Later operations are stripped as well, + since only the first query operation is read. + + Args: + document: The parsed selection query document. + + Returns: + The document without the directive, and the definitions it asked to keep. + """ + picked = PickedDefinitions() + seen_first_query = False + definitions = [] + + for definition in document.definitions: + if not isinstance(definition, OperationDefinitionNode): + definitions.append(definition) + continue + + applied = [node for node in definition.directives if node.name.value == Directive.PICK] + is_query = definition.operation.value == "query" + + if applied and is_query and not seen_first_query: + if len(applied) > 1: + raise ValueError(f"@{DIRECTIVE_NAME} is applied more than once on one operation") + picked = _read_directive_arguments(applied[0]) + elif applied: + log.warning(f"Ignoring @{DIRECTIVE_NAME} outside the first query operation") + + seen_first_query = seen_first_query or is_query + + if applied: + remaining = tuple(node for node in definition.directives if node.name.value != Directive.PICK) + definition = OperationDefinitionNode( + operation=definition.operation, + name=definition.name, + variable_definitions=definition.variable_definitions, + directives=remaining, + selection_set=definition.selection_set, + loc=definition.loc, + ) + definitions.append(definition) + + return DocumentNode(definitions=tuple(definitions), loc=document.loc), picked + + +def validate_picked_definitions(schema: GraphQLSchema, picked: PickedDefinitions) -> None: + """Check every name and definition kind in the selection against the source model. + + Args: + schema: The unfiltered schema the selection is written against. + picked: The definitions the selection query asked to keep. + + Raises: + ValueError: If any name is missing from the model or is of the wrong kind. + """ + errors: list[str] = [] + + def check_kind(names: list[str] | str | None, kind: type, label: str) -> None: + if not isinstance(names, list): + return + for name in names: + type_definition = schema.type_map.get(name) + if type_definition is None: + errors.append(f"'{name}' is not defined in the model") + elif not isinstance(type_definition, kind): + errors.append(f"'{name}' is not {label}") + + check_kind(picked.scalars, GraphQLScalarType, "a scalar") + check_kind(picked.enums, GraphQLEnumType, "an enum") + + if isinstance(picked.directives, list): + defined = {directive.name for directive in schema.directives} + errors.extend( + f"directive '@{name}' is not defined in the model" for name in picked.directives if name not in defined + ) + + if errors: + raise ValueError(f"@{DIRECTIVE_NAME} validation failed:\n" + "\n".join(f" - {error}" for error in errors)) + + +def picked_type_names(schema: GraphQLSchema, picked: PickedDefinitions) -> list[str]: + """Names of the scalar and enum types the selection keeps regardless of references.""" + names: list[str] = [] + + for selection, kind in ((picked.scalars, GraphQLScalarType), (picked.enums, GraphQLEnumType)): + if selection == ALL: + names += [name for name, t in schema.type_map.items() if isinstance(t, kind)] + elif isinstance(selection, list): + names += selection + + return [name for name in names if not name.startswith("__")] + + +def picked_directive_names(schema: GraphQLSchema, picked: PickedDefinitions) -> list[str]: + """Names of the directives the selection keeps regardless of use.""" + if picked.directives == ALL: + return [directive.name for directive in schema.directives] + return list(picked.directives or []) diff --git a/src/s2dm/exporters/utils/retained_definitions.py b/src/s2dm/exporters/utils/retained_definitions.py deleted file mode 100644 index e5927672..00000000 --- a/src/s2dm/exporters/utils/retained_definitions.py +++ /dev/null @@ -1,383 +0,0 @@ -"""Extraction and validation of the @retainedDefinitions selection query directive.""" - -from dataclasses import dataclass -from typing import Any, cast - -from graphql import ( - DocumentNode, - GraphQLEnumType, - GraphQLInputObjectType, - GraphQLInterfaceType, - GraphQLObjectType, - GraphQLScalarType, - GraphQLSchema, - get_named_type, -) -from graphql.language.ast import EnumValueNode, ListValueNode, OperationDefinitionNode, StringValueNode -from graphql.utilities import value_from_ast_untyped - -from s2dm import log -from s2dm.constants.directive import Directive, DirectiveArgument -from s2dm.exporters.utils.directive import has_given_directive - -DIRECTIVE_NAME = Directive.RETAINED_DEFINITIONS.value -ALL = "__all__" - - -@dataclass(frozen=True) -class RetainedDefinitions: - """Definitions a selection query asks to retain beyond the ones its fields reference. - - A value of None means the argument was absent and the existing dependency-based behavior - applies. A value of ALL means an empty list was given, which retains every definition of - that kind. - """ - - enums: dict[str, list[str] | str] | str | None = None - scalars: list[str] | str | None = None - directives: list[str] | str | None = None - - @property - def is_empty(self) -> bool: - return self.enums is None and self.scalars is None and self.directives is None - - -def _name_list(value: Any, argument: str) -> list[str] | str: - """Read a [String!] argument, where an empty list stands for every definition of that kind.""" - if not isinstance(value, list): - raise ValueError(f"@{DIRECTIVE_NAME}: '{argument}' must be a list of names") - if not value: - return ALL - non_strings = [entry for entry in value if not isinstance(entry, str)] - if non_strings: - raise ValueError(f"@{DIRECTIVE_NAME}: '{argument}' must contain only names") - return list(dict.fromkeys(value)) - - -def _enum_selection(value: Any) -> dict[str, list[str] | str] | str: - """Read the enums argument, which maps an enum name to the values to retain.""" - if isinstance(value, list): - if value: - raise ValueError(f"@{DIRECTIVE_NAME}: 'enums' must map enum names to values") - return ALL - if not isinstance(value, dict): - raise ValueError(f"@{DIRECTIVE_NAME}: 'enums' must map enum names to values") - if not value: - return ALL - - selection: dict[str, list[str] | str] = {} - for enum_name, values in value.items(): - selection[enum_name] = _name_list(values, f"enums.{enum_name}") - return selection - - -def _read_directive_arguments(node: Any) -> RetainedDefinitions: - arguments = {argument.name.value: value_from_ast_untyped(argument.value) for argument in node.arguments} - - unknown = sorted(set(arguments) - {"enums", "scalars", "directives"}) - if unknown: - raise ValueError(f"@{DIRECTIVE_NAME}: unknown argument(s) {unknown}") - - return RetainedDefinitions( - enums=_enum_selection(arguments["enums"]) if "enums" in arguments else None, - scalars=_name_list(arguments["scalars"], "scalars") if "scalars" in arguments else None, - directives=_name_list(arguments["directives"], "directives") if "directives" in arguments else None, - ) - - -def extract_retained_definitions(document: DocumentNode) -> tuple[DocumentNode, RetainedDefinitions]: - """Remove @retainedDefinitions from every operation and read it from the first query operation. - - The directive is defined by S2DM rather than by the model, so it is taken out of the document - before the document is validated against the schema. Later operations are stripped as well, - since only the first query operation is read. - - Args: - document: The parsed selection query document. - - Returns: - The document without the directive, and the definitions it asked to retain. - """ - retained = RetainedDefinitions() - seen_first_query = False - definitions = [] - - for definition in document.definitions: - if not isinstance(definition, OperationDefinitionNode): - definitions.append(definition) - continue - - applied = [d for d in definition.directives if d.name.value == Directive.RETAINED_DEFINITIONS] - is_query = definition.operation.value == "query" - - if applied and is_query and not seen_first_query: - if len(applied) > 1: - raise ValueError(f"@{DIRECTIVE_NAME} is applied more than once on one operation") - retained = _read_directive_arguments(applied[0]) - elif applied: - log.warning(f"Ignoring @{DIRECTIVE_NAME} outside the first query operation") - - seen_first_query = seen_first_query or is_query - - if applied: - remaining = tuple(d for d in definition.directives if d.name.value != Directive.RETAINED_DEFINITIONS) - definition = OperationDefinitionNode( - operation=definition.operation, - name=definition.name, - variable_definitions=definition.variable_definitions, - directives=remaining, - selection_set=definition.selection_set, - loc=definition.loc, - ) - definitions.append(definition) - - return DocumentNode(definitions=tuple(definitions), loc=document.loc), retained - - -def validate_retained_definitions(schema: GraphQLSchema, retained: RetainedDefinitions) -> None: - """Check every name, kind and enum value in the selection against the source model. - - Args: - schema: The unfiltered schema the selection is written against. - retained: The definitions the selection query asked to retain. - - Raises: - ValueError: If any name is missing, is of the wrong kind, or names an absent enum value. - """ - errors: list[str] = [] - - def check_kind(names: list[str] | str, kind: type, label: str) -> None: - if isinstance(names, str): - return - for name in names: - type_definition = schema.type_map.get(name) - if type_definition is None: - errors.append(f"'{name}' is not defined in the model") - elif not isinstance(type_definition, kind): - errors.append(f"'{name}' is not {label}") - - check_kind(retained.scalars or [], GraphQLScalarType, "a scalar") - - if isinstance(retained.enums, dict): - check_kind(list(retained.enums), GraphQLEnumType, "an enum") - for enum_name, values in retained.enums.items(): - enum_type = schema.type_map.get(enum_name) - if not isinstance(enum_type, GraphQLEnumType) or isinstance(values, str): - continue - missing = [value for value in values if value not in enum_type.values] - if missing: - errors.append(f"enum '{enum_name}' has no value(s) {sorted(missing)}") - - if isinstance(retained.directives, list): - defined = {directive.name for directive in schema.directives} - errors.extend( - f"directive '@{name}' is not defined in the model" for name in retained.directives if name not in defined - ) - - if errors: - raise ValueError(f"@{DIRECTIVE_NAME} validation failed:\n" + "\n".join(f" - {error}" for error in errors)) - - -def retained_type_names(schema: GraphQLSchema, retained: RetainedDefinitions) -> list[str]: - """Names of the scalar and enum types the selection asks to retain regardless of references.""" - names: list[str] = [] - - if retained.scalars == ALL: - names += [name for name, t in schema.type_map.items() if isinstance(t, GraphQLScalarType)] - elif retained.scalars: - names += list(retained.scalars) - - if retained.enums == ALL: - names += [name for name, t in schema.type_map.items() if isinstance(t, GraphQLEnumType)] - elif isinstance(retained.enums, dict): - names += list(retained.enums) - - return [name for name in names if not name.startswith("__")] - - -def retained_directive_names(schema: GraphQLSchema, retained: RetainedDefinitions) -> list[str]: - """Names of the directives the selection asks to retain regardless of use.""" - if retained.directives == ALL: - return [directive.name for directive in schema.directives] - return list(retained.directives or []) - - -def _removed_values(schema: GraphQLSchema, retained: RetainedDefinitions) -> dict[str, set[str]]: - if not isinstance(retained.enums, dict): - return {} - - removed: dict[str, set[str]] = {} - for enum_name, values in retained.enums.items(): - enum_type = schema.type_map.get(enum_name) - if not isinstance(enum_type, GraphQLEnumType) or isinstance(values, str): - continue - dropped = {name for name in enum_type.values if name not in values} - if dropped: - removed[enum_name] = dropped - return removed - - -def _reject_values_in_use(schema: GraphQLSchema, removed: dict[str, set[str]]) -> None: - """Refuse a selection that removes a value a retained default or directive argument needs.""" - conflicts: list[str] = [] - - def check_default(owner: str, value_type: Any, default: Any) -> None: - if not isinstance(value_type, GraphQLEnumType) or not isinstance(default, str): - return - if default in removed.get(value_type.name, set()): - conflicts.append(f"{owner} defaults to '{default}'") - - for type_name, type_definition in schema.type_map.items(): - if type_name.startswith("__"): - continue - if isinstance(type_definition, GraphQLObjectType | GraphQLInterfaceType): - for field_name, field in type_definition.fields.items(): - for argument_name, argument in field.args.items(): - owner = f"{type_name}.{field_name}({argument_name}:)" - check_default(owner, get_named_type(argument.type), argument.default_value) - elif isinstance(type_definition, GraphQLInputObjectType): - for field_name, input_field in type_definition.fields.items(): - owner = f"{type_name}.{field_name}" - check_default(owner, get_named_type(input_field.type), input_field.default_value) - - argument_types = { - directive.name: {name: get_named_type(argument.type) for name, argument in directive.args.items()} - for directive in schema.directives - } - for directive in schema.directives: - for argument_name, argument in directive.args.items(): - owner = f"@{directive.name}({argument_name}:)" - check_default(owner, get_named_type(argument.type), argument.default_value) - - for owner, node in _directive_carriers(schema): - for applied in node.directives: - for argument in applied.arguments: - value_type = argument_types.get(applied.name.value, {}).get(argument.name.value) - if not isinstance(value_type, GraphQLEnumType) or not isinstance(argument.value, EnumValueNode): - continue - if argument.value.value in removed.get(value_type.name, set()): - conflicts.append(f"@{applied.name.value}({argument.name.value}:) on {owner}") - - if conflicts: - raise ValueError( - f"@{DIRECTIVE_NAME} removes enum values that are still in use:\n" - + "\n".join(f" - {conflict}" for conflict in sorted(conflicts)) - ) - - -def _directive_carriers(schema: GraphQLSchema) -> list[tuple[str, Any]]: - """Every AST node in the schema that can carry applied directives, with a label for messages.""" - carriers: list[tuple[str, Any]] = [] - for type_name, type_definition in schema.type_map.items(): - if type_name.startswith("__"): - continue - if type_definition.ast_node is not None: - carriers.append((type_name, type_definition.ast_node)) - fields = getattr(type_definition, "fields", None) - if not fields: - continue - for field_name, field in fields.items(): - if field.ast_node is not None: - carriers.append((f"{type_name}.{field_name}", field.ast_node)) - return carriers - - -def _tag_dimensions(source_type: Any) -> list[str] | None: - """Enum type name per instance tag dimension, in the order the instances are built in.""" - if isinstance(source_type, GraphQLEnumType): - return [source_type.name] - if not isinstance(source_type, GraphQLObjectType): - return None - - dimensions: list[str] = [] - for field in source_type.fields.values(): - enum_type = get_named_type(field.type) - if not isinstance(enum_type, GraphQLEnumType): - return None - dimensions.append(enum_type.name) - return dimensions - - -def _exclude_nodes(schema: GraphQLSchema, source_name: str) -> list[Any]: - """Every AST node carrying an @instanceTag exclude list for the given instance tag source.""" - nodes: list[Any] = [] - source_type = schema.type_map.get(source_name) - if source_type is not None and source_type.ast_node is not None: - nodes.append(source_type.ast_node) - - for type_definition in schema.type_map.values(): - fields = getattr(type_definition, "fields", None) - if not fields: - continue - for field in fields.values(): - if field.ast_node is None or get_named_type(field.type).name != source_name: - continue - nodes.append(field.ast_node) - return nodes - - -def _prune_instance_tag_excludes(schema: GraphQLSchema, removed: dict[str, set[str]]) -> None: - """Drop the exclude entries that name a removed value, since they can no longer match. - - An exclude entry asserts that one instance is absent rather than consuming a value, so - removing a value it names leaves the entry vacuous rather than broken. - """ - for source_name, source_type in list(schema.type_map.items()): - if source_name.startswith("__") or not isinstance(source_type, GraphQLObjectType | GraphQLEnumType): - continue - if not has_given_directive(source_type, Directive.INSTANCE_TAG): - continue - dimensions = _tag_dimensions(source_type) - if dimensions is None: - continue - - for node in _exclude_nodes(schema, source_name): - for applied in node.directives: - if applied.name.value != Directive.INSTANCE_TAG: - continue - for argument in applied.arguments: - if argument.name.value != DirectiveArgument.EXCLUDE: - continue - if not isinstance(argument.value, ListValueNode): - continue - kept = tuple( - entry for entry in argument.value.values if not _entry_is_vacuous(entry, dimensions, removed) - ) - if len(kept) != len(argument.value.values): - log.debug(f"Dropping vacuous @{Directive.INSTANCE_TAG.value} exclude entries on {source_name}") - argument.value = ListValueNode(values=kept, loc=argument.value.loc) - - -def _entry_is_vacuous(entry: Any, dimensions: list[str], removed: dict[str, set[str]]) -> bool: - if not isinstance(entry, StringValueNode): - return False - segments = entry.value.split(".") - if len(segments) != len(dimensions): - return False - pairs = zip(segments, dimensions, strict=True) - return any(segment in removed.get(enum_name, set()) for segment, enum_name in pairs) - - -def apply_enum_value_selection(schema: GraphQLSchema, retained: RetainedDefinitions) -> None: - """Narrow the retained enums to their selected values and drop the exclude entries that follow. - - Args: - schema: The schema being filtered, modified in place. - retained: The definitions the selection query asked to retain. - - Raises: - ValueError: If a removed value is still needed by a retained default or directive argument. - """ - removed = _removed_values(schema, retained) - if not removed: - return - - _reject_values_in_use(schema, removed) - - for enum_name, dropped in removed.items(): - enum_type = cast(GraphQLEnumType, schema.type_map[enum_name]) - for value_name in dropped: - del enum_type.values[value_name] - log.debug(f"Retained {len(enum_type.values)} of {len(enum_type.values) + len(dropped)} {enum_name} values") - - _prune_instance_tag_excludes(schema, removed) diff --git a/src/s2dm/exporters/utils/schema_loader.py b/src/s2dm/exporters/utils/schema_loader.py index 37af9dc9..67306a0d 100644 --- a/src/s2dm/exporters/utils/schema_loader.py +++ b/src/s2dm/exporters/utils/schema_loader.py @@ -53,12 +53,11 @@ from s2dm.exporters.utils.instance_tag import expand_instances_in_schema, is_valid_instance_tag_field from s2dm.exporters.utils.naming import apply_naming_to_schema, convert_name, load_naming_config from s2dm.exporters.utils.naming_config import ContextType, ElementType, NamingConventionConfig, get_case_for_element -from s2dm.exporters.utils.retained_definitions import ( - apply_enum_value_selection, - extract_retained_definitions, - retained_directive_names, - retained_type_names, - validate_retained_definitions, +from s2dm.exporters.utils.pick import ( + extract_picked_definitions, + picked_directive_names, + picked_type_names, + validate_picked_definitions, ) from s2dm.exporters.utils.violations import ConstraintViolation, Severity from s2dm.ledger import Ledger, annotate_schema_with_ledger @@ -581,8 +580,8 @@ def prune_schema_using_query_selection( if not schema.query_type: raise ValueError("Schema has no query type defined") - document, retained = extract_retained_definitions(document) - validate_retained_definitions(schema, retained) + document, picked = extract_picked_definitions(document) + validate_picked_definitions(schema, picked) _validate_schema(schema, document) @@ -727,9 +726,9 @@ def collect_selections(type_name: str, selection_set: SelectionSetNode) -> None: query_operation = query_operations[0] collect_selections(schema.query_type.name, query_operation.selection_set) - for type_name in retained_type_names(schema, retained): + for type_name in picked_type_names(schema, picked): keep_type(type_name) - for directive_name in retained_directive_names(schema, retained): + for directive_name in picked_directive_names(schema, picked): keep_directive(directive_name) while pending_types: @@ -759,8 +758,6 @@ def collect_selections(type_name: str, selection_set: SelectionSetNode) -> None: schema.directives = tuple(directive for directive in schema.directives if directive.name in directives_used) - apply_enum_value_selection(schema, retained) - log.debug(f"Composed filtered schema with {len(fields_to_keep)} object types") return schema diff --git a/tests/data/retained_definitions_schema.graphql b/tests/data/pick_schema.graphql similarity index 61% rename from tests/data/retained_definitions_schema.graphql rename to tests/data/pick_schema.graphql index c2c1325e..1cb0784d 100644 --- a/tests/data/retained_definitions_schema.graphql +++ b/tests/data/pick_schema.graphql @@ -1,21 +1,12 @@ -directive @instanceTag(exclude: [String!]) on OBJECT | FIELD_DEFINITION | ENUM directive @range(min: Float, max: Float) on FIELD_DEFINITION directive @confidential on FIELD_DEFINITION scalar DateTime -enum RowEnum { ROW1 ROW2 } -enum PositionEnum { LEFT MIDDLE RIGHT } enum SeatMaterial { CLOTH LEATHER VINYL } enum VelocityUnit { KILOM_PER_HR MI_PER_HR M_PER_SEC } -type CabinPosition @instanceTag { - row: RowEnum! - position: PositionEnum! -} - type Seat { - cabinPosition: CabinPosition @instanceTag(exclude: ["ROW1.MIDDLE", "ROW2.LEFT"]) isOccupied: Boolean material: SeatMaterial lastAdjusted: DateTime diff --git a/tests/test_pick.py b/tests/test_pick.py new file mode 100644 index 00000000..abd46b77 --- /dev/null +++ b/tests/test_pick.py @@ -0,0 +1,92 @@ +"""Tests for the @pick selection query directive.""" + +from pathlib import Path + +import pytest +from graphql import GraphQLEnumType, GraphQLSchema, parse +from graphql import print_schema as print_graphql_schema + +from s2dm.exporters.utils.schema_loader import load_schema, prune_schema_using_query_selection + +SCHEMA_PATH = Path("tests/data/pick_schema.graphql") +SEATS = "vehicle { cabin { seats { isOccupied } } }" + + +def prune(query: str) -> GraphQLSchema: + schema = load_schema([SCHEMA_PATH]) + return prune_schema_using_query_selection(schema, parse(query)) + + +def directive_names(schema: GraphQLSchema) -> set[str]: + return {directive.name for directive in schema.directives} + + +class TestWithoutTheDirective: + def test_unreferenced_definitions_are_dropped(self) -> None: + schema = prune(f"query Selection {{ {SEATS} }}") + + assert "DateTime" not in schema.type_map + assert "SeatMaterial" not in schema.type_map + assert "confidential" not in directive_names(schema) + + def test_a_directive_on_a_pruned_field_is_still_kept(self) -> None: + """Existing behavior: directives are collected from a type before its fields are deleted.""" + schema = prune(f"query Selection {{ {SEATS} }}") + + assert "range" in directive_names(schema) + + +class TestPicking: + def test_unreferenced_scalar_is_kept(self) -> None: + schema = prune(f'query Selection @pick(scalars: ["DateTime"]) {{ {SEATS} }}') + + assert "DateTime" in schema.type_map + + def test_unapplied_directive_is_kept(self) -> None: + schema = prune(f'query Selection @pick(directives: ["confidential"]) {{ {SEATS} }}') + + assert "confidential" in directive_names(schema) + + def test_unreferenced_enum_is_kept_whole(self) -> None: + schema = prune(f'query Selection @pick(enums: ["SeatMaterial"]) {{ {SEATS} }}') + + enum_type = schema.type_map["SeatMaterial"] + assert isinstance(enum_type, GraphQLEnumType) + assert list(enum_type.values) == ["CLOTH", "LEATHER", "VINYL"] + + def test_empty_list_keeps_every_definition_of_that_kind(self) -> None: + schema = prune(f"query Selection @pick(scalars: [], enums: [], directives: []) {{ {SEATS} }}") + + assert "DateTime" in schema.type_map + assert "SeatMaterial" in schema.type_map + assert "VelocityUnit" in schema.type_map + assert "confidential" in directive_names(schema) + + def test_absent_arguments_leave_the_existing_behavior(self) -> None: + picked = prune(f'query Selection @pick(scalars: ["DateTime"]) {{ {SEATS} }}') + plain = prune(f"query Selection {{ {SEATS} }}") + + assert "SeatMaterial" not in picked.type_map + assert set(plain.type_map) < set(picked.type_map) + + def test_the_directive_does_not_reach_the_filtered_schema(self) -> None: + schema = prune(f'query Selection @pick(scalars: ["DateTime"]) {{ {SEATS} }}') + + assert "pick" not in print_graphql_schema(schema) + + +class TestValidation: + @pytest.mark.parametrize( + ("selection", "message"), + [ + ('scalars: ["Timestamp"]', "'Timestamp' is not defined in the model"), + ('enums: ["Vehicle"]', "'Vehicle' is not an enum"), + ('scalars: ["SeatMaterial"]', "'SeatMaterial' is not a scalar"), + ('directives: ["constraint"]', "is not defined in the model"), + ('enums: "SeatMaterial"', "must be a list of names"), + ('unknown: ["x"]', "unknown argument"), + ], + ) + def test_invalid_selections_are_reported(self, selection: str, message: str) -> None: + with pytest.raises(ValueError, match=message): + prune(f"query Selection @pick({selection}) {{ {SEATS} }}") diff --git a/tests/test_retained_definitions.py b/tests/test_retained_definitions.py deleted file mode 100644 index c8c62fab..00000000 --- a/tests/test_retained_definitions.py +++ /dev/null @@ -1,164 +0,0 @@ -"""Tests for the @retainedDefinitions selection query directive.""" - -from pathlib import Path -from typing import cast - -import pytest -from graphql import GraphQLEnumType, GraphQLObjectType, GraphQLSchema, parse -from graphql import print_schema as print_graphql_schema - -from s2dm import log -from s2dm.exporters.utils.schema_loader import ( - load_and_process_schema, - load_schema, - prune_schema_using_query_selection, -) - -SCHEMA_PATH = Path("tests/data/retained_definitions_schema.graphql") - - -def prune(query: str, expanded_instances: bool = False) -> GraphQLSchema: - schema = load_schema([SCHEMA_PATH]) - return prune_schema_using_query_selection(schema, parse(query), expanded_instances) - - -SEATS = "vehicle { cabin { seats { isOccupied } } }" - - -class TestWithoutTheDirective: - def test_unreferenced_definitions_are_dropped(self) -> None: - schema = prune(f"query Selection {{ {SEATS} }}") - - assert "DateTime" not in schema.type_map - assert "SeatMaterial" not in schema.type_map - assert "confidential" not in {directive.name for directive in schema.directives} - - def test_a_directive_on_a_pruned_field_is_still_retained(self) -> None: - """Existing behavior: directives are collected from a type before its fields are deleted.""" - schema = prune(f"query Selection {{ {SEATS} }}") - - assert "range" in {directive.name for directive in schema.directives} - - -class TestRetention: - def test_unreferenced_scalar_is_retained(self) -> None: - schema = prune(f'query Selection @retainedDefinitions(scalars: ["DateTime"]) {{ {SEATS} }}') - - assert "DateTime" in schema.type_map - - def test_unapplied_directive_is_retained(self) -> None: - schema = prune(f'query Selection @retainedDefinitions(directives: ["confidential"]) {{ {SEATS} }}') - - assert "confidential" in {directive.name for directive in schema.directives} - - def test_empty_list_retains_every_definition_of_that_kind(self) -> None: - schema = prune(f"query Selection @retainedDefinitions(scalars: [], enums: []) {{ {SEATS} }}") - - assert "DateTime" in schema.type_map - assert "SeatMaterial" in schema.type_map - assert "VelocityUnit" in schema.type_map - - def test_unreferenced_enum_is_retained_and_narrowed(self) -> None: - schema = prune( - f"query Selection @retainedDefinitions(enums: {{ SeatMaterial: [CLOTH, LEATHER] }}) {{ {SEATS} }}" - ) - - enum_type = schema.type_map["SeatMaterial"] - assert isinstance(enum_type, GraphQLEnumType) - assert list(enum_type.values) == ["CLOTH", "LEATHER"] - - def test_empty_value_list_keeps_every_value(self) -> None: - schema = prune(f"query Selection @retainedDefinitions(enums: {{ SeatMaterial: [] }}) {{ {SEATS} }}") - - enum_type = schema.type_map["SeatMaterial"] - assert isinstance(enum_type, GraphQLEnumType) - assert list(enum_type.values) == ["CLOTH", "LEATHER", "VINYL"] - - def test_the_directive_does_not_reach_the_filtered_schema(self) -> None: - schema = prune(f'query Selection @retainedDefinitions(scalars: ["DateTime"]) {{ {SEATS} }}') - - assert "retainedDefinitions" not in print_graphql_schema(schema) - - -class TestValuesInUse: - def test_a_retained_default_refuses_its_value_being_removed(self) -> None: - query = ( - "query Selection @retainedDefinitions(enums: { VelocityUnit: [MI_PER_HR] }) { vehicle { averageSpeed } }" - ) - - with pytest.raises(ValueError, match="Vehicle.averageSpeed"): - prune(query) - - def test_a_pruned_default_does_not_hold_its_value(self) -> None: - query = f"query Selection @retainedDefinitions(enums: {{ VelocityUnit: [MI_PER_HR] }}) {{ {SEATS} }}" - - schema = prune(query) - - enum_type = schema.type_map["VelocityUnit"] - assert isinstance(enum_type, GraphQLEnumType) - assert list(enum_type.values) == ["MI_PER_HR"] - - -class TestValidation: - @pytest.mark.parametrize( - ("selection", "message"), - [ - ('scalars: ["Timestamp"]', "'Timestamp' is not defined in the model"), - ("enums: { Vehicle: [] }", "'Vehicle' is not an enum"), - ("enums: { SeatMaterial: [SUEDE] }", "has no value"), - ('directives: ["constraint"]', "is not defined in the model"), - ], - ) - def test_invalid_selections_are_reported(self, selection: str, message: str) -> None: - with pytest.raises(ValueError, match=message): - prune(f"query Selection @retainedDefinitions({selection}) {{ {SEATS} }}") - - -def expand(tmp_path: Path, query: str, schema_path: Path = SCHEMA_PATH) -> dict[str, list[str]]: - """Filter and expand the fixture, returning the generated seat instances by row.""" - query_file = tmp_path / "query.graphql" - query_file.write_text(query) - annotated_schema, _, _ = load_and_process_schema([schema_path], None, query_file, None, True) - return { - name.split("_")[1]: sorted(cast(GraphQLObjectType, type_definition).fields) - for name, type_definition in annotated_schema.schema.type_map.items() - if name.startswith("Seat_") and name.endswith("_Position") - } - - -class TestInstanceTagDimensions: - def test_the_model_excludes_two_instances(self, tmp_path: Path) -> None: - instances = expand(tmp_path, f"query Selection {{ {SEATS} }}") - - assert instances == {"ROW1": ["LEFT", "RIGHT"], "ROW2": ["MIDDLE", "RIGHT"]} - - def test_narrowing_a_dimension_reduces_the_instances(self, tmp_path: Path) -> None: - query = f"query Selection @retainedDefinitions(enums: {{ PositionEnum: [LEFT, RIGHT] }}) {{ {SEATS} }}" - - instances = expand(tmp_path, query) - - assert instances == {"ROW1": ["LEFT", "RIGHT"], "ROW2": ["RIGHT"]} - - def test_a_misspelled_exclude_entry_still_fails(self, tmp_path: Path) -> None: - schema_path = tmp_path / "schema.graphql" - schema_path.write_text(SCHEMA_PATH.read_text().replace('"ROW1.MIDDLE"', '"ROW1.MIDLE"')) - query = f"query Selection @retainedDefinitions(enums: {{ PositionEnum: [LEFT, RIGHT] }}) {{ {SEATS} }}" - - with pytest.raises(ValueError, match="match no instance"): - expand(tmp_path, query, schema_path) - - def test_a_selection_that_excludes_every_instance_warns( - self, tmp_path: Path, caplog: pytest.LogCaptureFixture - ) -> None: - schema_path = tmp_path / "schema.graphql" - schema_path.write_text(SCHEMA_PATH.read_text().replace('"ROW2.LEFT"', '"ROW2.MIDDLE"')) - query = f"query Selection @retainedDefinitions(enums: {{ PositionEnum: [MIDDLE] }}) {{ {SEATS} }}" - - log.addHandler(caplog.handler) - try: - instances = expand(tmp_path, query, schema_path) - finally: - log.removeHandler(caplog.handler) - - assert instances == {} - assert "Cabin.seats' expands to no instances" in caplog.text From a35345128b24f3968e25353e6c90bfb0c047d990 Mon Sep 17 00:00:00 2001 From: Baris Akcam Date: Mon, 28 Sep 2026 12:30:02 +0200 Subject: [PATCH 05/11] feat(selection): read an empty selection set as selecting no fields A query that only picks definitions has no fields to name, but GraphQL rejects an empty selection set. A schema of units, enums or shared directives hits this immediately, since it carries nothing selectable. Read `{}` as `{ __typename }`, which every type carries and which names nothing in the model. The rewrite runs only after a parse failure and only on a trailing empty pair, so a query broken for any other reason still reports its own error. All four places that read a selection query share the parser, including the one that reads a vendored dependency's query. Report a name given under the wrong argument as such. A directive and a type are looked up in different places, so neither is found where the other is named, and "not defined in the model" was the only thing the check could say about either. It now names the argument that would accept the value instead. --- docs-gen/content/docs/tools/cli.md | 7 ++ src/s2dm/api/routes/query_validate.py | 10 ++- src/s2dm/deps/helpers.py | 7 +- src/s2dm/exporters/utils/pick.py | 79 +++++++++++++++++++---- src/s2dm/exporters/utils/schema_loader.py | 6 +- tests/test_pick.py | 29 ++++++++- 6 files changed, 116 insertions(+), 22 deletions(-) diff --git a/docs-gen/content/docs/tools/cli.md b/docs-gen/content/docs/tools/cli.md index 698ef611..5c465c0c 100644 --- a/docs-gen/content/docs/tools/cli.md +++ b/docs-gen/content/docs/tools/cli.md @@ -2633,6 +2633,13 @@ An enum is always kept whole. There is no way to select a subset of its values, Selecting a directive keeps its definition only. It does not apply the directive to any field or type. +A query that only picks definitions has no fields to name. Leave the selection set empty and s2dm reads it as selecting nothing, which is the usual case for a schema of units, enums or shared directives. + +```graphql +query Selection + @pick(enums: ["Weekday", "MonthOfYear"], directives: []) {} +``` + **Note:** `@pick` is defined by s2dm rather than by the model, and is removed from the query before the query is validated against the schema. A GraphQL tool that does not know about it reports an unknown directive for a selection query that uses it. ### Root Type Filtering diff --git a/src/s2dm/api/routes/query_validate.py b/src/s2dm/api/routes/query_validate.py index 064c8a21..47e36796 100644 --- a/src/s2dm/api/routes/query_validate.py +++ b/src/s2dm/api/routes/query_validate.py @@ -1,7 +1,7 @@ """Query validate route - validate GraphQL query against schema.""" from fastapi import APIRouter -from graphql import parse, print_schema, validate +from graphql import print_schema, validate from s2dm.api.config import COMMON_RESPONSES from s2dm.api.errors import ResponseError, format_error_list @@ -9,7 +9,11 @@ from s2dm.api.models.query_validate import ValidateQueryRequest from s2dm.api.services.response_service import execute_and_respond from s2dm.api.services.schema_service import path_for_content, process_schema_input, validate_schema_or_raise -from s2dm.exporters.utils.pick import extract_picked_definitions, validate_picked_definitions +from s2dm.exporters.utils.pick import ( + extract_picked_definitions, + parse_selection_query, + validate_picked_definitions, +) from s2dm.exporters.utils.schema_loader import load_schema router = APIRouter(responses=COMMON_RESPONSES) @@ -28,7 +32,7 @@ def process_request() -> list[str]: query_path = path_for_content(request.selection_query, "selection_query", ".graphql") query_text = query_path.read_text(encoding="utf-8") - query_document, picked = extract_picked_definitions(parse(query_text)) + query_document, picked = extract_picked_definitions(parse_selection_query(query_text)) validate_picked_definitions(schema, picked) validation_errors = validate(schema, query_document) diff --git a/src/s2dm/deps/helpers.py b/src/s2dm/deps/helpers.py index a5b77945..1a5acc27 100644 --- a/src/s2dm/deps/helpers.py +++ b/src/s2dm/deps/helpers.py @@ -3,7 +3,7 @@ from typing import Literal import yaml -from graphql import DocumentNode, parse +from graphql import DocumentNode from pydantic import ValidationError from s2dm.deps import DEPENDENCY_LOCK_FILENAME, clean_resolved_dependencies, resolve_dependencies @@ -26,6 +26,7 @@ from s2dm.deps.resolve.providers import RemoteIdentityProvider from s2dm.deps.resolve.resolve import validate_cached_dependency from s2dm.deps.resolve.warnings import WarningCollector +from s2dm.exporters.utils.pick import parse_selection_query from s2dm.exporters.utils.schema_loader import build_schema_str_with_optional_source_map from s2dm.utils.compose import SchemaDefinition, SharedDefinitionResolver from s2dm.utils.file import temp_file_from_content, temp_files_from_contents @@ -115,7 +116,9 @@ def resolve_schema_selection(schema_path: Path) -> DocumentNode | None: resolved_schema_path = schema_path.resolve() if dependency.selection is not None: - selection_by_schema_path[resolved_schema_path] = parse(dependency.selection.read_text(encoding="utf-8")) + selection_by_schema_path[resolved_schema_path] = parse_selection_query( + dependency.selection.read_text(encoding="utf-8") + ) try: schema_content, _ = build_schema_str_with_optional_source_map( diff --git a/src/s2dm/exporters/utils/pick.py b/src/s2dm/exporters/utils/pick.py index e6b1befd..5e4cd550 100644 --- a/src/s2dm/exporters/utils/pick.py +++ b/src/s2dm/exporters/utils/pick.py @@ -1,5 +1,6 @@ -"""Extraction and validation of the @pick selection query directive.""" +"""Parsing, extraction and validation of the @pick selection query directive.""" +import re from dataclasses import dataclass from typing import Any @@ -8,7 +9,9 @@ GraphQLEnumType, GraphQLScalarType, GraphQLSchema, + parse, ) +from graphql.error import GraphQLSyntaxError from graphql.language.ast import OperationDefinitionNode from graphql.utilities import value_from_ast_untyped @@ -19,6 +22,10 @@ ALL = "__all__" ARGUMENTS = ("enums", "scalars", "directives") +# GraphQL requires a non-empty selection set, which a schema of only definitions has nothing to fill. +EMPTY_SELECTION_SET = re.compile(r"\{\s*\}\s*$") +NOTHING_SELECTED = "{ __typename }" + @dataclass(frozen=True) class PickedDefinitions: @@ -34,6 +41,32 @@ class PickedDefinitions: directives: list[str] | str | None = None +def parse_selection_query(text: str) -> DocumentNode: + """Parse a selection query, reading an empty selection set as selecting no fields. + + A query that only picks definitions has no fields to name, but GraphQL rejects an empty + selection set. Such a query is read as selecting `__typename`, which every type carries and + which names nothing in the model. + + Args: + text: The contents of the selection query file. + + Returns: + The parsed document. + + Raises: + GraphQLSyntaxError: If the query does not parse for any other reason. + """ + try: + return parse(text) + except GraphQLSyntaxError: + stripped = text.rstrip() + repaired = EMPTY_SELECTION_SET.sub(NOTHING_SELECTED, stripped, count=1) + if repaired == stripped: + raise + return parse(repaired) + + def _name_list(value: Any, argument: str) -> list[str] | str: """Read a [String!] argument, where an empty list stands for every definition of that kind.""" if not isinstance(value, list): @@ -108,6 +141,9 @@ def extract_picked_definitions(document: DocumentNode) -> tuple[DocumentNode, Pi def validate_picked_definitions(schema: GraphQLSchema, picked: PickedDefinitions) -> None: """Check every name and definition kind in the selection against the source model. + A name given under the wrong argument is reported as such, since a directive and a type can + share a name and neither is found where the other is looked up. + Args: schema: The unfiltered schema the selection is written against. picked: The definitions the selection query asked to keep. @@ -116,25 +152,44 @@ def validate_picked_definitions(schema: GraphQLSchema, picked: PickedDefinitions ValueError: If any name is missing from the model or is of the wrong kind. """ errors: list[str] = [] + directive_names = {directive.name for directive in schema.directives} + + def argument_for(type_definition: Any) -> str | None: + """The argument that would accept this type, if any.""" + for kind, argument in ((GraphQLEnumType, "enums"), (GraphQLScalarType, "scalars")): + if isinstance(type_definition, kind): + return argument + return None - def check_kind(names: list[str] | str | None, kind: type, label: str) -> None: + def check_types(names: list[str] | str | None, kind: type, label: str) -> None: if not isinstance(names, list): return for name in names: type_definition = schema.type_map.get(name) + if isinstance(type_definition, kind): + continue if type_definition is None: - errors.append(f"'{name}' is not defined in the model") - elif not isinstance(type_definition, kind): - errors.append(f"'{name}' is not {label}") - - check_kind(picked.scalars, GraphQLScalarType, "a scalar") - check_kind(picked.enums, GraphQLEnumType, "an enum") + if name in directive_names: + errors.append(f"'{name}' is a directive, not {label}; list it under 'directives'") + else: + errors.append(f"'{name}' is not defined in the model") + continue + argument = argument_for(type_definition) + suffix = f"; list it under '{argument}'" if argument else "" + errors.append(f"'{name}' is not {label}{suffix}") + + check_types(picked.scalars, GraphQLScalarType, "a scalar") + check_types(picked.enums, GraphQLEnumType, "an enum") if isinstance(picked.directives, list): - defined = {directive.name for directive in schema.directives} - errors.extend( - f"directive '@{name}' is not defined in the model" for name in picked.directives if name not in defined - ) + for name in picked.directives: + if name in directive_names: + continue + argument = argument_for(schema.type_map.get(name)) + if argument: + errors.append(f"'{name}' is not a directive; list it under '{argument}'") + else: + errors.append(f"directive '@{name}' is not defined in the model") if errors: raise ValueError(f"@{DIRECTIVE_NAME} validation failed:\n" + "\n".join(f" - {error}" for error in errors)) diff --git a/src/s2dm/exporters/utils/schema_loader.py b/src/s2dm/exporters/utils/schema_loader.py index 67306a0d..7f2bc628 100644 --- a/src/s2dm/exporters/utils/schema_loader.py +++ b/src/s2dm/exporters/utils/schema_loader.py @@ -23,7 +23,6 @@ is_interface_type, is_object_type, is_union_type, - parse, print_schema, ) from graphql import validate as graphql_validate @@ -55,6 +54,7 @@ from s2dm.exporters.utils.naming_config import ContextType, ElementType, NamingConventionConfig, get_case_for_element from s2dm.exporters.utils.pick import ( extract_picked_definitions, + parse_selection_query, picked_directive_names, picked_type_names, validate_picked_definitions, @@ -380,7 +380,7 @@ def compose_schemas_to_string( query_document = None if selection_query: - query_document = parse(selection_query.read_text()) + query_document = parse_selection_query(selection_query.read_text()) naming_config_dict = load_naming_config(naming_config) annotated_schema = process_schema( @@ -880,7 +880,7 @@ def load_and_process_schema( query_document = None if selection_query_path: - query_document = parse(selection_query_path.read_text()) + query_document = parse_selection_query(selection_query_path.read_text()) annotated_schema = process_schema(schema, source_map, naming_config, query_document, root_type, expanded_instances) diff --git a/tests/test_pick.py b/tests/test_pick.py index abd46b77..a7137f0a 100644 --- a/tests/test_pick.py +++ b/tests/test_pick.py @@ -3,9 +3,11 @@ from pathlib import Path import pytest -from graphql import GraphQLEnumType, GraphQLSchema, parse +from graphql import GraphQLEnumType, GraphQLSchema from graphql import print_schema as print_graphql_schema +from graphql.error import GraphQLSyntaxError +from s2dm.exporters.utils.pick import parse_selection_query from s2dm.exporters.utils.schema_loader import load_schema, prune_schema_using_query_selection SCHEMA_PATH = Path("tests/data/pick_schema.graphql") @@ -14,7 +16,7 @@ def prune(query: str) -> GraphQLSchema: schema = load_schema([SCHEMA_PATH]) - return prune_schema_using_query_selection(schema, parse(query)) + return prune_schema_using_query_selection(schema, parse_selection_query(query)) def directive_names(schema: GraphQLSchema) -> set[str]: @@ -83,6 +85,10 @@ class TestValidation: ('enums: ["Vehicle"]', "'Vehicle' is not an enum"), ('scalars: ["SeatMaterial"]', "'SeatMaterial' is not a scalar"), ('directives: ["constraint"]', "is not defined in the model"), + ('enums: ["range"]', "is a directive, not an enum; list it under 'directives'"), + ('scalars: ["confidential"]', "is a directive, not a scalar; list it under 'directives'"), + ('directives: ["SeatMaterial"]', "is not a directive; list it under 'enums'"), + ('enums: ["DateTime"]', "is not an enum; list it under 'scalars'"), ('enums: "SeatMaterial"', "must be a list of names"), ('unknown: ["x"]', "unknown argument"), ], @@ -90,3 +96,22 @@ class TestValidation: def test_invalid_selections_are_reported(self, selection: str, message: str) -> None: with pytest.raises(ValueError, match=message): prune(f"query Selection @pick({selection}) {{ {SEATS} }}") + + +class TestEmptySelectionSet: + def test_a_query_that_only_picks_definitions_needs_no_fields(self) -> None: + schema = prune('query Selection @pick(enums: ["SeatMaterial"], scalars: ["DateTime"]) {}') + + assert "SeatMaterial" in schema.type_map + assert "DateTime" in schema.type_map + assert "Vehicle" not in schema.type_map + + def test_it_matches_selecting_typename(self) -> None: + braces = prune('query Selection @pick(scalars: ["DateTime"]) {}') + typename = prune('query Selection @pick(scalars: ["DateTime"]) { __typename }') + + assert print_graphql_schema(braces) == print_graphql_schema(typename) + + def test_a_broken_query_still_reports_its_own_error(self) -> None: + with pytest.raises(GraphQLSyntaxError, match="Expected ':'"): + prune("query Selection @pick(enums: [ { vehicle }") From 102604e1291f56f3d263baabcad89682b8cf81e5 Mon Sep 17 00:00:00 2001 From: Baris Akcam Date: Mon, 28 Sep 2026 13:47:47 +0200 Subject: [PATCH 06/11] refactor(selection): tidy the pick module after review Consolidate the two mirrored validation paths into one that asks where a name actually belongs and compares that against where it was written, so the three arguments share a single code path and a set of messages. Replace the ALL magic string with a typed sentinel, so a selection can no longer be built from an arbitrary string, and take the argument names from constants rather than repeating them at each use. Name the intermediate values that were previously nested inside calls, give the directive and type parameters their real types instead of Any, and stop using "selection" for both a picked name list and a query's selection set. --- src/s2dm/api/routes/query_validate.py | 3 +- src/s2dm/exporters/utils/pick.py | 180 ++++++++++++++-------- src/s2dm/exporters/utils/schema_loader.py | 6 +- tests/test_pick.py | 4 +- 4 files changed, 120 insertions(+), 73 deletions(-) diff --git a/src/s2dm/api/routes/query_validate.py b/src/s2dm/api/routes/query_validate.py index 47e36796..b8c87f3e 100644 --- a/src/s2dm/api/routes/query_validate.py +++ b/src/s2dm/api/routes/query_validate.py @@ -32,7 +32,8 @@ def process_request() -> list[str]: query_path = path_for_content(request.selection_query, "selection_query", ".graphql") query_text = query_path.read_text(encoding="utf-8") - query_document, picked = extract_picked_definitions(parse_selection_query(query_text)) + parsed_query = parse_selection_query(query_text) + query_document, picked = extract_picked_definitions(parsed_query) validate_picked_definitions(schema, picked) validation_errors = validate(schema, query_document) diff --git a/src/s2dm/exporters/utils/pick.py b/src/s2dm/exporters/utils/pick.py index 5e4cd550..e425fa4f 100644 --- a/src/s2dm/exporters/utils/pick.py +++ b/src/s2dm/exporters/utils/pick.py @@ -7,26 +7,43 @@ from graphql import ( DocumentNode, GraphQLEnumType, + GraphQLNamedType, GraphQLScalarType, GraphQLSchema, parse, ) from graphql.error import GraphQLSyntaxError -from graphql.language.ast import OperationDefinitionNode +from graphql.language.ast import DirectiveNode, OperationDefinitionNode from graphql.utilities import value_from_ast_untyped from s2dm import log from s2dm.constants.directive import Directive DIRECTIVE_NAME = Directive.PICK.value -ALL = "__all__" -ARGUMENTS = ("enums", "scalars", "directives") + +ENUMS_ARGUMENT = "enums" +SCALARS_ARGUMENT = "scalars" +DIRECTIVES_ARGUMENT = "directives" +ARGUMENTS = (ENUMS_ARGUMENT, SCALARS_ARGUMENT, DIRECTIVES_ARGUMENT) +ARGUMENT_FOR_KIND = ((GraphQLEnumType, ENUMS_ARGUMENT), (GraphQLScalarType, SCALARS_ARGUMENT)) # GraphQL requires a non-empty selection set, which a schema of only definitions has nothing to fill. EMPTY_SELECTION_SET = re.compile(r"\{\s*\}\s*$") NOTHING_SELECTED = "{ __typename }" +class EveryDefinition: + """Stands for an empty list, which names every definition of its kind.""" + + def __repr__(self) -> str: + return "ALL" + + +ALL = EveryDefinition() + +PickSelection = list[str] | EveryDefinition | None + + @dataclass(frozen=True) class PickedDefinitions: """Definitions a selection query asks to keep beyond the ones its fields reference. @@ -36,9 +53,9 @@ class PickedDefinitions: always kept whole; there is no way to select a subset of their values. """ - enums: list[str] | str | None = None - scalars: list[str] | str | None = None - directives: list[str] | str | None = None + enums: PickSelection = None + scalars: PickSelection = None + directives: PickSelection = None def parse_selection_query(text: str) -> DocumentNode: @@ -67,26 +84,61 @@ def parse_selection_query(text: str) -> DocumentNode: return parse(repaired) -def _name_list(value: Any, argument: str) -> list[str] | str: - """Read a [String!] argument, where an empty list stands for every definition of that kind.""" +def _read_name_list(value: Any, argument_name: str) -> list[str] | EveryDefinition: + """Read a [String!] argument into deduplicated names, where an empty list stands for all of them. + + Args: + value: The argument value as untyped AST. + argument_name: The argument being read, used to report where a problem is. + + Returns: + The names in the order given, or ALL when the list is empty. + + Raises: + ValueError: If the value is not a list, or holds anything other than names. + """ if not isinstance(value, list): - raise ValueError(f"@{DIRECTIVE_NAME}: '{argument}' must be a list of names") + raise ValueError(f"@{DIRECTIVE_NAME}: '{argument_name}' must be a list of names") if not value: return ALL if any(not isinstance(entry, str) for entry in value): - raise ValueError(f"@{DIRECTIVE_NAME}: '{argument}' must contain only names") + raise ValueError(f"@{DIRECTIVE_NAME}: '{argument_name}' must contain only names") return list(dict.fromkeys(value)) -def _read_directive_arguments(node: Any) -> PickedDefinitions: - arguments = {argument.name.value: value_from_ast_untyped(argument.value) for argument in node.arguments} +def _read_directive_arguments(directive_node: DirectiveNode) -> PickedDefinitions: + """Read the arguments applied to one @pick directive. + + Args: + directive_node: The applied directive node. + + Returns: + The definitions it named, with an absent argument left as None. + + Raises: + ValueError: If the directive carries an argument @pick does not define. + """ + arguments = {argument.name.value: value_from_ast_untyped(argument.value) for argument in directive_node.arguments} unknown = sorted(set(arguments) - set(ARGUMENTS)) if unknown: raise ValueError(f"@{DIRECTIVE_NAME}: unknown argument(s) {unknown}") - picked = {name: _name_list(arguments[name], name) for name in ARGUMENTS if name in arguments} - return PickedDefinitions(**picked) + picked_arguments = {name: _read_name_list(arguments[name], name) for name in ARGUMENTS if name in arguments} + return PickedDefinitions(**picked_arguments) + + +def _without_pick(definition: OperationDefinitionNode) -> OperationDefinitionNode: + """Return the operation with @pick removed from the directives applied to it.""" + remaining = tuple(node for node in definition.directives if node.name.value != Directive.PICK) + return OperationDefinitionNode( + operation=definition.operation, + name=definition.name, + variable_definitions=definition.variable_definitions, + directives=remaining, + selection_set=definition.selection_set, + loc=definition.loc, + ) def extract_picked_definitions(document: DocumentNode) -> tuple[DocumentNode, PickedDefinitions]: @@ -111,28 +163,20 @@ def extract_picked_definitions(document: DocumentNode) -> tuple[DocumentNode, Pi definitions.append(definition) continue - applied = [node for node in definition.directives if node.name.value == Directive.PICK] + applied_directives = [node for node in definition.directives if node.name.value == Directive.PICK] is_query = definition.operation.value == "query" - if applied and is_query and not seen_first_query: - if len(applied) > 1: + if applied_directives and is_query and not seen_first_query: + if len(applied_directives) > 1: raise ValueError(f"@{DIRECTIVE_NAME} is applied more than once on one operation") - picked = _read_directive_arguments(applied[0]) - elif applied: + picked = _read_directive_arguments(applied_directives[0]) + elif applied_directives: log.warning(f"Ignoring @{DIRECTIVE_NAME} outside the first query operation") seen_first_query = seen_first_query or is_query - if applied: - remaining = tuple(node for node in definition.directives if node.name.value != Directive.PICK) - definition = OperationDefinitionNode( - operation=definition.operation, - name=definition.name, - variable_definitions=definition.variable_definitions, - directives=remaining, - selection_set=definition.selection_set, - loc=definition.loc, - ) + if applied_directives: + definition = _without_pick(definition) definitions.append(definition) return DocumentNode(definitions=tuple(definitions), loc=document.loc), picked @@ -154,42 +198,40 @@ def validate_picked_definitions(schema: GraphQLSchema, picked: PickedDefinitions errors: list[str] = [] directive_names = {directive.name for directive in schema.directives} - def argument_for(type_definition: Any) -> str | None: - """The argument that would accept this type, if any.""" - for kind, argument in ((GraphQLEnumType, "enums"), (GraphQLScalarType, "scalars")): + def argument_for_type(type_definition: GraphQLNamedType | None) -> str | None: + """The argument that would accept this type, or None when no argument does.""" + for kind, argument_name in ARGUMENT_FOR_KIND: if isinstance(type_definition, kind): - return argument + return argument_name return None - def check_types(names: list[str] | str | None, kind: type, label: str) -> None: - if not isinstance(names, list): + def actual_argument(name: str) -> str | None: + """The argument the name belongs under, or None when the model does not define it.""" + if name in directive_names: + return DIRECTIVES_ARGUMENT + type_definition = schema.type_map.get(name) + return argument_for_type(type_definition) + + def collect_errors(picked_names: PickSelection, argument_name: str, label: str) -> None: + """Record an error for every name that does not belong under the given argument.""" + if not isinstance(picked_names, list): return - for name in names: - type_definition = schema.type_map.get(name) - if isinstance(type_definition, kind): + for name in picked_names: + belongs_under = actual_argument(name) + if belongs_under == argument_name: continue - if type_definition is None: - if name in directive_names: - errors.append(f"'{name}' is a directive, not {label}; list it under 'directives'") - else: - errors.append(f"'{name}' is not defined in the model") + if belongs_under is not None: + errors.append(f"'{name}' is not {label}; list it under '{belongs_under}'") continue - argument = argument_for(type_definition) - suffix = f"; list it under '{argument}'" if argument else "" - errors.append(f"'{name}' is not {label}{suffix}") - - check_types(picked.scalars, GraphQLScalarType, "a scalar") - check_types(picked.enums, GraphQLEnumType, "an enum") - - if isinstance(picked.directives, list): - for name in picked.directives: - if name in directive_names: + if name in schema.type_map: + errors.append(f"'{name}' is not {label}") continue - argument = argument_for(schema.type_map.get(name)) - if argument: - errors.append(f"'{name}' is not a directive; list it under '{argument}'") - else: - errors.append(f"directive '@{name}' is not defined in the model") + subject = f"directive '@{name}'" if argument_name == DIRECTIVES_ARGUMENT else f"'{name}'" + errors.append(f"{subject} is not defined in the model") + + collect_errors(picked.scalars, SCALARS_ARGUMENT, "a scalar") + collect_errors(picked.enums, ENUMS_ARGUMENT, "an enum") + collect_errors(picked.directives, DIRECTIVES_ARGUMENT, "a directive") if errors: raise ValueError(f"@{DIRECTIVE_NAME} validation failed:\n" + "\n".join(f" - {error}" for error in errors)) @@ -197,19 +239,23 @@ def check_types(names: list[str] | str | None, kind: type, label: str) -> None: def picked_type_names(schema: GraphQLSchema, picked: PickedDefinitions) -> list[str]: """Names of the scalar and enum types the selection keeps regardless of references.""" - names: list[str] = [] + type_names: list[str] = [] - for selection, kind in ((picked.scalars, GraphQLScalarType), (picked.enums, GraphQLEnumType)): - if selection == ALL: - names += [name for name, t in schema.type_map.items() if isinstance(t, kind)] - elif isinstance(selection, list): - names += selection + for picked_names, kind in ((picked.scalars, GraphQLScalarType), (picked.enums, GraphQLEnumType)): + if picked_names is ALL: + type_names += [ + name for name, type_definition in schema.type_map.items() if isinstance(type_definition, kind) + ] + elif isinstance(picked_names, list): + type_names += picked_names - return [name for name in names if not name.startswith("__")] + return [name for name in type_names if not name.startswith("__")] def picked_directive_names(schema: GraphQLSchema, picked: PickedDefinitions) -> list[str]: """Names of the directives the selection keeps regardless of use.""" - if picked.directives == ALL: + if picked.directives is ALL: return [directive.name for directive in schema.directives] - return list(picked.directives or []) + if isinstance(picked.directives, list): + return list(picked.directives) + return [] diff --git a/src/s2dm/exporters/utils/schema_loader.py b/src/s2dm/exporters/utils/schema_loader.py index 7f2bc628..3464e875 100644 --- a/src/s2dm/exporters/utils/schema_loader.py +++ b/src/s2dm/exporters/utils/schema_loader.py @@ -580,10 +580,10 @@ def prune_schema_using_query_selection( if not schema.query_type: raise ValueError("Schema has no query type defined") - document, picked = extract_picked_definitions(document) + stripped_document, picked = extract_picked_definitions(document) validate_picked_definitions(schema, picked) - _validate_schema(schema, document) + _validate_schema(schema, stripped_document) fields_to_keep: dict[str, set[str]] = {} types_to_keep: set[str] = set() @@ -714,7 +714,7 @@ def collect_selections(type_name: str, selection_set: SelectionSetNode) -> None: query_operations = [ definition - for definition in document.definitions + for definition in stripped_document.definitions if isinstance(definition, OperationDefinitionNode) and definition.operation.value == "query" ] diff --git a/tests/test_pick.py b/tests/test_pick.py index a7137f0a..91a93d6d 100644 --- a/tests/test_pick.py +++ b/tests/test_pick.py @@ -85,8 +85,8 @@ class TestValidation: ('enums: ["Vehicle"]', "'Vehicle' is not an enum"), ('scalars: ["SeatMaterial"]', "'SeatMaterial' is not a scalar"), ('directives: ["constraint"]', "is not defined in the model"), - ('enums: ["range"]', "is a directive, not an enum; list it under 'directives'"), - ('scalars: ["confidential"]', "is a directive, not a scalar; list it under 'directives'"), + ('enums: ["range"]', "is not an enum; list it under 'directives'"), + ('scalars: ["confidential"]', "is not a scalar; list it under 'directives'"), ('directives: ["SeatMaterial"]', "is not a directive; list it under 'enums'"), ('enums: ["DateTime"]', "is not an enum; list it under 'scalars'"), ('enums: "SeatMaterial"', "must be a list of names"), From 5ee81aef1484868f2381080b1699c4e552aa1296 Mon Sep 17 00:00:00 2001 From: Baris Akcam Date: Mon, 5 Oct 2026 15:52:33 +0200 Subject: [PATCH 07/11] feat(selection): make @pick usable from the playground MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The editor validated the selection query against the model alone, so every query using @pick was marked invalid and the apply button stayed disabled: the directive is unknown to the model, and an empty selection set does not parse. Read the query the way the server does — treat empty braces as selecting nothing, and strip @pick before validating — and give the editor's schema the directive definition so it stops underlining it. That schema is built for validation and completion only and never leaves the browser. Drop a query root left with no fields from printed output. Selecting only definitions leaves nothing on it, and an object type with no fields is invalid GraphQL, which graphql-js rejects even though graphql-core accepts it. Without the root the output is a fragment that composes into a model with one, which is the shape the vocabulary it was filtered from already has. --- .../src/components/QueryEditorWrapper.tsx | 9 ++- .../src/components/explore/ExplorerTab.tsx | 3 +- playground/src/utils/selectionQuery.ts | 69 +++++++++++++++++++ src/s2dm/exporters/utils/schema_loader.py | 23 ++++++- 4 files changed, 100 insertions(+), 4 deletions(-) create mode 100644 playground/src/utils/selectionQuery.ts diff --git a/playground/src/components/QueryEditorWrapper.tsx b/playground/src/components/QueryEditorWrapper.tsx index df4f6d6c..4773b836 100644 --- a/playground/src/components/QueryEditorWrapper.tsx +++ b/playground/src/components/QueryEditorWrapper.tsx @@ -1,6 +1,10 @@ import { QueryEditor, useEditorContext, useGraphiQL } from "@graphiql/react"; -import { parse, validate } from "graphql"; +import { validate } from "graphql"; import { useEffect, useRef } from "react"; +import { + parseSelectionQuery, + withoutPickDirective, +} from "@/utils/selectionQuery"; type QueryEditorWrapperProps = { selectionQuery: string; @@ -37,7 +41,8 @@ export function QueryEditorWrapper({ } try { - const document = parse(query); + const parsed = parseSelectionQuery(query); + const document = withoutPickDirective(parsed); if (schema) { const errors = validate(schema, document); onValidationChange(errors.length > 0); diff --git a/playground/src/components/explore/ExplorerTab.tsx b/playground/src/components/explore/ExplorerTab.tsx index e0f4580b..8bfe2eac 100644 --- a/playground/src/components/explore/ExplorerTab.tsx +++ b/playground/src/components/explore/ExplorerTab.tsx @@ -35,6 +35,7 @@ import { } from "@/store/selection/selectionSlice"; import { downloadTextFile } from "@/utils/download"; import { getErrorMessage } from "@/utils/getErrorMessage"; +import { withPickDirective } from "@/utils/selectionQuery"; import "@graphiql/react/style.css"; import "@/components/graphiql-theme.css"; @@ -76,7 +77,7 @@ export function ExplorerTab() { const graphqlSchema = useMemo(() => { if (!originalSchema?.trim()) return undefined; try { - return buildSchema(originalSchema); + return buildSchema(withPickDirective(originalSchema)); } catch { return undefined; } diff --git a/playground/src/utils/selectionQuery.ts b/playground/src/utils/selectionQuery.ts new file mode 100644 index 00000000..4c1f06d0 --- /dev/null +++ b/playground/src/utils/selectionQuery.ts @@ -0,0 +1,69 @@ +import type { DocumentNode } from "graphql"; +import { Kind, parse } from "graphql"; + +// GraphQL rejects an empty selection set, and @pick is defined by s2dm rather than by the +// model, so a selection query the server accepts does not parse or validate here untouched. +const EMPTY_SELECTION_SET = /\{\s*\}\s*$/; +const NOTHING_SELECTED = "{ __typename }"; +const PICK_DIRECTIVE = "pick"; + +/** + * Parse a selection query, reading an empty selection set as selecting no fields. + * + * A query that only picks definitions has no fields to name. Such a query is read as + * selecting `__typename`, which every type carries and which names nothing in the model. + */ +export function parseSelectionQuery(text: string): DocumentNode { + try { + return parse(text); + } catch (error) { + const stripped = text.trimEnd(); + const repaired = stripped.replace(EMPTY_SELECTION_SET, NOTHING_SELECTED); + if (repaired === stripped) { + throw error; + } + return parse(repaired); + } +} + +/** + * Remove @pick from every operation so the document can be validated against the model, + * which does not define the directive. + */ +export function withoutPickDirective(document: DocumentNode): DocumentNode { + return { + ...document, + definitions: document.definitions.map((definition) => { + if ( + definition.kind !== Kind.OPERATION_DEFINITION || + !definition.directives?.length + ) { + return definition; + } + return { + ...definition, + directives: definition.directives.filter( + (directive) => directive.name.value !== PICK_DIRECTIVE, + ), + }; + }), + }; +} + +// The model does not define @pick, so an editor validating against it alone underlines every +// selection query that uses one. Added for validation and completion only; never exported. +const PICK_DIRECTIVE_SDL = + "directive @pick(enums: [String!], scalars: [String!], directives: [String!]) on QUERY"; + +/** + * Return the schema text with the @pick definition, so an editor can validate queries using it. + * + * A model that already defines the directive is returned unchanged, since a duplicate + * definition would make the schema fail to build. + */ +export function withPickDirective(schemaText: string): string { + if (/^\s*directive\s+@pick\b/m.test(schemaText)) { + return schemaText; + } + return `${schemaText}\n\n${PICK_DIRECTIVE_SDL}\n`; +} diff --git a/src/s2dm/exporters/utils/schema_loader.py b/src/s2dm/exporters/utils/schema_loader.py index 3464e875..dc74fd6b 100644 --- a/src/s2dm/exporters/utils/schema_loader.py +++ b/src/s2dm/exporters/utils/schema_loader.py @@ -352,10 +352,31 @@ def print_schema_with_directives_preserved(schema: GraphQLSchema, source_map: di existing_directives.append(f'@reference(source: "{source_filename}")') directive_map[type_name] = existing_directives - base_schema = print_schema(schema) + base_schema = _without_empty_query_type(schema, print_schema(schema)) return add_directives_to_schema(base_schema, directive_map) +def _without_empty_query_type(schema: GraphQLSchema, printed_schema: str) -> str: + """Drop a query root left with no fields, which GraphQL does not accept as a type. + + Selecting only definitions leaves nothing on the query root, and an object type with no + fields is invalid. The remaining definitions compose into a model that has one. + + Args: + schema: The schema that was printed. + printed_schema: Its printed form. + + Returns: + The printed schema without an empty query root declaration. + """ + query_type = schema.query_type + if query_type is None or query_type.fields: + return printed_schema + + declaration = re.compile(rf"^type {re.escape(query_type.name)}$\n?", re.MULTILINE) + return declaration.sub("", printed_schema).rstrip() + "\n" + + def compose_schemas_to_string( schemas: list[Path], root_type: str | None, From e7d44eccc26f48f93fa0bf2e0d9467b7ec10b6ff Mon Sep 17 00:00:00 2001 From: Baris Akcam Date: Thu, 8 Oct 2026 17:41:19 +0200 Subject: [PATCH 08/11] refactor(selection): act on the @pick implementation review Stop stripping @pick in the playground before validating. The editor's schema defines the directive now, so stripping it only hid mistakes inside it: an unknown argument, a non-name in a list, and @pick on a mutation were all accepted in the editor and rejected by the server. Refuse a name GraphQL reserves for introspection. Such a name passed validation, because the introspection types are in the type map, and was then dropped when the picks were collected, so the user got neither the definition nor an error. Read @pick from the one query a selection file holds, rather than walking every operation to find the first. A @pick on a later operation now stays in the document, where validation rejects it as unknown. Give the extraction and its validation a single entry point so a caller cannot do one without the other, print inside the function that drops an empty query root, and cover the kinds a selection can name with a test that fails when one is added without being handled. --- .../src/components/QueryEditorWrapper.tsx | 8 +- .../src/components/explore/ExplorerTab.tsx | 3 +- playground/src/utils/selectionQuery.ts | 31 +--- src/s2dm/api/routes/query_validate.py | 9 +- src/s2dm/exporters/utils/pick.py | 133 ++++++++++-------- src/s2dm/exporters/utils/schema_loader.py | 18 +-- tests/test_pick.py | 91 +++++++++++- 7 files changed, 184 insertions(+), 109 deletions(-) diff --git a/playground/src/components/QueryEditorWrapper.tsx b/playground/src/components/QueryEditorWrapper.tsx index 4773b836..73675746 100644 --- a/playground/src/components/QueryEditorWrapper.tsx +++ b/playground/src/components/QueryEditorWrapper.tsx @@ -1,10 +1,7 @@ import { QueryEditor, useEditorContext, useGraphiQL } from "@graphiql/react"; import { validate } from "graphql"; import { useEffect, useRef } from "react"; -import { - parseSelectionQuery, - withoutPickDirective, -} from "@/utils/selectionQuery"; +import { parseSelectionQuery } from "@/utils/selectionQuery"; type QueryEditorWrapperProps = { selectionQuery: string; @@ -41,8 +38,7 @@ export function QueryEditorWrapper({ } try { - const parsed = parseSelectionQuery(query); - const document = withoutPickDirective(parsed); + const document = parseSelectionQuery(query); if (schema) { const errors = validate(schema, document); onValidationChange(errors.length > 0); diff --git a/playground/src/components/explore/ExplorerTab.tsx b/playground/src/components/explore/ExplorerTab.tsx index 8bfe2eac..06320343 100644 --- a/playground/src/components/explore/ExplorerTab.tsx +++ b/playground/src/components/explore/ExplorerTab.tsx @@ -77,7 +77,8 @@ export function ExplorerTab() { const graphqlSchema = useMemo(() => { if (!originalSchema?.trim()) return undefined; try { - return buildSchema(withPickDirective(originalSchema)); + const schemaText = withPickDirective(originalSchema); + return buildSchema(schemaText); } catch { return undefined; } diff --git a/playground/src/utils/selectionQuery.ts b/playground/src/utils/selectionQuery.ts index 4c1f06d0..4c526c7a 100644 --- a/playground/src/utils/selectionQuery.ts +++ b/playground/src/utils/selectionQuery.ts @@ -1,11 +1,10 @@ import type { DocumentNode } from "graphql"; -import { Kind, parse } from "graphql"; +import { parse } from "graphql"; -// GraphQL rejects an empty selection set, and @pick is defined by s2dm rather than by the -// model, so a selection query the server accepts does not parse or validate here untouched. +// GraphQL rejects an empty selection set, so a selection query the server accepts does +// not parse here untouched. const EMPTY_SELECTION_SET = /\{\s*\}\s*$/; const NOTHING_SELECTED = "{ __typename }"; -const PICK_DIRECTIVE = "pick"; /** * Parse a selection query, reading an empty selection set as selecting no fields. @@ -26,30 +25,6 @@ export function parseSelectionQuery(text: string): DocumentNode { } } -/** - * Remove @pick from every operation so the document can be validated against the model, - * which does not define the directive. - */ -export function withoutPickDirective(document: DocumentNode): DocumentNode { - return { - ...document, - definitions: document.definitions.map((definition) => { - if ( - definition.kind !== Kind.OPERATION_DEFINITION || - !definition.directives?.length - ) { - return definition; - } - return { - ...definition, - directives: definition.directives.filter( - (directive) => directive.name.value !== PICK_DIRECTIVE, - ), - }; - }), - }; -} - // The model does not define @pick, so an editor validating against it alone underlines every // selection query that uses one. Added for validation and completion only; never exported. const PICK_DIRECTIVE_SDL = diff --git a/src/s2dm/api/routes/query_validate.py b/src/s2dm/api/routes/query_validate.py index b8c87f3e..69dd10e1 100644 --- a/src/s2dm/api/routes/query_validate.py +++ b/src/s2dm/api/routes/query_validate.py @@ -9,11 +9,7 @@ from s2dm.api.models.query_validate import ValidateQueryRequest from s2dm.api.services.response_service import execute_and_respond from s2dm.api.services.schema_service import path_for_content, process_schema_input, validate_schema_or_raise -from s2dm.exporters.utils.pick import ( - extract_picked_definitions, - parse_selection_query, - validate_picked_definitions, -) +from s2dm.exporters.utils.pick import extract_and_validate_picks, parse_selection_query from s2dm.exporters.utils.schema_loader import load_schema router = APIRouter(responses=COMMON_RESPONSES) @@ -33,8 +29,7 @@ def process_request() -> list[str]: query_text = query_path.read_text(encoding="utf-8") parsed_query = parse_selection_query(query_text) - query_document, picked = extract_picked_definitions(parsed_query) - validate_picked_definitions(schema, picked) + query_document, _ = extract_and_validate_picks(schema, parsed_query) validation_errors = validate(schema, query_document) diff --git a/src/s2dm/exporters/utils/pick.py b/src/s2dm/exporters/utils/pick.py index e425fa4f..6eca4b8a 100644 --- a/src/s2dm/exporters/utils/pick.py +++ b/src/s2dm/exporters/utils/pick.py @@ -1,6 +1,7 @@ """Parsing, extraction and validation of the @pick selection query directive.""" import re +from copy import copy from dataclasses import dataclass from typing import Any @@ -13,11 +14,11 @@ parse, ) from graphql.error import GraphQLSyntaxError -from graphql.language.ast import DirectiveNode, OperationDefinitionNode +from graphql.language.ast import DirectiveNode, OperationDefinitionNode, OperationType from graphql.utilities import value_from_ast_untyped -from s2dm import log from s2dm.constants.directive import Directive +from s2dm.exporters.utils.graphql_type import is_introspection_type DIRECTIVE_NAME = Directive.PICK.value @@ -129,60 +130,55 @@ def _read_directive_arguments(directive_node: DirectiveNode) -> PickedDefinition def _without_pick(definition: OperationDefinitionNode) -> OperationDefinitionNode: - """Return the operation with @pick removed from the directives applied to it.""" - remaining = tuple(node for node in definition.directives if node.name.value != Directive.PICK) - return OperationDefinitionNode( - operation=definition.operation, - name=definition.name, - variable_definitions=definition.variable_definitions, - directives=remaining, - selection_set=definition.selection_set, - loc=definition.loc, + """Return a copy of the operation with @pick removed from the directives applied to it.""" + stripped_definition = copy(definition) + stripped_definition.directives = tuple( + directive for directive in definition.directives if directive.name.value != DIRECTIVE_NAME ) + return stripped_definition -def extract_picked_definitions(document: DocumentNode) -> tuple[DocumentNode, PickedDefinitions]: - """Remove @pick from every operation and read it from the first query operation. +def _extract_picked_definitions(document: DocumentNode) -> tuple[DocumentNode, PickedDefinitions]: + """Remove @pick from the query operation and read the definitions it asks to keep. The directive is defined by S2DM rather than by the model, so it is taken out of the document - before the document is validated against the schema. Later operations are stripped as well, - since only the first query operation is read. + before the document is validated against the schema. Args: document: The parsed selection query document. Returns: The document without the directive, and the definitions it asked to keep. - """ - picked = PickedDefinitions() - seen_first_query = False - definitions = [] - - for definition in document.definitions: - if not isinstance(definition, OperationDefinitionNode): - definitions.append(definition) - continue - - applied_directives = [node for node in definition.directives if node.name.value == Directive.PICK] - is_query = definition.operation.value == "query" - - if applied_directives and is_query and not seen_first_query: - if len(applied_directives) > 1: - raise ValueError(f"@{DIRECTIVE_NAME} is applied more than once on one operation") - picked = _read_directive_arguments(applied_directives[0]) - elif applied_directives: - log.warning(f"Ignoring @{DIRECTIVE_NAME} outside the first query operation") - - seen_first_query = seen_first_query or is_query - - if applied_directives: - definition = _without_pick(definition) - definitions.append(definition) - return DocumentNode(definitions=tuple(definitions), loc=document.loc), picked + Raises: + ValueError: If @pick is applied more than once, or carries an argument it does not define. + """ + query_operations = ( + definition + for definition in document.definitions + if isinstance(definition, OperationDefinitionNode) and definition.operation == OperationType.QUERY + ) + query_operation = next(query_operations, None) + if query_operation is None: + return document, PickedDefinitions() + + applied_directives = [ + directive for directive in query_operation.directives if directive.name.value == DIRECTIVE_NAME + ] + if not applied_directives: + return document, PickedDefinitions() + if len(applied_directives) > 1: + raise ValueError(f"@{DIRECTIVE_NAME} is applied more than once on one operation") + + picked = _read_directive_arguments(applied_directives[0]) + stripped_query = _without_pick(query_operation) + definitions = tuple( + stripped_query if definition is query_operation else definition for definition in document.definitions + ) + return DocumentNode(definitions=definitions, loc=document.loc), picked -def validate_picked_definitions(schema: GraphQLSchema, picked: PickedDefinitions) -> None: +def _validate_picked_definitions(schema: GraphQLSchema, picked: PickedDefinitions) -> None: """Check every name and definition kind in the selection against the source model. A name given under the wrong argument is reported as such, since a directive and a type can @@ -195,7 +191,6 @@ def validate_picked_definitions(schema: GraphQLSchema, picked: PickedDefinitions Raises: ValueError: If any name is missing from the model or is of the wrong kind. """ - errors: list[str] = [] directive_names = {directive.name for directive in schema.directives} def argument_for_type(type_definition: GraphQLNamedType | None) -> str | None: @@ -205,43 +200,69 @@ def argument_for_type(type_definition: GraphQLNamedType | None) -> str | None: return argument_name return None - def actual_argument(name: str) -> str | None: + def argument_accepting(name: str) -> str | None: """The argument the name belongs under, or None when the model does not define it.""" if name in directive_names: return DIRECTIVES_ARGUMENT type_definition = schema.type_map.get(name) return argument_for_type(type_definition) - def collect_errors(picked_names: PickSelection, argument_name: str, label: str) -> None: - """Record an error for every name that does not belong under the given argument.""" + def collect_errors(picked_names: PickSelection, argument_name: str, label: str) -> list[str]: + """Every name under the given argument that does not belong there, as an error.""" if not isinstance(picked_names, list): - return + return [] + collected: list[str] = [] for name in picked_names: - belongs_under = actual_argument(name) + if is_introspection_type(name): + collected.append(f"'{name}' starts with '__', which GraphQL reserves for introspection") + continue + belongs_under = argument_accepting(name) if belongs_under == argument_name: continue if belongs_under is not None: - errors.append(f"'{name}' is not {label}; list it under '{belongs_under}'") + collected.append(f"'{name}' is not {label}; list it under '{belongs_under}'") continue if name in schema.type_map: - errors.append(f"'{name}' is not {label}") + collected.append(f"'{name}' is not {label}") continue subject = f"directive '@{name}'" if argument_name == DIRECTIVES_ARGUMENT else f"'{name}'" - errors.append(f"{subject} is not defined in the model") + collected.append(f"{subject} is not defined in the model") + return collected - collect_errors(picked.scalars, SCALARS_ARGUMENT, "a scalar") - collect_errors(picked.enums, ENUMS_ARGUMENT, "an enum") - collect_errors(picked.directives, DIRECTIVES_ARGUMENT, "a directive") + errors = [ + *collect_errors(picked.scalars, SCALARS_ARGUMENT, "a scalar"), + *collect_errors(picked.enums, ENUMS_ARGUMENT, "an enum"), + *collect_errors(picked.directives, DIRECTIVES_ARGUMENT, "a directive"), + ] if errors: raise ValueError(f"@{DIRECTIVE_NAME} validation failed:\n" + "\n".join(f" - {error}" for error in errors)) +def extract_and_validate_picks(schema: GraphQLSchema, document: DocumentNode) -> tuple[DocumentNode, PickedDefinitions]: + """Remove @pick from the document and check the definitions it names against the model. + + Args: + schema: The unfiltered schema the selection is written against. + document: The parsed selection query. + + Returns: + The document without @pick, and the definitions @pick asked to keep. + + Raises: + ValueError: If @pick is malformed or names a definition the model does not have. + """ + stripped_document, picked = _extract_picked_definitions(document) + _validate_picked_definitions(schema, picked) + return stripped_document, picked + + def picked_type_names(schema: GraphQLSchema, picked: PickedDefinitions) -> list[str]: """Names of the scalar and enum types the selection keeps regardless of references.""" type_names: list[str] = [] - for picked_names, kind in ((picked.scalars, GraphQLScalarType), (picked.enums, GraphQLEnumType)): + selections_by_kind = ((picked.scalars, GraphQLScalarType), (picked.enums, GraphQLEnumType)) + for picked_names, kind in selections_by_kind: if picked_names is ALL: type_names += [ name for name, type_definition in schema.type_map.items() if isinstance(type_definition, kind) @@ -249,7 +270,7 @@ def picked_type_names(schema: GraphQLSchema, picked: PickedDefinitions) -> list[ elif isinstance(picked_names, list): type_names += picked_names - return [name for name in type_names if not name.startswith("__")] + return [name for name in type_names if not is_introspection_type(name)] def picked_directive_names(schema: GraphQLSchema, picked: PickedDefinitions) -> list[str]: diff --git a/src/s2dm/exporters/utils/schema_loader.py b/src/s2dm/exporters/utils/schema_loader.py index dc74fd6b..376905d9 100644 --- a/src/s2dm/exporters/utils/schema_loader.py +++ b/src/s2dm/exporters/utils/schema_loader.py @@ -53,11 +53,10 @@ from s2dm.exporters.utils.naming import apply_naming_to_schema, convert_name, load_naming_config from s2dm.exporters.utils.naming_config import ContextType, ElementType, NamingConventionConfig, get_case_for_element from s2dm.exporters.utils.pick import ( - extract_picked_definitions, + extract_and_validate_picks, parse_selection_query, picked_directive_names, picked_type_names, - validate_picked_definitions, ) from s2dm.exporters.utils.violations import ConstraintViolation, Severity from s2dm.ledger import Ledger, annotate_schema_with_ledger @@ -352,23 +351,27 @@ def print_schema_with_directives_preserved(schema: GraphQLSchema, source_map: di existing_directives.append(f'@reference(source: "{source_filename}")') directive_map[type_name] = existing_directives - base_schema = _without_empty_query_type(schema, print_schema(schema)) + base_schema = _print_schema_without_empty_query_type(schema) return add_directives_to_schema(base_schema, directive_map) -def _without_empty_query_type(schema: GraphQLSchema, printed_schema: str) -> str: +def _print_schema_without_empty_query_type(schema: GraphQLSchema) -> str: """Drop a query root left with no fields, which GraphQL does not accept as a type. Selecting only definitions leaves nothing on the query root, and an object type with no fields is invalid. The remaining definitions compose into a model that has one. Args: - schema: The schema that was printed. - printed_schema: Its printed form. + schema: The schema to print. Returns: The printed schema without an empty query root declaration. + + Note: + Runs before add_directives_to_schema, so a type prints without directives and the + declaration is matched whole. """ + printed_schema = print_schema(schema) query_type = schema.query_type if query_type is None or query_type.fields: return printed_schema @@ -601,8 +604,7 @@ def prune_schema_using_query_selection( if not schema.query_type: raise ValueError("Schema has no query type defined") - stripped_document, picked = extract_picked_definitions(document) - validate_picked_definitions(schema, picked) + stripped_document, picked = extract_and_validate_picks(schema, document) _validate_schema(schema, stripped_document) diff --git a/tests/test_pick.py b/tests/test_pick.py index 91a93d6d..361897ba 100644 --- a/tests/test_pick.py +++ b/tests/test_pick.py @@ -3,12 +3,23 @@ from pathlib import Path import pytest -from graphql import GraphQLEnumType, GraphQLSchema +from graphql import GraphQLEnumType, GraphQLSchema, print_ast, validate from graphql import print_schema as print_graphql_schema from graphql.error import GraphQLSyntaxError -from s2dm.exporters.utils.pick import parse_selection_query -from s2dm.exporters.utils.schema_loader import load_schema, prune_schema_using_query_selection +from s2dm.exporters.utils.pick import ( + ALL, + ARGUMENT_FOR_KIND, + PickedDefinitions, + extract_and_validate_picks, + parse_selection_query, + picked_type_names, +) +from s2dm.exporters.utils.schema_loader import ( + load_schema, + print_schema_with_directives_preserved, + prune_schema_using_query_selection, +) SCHEMA_PATH = Path("tests/data/pick_schema.graphql") SEATS = "vehicle { cabin { seats { isOccupied } } }" @@ -115,3 +126,77 @@ def test_it_matches_selecting_typename(self) -> None: def test_a_broken_query_still_reports_its_own_error(self) -> None: with pytest.raises(GraphQLSyntaxError, match="Expected ':'"): prune("query Selection @pick(enums: [ { vehicle }") + + +def extract(query: str) -> tuple[str, PickedDefinitions]: + """Extract the picks, returning the stripped document as text.""" + schema = load_schema([SCHEMA_PATH]) + document, picked = extract_and_validate_picks(schema, parse_selection_query(query)) + return print_ast(document), picked + + +class TestExtraction: + def test_the_query_keeps_its_fields_and_loses_the_directive(self) -> None: + text, picked = extract(f'query Selection @pick(scalars: ["DateTime"]) {{ {SEATS} }}') + + assert picked.scalars == ["DateTime"] + assert "@pick" not in text + assert "isOccupied" in text + + def test_a_document_without_a_query_picks_nothing(self) -> None: + text, picked = extract("fragment SeatFields on Seat { isOccupied }") + + assert picked == PickedDefinitions() + assert "SeatFields" in text + + def test_the_directive_cannot_be_applied_twice(self) -> None: + query = f'query Selection @pick(scalars: ["DateTime"]) @pick(enums: []) {{ {SEATS} }}' + + with pytest.raises(ValueError, match="applied more than once"): + extract(query) + + def test_a_later_query_keeps_the_directive_for_validation_to_reject(self) -> None: + query = f"query First {{ {SEATS} }}\nquery Second @pick(enums: []) {{ {SEATS} }}" + schema = load_schema([SCHEMA_PATH]) + + document, picked = extract_and_validate_picks(schema, parse_selection_query(query)) + + assert picked == PickedDefinitions() + errors = validate(schema, document) + assert any("pick" in error.message for error in errors) + + +class TestIntrospectionNames: + def test_a_reserved_name_is_refused(self) -> None: + with pytest.raises(ValueError, match="reserves for introspection"): + prune(f'query Selection @pick(enums: ["__TypeKind"]) {{ {SEATS} }}') + + def test_keeping_every_definition_leaves_the_reserved_ones_out(self) -> None: + schema = load_schema([SCHEMA_PATH]) + + names = picked_type_names(schema, PickedDefinitions(enums=ALL, scalars=ALL)) + + assert "SeatMaterial" in names + assert not [name for name in names if name.startswith("__")] + + +class TestPrintedOutput: + def test_a_query_root_left_without_fields_is_not_printed(self) -> None: + schema = prune('query Selection @pick(enums: ["SeatMaterial"]) {}') + + printed = print_schema_with_directives_preserved(schema) + + assert "enum SeatMaterial" in printed + assert "type Query" not in printed + + +class TestEveryPickableKind: + """Guards the pairing that validation and collection each spell out separately.""" + + @pytest.mark.parametrize("argument_name", [name for _kind, name in ARGUMENT_FOR_KIND]) + def test_keeping_every_definition_of_a_kind_keeps_something(self, argument_name: str) -> None: + schema = load_schema([SCHEMA_PATH]) + + names = picked_type_names(schema, PickedDefinitions(**{argument_name: ALL})) + + assert names, f"'{argument_name}: []' kept nothing; picked_type_names may not handle it" From be7e0e0902f043e6426e8231fcba8662a3ae0c2d Mon Sep 17 00:00:00 2001 From: Baris Akcam Date: Thu, 8 Oct 2026 18:06:29 +0200 Subject: [PATCH 09/11] fix(selection): drop an empty query root before printing, not after Removing the root with a line-anchored substitution on printed output left behind anything the regex could not see. A documented root left its description, which the next definition then silently claimed, and a root named something other than Query left the schema block that named it, so the output no longer rebuilt. Print a schema rebuilt without the root instead, carrying the model's own description across, which takes both with it and retires the constraint that this had to run before directives were reattached. Resolve a picked name against every namespace that defines it. Types and directives are separate namespaces, so a scalar sharing a name with a directive was unreachable: picking it under scalars was refused, and the message sent the user to directives, which keeps the directive instead. --- src/s2dm/exporters/utils/pick.py | 24 ++++++---- src/s2dm/exporters/utils/schema_loader.py | 28 +++++++----- tests/test_pick.py | 56 ++++++++++++++++++++++- 3 files changed, 87 insertions(+), 21 deletions(-) diff --git a/src/s2dm/exporters/utils/pick.py b/src/s2dm/exporters/utils/pick.py index 6eca4b8a..23335f4c 100644 --- a/src/s2dm/exporters/utils/pick.py +++ b/src/s2dm/exporters/utils/pick.py @@ -200,12 +200,17 @@ def argument_for_type(type_definition: GraphQLNamedType | None) -> str | None: return argument_name return None - def argument_accepting(name: str) -> str | None: - """The argument the name belongs under, or None when the model does not define it.""" - if name in directive_names: - return DIRECTIVES_ARGUMENT + def defining_arguments(name: str) -> set[str]: + """Every argument whose namespace defines this name. + + Types and directives occupy separate namespaces, so one name can be defined in both. + """ type_definition = schema.type_map.get(name) - return argument_for_type(type_definition) + type_argument = argument_for_type(type_definition) + arguments = {type_argument} if type_argument is not None else set() + if name in directive_names: + arguments.add(DIRECTIVES_ARGUMENT) + return arguments def collect_errors(picked_names: PickSelection, argument_name: str, label: str) -> list[str]: """Every name under the given argument that does not belong there, as an error.""" @@ -216,11 +221,12 @@ def collect_errors(picked_names: PickSelection, argument_name: str, label: str) if is_introspection_type(name): collected.append(f"'{name}' starts with '__', which GraphQL reserves for introspection") continue - belongs_under = argument_accepting(name) - if belongs_under == argument_name: + defined_under = defining_arguments(name) + if argument_name in defined_under: continue - if belongs_under is not None: - collected.append(f"'{name}' is not {label}; list it under '{belongs_under}'") + if defined_under: + elsewhere = sorted(defined_under)[0] + collected.append(f"'{name}' is not {label}; list it under '{elsewhere}'") continue if name in schema.type_map: collected.append(f"'{name}' is not {label}") diff --git a/src/s2dm/exporters/utils/schema_loader.py b/src/s2dm/exporters/utils/schema_loader.py index 376905d9..1181a977 100644 --- a/src/s2dm/exporters/utils/schema_loader.py +++ b/src/s2dm/exporters/utils/schema_loader.py @@ -356,28 +356,34 @@ def print_schema_with_directives_preserved(schema: GraphQLSchema, source_map: di def _print_schema_without_empty_query_type(schema: GraphQLSchema) -> str: - """Drop a query root left with no fields, which GraphQL does not accept as a type. + """Print the schema, leaving out a query root that was left with no fields. Selecting only definitions leaves nothing on the query root, and an object type with no - fields is invalid. The remaining definitions compose into a model that has one. + fields is invalid. The remaining definitions compose into a model that has one. The root is + dropped before printing rather than after, so its description and the schema block naming it + go with it. Args: schema: The schema to print. Returns: - The printed schema without an empty query root declaration. - - Note: - Runs before add_directives_to_schema, so a type prints without directives and the - declaration is matched whole. + The printed schema, without an empty query root. """ - printed_schema = print_schema(schema) query_type = schema.query_type if query_type is None or query_type.fields: - return printed_schema + return print_schema(schema) - declaration = re.compile(rf"^type {re.escape(query_type.name)}$\n?", re.MULTILINE) - return declaration.sub("", printed_schema).rstrip() + "\n" + remaining_types = [ + type_definition + for type_name, type_definition in schema.type_map.items() + if type_definition is not query_type and not is_introspection_type(type_name) + ] + rootless_schema = GraphQLSchema( + types=remaining_types, + directives=schema.directives, + description=schema.description, + ) + return print_schema(rootless_schema) def compose_schemas_to_string( diff --git a/tests/test_pick.py b/tests/test_pick.py index 361897ba..13ccbd26 100644 --- a/tests/test_pick.py +++ b/tests/test_pick.py @@ -3,7 +3,7 @@ from pathlib import Path import pytest -from graphql import GraphQLEnumType, GraphQLSchema, print_ast, validate +from graphql import GraphQLEnumType, GraphQLSchema, build_schema, print_ast, validate from graphql import print_schema as print_graphql_schema from graphql.error import GraphQLSyntaxError @@ -200,3 +200,57 @@ def test_keeping_every_definition_of_a_kind_keeps_something(self, argument_name: names = picked_type_names(schema, PickedDefinitions(**{argument_name: ALL})) assert names, f"'{argument_name}: []' kept nothing; picked_type_names may not handle it" + + +class TestPrintedRootRemoval: + def test_a_documented_root_takes_its_description_with_it(self, tmp_path: Path) -> None: + schema_path = tmp_path / "documented.graphql" + schema_path.write_text('scalar DateTime\n"""The root."""\ntype Query { a: String }\nenum FuelType { PETROL }\n') + schema = load_schema([schema_path]) + query = parse_selection_query('query Selection @pick(enums: ["FuelType"]) {}') + + printed = print_schema_with_directives_preserved(prune_schema_using_query_selection(schema, query)) + + assert "The root." not in printed + assert build_schema(printed).type_map["FuelType"].description is None + + def test_a_renamed_root_leaves_no_dangling_schema_block(self, tmp_path: Path) -> None: + schema_path = tmp_path / "renamed.graphql" + schema_path.write_text("schema { query: RootQuery }\nenum FuelType { PETROL }\ntype RootQuery { a: String }\n") + schema = load_schema([schema_path]) + query = parse_selection_query('query Selection @pick(enums: ["FuelType"]) {}') + + printed = print_schema_with_directives_preserved(prune_schema_using_query_selection(schema, query)) + + assert "RootQuery" not in printed + build_schema(printed) + + +class TestSharedNames: + def test_a_type_and_a_directive_may_share_a_name(self, tmp_path: Path) -> None: + schema_path = tmp_path / "shared.graphql" + schema_path.write_text( + "directive @unit(name: String) on FIELD_DEFINITION\nscalar unit\ntype Query { a: String }\n" + ) + + def pick(argument: str) -> str: + schema = load_schema([schema_path]) + query = parse_selection_query(f"query Selection @pick({argument}) {{}}") + return print_schema_with_directives_preserved(prune_schema_using_query_selection(schema, query)) + + assert "scalar unit" in pick('scalars: ["unit"]') + assert "directive @unit" in pick('directives: ["unit"]') + + +class TestPrintedSchemaDescription: + def test_a_documented_schema_keeps_its_description(self, tmp_path: Path) -> None: + schema_path = tmp_path / "documented.graphql" + schema_path.write_text( + '"""A documented model."""\nschema { query: Query }\ntype Query { a: String }\nenum FuelType { PETROL }\n' + ) + schema = load_schema([schema_path]) + query = parse_selection_query('query Selection @pick(enums: ["FuelType"]) {}') + + printed = print_schema_with_directives_preserved(prune_schema_using_query_selection(schema, query)) + + assert "A documented model" in printed From 45c38250fc80506deb1a87a1f1cf0e44063d1b6c Mon Sep 17 00:00:00 2001 From: Baris Akcam Date: Fri, 9 Oct 2026 13:16:35 +0200 Subject: [PATCH 10/11] test(selection): cover an enum sharing a name with a directive The shared-name test pinned only the scalar case. Both pickable kinds resolve through the same lookup, so cover the enum one too. --- tests/test_pick.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/test_pick.py b/tests/test_pick.py index 13ccbd26..3788d660 100644 --- a/tests/test_pick.py +++ b/tests/test_pick.py @@ -230,7 +230,11 @@ class TestSharedNames: def test_a_type_and_a_directive_may_share_a_name(self, tmp_path: Path) -> None: schema_path = tmp_path / "shared.graphql" schema_path.write_text( - "directive @unit(name: String) on FIELD_DEFINITION\nscalar unit\ntype Query { a: String }\n" + "directive @unit(name: String) on FIELD_DEFINITION\n" + "scalar unit\n" + "directive @grade on FIELD_DEFINITION\n" + "enum grade { A }\n" + "type Query { a: String }\n" ) def pick(argument: str) -> str: @@ -240,6 +244,8 @@ def pick(argument: str) -> str: assert "scalar unit" in pick('scalars: ["unit"]') assert "directive @unit" in pick('directives: ["unit"]') + assert "enum grade" in pick('enums: ["grade"]') + assert "directive @grade" in pick('directives: ["grade"]') class TestPrintedSchemaDescription: From 10efa043d6e2b1c9132c7c578b830fb86487baa6 Mon Sep 17 00:00:00 2001 From: Baris Akcam Date: Fri, 9 Oct 2026 13:29:59 +0200 Subject: [PATCH 11/11] test(selection): guard every @pick argument, not only the type kinds The pickable-kind guard drew its cases from ARGUMENT_FOR_KIND, so a kind missing from that pairing produced no case at all and passed in silence. Drive it from ARGUMENTS instead, which also brings directives under it. --- tests/test_pick.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/tests/test_pick.py b/tests/test_pick.py index 3788d660..bd57326d 100644 --- a/tests/test_pick.py +++ b/tests/test_pick.py @@ -9,10 +9,12 @@ from s2dm.exporters.utils.pick import ( ALL, - ARGUMENT_FOR_KIND, + ARGUMENTS, + DIRECTIVES_ARGUMENT, PickedDefinitions, extract_and_validate_picks, parse_selection_query, + picked_directive_names, picked_type_names, ) from s2dm.exporters.utils.schema_loader import ( @@ -193,13 +195,18 @@ def test_a_query_root_left_without_fields_is_not_printed(self) -> None: class TestEveryPickableKind: """Guards the pairing that validation and collection each spell out separately.""" - @pytest.mark.parametrize("argument_name", [name for _kind, name in ARGUMENT_FOR_KIND]) + @pytest.mark.parametrize("argument_name", ARGUMENTS) def test_keeping_every_definition_of_a_kind_keeps_something(self, argument_name: str) -> None: schema = load_schema([SCHEMA_PATH]) + picked = PickedDefinitions(**{argument_name: ALL}) - names = picked_type_names(schema, PickedDefinitions(**{argument_name: ALL})) + names = ( + picked_directive_names(schema, picked) + if argument_name == DIRECTIVES_ARGUMENT + else picked_type_names(schema, picked) + ) - assert names, f"'{argument_name}: []' kept nothing; picked_type_names may not handle it" + assert names, f"'{argument_name}: []' kept nothing; the kind may be missing from a pairing" class TestPrintedRootRemoval: