fix: key resource caches by resource directory

ovos_workshop/skills/auto_translatable.py

@@ -68,12 +68,13 @@ def _load_lang(self, root_directory=None, lang=None):
         """
         lang = lang or self.internal_language  # self.lang in base class
         root_directory = root_directory or self.res_dir
-        if lang not in self._lang_resources:
-            self._lang_resources[lang] = SkillResources(root_directory, lang,
-                                                        skill_id=self.skill_id)
+        key = (root_directory, lang)
+        if key not in self._lang_resources:
+            self._lang_resources[key] = SkillResources(root_directory, lang,
+                                                       skill_id=self.skill_id)
             # see OVOSSkill.load_lang - same auto entity-file discovery
-            self._auto_register_entity_files(lang)
-        return self._lang_resources[lang]
+            self._auto_register_entity_files(lang, self._lang_resources[key])
+        return self._lang_resources[key]
 
     def detect_language(self, utterance: str):
         """

ovos_workshop/skills/ovos.py

@@ -187,11 +187,11 @@ def __init__(self, name: Optional[str] = None,
         # loaded lang file resources
         self._lang_resources = {}
 
-        # OVOS-INTENT-3 §auto-entity: tracks which languages already had
-        # their locale .entity files auto-discovered and registered, so a
-        # repeated load_lang() for the same lang (reload/retrain) never
-        # double-emits the same entity registrations.
-        self._auto_registered_entity_langs = set()
+        # OVOS-INTENT-3 §auto-entity: tracks which (resource directory,
+        # lang) pairs already had their locale .entity files auto-discovered
+        # and registered, so a repeated load_lang() for the same pair
+        # (reload/retrain) never double-emits the same entity registrations.
+        self._auto_registered_entity_dirs = set()
         # OVOS-INTENT-3 §auto-entity: tracks (lang -> {entity_file, ...})
         # already sent to the intent service, keyed by the bare entity file
         # name (no extension). Shared by auto-discovery AND the explicit
@@ -611,18 +611,19 @@ def load_lang(self, root_directory: Optional[str] = None,
         """
         lang = standardize_lang(lang or self.lang)
         root_directory = root_directory or self.res_dir
-        if lang not in self._lang_resources:
-            self._lang_resources[lang] = SkillResources(root_directory, lang,
-                                                        skill_id=self.skill_id)
+        key = (root_directory, lang)
+        if key not in self._lang_resources:
+            self._lang_resources[key] = SkillResources(root_directory, lang,
+                                                       skill_id=self.skill_id)
             # OVOS-INTENT-3 §auto-entity: register every shipped .entity file
             # for this lang the first time its resources are loaded, so it
             # reaches the matcher in the same batch as (and strictly before)
             # any .intent template that references it. register_intent_file
             # calls load_lang() before building/emitting its own template,
             # so hooking the cache-miss path here guarantees ordering without
             # requiring skill authors to call register_entity_file() at all.
-            self._auto_register_entity_files(lang)
-        return self._lang_resources[lang]
+            self._auto_register_entity_files(lang, self._lang_resources[key])
+        return self._lang_resources[key]
 
     def load_dialog_files(self, root_directory: Optional[str] = None):
         """
@@ -1633,7 +1634,8 @@ def _register_entity_file_for_lang(self, entity_file: str, lang: str,
         self.intent_service.register_entity(name, samples, lang,
                                             blacklisted_words=blacklist)
 
-    def _auto_register_entity_files(self, lang: str):
+    def _auto_register_entity_files(self, lang: str,
+                                    resources: Optional[SkillResources] = None):
         """
         Auto-discover and register every ".entity" file shipped in this
         skill's locale resources for `lang`.
@@ -1656,23 +1658,24 @@ def _auto_register_entity_files(self, lang: str):
 
         Idempotency: guarded twice - `load_lang` only calls this on a
         cache-miss (one call per lang per skill instance in normal use),
-        and `_auto_registered_entity_langs` guards direct/repeated calls
-        (e.g. tests, or a future reload path) from double-emitting.
+        and `_auto_registered_entity_dirs` guards direct/repeated calls
+        (e.g. tests, or a future reload path) from double-emitting. The
+        guard is per (resource directory, lang): a skill pointed at a new
+        `res_dir` registers that directory's entity files.
 
         Can be disabled entirely via the "skills" section of mycroft.conf:
             {"skills": {"auto_register_entity_files": false}}
         """
-        if lang in self._auto_registered_entity_langs:
+        resources = resources or self.load_lang(lang=lang)
+        key = (resources.skill_directory, lang)
+        if key in self._auto_registered_entity_dirs:
             return
-        self._auto_registered_entity_langs.add(lang)
+        self._auto_registered_entity_dirs.add(key)
 
         if not self.config_core.get("skills", {}).get(
                 "auto_register_entity_files", True):
             return
 
-        resources = self._lang_resources.get(lang)
-        if resources is None:
-            return
         entity_dir = resources.types.entity.base_directory
         if not entity_dir or not Path(entity_dir).is_dir():
             return
@@ -2526,7 +2529,7 @@ def voc_list(self, voc_filename: str,
         @return: list of string vocab options
         """
         lang = standardize_lang(lang or self.lang)
-        cache_key = lang + voc_filename
+        cache_key = (self.res_dir, lang, voc_filename)
 
         if cache_key not in self._voc_cache:
             vocab = self.resources.load_vocabulary_file(voc_filename)

test/unittests/skills/test_auto_translatable.py

@@ -1,4 +1,8 @@
+import os
+import shutil
+import tempfile
 import unittest
+from os.path import join
 
 from ovos_workshop.skills.fallback import FallbackSkill
 from ovos_workshop.skills.ovos import OVOSSkill
@@ -12,6 +16,22 @@ def test_00_init(self):
         self.assertIsInstance(self.test_skill, self.UniversalSkill)
         self.assertIsInstance(self.test_skill, OVOSSkill)
 
+    def test_load_lang_honours_root_directory(self):
+        skill = self.UniversalSkill()
+        lang = skill.internal_language
+        skill._load_lang(lang=lang)
+
+        other = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, other)
+        voc_dir = join(other, "locale", lang)
+        os.makedirs(voc_dir)
+        with open(join(voc_dir, "condition.voc"), "w") as f:
+            f.write("sunny\n")
+
+        resources = skill._load_lang(root_directory=other, lang=lang)
+        self.assertEqual(resources.load_vocabulary_file("condition"),
+                         [["sunny"]])
+
     # TODO: Test other class methods
 
 

test/unittests/skills/test_base.py

@@ -14,6 +14,7 @@
 import json
 import os
 import shutil
+import tempfile
 import unittest
 
 from logging import Logger
@@ -280,7 +281,6 @@ def test_voc_list(self):
 
     def test_voc_match(self):
         skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.res_dir = join(dirname(__file__), "test_locale")
         lang = "en-US"
 
@@ -289,9 +289,48 @@ def test_voc_match(self):
         self.assertFalse(skill.voc_match("it is nice outside", "condition",
                                           lang=lang))
 
+    def test_voc_match_after_res_dir_reassigned(self):
+        skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id)
+        skill.res_dir = join(dirname(__file__), "test_locale")
+        lang = "en-US"
+        self.assertTrue(skill.voc_match("it is hot outside", "condition",
+                                        lang=lang))
+
+        other_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, other_dir)
+        voc_dir = join(other_dir, "locale", lang)
+        os.makedirs(voc_dir)
+        with open(join(voc_dir, "condition.voc"), "w") as f:
+            f.write("sunny\n")
+        skill.res_dir = other_dir
+
+        self.assertTrue(skill.voc_match("it is sunny outside", "condition",
+                                        lang=lang))
+        self.assertFalse(skill.voc_match("it is hot outside", "condition",
+                                         lang=lang))
+
+    def test_load_lang_honours_root_directory(self):
+        skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id)
+        skill.res_dir = join(dirname(__file__), "test_locale")
+        lang = "en-US"
+        self.assertEqual(
+            skill.load_lang(lang=lang).load_vocabulary_file("condition"),
+            [["hot"], ["cold"], ["freezing"]])
+
+        other_dir = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, other_dir)
+        voc_dir = join(other_dir, "locale", lang)
+        os.makedirs(voc_dir)
+        with open(join(voc_dir, "condition.voc"), "w") as f:
+            f.write("sunny\n")
+
+        self.assertEqual(
+            skill.load_lang(root_directory=other_dir,
+                            lang=lang).load_vocabulary_file("condition"),
+            [["sunny"]])
+
     def test_voc_match_span(self):
         skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.res_dir = join(dirname(__file__), "test_locale")
         lang = "en-US"
 
@@ -382,7 +421,6 @@ def handler(self, message):
         skill_cls = type("_IntentFileContextGateSkill", (OVOSSkill,),
                          {"handle_time_intent": handler})
         skill = skill_cls(bus=self.bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.intent_service = Mock()
         skill.res_dir = join(dirname(__file__), "test_locale")
         skill.config_core["lang"] = "en-US"
@@ -516,7 +554,6 @@ def test_register_intent_adapt_context_gating_reaches_intent_service(self):
         from ovos_spec_tools import IntentBuilder
 
         skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.intent_service = Mock()
         skill.intent_service.intent_names = []
         skill.intent_service.intent_is_detached.return_value = False
@@ -532,7 +569,6 @@ def test_register_intent_adapt_context_gating_reaches_intent_service(self):
 
     def test_register_intent_file(self):
         skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.intent_service = Mock()
         skill.res_dir = join(dirname(__file__), "test_locale")
         en_intent_file = join(skill.res_dir, "locale", "en-US", "time.intent")
@@ -566,7 +602,6 @@ def test_register_intent_file(self):
 
     def test_register_intent_file_with_context_gating(self):
         skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.intent_service = Mock()
         skill.res_dir = join(dirname(__file__), "test_locale")
         en_samples = ["what time is it"]
@@ -589,7 +624,6 @@ def test_register_intent_file_binds_the_canonical_event_only(self):
         # workshop must neither register nor listen on the suffixed twin —
         # that compat belongs to ovos-spec-tools at the bus layer.
         skill = OVOSSkill(bus=FakeBus(), skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.intent_service = Mock()
         skill.res_dir = join(dirname(__file__), "test_locale")
         skill.config_core["lang"] = "en-US"
@@ -608,7 +642,6 @@ def test_register_intent_file_registers_the_canonical_name(self):
         # the name that goes to the intent service (and so onto the wire in
         # the INTENT-4 registration payload) carries no authoring extension
         skill = OVOSSkill(bus=FakeBus(), skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.intent_service = Mock()
         skill.res_dir = join(dirname(__file__), "test_locale")
         skill.config_core["lang"] = "en-US"
@@ -628,7 +661,6 @@ def test_no_suffixed_topic_originates_from_workshop(self):
         bus.on("message", lambda m: emitted.append(
             json.loads(m)["type"] if isinstance(m, str) else m.msg_type))
         skill = OVOSSkill(bus=bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.res_dir = join(dirname(__file__), "test_locale")
         skill.config_core["lang"] = "en-US"
         skill.config_core["secondary_langs"] = []
@@ -646,7 +678,6 @@ def test_register_intent_file_canonical_topic_fires_handler(self):
         # the registered handler, matching how a pipeline dispatches
         bus = FakeBus()
         skill = OVOSSkill(bus=bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.intent_service = Mock()
         skill.res_dir = join(dirname(__file__), "test_locale")
         skill.config_core["lang"] = "en-US"
@@ -710,7 +741,6 @@ def test_dual_registration_does_not_double_fire(self):
     def test_disable_intent_removes_the_canonical_event(self):
         # the author still names the intent by its authoring file
         skill = OVOSSkill(bus=FakeBus(), skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.intent_service = Mock()
         skill.intent_service.__contains__ = Mock(return_value=True)
         skill.res_dir = join(dirname(__file__), "test_locale")
@@ -729,7 +759,6 @@ def test_disable_intent_removes_the_canonical_event(self):
 
     def test_register_entity_file(self):
         skill = OVOSSkill(bus=self.bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.intent_service = Mock()
         skill.res_dir = join(dirname(__file__), "test_locale")
         en_file = join(skill.res_dir, "locale", "en-US", "dow.entity")
@@ -775,7 +804,6 @@ def test_disable_intent(self):
 
         bus = FakeBus()
         skill = OVOSSkill(bus=bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.res_dir = join(dirname(__file__), "test_locale")
         skill.config_core["lang"] = "en-US"
         skill.config_core["secondary_langs"] = []
@@ -823,7 +851,6 @@ def test_disable_enable_intent_canonical_spelling_round_trip(self):
 
         bus = FakeBus()
         skill = OVOSSkill(bus=bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.res_dir = join(dirname(__file__), "test_locale")
         skill.config_core["lang"] = "en-US"
         skill.config_core["secondary_langs"] = []
@@ -906,7 +933,6 @@ def test_handle_disable_intent(self):
 
         bus = FakeBus()
         skill = OVOSSkill(bus=bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.res_dir = join(dirname(__file__), "test_locale")
         skill.config_core["lang"] = "en-US"
         skill.config_core["secondary_langs"] = []
@@ -928,7 +954,6 @@ def test_handle_enable_intent(self):
 
         bus = FakeBus()
         skill = OVOSSkill(bus=bus, skill_id=self.skill_id)
-        skill._lang_resources = dict()
         skill.res_dir = join(dirname(__file__), "test_locale")
         skill.config_core["lang"] = "en-US"
         skill.config_core["secondary_langs"] = []

test/unittests/test_abstract_app.py

@@ -61,7 +61,13 @@ def test_settings_path(self):
         remove(test_app.settings_path)
         remove(test_skill.settings_path)
 
-    @patch("ovos_workshop.app.OVOSSkill.default_shutdown")
+    # autospec records the instance, so this asserts WHICH skill was shut
+    # down. The patch replaces the method on OVOSSkill itself, and skills
+    # built by earlier tests stay alive on the shared FakeBus until the
+    # generational collector takes them, which can happen at any point
+    # inside this test; counting calls therefore counts other skills'
+    # finalisers too.
+    @patch("ovos_workshop.app.OVOSSkill.default_shutdown", autospec=True)
     def test_default_shutdown(self, skill_shutdown):
         real_clear_intents = self.app.clear_intents
         real_bus_close = self.app.bus.close
@@ -70,7 +76,13 @@ def test_default_shutdown(self, skill_shutdown):
         self.app.default_shutdown()
         self.app.clear_intents.assert_called_once()
         self.app.bus.close.assert_not_called()  # No dedicated bus here
-        skill_shutdown.assert_called_once()
+        # count only OUR app's shutdowns: identity keeps other skills'
+        # finalisers out, the count keeps a double shutdown in
+        ours = [c for c in skill_shutdown.call_args_list
+                if c.args[0] is self.app]
+        self.assertEqual(len(ours), 1,
+                         f"expected exactly one OVOSSkill.default_shutdown "
+                         f"for this app, got {len(ours)}")
 
         self.app.bus.close = real_bus_close
         self.app.clear_intents = real_clear_intents