KnetMiner querySuggestor broken

[AjitPS]

KnetMiner querySuggestor broken in master (works in r4.0.3).

This code in OndexServiceProvider had been unchanged for years before the recent backend code refactoring a few months ago.

In the KnetMiner UI (utils.js) the user presses query suggestor button which goes from https://github.com/Rothamsted/knetminer/blob/master/client-base/src/main/webapp/html/javascript/utils.js#L687 to the knetminer "synonyms" api (https://github.com/Rothamsted/knetminer/blob/master/server-datasource-api/src/main/java/rres/knetminer/datasource/api/SynonymsResponse.java) using: https://github.com/Rothamsted/knetminer/blob/master/server-datasource-ondexlocal/src/main/java/rres/knetminer/datasource/ondexlocal/OndexLocalDataSource.java#L125.
​
in the past (release4.0 and r4.0.3) the synonymTable code was all in OndexServiceProvider: https://github.com/Rothamsted/knetminer/blob/release_4.0/common/server-datasource-ondexlocal/src/main/java/rres/knetminer/datasource/ondexlocal/OndexServiceProvider.java#L2454.

But I don't see it in new backend refactoring in "master" (when OSP was split into various services, etc) : https://github.com/Rothamsted/knetminer/tree/master/server-datasource-ondexlocal/src/main/java/rres/knetminer/datasource/ondexlocal/service
maybe that's why it fails in test wheat-ci instance too on babvs73, but we (@KeywanHP, @AjitPS) never tested this since the java code refactoring in early 2021.

P.S. the querySuggestor calls a synonyms api (passing user keywords) that eventually used writeSynonymTable() in OSP which I can't find in new codebase. fails on my new tomato test knets on babvs73 but then we found it also fails on wheat-ci instance on babvs73:9100 (which we somehow missed when testing that after the backend code refactoring by @marco-brandizi). In the UI in the searchbox if user types a keyword and presses the suggestor button then it just says "no synonym found" which in utils.js is a misleading msg that comes when synonyms api call fails.

I don't believe this has anything to do with my latest push (or a bad merge) as our test wheat-ci babvs73 instance was last deployed before I every pushed my code and in that querySuggestor already was failing.

At api level, the same can be seen as our old release4.0 deployed Wheat knetminer has synonyms api working: https://knetminer.rothamsted.ac.uk/wheatknet/synonyms?keyword=dormancy but not in latest wheat-ci http://babvs73.rothamsted.ac.uk:8083/ws/wheatknet/synonyms?keyword=dormancy (see this for api error), fyi

[marco-brandizi]

This is rendered by UIService.renderSynonymTable(), here.

Certainly, it has an error at:

int synCount = entryCountsByType.compute ( type, 
  // ====> wrong, should be thisCount == null ? ...
  (thisType, thisCount) -> thisType == null ? 1 : ++thisCount
); 

Now it seems fixed, see the test instances again.

However, even so, the method has been copy-pasted, refactored, migrated many times and I cannot understand what exactly is supposed to do anymore.

@AjitPS, @KeywanHP , please, give me more information on that. For the moment, I can see it does this:

• splits a search string into searchKeys
• for each key, searches associated concepts via Lucene, merges the resulting concepts by using the max score of those having the same ID.
• Then sorts the concepts by Lucene score and starts counting from the top-scored.
• In this loop, it apparently filters away those concepts which of type has reached 25 hits, keeping the previous better-scored results.

Does the above make sense? Could you provide test cases? Eg, search strings and expected results.

Ideally, could someone write a Unit test for that? The existing *IT.java is a starting point for that. If it has to be called against the wheat API, we can disable it and re-enable when we want to check manually.

[KeywanHP]

That's correct Marco. For example when you search for "germination OR dormancy" you would expect to see two tabs at the top (one for each keyword) and several tabs on the left (one for each concept class). Each tab would show the top 25 hits (concept names).

[marco-brandizi]

thanks, @KeywanHP, have a look at the usual test instances, they should be fine now.

[KeywanHP]

Looks good @marco-brandizi - thanks for fixing it!

[AjitPS]

thanks for resolving this @marco-brandizi ❤️