gbv/cocoda-sdk

/status API endpoint overrides configured endpoint

Closed this issue · 9 comments

nichtich commented

Given a configuration

{
    "provider": "ConceptApi",
    "api": "http://example.org/",
    "schemes": "http://example.com/voc"
}

The schemes endpoint will be ignored but overridden by what is returned by /status endpoint.

I think this and #20 could be solved by filling this._api only after initialization with values from /status in _setup instead of _prepare.

stefandesu commented

I see your point. There are several different issues here though:

  • I agree that existing values in the configuration should definitely not be overridden by either assumed endpoints or the result of /status.
  • Then there's the question if any endpoints should even be assumed. My thought was that if we assume missing values, it would be easier to set up for example DANTE without having to explicitly naming every endpoint. On the other hand, assuming the endpoints also assumes that those endpoints even exist (which is the issue behind #20).
  • I would think that if there is a response on /status, we don't need to assume any another values. But /status itself has to be assumed if only api is given.

What do you think?

nichtich commented

A solved my use case for this issue by putting jskos-server instance behind a proxy so I could change endpoint base URIs. We could document the current behavior:

  • all endpoints are automatically set when api is given. Explicitly disable an endpoint by setting an it to undef (not tested, should solve #20)
  • when a status endpoint is set (explicitly or implicitly via api) it's return value will override all endpoints.
stefandesu commented

That's a good solution. Would you be opposed to using null instead of undefined for explicitly disabling an endpoint? It's much easier to check: status.type === null vs status.hasOwnProperty("type") && status.type === undefined.

nichtich commented

Nevermind null!

  • 👍1
stefandesu commented

So, it is now implemented this way:

  • Explicitly set endpoints always have priority and will not get overridden (even if they are null).
    • This allows us to override or disable endpoints if necessary for some reason.
  • Endpoints that are then still undefined will be set with the result of the status endpoint.
  • Endpoints that are then still undefined will be implicitly set if the api key is given.

Also, if the request to /status returns explicitly with a 404 error (i.e. it's not a network issue or some other server issue), we'll define that status endpoint as null.

Together with gbv/jskos-server#119, this should now work as expected.

stefandesu commented

We could change one more thing @nichtich, if you want: If the request to /status was successful, even endpoints that are not explicitly set to null will not be assumed to be there, considering that the server should know best which endpoints exist. What do you think?

nichtich commented
stefandesu commented

I forgot to implement this because the issue was closed, oops. Will ship a 1.0.1 tomorrow.

stefandesu commented
  • If the request to /status was successful, even endpoints that are not explicitly set to null will not be assumed to be there, considering that the server should know best which endpoints exist.
  • Explain this in the documentation!