diff --git a/mycroft/skills/intent_service_interface.py b/mycroft/skills/intent_service_interface.py index ca4063f11f45..7608e3a19e50 100644 --- a/mycroft/skills/intent_service_interface.py +++ b/mycroft/skills/intent_service_interface.py @@ -35,6 +35,7 @@ class IntentServiceInterface: def __init__(self, bus=None): self.bus = bus self.registered_intents = [] + self.detached_intents = [] self.skill_id = self.__class__.__name__ def set_bus(self, bus): @@ -78,15 +79,26 @@ def register_adapt_intent(self, name, intent_parser): self.bus.emit(Message("register_intent", intent_parser.__dict__, context={"skill_id": self.skill_id})) self.registered_intents.append((name, intent_parser)) + self.detached_intents = [detached for detached in self.detached_intents + if detached[0] != name] def detach_intent(self, intent_name): """Remove an intent from the intent service. + The intent is saved in the list of detached intents for use when + re-enabling an intent. + Args: intent_name(str): Intent reference """ - self.bus.emit(Message("detach_intent", {"intent_name": intent_name}, - context={"skill_id": self.skill_id})) + name = intent_name.split(':')[1] + if name in self: + self.bus.emit(Message("detach_intent", + {"intent_name": intent_name}, + context={"skill_id": self.skill_id})) + self.detached_intents.append((name, self.get_intent(name))) + self.registered_intents = [pair for pair in self.registered_intents + if pair[0] != name] def set_adapt_context(self, context, word, origin): """Set an Adapt context. @@ -160,6 +172,8 @@ def __contains__(self, val): def get_intent(self, intent_name): """Get intent from intent_name. + This will find both enabled and disabled intents. + Args: intent_name (str): name to find. @@ -169,8 +183,10 @@ def get_intent(self, intent_name): for name, intent in self: if name == intent_name: return intent - else: - return None + for name, intent in self.detached_intents: + if name == intent_name: + return intent + return None class IntentQueryApi: diff --git a/mycroft/skills/mycroft_skill/mycroft_skill.py b/mycroft/skills/mycroft_skill/mycroft_skill.py index b6dd610c3038..4a2ef41a4939 100644 --- a/mycroft/skills/mycroft_skill/mycroft_skill.py +++ b/mycroft/skills/mycroft_skill/mycroft_skill.py @@ -962,12 +962,25 @@ def remove_event(self, name): def _register_adapt_intent(self, intent_parser, handler): """Register an adapt intent. + Will handle registration of anonymous Args: intent_parser: Intent object to parse utterance for the handler. handler (func): function to register with intent """ # Default to the handler's function name if none given + is_anonymous = not intent_parser.name name = intent_parser.name or handler.__name__ + if is_anonymous: + # Find a good name + original_name = name + nbr = 0 + while name in self.intent_service: + nbr += 1 + name = f'{original_name}{nbr}' + else: + if name in self.intent_service: + raise ValueError(f'The intent name {name} is already taken') + munge_intent_parser(intent_parser, name, self.skill_id) self.intent_service.register_adapt_intent(name, intent_parser) if handler: @@ -1057,7 +1070,7 @@ def handle_enable_intent(self, message): """Listener to enable a registered intent if it belongs to this skill. """ intent_name = message.data['intent_name'] - for (name, _) in self.intent_service: + for (name, _) in self.intent_service.detached_intents: if name == intent_name: return self.enable_intent(intent_name) @@ -1079,7 +1092,7 @@ def disable_intent(self, intent_name): bool: True if disabled, False if it wasn't registered """ if intent_name in self.intent_service: - LOG.debug('Disabling intent ' + intent_name) + LOG.info('Disabling intent ' + intent_name) name = '{}:{}'.format(self.skill_id, intent_name) self.intent_service.detach_intent(name) return True diff --git a/test/unittests/skills/test_mycroft_skill.py b/test/unittests/skills/test_mycroft_skill.py index 0af99abf2899..0aa7b240983f 100644 --- a/test/unittests/skills/test_mycroft_skill.py +++ b/test/unittests/skills/test_mycroft_skill.py @@ -27,7 +27,8 @@ from mycroft.messagebus.message import Message from mycroft.skills.skill_data import (load_regex_from_file, load_regex, load_vocabulary, read_vocab_file) -from mycroft.skills.core import MycroftSkill, resting_screen_handler +from mycroft.skills import (MycroftSkill, resting_screen_handler, + intent_handler) from mycroft.skills.intent_service import open_intent_envelope from test.util import base_config @@ -621,6 +622,23 @@ def test_speak_dialog_render_not_initialized(self): s.speak_dialog(key='key') +class TestIntentCollisions(unittest.TestCase): + def test_two_intents_with_same_name(self): + emitter = MockEmitter() + skill = SameIntentNameSkill() + skill.bind(emitter) + with self.assertRaises(ValueError): + skill.initialize() + + def test_two_anonymous_intent_decorators(self): + """Two anonymous intent handlers should be ok.""" + emitter = MockEmitter() + skill = SameAnonymousIntentDecoratorsSkill() + skill.bind(emitter) + skill._register_decorated() + self.assertEqual(len(skill.intent_service.registered_intents), 2) + + class _TestSkill(MycroftSkill): def __init__(self): super().__init__() @@ -698,3 +716,27 @@ def initialize(self): def handler(self, message): pass + + +class SameIntentNameSkill(_TestSkill): + """Test skill for duplicate intent namesr.""" + skill_id = 'A' + + def initialize(self): + intent = IntentBuilder('TheName').require('Keyword') + intent2 = IntentBuilder('TheName').require('Keyword') + self.register_intent(intent, self.handler) + self.register_intent(intent2, self.handler) + + def handler(self, message): + pass + + +class SameAnonymousIntentDecoratorsSkill(_TestSkill): + """Test skill for duplicate anonymous intent handlers.""" + skill_id = 'A' + + @intent_handler(IntentBuilder('').require('Keyword')) + @intent_handler(IntentBuilder('').require('OtherKeyword')) + def handler(self, message): + pass