Simplified _log_metatada logic

- Single method (called on start but can be called manually)
- Image format and dimension logged for illustration only
- all printable meta logged
- fallback for other bytes and unexpected types
- Tests for all use cases
- Changed test to only test log_format directly

Author: rgaudin

CHANGELOG.md

@@ -7,12 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- `zim.creator.Creator._log_metadata()` to log (DEBUG) all metadata set on `_metadata` (prior to start())
+
 ### Changed
 
 - Migrate the **VideoWebmLow** and **VideoWebmHigh** presets to VP9 for smaller file size #79
   - New preset versions are v3 and v2 respectively
 - Simplify type annotations by replacing Union and Optional with pipe character ("|") for improved readability and clarity
-- Add debug logging of metadata fields
+- Calling `Creator._log_metadata()` on `Creator.start()` if running in DEBUG
 
 ### Fixed
 - Add back the `--runinstalled` flag for test execution to allow smooth testing on other build chains (#139)

src/zimscraperlib/zim/creator.py

@@ -150,33 +150,53 @@ def config_indexing(
         self.__indexing_configured = True
         return self
 
-    def _is_illustration_metadata_name(self, name: str) -> bool:
-        """Return True if name is a valid illustration metadata name"""
-        return name.startswith("Illustration_")
-
-    def _get_illustration_metadata_details(
-        self,
-        value: bytes,
-    ) -> str | None:
-        """Return image format for debug logging of illustration metadata"""
-        try:
-            with PIL.Image.open(io.BytesIO(value)) as img:
-                if img is not None:
-                    return f"{img.format} {img.size[0]}x{img.size[1]}"
-        except PIL.UnidentifiedImageError as e:
-            return f"Image format issue: {e}"
-        return f"Unknown image format, {len(value)} bytes"
-
     def _log_metadata(self):
+        """Log (DEBUG) all metadata set on (_metadata ~ config_metadata())
+
+        Does not log metadata set post-start (via add_metadata())"""
         for name, value in sorted(self._metadata.items()):
-            if self._is_illustration_metadata_name(name) and isinstance(value, bytes):
-                illus_md = self._get_illustration_metadata_details(value)
-            else:
-                illus_md = None
-            logger.debug(f"Metadata: {name} = {(illus_md if illus_md else value)}")
+            # illustration mandates an Image
+            if re.match(r"^Illustration_(\d+)x(\d+)@(\d+)$", name):
+                try:
+                    with PIL.Image.open(io.BytesIO(value)) as img:
+                        logger.debug(
+                            f"Metadata: {name} is a {len(value)} bytes "
+                            f"{img.size[0]}x{img.size[1]}px {img.format} Image"
+                        )
+                except Exception:
+                    logger.debug(
+                        f"Metadata: {name} is a {len(value)} bytes "
+                        f"{get_content_mimetype(value[:64])} blob "
+                        "not recognized as an Image"
+                    )
+                continue
+
+            # bytes are either encoded string or arbitrary data
+            if isinstance(value, bytes):
+                mimetype = get_content_mimetype(value[:64])
+                if not mimetype.startswith("text/"):
+                    logger.debug(
+                        f"Metadata: {name} is a {len(value)} bytes {mimetype} blob"
+                    )
+                    continue
+                try:
+                    logger.debug(f"Metadata: {name} = {value.decode('UTF-8')}")
+                except Exception:
+                    logger.debug(
+                        f"Metadata: {name} is a {len(value)} bytes {mimetype} blob"
+                    )
+                continue
+
+            # rest is either printable or unexpected
+            try:
+                logger.debug(f"Metadata: {name} = {value!s}")
+            except Exception:
+                logger.debug(
+                    f"Metadata: {name} is unexpected data type: {type(value).__name__}"
+                )
 
     def start(self):
-        if logger.isEnabledFor(logging.DEBUG):
+        if logger.isEnabledFor(logging.DEBUG):  # pragma: no cover
             self._log_metadata()
 
         if not all(self._metadata.get(key) for key in MANDATORY_ZIM_METADATA_KEYS):

tests/zim/test_zim_creator.py

@@ -567,7 +567,7 @@ def test_start_logs_metadata_log_contents(mocked_logger, png_image, tags, tmp_pa
     fpath = tmp_path / "test_config.zim"
     with open(png_image, "rb") as fh:
         png_data = fh.read()
-    creator = Creator(fpath, "").config_metadata(
+    creator = Creator(fpath, "", disable_metadata_checks=True).config_metadata(
         Name="wikipedia_fr_football",
         Title="English Wikipedia",
         Creator="English speaking Wikipedia contributors",
@@ -585,18 +585,38 @@ def test_start_logs_metadata_log_contents(mocked_logger, png_image, tags, tmp_pa
         Illustration_48x48_at_1=png_data,
         TestMetadata="Test Metadata",
     )
-    creator.start()
-    creator.finish()
+
+    class NotPrintable:
+        def __str__(self):
+            raise ValueError("Not printable I said")
+
+    creator._metadata.update(
+        {
+            "Illustration_96x96@1": b"%PDF-1.5\n%\xe2\xe3\xcf\xd3",
+            "Chars": b"\xc5\xa1\xc9\x94\xc9\x9b",
+            "Chars-32": b"\xff\xfe\x00\x00a\x01\x00\x00T\x02\x00\x00[\x02\x00\x00",
+            "Video": b"\x00\x00\x00 ftypisom\x00\x00\x02\x00isomiso2avc1mp41\x00",
+            "Toupie": NotPrintable(),
+        }
+    )
+    creator._log_metadata()
+    # /!\ this must be alpha sorted
     mocked_logger.debug.assert_has_calls(
         [
+            call("Metadata: Chars = šɔɛ"),
+            call("Metadata: Chars-32 is a 16 bytes text/plain blob"),
             call("Metadata: Creator = English speaking Wikipedia contributors"),
             call("Metadata: Date = 2009-11-21"),
             call(
                 "Metadata: Description = All articles (without images) from the "
                 "english Wikipedia"
             ),
             call("Metadata: Flavour = nopic"),
-            call("Metadata: Illustration_48x48@1 = PNG 48x48"),
+            call("Metadata: Illustration_48x48@1 is a 3274 bytes 48x48px PNG Image"),
+            call(
+                "Metadata: Illustration_96x96@1 is a 14 bytes "
+                "application/pdf blob not recognized as an Image"
+            ),
             call("Metadata: Language = eng"),
             call("Metadata: License = CC-BY"),
             call(
@@ -612,6 +632,8 @@ def test_start_logs_metadata_log_contents(mocked_logger, png_image, tags, tmp_pa
             call(f"Metadata: Tags = {tags}"),
             call("Metadata: TestMetadata = Test Metadata"),
             call("Metadata: Title = English Wikipedia"),
+            call("Metadata: Toupie is unexpected data type: NotPrintable"),
+            call("Metadata: Video is a 33 bytes video/mp4 blob"),
         ]
     )