Improve testing of gallery and behaviour if card data not set for a gallery provider

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:
+    def _set_card_types(
-        """Find the card types from each provider."""
+        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():
+        # cache initial list of keys as it may change in the loop
-            card_schema = thing.gallery_data_schema.schema()
+        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(
+                self.logger.error(
-                    f"Gallery data card for {thing.name} doesn't have a staticcard_type"
+                    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
 

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():
+def test_error_if_accessing_properties_before_started():
-    """Check that gallery_providing_things property errors before init."""
+    """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):