diff --git a/airbyte_cdk/test/standard_tests/__init__.py b/airbyte_cdk/test/standard_tests/__init__.py index e99712ae2..6debc4090 100644 --- a/airbyte_cdk/test/standard_tests/__init__.py +++ b/airbyte_cdk/test/standard_tests/__init__.py @@ -34,10 +34,15 @@ class TestSuiteSourcePokeAPI(standard_tests.DeclarativeSourceTestSuite): airbyte-cdk connector test --pytest-arg=-Werror::airbyte_cdk.test.standard_tests.DiscoveredCatalogWarning ``` +The spec tests lint the spec in the same way: problems found by spec lints that are not enforced +yet are reported as a `SpecLintWarning`, which `-W error::...SpecLintWarning` also turns into a +failure. + ''' from airbyte_cdk.test.models.scenario import ConnectorTestScenario from airbyte_cdk.test.standard_tests._catalog_checks import DiscoveredCatalogWarning +from airbyte_cdk.test.standard_tests._spec_lint import SpecLintWarning from airbyte_cdk.test.standard_tests.connector_base import ConnectorTestSuiteBase from airbyte_cdk.test.standard_tests.declarative_sources import ( DeclarativeSourceTestSuite, @@ -52,4 +57,5 @@ class TestSuiteSourcePokeAPI(standard_tests.DeclarativeSourceTestSuite): "DestinationTestSuiteBase", "DiscoveredCatalogWarning", "SourceTestSuiteBase", + "SpecLintWarning", ] diff --git a/airbyte_cdk/test/standard_tests/_spec_lint.py b/airbyte_cdk/test/standard_tests/_spec_lint.py index b2a0ce6f7..413475af3 100644 --- a/airbyte_cdk/test/standard_tests/_spec_lint.py +++ b/airbyte_cdk/test/standard_tests/_spec_lint.py @@ -10,7 +10,8 @@ import json import re -from collections.abc import Iterable, Iterator, Mapping +import warnings +from collections.abc import Callable, Iterable, Iterator, Mapping from dataclasses import dataclass from typing import Any from urllib.parse import quote @@ -19,9 +20,11 @@ AirbyteMessage, AirbyteMessageSerializer, ConnectorSpecification, + ConnectorSpecificationSerializer, Type, ) from airbyte_cdk.test.entrypoint_wrapper import EntrypointOutput +from airbyte_cdk.test.standard_tests._catalog_checks import _iter_subschemas, _meta_validator SECRET_PROPERTY_NAMES = frozenset( { @@ -352,6 +355,606 @@ def _has_unmasked_variant_marking(schema: Mapping[str, Any], under_unmasked: boo return False +# Structural, presentational and OAuth lints, ported from CAT's `TestSpec`. + + +class SpecLintWarning(UserWarning): + """A spec problem that the standard tests report but do not enforce yet. + + To make these problems fail the test, turn the warning into an error, either with + `pytest -W error::airbyte_cdk.test.standard_tests.SpecLintWarning` or with + `airbyte-cdk connector test --pytest-arg=-Werror::airbyte_cdk.test.standard_tests.SpecLintWarning`. + """ + + +SPEC_REFERENCE_URL = ( + "https://docs.airbyte.com/platform/connector-development/connector-specification-reference" +) + +FORBIDDEN_SPEC_KEYWORDS = ( + "not", + "anyOf", + "patternProperties", + "prefixItems", + "allOf", + "if", + "then", + "else", + "dependentSchemas", + "dependentRequired", +) +"""JSON Schema keywords that the connector setup form cannot render.""" + +ARRAY_ITEM_TYPES = frozenset({"object", "string", "number", "integer"}) +"""Item types the connector setup form can render as an array field, besides an `enum`.""" + +DISPLAY_TYPES = frozenset({"dropdown", "radio"}) +"""Values `display_type` may take on a field with `oneOf` variants.""" + + +def _schema_nodes(schema: Any) -> Iterator[tuple[tuple[str, ...], Mapping[str, Any]]]: + """Yield `(path, subschema)` for `schema` and every subschema in it. + + Only keywords that hold subschemas are followed, so a property named `type`, `enum` or + `not` is never mistaken for a keyword, and values such as `default` or `examples` are + never read as schemas. + """ + yield from _iter_subschemas(schema) + + +def _spec_pointer(path: Iterable[str]) -> str: + """Render a schema path as the JSON pointer used in failure messages, `` `/` `` at the root.""" + return "`/" + "/".join(_pointer_token(segment) for segment in path) + "`" + + +def _is_property(path: tuple[str, ...]) -> bool: + return len(path) >= 2 and path[-2] == "properties" + + +def _is_single_valued(schema: Any) -> bool: + """Return whether a property can hold one value only: a `const`, or an `enum` of one item.""" + if not isinstance(schema, Mapping): + return False + enum = schema.get("enum") + return "const" in schema or (isinstance(enum, list) and len(enum) == 1) + + +def find_oneof_usage_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per `oneOf` that the setup form cannot render as a choice of variants. + + Ported from CAT's `test_oneof_usage`. The schema that declares `oneOf` must be an object, + and every variant must be an object with `properties`. The variants must share exactly one + discriminator: a property that sets `const` in every variant or, failing that, one that + sets a single-item `enum` in every variant. The setup form picks the variant by that + value, so a `const` discriminator may set `default` and `enum` only to the same value, an + `enum` discriminator must set its one item as `default` (the form reads it as the + `const`), and no two variants may share a value. + + Unlike CAT, a property whose `enum` has several items is not a discriminator candidate, + since it cannot identify a variant (such as a `compression_codec` next to `format_type`). + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + variants = schema.get("oneOf") + if not isinstance(variants, list): + continue + where = _spec_pointer((*path, "oneOf")) + if schema.get("type") != "object": + errors.append( + f"{where} is declared on a schema that is not `type: object`; the setup form " + "renders `oneOf` only as a choice between object variants." + ) + variant_properties: list[Mapping[str, Any]] = [] + for index, variant in enumerate(variants): + properties = variant.get("properties") if isinstance(variant, Mapping) else None + if not isinstance(properties, Mapping): + errors.append( + f"{_spec_pointer((*path, 'oneOf', str(index)))} has no `properties`; every " + "`oneOf` variant must be an object with properties." + ) + continue + variant_properties.append(properties) + if not variants or len(variant_properties) != len(variants): + continue + + common = set.intersection(*(set(properties) for properties in variant_properties)) + candidates = sorted( + name + for name in common + if all(_is_single_valued(properties[name]) for properties in variant_properties) + ) + const_candidates = [ + name + for name in candidates + if all("const" in properties[name] for properties in variant_properties) + ] + if len(const_candidates) == 1: + discriminator = const_candidates[0] + elif not const_candidates and len(candidates) == 1: + discriminator = candidates[0] + else: + found = const_candidates or candidates + detail = ", ".join(f"`{name}`" for name in found) if found else "none" + errors.append( + f"{where} variants must share exactly one property that sets `const` in every " + f"variant, so the setup form can tell them apart (found: {detail})." + ) + continue + + values: list[Any] = [] + for index, properties in enumerate(variant_properties): + discriminator_schema = properties[discriminator] + discriminator_where = _spec_pointer( + (*path, "oneOf", str(index), "properties", discriminator) + ) + if "const" in discriminator_schema: + value = discriminator_schema["const"] + if "default" in discriminator_schema and not _same_json( + discriminator_schema["default"], value + ): + errors.append( + f"{discriminator_where} sets a `default` that differs from its `const`." + ) + if "enum" in discriminator_schema and not _same_json( + discriminator_schema["enum"], [value] + ): + errors.append( + f"{discriminator_where} sets an `enum` other than a single item equal " + "to its `const`." + ) + elif "default" in discriminator_schema and _same_json( + discriminator_schema["enum"], [discriminator_schema["default"]] + ): + value = discriminator_schema["default"] + else: + errors.append( + f"{discriminator_where} must set `const`, or set its single `enum` item as " + "its `default`." + ) + continue + values.append(value) + if any(_count_json(values, value) > 1 for value in values): + errors.append( + f"{where} variants share a value of the discriminator `{discriminator}`, so " + "the setup form cannot tell them apart." + ) + return errors + + +def _same_json(left: Any, right: Any) -> bool: + return json.dumps(left, sort_keys=True) == json.dumps(right, sort_keys=True) + + +def _count_json(values: list[Any], value: Any) -> int: + return sum(_same_json(item, value) for item in values) + + +def find_enum_usage_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per `enum` that lists the same value more than once. + + Ported from CAT's `test_enum_usage`. + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + enum = schema.get("enum") + if not isinstance(enum, list): + continue + seen: dict[str, int] = {} + for index, value in enumerate(enum): + key = json.dumps(value, sort_keys=True) + if key in seen: + errors.append( + f"{_spec_pointer((*path, 'enum'))} lists the same value at positions " + f"{seen[key]} and {index}." + ) + else: + seen[key] = index + return errors + + +def find_forbidden_keyword_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per JSON Schema keyword that the setup form cannot render. + + Ported from CAT's `test_forbidden_complex_types`: see `FORBIDDEN_SPEC_KEYWORDS`. + """ + return [ + f"{_spec_pointer((*path, keyword))} uses `{keyword}`, which the connector setup form " + "does not support." + for path, schema in _schema_nodes(connection_specification) + for keyword in FORBIDDEN_SPEC_KEYWORDS + if keyword in schema + ] + + +def find_array_type_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per array whose items the setup form cannot render. + + Ported from CAT's `test_array_type`: `items` must be a single schema, of type `object`, + `string`, `number` or `integer`, or with an `enum`. + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + if schema.get("type") != "array" or "items" not in schema: + continue + items = schema["items"] + where = _spec_pointer((*path, "items")) + if not isinstance(items, Mapping): + errors.append(f"{where} must be a single schema, not a list of item schemas.") + elif "enum" in items: + continue + elif not isinstance(items.get("type"), str) or items["type"] not in ARRAY_ITEM_TYPES: + errors.append( + f"{where} must be of type `object`, `string`, `number` or `integer`, or set " + "an `enum`." + ) + return errors + + +def find_empty_object_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per object that declares no properties and no `oneOf` variants. + + Ported from CAT's `test_object_not_empty`: the setup form shows nothing for such an object. + The root schema may be an empty object, for a connector that takes no configuration. + """ + return [ + f"{_spec_pointer(path)} is an object with no `properties` and no `oneOf`, so the setup " + "form cannot show it. Add its properties or remove it." + for path, schema in _schema_nodes(connection_specification) + if path + and schema.get("type") == "object" + and "oneOf" not in schema + and not schema.get("properties") + ] + + +def find_property_type_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per property whose `type` lists types the setup form cannot render. + + Ported from CAT's `test_property_type_is_not_array`: a list of types must hold one type, + or one type followed by `null`, such as `[string, null]`. + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + types = schema.get("type") + if not _is_property(path) or not isinstance(types, list): + continue + if len(types) not in (1, 2) or (len(types) == 2 and types[1] != "null"): + errors.append( + f"{_spec_pointer((*path, 'type'))} must be a single type, or a type followed by " + "`null`, such as `[string, null]`." + ) + return errors + + +def _iter_keys(value: Any, path: tuple[str, ...] = ()) -> Iterator[tuple[tuple[str, ...], str]]: + """Yield `(path, key)` for every mapping key anywhere in a JSON-like value.""" + if isinstance(value, Mapping): + for key, item in value.items(): + yield path, str(key) + yield from _iter_keys(item, (*path, str(key))) + elif isinstance(value, list): + for index, item in enumerate(value): + yield from _iter_keys(item, (*path, str(index))) + + +def find_unresolved_ref_errors(spec: Mapping[str, Any]) -> list[str]: + """Return one error per `$ref` left in the emitted spec. + + Ported from CAT's `test_defined_refs_exist_in_json_spec_file`. The platform does not + resolve references in a spec, so the connector must emit it with every `$ref` resolved. + `spec` is the whole SPEC message payload, so `advanced_auth` is checked too. + """ + return [ + f"{_spec_pointer((*path, key))} is an unresolved reference; emit the spec with every " + "`$ref` resolved." + for path, key in _iter_keys(spec) + if key == "$ref" + ] + + +def find_additional_properties_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per `additionalProperties` that is set to anything but `true`. + + Ported from CAT's `test_additional_properties_is_true`. With `false` or a schema, removing + a property in a later version makes every existing config that sets it invalid. + """ + return [ + f"{_spec_pointer((*path, 'additionalProperties'))} must be `true` when set; any other " + "value makes existing configs invalid as soon as a property is removed." + for path, schema in _schema_nodes(connection_specification) + if "additionalProperties" in schema and schema["additionalProperties"] is not True + ] + + +def find_display_type_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per misused `display_type`. + + Ported from CAT's `test_display_type`: `display_type` belongs on a field with `oneOf` + variants, and must be `dropdown` or `radio`. + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + if "display_type" not in schema: + continue + where = _spec_pointer((*path, "display_type")) + if "oneOf" not in schema: + errors.append(f"{where} is set on a field without `oneOf` variants.") + display_type = schema["display_type"] + if not isinstance(display_type, str) or display_type not in DISPLAY_TYPES: + errors.append(f"{where} must be `dropdown` or `radio`.") + return errors + + +def find_duplicate_order_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per `order` value used twice among the properties of one object. + + Ported from CAT's `test_duplicate_order`. Properties in different `group`s may share an + `order`. + """ + errors: list[str] = [] + for path, schema in _schema_nodes(connection_specification): + properties = schema.get("properties") + if schema.get("type") != "object" or not isinstance(properties, Mapping): + continue + used: dict[tuple[str, str], str] = {} + for name, prop in properties.items(): + if not isinstance(prop, Mapping) or "order" not in prop: + continue + key = (str(prop.get("group", "")), json.dumps(prop["order"])) + if key in used: + errors.append( + f"{_spec_pointer((*path, 'properties', name, 'order'))} repeats the " + f"`order` of `{used[key]}`" + (f" in group `{key[0]}`." if key[0] else ".") + ) + else: + used[key] = name + return errors + + +def find_nested_group_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per `group` set below the top-level properties. + + Ported from CAT's `test_nested_group`: the setup form groups top-level fields only. + """ + return [ + f"{_spec_pointer((*path, 'group'))} is set below the top level; only top-level " + "properties can be grouped." + for path, schema in _schema_nodes(connection_specification) + if "group" in schema and path and not (len(path) == 2 and path[0] == "properties") + ] + + +def _config_paths(schema: Any, prefix: tuple[str, ...] = ()) -> Iterator[tuple[str, ...]]: + """Yield the path of every property a config can set, through `oneOf`/`anyOf` variants. + + An array's items add a `[]` segment, as in CAT's `get_expected_schema_structure`. + """ + if not isinstance(schema, Mapping): + return + for keyword in _COMBINATORS: + for variant in _variants(schema, (keyword,)): + yield from _config_paths(variant, prefix) + properties = schema.get("properties") + if isinstance(properties, Mapping): + for name, prop in properties.items(): + yield (*prefix, str(name)) + yield from _config_paths(prop, (*prefix, str(name))) + items = schema.get("items") + if isinstance(items, Mapping): + yield from _config_paths(items, (*prefix, "[]")) + + +def find_oauth_parameter_errors(spec: Mapping[str, Any]) -> list[str]: + """Return one error per `advanced_auth` config path that the connection spec lacks. + + Ported from CAT's `test_oauth_flow_parameters`. `predicate_key` and every + `path_in_connector_config` in the OAuth input and output specifications must name a + property of `connectionSpecification`, or the platform reads or writes a config field + that does not exist. + """ + advanced_auth = spec.get("advanced_auth") + if not isinstance(advanced_auth, Mapping): + return [] + known = set(_config_paths(spec.get("connectionSpecification") or {})) + referenced: list[tuple[str, list[Any]]] = [] + if isinstance(advanced_auth.get("predicate_key"), list): + referenced.append(("`/advanced_auth/predicate_key`", advanced_auth["predicate_key"])) + oauth = advanced_auth.get("oauth_config_specification") + if isinstance(oauth, Mapping): + for section in ( + "oauth_user_input_from_connector_config_specification", + "complete_oauth_output_specification", + "complete_oauth_server_output_specification", + ): + properties = (oauth.get(section) or {}).get("properties") + if not isinstance(properties, Mapping): + continue + for name, prop in properties.items(): + if isinstance(prop, Mapping) and isinstance( + prop.get("path_in_connector_config"), list + ): + referenced.append( + ( + _spec_pointer( + ( + "advanced_auth", + "oauth_config_specification", + section, + "properties", + name, + "path_in_connector_config", + ) + ), + prop["path_in_connector_config"], + ) + ) + return [ + f"{where} names the config path `/{'/'.join(str(part) for part in config_path)}`, " + "which is not a property of `connectionSpecification`." + for where, config_path in referenced + if tuple(str(part) for part in config_path) not in known + ] + + +def find_invalid_schema_errors(connection_specification: Mapping[str, Any]) -> list[str]: + """Return one error per place where the spec is not a valid JSON Schema. + + The spec is checked against the meta-schema of the draft it declares in `$schema`, draft 7 + by default, and draft 7 when `$schema` is not a string (its meta-schema then reports + `$schema`). CAT only found this through `test_config_match_spec`, when a scenario config + was validated against an invalid spec. + """ + return [ + f"{_spec_pointer(map(str, error.path))} is not valid JSON Schema: it fails the " + f"`{error.validator}` rule of the meta-schema." + for error in _meta_validator(connection_specification).iter_errors(connection_specification) + ] + + +@dataclass(frozen=True) +class SpecLint: + """A named credential-free assertion on a connector spec.""" + + name: str + rule: str + """What the lint requires and why, printed above its problems.""" + run: Callable[[Mapping[str, Any]], list[str]] + """Takes the whole SPEC payload (`connectionSpecification`, `advanced_auth`, ...).""" + enforced: bool = True + + +def _on_connection_specification( + check: Callable[[Mapping[str, Any]], list[str]], +) -> Callable[[Mapping[str, Any]], list[str]]: + def run(spec: Mapping[str, Any]) -> list[str]: + connection_specification = spec.get("connectionSpecification") + if not isinstance(connection_specification, Mapping): + return [] + return check(connection_specification) + + return run + + +SPEC_LINTS: tuple[SpecLint, ...] = ( + SpecLint( + "secret marking", + "A property that holds a credential must set `airbyte_secret: true` so the platform " + "masks it in the UI and logs and stores it in the secrets manager.", + _on_connection_specification(find_secret_marking_errors), + ), + SpecLint( + "valid JSON Schema", + "The spec must be a valid JSON Schema, or the platform cannot validate configs with it.", + _on_connection_specification(find_invalid_schema_errors), + ), + SpecLint( + "refs resolved", + "The platform does not resolve `$ref`, so the emitted spec must contain none.", + find_unresolved_ref_errors, + ), + SpecLint( + "additionalProperties is true", + "`additionalProperties` must be `true` when set, so that removing a property later " + "does not invalidate saved configs.", + _on_connection_specification(find_additional_properties_errors), + ), + SpecLint( + "oneOf usage", + "The setup form renders `oneOf` as a choice of object variants told apart by one " + "shared `const` property.", + _on_connection_specification(find_oneof_usage_errors), + ), + SpecLint( + "enum usage", + "An `enum` must not list the same value twice.", + _on_connection_specification(find_enum_usage_errors), + ), + SpecLint( + "array items", + "The setup form renders arrays of objects, strings, numbers, integers or enum values only.", + _on_connection_specification(find_array_type_errors), + ), + SpecLint( + "property types", + "The setup form renders a property's first type, so a list of types must be one type, " + "optionally followed by `null`.", + _on_connection_specification(find_property_type_errors), + ), + SpecLint( + "display_type", + "`display_type` belongs on a field with `oneOf` variants and must be `dropdown` or " + "`radio`.", + _on_connection_specification(find_display_type_errors), + ), + SpecLint( + "duplicate order", + "The setup form orders fields by `order`, which must be unique within one object and " + "group.", + _on_connection_specification(find_duplicate_order_errors), + ), + SpecLint( + "nested group", + "The setup form groups top-level fields only.", + _on_connection_specification(find_nested_group_errors), + ), + SpecLint( + "OAuth config paths", + "Every config path in `advanced_auth` must name a property of the connection " + "specification.", + find_oauth_parameter_errors, + ), + # Not enforced yet. `source-facebook-marketing` (a single-item `allOf` that pydantic emits + # for an enum field) and `destination-ragie` (pydantic-v2 `Optional` fields as `anyOf`) + # build their specs in code. Inlining the `allOf`, or rewriting `anyOf: [X, null]` as `X` with + # `type: [, "null"]`, accepts exactly the same configs, but the spec + # backward-compatibility check reports it as breaking. + SpecLint( + "forbidden keywords", + "The connector setup form cannot render these JSON Schema keywords.", + _on_connection_specification(find_forbidden_keyword_errors), + enforced=False, + ), + # Not enforced yet: `source-smoke-test` builds its spec in PyAirbyte, outside the monorepo, + # and its free-form `json_schema` and `records` objects cannot gain properties compatibly. + SpecLint( + "object not empty", + "The setup form cannot show an object with no `properties` and no `oneOf`.", + _on_connection_specification(find_empty_object_errors), + enforced=False, + ), +) +"""The lints `assert_spec_is_valid` runs, in the order their problems are reported.""" + + +def find_spec_problems( + spec: Mapping[str, Any], + lints: Iterable[SpecLint] = SPEC_LINTS, +) -> dict[SpecLint, list[str]]: + """Run each lint on a SPEC payload and return the problems of every failing lint. + + `spec` is the SPEC message's `spec` object as plain JSON. A lint that raises is reported + as a problem of that lint, so that a lint which is not enforced cannot fail the test by + crashing on a spec it did not anticipate. + """ + results: dict[SpecLint, list[str]] = {} + for lint in lints: + try: + problems = lint.run(spec) + except Exception as error: # Reported, not raised; see the docstring. + problems = [f"The lint could not run: {type(error).__name__}."] + if problems: + results[lint] = problems + return results + + +def format_spec_problems(problems: Mapping[SpecLint, list[str]]) -> str: + """Render problems grouped by lint, for an assertion or warning message.""" + return "\n".join( + f"[{lint.name}] {lint.rule}\n" + "\n".join(f" - {problem}" for problem in found) + for lint, found in problems.items() + ) + + def get_single_spec(result: EntrypointOutput, *, connector_name: str) -> ConnectorSpecification: """Assert that `spec` emitted exactly one SPEC message, and return its specification.""" spec_messages = result.spec_messages @@ -367,12 +970,24 @@ def get_single_spec(result: EntrypointOutput, *, connector_name: str) -> Connect def assert_spec_is_valid(spec: ConnectorSpecification, *, connector_name: str) -> None: - """Assert that a connector spec passes every credential-free lint.""" - errors = find_secret_marking_errors(spec.connectionSpecification) - assert not errors, ( - f"The spec of connector '{connector_name}' marks secrets incorrectly. A property that " - "holds a credential must set `airbyte_secret: true` so the platform masks it in the UI " - "and logs and stores it in the secrets manager:\n" + "\n".join(errors) + """Assert that a connector spec passes every enforced credential-free lint. + + Problems found by lints that are not enforced yet (see `SPEC_LINTS`) are reported as a + `SpecLintWarning` instead. No lint prints a config value: a spec holds none. + """ + problems = find_spec_problems(ConnectorSpecificationSerializer.dump(spec)) + advisory = {lint: found for lint, found in problems.items() if not lint.enforced} + if advisory: + warnings.warn( + f"The spec of connector '{connector_name}' has problems that will fail this test in " + f"a future CDK release:\n{format_spec_problems(advisory)}", + category=SpecLintWarning, + stacklevel=2, + ) + enforced = {lint: found for lint, found in problems.items() if lint.enforced} + assert not enforced, ( + f"The spec of connector '{connector_name}' fails the spec lint. See " + f"{SPEC_REFERENCE_URL} for the rules.\n{format_spec_problems(enforced)}" ) diff --git a/cdk-migrations.md b/cdk-migrations.md index 31426600c..8d19cfe5d 100644 --- a/cdk-migrations.md +++ b/cdk-migrations.md @@ -2,6 +2,47 @@ ## Upgrading to the next minor release (unreleased) +This release adds structural lints to the spec tests (`test_spec` and `test_docker_image_build_and_spec`), ported from the spec checks of the Connector Acceptance Tests (CAT). They run on the spec that `spec` emits, need no credentials, and report every problem with the JSON pointer of the place in the spec, never a value. The release is a minor one, since only the test harness changes. The monorepo connectors whose CI runs the CDK spec tests and that failed an enforced lint were fixed before this release, with these known exceptions: + +- `destination-timeplus` fails `additionalProperties is true`. It is archived and disabled in both registries, so nobody ships it. Its tests fail only when a pull request edits it. +- `source-mssql` (Java) fails `additionalProperties is true`: it emits a map property whose `additionalProperties` is a schema. It fails the `airbyte-cdk image test` step of airbyte's Dockerfile Tests workflow, which runs only when a pull request changes the Java connector Dockerfiles. +- `source-tplcentral` fails `additionalProperties is true` until its spec fix merges. Its legacy top-level `tests:` layout in `acceptance-test-config.yml` fails test collection, so the lints do not run on it until that config is converted. + +### Enforced spec lints + +The spec tests now fail when the emitted spec breaks one of these rules. Each failure names the lint, its rule and the pointer of every problem, and links the [connector specification reference](https://docs.airbyte.com/platform/connector-development/connector-specification-reference). + +| Lint | Rule | How to fix it | +|---|---|---| +| valid JSON Schema | `connectionSpecification` is valid against the meta-schema of the draft it declares in `$schema`, draft 7 by default. | Fix the reported keyword, such as a `type` that is not a JSON Schema type or a `required` that is not a list. | +| refs resolved | The emitted spec contains no `$ref`, in `connectionSpecification` or `advanced_auth`. | Emit the spec with every reference resolved (pydantic: resolve `$defs` before emitting it). | +| additionalProperties is true | `additionalProperties` is `true` wherever it is set. With `false` or a schema, removing a property later invalidates every saved config that sets it. | Set it to `true`, or remove it. Relaxing `false` to `true` is backward compatible. | +| oneOf usage | A schema with `oneOf` is `type: object`, every variant has `properties`, and the variants share exactly one discriminator: a property that sets `const` in every variant or, failing that, a single-item `enum` with that item as `default`. A `const` discriminator may set `default` and `enum` only to its `const`, and no two variants share a discriminator value. | Add a shared `const` property, such as `auth_type`, to each variant. | +| enum usage | An `enum` does not list the same value twice. | Remove the duplicate. | +| array items | An array's `items` is one schema, of type `object`, `string`, `number` or `integer` (a single type, not a list such as `[string, null]`), or with an `enum`. | Change the item type. | +| property types | A property whose `type` is a list holds one type, or one type followed by `null`. The setup form renders the first type only. | Reorder `[null, string]` to `[string, null]`. | +| display_type | `display_type` is set only on a field with `oneOf` variants, to `dropdown` or `radio`. | Remove it or fix its value. | +| duplicate order | No two properties of one object (and one `group`) share an `order`. | Give each property a unique `order`. `order` is an annotation, so changing it is backward compatible. | +| nested group | `group` is set only on top-level properties. | Move the `group` to the top-level property. | +| OAuth config paths | `advanced_auth.predicate_key` and every `path_in_connector_config` in `oauth_config_specification` name a property of `connectionSpecification` (through `oneOf` variants; `[]` for array items). | Fix the path, or add the property. | + +### Advisory spec lints + +These lints are reported as a `SpecLintWarning` and do not fail the test yet; a later release will enforce them: + +- **forbidden keywords**: the spec uses a keyword the setup form cannot render: `not`, `anyOf`, `allOf`, `patternProperties`, `prefixItems`, `if`, `then`, `else`, `dependentSchemas` or `dependentRequired`. Pydantic emits `allOf` for a field typed as an enum and `anyOf` for an `Optional` field (pydantic v2); inline the referenced schema. For an `Optional` field, replace `anyOf: [X, {type: null}]` with `X`'s keywords on the property and a `type` that lists `X`'s type followed by `null`, for example `type: [string, "null"]`. Keep the `null`: dropping it rejects every saved config that sets the field to `null`. +- **object not empty**: a nested `type: object` declares no `properties` and no `oneOf`, so the setup form shows nothing for it. Declare its properties, or remove it. + +To make the warnings fail the test now, run `airbyte-cdk connector test --pytest-arg=-Werror::airbyte_cdk.test.standard_tests.SpecLintWarning`, or pass `-W error::airbyte_cdk.test.standard_tests.SpecLintWarning` to `pytest`. + +### CAT spec checks that are not ported + +- `test_date_pattern` (CAT only warned) is dropped: it asked for `MM-DD-YYYY` on `format: date` fields, which contradicts the setup form's date picker, and adding a `pattern` narrows the accepted configs. +- `test_date_format` and `test_oauth_is_default_method` are handed to the connector QA checks (`airbyte-ops local connector qa`): each has a known violator that cannot be fixed compatibly or chose its default deliberately, so it needs a per-connector exception that the standard tests do not offer. +- `test_image_labels` and `test_image_environment_variables` are dropped: `airbyte-cdk image build` sets the `io.airbyte.name` and `io.airbyte.version` labels from `metadata.yaml`, and the shared connector Dockerfiles set `AIRBYTE_ENTRYPOINT` to the entrypoint, so a connector cannot get them wrong. + +## Upgrading to 7.39.0 + ### `acceptance-test-config.yml` keys the Standard Tests do not read fail collection `ConnectorTestScenario` declared `expect_records` (with `exact_order`), `timeout_seconds` and `file_types`, and no test read them: they parsed, and the check they configured in the retired Connector Acceptance Tests (CAT) never ran. They are removed. Any other key that the model did not declare was dropped without a word, so a misspelled key (`empty_stream` for `empty_streams`) was ignored as well. diff --git a/unit_tests/test/test_spec_lint.py b/unit_tests/test/test_spec_lint.py index e36704c79..5a49f8b2a 100644 --- a/unit_tests/test/test_spec_lint.py +++ b/unit_tests/test/test_spec_lint.py @@ -1303,3 +1303,37 @@ def run_docker_command(cmd: list[str], **kwargs: Any) -> subprocess.CompletedPro monkeypatch.setattr(docker_utils, "run_docker_command", run_docker_command) output = docker_utils.run_docker_airbyte_command(["docker", "run", "image", "spec"]) assert output.stderr == "warning: retrying\n" + + +def test_docker_spec_test_fails_on_a_structural_lint(monkeypatch: pytest.MonkeyPatch) -> None: + spec_message = _spec_message({"a": {"type": "string", "display_type": "dropdown"}}) + _fake_docker(monkeypatch, {"spec": _output(spec_message)}) + with pytest.raises(AssertionError, match="/properties/a/display_type"): + _DockerSuite().test_docker_image_build_and_spec( + connector_image_override="source-test:dev", + connector_base_image_override=None, + ) + + +def test_docker_spec_test_warns_on_an_advisory_lint(monkeypatch: pytest.MonkeyPatch) -> None: + from airbyte_cdk.test.standard_tests import SpecLintWarning + + _fake_docker(monkeypatch, {"spec": _output(_spec_message({"a": {"type": "object"}}))}) + with pytest.warns(SpecLintWarning, match="/properties/a"): + _DockerSuite().test_docker_image_build_and_spec( + connector_image_override="source-test:dev", + connector_base_image_override=None, + ) + + +class _EmptyObjectSpecSource(_LeakySource): + def spec(self, logger: logging.Logger) -> ConnectorSpecification: + return ConnectorSpecification(connectionSpecification=_spec({"a": {"type": "object"}})) + + +def test_in_process_spec_test_warns_on_an_advisory_lint(monkeypatch: pytest.MonkeyPatch) -> None: + from airbyte_cdk.test.standard_tests import SpecLintWarning + + monkeypatch.setattr(_InProcessSuite, "connector", _EmptyObjectSpecSource) + with pytest.warns(SpecLintWarning, match="/properties/a"): + _InProcessSuite().test_spec() diff --git a/unit_tests/test/test_spec_structural_lints.py b/unit_tests/test/test_spec_structural_lints.py new file mode 100644 index 000000000..d96620cc2 --- /dev/null +++ b/unit_tests/test/test_spec_structural_lints.py @@ -0,0 +1,643 @@ +# Copyright (c) 2026 Airbyte, Inc., all rights reserved. +"""Unit tests for the structural, presentational and OAuth spec lints in `_spec_lint`.""" + +import warnings +from typing import Any, Callable, Mapping + +import pytest + +from airbyte_cdk.models import ConnectorSpecification +from airbyte_cdk.test.standard_tests import SpecLintWarning +from airbyte_cdk.test.standard_tests._spec_lint import ( + SPEC_LINTS, + SpecLint, + assert_spec_is_valid, + find_additional_properties_errors, + find_array_type_errors, + find_display_type_errors, + find_duplicate_order_errors, + find_empty_object_errors, + find_enum_usage_errors, + find_forbidden_keyword_errors, + find_invalid_schema_errors, + find_nested_group_errors, + find_oauth_parameter_errors, + find_oneof_usage_errors, + find_property_type_errors, + find_spec_problems, + find_unresolved_ref_errors, + format_spec_problems, +) + +Lint = Callable[[Mapping[str, Any]], list[str]] + + +def _object(properties: dict[str, Any], **keywords: Any) -> dict[str, Any]: + return {"type": "object", "properties": properties, **keywords} + + +def _variant( + discriminator: dict[str, Any], name: str = "auth_type", **properties: Any +) -> dict[str, Any]: + return _object({name: discriminator, **properties}) + + +def _with_oneof(*variants: Any, **keywords: Any) -> dict[str, Any]: + return _object({"credentials": {"type": "object", "oneOf": list(variants), **keywords}}) + + +def _assert_errors(lint: Lint, schema: Mapping[str, Any], expected: list[str]) -> None: + """Assert one error per expected pointer, each naming its pointer, in order.""" + errors = lint(schema) + assert len(errors) == len(expected), errors + for error, pointer in zip(errors, expected): + assert error.startswith(f"`{pointer}`"), (error, pointer) + + +OAUTH_TYPE = {"type": "string", "const": "oauth"} +TOKEN_TYPE = {"type": "string", "const": "token"} + + +@pytest.mark.parametrize( + "schema, expected", + [ + pytest.param(_with_oneof(_variant(OAUTH_TYPE), _variant(TOKEN_TYPE)), [], id="const"), + pytest.param( + _with_oneof( + _variant({**OAUTH_TYPE, "default": "oauth", "enum": ["oauth"]}), + _variant(TOKEN_TYPE), + ), + [], + id="const_with_matching_default_and_enum", + ), + pytest.param( + _with_oneof( + _variant({"type": "string", "enum": ["oauth"], "default": "oauth"}), + _variant({"type": "string", "enum": ["token"], "default": "token"}), + ), + [], + id="single_item_enum_with_default", + ), + pytest.param( + _with_oneof( + _variant(OAUTH_TYPE, codec={"type": "string", "enum": ["gzip", "none"]}), + _variant(TOKEN_TYPE, codec={"type": "string", "enum": ["gzip", "none"]}), + ), + [], + id="shared_multi_item_enum_is_not_a_candidate", + ), + pytest.param( + _with_oneof( + _variant(OAUTH_TYPE, mode={"type": "string", "enum": ["a"], "default": "a"}), + _variant(TOKEN_TYPE, mode={"type": "string", "enum": ["b"], "default": "b"}), + ), + [], + id="one_const_candidate_wins_over_enum_candidates", + ), + pytest.param( + _object({"credentials": {"oneOf": [_variant(OAUTH_TYPE), _variant(TOKEN_TYPE)]}}), + ["/properties/credentials/oneOf"], + id="not_an_object", + ), + pytest.param( + _with_oneof(_variant(OAUTH_TYPE), {"type": "object"}), + ["/properties/credentials/oneOf/1"], + id="variant_without_properties", + ), + pytest.param( + _with_oneof(_variant(OAUTH_TYPE), _variant(TOKEN_TYPE, name="method")), + ["/properties/credentials/oneOf"], + id="no_shared_discriminator", + ), + pytest.param( + _with_oneof( + _variant(OAUTH_TYPE, kind={"const": "a"}), + _variant(TOKEN_TYPE, kind={"const": "b"}), + ), + ["/properties/credentials/oneOf"], + id="two_const_discriminators", + ), + pytest.param( + _with_oneof( + _variant({"type": "string", "enum": ["oauth"], "default": "oauth"}), + _variant({"type": "string", "enum": ["token"], "default": "token"}), + _variant({"type": "string", "enum": ["key"], "default": "key"}, name="x"), + ), + ["/properties/credentials/oneOf"], + id="discriminator_missing_from_a_variant", + ), + pytest.param( + _with_oneof(_variant({**OAUTH_TYPE, "default": "token"}), _variant(TOKEN_TYPE)), + ["/properties/credentials/oneOf/0/properties/auth_type"], + id="default_differs_from_const", + ), + pytest.param( + _with_oneof(_variant({**OAUTH_TYPE, "enum": ["token"]}), _variant(TOKEN_TYPE)), + ["/properties/credentials/oneOf/0/properties/auth_type"], + id="enum_differs_from_const", + ), + pytest.param( + _with_oneof( + _variant({"type": "string", "enum": ["oauth"]}), + _variant({"type": "string", "enum": ["token"], "default": "token"}), + ), + ["/properties/credentials/oneOf/0/properties/auth_type"], + id="enum_discriminator_without_default", + ), + pytest.param( + _with_oneof(_variant(OAUTH_TYPE), _variant(OAUTH_TYPE)), + ["/properties/credentials/oneOf"], + id="duplicate_discriminator_value", + ), + ], +) +def test_find_oneof_usage_errors(schema: dict[str, Any], expected: list[str]) -> None: + _assert_errors(find_oneof_usage_errors, schema, expected) + + +@pytest.mark.parametrize( + "lint, schema, expected", + [ + # enum usage + pytest.param( + find_enum_usage_errors, + _object({"a": {"enum": ["x", "y", 1, {"k": 1}]}}), + [], + id="enum_ok", + ), + pytest.param( + find_enum_usage_errors, + _object({"a": {"enum": ["x", "y", "x", "x"]}}), + ["/properties/a/enum", "/properties/a/enum"], + id="enum_duplicates", + ), + # forbidden keywords + pytest.param( + find_forbidden_keyword_errors, + _object({"not": {"type": "string"}, "if": {"type": "string"}}), + [], + id="forbidden_keyword_as_property_name", + ), + pytest.param( + find_forbidden_keyword_errors, + _object( + { + "a": {"allOf": [{"type": "string"}]}, + "b": {"anyOf": [{"type": "string"}, {"type": "null"}]}, + }, + patternProperties={"^x": {"type": "string"}}, + ), + ["/patternProperties", "/properties/a/allOf", "/properties/b/anyOf"], + id="forbidden_keywords", + ), + # array items + pytest.param( + find_array_type_errors, + _object( + { + "a": {"type": "array", "items": {"type": "string"}}, + "b": {"type": "array", "items": {"enum": ["x", "y"]}}, + "c": {"type": "array", "items": _object({"x": {"type": "string"}})}, + "d": {"type": "array"}, + } + ), + [], + id="array_ok", + ), + pytest.param( + find_array_type_errors, + _object( + { + "a": {"type": "array", "items": [{"type": "string"}]}, + "b": {"type": "array", "items": {"type": "boolean"}}, + "c": {"type": "array", "items": {"type": "array"}}, + } + ), + ["/properties/a/items", "/properties/b/items", "/properties/c/items"], + id="array_bad_items", + ), + # object not empty + pytest.param(find_empty_object_errors, {"type": "object"}, [], id="empty_root_ok"), + pytest.param( + find_empty_object_errors, + _object( + { + "a": _object({"x": {"type": "string"}}), + "b": {"type": "object", "oneOf": [_object({"x": {"const": "y"}})]}, + } + ), + [], + id="objects_ok", + ), + pytest.param( + find_empty_object_errors, + _object( + { + "a": {"type": "object"}, + "b": {"type": "object", "properties": {}}, + "c": {"type": "array", "items": {"type": "object"}}, + } + ), + ["/properties/a", "/properties/b", "/properties/c/items"], + id="empty_objects", + ), + # property types + pytest.param( + find_property_type_errors, + _object( + { + "a": {"type": "string"}, + "b": {"type": ["string"]}, + "c": {"type": ["integer", "null"]}, + } + ), + [], + id="property_types_ok", + ), + pytest.param( + find_property_type_errors, + _object( + { + "a": {"type": ["null", "string"]}, + "b": {"type": ["string", "integer"]}, + "c": {"type": ["string", "integer", "null"]}, + } + ), + ["/properties/a/type", "/properties/b/type", "/properties/c/type"], + id="property_types_bad", + ), + # additionalProperties + pytest.param( + find_additional_properties_errors, + _object({"a": _object({}, additionalProperties=True)}, additionalProperties=True), + [], + id="additional_properties_true", + ), + pytest.param( + find_additional_properties_errors, + _object( + {"a": _object({}, additionalProperties={"type": "string"})}, + additionalProperties=False, + ), + ["/additionalProperties", "/properties/a/additionalProperties"], + id="additional_properties_not_true", + ), + # display_type + pytest.param( + find_display_type_errors, + _with_oneof(_variant(OAUTH_TYPE), display_type="radio"), + [], + id="display_type_ok", + ), + pytest.param( + find_display_type_errors, + _object( + { + "a": {"type": "string", "display_type": "dropdown"}, + "b": {"type": "object", "oneOf": [], "display_type": "list"}, + } + ), + ["/properties/a/display_type", "/properties/b/display_type"], + id="display_type_bad", + ), + # duplicate order + pytest.param( + find_duplicate_order_errors, + _object( + { + "a": {"type": "string", "order": 0}, + "b": {"type": "string", "order": 1}, + "c": {"type": "string", "order": 0, "group": "advanced"}, + "d": _object( + {"x": {"type": "string", "order": 0}, "y": {"type": "string", "order": 1}} + ), + } + ), + [], + id="order_ok", + ), + pytest.param( + find_duplicate_order_errors, + _object( + { + "a": {"type": "string", "order": 0}, + "b": {"type": "string", "order": 0}, + "c": {"type": "string", "order": 1, "group": "g"}, + "d": {"type": "string", "order": 1, "group": "g"}, + } + ), + ["/properties/b/order", "/properties/d/order"], + id="order_duplicates", + ), + # nested group + pytest.param( + find_nested_group_errors, + _object({"a": {"type": "string", "group": "auth"}}), + [], + id="group_top_level", + ), + pytest.param( + find_nested_group_errors, + _object({"a": _object({"b": {"type": "string", "group": "auth"}})}), + ["/properties/a/properties/b/group"], + id="group_nested", + ), + # valid JSON Schema + pytest.param( + find_invalid_schema_errors, + _object({"a": {"type": "string", "minLength": 1}}, required=["a"]), + [], + id="valid_schema", + ), + pytest.param( + find_invalid_schema_errors, + _object({"a": {"type": "text"}}, required="a"), + ["/properties/a/type", "/required"], + id="invalid_schema", + ), + # Keyword values that are not strings are reported at their pointer, never crash. + pytest.param( + find_array_type_errors, + _object( + { + "a": {"type": "array", "items": {"type": ["string", "null"]}}, + "b": {"type": "array", "items": {"type": {"x": 1}}}, + } + ), + ["/properties/a/items", "/properties/b/items"], + id="array_items_type_not_a_string", + ), + pytest.param( + find_display_type_errors, + _object( + { + "a": { + "type": "object", + "oneOf": [_variant(OAUTH_TYPE)], + "display_type": ["radio"], + }, + "b": { + "type": "object", + "oneOf": [_variant(OAUTH_TYPE)], + "display_type": {"x": 1}, + }, + } + ), + ["/properties/a/display_type", "/properties/b/display_type"], + id="display_type_not_a_string", + ), + pytest.param( + find_invalid_schema_errors, + {**_object({"a": {"type": "string"}}), "$schema": ["draft-07"]}, + ["/$schema"], + id="schema_keyword_not_a_string", + ), + pytest.param( + find_invalid_schema_errors, + {**_object({"a": {"type": "string"}}), "$schema": {"x": 1}}, + ["/$schema"], + id="schema_keyword_an_object", + ), + ], +) +def test_structural_lints(lint: Lint, schema: dict[str, Any], expected: list[str]) -> None: + errors = lint(schema) + assert len(errors) == len(expected), errors + for error, pointer in zip(sorted(errors), sorted(expected)): + assert error.startswith(f"`{pointer}"), (error, pointer) + + +def test_nested_group_error_names_the_group_keyword() -> None: + errors = find_nested_group_errors(_object({"a": _object({"b": {"group": "g"}})})) + assert errors == [ + "`/properties/a/properties/b/group` is set below the top level; only top-level " + "properties can be grouped." + ] + + +def test_lints_ignore_values_that_look_like_schemas() -> None: + schema = _object( + {"a": {"type": "string", "default": {"type": "object"}, "examples": [{"allOf": []}]}} + ) + for lint in (find_empty_object_errors, find_forbidden_keyword_errors): + assert lint(schema) == [] + + +@pytest.mark.parametrize( + "spec, expected", + [ + pytest.param({"connectionSpecification": _object({"a": {"type": "string"}})}, [], id="ok"), + pytest.param( + { + "connectionSpecification": _object({"a": {"$ref": "#/definitions/a"}}), + "advanced_auth": {"oauth_config_specification": {"x": {"$ref": "#/y"}}}, + }, + [ + "/connectionSpecification/properties/a/$ref", + "/advanced_auth/oauth_config_specification/x/$ref", + ], + id="refs", + ), + ], +) +def test_find_unresolved_ref_errors(spec: dict[str, Any], expected: list[str]) -> None: + _assert_errors(find_unresolved_ref_errors, spec, expected) + + +def _oauth_spec(predicate_key: list[str], output_path: list[str]) -> dict[str, Any]: + return { + "connectionSpecification": _object( + { + "credentials": { + "type": "object", + "oneOf": [ + _variant( + OAUTH_TYPE, + client_id={"type": "string", "airbyte_secret": True}, + refresh_token={"type": "string", "airbyte_secret": True}, + ), + _variant(TOKEN_TYPE, api_token={"type": "string", "airbyte_secret": True}), + ], + }, + "accounts": { + "type": "array", + "items": _object({"id": {"type": "string"}}), + }, + } + ), + "advanced_auth": { + "auth_flow_type": "oauth2.0", + "predicate_key": predicate_key, + "predicate_value": "oauth", + "oauth_config_specification": { + "complete_oauth_output_specification": { + "type": "object", + "properties": { + "refresh_token": { + "type": "string", + "path_in_connector_config": output_path, + } + }, + }, + "complete_oauth_server_output_specification": { + "type": "object", + "properties": { + "client_id": { + "type": "string", + "path_in_connector_config": ["credentials", "client_id"], + }, + "account": { + "type": "string", + "path_in_connector_config": ["accounts", "[]", "id"], + }, + }, + }, + }, + }, + } + + +def test_find_oauth_parameter_errors_accepts_paths_through_oneof_and_arrays() -> None: + spec = _oauth_spec(["credentials", "auth_type"], ["credentials", "refresh_token"]) + assert find_oauth_parameter_errors(spec) == [] + + +def test_find_oauth_parameter_errors_reports_each_missing_path() -> None: + spec = _oauth_spec(["credentials", "method"], ["refresh_token"]) + assert find_oauth_parameter_errors(spec) == [ + ( + "`/advanced_auth/predicate_key` names the config path `/credentials/method`, which is " + "not a property of `connectionSpecification`." + ), + ( + "`/advanced_auth/oauth_config_specification/complete_oauth_output_specification/" + "properties/refresh_token/path_in_connector_config` names the config path " + "`/refresh_token`, which is not a property of `connectionSpecification`." + ), + ] + + +def test_find_oauth_parameter_errors_without_advanced_auth() -> None: + assert find_oauth_parameter_errors({"connectionSpecification": _object({})}) == [] + + +def test_pointer_escapes_slash_and_tilde() -> None: + errors = find_additional_properties_errors( + _object({"a/b": _object({}, additionalProperties=False), "c~d": {"type": "string"}}) + ) + assert errors[0].startswith("`/properties/a~1b/additionalProperties`") + + +def test_spec_lints_have_unique_names() -> None: + names = [lint.name for lint in SPEC_LINTS] + assert len(names) == len(set(names)) + + +def test_advisory_lints() -> None: + assert {lint.name for lint in SPEC_LINTS if not lint.enforced} == { + "forbidden keywords", + "object not empty", + } + + +def test_find_spec_problems_reports_a_crashing_lint_without_raising() -> None: + def crash(spec: Mapping[str, Any]) -> list[str]: + raise KeyError("sk_live_should_not_be_printed") + + lint = SpecLint("crash", "Never crashes.", crash, enforced=False) + problems = find_spec_problems({"connectionSpecification": _object({})}, lints=[lint]) + assert problems == {lint: ["The lint could not run: KeyError."]} + + +@pytest.mark.parametrize( + "connection_specification", + [ + pytest.param( + _object({"a": {"type": "array", "items": {"type": ["string", "null"]}}}), + id="array_items_type_list", + ), + pytest.param( + _object({"a": {"type": "object", "oneOf": [], "display_type": ["radio"]}}), + id="display_type_list", + ), + pytest.param({"type": "object", "$schema": ["draft-07"]}, id="schema_keyword_list"), + ], +) +def test_find_spec_problems_names_the_pointer_of_a_non_string_keyword_value( + connection_specification: dict[str, Any], +) -> None: + problems = find_spec_problems({"connectionSpecification": connection_specification}) + messages = [message for found in problems.values() for message in found] + assert messages + assert not any("could not run" in message for message in messages), messages + assert all(message.startswith("`/") for message in messages), messages + + +def test_find_spec_problems_on_a_clean_spec() -> None: + spec = { + "connectionSpecification": _object( + {"api_key": {"type": "string", "airbyte_secret": True, "order": 0}}, + additionalProperties=True, + ) + } + assert find_spec_problems(spec) == {} + + +def test_format_spec_problems_groups_by_lint() -> None: + first = SpecLint("first", "Rule one.", lambda spec: []) + second = SpecLint("second", "Rule two.", lambda spec: []) + text = format_spec_problems({first: ["a", "b"], second: ["c"]}) + assert text == "[first] Rule one.\n - a\n - b\n[second] Rule two.\n - c" + + +def _connector_spec(properties: dict[str, Any], **keywords: Any) -> ConnectorSpecification: + return ConnectorSpecification(connectionSpecification=_object(properties, **keywords)) + + +def test_assert_spec_is_valid_fails_on_an_enforced_lint_and_names_it() -> None: + spec = _connector_spec({"a": {"type": "string"}}, additionalProperties=False) + with pytest.raises(AssertionError) as error: + assert_spec_is_valid(spec, connector_name="source-test") + message = str(error.value) + assert "[additionalProperties is true]" in message + assert "`/additionalProperties`" in message + assert "connector-specification-reference" in message + + +def test_assert_spec_is_valid_warns_on_an_advisory_lint() -> None: + spec = _connector_spec({"a": {"type": "object"}, "b": {"allOf": [{"type": "string"}]}}) + with pytest.warns(SpecLintWarning) as record: + assert_spec_is_valid(spec, connector_name="source-test") + assert len(record) == 1 + message = str(record[0].message) + assert "[object not empty]" in message and "`/properties/a`" in message + assert "[forbidden keywords]" in message and "`/properties/b/allOf`" in message + + +def test_assert_spec_is_valid_warns_before_it_fails() -> None: + spec = _connector_spec({"a": {"type": "object"}}, additionalProperties=False) + with pytest.warns(SpecLintWarning), pytest.raises(AssertionError) as error: + assert_spec_is_valid(spec, connector_name="source-test") + assert "object not empty" not in str(error.value) + + +def test_assert_spec_is_valid_is_silent_on_a_clean_spec() -> None: + spec = _connector_spec({"a": {"type": "string", "order": 0}}) + with warnings.catch_warnings(): + warnings.simplefilter("error") + assert_spec_is_valid(spec, connector_name="source-test") + + +def test_failure_messages_do_not_print_values() -> None: + marker = "sk_live_value_from_the_spec" + spec = _connector_spec( + { + "a": {"type": "string", "enum": [marker, marker], "display_type": marker}, + "b": {"type": "string", "order": 3, "default": marker}, + "c": {"type": "string", "order": 3, "examples": [marker]}, + }, + additionalProperties={"description": marker}, + ) + with pytest.raises(AssertionError) as error: + assert_spec_is_valid(spec, connector_name="source-test") + assert marker not in str(error.value)