Review Request 129394: [filenamesearch] Fix huge ram usage in kded module

[David Faure]

> On Nov. 21, 2016, 8:34 a.m., David Faure wrote:
> > filenamesearch/kded/filenamesearchmodule.cpp, line 84
> > <https://git.reviewboard.kde.org/r/129394/diff/1/?file=485431#file485431line84>
> >
> >     Well, if dirUrl looks like "filenamesearch:?search=file&url=file:///path/to/file" then dirUrl.path() is empty, and this code is incorrect (it should use the query item "url", not the path). What am I missing?
> 
> Anthony Fieroni wrote:
>     This is a big misunderstanding mainly by me. Emitted url should contains query with new path ?
>     for (const QString &file : files) {
>             const QUrl url(file);
>             if (!url.isLocalFile()) {
>                 continue;
>             }
>             const QString urlPath = url.path();
>             for (const QUrl &dirUrl : m_searchUrls) {
>                 QUrlQuery urlQuery(dirUrl);
>                 QString str = urlQuery.queryItemValue(QStringLiteral("url"));
>                 if (urlPath.startsWith(QUrl(str).path())) {
>                     QUrl temp;
>                     temp.setScheme(QStringLiteral("filenamesearch"));
>                     urlQuery.removeQueryItem(QStringLiteral("url");
>                     urlQuery.addQueryItem(QStringLiteral('url"), url);
>                     temp.setQuery(urlQuery);
>                     fileList << temp;
>                 }
>             }
>         }

Maybe, but I'm still in the dark about something. How can KDirLister cope with listing such URLs? It wants a directory URL and files inside that directory. Such a filenamesearch URL doesn't look like it's a file inside a directory, in terms of paths. Ideally I would look into the code to understand what is being done but I'm short on time.

Does kio_filenamesearch really return items from listDir(), which have an empty path too, just like the listed directory? I would assume this breaks many things in KDirLister.

Please clarify with the dolphin people (or whoever wrote the filenamesearch KIO) about the URL structure, then it will be straightforward to do the URL conversions in this code.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/129394/#review101007
-----------------------------------------------------------


On Nov. 14, 2016, 11:44 a.m., Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129394/
> -----------------------------------------------------------
> 
> Review request for KDE Frameworks, Anthony Fieroni, David Faure, and Emmanuel Pescosta.
> 
> 
> Repository: kio-extras
> 
> 
> Description
> -------
> 
> Bug is introduced in https://git.reviewboard.kde.org/r/129297/
> When is fixed new kio-extras realease is needed for 16.08 branch.
> 
> 
> Diffs
> -----
> 
>   filenamesearch/kded/filenamesearchmodule.cpp 3f9f582 
> 
> Diff: https://git.reviewboard.kde.org/r/129394/diff/
> 
> 
> Testing
> -------
> 
> No big ram usage but still not works as expected.
> 1. Perform search in Dolphin
> 2. Delete one result item
> 3. View must be update, but it's not
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20161121/02fc28e6/attachment.html>