Repository: Materials-Consortia/optimade-python-tools (python)

Issue:
Links under top-level 'links' may be objects
I am trying to validate COD implementation (in development) using `optimade-python-tools` v0.9.8. The validator seems to require the values under the top-level `links` being strings:
```
$ optimade_validator https://www.crystallography.net/cod-test/optimade -v 5
Testing entire implementation at https://www.crystallography.net/cod-test/optimade...

Mandatory tests:
[snip]
2020-07-10 14:51:48,137 - optimade.validator.validator |    DEBUG: Testing multiple entry endpoint of references
✔: https://www.crystallography.net/cod-test/optimade/references?page_limit=5 - request successful.
✖: https://www.crystallography.net/cod-test/optimade/references?page_limit=5 - deserialize_response - failed with error
[snip]
	links -> first
	  str type expected (type=type_error.str)
	links -> next
	  str type expected (type=type_error.str)
Traceback (most recent call last):
[snip]
  File "/home/andrius/.local/lib/python3.6/site-packages/optimade/validator/validator.py", line 619, in get_endpoint
    request_str = request_str.replace("\n", "")
AttributeError: 'dict' object has no attribute 'replace'
```
In the COD implementation these values are "link objects", as per OPTIMADE specification:
```json
   "links" : {
      "first" : {
         "href" : "http://www.crystallography.net/cod-test/optimade/v1.0.0/references?page_limit=5"
      },
      "next" : {
         "href" : "http://www.crystallography.net/cod-test/optimade/v1.0.0/references?page_limit=5&page_offset=5"
      },
      "base_url" : "http://www.crystallography.net/cod-test/optimade/v1.0.0/"
   }
```

Pull request:
Allow Link objects for pagination
I found a few more links that can be `jsonapi.Link` objects that must have been causing problems. Closes #394 (I hope - sorry it took so long @merkys!)

Continues to add onto our issues of unravelling the schema models with nested `$ref`'s under `allOf` etc... in discussion with @CasperWA earlier, the suggestion is to try to find a general tool to dereference the JSON schema in place of our current homebrewed solution...

Patch:
diff --git a/openapi/index_openapi.json b/openapi/index_openapi.json
index f5bc5837..6a011bc7 100644
--- a/openapi/index_openapi.json
+++ b/openapi/index_openapi.json
@@ -1665,35 +1665,63 @@
           },
           "first": {
             "title": "First",
-            "maxLength": 65536,
-            "minLength": 1,
-            "type": "string",
-            "description": "The first page of data",
-            "format": "uri"
+            "anyOf": [
+              {
+                "type": "string",
+                "minLength": 1,
+                "maxLength": 65536,
+                "format": "uri"
+              },
+              {
+                "$ref": "#/components/schemas/Link"
+              }
+            ],
+            "description": "The first page of data"
           },
           "last": {
             "title": "Last",
-            "maxLength": 65536,
-            "minLength": 1,
-            "type": "string",
-            "description": "The last page of data",
-            "format": "uri"
+            "anyOf": [
+              {
+                "type": "string",
+                "minLength": 1,
+                "maxLength": 65536,
+                "format": "uri"
+              },
+              {
+                "$ref": "#/components/schemas/Link"
+              }
+            ],
+            "description": "The last page of data"
           },
           "prev": {
             "title": "Prev",
-            "maxLength": 65536,
-            "minLength": 1,
-            "type": "string",
-            "description": "The previous page of data",
-            "format": "uri"
+            "anyOf": [
+              {
+                "type": "string",
+                "minLength": 1,
+                "maxLength": 65536,
+                "format": "uri"
+              },
+              {
+                "$ref": "#/components/schemas/Link"
+              }
+            ],
+            "description": "The previous page of data"
           },
           "next": {
             "title": "Next",
-            "maxLength": 65536,
-            "minLength": 1,
-            "type": "string",
-            "description": "The next page of data",
-            "format": "uri"
+            "anyOf": [
+              {
+                "type": "string",
+                "minLength": 1,
+                "maxLength": 65536,
+                "format": "uri"
+              },
+              {
+                "$ref": "#/components/schemas/Link"
+              }
+            ],
+            "description": "The next page of data"
           }
         },
         "description": "A set of Links objects, possibly including pagination"
diff --git a/openapi/openapi.json b/openapi/openapi.json
index dd7d43b2..cb4f49c6 100644
--- a/openapi/openapi.json
+++ b/openapi/openapi.json
@@ -3340,35 +3340,63 @@
           },
           "first": {
             "title": "First",
-            "maxLength": 65536,
-            "minLength": 1,
-            "type": "string",
-            "description": "The first page of data",
-            "format": "uri"
+            "anyOf": [
+              {
+                "type": "string",
+                "minLength": 1,
+                "maxLength": 65536,
+                "format": "uri"
+              },
+              {
+                "$ref": "#/components/schemas/Link"
+              }
+            ],
+            "description": "The first page of data"
           },
           "last": {
             "title": "Last",
-            "maxLength": 65536,
-            "minLength": 1,
-            "type": "string",
-            "description": "The last page of data",
-            "format": "uri"
+            "anyOf": [
+              {
+                "type": "string",
+                "minLength": 1,
+                "maxLength": 65536,
+                "format": "uri"
+              },
+              {
+                "$ref": "#/components/schemas/Link"
+              }
+            ],
+            "description": "The last page of data"
           },
           "prev": {
             "title": "Prev",
-            "maxLength": 65536,
-            "minLength": 1,
-            "type": "string",
-            "description": "The previous page of data",
-            "format": "uri"
+            "anyOf": [
+              {
+                "type": "string",
+                "minLength": 1,
+                "maxLength": 65536,
+                "format": "uri"
+              },
+              {
+                "$ref": "#/components/schemas/Link"
+              }
+            ],
+            "description": "The previous page of data"
           },
           "next": {
             "title": "Next",
-            "maxLength": 65536,
-            "minLength": 1,
-            "type": "string",
-            "description": "The next page of data",
-            "format": "uri"
+            "anyOf": [
+              {
+                "type": "string",
+                "minLength": 1,
+                "maxLength": 65536,
+                "format": "uri"
+              },
+              {
+                "$ref": "#/components/schemas/Link"
+              }
+            ],
+            "description": "The next page of data"
           }
         },
         "description": "A set of Links objects, possibly including pagination"
diff --git a/optimade/models/jsonapi.py b/optimade/models/jsonapi.py
index 27881bdb..34bf1e4e 100644
--- a/optimade/models/jsonapi.py
+++ b/optimade/models/jsonapi.py
@@ -6,6 +6,7 @@ from pydantic import (  # pylint: disable=no-name-in-module
     BaseModel,
     AnyUrl,
     Field,
+    parse_obj_as,
     root_validator,
 )
 
@@ -61,10 +62,33 @@ class ToplevelLinks(BaseModel):
     )
 
     # Pagination
-    first: Optional[AnyUrl] = Field(None, description="The first page of data")
-    last: Optional[AnyUrl] = Field(None, description="The last page of data")
-    prev: Optional[AnyUrl] = Field(None, description="The previous page of data")
-    next: Optional[AnyUrl] = Field(None, description="The next page of data")
+    first: Optional[Union[AnyUrl, Link]] = Field(
+        None, description="The first page of data"
+    )
+    last: Optional[Union[AnyUrl, Link]] = Field(
+        None, description="The last page of data"
+    )
+    prev: Optional[Union[AnyUrl, Link]] = Field(
+        None, description="The previous page of data"
+    )
+    next: Optional[Union[AnyUrl, Link]] = Field(
+        None, description="The next page of data"
+    )
+
+    @root_validator(pre=False)
+    def check_additional_keys_are_links(cls, values):
+        """The `ToplevelLinks` class allows any additional keys, as long as
+        they are also Links or Urls themselves.
+
+        """
+        for key, value in values.items():
+            if key not in cls.schema()["properties"]:
+                values[key] = parse_obj_as(Optional[Union[AnyUrl, Link]], value)
+
+        return values
+
+    class Config:
+        extra = "allow"
 
 
 class ErrorLinks(BaseModel):
diff --git a/optimade/models/links.py b/optimade/models/links.py
index d1e440af..c6e6c57a 100644
--- a/optimade/models/links.py
+++ b/optimade/models/links.py
@@ -47,12 +47,12 @@ class LinksResourceAttributes(Attributes):
         ...,
         description="Human-readable description for the OPTIMADE API implementation, e.g., for use in clients to show a description to the end-user.",
     )
-    base_url: Union[AnyUrl, Link, None] = Field(
+    base_url: Optional[Union[AnyUrl, Link]] = Field(
         ...,
         description="JSON API links object, pointing to the base URL for this implementation",
     )
 
-    homepage: Union[AnyUrl, Link, None] = Field(
+    homepage: Optional[Union[AnyUrl, Link]] = Field(
         ...,
         description="JSON API links object, pointing to a homepage URL for this implementation",
     )


Test patch:
diff --git a/tests/models/test_jsonapi.py b/tests/models/test_jsonapi.py
index a63f5dc3..3d684666 100644
--- a/tests/models/test_jsonapi.py
+++ b/tests/models/test_jsonapi.py
@@ -1,6 +1,57 @@
-from optimade.models.jsonapi import Error
+from pydantic import ValidationError
+import pytest
+from optimade.models.jsonapi import Error, ToplevelLinks
 
 
 def test_hashability():
     error = Error(id="test")
     assert set([error])
+
+
+def test_toplevel_links():
+    """Test top-level links responses as both URLs and Links objects,
+    and check that any arbitrary key is allowed as long as the value
+    can be validated as a URL or a Links object too.
+
+    """
+    test_links = {
+        "first": {"href": "http://example.org/structures?page_limit=3&page_offset=0"},
+        "last": {"href": "http://example.org/structures?page_limit=3&page_offset=10"},
+        "prev": {
+            "href": "http://example.org/structures?page_limit=3&page_offset=2",
+            "meta": {"description": "the previous link"},
+        },
+        "next": {
+            "href": "http://example.org/structures?page_limit=3&page_offset=3",
+            "meta": {"description": "the next link"},
+        },
+    }
+
+    # Test all defined fields as URLs and Links objects
+    for link in test_links:
+        assert ToplevelLinks(**{link: test_links[link]})
+        assert ToplevelLinks(**{link: test_links[link]["href"]})
+
+    # Allow arbitrary keys are as long as they are links
+    assert ToplevelLinks(
+        **{
+            "base_url": "https://example.org/structures",
+            "other_url": {"href": "https://example.org"},
+        }
+    )
+    assert ToplevelLinks(
+        **{
+            "base_url5": {
+                "href": "https://example.org/structures",
+                "meta": {"description": "the base URL"},
+            },
+            "none_url": None,
+        }
+    )
+
+    # Check that non-URL and non-Links objects will fail to validate
+    with pytest.raises(ValidationError):
+        ToplevelLinks(**{"base_url": {"this object": "is not a URL or a Links object"}})
+
+    with pytest.raises(ValidationError):
+        ToplevelLinks(**{"base_url": {"href": "not a link"}})