-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Advanced search #24792
Advanced search #24792
Conversation
By analyzing the blame information on this pull request, we identified @PVince81, @LukasReschke and @tobiasKaminsky to be potential reviewers |
use OCP\Files\Folder; | ||
use OCP\IUserSession; | ||
use \OCP\Files\IRootFolder; | ||
use Zend_Search_Lucene_Search_Query_Boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm .... I don't like Zend search stuff being moved into core/files ....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I discussed this with @butonic, it's one of the most elaborated query language out there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it Matches our needs with respect to the Query requirements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But also the files app is the wrong location for this ....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIR It was decided that each app should implement its own search. An you were arguing in favor of it ... If you now want this in core I am all for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Frontend is files app yes. But the Backend needs to go via public APIs which should be webdav.
We removed webdav API from 9.1 so we go with the search Controller in files for now.
I m not happy with the introduction of the Query language Parser at this point because I have no understanding how this will Match with the webdav API and I don't want to break a feature in 9.2
Anyway .... Givn the time schedule i don't see this being merged into 9.1 anyhow ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while there is a webdav rfc 5323 for search it basically tell us to implement whatever we need. We could start with DAV:basicsearch but it does not even have full pagin support (it only knows limit
).
Since we have to define our own schema anyway we should just use the query objects that are in zend search lucene. Then we can at least already use a sophisticated query parser.
But let us take a step back. In the discussion with @schiesbn and @georgehrke it became clear that the web api should only take a string that is parsed on the server side. additional ui can be added to limit search to metadata by manipulating the string, eg add 'author:jfd'. Similar to how the github search fields work.
So the webdav api and our custom schema is protty small, in that it takes a string containing lucene search query as documented in http://lucene.apache.org/core/6_0_0/queryparser/org/apache/lucene/queryparser/classic/package-summary.html#Overview
On the server side we can parse that string back into an object tree, modify it and then query the indexes accordingly.
also see #12884 if you want to learn from the wrong approach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The original spec contained a webdav Query similar to caldav and carddav.
Giving any client (web, mobile, desktop, custom) to perform file based search.
This is not related to the webdav search spec which will not help as we agreed on in the past.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And from my pov the search query will not only be a string but hold additional criteria like mime type, file Age, sub folder and so on. Please have a look at the specification.
@MTRichards this could be a pebble to work on Cross Teams .... We should consider moving this to 9.2 and discuss with the client Teams ... |
Fixes #22741 ? |
* | ||
*/ | ||
|
||
namespace OCP\Search; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File can not be loaded, search folder needs to be uppercase
68472e8
to
cb035c6
Compare
@DeepDiver1975 Certainly a pebble, so we can look at this from all parts of the platform. Add it to #24684 ? |
We should add this for platform X |
WIP: add searchController Revert "checkout zend search lucene branch in 3rdparty submodule" This reverts commit e0c5955. further work on searchcontroller
cb035c6
to
246154a
Compare
@georgehrke any progress on this ? |
There is definitely progress, but I'm not sure if it's still feasible until Wednesday. Do you want me to continue or rather get the bug count low? |
After discussing with @DeepDiver1975 and @dragotin, moving this to 9.2. @georgehrke bugfixing it is, then |
we need to design this starting the webdav api and move forward ... |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Modifications for core to provide advanced search capabilities like Full Text Search