diff --git a/frictionless/console/__spec__/test_console.py b/frictionless/console/__spec__/test_console.py index 26c087eafe..545dc70d7f 100644 --- a/frictionless/console/__spec__/test_console.py +++ b/frictionless/console/__spec__/test_console.py @@ -18,13 +18,13 @@ def test_console(): def test_console_version(): result = runner.invoke(console, "--version") assert result.exit_code == 0 - assert result.stdout.count(__version__) + assert result.output.count(__version__) def test_console_help(): result = runner.invoke(console, "--help") assert result.exit_code == 0 - assert result.stdout.count("Usage") + assert result.output.count("Usage") def test_console_error_bad_command(): diff --git a/frictionless/resources/__spec__/table/test_schema.py b/frictionless/resources/__spec__/table/test_schema.py index b9bf3ba91a..a7883d1d19 100644 --- a/frictionless/resources/__spec__/table/test_schema.py +++ b/frictionless/resources/__spec__/table/test_schema.py @@ -1,8 +1,16 @@ import sys +from typing import List, Optional import pytest -from frictionless import Detector, FrictionlessException, Schema, platform +from frictionless import ( + Detector, + FrictionlessException, + Package, + Resource, + Schema, + platform, +) from frictionless.resources import TableResource BASEURL = "https://raw.githubusercontent.com/frictionlessdata/frictionless-py/master/%s" @@ -213,3 +221,181 @@ def test_resource_schema_foreign_keys_invalid(): assert rows[2].to_dict() == {"id": 3, "cat": 1, "name": "London"} assert rows[3].to_dict() == {"id": 4, "cat": 2, "name": "Paris"} assert rows[4].to_dict() == {"id": 5, "cat": 6, "name": "Rome"} + + +def _handle_expected_validity_and_errors( + resource: Resource, + expected_validity: List[bool], + expected_errors: List[Optional[str]], +): + rows = resource.read_rows() + for i, (expected_valid, expected_error) in enumerate( + zip(expected_validity, expected_errors) + ): + assert rows[i].valid == expected_valid + if expected_error: + assert rows[i].errors[0].type == expected_error + + +@pytest.mark.parametrize( + "test_case", + [ + { + "name": "valid_self_referencing", + "data": [ + ["eventID", "parentEventID"], + ["1", ""], + ["2", "1"], + ["3", "1"], + ["4", "2"], + ["5", "3"], + ], + "expected_validity": [True, True, True, True, True], + "expected_errors": [None, None, None, None, None], + }, + { + "name": "invalid_self_referencing", + "data": [ + ["eventID", "parentEventID"], + ["1", ""], + ["2", "1"], + ["3", "999"], # Invalid reference to non-existent parent + ], + "expected_validity": [True, True, False], + "expected_errors": [None, None, "foreign-key"], + }, + ], +) +def test_resource_schema_self_referencing_foreign_keys(test_case): + """Test self-referencing foreign keys with explicit resource reference""" + descriptor = { + "name": "event", + "data": test_case["data"], + "schema": { + "fields": [ + {"name": "eventID", "type": "string"}, + {"name": "parentEventID", "type": "string"}, + ], + "primaryKey": ["eventID"], + "foreignKeys": [ + { + "fields": "parentEventID", + "reference": {"resource": "event", "fields": "eventID"}, + } + ], + }, + } + + resource = TableResource.from_descriptor(descriptor) + + _handle_expected_validity_and_errors( + resource, test_case["expected_validity"], test_case["expected_errors"] + ) + + # Same test but with implicit self-reference + descriptor["schema"]["foreignKeys"][0]["reference"].pop("resource", None) + + resource = TableResource.from_descriptor(descriptor) + + _handle_expected_validity_and_errors( + resource, test_case["expected_validity"], test_case["expected_errors"] + ) + + +@pytest.mark.parametrize( + "test_case", + [ + { + "name": "valid_circular_references", + "data_a": [ + ["id", "name", "ref_b"], + [1, "Item A1", 10], + [2, "Item A2", 20], + [3, "Item A3", ""], + ], + "data_b": [ + ["id", "name", "ref_a"], + [10, "Item B1", 1], + [20, "Item B2", 2], + [30, "Item B3", ""], + ], + "expected_validity_a": [True, True, True], + "expected_validity_b": [True, True, True], + "expected_errors_a": [None, None, None], + "expected_errors_b": [None, None, None], + }, + { + "name": "invalid_circular_references", + "data_a": [ + ["id", "name", "ref_b"], + [1, "Item A1", 10], + [2, "Item A2", 999], # Invalid reference + ], + "data_b": [ + ["id", "name", "ref_a"], + [10, "Item B1", 1], + [20, "Item B2", 888], # Invalid reference + ], + "expected_validity_a": [True, False], + "expected_validity_b": [True, False], + "expected_errors_a": [None, "foreign-key"], + "expected_errors_b": [None, "foreign-key"], + }, + ], +) +def test_resource_schema_circular_foreign_keys(test_case): + """Test circular foreign keys between two resources""" + package_descriptor = { + "name": "circular-package", + "resources": [ + { + "name": "resource_a", + "data": test_case["data_a"], + "schema": { + "fields": [ + {"name": "id", "type": "integer"}, + {"name": "name", "type": "string"}, + {"name": "ref_b", "type": "integer"}, + ], + "primaryKey": ["id"], + "foreignKeys": [ + { + "fields": "ref_b", + "reference": {"resource": "resource_b", "fields": "id"}, + } + ], + }, + }, + { + "name": "resource_b", + "data": test_case["data_b"], + "schema": { + "fields": [ + {"name": "id", "type": "integer"}, + {"name": "name", "type": "string"}, + {"name": "ref_a", "type": "integer"}, + ], + "primaryKey": ["id"], + "foreignKeys": [ + { + "fields": "ref_a", + "reference": {"resource": "resource_a", "fields": "id"}, + } + ], + }, + }, + ], + } + + package = Package.from_descriptor(package_descriptor) + + _handle_expected_validity_and_errors( + package.get_resource("resource_a"), + test_case["expected_validity_a"], + test_case["expected_errors_a"], + ) + _handle_expected_validity_and_errors( + package.get_resource("resource_b"), + test_case["expected_validity_b"], + test_case["expected_errors_b"], + ) diff --git a/frictionless/resources/table.py b/frictionless/resources/table.py index 5891ace0b9..056fee6e53 100644 --- a/frictionless/resources/table.py +++ b/frictionless/resources/table.py @@ -233,18 +233,25 @@ def __open_lookup(self): # Prepare source source_name = fk["reference"]["resource"] source_key = tuple(fk["reference"]["fields"]) - if source_name != "" and not self.package: - continue - if source_name: + if source_name == self.name or not source_name: + # Self reference + # A copy is needed as the resource is closed after the lookup + source_res = self.to_copy() + else: if not self.package: - note = 'package is required for FK: "{fk}"' + note = ( + 'package is required for foreign keys to other resources: "{fk}"' + ) raise FrictionlessException(errors.ResourceError(note=note)) + if not self.package.has_resource(source_name): note = f'failed to handle a foreign key for resource "{self.name}" as resource "{source_name}" does not exist' raise FrictionlessException(errors.ResourceError(note=note)) - source_res = self.package.get_resource(source_name) - else: - source_res = self.to_copy() + + # A copy is needed as the resource is closed after the lookup. + # Otherwise, this would cause issues in case of circular references. + source_res = self.package.get_resource(source_name).to_copy() + if source_res.schema: source_res.schema.foreign_keys = []