Term controller issues

[jamesamcl]

I think we need to review acdb56d with some urgency --

The logic which was removed in favour of getIdFromMultipleOptions I believe was actually copied from OLS3 so should have been correct already:

Also there are still issues with this endpoint e.g. #388

This issue should be our main priority right now as the endpoint is used by probably all existing API consumers.

@henrietteharmse can you identify exactly how the endpoint is supposed to behave by looking at the OLS3 controller and repository? I can help with any mappings to the OLS4 database but I think they are very straightforward (e.g. obo id -> curie, short_form -> shortForm)

@serjoshua I think we will probably need to revert the commit and have a rethink about how to implement it (to be clear, not your fault AT ALL - we just weren't aware of all the use cases until now.) -- it would really help it you could find the closed tickets before you changed the endpoint; reopen them; and link them here, so we can make sure we cover and test everything in the new fix before we close them again.

It's entirely possible there is a dataload issue hidden here that needs to be addressed, rather than something we can fix in the API backend.

[henrietteharmse]

The main differences seems to be the following:

1. The introduction of an additional id parameter to the termsByOntology method. @serjoshua @udp Is this a requirement from the frontend? My (incorrect?) impression was that the UI is using V2 rather than V1 API - or is there some cases where the UI calls V1 directly? If id is only used internally in the API, it should not be exposed on the API as a parameter.
2. The following code in commit acdb56d will cause nothing to be returned instead of all terms of the ontology as in OLS3. However, in the latest commit this is fixed.

        id = getIdFromMultipleOptions(iri, shortForm, oboId, id);
        if (id == null) {
            return new ResponseEntity<>(assembler.toModel(new PageImpl<V1Term>(Collections.emptyList()), termAssembler), HttpStatus.OK);
        }

The logic of @udp (before commit referred to here) is correct and @serjoshua code is correct at the latest commit as compared to OLS3. This commit referred to in this issue has indeed been problematic, but this has been corrected at the latest commit as far as I can see.

[henrietteharmse]

@serjoshua Can you please clarify the need for the additional id parameter?

[serjoshua]

The id parameter was taken from our select and search APIs. This is me assuming that we are streamlining our parameters across APIs. Like James mentioned, it is the same code as our OLS3. I could easily remove it if not needed in terms endpoint. But the thing is, I believe that this is not the main issue.

I already, commented on issue #388 regarding the actual problems we need to resolve for the API to work. They are all related to how V2 APIs work, which I still need to investigate more or discuss with James and then the others are more like dataload issues. As for the discrepancy in the number of elements returned between OLS3 and OLS4 search/select APIs, I am yet to start investigating on it.

[henrietteharmse]

@serjoshua Please check and close if we already dealt with.

[serjoshua]

Done checking. All issues directly related to this ticket has been handled or monitored elsewhere.