From 2d0794743fdcf459e26fe454e66ce8196558fadd Mon Sep 17 00:00:00 2001 From: Eugene Kulak Date: Fri, 28 Jan 2022 09:10:54 +0200 Subject: [PATCH 1/9] fix --- .../bases/source-acceptance-test/Dockerfile | 2 +- .../source_acceptance_test/tests/test_core.py | 9 ++-- .../utils/json_schema_helper.py | 53 ++++++++++++++++--- 3 files changed, 51 insertions(+), 13 deletions(-) diff --git a/airbyte-integrations/bases/source-acceptance-test/Dockerfile b/airbyte-integrations/bases/source-acceptance-test/Dockerfile index 849e15043350..ae1adebcf11b 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.44 +LABEL io.airbyte.version=0.1.45 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 a53116e5d419..3ddaa7e358db 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 @@ -76,18 +76,15 @@ def test_oneof_usage(self, actual_connector_spec: ConnectorSpecification): docs_msg = f"See specification reference at {docs_url}." schema_helper = JsonSchemaHelper(actual_connector_spec.connectionSpecification) - variant_paths = schema_helper.find_variant_paths() + variant_paths = schema_helper.find_nodes(keys=["oneOf", "anyOf"]) for variant_path in variant_paths: - top_level_obj = dpath.util.get(self._schema, "/".join(variant_path[:-1])) - if "$ref" in top_level_obj: - obj_def = top_level_obj["$ref"].split("/")[-1] - top_level_obj = self._schema["definitions"][obj_def] + top_level_obj = schema_helper.get_property(variant_path[:-1]) assert ( top_level_obj.get("type") == "object" ), f"The top-level definition in a `oneOf` block should have type: object. misconfigured object: {top_level_obj}. {docs_msg}" - variants = dpath.util.get(self._schema, "/".join(variant_path)) + variants = schema_helper.get_property(variant_path) for variant in variants: assert "properties" in variant, f"Each item in the oneOf array should be a property with type object. {docs_msg}" diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py index e96f9bc22e20..d81a2b8fd519 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py @@ -11,7 +11,9 @@ class CatalogField: - """Field class to represent cursor/pk fields""" + """Field class to represent cursor/pk fields. + It eases the read of values from records according to schema definition. + """ def __init__(self, schema: Mapping[str, Any], path: List[str]): self.schema = schema @@ -48,16 +50,45 @@ def parse(self, record: Mapping[str, Any], path: Optional[List[str]] = None) -> class JsonSchemaHelper: + """Helper class to simplify schema validation and read of records according to their schema.""" def __init__(self, schema): self._schema = schema - def get_ref(self, path: str): + def get_ref(self, path: str) -> Any: + """Resolve reference + + :param path: reference (#/definitions/SomeClass, etc) + :return: part of schema that is definition of the reference + :raises KeyError: in case path can't be followed + """ node = self._schema for segment in path.split("/")[1:]: node = node[segment] return node def get_property(self, path: List[str]) -> Mapping[str, Any]: + """Get any part of schema according to provided path, resolves $refs if necessary + + schema = { + "properties": { + "field1": { + "properties": { + "nested_field": { + + } + } + }, + "field2": ... + } + } + + helper = JsonSchemaHelper(schema) + helper.get_property(["field1", "nested_field"]) == + + :param path: list of fields in the order of navigation + :return: discovered part of schema + :raises KeyError: in case path can't be followed + """ node = self._schema for segment in path: if "$ref" in node: @@ -66,17 +97,27 @@ def get_property(self, path: List[str]) -> Mapping[str, Any]: return node def field(self, path: List[str]) -> CatalogField: - return CatalogField(schema=self.get_property(path), path=path) + """Get schema property and wrap it into CatalogField. + + CatalogField is a helper to ease the read of values from records according to schema definition. - def find_variant_paths(self) -> List[List[str]]: + :param path: list of fields in the order of navigation + :return: discovered part of schema wrapped in CatalogField + :raises KeyError: in case path can't be followed """ - return list of json object paths for oneOf or anyOf attributes + return CatalogField(schema=self.get_property(path), path=path) + + def find_nodes(self, keys: List[str]) -> List[List[str]]: + """Get all nodes of schema that has specifies properties + + :param keys: + :return: list of json object paths """ variant_paths = [] def traverse_schema(_schema, path=None): path = path or [] - if path and path[-1] in ["oneOf", "anyOf"]: + if path and path[-1] in keys: variant_paths.append(path) for item in _schema: next_obj = _schema[item] if isinstance(_schema, dict) else item From f64fcad6a989fc5487dda8ac5ccd9c7cdcf27297 Mon Sep 17 00:00:00 2001 From: Eugene Kulak Date: Fri, 28 Jan 2022 09:14:09 +0200 Subject: [PATCH 2/9] WIP --- airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md index 0c3b4dcc7c6c..b02cc5a2941a 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.44 +Fix test_oneof_usage test: [#9861](https://github.com/airbytehq/airbyte/pull/9861) + ## 0.1.44 Fix incorrect name of primary_keys attribute: [#9768](https://github.com/airbytehq/airbyte/pull/9768) From bea7b53f83d9ae0941167c3500f7825c036ed7d2 Mon Sep 17 00:00:00 2001 From: Eugene Kulak Date: Fri, 28 Jan 2022 09:15:27 +0200 Subject: [PATCH 3/9] fix pre-commit config --- .pre-commit-config.yaml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e62b52145420..90ac665c2b9d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,11 +11,17 @@ repos: rev: 21.11b1 hooks: - id: black + args: ["--line-length=140"] - repo: https://github.com/timothycrosley/isort rev: 5.10.1 hooks: - id: isort - args: ["--dont-follow-links", "--jobs=-1"] + args: + [ + "--settings-path=tools/python/.isort.cfg", + "--dont-follow-links", + "--jobs=-1", + ] additional_dependencies: ["colorama"] - repo: https://github.com/pre-commit/mirrors-prettier rev: v2.5.0 @@ -34,12 +40,14 @@ repos: rev: v0.0.1a2.post1 hooks: - id: pyproject-flake8 + args: ["--config=tools/python/.flake8"] additional_dependencies: ["mccabe"] alias: flake8 - repo: https://github.com/pre-commit/mirrors-mypy rev: v0.910-1 hooks: - id: mypy + args: ["--config-file=tools/python/.mypy.ini"] exclude: | (?x)^.*( octavia-cli/unit_tests/| From 8f2b8a6bfcd4cc662b66eb7b65a65fd2b84bddcf Mon Sep 17 00:00:00 2001 From: Eugene Kulak Date: Fri, 28 Jan 2022 09:16:09 +0200 Subject: [PATCH 4/9] format --- .../source_acceptance_test/utils/json_schema_helper.py | 1 + 1 file changed, 1 insertion(+) diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py index d81a2b8fd519..52a6d51cafd9 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py @@ -51,6 +51,7 @@ def parse(self, record: Mapping[str, Any], path: Optional[List[str]] = None) -> class JsonSchemaHelper: """Helper class to simplify schema validation and read of records according to their schema.""" + def __init__(self, schema): self._schema = schema From 5f655345dff65ca61f6c8d7dd6084294df3d81b0 Mon Sep 17 00:00:00 2001 From: Eugene Kulak Date: Fri, 28 Jan 2022 09:27:06 +0200 Subject: [PATCH 5/9] fix tests --- .../unit_tests/test_json_schema_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_json_schema_helper.py b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_json_schema_helper.py index 5e82660292f7..0cf64c67ea92 100644 --- a/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_json_schema_helper.py +++ b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_json_schema_helper.py @@ -148,7 +148,7 @@ class Root(BaseModel): f: Union[A, B] js_helper = JsonSchemaHelper(Root.schema()) - variant_paths = js_helper.find_variant_paths() + variant_paths = js_helper.find_nodes(keys=["anyOf", "oneOf"]) assert len(variant_paths) == 2 assert variant_paths == [["properties", "f", "anyOf"], ["definitions", "C", "properties", "e", "anyOf"]] # TODO: implement validation for pydantic generated objects as well From 9ecd418742d854354421a0f24c866970c0bfd517 Mon Sep 17 00:00:00 2001 From: Eugene Kulak Date: Fri, 28 Jan 2022 12:21:55 +0200 Subject: [PATCH 6/9] fix tests --- .../source_acceptance_test/tests/test_core.py | 4 +- .../utils/json_schema_helper.py | 14 + .../unit_tests/test_core.py | 343 ------------ .../unit_tests/test_spec.py | 517 ++++++++++++++++++ 4 files changed, 533 insertions(+), 345 deletions(-) create mode 100644 airbyte-integrations/bases/source-acceptance-test/unit_tests/test_spec.py 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 3ddaa7e358db..74fa5036805b 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 @@ -79,12 +79,12 @@ def test_oneof_usage(self, actual_connector_spec: ConnectorSpecification): variant_paths = schema_helper.find_nodes(keys=["oneOf", "anyOf"]) for variant_path in variant_paths: - top_level_obj = schema_helper.get_property(variant_path[:-1]) + top_level_obj = schema_helper.get_node(variant_path[:-1]) assert ( top_level_obj.get("type") == "object" ), f"The top-level definition in a `oneOf` block should have type: object. misconfigured object: {top_level_obj}. {docs_msg}" - variants = schema_helper.get_property(variant_path) + variants = schema_helper.get_node(variant_path) for variant in variants: assert "properties" in variant, f"Each item in the oneOf array should be a property with type object. {docs_msg}" diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py index 52a6d51cafd9..69913fa72a24 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py @@ -108,6 +108,20 @@ def field(self, path: List[str]) -> CatalogField: """ return CatalogField(schema=self.get_property(path), path=path) + def get_node(self, path: List[str]) -> Any: + """Return part of schema by specified path + + :param path: list of fields in the order of navigation + """ + + node = self._schema + for segment in path: + if "$ref" in node: + node = self.get_ref(node["$ref"]) + node = node[segment] + return node + + def find_nodes(self, keys: List[str]) -> List[List[str]]: """Get all nodes of schema that has specifies properties diff --git a/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_core.py b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_core.py index c925f5a82ab4..11a6f452791a 100644 --- a/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_core.py +++ b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_core.py @@ -11,134 +11,11 @@ AirbyteStream, ConfiguredAirbyteCatalog, ConfiguredAirbyteStream, - ConnectorSpecification, Type, ) from source_acceptance_test.config import BasicReadTestConfig from source_acceptance_test.tests.test_core import TestBasicRead as _TestBasicRead from source_acceptance_test.tests.test_core import TestDiscovery as _TestDiscovery -from source_acceptance_test.tests.test_core import TestSpec as _TestSpec - - -@pytest.mark.parametrize( - "connector_spec, should_fail", - [ - ( - { - "connectionSpecification": { - "type": "object", - "properties": { - "client_id": {"type": "string"}, - "client_secret": {"type": "string"}, - "access_token": {"type": "string"}, - "refresh_token": {"type": "string"}, - "$ref": None, - }, - } - }, - True, - ), - ( - { - "advanced_auth": { - "auth_flow_type": "oauth2.0", - "predicate_key": ["credentials", "auth_type"], - "predicate_value": "Client", - "oauth_config_specification": { - "complete_oauth_output_specification": { - "type": "object", - "properties": {"refresh_token": {"type": "string"}, "$ref": None}, - } - }, - } - }, - True, - ), - ( - { - "advanced_auth": { - "auth_flow_type": "oauth2.0", - "predicate_key": ["credentials", "auth_type"], - "predicate_value": "Client", - "oauth_config_specification": { - "complete_oauth_server_input_specification": { - "type": "object", - "properties": {"refresh_token": {"type": "string"}, "$ref": None}, - } - }, - } - }, - True, - ), - ( - { - "advanced_auth": { - "auth_flow_type": "oauth2.0", - "predicate_key": ["credentials", "auth_type"], - "predicate_value": "Client", - "oauth_config_specification": { - "complete_oauth_server_output_specification": { - "type": "object", - "properties": {"refresh_token": {"type": "string"}, "$ref": None}, - } - }, - } - }, - True, - ), - ( - { - "connectionSpecification": { - "type": "object", - "properties": { - "client_id": {"type": "string"}, - "client_secret": {"type": "string"}, - "access_token": {"type": "string"}, - "refresh_token": {"type": "string"}, - }, - } - }, - False, - ), - ( - { - "connectionSpecification": { - "type": "object", - "properties": { - "client_id": {"type": "string"}, - "client_secret": {"type": "string"}, - "access_token": {"type": "string"}, - "refresh_token": {"type": "string"}, - }, - }, - "advanced_auth": { - "auth_flow_type": "oauth2.0", - "predicate_key": ["credentials", "auth_type"], - "predicate_value": "Client", - "oauth_config_specification": { - "complete_oauth_server_output_specification": { - "type": "object", - "properties": {"refresh_token": {"type": "string"}}, - } - }, - }, - }, - False, - ), - ({"$ref": None}, True), - ({"properties": {"user": {"$ref": None}}}, True), - ({"properties": {"user": {"$ref": "user.json"}}}, True), - ({"properties": {"user": {"type": "object", "properties": {"username": {"type": "string"}}}}}, False), - ({"properties": {"fake_items": {"type": "array", "items": {"$ref": "fake_item.json"}}}}, True), - ], -) -def test_ref_in_spec_schemas(connector_spec, should_fail): - t = _TestSpec() - if should_fail is True: - with pytest.raises(AssertionError): - t.test_defined_refs_exist_in_json_spec_file(connector_spec_dict=connector_spec) - else: - t.test_defined_refs_exist_in_json_spec_file(connector_spec_dict=connector_spec) @pytest.mark.parametrize( @@ -234,226 +111,6 @@ def test_read(schema, record, should_fail): t.test_read(None, catalog, input_config, [], docker_runner_mock, MagicMock()) -@pytest.mark.parametrize( - "connector_spec, expected_error", - [ - # SUCCESS: no authSpecification specified - (ConnectorSpecification(connectionSpecification={}), ""), - # FAIL: Field specified in root object does not exist - ( - ConnectorSpecification( - connectionSpecification={"type": "object"}, - authSpecification={ - "auth_type": "oauth2.0", - "oauth2Specification": { - "rootObject": ["credentials", 0], - "oauthFlowInitParameters": [["client_id"], ["client_secret"]], - "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], - }, - }, - ), - "Specified oauth fields are missed from spec schema:", - ), - # SUCCESS: Empty root object - ( - ConnectorSpecification( - connectionSpecification={ - "type": "object", - "properties": { - "client_id": {"type": "string"}, - "client_secret": {"type": "string"}, - "access_token": {"type": "string"}, - "refresh_token": {"type": "string"}, - }, - }, - authSpecification={ - "auth_type": "oauth2.0", - "oauth2Specification": { - "rootObject": [], - "oauthFlowInitParameters": [["client_id"], ["client_secret"]], - "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], - }, - }, - ), - "", - ), - # FAIL: Some oauth fields missed - ( - ConnectorSpecification( - connectionSpecification={ - "type": "object", - "properties": { - "credentials": { - "type": "object", - "properties": { - "client_id": {"type": "string"}, - "client_secret": {"type": "string"}, - "access_token": {"type": "string"}, - }, - } - }, - }, - authSpecification={ - "auth_type": "oauth2.0", - "oauth2Specification": { - "rootObject": ["credentials", 0], - "oauthFlowInitParameters": [["client_id"], ["client_secret"]], - "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], - }, - }, - ), - "Specified oauth fields are missed from spec schema:", - ), - # SUCCESS: case w/o oneOf property - ( - ConnectorSpecification( - connectionSpecification={ - "type": "object", - "properties": { - "credentials": { - "type": "object", - "properties": { - "client_id": {"type": "string"}, - "client_secret": {"type": "string"}, - "access_token": {"type": "string"}, - "refresh_token": {"type": "string"}, - }, - } - }, - }, - authSpecification={ - "auth_type": "oauth2.0", - "oauth2Specification": { - "rootObject": ["credentials"], - "oauthFlowInitParameters": [["client_id"], ["client_secret"]], - "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], - }, - }, - ), - "", - ), - # SUCCESS: case w/ oneOf property - ( - ConnectorSpecification( - connectionSpecification={ - "type": "object", - "properties": { - "credentials": { - "type": "object", - "oneOf": [ - { - "properties": { - "client_id": {"type": "string"}, - "client_secret": {"type": "string"}, - "access_token": {"type": "string"}, - "refresh_token": {"type": "string"}, - } - }, - { - "properties": { - "api_key": {"type": "string"}, - } - }, - ], - } - }, - }, - authSpecification={ - "auth_type": "oauth2.0", - "oauth2Specification": { - "rootObject": ["credentials", 0], - "oauthFlowInitParameters": [["client_id"], ["client_secret"]], - "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], - }, - }, - ), - "", - ), - # FAIL: Wrong root object index - ( - ConnectorSpecification( - connectionSpecification={ - "type": "object", - "properties": { - "credentials": { - "type": "object", - "oneOf": [ - { - "properties": { - "client_id": {"type": "string"}, - "client_secret": {"type": "string"}, - "access_token": {"type": "string"}, - "refresh_token": {"type": "string"}, - } - }, - { - "properties": { - "api_key": {"type": "string"}, - } - }, - ], - } - }, - }, - authSpecification={ - "auth_type": "oauth2.0", - "oauth2Specification": { - "rootObject": ["credentials", 1], - "oauthFlowInitParameters": [["client_id"], ["client_secret"]], - "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], - }, - }, - ), - "Specified oauth fields are missed from spec schema:", - ), - # SUCCESS: root object index equal to 1 - ( - ConnectorSpecification( - connectionSpecification={ - "type": "object", - "properties": { - "credentials": { - "type": "object", - "oneOf": [ - { - "properties": { - "api_key": {"type": "string"}, - } - }, - { - "properties": { - "client_id": {"type": "string"}, - "client_secret": {"type": "string"}, - "access_token": {"type": "string"}, - "refresh_token": {"type": "string"}, - } - }, - ], - } - }, - }, - authSpecification={ - "auth_type": "oauth2.0", - "oauth2Specification": { - "rootObject": ["credentials", 1], - "oauthFlowInitParameters": [["client_id"], ["client_secret"]], - "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], - }, - }, - ), - "", - ), - ], -) -def test_validate_oauth_flow(connector_spec, expected_error): - t = _TestSpec() - if expected_error: - with pytest.raises(AssertionError, match=expected_error): - t.test_oauth_flow_parameters(connector_spec) - else: - t.test_oauth_flow_parameters(connector_spec) - - @pytest.mark.parametrize( "records, configured_catalog, expected_error", [ 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 new file mode 100644 index 000000000000..34527fa59f98 --- /dev/null +++ b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_spec.py @@ -0,0 +1,517 @@ +# +# Copyright (c) 2021 Airbyte, Inc., all rights reserved. +# + +from typing import Callable, Dict, Any + +import pytest +from airbyte_cdk.models import ConnectorSpecification +from source_acceptance_test.tests.test_core import TestSpec as _TestSpec + + +@pytest.mark.parametrize( + "connector_spec, should_fail", + [ + ( + { + "connectionSpecification": { + "type": "object", + "properties": { + "client_id": {"type": "string"}, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + "refresh_token": {"type": "string"}, + "$ref": None, + }, + } + }, + True, + ), + ( + { + "advanced_auth": { + "auth_flow_type": "oauth2.0", + "predicate_key": ["credentials", "auth_type"], + "predicate_value": "Client", + "oauth_config_specification": { + "complete_oauth_output_specification": { + "type": "object", + "properties": {"refresh_token": {"type": "string"}, "$ref": None}, + } + }, + } + }, + True, + ), + ( + { + "advanced_auth": { + "auth_flow_type": "oauth2.0", + "predicate_key": ["credentials", "auth_type"], + "predicate_value": "Client", + "oauth_config_specification": { + "complete_oauth_server_input_specification": { + "type": "object", + "properties": {"refresh_token": {"type": "string"}, "$ref": None}, + } + }, + } + }, + True, + ), + ( + { + "advanced_auth": { + "auth_flow_type": "oauth2.0", + "predicate_key": ["credentials", "auth_type"], + "predicate_value": "Client", + "oauth_config_specification": { + "complete_oauth_server_output_specification": { + "type": "object", + "properties": {"refresh_token": {"type": "string"}, "$ref": None}, + } + }, + } + }, + True, + ), + ( + { + "connectionSpecification": { + "type": "object", + "properties": { + "client_id": {"type": "string"}, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + "refresh_token": {"type": "string"}, + }, + } + }, + False, + ), + ( + { + "connectionSpecification": { + "type": "object", + "properties": { + "client_id": {"type": "string"}, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + "refresh_token": {"type": "string"}, + }, + }, + "advanced_auth": { + "auth_flow_type": "oauth2.0", + "predicate_key": ["credentials", "auth_type"], + "predicate_value": "Client", + "oauth_config_specification": { + "complete_oauth_server_output_specification": { + "type": "object", + "properties": {"refresh_token": {"type": "string"}}, + } + }, + }, + }, + False, + ), + ({"$ref": None}, True), + ({"properties": {"user": {"$ref": None}}}, True), + ({"properties": {"user": {"$ref": "user.json"}}}, True), + ({"properties": {"user": {"type": "object", "properties": {"username": {"type": "string"}}}}}, False), + ({"properties": {"fake_items": {"type": "array", "items": {"$ref": "fake_item.json"}}}}, True), + ], +) +def test_ref_in_spec_schemas(connector_spec, should_fail): + t = _TestSpec() + if should_fail is True: + with pytest.raises(AssertionError): + t.test_defined_refs_exist_in_json_spec_file(connector_spec_dict=connector_spec) + else: + t.test_defined_refs_exist_in_json_spec_file(connector_spec_dict=connector_spec) + + +def parametrize_test_case(*test_cases: Dict[str, Any]) -> Callable: + """Util to wrap pytest.mark.parametrize and provider more friendlier interface. + + @parametrize_test_case({"value": 10, "expected_to_fail": True}, {"value": 100, "expected_to_fail": False}) + + an equivalent to: + + @pytest.mark.parametrize("value,expected_to_fail", [(10, True), (100, False)]) + + :param test_cases: list of dicts + :return: pytest.mark.parametrize decorator + """ + all_keys = set() + for test_case in test_cases: + all_keys = all_keys.union(set(test_case.keys())) + all_keys.discard("test_id") + + test_ids = [] + values = [] + for test_case in test_cases: + test_ids.append(test_case.pop("test_id", None)) + values.append(tuple(test_case.get(k) for k in all_keys)) + + return pytest.mark.parametrize(",".join(all_keys), values, ids=test_ids) + +@parametrize_test_case( + { + "test_id": "all_good", + "connector_spec": { + "type": "object", + "properties": { + "select_type": { + "type": "object", + "oneOf": [ + { + "type": "object", + "properties": { + "option_title": { + "type": "string", + "title": "Title", + "const": "first option" + }, + "something": { + "type": "string" + } + } + }, + { + "type": "object", + "properties": { + "option_title": { + "type": "string", + "title": "Title", + "const": "second option" + }, + "some_field": { + "type": "boolean" + } + } + }, + + ], + }, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + }, + }, + "should_fail": False, + }, + { + "test_id": "top_level_node_is_not_of_object_type", + "connector_spec": { + "type": "object", + "properties": { + "select_type": { + "oneOf": [], + }, + }, + }, + "should_fail": True, + }, + { + "test_id": "all_oneof_options_should_have_same_constant_attribute", + "connector_spec": { + "type": "object", + "properties": { + "select_type": { + "type": "object", + "oneOf": [ + { + "type": "object", + "properties": { + "wrong_title": { + "type": "string", + "title": "Title", + "const": "first option" + }, + "something": { + "type": "string" + } + } + }, + { + "type": "object", + "properties": { + "option_title": { + "type": "string", + "title": "Title", + "const": "second option" + }, + "some_field": { + "type": "boolean" + } + } + }, + + ], + }, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + }, + }, + "should_fail": True, + }, + { + "test_id": "one_of_item_is_not_of_type_object", + "connector_spec": { + "type": "object", + "properties": { + "select_type": { + "type": "object", + "oneOf": [ + { + "type": "string", + }, + { + "type": "object", + "properties": { + "option_title": { + "type": "string", + "title": "Title", + "const": "second option" + }, + "some_field": { + "type": "boolean" + } + } + }, + + ], + }, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + }, + }, + "should_fail": True, + }, +) +def test_oneof_usage(connector_spec, should_fail): + t = _TestSpec() + if should_fail is True: + with pytest.raises(AssertionError): + t.test_oneof_usage(actual_connector_spec=ConnectorSpecification(connectionSpecification=connector_spec)) + else: + t.test_oneof_usage(actual_connector_spec=ConnectorSpecification(connectionSpecification=connector_spec)) + + +@pytest.mark.parametrize( + "connector_spec, expected_error", + [ + # SUCCESS: no authSpecification specified + (ConnectorSpecification(connectionSpecification={}), ""), + # FAIL: Field specified in root object does not exist + ( + ConnectorSpecification( + connectionSpecification={"type": "object"}, + authSpecification={ + "auth_type": "oauth2.0", + "oauth2Specification": { + "rootObject": ["credentials", 0], + "oauthFlowInitParameters": [["client_id"], ["client_secret"]], + "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], + }, + }, + ), + "Specified oauth fields are missed from spec schema:", + ), + # SUCCESS: Empty root object + ( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "client_id": {"type": "string"}, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + "refresh_token": {"type": "string"}, + }, + }, + authSpecification={ + "auth_type": "oauth2.0", + "oauth2Specification": { + "rootObject": [], + "oauthFlowInitParameters": [["client_id"], ["client_secret"]], + "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], + }, + }, + ), + "", + ), + # FAIL: Some oauth fields missed + ( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "type": "object", + "properties": { + "client_id": {"type": "string"}, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + }, + } + }, + }, + authSpecification={ + "auth_type": "oauth2.0", + "oauth2Specification": { + "rootObject": ["credentials", 0], + "oauthFlowInitParameters": [["client_id"], ["client_secret"]], + "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], + }, + }, + ), + "Specified oauth fields are missed from spec schema:", + ), + # SUCCESS: case w/o oneOf property + ( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "type": "object", + "properties": { + "client_id": {"type": "string"}, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + "refresh_token": {"type": "string"}, + }, + } + }, + }, + authSpecification={ + "auth_type": "oauth2.0", + "oauth2Specification": { + "rootObject": ["credentials"], + "oauthFlowInitParameters": [["client_id"], ["client_secret"]], + "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], + }, + }, + ), + "", + ), + # SUCCESS: case w/ oneOf property + ( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "type": "object", + "oneOf": [ + { + "properties": { + "client_id": {"type": "string"}, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + "refresh_token": {"type": "string"}, + } + }, + { + "properties": { + "api_key": {"type": "string"}, + } + }, + ], + } + }, + }, + authSpecification={ + "auth_type": "oauth2.0", + "oauth2Specification": { + "rootObject": ["credentials", 0], + "oauthFlowInitParameters": [["client_id"], ["client_secret"]], + "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], + }, + }, + ), + "", + ), + # FAIL: Wrong root object index + ( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "type": "object", + "oneOf": [ + { + "properties": { + "client_id": {"type": "string"}, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + "refresh_token": {"type": "string"}, + } + }, + { + "properties": { + "api_key": {"type": "string"}, + } + }, + ], + } + }, + }, + authSpecification={ + "auth_type": "oauth2.0", + "oauth2Specification": { + "rootObject": ["credentials", 1], + "oauthFlowInitParameters": [["client_id"], ["client_secret"]], + "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], + }, + }, + ), + "Specified oauth fields are missed from spec schema:", + ), + # SUCCESS: root object index equal to 1 + ( + ConnectorSpecification( + connectionSpecification={ + "type": "object", + "properties": { + "credentials": { + "type": "object", + "oneOf": [ + { + "properties": { + "api_key": {"type": "string"}, + } + }, + { + "properties": { + "client_id": {"type": "string"}, + "client_secret": {"type": "string"}, + "access_token": {"type": "string"}, + "refresh_token": {"type": "string"}, + } + }, + ], + } + }, + }, + authSpecification={ + "auth_type": "oauth2.0", + "oauth2Specification": { + "rootObject": ["credentials", 1], + "oauthFlowInitParameters": [["client_id"], ["client_secret"]], + "oauthFlowOutputParameters": [["access_token"], ["refresh_token"]], + }, + }, + ), + "", + ), + ], +) +def test_validate_oauth_flow(connector_spec, expected_error): + t = _TestSpec() + if expected_error: + with pytest.raises(AssertionError, match=expected_error): + t.test_oauth_flow_parameters(connector_spec) + else: + t.test_oauth_flow_parameters(connector_spec) From ceaa0dc624be0fedacfd0de2363933a60b578241 Mon Sep 17 00:00:00 2001 From: Eugene Kulak Date: Fri, 28 Jan 2022 12:23:03 +0200 Subject: [PATCH 7/9] format --- .../utils/json_schema_helper.py | 1 - .../unit_tests/test_core.py | 9 +-- .../unit_tests/test_spec.py | 68 +++++-------------- 3 files changed, 19 insertions(+), 59 deletions(-) diff --git a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py index 69913fa72a24..c7613756b6cb 100644 --- a/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py +++ b/airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/utils/json_schema_helper.py @@ -121,7 +121,6 @@ def get_node(self, path: List[str]) -> Any: node = node[segment] return node - def find_nodes(self, keys: List[str]) -> List[List[str]]: """Get all nodes of schema that has specifies properties diff --git a/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_core.py b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_core.py index 11a6f452791a..b1a51786b418 100644 --- a/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_core.py +++ b/airbyte-integrations/bases/source-acceptance-test/unit_tests/test_core.py @@ -5,14 +5,7 @@ from unittest.mock import MagicMock import pytest -from airbyte_cdk.models import ( - AirbyteMessage, - AirbyteRecordMessage, - AirbyteStream, - ConfiguredAirbyteCatalog, - ConfiguredAirbyteStream, - Type, -) +from airbyte_cdk.models import AirbyteMessage, AirbyteRecordMessage, AirbyteStream, ConfiguredAirbyteCatalog, ConfiguredAirbyteStream, Type from source_acceptance_test.config import BasicReadTestConfig from source_acceptance_test.tests.test_core import TestBasicRead as _TestBasicRead from source_acceptance_test.tests.test_core import TestDiscovery as _TestDiscovery 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 34527fa59f98..4ea6a67229e1 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 @@ -2,7 +2,7 @@ # Copyright (c) 2021 Airbyte, Inc., all rights reserved. # -from typing import Callable, Dict, Any +from typing import Any, Callable, Dict import pytest from airbyte_cdk.models import ConnectorSpecification @@ -147,7 +147,7 @@ def parametrize_test_case(*test_cases: Dict[str, Any]) -> Callable: all_keys = all_keys.union(set(test_case.keys())) all_keys.discard("test_id") - test_ids = [] + test_ids = [] values = [] for test_case in test_cases: test_ids.append(test_case.pop("test_id", None)) @@ -155,6 +155,7 @@ def parametrize_test_case(*test_cases: Dict[str, Any]) -> Callable: return pytest.mark.parametrize(",".join(all_keys), values, ids=test_ids) + @parametrize_test_case( { "test_id": "all_good", @@ -167,30 +168,17 @@ def parametrize_test_case(*test_cases: Dict[str, Any]) -> Callable: { "type": "object", "properties": { - "option_title": { - "type": "string", - "title": "Title", - "const": "first option" - }, - "something": { - "type": "string" - } - } + "option_title": {"type": "string", "title": "Title", "const": "first option"}, + "something": {"type": "string"}, + }, }, { "type": "object", "properties": { - "option_title": { - "type": "string", - "title": "Title", - "const": "second option" - }, - "some_field": { - "type": "boolean" - } - } + "option_title": {"type": "string", "title": "Title", "const": "second option"}, + "some_field": {"type": "boolean"}, + }, }, - ], }, "client_secret": {"type": "string"}, @@ -222,30 +210,17 @@ def parametrize_test_case(*test_cases: Dict[str, Any]) -> Callable: { "type": "object", "properties": { - "wrong_title": { - "type": "string", - "title": "Title", - "const": "first option" - }, - "something": { - "type": "string" - } - } + "wrong_title": {"type": "string", "title": "Title", "const": "first option"}, + "something": {"type": "string"}, + }, }, { "type": "object", "properties": { - "option_title": { - "type": "string", - "title": "Title", - "const": "second option" - }, - "some_field": { - "type": "boolean" - } - } + "option_title": {"type": "string", "title": "Title", "const": "second option"}, + "some_field": {"type": "boolean"}, + }, }, - ], }, "client_secret": {"type": "string"}, @@ -268,17 +243,10 @@ def parametrize_test_case(*test_cases: Dict[str, Any]) -> Callable: { "type": "object", "properties": { - "option_title": { - "type": "string", - "title": "Title", - "const": "second option" - }, - "some_field": { - "type": "boolean" - } - } + "option_title": {"type": "string", "title": "Title", "const": "second option"}, + "some_field": {"type": "boolean"}, + }, }, - ], }, "client_secret": {"type": "string"}, From 88dfe6e4c2ed1abc23332d9c5a00f720080b990a Mon Sep 17 00:00:00 2001 From: Eugene Kulak Date: Fri, 28 Jan 2022 13:54:45 +0200 Subject: [PATCH 8/9] bump version again to avoid conflict with existing --- .../bases/source-acceptance-test/CHANGELOG.md | 5 ++++- airbyte-integrations/bases/source-acceptance-test/Dockerfile | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md index b02cc5a2941a..03b723d956e7 100644 --- a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md +++ b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md @@ -1,8 +1,11 @@ # Changelog -## 0.1.44 +## 0.1.46 Fix test_oneof_usage test: [#9861](https://github.com/airbytehq/airbyte/pull/9861) +## 0.1.45 +_placeholder_ + ## 0.1.44 Fix incorrect name of primary_keys attribute: [#9768](https://github.com/airbytehq/airbyte/pull/9768) diff --git a/airbyte-integrations/bases/source-acceptance-test/Dockerfile b/airbyte-integrations/bases/source-acceptance-test/Dockerfile index ae1adebcf11b..016e23dbed02 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.45 +LABEL io.airbyte.version=0.1.46 LABEL io.airbyte.name=airbyte/source-acceptance-test ENTRYPOINT ["python", "-m", "pytest", "-p", "source_acceptance_test.plugin", "-r", "fEsx"] From e102329784010f41c75fa3811666ceabfa1bc224 Mon Sep 17 00:00:00 2001 From: Eugene Kulak Date: Fri, 28 Jan 2022 22:54:11 +0200 Subject: [PATCH 9/9] beatify CHANGELOG.md --- .../bases/source-acceptance-test/CHANGELOG.md | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md index 03b723d956e7..691d0595c00a 100644 --- a/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md +++ b/airbyte-integrations/bases/source-acceptance-test/CHANGELOG.md @@ -1,25 +1,25 @@ # Changelog ## 0.1.46 -Fix test_oneof_usage test: [#9861](https://github.com/airbytehq/airbyte/pull/9861) +Fix `test_oneof_usage` test: [#9861](https://github.com/airbytehq/airbyte/pull/9861) ## 0.1.45 _placeholder_ ## 0.1.44 -Fix incorrect name of primary_keys attribute: [#9768](https://github.com/airbytehq/airbyte/pull/9768) +Fix incorrect name of `primary_keys` attribute: [#9768](https://github.com/airbytehq/airbyte/pull/9768) ## 0.1.43 -FullRefresh test can compare records using PKs: [#9768](https://github.com/airbytehq/airbyte/pull/9768) +`TestFullRefresh` test can compare records using PKs: [#9768](https://github.com/airbytehq/airbyte/pull/9768) ## 0.1.36 -Add assert that spec.json file does not have any `$ref` in it: [#8842](https://github.com/airbytehq/airbyte/pull/8842) +Add assert that `spec.json` file does not have any `$ref` in it: [#8842](https://github.com/airbytehq/airbyte/pull/8842) ## 0.1.32 -Add info about skipped failed tests in /test command message on GitHub: [#8691](https://github.com/airbytehq/airbyte/pull/8691) +Add info about skipped failed tests in `/test` command message on GitHub: [#8691](https://github.com/airbytehq/airbyte/pull/8691) ## 0.1.31 -Take ConfiguredAirbyteCatalog from discover command by default +Take `ConfiguredAirbyteCatalog` from discover command by default ## 0.1.30 Validate if each field in a stream has appeared at least once in some record. @@ -40,13 +40,13 @@ Add ignored fields for full refresh test Fix incorrect nested structures compare. ## 0.1.24 -Improve message about errors in the stream's schema: https://github.com/airbytehq/airbyte/pull/6934 +Improve message about errors in the stream's schema: [#6934](https://github.com/airbytehq/airbyte/pull/6934) ## 0.1.23 Fix incorrect auth init flow check defect. ## 0.1.22 -Fix checking schemas with root $ref keyword +Fix checking schemas with root `$ref` keyword ## 0.1.21 Fix rootObject oauth init parameter check @@ -61,22 +61,22 @@ Assert a non-empty overlap between the fields present in the record and the decl Fix checking date-time format against nullable field. ## 0.1.17 -Fix serialize function for acceptance-tests: https://github.com/airbytehq/airbyte/pull/5738 +Fix serialize function for acceptance-tests: [#5738](https://github.com/airbytehq/airbyte/pull/5738) ## 0.1.16 -Fix for flake8-ckeck for acceptance-tests: https://github.com/airbytehq/airbyte/pull/5785 +Fix for flake8-ckeck for acceptance-tests: [#5785](https://github.com/airbytehq/airbyte/pull/5785) ## 0.1.15 -Add detailed logging for acceptance tests: https://github.com/airbytehq/airbyte/pull/5392 +Add detailed logging for acceptance tests: [5392](https://github.com/airbytehq/airbyte/pull/5392) ## 0.1.14 -Fix for NULL datetime in MySQL format (i.e. 0000-00-00): https://github.com/airbytehq/airbyte/pull/4465 +Fix for NULL datetime in MySQL format (i.e. `0000-00-00`): [#4465](https://github.com/airbytehq/airbyte/pull/4465) ## 0.1.13 -Replace `validate_output_from_all_streams` with `empty_streams` param: https://github.com/airbytehq/airbyte/pull/4897 +Replace `validate_output_from_all_streams` with `empty_streams` param: [#4897](https://github.com/airbytehq/airbyte/pull/4897) ## 0.1.12 -Improve error message when data mismatches schema: https://github.com/airbytehq/airbyte/pull/4753 +Improve error message when data mismatches schema: [#4753](https://github.com/airbytehq/airbyte/pull/4753) ## 0.1.11 Fix error in the naming of method `test_match_expected` for class `TestSpec`. @@ -85,19 +85,20 @@ Fix error in the naming of method `test_match_expected` for class `TestSpec`. Add validation of input config.json against spec.json. ## 0.1.9 -Add configurable validation of schema for all records in BasicRead test: https://github.com/airbytehq/airbyte/pull/4345 +Add configurable validation of schema for all records in BasicRead test: [#4345](https://github.com/airbytehq/airbyte/pull/4345) + The validation is ON by default. To disable validation for the source you need to set `validate_schema: off` in the config file. ## 0.1.8 -Fix cursor_path to support nested and absolute paths: https://github.com/airbytehq/airbyte/pull/4552 +Fix cursor_path to support nested and absolute paths: [#4552](https://github.com/airbytehq/airbyte/pull/4552) ## 0.1.7 Add: `test_spec` additionally checks if Dockerfile has `ENV AIRBYTE_ENTRYPOINT` defined and equal to space_joined `ENTRYPOINT` ## 0.1.6 -Add test whether PKs present and not None if `source_defined_primary_key` defined: https://github.com/airbytehq/airbyte/pull/4140 +Add test whether PKs present and not None if `source_defined_primary_key` defined: [#4140](https://github.com/airbytehq/airbyte/pull/4140) ## 0.1.5 -Add configurable timeout for the acceptance tests: https://github.com/airbytehq/airbyte/pull/4296 +Add configurable timeout for the acceptance tests: [#4296](https://github.com/airbytehq/airbyte/pull/4296)