User and owner filters #237
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
If only I had seen your pull request earlier. It took me a complete day to find out why the search did not work for me. This PR fixes: #300 Edit: I am wondering how this pull request can be open for more than half a year. Is Nextcloud not used in corporate environments with AD integration? This is a killer bug for us, as it effectively breaks fulltext search for all our users. |
vbier
approved these changes
Sep 13, 2023
Hi. We have a patch on our environment and we apply it after every upgrade. |
|
@silvioq , can you try to request a review? See https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/requesting-a-pull-request-review. A suitable reviewer might be R0Wi or ArtificialOwl. There has to be a way to get the changes merged. |
vbier
approved these changes
Sep 15, 2023
|
I have approved the changes, but that does not seem to help as I do not have the needed write permissions on the repository. That is why I mentioned R0Wi and ArtificialOwl as reviewers. Can you request a review from one of them? |
|
Sorry. Only @vbier appears to review for me. I can close this pull and create again.
|
|
@R0Wi , @ArtificialOwl , sorry to bother you. Can you help top get the changes merged? |
|
@ArtificialOwl this seems to be closely related to #265 where you fixed the uppercase group filter by just lowercasing the input. I was working together with @Kdubs937 recently because he was facing a similar issue when trying to search for files from an external provider ( My fix here was to also call But to me using @silvioq did you find some official ES docs where they describe the behaviour of putting the |
The use of "keyword" will allow the search key to be exact, Elasticsearch will not perform any transformations. This use case will consider external user sources (LDAP), where the username may have dashes or symbols that could be misinterpreted by the engine. Signed-off-by: Silvio <[email protected]>
Signed-off-by: Robin Windey <[email protected]>
|
I took the liberty to also add the |
R0Wi
previously approved these changes
Sep 16, 2023
vbier
approved these changes
Sep 16, 2023
|
Thanks @R0Wi I'll test the changes in our nextcloud instance soon. About usage of |
|
I've applied this PR to NC27.1.0 and Fulltextsearch_elasticsearch 27.0.2 but afterwards fulltext search does not work anymore and test fails (see below). Applying nextcloud/fulltextsearch#771 does not solve this issue. Any idea? Any additional patch required to get this working? |
|
Thanks @XueSheng-GIT for your feedback! I tested with a clean install of Nextcloud 28 (current |
|
@R0Wi Thanks for providing some guidance! Log for error shown above #237 (comment): Same without this PR (no error, alls tests pass): I can just see that there's no hit if this PR is applied. Does the query look as expected? |
|
Yes both queries look exactly the same, except the filter part which has been changed from {
"bool": {
"should": [
{
"term": {
"owner": "user1"
}
},
{
"term": {
"users": "user1"
}
},
{
"term": {
"users": "__all"
}
}
]
}
}to {
"bool": {
"should": [
{
"term": {
"owner.keyword": "user1"
}
},
{
"term": {
"users.keyword": "user1"
}
},
{
"term": {
"users.keyword": "__all"
}
}
]
}
}which is expected. @XueSheng-GIT would you mind sharing the ES index |
|
/This is the _mapping info: {
"nextcloud" : {
"mappings" : {
"dynamic" : "true",
"properties" : {
"attachment" : {
"properties" : {
"author" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
},
"content_length" : {
"type" : "long"
},
"content_type" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
},
"creator_tool" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
},
"date" : {
"type" : "date"
},
"format" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
},
"keywords" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
},
"language" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
},
"modified" : {
"type" : "date"
},
"title" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"circles" : {
"type" : "keyword"
},
"combined" : {
"type" : "text",
"term_vector" : "with_positions_offsets",
"analyzer" : "analyzer"
},
"content" : {
"type" : "text",
"copy_to" : [
"combined"
],
"term_vector" : "with_positions_offsets",
"analyzer" : "analyzer"
},
"groups" : {
"type" : "keyword"
},
"hash" : {
"type" : "keyword"
},
"links" : {
"type" : "keyword"
},
"metatags" : {
"type" : "keyword"
},
"owner" : {
"type" : "keyword"
},
"parts" : {
"properties" : {
"comments" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
},
"ocr" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"provider" : {
"type" : "keyword"
},
"share_names" : {
"properties" : {
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
},
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX\\" : {
"properties" : {
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"XXX" : {
"type" : "text",
"fields" : {
"keyword" : {
"type" : "keyword",
"ignore_above" : 256
}
}
}
}
},
"source" : {
"type" : "keyword"
},
"subtags" : {
"type" : "keyword"
},
"tags" : {
"type" : "keyword"
},
"title" : {
"type" : "text",
"copy_to" : [
"combined"
],
"term_vector" : "with_positions_offsets",
"analyzer" : "keyword"
},
"users" : {
"type" : "keyword"
}
}
}
} |
|
Thanks. This looks entirely correct to me. I will try to do some tests with the latest ES |
|
This does not look like my mapping. The respective fields are of type keyword, whereas my fields are of type text and have the keyword subfield. |
|
Good spot @vbier ! Indeed mine looks the same and for example |
ArtificialOwl
approved these changes
Sep 20, 2023
|
/backport to stable27 |
|
/backport to stable26 |
|
Nice work and, well, thanks for your patience :-] |
I started a rebuild of the index (stop, delete, reset, index). After it was finished, it now started to rebuild again. Could be related to nextcloud/fulltextsearch#767 and nextcloud/fulltextsearch#723. I'll add my comments over there and add a new issue if required. Thanks for your help. |
|
I was now able to do a "quick" index with disabled tesseract. Mapping looks now like shown above #237 (comment) and test runs without issues. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The use of "keyword" will allow the search key to be exact, Elasticsearch will not perform any transformations. This use case will consider external user sources (LDAP), where the username may have dashes or symbols that could be misinterpreted by the engine.
Signed-off-by: Silvio [email protected]