Merge pull request #754 from benoit-pierre/reload_icon_for_outdated_d…

…ictionaries

show reload icon for outdated dictionaries

plover/dictionary/loading_manager.py

@@ -17,6 +17,15 @@ class DictionaryLoadingManager(object):
     def __init__(self):
         self.dictionaries = {}
 
+    def __len__(self):
+        return len(self.dictionaries)
+
+    def __getitem__(self, filename):
+        return self.dictionaries[filename].get()
+
+    def __contains__(self, filename):
+        return filename in self.dictionaries
+
     def start_loading(self, filename):
         op = self.dictionaries.get(filename)
         if op is not None and not op.needs_reloading():
@@ -26,6 +35,11 @@ def start_loading(self, filename):
         self.dictionaries[filename] = op
         return op
 
+    def unload_outdated(self):
+        for filename, op in list(self.dictionaries.items()):
+            if op.needs_reloading():
+                del self.dictionaries[filename]
+
     def load(self, filenames):
         start_time = time.time()
         self.dictionaries = {f: self.start_loading(f) for f in filenames}

plover/engine.py

@@ -16,7 +16,7 @@
 from plover.registry import registry
 from plover.resource import ASSET_SCHEME, resource_filename
 from plover.steno import Stroke
-from plover.steno_dictionary import StenoDictionary
+from plover.steno_dictionary import StenoDictionary, StenoDictionaryCollection
 from plover.suggestions import Suggestions
 from plover.translation import Translator
 
@@ -109,7 +109,6 @@ def __init__(self, config, keyboard_emulation):
         self._dictionaries = self._translator.get_dictionary()
         self._dictionaries_manager = DictionaryLoadingManager()
         self._running_state = self._translator.get_state()
-        self._suggestions = Suggestions(self._dictionaries)
         self._keyboard_emulation = keyboard_emulation
         self._hooks = { hook: [] for hook in self.HOOKS }
         self._running_extensions = {}
@@ -150,6 +149,21 @@ def _start(self):
         self._set_output(self._config.get_auto_start())
         self._update(full=True)
 
+    def _set_dictionaries(self, dictionaries):
+        def dictionaries_changed(l1, l2):
+            if len(l1) != len(l2):
+                return True
+            for d1, d2 in zip(l1, l2):
+                if d1 is not d2:
+                    return True
+            return False
+        if not dictionaries_changed(dictionaries, self._dictionaries.dicts):
+            # No change.
+            return
+        self._dictionaries = StenoDictionaryCollection(dictionaries)
+        self._translator.set_dictionary(self._dictionaries)
+        self._trigger_hook('dictionaries_loaded', self._dictionaries)
+
     def _update(self, config_update=None, full=False, reset_machine=False):
         original_config = self._config.as_dict()
         # Update configuration.
@@ -238,6 +252,14 @@ def _update(self, config_update=None, full=False, reset_machine=False):
             for d in config['dictionaries']
         )
         copy_default_dictionaries(config_dictionaries.keys())
+        # Start by unloading outdated dictionaries.
+        self._dictionaries_manager.unload_outdated()
+        self._set_dictionaries([
+            d for d in self._dictionaries.dicts
+            if d.path in config_dictionaries and \
+               d.path in self._dictionaries_manager
+        ])
+        # And then (re)load all dictionaries.
         dictionaries = []
         for result in self._dictionaries_manager.load(config_dictionaries.keys()):
             if isinstance(result, DictionaryLoaderException):
@@ -250,16 +272,7 @@ def _update(self, config_update=None, full=False, reset_machine=False):
                 d = result
             d.enabled = config_dictionaries[d.path].enabled
             dictionaries.append(d)
-        def dictionaries_changed(l1, l2):
-            if len(l1) != len(l2):
-                return True
-            for d1, d2 in zip(l1, l2):
-                if d1 is not d2:
-                    return True
-            return False
-        if dictionaries_changed(dictionaries, self._dictionaries.dicts):
-            self._dictionaries.set_dicts(dictionaries)
-            self._trigger_hook('dictionaries_loaded', self._dictionaries)
+        self._set_dictionaries(dictionaries)
 
     def _start_extensions(self, extension_list):
         for extension_name in extension_list:
@@ -460,7 +473,7 @@ def remove_dictionary_filter(self, dictionary_filter):
 
     @with_lock
     def get_suggestions(self, translation):
-        return self._suggestions.find(translation)
+        return Suggestions(self._dictionaries).find(translation)
 
     @property
     @with_lock

plover/steno_dictionary.py

@@ -44,6 +44,9 @@ def __init__(self):
     def __str__(self):
         return '%s(%r)' % (self.__class__.__name__, self.path)
 
+    def __repr__(self):
+        return str(self)
+
     @classmethod
     def create(cls, resource):
         assert not resource.startswith(ASSET_SCHEME)
@@ -180,11 +183,12 @@ def remove_longest_key_listener(self, callback):
 
 class StenoDictionaryCollection(object):
 
-    def __init__(self):
+    def __init__(self, dicts=[]):
         self.dicts = []
         self.filters = []
         self.longest_key = 0
         self.longest_key_callbacks = set()
+        self.set_dicts(dicts)
 
     def set_dicts(self, dicts):
         for d in self.dicts:
@@ -212,6 +216,12 @@ def _lookup(self, key, dicts=None, filters=()):
                         return None
                 return value
 
+    def __str__(self):
+        return 'StenoDictionaryCollection' + repr(tuple(self.dicts))
+
+    def __repr__(self):
+        return str(self)
+
     def lookup(self, key):
         return self._lookup(key, filters=self.filters)
 

test/test_engine.py

@@ -7,10 +7,13 @@
 import mock
 
 from plover import system
-from plover.config import DEFAULT_SYSTEM_NAME
-from plover.engine import StenoEngine
+from plover.config import DEFAULT_SYSTEM_NAME, DictionaryConfig
+from plover.engine import ErroredDictionary, StenoEngine
 from plover.registry import Registry
 from plover.machine.base import StenotypeBase
+from plover.steno_dictionary import StenoDictionaryCollection
+
+from .utils import make_dict
 
 
 class FakeConfig(object):
@@ -175,3 +178,80 @@ def test_engine(self):
                 ('machine_state_changed', ('Fake', 'stopped'), {}),
             ])
             self.assertIsNone(FakeMachine.instance)
+
+    def test_loading_dictionaries(self):
+        def check_loaded_events(actual_events, expected_events):
+            self.assertEqual(len(actual_events), len(expected_events), msg='events: %r' % self.events)
+            for n, event in enumerate(actual_events):
+                event_type, event_args, event_kwargs = event
+                msg = 'event %u: %r' % (n, event)
+                self.assertEqual(event_type, 'dictionaries_loaded', msg=msg)
+                self.assertEqual(event_kwargs, {}, msg=msg)
+                self.assertEqual(len(event_args), 1, msg=msg)
+                self.assertIsInstance(event_args[0], StenoDictionaryCollection, msg=msg)
+                self.assertEqual([
+                    (d.path, d.enabled, isinstance(d, ErroredDictionary))
+                    for d in event_args[0].dicts
+                ], expected_events[n], msg=msg)
+        with \
+                make_dict(b'{}', 'json', 'valid1') as valid_dict_1, \
+                make_dict(b'{}', 'json', 'valid2') as valid_dict_2, \
+                make_dict(b'', 'json', 'invalid1') as invalid_dict_1, \
+                make_dict(b'', 'json', 'invalid2') as invalid_dict_2, \
+                self._setup():
+            self.engine.start()
+            for test in (
+                # Load one valid dictionary.
+                [[
+                    # path, enabled
+                    (valid_dict_1, True),
+                ], [
+                    # path, enabled, errored
+                    (valid_dict_1, True, False),
+                ]],
+                # Load another invalid dictionary.
+                [[
+                    (valid_dict_1, True),
+                    (invalid_dict_1, True),
+                ], [
+                    (valid_dict_1, True, False),
+                    (invalid_dict_1, True, True),
+                ]],
+                # Disable first dictionary.
+                [[
+                    (valid_dict_1, False),
+                    (invalid_dict_1, True),
+                ], [
+                    (valid_dict_1, False, False),
+                    (invalid_dict_1, True, True),
+                ]],
+                # Replace invalid dictonary with another invalid one.
+                [[
+                    (valid_dict_1, False),
+                    (invalid_dict_2, True),
+                ], [
+                    (valid_dict_1, False, False),
+                ], [
+                    (valid_dict_1, False, False),
+                    (invalid_dict_2, True, True),
+                ]]
+            ):
+                config_dictionaries = [
+                    DictionaryConfig(path, enabled)
+                    for path, enabled in test[0]
+                ]
+                self.events = []
+                config_update = { 'dictionaries': list(config_dictionaries), }
+                self.engine.config = dict(config_update)
+                self.assertEqual(self.events[0], ('config_changed', (config_update,), {}))
+                check_loaded_events(self.events[1:], test[1:])
+            # Simulate an outdated dictionary.
+            self.events = []
+            self.engine.dictionaries[valid_dict_1].timestamp -= 1
+            self.engine.config = {}
+            check_loaded_events(self.events, [[
+                (invalid_dict_2, True, True),
+            ], [
+                (valid_dict_1, False, False),
+                (invalid_dict_2, True, True),
+            ]])

test/test_loading_manager.py

@@ -38,7 +38,6 @@ def __init__(self, name, contents):
                 self.name = name
                 self.contents = contents
                 self.tf = tempfile.NamedTemporaryFile()
-                self.timestamp = os.path.getmtime(self.tf.name)
 
             def __repr__(self):
                 return 'FakeDictionaryInfo(%r, %r)' % (self.name, self.contents)
@@ -53,7 +52,8 @@ def __call__(self, filename):
                 d = self.files[filename]
                 if isinstance(d.contents, Exception):
                     raise d.contents
-                return FakeDictionaryContents(d.contents, d.timestamp)
+                timestamp = os.path.getmtime(filename)
+                return FakeDictionaryContents(d.contents, timestamp)
 
         dictionaries = {}
         for i in range(8):
@@ -73,13 +73,21 @@ def df(name):
         loader = MockLoader(dictionaries)
         with patch('plover.dictionary.loading_manager.load_dictionary', loader):
             manager = loading_manager.DictionaryLoadingManager()
-            manager.start_loading(df('a'))
-            manager.start_loading(df('b'))
+            manager.start_loading(df('a')).get()
+            manager.start_loading(df('b')).get()
             results = manager.load([df('c'), df('b')])
             # Returns the right values in the right order.
             self.assertEqual(results, ['ccccc', 'bbbbb'])
             # Dropped superfluous files.
             assertCountEqual(self, [df('b'), df('c')], manager.dictionaries.keys())
+            # Check dict like interface.
+            self.assertEqual(len(manager), 2)
+            self.assertFalse(df('a') in manager)
+            with self.assertRaises(KeyError):
+                manager[df('a')]
+            self.assertTrue(df('b') in manager)
+            self.assertTrue(df('c') in manager)
+            self.assertEqual(results, [manager[df('c')], manager[df('b')]])
             # Return a DictionaryLoaderException for load errors.
             results = manager.load([df('c'), df('e'), df('b'), df('f')])
             self.assertEqual(len(results), 4)
@@ -93,22 +101,22 @@ def df(name):
             self.assertIsInstance(results[3].exception, Exception)
             # Only loaded the files once.
             self.assertTrue(all(x == 1 for x in loader.load_counts.values()))
-            # No reload if timestamp is unchanged, or more recent.
-            # (use case: dictionary edited with Plover and saved back)
+            # No reload if file timestamp is unchanged, or the dictionary
+            # timestamp is more recent. (use case: dictionary edited with
+            # Plover and saved back)
+            file_timestamp = results[0].timestamp
+            results[0].timestamp = file_timestamp + 1
             dictionaries['c'].contents = 'CCCCC'
-            dictionaries['c'].timestamp += 1
             results = manager.load([df('c')])
             self.assertEqual(results, ['ccccc'])
             self.assertEqual(loader.load_counts[df('c')], 1)
-            # Check files are reloaded when modified.
-            # Note: do not try to "touch" the file using os.utime
-            # as it's not reliable without sleeping to account
-            # for operating system resolution.
-            mtime = os.path.getmtime(df('c'))
-            def getmtime(path):
-                assert path == df('c')
-                return mtime + 1
-            with patch('os.path.getmtime', getmtime):
-                results = manager.load([df('c')])
+            # Check outdated dictionaries are reloaded.
+            results[0].timestamp = file_timestamp - 1
+            results = manager.load([df('c')])
             self.assertEqual(results, ['CCCCC'])
             self.assertEqual(loader.load_counts[df('c')], 2)
+            # Check trimming of outdated dictionaries.
+            results[0].timestamp = file_timestamp - 1
+            manager.unload_outdated()
+            self.assertEqual(len(manager), 0)
+            self.assertFalse(df('c') in manager)

test/test_steno_dictionary.py

@@ -55,7 +55,6 @@ def listener(longest_key):
         self.assertEqual(notifications, [1, 4, 2, 0])
 
     def test_dictionary_collection(self):
-        dc = StenoDictionaryCollection()
         d1 = StenoDictionary()
         d1[('S',)] = 'a'
         d1[('T',)] = 'b'
@@ -64,7 +63,7 @@ def test_dictionary_collection(self):
         d2[('S',)] = 'c'
         d2[('W',)] = 'd'
         d2.path = 'd2'
-        dc.set_dicts([d2, d1])
+        dc = StenoDictionaryCollection([d2, d1])
         self.assertEqual(dc.lookup(('S',)), 'c')
         self.assertEqual(dc.lookup(('W',)), 'd')
         self.assertEqual(dc.lookup(('T',)), 'b')
@@ -104,15 +103,14 @@ def test_dictionary_collection(self):
             dc['invalid']
 
     def test_dictionary_collection_writeable(self):
-        dc = StenoDictionaryCollection()
         d1 = StenoDictionary()
         d1[('S',)] = 'a'
         d1[('T',)] = 'b'
         d2 = StenoDictionary()
         d2[('S',)] = 'c'
         d2[('W',)] = 'd'
         d2.readonly = True
-        dc.set_dicts([d2, d1])
+        dc = StenoDictionaryCollection([d2, d1])
         self.assertEqual(dc.first_writable(), d1)
         dc.set(('S',), 'A')
         self.assertEqual(d1[('S',)], 'A')

test/test_translation.py

@@ -75,8 +75,7 @@ def setUp(self):
         self.s = type(self).FakeState()
         self.t._state = self.s
         self.d = StenoDictionary()
-        self.dc = StenoDictionaryCollection()
-        self.dc.set_dicts([self.d])
+        self.dc = StenoDictionaryCollection([self.d])
         self.t.set_dictionary(self.dc)
 
     def test_dictionary_update_grows_size1(self):
@@ -170,8 +169,7 @@ def listener(undo, do, prev):
 
         d = StenoDictionary()
         d[('S', 'P')] = 'hi'
-        dc = StenoDictionaryCollection()
-        dc.set_dicts([d])
+        dc = StenoDictionaryCollection([d])
         t = Translator()
         t.set_dictionary(dc)
         t.translate(stroke('T'))
@@ -230,11 +228,10 @@ def get(self):
             def clear(self):
                 del self._output[:]
 
-        d = StenoDictionary()        
-        out = Output()        
+        d = StenoDictionary()
+        out = Output()
         t = Translator()
-        dc = StenoDictionaryCollection()
-        dc.set_dicts([d])
+        dc = StenoDictionaryCollection([d])
         t.set_dictionary(dc)
         t.add_listener(out.write)
 
@@ -457,8 +454,7 @@ def assertOutput(self, undo, do, prev):
 
     def setUp(self):
         self.d = StenoDictionary()
-        self.dc = StenoDictionaryCollection()
-        self.dc.set_dicts([self.d])
+        self.dc = StenoDictionaryCollection([self.d])
         self.s = _State()
         self.o = self.CaptureOutput()
         self.tlor = Translator()