diff --git a/mycroft/skills/intent_service.py b/mycroft/skills/intent_service.py index 4a17a001272c..f3ab801fc3c7 100644 --- a/mycroft/skills/intent_service.py +++ b/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'] diff --git a/mycroft/skills/intent_service_interface.py b/mycroft/skills/intent_service_interface.py index 6fe69aadcf60..89a2f292be93 100644 --- a/mycroft/skills/intent_service_interface.py +++ b/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. diff --git a/mycroft/skills/intent_services/adapt_service.py b/mycroft/skills/intent_services/adapt_service.py index 9025d2b775a6..a1e46115833d 100644 --- a/mycroft/skills/intent_services/adapt_service.py +++ b/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. diff --git a/mycroft/skills/mycroft_skill/mycroft_skill.py b/mycroft/skills/mycroft_skill/mycroft_skill.py index 67bfac529f8a..4f331e27b6a5 100644 --- a/mycroft/skills/mycroft_skill/mycroft_skill.py +++ b/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. diff --git a/test/unittests/skills/test_intent_service.py b/test/unittests/skills/test_intent_service.py index 2363561f48ee..776c442926f4 100644 --- a/test/unittests/skills/test_intent_service.py +++ b/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') diff --git a/test/unittests/skills/test_intent_service_interface.py b/test/unittests/skills/test_intent_service_interface.py index f5d60c9024b2..0645b86857f2 100644 --- a/test/unittests/skills/test_intent_service_interface.py +++ b/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) diff --git a/test/unittests/skills/test_mycroft_skill.py b/test/unittests/skills/test_mycroft_skill.py index 0af99abf2899..bc60ca5c90a7 100644 --- a/test/unittests/skills/test_mycroft_skill.py +++ b/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