Merge pull request #2963 from forslund/feature/update-adapt-keyword-r…

…egistration-message

Update key names in adapt keyword registration message

mycroft/skills/intent_service.py

@@ -358,12 +358,18 @@ def handle_register_vocab(self, message):
         Args:
             message (Message): message containing vocab info
         """
-        start_concept = message.data.get('start')
-        end_concept = message.data.get('end')
+        # TODO: 22.02 Remove backwards compatibility
+        if _is_old_style_keyword_message(message):
+            LOG.warning('Deprecated: Registering keywords with old message. '
+                        'This will be removed in v22.02.')
+            _update_keyword_message(message)
+
+        entity_value = message.data.get('entity_value')
+        entity_type = message.data.get('entity_type')
         regex_str = message.data.get('regex')
         alias_of = message.data.get('alias_of')
-        self.adapt_service.register_vocab(start_concept, end_concept,
-                                          alias_of, regex_str)
+        self.adapt_service.register_vocabulary(entity_value, entity_type,
+                                               alias_of, regex_str)
         self.registered_vocab.append(message.data)
 
     def handle_register_intent(self, message):
@@ -554,3 +560,29 @@ def handle_entity_manifest(self, message):
         self.bus.emit(message.reply(
             "intent.service.padatious.entities.manifest",
             {"entities": self.padatious_service.registered_entities}))
+
+
+def _is_old_style_keyword_message(message):
+    """Simple check that the message is not using the updated format.
+
+    TODO: Remove in v22.02
+
+    Args:
+        message (Message): Message object to check
+
+    Returns:
+        (bool) True if this is an old messagem, else False
+    """
+    return ('entity_value' not in message.data and 'start' in message.data)
+
+
+def _update_keyword_message(message):
+    """Make old style keyword registration message compatible.
+
+    Copies old keys in message data to new names.
+
+    Args:
+        message (Message): Message to update
+    """
+    message.data['entity_value'] = message.data['start']
+    message.data['entity_type'] = message.data['end']

mycroft/skills/intent_service_interface.py

@@ -44,15 +44,27 @@ def register_adapt_keyword(self, vocab_type, entity, aliases=None):
 
             vocab_type(str): Keyword reference
             entity (str): Primary keyword
-            aliases (list): List of alternative kewords
+            aliases (list): List of alternative keywords
         """
+        # TODO 22.02: Remove compatibility data
         aliases = aliases or []
-        self.bus.emit(Message("register_vocab",
-                              {'start': entity, 'end': vocab_type}))
+        entity_data = {'entity_value': entity, 'entity_type': vocab_type}
+        compatibility_data = {'start': entity, 'end': vocab_type}
+
+        self.bus.emit(
+            Message("register_vocab",
+                    {**entity_data, **compatibility_data})
+        )
         for alias in aliases:
-            self.bus.emit(Message("register_vocab", {
-                'start': alias, 'end': vocab_type, 'alias_of': entity
-            }))
+            alias_data = {
+                'entity_value': alias,
+                'entity_type': vocab_type,
+                'alias_of': entity}
+            compatibility_data = {'start': alias, 'end': vocab_type}
+            self.bus.emit(
+                Message("register_vocab",
+                        {**alias_data, **compatibility_data})
+            )
 
     def register_adapt_regex(self, regex):
         """Register a regex with the intent service.

mycroft/skills/intent_services/adapt_service.py

@@ -249,14 +249,35 @@ def take_best(intent, utt):
             ret = None
         return ret
 
+    # TODO 22.02: Remove this deprecated method
     def register_vocab(self, start_concept, end_concept, alias_of, regex_str):
-        """Register vocabulary."""
+        """Register Vocabulary. DEPRECATED
+
+        This method should not be used, it has been replaced by
+        register_vocabulary().
+        """
+        self.register_vocabulary(start_concept, end_concept,
+                                 alias_of, regex_str)
+
+    def register_vocabulary(self, entity_value, entity_type,
+                            alias_of, regex_str):
+        """Register skill vocabulary as adapt entity.
+
+        This will handle both regex registration and registration of normal
+        keywords. if the "regex_str" argument is set all other arguments will
+        be ignored.
+
+        Argument:
+            entity_value: the natural langauge word
+            entity_type: the type/tag of an entity instance
+            alias_of: entity this is an alternative for
+        """
         with self.lock:
             if regex_str:
                 self.engine.register_regex_entity(regex_str)
             else:
                 self.engine.register_entity(
-                    start_concept, end_concept, alias_of=alias_of)
+                    entity_value, entity_type, alias_of=alias_of)
 
     def register_intent(self, intent):
         """Register new intent with adapt engine.

mycroft/skills/mycroft_skill/mycroft_skill.py

@@ -1168,9 +1168,8 @@ def register_vocabulary(self, entity, entity_type):
             entity:         word to register
             entity_type:    Intent handler entity to tie the word to
         """
-        self.bus.emit(Message('register_vocab', {
-            'start': entity, 'end': to_alnum(self.skill_id) + entity_type
-        }))
+        keyword_type = to_alnum(self.skill_id) + entity_type
+        self.intent_service.register_adapt_keyword(keyword_type, entity)
 
     def register_regex(self, regex_str):
         """Register a new regex.

test/unittests/skills/test_intent_service.py

@@ -214,12 +214,18 @@ def test_lang_exists(self):
         self.assertEqual(_get_message_lang(msg), 'sv-se')
 
 
-def create_vocab_msg(keyword, value):
+def create_old_style_vocab_msg(keyword, value):
     """Create a message for registering an adapt keyword."""
     return Message('register_vocab',
                    {'start': value, 'end': keyword})
 
 
+def create_vocab_msg(keyword, value):
+    """Create a message for registering an adapt keyword."""
+    return Message('register_vocab',
+                   {'entity_value': value, 'entity_type': keyword})
+
+
 def get_last_message(bus):
     """Get last sent message on mock bus."""
     last = bus.emit.call_args
@@ -230,14 +236,27 @@ class TestIntentServiceApi(TestCase):
     def setUp(self):
         self.intent_service = IntentService(mock.Mock())
 
-    def setup_simple_adapt_intent(self):
-        msg = create_vocab_msg('testKeyword', 'test')
+    def setup_simple_adapt_intent(self,
+                                  msg=create_vocab_msg('testKeyword', 'test')):
         self.intent_service.handle_register_vocab(msg)
 
         intent = IntentBuilder('skill:testIntent').require('testKeyword')
         msg = Message('register_intent', intent.__dict__)
         self.intent_service.handle_register_intent(msg)
 
+    def test_keyword_backwards_compatibility(self):
+        self.setup_simple_adapt_intent(
+            create_old_style_vocab_msg('testKeyword', 'test')
+        )
+
+        # Check that the intent is returned
+        msg = Message('intent.service.adapt.get', data={'utterance': 'test'})
+        self.intent_service.handle_get_adapt(msg)
+
+        reply = get_last_message(self.intent_service.bus)
+        self.assertEqual(reply.data['intent']['intent_type'],
+                         'skill:testIntent')
+
     def test_get_adapt_intent(self):
         self.setup_simple_adapt_intent()
         # Check that the intent is returned
@@ -300,8 +319,8 @@ def test_get_adapt_vocab_manifest(self):
         msg = Message('intent.service.adapt.vocab.manifest.get')
         self.intent_service.handle_vocab_manifest(msg)
         reply = get_last_message(self.intent_service.bus)
-        value = reply.data['vocab'][0]['start']
-        keyword = reply.data['vocab'][0]['end']
+        value = reply.data['vocab'][0]['entity_value']
+        keyword = reply.data['vocab'][0]['entity_type']
         self.assertEqual(keyword, 'testKeyword')
         self.assertEqual(value, 'test')
 

test/unittests/skills/test_intent_service_interface.py

@@ -25,13 +25,15 @@ def reset(self):
         self.results = []
 
 
-class FunctionTest(unittest.TestCase):
-    def check_emitter(self, result_list):
-        for type in self.emitter.get_types():
-            self.assertEqual(type, 'register_vocab')
-        self.assertEqual(sorted(self.emitter.get_results(),
-                                key=lambda d: sorted(d.items())),
-                         sorted(result_list, key=lambda d: sorted(d.items())))
+class KeywordRegistrationTest(unittest.TestCase):
+    def check_emitter(self, expected_message_data):
+        """Verify that the registration messages matches the expected."""
+        for msg_type in self.emitter.get_types():
+            self.assertEqual(msg_type, 'register_vocab')
+        self.assertEqual(
+            sorted(self.emitter.get_results(),
+                   key=lambda d: sorted(d.items())),
+            sorted(expected_message_data, key=lambda d: sorted(d.items())))
         self.emitter.reset()
 
     def setUp(self):
@@ -40,18 +42,40 @@ def setUp(self):
     def test_register_keyword(self):
         intent_service = IntentServiceInterface(self.emitter)
         intent_service.register_adapt_keyword('test_intent', 'test')
-        self.check_emitter([{'start': 'test', 'end': 'test_intent'}])
+        entity_data = {'entity_value': 'test', 'entity_type': 'test_intent'}
+        compatibility_data = {'start': 'test', 'end': 'test_intent'}
+        expected_data = {**entity_data, **compatibility_data}
+        self.check_emitter([expected_data])
 
     def test_register_keyword_with_aliases(self):
+        # TODO 22.02: Remove compatibility data
         intent_service = IntentServiceInterface(self.emitter)
         intent_service.register_adapt_keyword('test_intent', 'test',
                                               ['test2', 'test3'])
-        self.check_emitter([{'start': 'test', 'end': 'test_intent'},
-                            {'start': 'test2', 'end': 'test_intent',
-                             'alias_of': 'test'},
-                            {'start': 'test3', 'end': 'test_intent',
-                             'alias_of': 'test'},
-                            ])
+
+        entity_data = {'entity_value': 'test', 'entity_type': 'test_intent'}
+        compatibility_data = {'start': 'test', 'end': 'test_intent'}
+        expected_initial_vocab = {**entity_data, **compatibility_data}
+
+        alias_data = {
+            'entity_value': 'test2',
+            'entity_type': 'test_intent',
+            'alias_of': 'test'
+        }
+        alias_compatibility = {'start': 'test2', 'end': 'test_intent'}
+        expected_alias1 = {**alias_data, **alias_compatibility}
+
+        alias_data2 = {
+            'entity_value': 'test3',
+            'entity_type': 'test_intent',
+            'alias_of': 'test'
+        }
+        alias_compatibility2 = {'start': 'test3', 'end': 'test_intent'}
+        expected_alias2 = {**alias_data2, **alias_compatibility2}
+
+        self.check_emitter([expected_initial_vocab,
+                            expected_alias1,
+                            expected_alias2])
 
     def test_register_regex(self):
         intent_service = IntentServiceInterface(self.emitter)

test/unittests/skills/test_mycroft_skill.py

@@ -303,7 +303,14 @@ def test_register_vocab(self):
 
         # Normal vocaubulary
         self.emitter.reset()
-        expected = [{'start': 'hello', 'end': 'AHelloKeyword'}]
+        expected = [
+            {
+                'start': 'hello',
+                'end': 'AHelloKeyword',
+                'entity_value': 'hello',
+                'entity_type': 'AHelloKeyword'
+            }
+        ]
         s.register_vocabulary('hello', 'HelloKeyword')
         self.check_register_vocabulary(expected)
         # Regex