[full-ci] fix: [OCISDEV-330] limit search only when scope is passed#11664
[full-ci] fix: [OCISDEV-330] limit search only when scope is passed#11664
Conversation
LukasHirt
force-pushed
the
fix/search-scope
branch
3 times, most recently
from
b0d555f to
1f682d4
Compare
|
A lot of tests related to public links are failing. Not sure why those are affected but maybe it's due to the changed |
LukasHirt
force-pushed
the
fix/search-scope
branch
from
1f682d4 to
b34780c
Compare
LukasHirt
force-pushed
the
fix/search-scope
branch
from
b34780c to
cf8bd5c
Compare
LukasHirt
force-pushed
the
fix/search-scope
branch
from
d796668 to
9fc7ca4
Compare
| Bugfix: Limit search only when scope is passed | ||
|
|
||
| Previously, the search service would limit the search to the according space when searching `/dav/spaces/`. | ||
| This was not correct, as the search should be limited to the according space when a `scope` is passed in the search pattern instead. |
There was a problem hiding this comment.
Are we already doing this? Or are we taking away the ability to search in spaces for now?
There was a problem hiding this comment.
Yes, scope is already working
ocis/services/search/pkg/search/service.go
Line 105 in 54ed483
Also tested locally to verify this and works as expected.
There was a problem hiding this comment.
Ok 👍 We should still double check with clients. As far as I understand we are taking away the possibility to filter the space via URL, correct? This could break client behaviour if they filter like this.
There was a problem hiding this comment.
@DeepDiver1975 can you please let us know if we would break clients with this change? The topic is: searching for resources using /dav/spaces/* endpoint to limit results to a specific space instead of using scope in the search pattern.
@kobergj even if clients wouldn't be affected, should we change this then from enhancement to change to mark a breaking change? I kind of dislike bumping the major version to 8 just because of this... alternative is of course to adjust the behaviour to checking if there is actually the space ID passed in the endpoint and only limit it in such cases instead of completely dropping it.
There was a problem hiding this comment.
alternative is of course to adjust the behaviour to checking if there is actually the space ID passed in the endpoint and only limit it in such cases instead of completely dropping it
Isn't that already the case? When no spaceid is giving, parsing of the spaceID would fail, which leads to a debug level log. But the request will not get limited in that case.
There was a problem hiding this comment.
Is that happening in some different place? The change would drop it in this specific function.
There was a problem hiding this comment.
Clarified in chat, no breaking change needed and clients are not using this. We should be good to go.
LukasHirt
force-pushed
the
fix/search-scope
branch
from
9fc7ca4 to
e43e870
Compare
|
@kobergj any idea about the sonarcloud issue? What annotation does it want? :D |
Previously, the search service would limit the search to the according space when searching `/dav/spaces/`. This was not correct, as the search should be limited to the according space when a `scope` is passed in the search pattern instead.
LukasHirt
force-pushed
the
fix/search-scope
branch
from
925d08b to
a917473
Compare
|
|
It wanted an xml tag on the struct that is xml parsed. This makes sense but since you didn't touch it it is not relevant for this PR |
[full-ci] fix: [OCISDEV-330] limit search only when scope is passed
3 participants
Description
Previously, the search service would limit the search to the according space when searching
/dav/spaces/. This was not correct, as the search should be limited to the according space when ascopeis passed in the search pattern instead.Motivation and Context
Search behaviour is not dependent on
remote.php.Related issues
remote.phpreturns empty result (only withdav/spacespath) #10329How Has This Been Tested?
Types of changes