diff --git a/src/openflexure_microscope_server/things/gallery.py b/src/openflexure_microscope_server/things/gallery.py index 532fff81..23f4f1bd 100644 --- a/src/openflexure_microscope_server/things/gallery.py +++ b/src/openflexure_microscope_server/things/gallery.py @@ -44,7 +44,7 @@ class GalleryThing(lt.Thing): all_ofm_things: Mapping[str, OFMThing] = lt.thing_slot() - _gallery_providing_things: Optional[Mapping[str, GalleryCompatibleThing]] = None + _gallery_providing_things: Optional[dict[str, GalleryCompatibleThing]] = None _card_types: Optional[list[str]] = None _card_type_map: dict[str, str] = {} @@ -60,7 +60,6 @@ class GalleryThing(lt.Thing): def __enter__(self) -> Self: """Check for all gallery providing things on server startup.""" self._set_gallery_providers() - self._set_card_types() return self def _set_gallery_providers(self) -> None: @@ -86,23 +85,36 @@ class GalleryThing(lt.Thing): # Cast the type of each thing to "GalleryCompatibleThing" as other Things have # been popped. self._gallery_providing_things = cast( - Mapping[str, GalleryCompatibleThing], gallery_providers + dict[str, GalleryCompatibleThing], gallery_providers ) + self._set_card_types(self._gallery_providing_things) - def _set_card_types(self) -> None: - """Find the card types from each provider.""" + def _set_card_types( + self, gallery_providers: dict[str, GalleryCompatibleThing] + ) -> None: + """Find the card types from each provider. + + :param gallery_providers: The dictionary of gallery providing things. If card + data cannot be extracted for a Thing it will be popped from the provider + dictionary. + """ card_types: list[str] = [] - for thing in self.gallery_providing_things.values(): - card_schema = thing.gallery_data_schema.schema() + # cache initial list of keys as it may change in the loop + keys = list(gallery_providers.keys()) + for key in keys: + card_schema = gallery_providers[key].gallery_data_schema.schema() props = card_schema["properties"] if "card_type" not in props or "const" not in props["card_type"]: - raise TypeError( - f"Gallery data card for {thing.name} doesn't have a staticcard_type" + self.logger.error( + f"Data from {key} cannot be shown in gallery as data card doesn't " + "have a static card_type." ) + gallery_providers.pop(key) + continue card_type = props["card_type"]["const"] if card_type not in card_types: card_types.append(card_type) - self._card_type_map[thing.name] = card_type + self._card_type_map[key] = card_type self._card_types = card_types diff --git a/tests/unit_tests/test_gallery.py b/tests/unit_tests/test_gallery.py index 7d0d465c..2a099185 100644 --- a/tests/unit_tests/test_gallery.py +++ b/tests/unit_tests/test_gallery.py @@ -19,14 +19,19 @@ from openflexure_microscope_server.things.gallery import ( from ..shared_utils.lt_test_utils import LabThingsTestEnv -def test_error_if_accessing_gallery_things_before_started(): - """Check that gallery_providing_things property errors before init.""" +def test_error_if_accessing_properties_before_started(): + """Check that unfefined properties errors before enter.""" thing = create_thing_without_server(GalleryThing) with pytest.raises( RuntimeError, match="Cannot access gallery_providing_things before server has started.", ): thing.gallery_providing_things + with pytest.raises( + RuntimeError, + match="Cannot access card_types before server has started.", + ): + thing.card_types class MockGalleryData(BaseModel): @@ -90,6 +95,31 @@ class BadGallerySource(OFMThing): """Mock deleting all the data.""" +class BadMockGalleryData(BaseModel): + """Mock gallery data without a card type.""" + + mock: str + foobar: int + + +class BadGallerySource2(OFMThing): + """An OFMThing that defines enough to show in the gallery with bad card data. + + Matches the protocol but will be rejected when setting card types. + """ + + show_data_in_gallery = True + + gallery_data_schema: type[BaseModel] = BadMockGalleryData + + def get_data_for_gallery(self) -> list[BaseModel]: + """Return a list of Mock Gallery Data.""" + return [BadMockGalleryData(mock="mock", foobar="foobar")] + + def delete_all_gallery_items(self) -> None: + """Mock deleting all the data.""" + + def test_gallery_compatible_protocol(): """Check the gallery compatible protocol detects compatible classes only.""" # Minimal class has the gallery specific methods but is not an OFMThing. @@ -127,18 +157,23 @@ def test_gallery_thing_finds_all_providers(simulation_test_env): assert camera in gallery.gallery_providing_things.values() assert smart_scan in gallery.gallery_providing_things.values() + # Also check the card types: + assert gallery.card_types == ["Capture", "Scan"] + assert gallery._card_type_map == {"camera": "Capture", "smart_scan": "Scan"} + # Also check the runtime checkable protocol GalleryCompatibleThing works directly # when called with isinstance. assert not isinstance(snake_workflow, GalleryCompatibleThing) assert isinstance(smart_scan, GalleryCompatibleThing) -def test_gallery_logs_for_bad_providers(caplog): +@pytest.mark.parametrize("bad_source_class", [BadGallerySource, BadGallerySource2]) +def test_gallery_logs_for_bad_providers(bad_source_class, caplog): """Test that an error is logged if a gallery source doesn't match the protocol.""" # Create a config with the minimal Thing and the bad Thing things = { "minimal_thing": MinimalGallerySource, - "bad_thing": BadGallerySource, + "bad_thing": bad_source_class, "gallery": GalleryThing, } with caplog.at_level(logging.INFO):