improve collision resolver and resolved schema tests

Author: p1-alexandrevaney

openapi_python_client/resolver/collision_resolver.py

@@ -44,7 +44,7 @@ def _browse_schema(self, attr: Any, root_attr: Any) -> None:
                             existing_ref.pointer.value != ref.pointer.value
                             and ref.pointer.tokens()[-1] == existing_ref.pointer.tokens()[-1]
                         ):
-                            print("Found same schema for different pointer")
+                            self._errors.append(f"Found a duplicate schema in {existing_ref.value} and {ref.value}")
                     else:
                         self._schema_index[hashed_schema] = ref
 
@@ -69,7 +69,7 @@ def _get_from_ref(self, ref: Reference, attr: SchemaData) -> SchemaData:
             if isinstance(cursor, dict) and key in cursor:
                 cursor = cursor[key]
             else:
-                print("ERROR")
+                self._errors.append(f"Did not find data corresponding to the reference {ref.value}")
 
         if list(cursor) == ["$ref"]:
             ref2 = Reference(cursor["$ref"], self._parent)

openapi_python_client/resolver/resolved_schema.py

@@ -41,6 +41,7 @@ def _process(self) -> None:
 
     def _process_remote_paths(self) -> None:
         refs_to_replace = []
+        refs_to_remove = []
         for owner, ref_key, ref_val in self._lookup_schema_references_in(self._root, "paths"):
             ref = Reference(ref_val, self._parent)
 
@@ -58,13 +59,17 @@ def _process_remote_paths(self) -> None:
                 remote_value = self._lookup_dict(remote_schema, tokens)
                 if not remote_value:
                     self._errors.append("Failed to read remote value {}, in remote ref {}".format(path, remote_path))
+                    refs_to_remove.append((owner, ref_key))
                 else:
                     refs_to_replace.append((owner, remote_schema, remote_value))
 
         for owner, remote_schema, remote_value in refs_to_replace:
             self._process_remote_components(remote_schema, remote_value, 1, self._parent)
             self._replace_reference_with(owner, remote_value)
 
+        for owner, ref_key in refs_to_remove:
+            owner.pop(ref_key)
+
     def _process_remote_components(
         self, owner: SchemaData, subpart: Union[SchemaData, None] = None, depth: int = 0, parent_path: str = None
     ) -> None:

tests/test_resolver/test_resolver_collision_resolver.py

@@ -5,6 +5,44 @@
 import pytest
 
 
+def test__collision_resolver_get_schema_from_ref():
+
+    from openapi_python_client.resolver.collision_resolver import CollisionResolver
+
+    root_schema = {"foo": {"$ref": "first_instance.yaml#/foo"}}
+
+    external_schemas = {"/home/user/first_instance.yaml": {"food": {"description": "food_first_description"}}}
+
+    errors = []
+
+    CollisionResolver(root_schema, external_schemas, errors, "/home/user").resolve()
+
+    assert len(errors) == 1
+    assert errors == ["Did not find data corresponding to the reference first_instance.yaml#/foo"]
+
+
+def test__collision_resolver_duplicate_schema():
+
+    from openapi_python_client.resolver.collision_resolver import CollisionResolver
+
+    root_schema = {
+        "foo": {"$ref": "first_instance.yaml#/foo"},
+        "bar": {"$ref": "second_instance.yaml#/bar/foo"},
+    }
+
+    external_schemas = {
+        "/home/user/first_instance.yaml": {"foo": {"description": "foo_first_description"}},
+        "/home/user/second_instance.yaml": {"bar": {"foo": {"description": "foo_first_description"}}},
+    }
+
+    errors = []
+
+    CollisionResolver(root_schema, external_schemas, errors, "/home/user").resolve()
+
+    assert len(errors) == 1
+    assert errors == ["Found a duplicate schema in first_instance.yaml#/foo and second_instance.yaml#/bar/foo"]
+
+
 def test__collision_resolver():
 
     from openapi_python_client.resolver.collision_resolver import CollisionResolver
@@ -17,6 +55,7 @@ def test__collision_resolver():
         "barfoobar": {"$ref": "first_instance.yaml#/bar/foo"},
         "localref": {"$ref": "#/local_ref"},
         "local_ref": {"description": "a local ref"},
+        "array": ["array_item_one", "array_item_two"],
         "last": {"$ref": "first_instance.yaml#/fourth_instance"},
         "baz": {"$ref": "fifth_instance.yaml#/foo"},
     }
@@ -27,7 +66,10 @@ def test__collision_resolver():
             "bar": {"foo": {"description": "nested foo"}},
             "fourth_instance": {"$ref": "fourth_instance.yaml#/foo"},
         },
-        "/home/user/second_instance.yaml": {"foo": {"description": "foo_second_description"}},
+        "/home/user/second_instance.yaml": {
+            "foo": {"description": "foo_second_description"},
+            "another_local_ref": {"$ref": "#/foo"},
+        },
         "/home/user/third_instance.yaml": {"foo": {"description": "foo_third_description"}},
         "/home/user/fourth_instance.yaml": {"foo": {"description": "foo_fourth_description"}},
         "/home/user/fifth_instance.yaml": {"foo": {"description": "foo_second_description"}},
@@ -41,6 +83,7 @@ def test__collision_resolver():
         "barfoobar": {"$ref": "first_instance.yaml#/bar/foo"},
         "localref": {"$ref": "#/local_ref"},
         "local_ref": {"description": "a local ref"},
+        "array": ["array_item_one", "array_item_two"],
         "last": {"$ref": "first_instance.yaml#/fourth_instance"},
         "baz": {"$ref": "fifth_instance.yaml#/foo_2"},
     }
@@ -51,7 +94,10 @@ def test__collision_resolver():
             "bar": {"foo": {"description": "nested foo"}},
             "fourth_instance": {"$ref": "fourth_instance.yaml#/foo_4"},
         },
-        "/home/user/second_instance.yaml": {"foo_2": {"description": "foo_second_description"}},
+        "/home/user/second_instance.yaml": {
+            "foo_2": {"description": "foo_second_description"},
+            "another_local_ref": {"$ref": "#/foo_2"},
+        },
         "/home/user/third_instance.yaml": {"foo_3": {"description": "foo_third_description"}},
         "/home/user/fourth_instance.yaml": {"foo_4": {"description": "foo_fourth_description"}},
         "/home/user/fifth_instance.yaml": {"foo_2": {"description": "foo_second_description"}},
@@ -64,3 +110,54 @@ def test__collision_resolver():
     assert len(errors) == 0
     assert root_schema == root_schema_result
     assert external_schemas == external_schemas_result
+
+
+def test__collision_resolver_deep_root_keys():
+
+    from openapi_python_client.resolver.collision_resolver import CollisionResolver
+
+    root_schema = {
+        "foobar": {"$ref": "first_instance.yaml#/bar/foo"},
+        "barfoo": {"$ref": "second_instance.yaml#/bar/foo"},
+        "barfoobar": {"$ref": "second_instance.yaml#/barfoobar"},
+    }
+
+    external_schemas = {
+        "/home/user/first_instance.yaml": {
+            "bar": {"foo": {"description": "foo_first_description"}},
+        },
+        "/home/user/second_instance.yaml": {
+            "bar": {"foo": {"description": "foo_second_description"}},
+            "barfoobar": {
+                "type": "object",
+                "allOf": [{"description": "first_description"}, {"description": "second_description"}],
+            },
+        },
+    }
+
+    root_schema_result = {
+        "foobar": {"$ref": "first_instance.yaml#/bar/foo"},
+        "barfoo": {"$ref": "second_instance.yaml#/bar/foo_2"},
+        "barfoobar": {"$ref": "second_instance.yaml#/barfoobar"},
+    }
+
+    external_schemas_result = {
+        "/home/user/first_instance.yaml": {
+            "bar": {"foo": {"description": "foo_first_description"}},
+        },
+        "/home/user/second_instance.yaml": {
+            "bar": {"foo_2": {"description": "foo_second_description"}},
+            "barfoobar": {
+                "type": "object",
+                "allOf": [{"description": "first_description"}, {"description": "second_description"}],
+            },
+        },
+    }
+
+    errors = []
+
+    CollisionResolver(root_schema, external_schemas, errors, "/home/user").resolve()
+
+    assert len(errors) == 0
+    assert root_schema == root_schema_result
+    assert external_schemas == external_schemas_result

tests/test_resolver/test_resolver_resolved_schema.py

@@ -10,7 +10,11 @@ def test__resolved_schema_with_resolved_external_references():
     from openapi_python_client.resolver.resolved_schema import ResolvedSchema
 
     root_schema = {"foobar": {"$ref": "foobar.yaml#/foo"}}
-    external_schemas = {"/home/user/foobar.yaml": {"foo": {"description": "foobar_description"}}}
+
+    external_schemas = {
+        "/home/user/foobar.yaml": {"foo": {"$ref": "/home/user/foobar_2.yaml#/foo"}},
+        "/home/user/foobar_2.yaml": {"foo": {"description": "foobar_description"}},
+    }
     errors = []
 
     resolved_schema = ResolvedSchema(root_schema, external_schemas, errors, "/home/user").schema
@@ -24,6 +28,97 @@ def test__resolved_schema_with_resolved_external_references():
     assert "foobar_description" in resolved_schema["foo"]["description"]
 
 
+def test__resolved_schema_with_duplicate_ref():
+
+    from openapi_python_client.resolver.resolved_schema import ResolvedSchema
+
+    root_schema = {
+        "foo": {"$ref": "foobar.yaml#/foo"},
+        "bar": {"$ref": "foobar.yaml#/foo"},
+        "list": [{"foobar": {"$ref": "foobar.yaml#/bar"}}, {"barfoo": {"$ref": "foobar.yaml#/bar2/foo"}}],
+    }
+
+    external_schemas = {
+        "/home/user/foobar.yaml": {
+            "foo": {"description": "foo_description"},
+            "bar": {"$ref": "#/foo"},
+            "bar2": {"foo": {"description": "foo_second_description"}},
+        },
+    }
+
+    errors = []
+
+    resolved_schema = ResolvedSchema(root_schema, external_schemas, errors, "/home/user").schema
+
+    assert len(errors) == 0
+
+
+def test__resolved_schema_with_malformed_schema():
+
+    from openapi_python_client.resolver.resolved_schema import ResolvedSchema
+
+    root_schema = {
+        "paths": {
+            "/foo/bar": {"$ref": "inexistant.yaml#/paths/~1foo~1bar"},
+            "/bar": {"$ref": "foobar.yaml#/paths/~1bar"},
+        },
+        "foo": {"$ref": "inexistant.yaml#/foo"},
+    }
+
+    external_schemas = {
+        "/home/user/foobar.yaml": {
+            "paths": {
+                "/foo/bar": {"description": "foobar_description"},
+            },
+        },
+    }
+
+    errors = []
+
+    resolved_schema = ResolvedSchema(root_schema, external_schemas, errors, "/home/user").schema
+
+    assert len(errors) == 4
+    assert errors == [
+        "Failed to resolve remote reference > /home/user/inexistant.yaml",
+        "Failed to read remote value /paths//bar, in remote ref /home/user/foobar.yaml",
+        "Failed to resolve remote reference > /home/user/inexistant.yaml",
+        "Failed to resolve remote reference > /home/user/inexistant.yaml",
+    ]
+
+
+def test__resolved_schema_with_remote_paths():
+
+    from openapi_python_client.resolver.resolved_schema import ResolvedSchema
+
+    root_schema = {
+        "paths": {
+            "/foo/bar": {"$ref": "foobar.yaml#/paths/~1foo~1bar"},
+            "/foo/bar2": {"$ref": "#/bar2"},
+        },
+        "bar2": {"description": "bar2_description"},
+    }
+
+    external_schemas = {
+        "/home/user/foobar.yaml": {
+            "paths": {
+                "/foo/bar": {"description": "foobar_description"},
+            },
+        },
+    }
+
+    expected_result = {
+        "paths": {"/foo/bar": {"description": "foobar_description"}, "/foo/bar2": {"$ref": "#/bar2"}},
+        "bar2": {"description": "bar2_description"},
+    }
+
+    errors = []
+
+    resolved_schema = ResolvedSchema(root_schema, external_schemas, errors, "/home/user").schema
+
+    assert len(errors) == 0
+    assert resolved_schema == expected_result
+
+
 def test__resolved_schema_with_absolute_paths():
 
     from openapi_python_client.resolver.resolved_schema import ResolvedSchema