Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SAT: check oneOf common property has only const keyword, no default and enum keywords #11704

Merged
merged 12 commits into from
Apr 7, 2022
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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": {
Expand Down