Merge lp:~mhr3/unity-lens-files/with-locate into lp:unity-lens-files

[Michal Hruby]

Use locate to make sure we're able to find files which aren't logged by Zeitgeist. Also adds a gsettings schema where this can be easily disabled.

UNBLOCK

[Mikkel Kamstrup Erlandsen (kamstrup)]

175 + /* We want to display something once dash is shown */
176 + scope.queue_search_changed (SearchType.GLOBAL);

I think this deserves a branch and bug report on its own.

240 + if (cancellable.is_cancelled ()) throw new IOError.CANCELLED ("Cancelled");

Can you add a more useful message, like "Cancelled locate search for '%s'" or something?

242 + var results = yield Locate.locate (query, cancellable);
243 + results.sort (Utils.cmp_file_info_by_mtime);

Should we not be checking cancellable before proceeding after the async call?

Otherwise, awesome work! It works quite well here on my sandybridge lappy. I wanna test this on my elcheapo netbook before I give it the final ack.

[lp:~mhr3/unity-lens-files/with-locate]

214. By Michal Hruby on 2012-03-02

Simplify cancellable usage

215. By Michal Hruby on 2012-03-02

Remove an unrelated fix, deserves its own branch

[Michal Hruby (mhr3)]

> 175 + /* We want to display something once dash is shown */
> 176 + scope.queue_search_changed (SearchType.GLOBAL);
>
> I think this deserves a branch and bug report on its own.

Fair enough. Removing from this branch.

>
>
> 240 + if (cancellable.is_cancelled ()) throw new IOError.CANCELLED
> ("Cancelled");
>
> Can you add a more useful message, like "Cancelled locate search for '%s'" or
> something?
>

Actually I should be just using cancellable.set_error_if_cancelled()

Moreover, daemon.vala:428:
      } catch (IOError.CANCELLED ioe) {
        return;

>
> 242 + var results = yield Locate.locate (query, cancellable);
> 243 + results.sort (Utils.cmp_file_info_by_mtime);
>
> Should we not be checking cancellable before proceeding after the async call?
>

No, an IOError is thrown in that case.

[Mikkel Kamstrup Erlandsen (kamstrup)]

> > 242 + var results = yield Locate.locate (query, cancellable);
> > 243 + results.sort (Utils.cmp_file_info_by_mtime);
> >
> > Should we not be checking cancellable before proceeding after the async
> call?
> >
>
> No, an IOError is thrown in that case.

Not necessarily. If I understand GIO correctly it will raise G_IO_ERROR_CANCELLED if an async io call is cancelled. The problem is that you may also be cancelled when not in an IO call. In between the idles implied by the yields. Or...I have no idea how Vala async semantics intersects with exception handling...

[Mikkel Kamstrup Erlandsen (kamstrup)]

I tested it out on my netbook and it was pretty smooth.

During testing I still couldn't help getting a creepy feeling that I was seeing an alarmingly low signal to noise ratio in the results. This is not a problem for people who can immediately recognize that they are seeing a bunch of system files, but for mortal users this must be most disconcerting.Doubly so because we have no contextual information on all of these system files.

I have a feeling that we should blacklist /usr /bin /lib /opt and /var.

The results of this tool on a multi user system is also pretty angst provoking...

[Michal Hruby (mhr3)]

> Not necessarily. If I understand GIO correctly it will raise G_IO_ERROR_CANCELLED if an ...

Sorted out on IRC.

> I tested it out on my netbook and it was pretty smooth.
>
> During testing I still couldn't help getting a creepy feeling that I was
> seeing an alarmingly low signal to noise ratio in the results. This is not a
> problem for people who can immediately recognize that they are seeing a bunch
> of system files, but for mortal users this must be most disconcerting.Doubly
> so because we have no contextual information on all of these system files.
>
> I have a feeling that we should blacklist /usr /bin /lib /opt and /var.
>
> The results of this tool on a multi user system is also pretty angst
> provoking...

Oh I remember we talked about filtering out of non-writable files, this branch isn't doing that yet, does anyone see any problems with adding it? (unfortunately it'll have to be post-locate filtering)

[Mikkel Kamstrup Erlandsen (kamstrup)]

> > I have a feeling that we should blacklist /usr /bin /lib /opt and /var.
> >
> > The results of this tool on a multi user system is also pretty angst
> > provoking...
>
> Oh I remember we talked about filtering out of non-writable files, this branch
> isn't doing that yet, does anyone see any problems with adding it?
> (unfortunately it'll have to be post-locate filtering)

You're right forgot about that trick. I like that a lot better.

[Sebastien Bacher (seb128)]

couldn't you just limit the results to the user and /media,mnt? That would solve the "too much noise" and "other user files are listed"...

[Sebastien Bacher (seb128)]

to the user "directory"...

[lp:~mhr3/unity-lens-files/with-locate]

216. By Michal Hruby on 2012-03-02

Filter out backup files and files that are not writable

[Michal Hruby (mhr3)]

Pushed a version which filters out non-writable files, limiting the search to a couple of directories required using --regex for locate and that slowed it down far too much (up to 8 seconds for some queries on my system)

[Mikkel Kamstrup Erlandsen (kamstrup)]

On 2 March 2012 17:12, Michal Hruby <email address hidden> wrote:
> Pushed a version which filters out non-writable files, limiting the search to a couple of directories required using --regex for locate and that slowed it down far too much (up to 8 seconds for some queries on my system)

I am seeing some confusing matching... Searching for 'config' gives me
6 hits while searching for 'config.' gives me 81 hits... Do you know
what gives?

Also, this kind of spurious matching behaviour will inevitably pop out
here and there, but that is a price we'll have to agree to pay...

[lp:~mhr3/unity-lens-files/with-locate]

217. By Michal Hruby on 2012-03-08

Add manual test for locate scope

218. By Michal Hruby on 2012-03-08

Make sure the manual-test performer uses touch (and didrocks will live happily ever after)

[Didier Roche-Tolomelli (didrocks)]

Ok, Michal ensured me that the technical concerns from Mikkel have been addressed. Now that there is a manual test for it (maybe some autopilot one day if we can run some command as root?) that's good for me (and it had a few days of testing already in the ubuntu-desktop ppa).

Nice to see a fix for that, well done :)

review: Approve

=== modified file '.bzrignore'
--- .bzrignore	2011-08-07 09:01:34 +0000
+++ .bzrignore	2012-03-08 16:33:30 +0000
@@ -1,6 +1,7 @@
 src/config.c
 src/daemon.c
 src/folder.c
+src/locate.c
 src/main.c
 src/schemas.c
 src/url-checker.c
@@ -43,6 +44,9 @@
 data/Makefile
 data/Makefile.in
 data/unity-lens-files.service
+data/com.canonical.Unity.FilesLens.gschema.valid
+data/com.canonical.Unity.FilesLens.gschema.xml
+data/com.canonical.Unity.FilesLens.gschema.xml.in
 src/Makefile
 src/Makefile.in
 src/config.vala

=== modified file 'configure.ac'
--- configure.ac	2012-02-17 10:43:57 +0000
+++ configure.ac	2012-03-08 16:33:30 +0000
@@ -100,6 +100,12 @@
 fi
 AC_SUBST(DBUSSERVICEDIR)
 
+#############################################
+# GSettings macros
+#############################################
+
+GLIB_GSETTINGS
+
 #####################################################
 # Look for correct Lenses dir
 #####################################################
@@ -117,6 +123,7 @@
   Makefile
   files.lens.in
   data/Makefile
+  data/com.canonical.Unity.FilesLens.gschema.xml.in
   src/Makefile
   src/config.vala
   po/Makefile.in

=== modified file 'data/Makefile.am'
--- data/Makefile.am	2011-08-04 14:15:30 +0000
+++ data/Makefile.am	2012-03-08 16:33:30 +0000
@@ -5,8 +5,20 @@
 %.service: %.service.in
 	sed -e "s|\@libexecdir\@|$(libexecdir)|" $< > $@
 
+#############################################################
+# GSettings schemas
+#############################################################
+gsettings_SCHEMAS = com.canonical.Unity.FilesLens.gschema.xml
+
+@INTLTOOL_XML_NOMERGE_RULE@
+
+@GSETTINGS_RULES@
+
 EXTRA_DIST = \
+  com.canonical.Unity.FilesLens.gschema.xml.in.in \
   $(service_in_files)
 
 CLEANFILES = \
-  unity-lens-files.service
+  unity-lens-files.service \
+  $(gsettings_SCHEMAS)
+

=== added file 'data/com.canonical.Unity.FilesLens.gschema.xml.in.in'
--- data/com.canonical.Unity.FilesLens.gschema.xml.in.in	1970-01-01 00:00:00 +0000
+++ data/com.canonical.Unity.FilesLens.gschema.xml.in.in	2012-03-08 16:33:30 +0000
@@ -0,0 +1,9 @@
+<schemalist>
+  <schema path="/desktop/unity/lenses/files/" id="com.canonical.Unity.FilesLens" gettext-domain="unity-lens-files">
+    <key type="b" name="use-locate">
+      <default>true</default>
+      <summary>Use locate during searches.</summary>
+      <description>Use locate during searches to make sure the lens is able to find most of their files even it they're not logged by Zeitgeist.</description>
+    </key>
+  </schema>
+</schemalist>

=== modified file 'src/Makefile.am'
--- src/Makefile.am	2011-09-29 10:19:09 +0000
+++ src/Makefile.am	2012-03-08 16:33:30 +0000
@@ -39,6 +39,7 @@
   config.vala \
   daemon.vala \
   folder.vala \
+  locate.vala \
   main.vala \
   schemas.vala \
   url-checker.vala \

=== modified file 'src/daemon.vala'
--- src/daemon.vala	2012-02-14 13:44:37 +0000
+++ src/daemon.vala	2012-03-08 16:33:30 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2010 Canonical Ltd
+ * Copyright (C) 2010-2012 Canonical Ltd
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
@@ -14,6 +14,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  * Authored by Mikkel Kamstrup Erlandsen <[e-mail]>
+ *             Michal Hruby <[e-mail]>
  *
  */
 using Zeitgeist;
@@ -41,9 +42,22 @@
      * we use to query Zeitgeist */
     private HashTable<string, Event> type_templates;
 
+    private const string SCHEMA_NAME = "com.canonical.Unity.FilesLens";
+    private const string USE_LOCATE_KEY = "use-locate";
+    public bool use_locate { get; set; default = true; }
+
+    private Settings lens_settings;
+
     construct
     {
-      prepare_type_templates();
+      prepare_type_templates ();
+
+      if (SCHEMA_NAME in Settings.list_schemas ())
+      {
+        lens_settings = new Settings (SCHEMA_NAME);
+        lens_settings.bind (USE_LOCATE_KEY, this, USE_LOCATE_KEY,
+                            SettingsBindFlags.GET);
+      }
 
       scope = new Unity.Scope ("/com/canonical/unity/scope/files");
       scope.search_in_global = true;
@@ -54,15 +68,15 @@
       lens.search_hint = _("Search Files & Folders");
       lens.visible = true;
       populate_categories ();
-      populate_filters();
+      populate_filters ();
       lens.add_local_scope (scope);
       
       /* Bring up Zeitgeist interfaces */
-      log = new Zeitgeist.Log();
-      index = new Zeitgeist.Index();
+      log = new Zeitgeist.Log ();
+      index = new Zeitgeist.Index ();
       
       /* Listen for all file:// related events from Zeitgeist */
-      var templates = new PtrArray();
+      var templates = new PtrArray ();
       var event = new Zeitgeist.Event ();
       var subject = new Zeitgeist.Subject ();
       subject.set_uri ("file://*");
@@ -73,7 +87,7 @@
       monitor.events_inserted.connect (on_zeitgeist_changed);
       monitor.events_deleted.connect (on_zeitgeist_changed);
       log.install_monitor (monitor);
-      
+
       bookmarks = new Bookmarks ();
       urls = new UrlChecker ();
 
@@ -400,6 +414,14 @@
                                         search.search_string);
         }
 
+        if (has_search && use_locate)
+        {
+          yield perform_locate (500, search.search_string,
+                                search.results_model, cancellable,
+                                Categories.FILES_AND_FOLDERS,
+                                Categories.FILES_AND_FOLDERS);
+        }
+
       } catch (IOError.CANCELLED ioe) {
         return;
       } catch (GLib.Error e) {
@@ -504,6 +526,12 @@
           if (!txn.is_committed ()) txn.commit ();
         }
 
+        if (has_search && use_locate)
+        {
+          yield perform_locate (500, search.search_string,
+                                search.results_model, cancellable);
+        }
+
       } catch (IOError.CANCELLED ioe) {
         return;
       } catch (GLib.Error e) {
@@ -710,9 +738,8 @@
         return;
       }
 
-      if (cancellable.is_cancelled ())
-        throw new IOError.CANCELLED ("Search was cancelled");
-      
+      cancellable.set_error_if_cancelled (); // throws IOError
+
       /* Sort files by mtime, we do an ugly nested ternary
        * to avoid potential long/int overflow */
       downloads.sort ((CompareFunc) Utils.cmp_file_info_by_mtime);
@@ -720,6 +747,7 @@
       var timerange = get_current_timerange ();
       int64 min_size, max_size;
       get_current_size_limits (out min_size, out max_size);
+      var types = get_current_types ();
       
       foreach (var info in downloads)
       {
@@ -734,7 +762,6 @@
           continue;
 
         // check if type matches
-        var types = get_current_types ();
         if (types != null && !Utils.file_info_matches_any (info, types))
           continue;
         
@@ -753,6 +780,68 @@
       }
     }
 
+    private async void perform_locate (uint timeout,
+                                       string query,
+                                       Dee.Model results_model,
+                                       Cancellable cancellable,
+                                       int files_category = Categories.RECENT,
+                                       int dirs_category = Categories.FOLDERS)
+      throws Error
+    {
+      /* Wait a while before starting the locate search */
+      Timeout.add_full (Priority.LOW, timeout, perform_locate.callback);
+      yield;
+      cancellable.set_error_if_cancelled (); // throws IOError
+
+      var results = yield Locate.locate (query, cancellable);
+      results.sort (Utils.cmp_file_info_by_mtime);
+
+      /* make a list of uris we already have */
+      var present_uris = new Gee.HashSet<string> ();
+      var model_iter = results_model.get_first_iter ();
+      var model_end_iter = results_model.get_last_iter ();
+      while (model_iter != model_end_iter)
+      {
+        present_uris.add (results_model.get_string (model_iter, 0));
+        model_iter = results_model.next (model_iter);
+      }
+
+      var timerange = get_current_timerange ();
+      int64 min_size, max_size;
+      get_current_size_limits (out min_size, out max_size);
+      var types = get_current_types ();
+
+      foreach (var info in results)
+      {
+        var uri = info.get_data<File> ("associated-gfile").get_uri ();
+        warn_if_fail (uri != null);
+        if (uri == null || uri in present_uris) continue;
+        var mimetype = info.get_attribute_string (FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE);
+        var icon_hint = Utils.check_icon_string (uri, mimetype, info);
+
+        // check if we match the timerange
+        uint64 mtime = info.get_attribute_uint64 (FILE_ATTRIBUTE_TIME_MODIFIED) * 1000;
+
+        if (mtime < timerange.get_start() || mtime > timerange.get_end ())
+          continue;
+
+        // check if type matches
+        if (types != null && !Utils.file_info_matches_any (info, types))
+          continue;
+
+        // check if size is within bounds
+        int64 size = info.get_size ();
+        if (size < min_size || size > max_size)
+          continue;
+
+        uint category_id = mimetype == "inode/directory" ?
+          dirs_category : files_category;
+
+        results_model.append (uri, icon_hint, category_id,
+                              mimetype, info.get_display_name (), uri);
+      }
+    }
+
     public Unity.ActivationResponse activate (string uri)
     {
       debug (@"Activating: $uri");

=== added file 'src/locate.vala'
--- src/locate.vala	1970-01-01 00:00:00 +0000
+++ src/locate.vala	2012-03-08 16:33:30 +0000
@@ -0,0 +1,82 @@
+/*
+ * Copyright (C) 2010-2012 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by Michal Hruby <[e-mail]>
+ *
+ */
+using Zeitgeist;
+using Config;
+using Gee;
+
+namespace Unity.FilesLens.Locate
+{
+  const uint MAX_RESULTS = 128;
+
+  /* Careful about usage of FILE_ATTRIBUTE_STANDARD_ICON, this is the same as
+   * requesting FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE and causes reading
+   * of the target file, we just want to stat it */
+  const string ATTRIBS = FILE_ATTRIBUTE_STANDARD_IS_HIDDEN + "," +
+      FILE_ATTRIBUTE_STANDARD_IS_BACKUP + "," +
+      FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME + "," +
+      FILE_ATTRIBUTE_STANDARD_SIZE + "," +
+      FILE_ATTRIBUTE_TIME_MODIFIED + "," +
+      FILE_ATTRIBUTE_THUMBNAIL_PATH + "," +
+      FILE_ATTRIBUTE_ACCESS_CAN_WRITE + "," +
+      FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE;
+
+  public async GLib.SList<FileInfo> locate (string input_query,
+                                            Cancellable cancellable) throws Error
+  {
+    var result = new GLib.SList<FileInfo> ();
+    var query = Regex.escape_string (input_query.strip ());
+
+    // FIXME: we could limit the search to specific directories, but using regex
+    //  matching slows down the search considerably
+    string[] argv = { "locate", "-i", "-e", "-l", "%u".printf (MAX_RESULTS),
+                      "*%s*".printf (query.replace (" ", "*")) };
+
+    Pid pid;
+    int read_fd;
+    string? line = null;
+
+    Process.spawn_async_with_pipes (null, argv, null, SpawnFlags.SEARCH_PATH,
+                                    null, out pid, null, out read_fd);
+
+    UnixInputStream read_stream = new UnixInputStream (read_fd, true);
+    DataInputStream locate_output = new DataInputStream (read_stream);
+
+    do
+    {
+      line = yield locate_output.read_line_async (Priority.DEFAULT_IDLE,
+                                                  cancellable);
+      if (line == null) break;
+      if ("/." in line) continue; // we don't want no hidden files
+
+      var file = File.new_for_path (line);
+      var fi = yield file.query_info_async (ATTRIBS, 0, Priority.DEFAULT_IDLE,
+                                            cancellable);
+      if (fi.get_is_hidden () || fi.get_is_backup () ||
+          !fi.get_attribute_boolean (FILE_ATTRIBUTE_ACCESS_CAN_WRITE)) continue;
+      fi.set_data ("associated-gfile", file);
+      result.prepend (fi);
+
+    } while (line != null);
+
+    result.reverse ();
+    return result;
+  }
+
+} /* namespace */
+

=== modified file 'src/utils.vala'
--- src/utils.vala	2012-01-27 17:14:56 +0000
+++ src/utils.vala	2012-03-08 16:33:30 +0000
@@ -85,23 +85,25 @@
 
     public string check_icon_string (string uri, string mimetype, FileInfo info)
     {
-      Icon icon = info.get_icon();
+      Icon icon = info.get_icon ();
       string thumbnail_path = info.get_attribute_byte_string (FILE_ATTRIBUTE_THUMBNAIL_PATH);
 
-      if (thumbnail_path == null)
-        return icon.to_string();
+      if (thumbnail_path == null && icon != null)
+        return icon.to_string ();
+      else if (icon == null)
+        return ContentType.get_icon (mimetype).to_string ();
       else
-        return new FileIcon(File.new_for_path (thumbnail_path)).to_string ();
+        return new FileIcon (File.new_for_path (thumbnail_path)).to_string ();
     }
     
-  public int cmp_file_info_by_mtime (FileInfo info1, FileInfo info2)
-  {
-    TimeVal tv1, tv2;
-    info1.get_modification_time (out tv1);
-    info2.get_modification_time (out tv2);
-    long cmp = tv1.tv_sec - tv2.tv_sec;
-    return cmp < 0 ? 1 : ( cmp > 0 ? -1 : 0);
-  }
+    public int cmp_file_info_by_mtime (FileInfo info1, FileInfo info2)
+    {
+      TimeVal tv1, tv2;
+      info1.get_modification_time (out tv1);
+      info2.get_modification_time (out tv2);
+      long cmp = tv1.tv_sec - tv2.tv_sec;
+      return cmp < 0 ? 1 : ( cmp > 0 ? -1 : 0);
+    }
 
     public string get_month_name (DateMonth month)
     {
@@ -333,10 +335,15 @@
 
     public bool file_info_matches_type (FileInfo info, string type_id)
     {
+      unowned string interpretation;
       unowned string content_type = info.get_content_type ();
-      unowned string interpretation;
+      if (content_type == null)
+        content_type = info.get_attribute_string (FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE);
+
+      if (content_type == null) content_type = "application/octet-stream";
 
       interpretation = Zeitgeist.interpretation_for_mimetype (content_type);
+      if (interpretation == null) return type_id == "other" || type_id == "all";
 
       /* type_id is one of: documents, folders, images, audio, videos,
        *   presentations, other */

=== added directory 'tests'
=== added directory 'tests/manual'
=== added file 'tests/manual/locate.txt'
--- tests/manual/locate.txt	1970-01-01 00:00:00 +0000
+++ tests/manual/locate.txt	2012-03-08 16:33:30 +0000
@@ -0,0 +1,11 @@
+Locate scope
+============
+Tests that the locate scope works.
+
+#. Create a file named "report.txt" in your home directory (`touch report.txt`).
+#. Run updatedb (`sudo updatedb`).
+#. Search for this file with files lens.
+
+Outcome
+  The "report.txt" should show up as a result in the Recent category of files lens.
+