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

/data endpoint should support concordances and mappings as well #193

Closed
nichtich opened this issue Feb 15, 2023 · 5 comments
Closed

/data endpoint should support concordances and mappings as well #193

nichtich opened this issue Feb 15, 2023 · 5 comments
Labels
feature Additional functionality
Milestone

Comments

@nichtich
Copy link
Member

nichtich commented Feb 15, 2023

Endpoint GET /data should also be usable to get specific mappings or concordances by their URIs (unless additional query parameters notation and voc are given).

Required by gbv/jskos-web#5

@nichtich nichtich added the feature Additional functionality label Feb 15, 2023
@stefandesu stefandesu added this to the 2.0.0 milestone Apr 5, 2023
@stefandesu
Copy link
Member

I've encountered one issue with this: Currently, the GET /data endpoint does return concept schemes in addition to concepts, but the code is written in a way that assumes that the endpoint returns concept data. This is important in particular for authentication.

In theory, I could rewrite the auth part so that among concepts, concept schemes, concordances, and mappings, only those that the current user is allowed to read are returned. I think that makes the most sense. What do you think, @nichtich?

Also, should annotations also be returned by GET /data?

Also, the other methods on the /data endpoint (POST, PUT, PATCH, DELETE) only refer to concepts. I wonder if at some point we should adjust the API to add a /concept endpoint for these methods instead. This is very much related to this comment in #171. For consistency, concept data should be handled under /concepts, and /narrower, /ancestors, /search, and /suggest should be moved to /concepts as well. That's definitely a separate issue and another breaking change, and I would not include this in version 2.0.0 (or if we do, the old endpoints should keep working for now).

@nichtich
Copy link
Member Author

I 'd prefer to restrict /data enpoint to read-access and deprecate POST/PUT/DELETE /data in favour of a new /concepts endpoint.

  • Instead of POST /data to add concepts, use POST /concepts
  • Instead of PUT /data to modify an existing concept, use PUT /concepts
  • Instead of DELETE /data to delete a concept, use DELETE /concepts
  • Instead of GET /data to get concepts, use GET /concepts or GET /voc/concepts

Then GET /data could be extended to return JSKOS items of all item types (concepts, vocabularies, mappings, concordances, annotations) based on a known URI. The endpoint could also be limited to public, unauthenticated access, so JSKOS items are only returned if their item type has been configured with "read": { "auth": false }. Would this be a breaking change?

@stefandesu
Copy link
Member

@nichtich I fully agree! Here's my suggestion:

For version 2.0, we modify the concept endpoints exactly like you described, completely removing the write-access to /data. cocoda-sdk has not yet implemented concept write access, so there's nothing to change over there yet.

We will also add aliases for /narrower, /ancestors, /search, and /suggest under /concepts (so /concepts/narrower and so on) because they are concept-specific endpoints. For the old endpoints, we will add deprecation warnings in the documentation and remove them in version 3.0 some time in the future. We then have enough time to make the necessary adjustments in cocoda-sdk and release a new major version for it. (This way, we don't need to update EVERYTHING at the exact same time, because removing the old endpoints would break Cocoda and other applications.)

Anything I forgot?

stefandesu added a commit that referenced this issue Apr 14, 2023
The old endpoints (narrower, ancestors, search, and suggest) are still available as deprecated aliases, but will be removed in version 3.0.
stefandesu added a commit that referenced this issue Apr 17, 2023
@stefandesu
Copy link
Member

The new GET /data endpoint is now implemented. I will add some tests before closing this issue. See the section in README.md for details about the endpoint (I kind of overspecified this particular endpoint because of the breaking change).

@stefandesu
Copy link
Member

Implementation is now complete. It should work as expected (i.e. the properties parameter also works and adjustments to the returned items are performed just as expected) and some of it is tested. We don't actually have tests for read authentication, so that's one thing which should work (and does work locally for me), but is not tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Additional functionality
Projects
None yet
Development

No branches or pull requests

2 participants