gbv / jskos-server

Web service to access JSKOS data
https://coli-conc.gbv.de/api/
MIT License
6 stars 4 forks source link

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

Closed nichtich closed 1 year ago

nichtich commented 1 year ago

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 https://github.com/gbv/jskos-web/issues/5

stefandesu commented 1 year ago

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 commented 1 year ago

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

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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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.