[tracker/sam/index-mount-points: 11/11] TEMP: Add IndexFileForProcess, and functional tests
- From: Sam Thursfield <sthursfield src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [tracker/sam/index-mount-points: 11/11] TEMP: Add IndexFileForProcess, and functional tests
- Date: Wed, 12 Aug 2015 16:35:25 +0000 (UTC)
commit 1218860effafce28d8e00e31ce7b921518ec0ba3
Author: Sam Thursfield <sam afuera me uk>
Date: Wed Aug 12 17:33:48 2015 +0100
TEMP: Add IndexFileForProcess, and functional tests
src/libtracker-miner/tracker-file-notifier.c | 5 +
src/libtracker-miner/tracker-miner-object.c | 2 +-
src/miners/fs/tracker-miner-files-index.c | 93 +++++++++++++---
tests/functional-tests/302-miner-on-demand.py | 121 ++++++++++++++++----
.../common/utils/configuration.py.in | 4 +
tests/functional-tests/common/utils/helpers.py | 11 ++-
tests/functional-tests/common/utils/minertest.py | 9 +-
tests/functional-tests/common/utils/options.py | 1 +
utils/sandbox/tracker-sandbox.py | 8 +-
9 files changed, 209 insertions(+), 45 deletions(-)
---
diff --git a/src/libtracker-miner/tracker-file-notifier.c b/src/libtracker-miner/tracker-file-notifier.c
index 49a0998..8b6213e 100644
--- a/src/libtracker-miner/tracker-file-notifier.c
+++ b/src/libtracker-miner/tracker-file-notifier.c
@@ -1361,6 +1361,11 @@ indexing_tree_directory_removed (TrackerIndexingTree *indexing_tree,
tracker_crawler_stop (priv->crawler);
g_cancellable_cancel (priv->cancellable);
+ /* At some point between the 'if' and here, priv->current_index_root is
+ * set to NULL! */
+ /* crawl_directories_start() or finish_current_directory() are the only
+ * place it could really be happening ...
+ */
root_data_free (priv->current_index_root);
priv->current_index_root = NULL;
diff --git a/src/libtracker-miner/tracker-miner-object.c b/src/libtracker-miner/tracker-miner-object.c
index 360ac7c..c6479fd 100644
--- a/src/libtracker-miner/tracker-miner-object.c
+++ b/src/libtracker-miner/tracker-miner-object.c
@@ -1000,7 +1000,7 @@ miner_pause_internal (TrackerMiner *miner,
}
if (calling_name) {
- g_message ("Watching process with name:'%s'", calling_name);
+ g_message ("Pause: Watching process with name:'%s'", calling_name);
watch_name_id = g_bus_watch_name (TRACKER_IPC_BUS,
calling_name,
G_BUS_NAME_WATCHER_FLAGS_NONE,
diff --git a/src/miners/fs/tracker-miner-files-index.c b/src/miners/fs/tracker-miner-files-index.c
index 9432382..a94ba27 100644
--- a/src/miners/fs/tracker-miner-files-index.c
+++ b/src/miners/fs/tracker-miner-files-index.c
@@ -58,6 +58,7 @@ typedef struct {
typedef struct {
TrackerMinerFS *miner;
GFile *file;
+ guint watch_id;
} IndexFileForProcessData;
typedef struct {
@@ -399,7 +400,7 @@ handle_method_call_index_file (TrackerMinerFilesIndex *miner,
#endif /* REQUIRE_LOCATION_IN_CONFIG */
if (is_dir) {
- tracker_miner_fs_check_directory (TRACKER_MINER_FS (priv->files_miner), file, do_checks);
+ tracker_miner_fs_check_directory (TRACKER_MINER_FS (priv->files_miner), file, do_checks,
"tracker-IndexFile");
} else {
tracker_miner_fs_check_file (TRACKER_MINER_FS (priv->files_miner), file, do_checks);
}
@@ -410,6 +411,35 @@ handle_method_call_index_file (TrackerMinerFilesIndex *miner,
g_object_unref (file);
}
+/* Since D-Bus provides an external interface for the miner process, we
+ * should be wary of rogue clients. The 'ownership' tracking in
+ * TrackerIndexingTree doesn't do any kind of sanity checking because it is
+ * internal API only. This function ensures that the application name on the
+ * bus is safe to use as an 'owner name' internally.
+ *
+ * In practice, the D-Bus server itself chooses the name and not the
+ * application, so this is unlikely to be a source of exploits anyway.
+ */
+static gboolean
+index_file_for_process_owner_name_is_safe (const char *owner_name) {
+ const int MAX_OWNER_NAME_LENGTH = 128;
+
+ /* This prevents attacks that cause Tracker to allocate lots of RAM. */
+ if (strlen (owner_name) > MAX_OWNER_NAME_LENGTH) {
+ g_warning ("In call to IndexFileForProcess: Owner name is longer than "
+ "%i characters. Ignoring request.", MAX_OWNER_NAME_LENGTH);
+ return FALSE;
+ }
+
+ if (g_ascii_strncasecmp (owner_name, "tracker", strlen("tracker")) == 0) {
+ g_warning ("In call to IndexFileForProcess: Owner name '%s' uses "
+ "reserved 'tracker' prefix. Ignoring request.", owner_name);
+ return FALSE;
+ }
+
+ return TRUE;
+}
+
static void
index_file_for_process_app_appears (GDBusConnection *connection,
const gchar *name,
@@ -421,6 +451,12 @@ index_file_for_process_app_appears (GDBusConnection *connection,
gboolean is_dir;
gboolean is_mount;
+ if (! index_file_for_process_owner_name_is_safe (name)) {
+ return;
+ }
+
+ g_debug ("Client app appeared: name %s, name owner %s", name, name_owner);
+
file_info = g_file_query_info (data->file,
G_FILE_ATTRIBUTE_STANDARD_TYPE,
G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS,
@@ -432,29 +468,48 @@ index_file_for_process_app_appears (GDBusConnection *connection,
if (is_mount) {
tracker_miner_fs_mount_add (TRACKER_MINER_FS (data->miner),
- g_file_find_enclosing_mount (data->file, NULL, NULL));
+ g_file_find_enclosing_mount (data->file, NULL, NULL),
+ name);
} else if (is_dir) {
tracker_miner_fs_check_directory (TRACKER_MINER_FS (data->miner),
data->file,
- FALSE);
+ FALSE,
+ name);
} else {
+ /* FIXME: there's no ownership tracking when we do this... but the
+ * parent directory might still be enqueued for monitoring, so it's
+ * a bit weird. Maybe this should be IndexDirectoryForProcess rather
+ * than IndexFileForProcess ... ?
+ */
tracker_miner_fs_check_file (TRACKER_MINER_FS (data->miner),
data->file,
FALSE);
}
-
- index_file_for_process_data_destroy (data);
}
static void
index_file_for_process_app_vanishes (GDBusConnection *connection,
- const gchar *name,
- gpointer user_data)
+ const gchar *name,
+ gpointer user_data)
{
IndexFileForProcessData *data = user_data;
- /* FIXME: What if the directory was already indexed? */
- tracker_miner_fs_directory_remove (TRACKER_MINER_FS (data->miner), data->file);
+ g_debug ("Client app vanished: name %s", name);
+
+ /* When the app vanishes and was the only owner of a directory, the
+ * directory is NOT removed from the store, but any indexing that was in
+ * process is cancelled, and all monitors are removed.
+ *
+ * If the directory being watched was a removable device, tracker:available
+ * will be set to false when the device disconnects.
+ */
+ tracker_miner_fs_ignore (TRACKER_MINER_FS (data->miner), data->file, name);
+
+ /* Manually disable the watch and free the state struct. The app will need
+ * to call IndexFileForProcess again if it crashed or something.
+ */
+ g_bus_unwatch_name (data->watch_id);
+ index_file_for_process_data_destroy (data);
}
/* The IndexFileForProcess D-Bus method does the same as IndexFile, except that
@@ -477,11 +532,16 @@ handle_method_call_index_file_for_process (TrackerMinerFilesIndex *miner,
priv = TRACKER_MINER_FILES_INDEX_GET_PRIVATE (miner);
+ g_warning("CHUNT");
+
g_variant_get (parameters, "(&s)", &file_uri);
tracker_gdbus_async_return_if_fail (file_uri != NULL, invocation);
request = tracker_g_dbus_request_begin (invocation, "%s(uri:'%s')", __FUNCTION__, file_uri);
+ /* FIXME: this seems to return before adding the watch, without returning
+ * an error ... need to debug, I guess.
+ */
file = g_file_new_for_uri (file_uri);
file_info = g_file_query_info (file,
G_FILE_ATTRIBUTE_STANDARD_TYPE,
@@ -510,13 +570,14 @@ handle_method_call_index_file_for_process (TrackerMinerFilesIndex *miner,
* index_file_for_process_app_appears(). This approach is race-free.
*/
sender = g_dbus_method_invocation_get_sender (invocation);
- g_bus_watch_name (G_BUS_TYPE_SESSION,
- sender,
- G_BUS_NAME_WATCHER_FLAGS_NONE,
- index_file_for_process_app_appears,
- index_file_for_process_app_vanishes,
- data,
- index_file_for_process_data_destroy);
+ g_message ("IndexFileForProcess: Watching process with name: '%s'", sender);
+ data->watch_id = g_bus_watch_name (G_BUS_TYPE_SESSION,
+ sender,
+ G_BUS_NAME_WATCHER_FLAGS_NONE,
+ index_file_for_process_app_appears,
+ index_file_for_process_app_vanishes,
+ data,
+ NULL);
tracker_dbus_request_end (request, NULL);
g_dbus_method_invocation_return_value (invocation, NULL);
diff --git a/tests/functional-tests/302-miner-on-demand.py b/tests/functional-tests/302-miner-on-demand.py
index 6f6e6af..be29e36 100755
--- a/tests/functional-tests/302-miner-on-demand.py
+++ b/tests/functional-tests/302-miner-on-demand.py
@@ -18,41 +18,84 @@
"""
Test on-demand indexing of locations.
-This feature exists so that applications can trigger the indexing of a
-removable device.
+This feature exists so that users can manually add files to the Tracker
+store, or trigger the indexing of a removable device.
-See: https://bugzilla.gnome.org/show_bug.cgi?id=680834
+Related bugs: https://bugzilla.gnome.org/show_bug.cgi?id=680834
"""
-from gi.repository import Gio
+from gi.repository import Gio, GLib
import time
import unittest2 as ut
from common.utils.helpers import log
-from common.utils.minertest import CommonTrackerMinerTest, uri
+from common.utils.minertest import CommonTrackerMinerTest, DEFAULT_TEXT, path, uri
class MinerOnDemandIndexingTest (CommonTrackerMinerTest):
# def test_01_index_file (self):
# """
-# Indexing a file outside the configured indexing locations.
+# Indexing and monitoring a file outside the configured locations.
#
# This can also be done from the commandline with `tracker index FILE`.
# """
#
+# store = self.system.store
+#
# # This is created by CommonTrackerMinerTest.setup() from
# # common.utils.minertest module.
# unmonitored_file = 'test-no-monitored/file0.txt'
-#
# self.assertFileMissing(uri(unmonitored_file))
#
# log("Queuing %s for indexing" % uri(unmonitored_file))
# self.system.miner_fs.index_iface.IndexFile(uri(unmonitored_file))
-# self.system.store.await_resource_inserted('nfo:TextDocument',
-# url=uri(unmonitored_file))
+# resource_id, resource_urn = store.await_resource_inserted(
+# 'nfo:TextDocument',
+# url=uri(unmonitored_file),
+# required_property='nie:plainTextContent')
+# self.assertFileContents(resource_urn, DEFAULT_TEXT)
+#
+# # When you pass a file to IndexFile, Tracker doesn't set up a monitor
+# # so changes to the file are ignored. This is a bit inconsistent
+# # compared to how directories are treated. The commented code will
+# # not work, because of that.
+#
+# #with open(path(unmonitored_file), 'w') as f:
+# # f.write('Version 2.0')
+# #store.await_property_changed(resource_id, 'nie:plainTextContent')
+# #self.assertFileContents(resource_urn, 'Version 2.0')
+#
+# def test_02_index_directory (self):
+# """
+# Indexing and monitoring a directory outside the configured locations.
#
- def test_02_index_file_for_process(self):
+# This can also be done from the commandline with `tracker index DIR`.
+# """
+#
+# store = self.system.store
+#
+# # These are created by CommonTrackerMinerTest.setup() from
+# # common.utils.minertest module.
+# unmonitored_dir = 'test-no-monitored'
+# unmonitored_file = 'test-no-monitored/file0.txt'
+# self.assertFileMissing(uri(unmonitored_dir))
+# self.assertFileMissing(uri(unmonitored_file))
+#
+# log("Queuing %s for indexing" % uri(unmonitored_dir))
+# self.system.miner_fs.index_iface.IndexFile(uri(unmonitored_dir))
+# resource_id, resource_urn = store.await_resource_inserted(
+# 'nfo:TextDocument',
+# url=uri(unmonitored_file),
+# required_property='nie:plainTextContent')
+# self.assertFileContents(resource_urn, DEFAULT_TEXT)
+#
+# with open(path(unmonitored_file), 'w') as f:
+# f.write('Version 2.0')
+# store.await_property_changed(resource_id, 'nie:plainTextContent')
+# self.assertFileContents(resource_urn, 'Version 2.0')
+
+ def test_04_index_directory_for_process(self):
"""
Indexing a directory tree for a specific D-Bus name.
@@ -60,29 +103,65 @@ class MinerOnDemandIndexingTest (CommonTrackerMinerTest):
that indexing of large removable devices can be tied to the lifetime of
certain applications that let users 'opt in' to indexing a device.
"""
+
+ miner = self.system.miner_fs
+
+ unmonitored_dir = 'test-no-monitored/'
unmonitored_file = 'test-no-monitored/file0.txt'
+ self.assertFileMissing(uri(unmonitored_dir))
self.assertFileMissing(uri(unmonitored_file))
- miner = self.system.miner_fs
+ # Open a separate connection to the session bus, so we can simulate the
+ # process ending again.
+ address = Gio.dbus_address_get_for_bus_sync(Gio.BusType.SESSION, None)
+ fake_app = Gio.DBusConnection.new_for_address_sync(
+ address,
+ Gio.DBusConnectionFlags.MESSAGE_BUS_CONNECTION |
+ Gio.DBusConnectionFlags.AUTHENTICATION_CLIENT,
+ None, None)
+ fake_app.init()
- fake_app = Gio.bus_get_sync(Gio.BusType.SESSION)
log("Opened D-Bus connection %s" % fake_app.get_unique_name())
+ # FIXME: there must be a better way of using GDBus in Python than
+ # this...
+
# Index a file for the fake_app process, but then close the fake_app
# straight away so the file doesn't get indexed. We do this while the
# miner is paused, because otherwise the file might get indexed before
# the app disappears, which would cause a spurious test failure.
- cookie = miner.miner_fs.PauseForProcess(
- fake_app.get_unique_name(),
- "Avoid test process racing with miner process.")
- miner.index_iface.IndexFile(uri(unmonitored_file))
- fake_app.close()
- log("Closed temporary D-Bus connection.")
-
+ DBUS_TIMEOUT = 5
+ pause_for_process_args = GLib.Variant('(ss)', (fake_app.get_unique_name(),
+ "Avoid test process racing with miner process."))
+ cookie = fake_app.call_sync(
+ 'org.freedesktop.Tracker1.Miner.Files',
+ '/org/freedesktop/Tracker1/Miner/Files',
+ 'org.freedesktop.Tracker1.Miner',
+ 'PauseForProcess',
+ pause_for_process_args, None, 0, DBUS_TIMEOUT, None)
+
+ index_file_for_process_args = GLib.Variant('(s)', (uri(unmonitored_dir),))
+ fake_app.call_sync(
+ 'org.freedesktop.Tracker1.Miner.Files.Index',
+ '/org/freedesktop/Tracker1/Miner/Files/Index',
+ 'org.freedesktop.Tracker1.Miner.Files.Index',
+ 'IndexFileForProcess',
+ index_file_for_process_args, None, 0, DBUS_TIMEOUT, None)
+
+ # This will cause Tracker to resume mining, and also stop indexing the
+ # file, possibly.
+ log("Closing temporary D-Bus connection.")
+ fake_app.close_sync()
+
+ bus2 = Gio.bus_get_sync(Gio.BusType.SESSION)
+ print bus2.call_sync('org.freedesktop.DBus', '/', 'org.freedesktop.DBus',
+ 'ListNames', None, None, 0, 1, None)
+
+ # We don't have to unpause the miner, it gets resumed
# The file should never get indexed, because the process disappeared.
- miner.miner_fs.Resume(cookie)
- time.sleep(5)
+ #miner.miner_fs.Resume(cookie)
#self.assertFileMissing(uri(unmonitored_file))
+ # Currently this passes, when it should actually fail!
self.system.store.await_resource_inserted('nfo:TextDocument',
url=uri(unmonitored_file))
self.assertFilePresent(uri(unmonitored_file))
diff --git a/tests/functional-tests/common/utils/configuration.py.in
b/tests/functional-tests/common/utils/configuration.py.in
index d6f5441..7eb886f 100644
--- a/tests/functional-tests/common/utils/configuration.py.in
+++ b/tests/functional-tests/common/utils/configuration.py.in
@@ -30,6 +30,10 @@ MINERFS_BUSNAME = "org.freedesktop.Tracker1.Miner.Files"
MINERFS_OBJ_PATH = "/org/freedesktop/Tracker1/Miner/Files"
MINER_IFACE = "org.freedesktop.Tracker1.Miner"
+MINERFS_INDEX_BUSNAME = "org.freedesktop.Tracker1.Miner.Files.Index"
+MINERFS_INDEX_OBJ_PATH = "/org/freedesktop/Tracker1/Miner/Files/Index"
+MINER_FILES_INDEX_IFACE = "org.freedesktop.Tracker1.Miner.Files.Index"
+
TRACKER_BACKUP_OBJ_PATH = "/org/freedesktop/Tracker1/Backup"
BACKUP_IFACE = "org.freedesktop.Tracker1.Backup"
diff --git a/tests/functional-tests/common/utils/helpers.py b/tests/functional-tests/common/utils/helpers.py
index 7b3ca88..4780ade 100644
--- a/tests/functional-tests/common/utils/helpers.py
+++ b/tests/functional-tests/common/utils/helpers.py
@@ -219,6 +219,10 @@ class Helper:
self.abort_if_process_exits_with_status_0 = False
def stop (self):
+ if options.is_manual_start():
+ log ("Manually stop %s" % self.PROCESS_NAME)
+ return
+
start = time.time()
if self.process.poll() == None:
# It should step out of this loop when the miner disappear from the bus
@@ -235,10 +239,15 @@ class Helper:
self.process.wait()
log ("[%s] stopped." % self.PROCESS_NAME)
+
# Disconnect the signals of the next start we get duplicated messages
self.bus._clean_up_signal_match (self.name_owner_match)
def kill (self):
+ if options.is_manual_start():
+ log ("Manually kill %s" % self.PROCESS_NAME)
+ return
+
self.process.kill ()
# Name owner changed callback should take us out from this loop
@@ -653,7 +662,7 @@ class MinerFsHelper (Helper):
index_bus_object = self.bus.get_object (cfg.MINERFS_INDEX_BUSNAME,
cfg.MINERFS_INDEX_OBJ_PATH)
self.index_iface = dbus.Interface (index_bus_object,
- dbus_interface = cfg.MINER_INDEX_IFACE)
+ dbus_interface = cfg.MINER_FILES_INDEX_IFACE)
def stop (self):
Helper.stop (self)
diff --git a/tests/functional-tests/common/utils/minertest.py
b/tests/functional-tests/common/utils/minertest.py
index 1aa63ac..03aac88 100644
--- a/tests/functional-tests/common/utils/minertest.py
+++ b/tests/functional-tests/common/utils/minertest.py
@@ -45,13 +45,13 @@ CONF_OPTIONS = {
'index-single-directories': GLib.Variant.new_strv([]),
'index-optical-discs': GLib.Variant.new_boolean(False),
'index-removable-devices': GLib.Variant.new_boolean(False),
- 'throttle': GLib.Variant.new_int32(5),
+ 'throttle': GLib.Variant.new_int32(0),
+ 'verbosity': GLib.Variant.new_string('detailed'),
}
}
class CommonTrackerMinerTest (ut.TestCase):
-
def prepare_directories (self):
#
# ~/test-monitored/
@@ -130,3 +130,8 @@ class CommonTrackerMinerTest (ut.TestCase):
if self.tracker.ask (query) == True:
self.fail ("File <%s> should not be present in the database" %
file_url)
+
+ def assertFileContents(self, file_urn, expected_contents):
+ query = 'SELECT ?content { <%s> nie:plainTextContent ?content }' % file_urn
+ result = self.tracker.query(query)
+ self.assertEqual(result[0][0], expected_contents)
diff --git a/tests/functional-tests/common/utils/options.py b/tests/functional-tests/common/utils/options.py
index 6bc8379..4537e2d 100644
--- a/tests/functional-tests/common/utils/options.py
+++ b/tests/functional-tests/common/utils/options.py
@@ -35,4 +35,5 @@ def is_manual_start ():
"""
False to start the processes automatically
"""
+ return True
return options.startmanually
diff --git a/utils/sandbox/tracker-sandbox.py b/utils/sandbox/tracker-sandbox.py
index 42d0e33..1e51e5f 100755
--- a/utils/sandbox/tracker-sandbox.py
+++ b/utils/sandbox/tracker-sandbox.py
@@ -320,7 +320,7 @@ def environment_set():
os.environ['TRACKER_LANGUAGE_STOPWORDS_DIR'] = os.path.join(opts.prefix, 'share', 'tracker',
'stop-words')
# Preferences
- os.environ['TRACKER_USE_CONFIG_FILES'] = 'yes'
+ ########os.environ['TRACKER_USE_CONFIG_FILES'] = 'yes'
#if opts.debug:
# os.environ['TRACKER_USE_LOG_FILES'] = 'yes'
@@ -329,8 +329,8 @@ def environment_set():
os.environ['G_MESSAGES_DEBUG'] = 'all'
os.environ['TRACKER_VERBOSITY'] = '%d' % default_debug_verbosity
os.environ['DBUS_VERBOSE'] = '1'
- else:
- os.environ['TRACKER_VERBOSITY'] = '0'
+ #else:
+ #os.environ['TRACKER_VERBOSITY'] = '0'
debug('Using prefix location "%s"' % opts.prefix)
debug('Using index location "%s"' % index_location_abs)
@@ -491,7 +491,7 @@ if __name__ == "__main__":
# Set up environment variables and foo needed to get started.
environment_set()
- config_set()
+ #config_set()
try:
if opts.update:
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
