fix: distinguish JSON parse errors from validation errors in metadata

productmd/common.py

@@ -347,6 +347,10 @@ def load(self, f):
 
         :param f: file-like object or path to file
         :type f: file or str
+
+        :raises json.JSONDecodeError: if the file contains invalid JSON
+        :raises ValueError: if the data contains unsupported or invalid values
+        :raises TypeError: if the data contains fields with wrong types
         """
         with open_file_obj(f) as f:
             parser = self.parse_file(f)

productmd/compose.py

@@ -31,6 +31,7 @@
   compose.modules
 """
 
+import json
 import os
 
 import productmd.composeinfo
@@ -151,6 +152,8 @@ def _load_metadata(self, paths, cls):
         obj = cls()
         try:
             obj.load(path)
-        except ValueError as exc:
-            raise RuntimeError('%s can not be deserialized: %s.' % (path, exc))
+        except json.JSONDecodeError as exc:
+            raise RuntimeError(f"{path} is not valid JSON: {exc}") from exc
+        except (ValueError, TypeError) as exc:
+            raise RuntimeError(f"{path} metadata is not supported by this version of productmd: {exc}") from exc
         return obj

tests/test_compose.py

@@ -19,8 +19,11 @@
 
 import unittest
 
+import json
 import os
+import shutil
 import sys
+import tempfile
 import urllib.request
 
 DIR = os.path.dirname(__file__)
@@ -94,5 +97,103 @@ def test_read_composeinfo(self):
         variant = compose.info["Foo-Bar"]  # noqa: F841
 
 
+class TestComposeErrorHandling(unittest.TestCase):
+    """Test Compose._load_metadata error handling with v1.x format data.
+
+    Addresses https://github.com/release-engineering/productmd/issues/110:
+    error messages should distinguish invalid JSON from unsupported field values.
+    """
+
+    def setUp(self):
+        self.tmp_dir = tempfile.mkdtemp()
+        # Build a minimal compose directory: <tmp>/compose/metadata/
+        self.metadata_dir = os.path.join(self.tmp_dir, "compose", "metadata")
+        os.makedirs(self.metadata_dir)
+        # Write a valid v1.2 composeinfo.json so Compose.__init__ finds it
+        composeinfo = {
+            "header": {"type": "productmd.composeinfo", "version": "1.2"},
+            "payload": {
+                "compose": {
+                    "date": "20180101",
+                    "id": "Test-1.0-20180101.0",
+                    "respin": 0,
+                    "type": "production",
+                },
+                "release": {
+                    "name": "Test",
+                    "short": "Test",
+                    "version": "1.0",
+                    "type": "ga",
+                },
+                "variants": {},
+            },
+        }
+        with open(os.path.join(self.metadata_dir, "composeinfo.json"), "w") as f:
+            json.dump(composeinfo, f)
+
+    def tearDown(self):
+        shutil.rmtree(self.tmp_dir)
+
+    def _build_v1_images_data(self, image_type="dvd"):
+        return {
+            "header": {"type": "productmd.images", "version": "1.1"},
+            "payload": {
+                "compose": {
+                    "date": "20180101",
+                    "id": "Test-1.0-20180101.0",
+                    "respin": 0,
+                    "type": "production",
+                },
+                "images": {
+                    "Server": {
+                        "x86_64": [
+                            {
+                                "arch": "x86_64",
+                                "bootable": True,
+                                "checksums": {"sha256": "a" * 64},
+                                "disc_count": 1,
+                                "disc_number": 1,
+                                "format": "iso",
+                                "implant_md5": "b" * 32,
+                                "mtime": 1514764800,
+                                "path": "Server/x86_64/iso/test.iso",
+                                "size": 2147483648,
+                                "subvariant": "Server",
+                                "type": image_type,
+                                "volume_id": "Test-1.0",
+                            }
+                        ]
+                    }
+                },
+            },
+        }
+
+    def _write_images(self, content):
+        path = os.path.join(self.metadata_dir, "images.json")
+        with open(path, "w") as f:
+            if isinstance(content, str):
+                f.write(content)
+            else:
+                json.dump(content, f)
+
+    def test_invalid_json_mentions_not_valid_json(self):
+        self._write_images("{not valid json")
+        compose = Compose(self.tmp_dir)
+        with self.assertRaises(RuntimeError) as ctx:
+            compose.images
+        self.assertIn("not valid JSON", str(ctx.exception))
+
+    def test_unknown_image_type_error_message(self):
+        """Unknown type should say 'not supported' with the bad value, not 'not valid JSON'."""
+        self._write_images(self._build_v1_images_data(image_type="future-hologram"))
+        compose = Compose(self.tmp_dir)
+        with self.assertRaises(RuntimeError) as ctx:
+            compose.images
+        msg = str(ctx.exception)
+        self.assertIn("not supported", msg)
+        self.assertIn("future-hologram", msg)
+        self.assertNotIn("not valid JSON", msg)
+
+
 if __name__ == "__main__":
     unittest.main()

tests/test_images.py

@@ -19,6 +19,7 @@
 
 import unittest
 
+import json
 import os
 import sys
 import tempfile
@@ -384,5 +385,85 @@ def test_unique_id_enforcement(self):
         self.assertRaises(ValueError, im.add, "Server", "x86_64", i2)
 
 
+class TestErrorHandling(unittest.TestCase):
+    """Test error handling when loading v1.x image metadata with bad data.
+
+    Addresses https://github.com/release-engineering/productmd/issues/110
+    """
+
+    def setUp(self):
+        self.tmp_dir = tempfile.mkdtemp()
+
+    def tearDown(self):
+        shutil.rmtree(self.tmp_dir)
+
+    def _build_v1_images_data(self, image_type="dvd", image_format="iso"):
+        return {
+            "header": {"type": "productmd.images", "version": "1.1"},
+            "payload": {
+                "compose": {
+                    "date": "20180101",
+                    "id": "Test-1.0-20180101.0",
+                    "respin": 0,
+                    "type": "production",
+                },
+                "images": {
+                    "Server": {
+                        "x86_64": [
+                            {
+                                "arch": "x86_64",
+                                "bootable": True,
+                                "checksums": {"sha256": "a" * 64},
+                                "disc_count": 1,
+                                "disc_number": 1,
+                                "format": image_format,
+                                "implant_md5": "b" * 32,
+                                "mtime": 1514764800,
+                                "path": "Server/x86_64/iso/test.iso",
+                                "size": 2147483648,
+                                "subvariant": "Server",
+                                "type": image_type,
+                                "volume_id": "Test-1.0",
+                            }
+                        ]
+                    }
+                },
+            },
+        }
+
+    def test_unknown_type_raises_value_error_with_details(self):
+        data = self._build_v1_images_data(image_type="future-type")
+        images = Images()
+        with self.assertRaises(ValueError) as ctx:
+            images.deserialize(data)
+        self.assertIn("future-type", str(ctx.exception))
+
+    def test_unknown_format_raises_value_error_with_details(self):
+        data = self._build_v1_images_data(image_format="future-format")
+        images = Images()
+        with self.assertRaises(ValueError) as ctx:
+            images.deserialize(data)
+        self.assertIn("future-format", str(ctx.exception))
+
+    def test_invalid_json_raises_json_decode_error(self):
+        bad_file = os.path.join(self.tmp_dir, "images.json")
+        with open(bad_file, "w") as f:
+            f.write("{not valid json")
+        images = Images()
+        self.assertRaises(json.JSONDecodeError, images.load, bad_file)
+
+    def test_valid_json_unknown_type_via_load(self):
+        """load() with valid JSON but unknown type should raise ValueError, not JSONDecodeError."""
+        data = self._build_v1_images_data(image_type="future-type")
+        path = os.path.join(self.tmp_dir, "images.json")
+        with open(path, "w") as f:
+            json.dump(data, f)
+        images = Images()
+        with self.assertRaises(ValueError) as ctx:
+            images.load(path)
+        self.assertIn("future-type", str(ctx.exception))
+        self.assertNotIsInstance(ctx.exception, json.JSONDecodeError)
+
+
 if __name__ == "__main__":
     unittest.main()