Merge branch 'master' of git://git.sugarlabs.org/sugar/mainline

8 files changed, 309 insertions, 131 deletions

diff --git a/src/jarabe/journal/detailview.py b/src/jarabe/journal/detailview.py
index 363e152..47fdb1f 100644
--- a/src/jarabe/journal/detailview.py
+++ b/src/jarabe/journal/detailview.py
@@ -66,11 +66,6 @@ class DetailView(gtk.VBox):
     def _update_view(self):
         if self._expanded_entry:
             self._root.remove(self._expanded_entry)
-
-            # Work around pygobject bug #479227
-            self._expanded_entry.remove_all()
-            import gc
-            gc.collect()
         self._expanded_entry = ExpandedEntry(self._metadata)
         self._root.append(self._expanded_entry, hippo.PACK_EXPAND)
 
diff --git a/src/jarabe/journal/expandedentry.py b/src/jarabe/journal/expandedentry.py
index 5b0d0f4..f57b89c 100644
--- a/src/jarabe/journal/expandedentry.py
+++ b/src/jarabe/journal/expandedentry.py
@@ -351,8 +351,7 @@ class ExpandedEntry(hippo.CanvasBox):
         self._update_title_sid = None
  
     def get_keep(self):
-        return self._metadata.has_key('keep') and \
-               self._metadata['keep'] == 1
+        return int(self._metadata.get('keep', 0)) == 1
 
     def _keep_icon_activated_cb(self, keep_icon):
         if self.get_keep():
diff --git a/src/jarabe/journal/journalactivity.py b/src/jarabe/journal/journalactivity.py
index a1fd269..3da689b 100644
--- a/src/jarabe/journal/journalactivity.py
+++ b/src/jarabe/journal/journalactivity.py
@@ -79,14 +79,14 @@ class JournalActivityDBusService(dbus.service.Object):
         chooser.destroy()
         del chooser
 
-    @dbus.service.method(J_DBUS_INTERFACE, in_signature='i', out_signature='s')
-    def ChooseObject(self, parent_xid):
+    @dbus.service.method(J_DBUS_INTERFACE, in_signature='is', out_signature='s')
+    def ChooseObject(self, parent_xid, what_filter=''):
         chooser_id = uuid.uuid4().hex
         if parent_xid > 0:
             parent = gtk.gdk.window_foreign_new(parent_xid)
         else:
             parent = None
-        chooser = ObjectChooser(parent)
+        chooser = ObjectChooser(parent, what_filter)
         chooser.connect('response', self._chooser_response_cb, chooser_id)
         chooser.show()
 
diff --git a/src/jarabe/journal/journaltoolbox.py b/src/jarabe/journal/journaltoolbox.py
index ce20d6b..4ab925a 100644
--- a/src/jarabe/journal/journaltoolbox.py
+++ b/src/jarabe/journal/journaltoolbox.py
@@ -19,6 +19,7 @@ import logging
 from datetime import datetime, timedelta
 import os
 import gconf
+import time
 
 import gobject
 import gio
@@ -27,6 +28,7 @@ import gtk
 from sugar.graphics.toolbox import Toolbox
 from sugar.graphics.toolcombobox import ToolComboBox
 from sugar.graphics.toolbutton import ToolButton
+from sugar.graphics.toggletoolbutton import ToggleToolButton
 from sugar.graphics.combobox import ComboBox
 from sugar.graphics.menuitem import MenuItem
 from sugar.graphics.icon import Icon
@@ -62,10 +64,6 @@ class MainToolbox(Toolbox):
         self.search_toolbar.set_size_request(-1, style.GRID_CELL_SIZE)
         self.add_toolbar(_('Search'), self.search_toolbar)
         self.search_toolbar.show()
-        
-        #self.manage_toolbar = ManageToolbar()
-        #self.add_toolbar(_('Manage'), self.manage_toolbar)
-        #self.manage_toolbar.show()
 
 class SearchToolbar(gtk.Toolbar):
     __gtype_name__ = 'SearchToolbar'
@@ -90,6 +88,12 @@ class SearchToolbar(gtk.Toolbar):
         self._autosearch_timer = None
         self._add_widget(self._search_entry, expand=True)
 
+        self._favorite_button = ToggleToolButton('emblem-favorite')
+        self._favorite_button.connect('toggled',
+                                      self.__favorite_button_toggled_cb)
+        self.insert(self._favorite_button, -1)
+        self._favorite_button.show()
+
         self._what_search_combo = ComboBox()
         self._what_combo_changed_sid = self._what_search_combo.connect(
                 'changed', self._combo_changed_cb)
@@ -159,8 +163,13 @@ class SearchToolbar(gtk.Toolbar):
 
     def _build_query(self):
         query = {}
+
         if self._mount_point:
             query['mountpoints'] = [self._mount_point]
+
+        if self._favorite_button.props.active:
+            query['keep'] = 1
+
         if self._what_search_combo.props.value:
             value = self._what_search_combo.props.value
             generic_type = mime.get_generic_type(value)
@@ -169,25 +178,15 @@ class SearchToolbar(gtk.Toolbar):
                 query['mime_type'] = mime_types
             else:
                 query['activity'] = self._what_search_combo.props.value
+
         if self._when_search_combo.props.value:
             date_from, date_to = self._get_date_range()
-            query['mtime'] = {'start': date_from, 'end': date_to}
+            query['timestamp'] = {'start': date_from, 'end': date_to}
+
         if self._search_entry.props.text:
             text = self._search_entry.props.text.strip()
-
-            if not text.startswith('"'):
-                query_text = ''
-                words = text.split(' ')
-                for word in words:
-                    if word:
-                        if query_text:
-                            query_text += ' '
-                        query_text += word + '*'
-            else:
-                query_text = text
-
-            if query_text:
-                query['query'] = query_text
+            if text:
+                query['query'] = text
 
         return query
 
@@ -205,10 +204,13 @@ class SearchToolbar(gtk.Toolbar):
         elif self._when_search_combo.props.value == _ACTION_PAST_YEAR:
             date_range = (today_start - timedelta(356), right_now)
         
-        return (date_range[0].isoformat(),
-                date_range[1].isoformat())
+        return (time.mktime(date_range[0].timetuple()),
+                time.mktime(date_range[1].timetuple()))
 
     def _combo_changed_cb(self, combo):
+        self._update_if_needed()
+
+    def _update_if_needed(self):
         new_query = self._build_query()
         if self._query != new_query:
             self._query = new_query
@@ -245,6 +247,19 @@ class SearchToolbar(gtk.Toolbar):
             self._query = new_query
             self.emit('query-changed', self._query)
 
+    def set_what_filter(self, what_filter):
+        combo_model = self._what_search_combo.get_model()
+        what_filter_index = -1
+        for i in range(0, len(combo_model) - 1):
+            if combo_model[i][0] == what_filter:
+                what_filter = i
+                break
+
+        if what_filter_index == -1:
+            logging.warning('what_filter %r not known' % what_filter)
+        else:
+            self._what_search_combo.set_active(what_filter_index)
+
     def refresh_filters(self):
         current_value = self._what_search_combo.props.value
         current_value_index = 0
@@ -292,11 +307,8 @@ class SearchToolbar(gtk.Toolbar):
             self._what_search_combo.handler_unblock(
                     self._what_combo_changed_sid)
 
-class ManageToolbar(gtk.Toolbar):
-    __gtype_name__ = 'ManageToolbar'
-
-    def __init__(self):
-        gtk.Toolbar.__init__(self)
+    def __favorite_button_toggled_cb(self, favorite_button):
+        self._update_if_needed()
 
 class DetailToolbox(Toolbox):
     def __init__(self):
diff --git a/src/jarabe/journal/listview.py b/src/jarabe/journal/listview.py
index 323da8d..3a5a909 100644
--- a/src/jarabe/journal/listview.py
+++ b/src/jarabe/journal/listview.py
@@ -18,6 +18,7 @@ import logging
 import traceback
 import sys
 from gettext import gettext as _
+import time
 
 import hippo
 import gobject
@@ -47,8 +48,10 @@ class BaseListView(gtk.HBox):
         self._result_set = None
         self._entries = []
         self._page_size = 0
-        self._last_value = -1
         self._reflow_sid = 0
+        self._do_scroll_hid = None
+        self._progress_bar = None
+        self._last_progress_bar_pulse = None
 
         gtk.HBox.__init__(self)
         self.set_flags(gtk.HAS_FOCUS|gtk.CAN_FOCUS)
@@ -126,19 +129,13 @@ class BaseListView(gtk.HBox):
             self._vscrollbar.hide()
 
     def _vadjustment_value_changed_cb(self, vadjustment):
-        gobject.idle_add(self._do_scroll)
+        if self._do_scroll_hid is None:
+            self._do_scroll_hid = gobject.idle_add(self._do_scroll)
 
-    def _do_scroll(self, force=False):
-        import time
-        t = time.time()
+    def _do_scroll(self):
+        current_position = int(self._vadjustment.props.value)
 
-        value = int(self._vadjustment.props.value)
-
-        if value == self._last_value and not force:
-            return
-        self._last_value = value
-
-        self._result_set.seek(value)
+        self._result_set.seek(current_position)
         metadata_list = self._result_set.read(self._page_size)
 
         if self._result_set.length != self._vadjustment.props.upper:
@@ -147,9 +144,8 @@ class BaseListView(gtk.HBox):
 
         self._refresh_view(metadata_list)
         self._dirty = False
-        
-        logging.debug('_do_scroll %r %r\n' % (value, (time.time() - t)))
-        
+
+        self._do_scroll_hid = None                
         return False
 
     def _refresh_view(self, metadata_list):
@@ -192,22 +188,68 @@ class BaseListView(gtk.HBox):
 
     def refresh(self):
         logging.debug('ListView.refresh query %r' % self._query)
+        self._stop_progress_bar()
+        self._start_progress_bar()
+        if self._result_set is not None:
+            self._result_set.stop()
+
         self._result_set = model.find(self._query)
+        self._result_set.ready.connect(self.__result_set_ready_cb)
+        self._result_set.progress.connect(self.__result_set_progress_cb)
+        self._result_set.setup()
+
+    def __result_set_ready_cb(self, **kwargs):
+        if kwargs['sender'] != self._result_set:
+            return
+
+        self._stop_progress_bar()
+
         self._vadjustment.props.upper = self._result_set.length
         self._vadjustment.changed()
 
         self._vadjustment.props.value = min(self._vadjustment.props.value,
                 self._result_set.length - self._page_size)
         if self._result_set.length == 0:
+            # FIXME: This is a hack, we shouldn't have to update this every time
+            # a new search term is added.
             if self._query.get('query', '') or \
                    self._query.get('mime_type', '') or \
+                   self._query.get('keep', '') or \
                    self._query.get('mtime', ''):
                 self._show_message(NO_MATCH)
             else:
                 self._show_message(EMPTY_JOURNAL)
         else:
             self._clear_message()
-            self._do_scroll(force=True)
+            self._do_scroll()
+
+    def __result_set_progress_cb(self, **kwargs):
+        if time.time() - self._last_progress_bar_pulse > 0.05:
+            if self._progress_bar is not None:
+                self._progress_bar.pulse()
+                self._last_progress_bar_pulse = time.time()
+
+    def _start_progress_bar(self):
+        self.remove(self._canvas)
+        self.remove(self._vscrollbar)
+
+        alignment = gtk.Alignment(xalign=0.5, yalign=0.5, xscale=0.5)
+        self.pack_start(alignment)
+        alignment.show()
+
+        self._progress_bar = gtk.ProgressBar()
+        self._progress_bar.props.pulse_step = 0.01
+        self._last_progress_bar_pulse = time.time()
+        alignment.add(self._progress_bar)
+        self._progress_bar.show()
+
+    def _stop_progress_bar(self):
+        for widget in self.get_children():
+            self.remove(widget)
+        self._progress_bar = None
+
+        self.pack_start(self._canvas)
+        self.pack_end(self._vscrollbar, expand=False, fill=False)
 
     def _scroll_event_cb(self, hbox, event):
         if event.direction == gtk.gdk.SCROLL_UP:
@@ -286,7 +328,7 @@ class BaseListView(gtk.HBox):
         if self._vadjustment.props.value > max_value:
             self._vadjustment.props.value = max_value
         else:
-            self._do_scroll(force=True)
+            self._do_scroll()
 
         self._reflow_sid = 0
 
diff --git a/src/jarabe/journal/misc.py b/src/jarabe/journal/misc.py
index a3ba0cf..699ac21 100644
--- a/src/jarabe/journal/misc.py
+++ b/src/jarabe/journal/misc.py
@@ -21,6 +21,7 @@ import sys
 import os
 from gettext import gettext as _
 
+import gio
 import gtk
 
 from sugar.activity import activityfactory
@@ -65,11 +66,13 @@ def get_icon_name(metadata):
 
     mime_type = metadata.get('mime_type', '')
     if not file_name and mime_type:
-        icon_name = mime.get_mime_icon(mime_type)
-        if icon_name:
+        icons = gio.content_type_get_icon(mime_type)
+        for icon_name in icons.props.names:
             file_name = _get_icon_file_name(icon_name)
+            if file_name is not None:
+                break
 
-    if not file_name or not os.path.exists(file_name):
+    if file_name is None or not os.path.exists(file_name):
         file_name = _get_icon_file_name('application-octet-stream')
 
     return file_name
diff --git a/src/jarabe/journal/model.py b/src/jarabe/journal/model.py
index 9a686f8..4d1751b 100644
--- a/src/jarabe/journal/model.py
+++ b/src/jarabe/journal/model.py
@@ -19,9 +19,14 @@ import os
 from datetime import datetime
 import time
 import shutil
+from stat import S_IFMT, S_IFDIR, S_IFREG
+import traceback
+import re
 
+import gobject
 import dbus
 import gconf
+import gio
 
 from sugar import dispatch
 from sugar import mime
@@ -72,7 +77,7 @@ class _Cache(object):
         else:
             return self._array[key]
 
-class ResultSet(object):
+class BaseResultSet(object):
     """Encapsulates the result of a query
     """
 
@@ -86,65 +91,28 @@ class ResultSet(object):
         self._offset = 0
         self._cache = _Cache()
 
+        self.ready = dispatch.Signal()
+        self.progress = dispatch.Signal()
+
+    def setup(self):
+        self.ready.send(self)
+
+    def stop(self):
+        pass
+
     def get_length(self):
         if self._total_count == -1:
             query = self._query.copy()
-            query['limit'] = ResultSet._CACHE_LIMIT
-            entries, self._total_count = self._find(query)
+            query['limit'] = BaseResultSet._CACHE_LIMIT
+            entries, self._total_count = self.find(query)
             self._cache.append_all(entries)
             self._offset = 0
         return self._total_count
 
     length = property(get_length)
 
-    def _get_all_files(self, dir_path):
-        files = []
-        for entry in os.listdir(dir_path):
-            full_path = os.path.join(dir_path, entry)
-            if os.path.isdir(full_path):
-                files.extend(self._get_all_files(full_path))
-            elif os.path.isfile(full_path):
-                stat = os.stat(full_path)
-                files.append((full_path, stat.st_mtime))
-        return files
-
-    def _query_mount_point(self, mount_point, query):
-        t = time.time()
-
-        files = self._get_all_files(mount_point)
-        offset = int(query.get('offset', 0))
-        limit  = int(query.get('limit', len(files)))
-
-        total_count = len(files)
-        files.sort(lambda a, b: int(b[1] - a[1]))
-        files = files[offset:offset + limit]
-
-        result = []
-        for file_path, timestamp_ in files:
-            metadata = _get_file_metadata(file_path)
-            result.append(metadata)
-
-        logging.debug('_query_mount_point took %f s.' % (time.time() - t))
-
-        return result, total_count
-
-    def _find(self, query):
-        mount_points = query.get('mountpoints', ['/'])
-        if mount_points is None or len(mount_points) != 1:
-            raise ValueError('Exactly one mount point must be specified')
-
-        if mount_points[0] == '/':
-            data_store = _get_datastore()
-            entries, total_count = _get_datastore().find(query, PROPERTIES,
-                                                         byte_arrays=True)
-        else:
-            entries, total_count = self._query_mount_point(mount_points[0],
-                                                           query)
-
-        for entry in entries:
-            entry['mountpoint'] = mount_points[0]
-
-        return entries, total_count
+    def find(self, query):
+        raise NotImplementedError()
 
     def seek(self, position):
         self._position = position
@@ -152,10 +120,10 @@ class ResultSet(object):
     def read(self, max_count):
         logging.debug('ResultSet.read position: %r' % self._position)
 
-        if max_count * 5 > ResultSet._CACHE_LIMIT:
+        if max_count * 5 > BaseResultSet._CACHE_LIMIT:
             raise RuntimeError(
-                    'max_count (%i) too big for ResultSet._CACHE_LIMIT'
-                    ' (%i).' % (max_count, ResultSet._CACHE_LIMIT))
+                    'max_count (%i) too big for BaseResultSet._CACHE_LIMIT'
+                    ' (%i).' % (max_count, BaseResultSet._CACHE_LIMIT))
 
         if self._position == -1:
             self.seek(0)
@@ -175,16 +143,16 @@ class ResultSet(object):
 
         if (remaining_forward_entries <= 0 and
                     remaining_backwards_entries <= 0) or \
-                max_count > ResultSet._CACHE_LIMIT:
+                max_count > BaseResultSet._CACHE_LIMIT:
 
             # Total cache miss: remake it
             offset = max(0, self._position - max_count)
             logging.debug('remaking cache, offset: %r limit: %r' % \
                           (offset, max_count * 2))
             query = self._query.copy()
-            query['limit'] = ResultSet._CACHE_LIMIT
+            query['limit'] = BaseResultSet._CACHE_LIMIT
             query['offset'] = offset
-            entries, self._total_count = self._find(query)
+            entries, self._total_count = self.find(query)
 
             self._cache.remove_all(self._cache)
             self._cache.append_all(entries)
@@ -199,13 +167,13 @@ class ResultSet(object):
             query = self._query.copy()
             query['limit'] = max_count
             query['offset'] = last_cached_entry
-            entries, self._total_count = self._find(query)
+            entries, self._total_count = self.find(query)
 
             # update cache
             self._cache.append_all(entries)
 
             # apply the cache limit
-            objects_excess = len(self._cache) - ResultSet._CACHE_LIMIT
+            objects_excess = len(self._cache) - BaseResultSet._CACHE_LIMIT
             if objects_excess > 0:
                 self._offset += objects_excess
                 self._cache.remove_all(self._cache[:objects_excess])
@@ -221,13 +189,13 @@ class ResultSet(object):
             query = self._query.copy()
             query['limit'] = limit
             query['offset'] = self._offset
-            entries, self._total_count = self._find(query)
+            entries, self._total_count = self.find(query)
 
             # update cache
             self._cache.prepend_all(entries)
 
             # apply the cache limit
-            objects_excess = len(self._cache) - ResultSet._CACHE_LIMIT
+            objects_excess = len(self._cache) - BaseResultSet._CACHE_LIMIT
             if objects_excess > 0:
                 self._cache.remove_all(self._cache[-objects_excess:])
         else:
@@ -237,16 +205,134 @@ class ResultSet(object):
         last_pos = self._position - self._offset + max_count
         return self._cache[first_pos:last_pos]
 
-def _get_file_metadata(path):
-    stat = os.stat(path)
+class DatastoreResultSet(BaseResultSet):
+    """Encapsulates the result of a query on the datastore
+    """
+    def __init__(self, query):
+
+        if query.get('query', '') and not query['query'].startswith('"'):
+            query_text = ''
+            words = query['query'].split(' ')
+            for word in words:
+                if word:
+                    if query_text:
+                        query_text += ' '
+                    query_text += word + '*'
+
+            query['query'] = query_text
+
+        BaseResultSet.__init__(self, query)
+
+    def find(self, query):
+        entries, total_count = _get_datastore().find(query, PROPERTIES,
+                                                     byte_arrays=True)
+
+        for entry in entries:
+            entry['mountpoint'] = '/'
+
+        return entries, total_count
+
+class InplaceResultSet(BaseResultSet):
+    """Encapsulates the result of a query on a mount point
+    """
+    def __init__(self, query, mount_point):
+        BaseResultSet.__init__(self, query)
+        self._mount_point = mount_point
+        self._file_list = None
+        self._pending_directories = 0
+        self._stopped = False
+
+        query_text = query.get('query', '')
+        if query_text.startswith('"') and query_text.endswith('"'):
+            self._regex = re.compile('*%s*' % query_text.strip(['"']))
+        elif query_text:
+            expression = ''
+            for word in query_text.split(' '):
+                expression += '(?=.*%s.*)' % word
+            self._regex = re.compile(expression, re.IGNORECASE)
+        else:
+            self._regex = None
+
+    def setup(self):
+        self._file_list = []
+        self._recurse_dir(self._mount_point)
+
+    def stop(self):
+        self._stopped = True
+
+    def setup_ready(self):
+        self._file_list.sort(lambda a, b: b[2] - a[2])
+        self.ready.send(self)
+
+    def find(self, query):
+        if self._file_list is None:
+            raise ValueError('Need to call setup() first')
+
+        if self._stopped:
+            raise ValueError('InplaceResultSet already stopped')
+
+        t = time.time()
+
+        offset = int(query.get('offset', 0))
+        limit  = int(query.get('limit', len(self._file_list)))
+        total_count = len(self._file_list)
+
+        files = self._file_list[offset:offset + limit]
+
+        entries = []
+        for file_path, stat, mtime_ in files:
+            metadata = _get_file_metadata(file_path, stat)
+            metadata['mountpoint'] = self._mount_point
+            entries.append(metadata)
+
+        logging.debug('InplaceResultSet.find took %f s.' % (time.time() - t))
+
+        return entries, total_count
+
+    def _recurse_dir(self, dir_path):
+        if self._stopped:
+            return
+
+        for entry in os.listdir(dir_path):
+            if entry.startswith('.'):
+                continue
+            full_path = dir_path + '/' + entry
+            try:
+                stat = os.stat(full_path)
+                if S_IFMT(stat.st_mode) == S_IFDIR:
+                    self._pending_directories += 1
+                    gobject.idle_add(lambda s=full_path: self._recurse_dir(s))
+
+                elif S_IFMT(stat.st_mode) == S_IFREG:
+                    add_to_list = False
+                    if self._regex is None or self._regex.match(full_path):
+                        add_to_list = True
+
+                    if add_to_list:
+                        file_info = (full_path, stat, int(stat.st_mtime))
+                        self._file_list.append(file_info)
+
+                    self.progress.send(self)
+
+            except Exception:
+                logging.error('Error reading file %r: %r' % \
+                              (full_path, traceback.format_exc()))
+
+        if self._pending_directories == 0:
+            self.setup_ready()
+        else:
+            self._pending_directories -= 1
+
+def _get_file_metadata(path, stat):
     client = gconf.client_get_default()
     return {'uid': path,
             'title': os.path.basename(path),
             'timestamp': stat.st_mtime,
-            'mime_type': mime.get_for_file(path),
+            'mime_type': gio.content_type_guess(filename=path),
             'activity': '',
             'activity_id': '',
-            'icon-color': client.get_string('/desktop/sugar/user/color')}
+            'icon-color': client.get_string('/desktop/sugar/user/color'),
+            'description': path}
 
 _datastore = None
 def _get_datastore():
@@ -276,7 +362,15 @@ def find(query):
     """
     if 'order_by' not in query:
         query['order_by'] = ['-mtime']
-    return ResultSet(query)
+    
+    mount_points = query.pop('mountpoints', ['/'])
+    if mount_points is None or len(mount_points) != 1:
+        raise ValueError('Exactly one mount point must be specified')
+
+    if mount_points[0] == '/':
+        return DatastoreResultSet(query)
+    else:
+        return InplaceResultSet(query, mount_points[0])
 
 def _get_mount_point(path):
     dir_path = os.path.dirname(path)
@@ -290,7 +384,8 @@ def get(object_id):
     """Returns the metadata for an object
     """
     if os.path.exists(object_id):
-        metadata = _get_file_metadata(object_id)
+        stat = os.stat(object_id)
+        metadata = _get_file_metadata(object_id, stat)
         metadata['mountpoint'] = _get_mount_point(object_id)
     else:
         metadata = _get_datastore().get_properties(object_id, byte_arrays=True)
@@ -329,9 +424,7 @@ def copy(metadata, mount_point):
     """Copies an object to another mount point
     """
     metadata = get(metadata['uid'])
-
     file_path = get_file(metadata['uid'])
-    file_path.delete = False
 
     metadata['mountpoint'] = mount_point
     del metadata['uid']
@@ -341,7 +434,7 @@ def copy(metadata, mount_point):
 def write(metadata, file_path='', update_mtime=True):
     """Creates or updates an entry for that id
     """
-    logging.debug('model.write %r %r %r' % (metadata['uid'], file_path,
+    logging.debug('model.write %r %r %r' % (metadata.get('uid', ''), file_path,
                                             update_mtime))
     if update_mtime:
         metadata['mtime'] = datetime.now().isoformat()
@@ -361,7 +454,10 @@ def write(metadata, file_path='', update_mtime=True):
         if not os.path.exists(file_path):
             raise ValueError('Entries without a file cannot be copied to '
                              'removable devices')
+
         file_name = _get_file_name(metadata['title'], metadata['mime_type'])
+        file_name = _get_unique_file_name(metadata['mountpoint'], file_name)
+
         destination_path = os.path.join(metadata['mountpoint'], file_name)
         shutil.copy(file_path, destination_path)
         object_id = destination_path
@@ -369,10 +465,38 @@ def write(metadata, file_path='', update_mtime=True):
     return object_id
 
 def _get_file_name(title, mime_type):
-    # TODO: sanitize title for common filesystems
-    # TODO: make as robust as possible, this function should never fail.
-    # TODO: don't append the same extension again and again
-    return '%s.%s' % (title, mime.get_primary_extension(mime_type))
+    file_name = title
+
+    extension = '.' + mime.get_primary_extension(mime_type)
+    if not file_name.endswith(extension):
+        file_name += extension
+
+    # Invalid characters in VFAT filenames. From
+    # http://en.wikipedia.org/wiki/File_Allocation_Table
+    invalid_chars = ['/', '\\', ':', '*', '?', '"', '<', '>', '|', '\x7F']
+    invalid_chars.extend([chr(x) for x in range(0, 32)])
+    for char in invalid_chars:
+        file_name = file_name.replace(char, '_')
+
+    # FAT limit is 255, leave some space for uniqueness
+    max_len = 250
+    if len(file_name) > max_len:
+        name, extension = os.path.splitext(file_name)
+        file_name = name[0:max_len - extension] + extension
+    
+    return file_name
+
+def _get_unique_file_name(mount_point, file_name):
+    if os.path.exists(os.path.join(mount_point, file_name)):
+        i = 1
+        while len(file_name) <= 255:
+            name, extension = os.path.splitext(file_name)
+            file_name = name + '_' + str(i) + extension
+            if not os.path.exists(os.path.join(mount_point, file_name)):
+                break
+            i += 1
+
+    return file_name
 
 created = dispatch.Signal()
 updated = dispatch.Signal()
diff --git a/src/jarabe/journal/objectchooser.py b/src/jarabe/journal/objectchooser.py
index fcaed55..ee894cf 100644
--- a/src/jarabe/journal/objectchooser.py
+++ b/src/jarabe/journal/objectchooser.py
@@ -39,7 +39,7 @@ class ObjectChooser(gtk.Window):
                      ([int]))
     }
 
-    def __init__(self, parent=None):
+    def __init__(self, parent=None, what_filter=''):
         gtk.Window.__init__(self)
         self.set_type_hint(gtk.gdk.WINDOW_TYPE_HINT_DIALOG)
         self.set_decorated(False)
@@ -89,6 +89,9 @@ class ObjectChooser(gtk.Window):
         height = gtk.gdk.screen_height() - style.GRID_CELL_SIZE * 2        
         self.set_size_request(width, height)
 
+        if what_filter:
+            self._toolbar.set_what_filter(what_filter)
+
     def __realize_cb(self, chooser, parent):
         self.window.set_transient_for(parent)
         # TODO: Should we disconnect the signal here?