diff --git a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md index 777cd1a279685..08733983d16cb 100644 --- a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md +++ b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## 0.1.48 +Add checking that oneOf common property has only `const` keyword, no `default` and `enum` keywords: [#11704](https://github.com/airbytehq/airbyte/pull/11704) + ## 0.1.47 Added local test success message containing git hash: [#11497](https://github.com/airbytehq/airbyte/pull/11497) diff --git a/airbyte-integrations/bases/source-acceptance-test/Dockerfile b/airbyte-integrations/bases/source-acceptance-test/Dockerfile index ed3488439ad72..614b54bcb5f35 100644 --- a/airbyte-integrations/bases/source-acceptance-test/Dockerfile +++ b/airbyte-integrations/bases/source-acceptance-test/Dockerfile @@ -33,7 +33,7 @@ COPY pytest.ini setup.py ./ COPY source_acceptance_test ./source_acceptance_test RUN pip install . -LABEL io.airbyte.version=0.1.47 +LABEL io.airbyte.version=0.1.48 LABEL io.airbyte.name=airbyte/source-acceptance-test ENTRYPOINT ["python", "-m", "pytest", "-p", "source_acceptance_test.plugin", "-r", "fEsx"] diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py index c1d1da13a4785..251eb237f653d 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py @@ -88,12 +88,28 @@ def test_oneof_usage(self, actual_connector_spec: ConnectorSpecification): for variant in variants: assert "properties" in variant, f"Each item in the oneOf array should be a property with type object. {docs_msg}" - variant_props = [set(list(v["properties"].keys())) for v in variants] + oneof_path = ".".join(variant_path) + variant_props = [set(v["properties"].keys()) for v in variants] common_props = set.intersection(*variant_props) - assert common_props, "There should be at least one common property for oneOf subobjects" - assert any( - [all(["const" in var["properties"][prop] for var in variants]) for prop in common_props] - ), f"Any of {common_props} properties in {'.'.join(variant_path)} has no const keyword. {docs_msg}" + assert common_props, f"There should be at least one common property for {oneof_path} subobjects. {docs_msg}" + + const_common_props = set() + for common_prop in common_props: + if all(["const" in variant["properties"][common_prop] for variant in variants]): + const_common_props.add(common_prop) + assert ( + len(const_common_props) == 1 + ), f"There should be exactly one common property with 'const' keyword for {oneof_path} subobjects. {docs_msg}" + + const_common_prop = const_common_props.pop() + for n, variant in enumerate(variants): + prop_obj = variant["properties"][const_common_prop] + assert ( + "default" not in prop_obj + ), f"There should not be 'default' keyword in common property {oneof_path}[{n}].{const_common_prop}. Use `const` instead. {docs_msg}" + assert ( + "enum" not in prop_obj + ), f"There should not be 'enum' keyword in common property {oneof_path}[{n}].{const_common_prop}. Use `const` instead. {docs_msg}" def test_required(self): """Check that connector will fail if any required field is missing""" diff --git a/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_spec.py b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_spec.py index 4ea6a67229e1b..ad1e332d05b7e 100644 --- a/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_spec.py +++ b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_spec.py @@ -255,6 +255,90 @@ def parametrize_test_case(*test_cases: Dict[str, Any]) -> Callable: }, "should_fail": True, }, + { + "test_id": "no_common_property_for_all_oneof_subobjects", + "connector_spec": { + "type": "object", + "properties": { + "credentials": { + "type": "object", + "oneOf": [ + { + "type": "object", + "properties": { + "option1": {"type": "string"}, + "option2": {"type": "string"}, + }, + }, + { + "type": "object", + "properties": { + "option3": {"type": "string"}, + "option4": {"type": "string"}, + }, + }, + ], + } + }, + }, + "should_fail": True, + }, + { + "test_id": "two_common_properties_with_const_keyword", + "connector_spec": { + "type": "object", + "properties": { + "credentials": { + "type": "object", + "oneOf": [ + { + "type": "object", + "properties": { + "common1": {"type": "string", "const": "common1"}, + "common2": {"type": "string", "const": "common2"}, + }, + }, + { + "type": "object", + "properties": { + "common1": {"type": "string", "const": "common1"}, + "common2": {"type": "string", "const": "common2"}, + }, + }, + ], + } + }, + }, + "should_fail": True, + }, + { + "test_id": "default_keyword_in_common_property", + "connector_spec": { + "type": "object", + "properties": { + "credentials": { + "type": "object", + "oneOf": [ + { + "type": "object", + "properties": { + "common": {"type": "string", "const": "option1", "default": "option1"}, + "option1": {"type": "string"}, + }, + }, + { + "type": "object", + "properties": { + "common": {"type": "string", "const": "option2", "default": "option2"}, + "option2": {"type": "string"}, + }, + }, + ], + } + }, + }, + "should_fail": True, + }, ) def test_oneof_usage(connector_spec, should_fail): t = _TestSpec() diff --git a/docs/connector-development/connector-specification-reference.md b/docs/connector-development/connector-specification-reference.md index b0d881a7f4d53..6e9f94d3de06e 100644 --- a/docs/connector-development/connector-specification-reference.md +++ b/docs/connector-development/connector-specification-reference.md @@ -93,7 +93,7 @@ In order for the Airbyte UI to correctly render a specification, however, a few Let's look at the [source-file](../integrations/sources/file.md) implementation as an example. In this example, we have `provider` as a dropdown list option, which allows the user to select what provider their file is being hosted on. We note that the `oneOf` keyword lives under the `provider` object as follows: -In each item in the `oneOf` array, the `option_title` string field exists with the aforementioned `const`, `default` and `enum` value unique to that item. There is a [Github issue](https://github.com/airbytehq/airbyte/issues/6384) to improve it and use only `const` in the specification. This helps the UI and the connector distinguish between the option that was chosen by the user. This can be displayed with adapting the file source spec to this example: +In each item in the `oneOf` array, the `option_title` string field exists with the aforementioned `const` value unique to that item. This helps the UI and the connector distinguish between the option that was chosen by the user. This can be displayed with adapting the file source spec to this example: ```javascript { @@ -127,8 +127,6 @@ In each item in the `oneOf` array, the `option_title` string field exists with t "option_title": { "type": "string", "const": "HTTPS: Public Web", - "enum": ["HTTPS: Public Web"], - "default": "HTTPS: Public Web", "order": 0 } } @@ -141,8 +139,6 @@ In each item in the `oneOf` array, the `option_title` string field exists with t "option_title": { "type": "string", "const": "GCS: Google Cloud Storage", - "enum": ["GCS: Google Cloud Storage"], - "default": "GCS: Google Cloud Storage", "order": 0 }, "service_account_json": {