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

Allow all types of JSON API relationships #429

Closed
merkys opened this issue Jul 24, 2020 · 5 comments
Closed

Allow all types of JSON API relationships #429

merkys opened this issue Jul 24, 2020 · 5 comments
Labels
validator Related to the OPTIMADE validator

Comments

@merkys
Copy link
Member

merkys commented Jul 24, 2020

Currently the validator insists on list-only representations of relationships:

✖: https://www.crystallography.net/cod-test/optimade/structures?page_limit=5 - deserialize_response - failed with error
	ValidationError: 52 validation errors for ValidatorStructureResponseMany
[snip]
	data -> 0 -> relationships -> references -> data
	  `data` key in a relationship must always store a list. (type=value_error)
[snip]

In #397 (comment) @ml-evs and I have agreed that the specification does not limit relationship representations to lists only. Thus both of the following cases should be accepted as valid (citing @ml-evs fine examples):

  • Without the intermediate list:
      "relationships": {
        "references": {
          "data": {
            "id": "1000000",
            "type": "references"
          }
  • With the intermediate list:
      "relationships": {
        "references": {
          "data": [
            {
                "id": "1000000",
                "type": "references"
            }
          ]
@merkys merkys added the validator Related to the OPTIMADE validator label Jul 24, 2020
@CasperWA
Copy link
Member

I don't agree with this on the face of it.
In accordance with the JSON API specification there is a clear difference between to-one and to-many relationships. There are even conditions only applying to one and not the other (like pagination).
Since a structure may have several references related to it, it is clearly a to-many relationship, hence the value for the references relationships should be a list, empty of otherwise, if included.

However, if you understand the specification differently, please try to clarify.

@merkys
Copy link
Member Author

merkys commented Jul 28, 2020

In accordance with the JSON API specification there is a clear difference between to-one and to-many relationships. There are even conditions only applying to one and not the other (like pagination).

Indeed.

Since a structure may have several references related to it, it is clearly a to-many relationship, hence the value for the references relationships should be a list, empty of otherwise, if included.

After consulting JSON API and OPTIMADE specifications yet once more, I now lean towards thinking the same. Thanks for explanation!

@merkys merkys closed this as completed Jul 28, 2020
@ml-evs
Copy link
Member

ml-evs commented Jul 28, 2020

Just my two cents, I agree that to-many relationships were the aim of the specification (and would be my preferred choice for sure), but I'm not sure this is so clear cut in the specification as it stands. The spec just defers to the overall JSON:API relationships definition which contains the sentence:

Relationships may be to-one or to-many.

I would be in favour of adding a note to the specification that restricts our relationships to always be to-many.

@CasperWA
Copy link
Member

Just my two cents, I agree that to-many relationships were the aim of the specification (and would be my preferred choice for sure), but I'm not sure this is so clear cut in the specification as it stands. The spec just defers to the overall JSON:API relationships definition which contains the sentence:

Relationships may be to-one or to-many.

I would be in favour of adding a note to the specification that restricts our relationships to always be to-many.

Sure.
I think the intention in that sentence from the JSON API spec, however, is to reference the previous relationships resources can have, especially one-to-one or one-to-many.
But a clarification would sure be good 👍

@merkys
Copy link
Member Author

merkys commented Jul 29, 2020

I would not restrict to-many relationships altogether. I would leave sentence "Relationships may be to-one or to-many" as it is (because in the future we may want that for one reason or another), but clarify that references relationship is always to-many.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
validator Related to the OPTIMADE validator
Projects
None yet
Development

No branches or pull requests

3 participants