engine: improve handling of outdated dictionaries

The `dictionaries_loaded` hook will be now potentially triggered
2 times during an update:
- after unloading outdated dictionaries
- after (re)loading dictionaries

This way the GUI can show a `reloading` icon for dictionaries
being reloaded because they were externally modified.

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

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/utils.py

@@ -4,12 +4,17 @@
 
 
 @contextmanager
-def make_dict(contents):
-    tf = tempfile.NamedTemporaryFile(delete=False)
+def make_dict(contents, extension=None, name=None):
+    kwargs = { 'delete': False }
+    if name is not None:
+        kwargs['prefix'] = name + '_'
+    if extension is not None:
+        kwargs['suffix'] = '.' + extension
+    tf = tempfile.NamedTemporaryFile(**kwargs)
     try:
         tf.write(contents)
         tf.close()
-        yield tf.name
+        yield os.path.realpath(tf.name)
     finally:
         os.unlink(tf.name)