feat(Gravsearch): Enable ORDER BY external link (DEV-2704)

[mpro7]

Pull Request Checklist

Task Description/Number

Issue Number: DEV-2704

PR Type

• fix: represents bug fixes
• refactor: represents production code refactoring
• feat: represents a new feature
• docs: documentation changes (no production code change)
• chore: maintenance tasks (no production code change)
• test: all about tests: adding, refactoring tests (no production code change)
• other... Please describe:

Basic Requirements for bug fixes and features

• Tests for the changes have been added
• Docs have been added / updated

Does this PR introduce a breaking change?

• Yes

Does this PR change client-test-data?

• Yes

[linear]

DEV-2704 Gravsearch - Order by external link

It seems it is not possible to sort by external links via gravsearch. We should investigate to figure out why this is and make it possible if it doesn't require a major refactoring.

Here is the error message:
"knora-api:error": "dsp.errors.GravsearchException: ?prop0 cannot be used in ORDER BY","context":

{"knora-api": "http://api.knora.org/ontology/knora-api/v2#"}

Here is the gravsearch that causes the error:

PREFIX knora-api: ``[http://api.knora.org/ontology/knora-api/v2#](http://api.knora.org/ontology/knora-api/v2#)

CONSTRUCT {

?mainRes knora-api:isMainResource true .

?mainRes ``[http://api.stage.dasch.swiss/ontology/0801/beol/v2#letterHasURI](http://api.stage.dasch.swiss/ontology/0801/beol/v2#letterHasURI)`` ?prop0 .

} WHERE {

?mainRes a knora-api:Resource .

?mainRes a ``[http://api.stage.dasch.swiss/ontology/0801/beol/v2#letter](http://api.stage.dasch.swiss/ontology/0801/beol/v2#letter)`` .

?mainRes ``[http://api.stage.dasch.swiss/ontology/0801/beol/v2#letterHasURI](http://api.stage.dasch.swiss/ontology/0801/beol/v2#letterHasURI)`` ?prop0 .

}

ORDER BY ?prop0

If you take out the ORDER BY line at the end, then the gravsearch works.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (12402f3) 18.00% compared to head (8995684) 86.16%.
Report is 110 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #2902       +/-   ##
===========================================
+ Coverage   18.00%   86.16%   +68.16%     
===========================================
  Files         281      251       -30     
  Lines       28899    23072     -5827     
===========================================
+ Hits         5202    19879    +14677     
+ Misses      23697     3193    -20504     

see 274 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[BalduinLandolt]

LGTM

[seakayone]

I'm uncertain about how to interpret this PR. The underlying issue is related to an error in a specific query. You've added tests for two similar queries, but it appears that no changes were made to the production code. This raises some questions for me:

1. Does the error still exists in our stage environment where @mdelez has reported the problem? If it does, how does this PR address that issue? If not, do you know what resolved the original problem?
2. Your tests currently only assert the HTTP status code. Are we confident that the ORDER BY clause is functioning correctly? Would it be a good idea to include assertions to ensure that the returned results are ordered as expected? I would even suggest having a test that checks if ORDER BY DESC(?prop0) returns the reverse order.

Edit: I initially misunderstood the change to the AbstractPreQueryGenerator as a simple property ordering adjustment. However, upon closer inspection, I see that you've added two mappings: Xsd.Uri and KnoraApiV2Complex.UriValue. This addresses my initial question nr. 1.

Upon further reflection, and this may be a question for the issue creator, @mdelez: If I understand correctly, the 'uri' property being used for sorting contains the value of the actual URI. Is this really suitable for creating a meaningful order?

[mpro7]

I'm uncertain about how to interpret this PR. The underlying issue is related to an error in a specific query. You've added tests for two similar queries, but it appears that no changes were made to the production code. This raises some questions for me:

1. Does the error still exists in our stage environment where @mdelez has reported the problem? If it does, how does this PR address that issue? If not, do you know what resolved the original problem?
2. Your tests currently only assert the HTTP status code. Are we confident that the ORDER BY clause is functioning correctly? Would it be a good idea to include assertions to ensure that the returned results are ordered as expected? I would even suggest having a test that checks if ORDER BY DESC(?prop0) returns the reverse order.

Edit: I initially misunderstood the change to the AbstractPreQueryGenerator as a simple property ordering adjustment. However, upon closer inspection, I see that you've added two mappings: Xsd.Uri and KnoraApiV2Complex.UriValue. This addresses my initial question nr. 1.

Upon further reflection, and this may be a question for the issue creator, @mdelez: If I understand correctly, the 'uri' property being used for sorting contains the value of the actual URI. Is this really suitable for creating a meaningful order?

This small PR just enables something in already working sorting feature. I find the idea of checking the order in this case simply as the over-testing.

[mdelez]

@seakayone Whether or not the ordering of a URI value is meaningful or not, I don't see a reason why we wouldn't allow a user to do it. It may be meaningful for the user in some way if they have very specific URIs that they know can be sorted alphabetically to help them find their data faster.

[mpro7]

I found the searchextended route order being already checked in one of the SearchResponderV2Spec tests.