search

gbv / jskos-server

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

Possibly incorrect behavior in GET /mappings with `direction=both` #219

Closed stefandesu closed 2 weeks ago

stefandesu commented 2 months ago

I have noticed an edge case in the behavior in GET /mappings when direction=both, from, and toScheme (or to and fromScheme respectively) are given.

The documentation specifies:

direction=[direction] specify the direction of the mapping. Available values are: forward (default), backward (essentially swaps from and to), both (combines forward and backward).

For the above case, I would expect the results to contain the forward case (from and toScheme as is) plus the backward case (to=from and fromScheme=toScheme). However, direction=both does not always return the union of the forward and backward cases. For example:

https://coli-conc.gbv.de/api/mappings?from=http://uri.gbv.de/terminology/sdnb/100&toScheme=http://uri.gbv.de/terminology/bk/|http://bartoc.org/en/node/20049&direction=both

returns 35 mappings, while

https://coli-conc.gbv.de/api/mappings?from=http://uri.gbv.de/terminology/sdnb/100&toScheme=http://uri.gbv.de/terminology/bk/|http://bartoc.org/en/node/20049&direction=forward and https://coli-conc.gbv.de/api/mappings?from=http://uri.gbv.de/terminology/sdnb/100&toScheme=http://uri.gbv.de/terminology/bk/|http://bartoc.org/en/node/20049&direction=backward

contain 2 and 0 mappings, respectively.

What is happening here is that the forward and backward cases are mixed in the query so that is something like this:

(from is SDNB 100 OR to is SDNB 100) AND (toScheme is BK OR toScheme is SDNB OR fromScheme is BK OR fromScheme is SDNB)

The 33 unwanted mappings satisfy the conditions from is SDNB 100 AND fromScheme is SDNB, which is something I would not have expected given the query.

Of course, this is a weird example, giving the from's vocabulary in toScheme, but it is an actual use case I have where I now had to implement a workaround (two separate requests for forward and backward).

However, I'm not 100% sure that the behavior is actually incorrect. It is just not what I would have expected or how I'd interpret our documentation. @nichtich any thoughts?

nichtich commented 2 months ago

How about deprecating direction=both and asking clients to do two requests instead? The parameter makes implementations of mapping API more complex than needed.

stefandesu commented 2 months ago

I lean slightly towards just implementing it correctly (i.e. as the union of direction=forward and direction=backward), mostly because we don't have plans to release a new major version anytime soon, so direction=both might be used still for a long time. We use it ourselves in Cocoda (and in coli-rich-web, if not for this incorrect behavior).