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

Add endpoind field to linked_spaces, rename url field to website #92

Merged
merged 4 commits into from
May 7, 2024

Conversation

renzenicolai
Copy link
Contributor

@renzenicolai renzenicolai commented May 26, 2021

By adding a field to specify the SpaceAPI endpoints of other spaces self discovery of the spaces in the SpaceAPI network becomes possible. This change would eventually allow for a SpaceAPI that is functional without the central directory.

If the endpoint field is filled out then the URL field of the linked space should be left absent as the url can instead be read from the spaces own SpaceAPI endpoint. Because of this I've made the URL endpoint optional.

@rnestler
Copy link
Member

rnestler commented May 26, 2021

I remember discussing this as part of #75, but apparently not on the PR itself but somewhere else. As far as I remember @gidsi had some reasons not to use the SpaceAPI endpoint, but I can't recall them.

Edit: Found some stuff in the meeting notes:

@rnestler rnestler requested a review from gidsi May 26, 2021 15:28
@gidsi
Copy link
Member

gidsi commented May 26, 2021

The discussions are linked in the PR :)

I can write something about the "why" later

@s3lph
Copy link
Member

s3lph commented Sep 24, 2023

@SpaceApi/core I think we should have another look at this PR:

  • I agree that it would be beneficial if the SpaceAPI endpoints were all published at e.g. /.well-known/spaceapi.json. However, this is not the case currently, and even if we decided to go this way, it would take ages for all endpoints to migrate to the new path. Only linking to the base URL would make it impossible to link to spaces that have not migrated their endpoint to the new location.
  • Implementing other mechanisms for SpaceAPI endpoint discovery would require much more complicated clients. For example, with the <meta name="spaceapi"> HTML tag proposed in #144, clients would suddenly need to parse HTML in addition to JSON.

If we decide to merge this PR, I'd even go so far as to make the endpoint field mandatory and remove the url field (at least for now), as it would not serve any purpose at the moment, and would probably create confusion.

@dbrgn
Copy link
Contributor

dbrgn commented Sep 24, 2023

I agree with @s3lph. I think we should stick to endpoint URLs for now. They are easy to understand, allow for easy redirecting, and don't require any additional parsing.

If we decide to support some .well-known/...-scheme, we can always add that later on.

@dbrgn dbrgn added this to the API v15 milestone Apr 11, 2024
@dbrgn
Copy link
Contributor

dbrgn commented Apr 11, 2024

Decision in today's SpaceAPI core meeting:

  • Rename url to website
  • Support both website and endpoint, with anyOf requirement (i.e. make none of them required)

@s3lph
Copy link
Member

s3lph commented Apr 13, 2024

Hm unless I'm mistaken the "any of" requirement can't be modeled with JSONSchema. We could use minProperties: 1, but that would also pass validation if ONLY non-standard ext_ fields are provided. We could work around that with additionalProperties: false but that would prevent the use of ext_ fields entirely in this specific location.

@rnestler
Copy link
Member

Hm unless I'm mistaken the "any of" requirement can't be modeled with JSONSchema.

Why not? Reading https://json-schema.org/understanding-json-schema/reference/combining#anyOf it looks like it should be possible.

@s3lph
Copy link
Member

s3lph commented Apr 13, 2024

Huh... why is anyOf in the JSONSchema core spec, but required is in the validation spec? No wonder I didn't find it in the latter one.

So I guess we'd need to use something like this?

"linked_spaces": {
  "type": "array",
  "items": {
    "anyOf": [
      {
        "type": "object",
        "properties": {
          "endpoint": {
            "type": "string"
          }
        },
        "required": ["endpoint"]
      },
      {
        "type": "object",
        "properties": {
          "website": {
            "type": "string"
          }
        },
        "required": ["website"]
      }
    ]
  }
}

@rnestler
Copy link
Member

To my understanding you can extract all commen things out of the anyOf and just keep the required in it.

@rnestler

This comment was marked as resolved.

@dbrgn

This comment was marked as resolved.

@rnestler

This comment was marked as resolved.

15-draft.json Outdated
Comment on lines 1194 to 1200
"anyOf": [
"required": [
"endpoint"
],
"required": [
"website"
]

This comment was marked as outdated.

@rnestler rnestler requested a review from a team May 5, 2024 20:44
Copy link
Member

@rnestler rnestler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it with https://json-schema.hyperjump.io/ and it worked the way I expected it to work.

@rnestler rnestler requested a review from a team May 5, 2024 20:45
@rnestler rnestler changed the title Add endpoind field to linked_spaces, make url field optional Add endpoind field to linked_spaces, rename url field to website May 5, 2024
@rnestler rnestler merged commit 359b3a2 into SpaceApi:master May 7, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants