From b62be770bf2a24c06d2e0f3ad976b6a39127b137 Mon Sep 17 00:00:00 2001 From: guillermodotn Date: Tue, 9 Jun 2026 19:37:40 +0200 Subject: [PATCH] fix: distinguish JSON parse errors from validation errors in metadata loading Assisted-by: Claude Opus --- productmd/common.py | 4 ++ productmd/compose.py | 7 ++- tests/test_compose.py | 101 ++++++++++++++++++++++++++++++++++++++++++ tests/test_images.py | 81 +++++++++++++++++++++++++++++++++ 4 files changed, 191 insertions(+), 2 deletions(-) diff --git a/productmd/common.py b/productmd/common.py index dc9fd7a..4cfa7b4 100644 --- a/productmd/common.py +++ b/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) diff --git a/productmd/compose.py b/productmd/compose.py index ce3472d..3034107 100644 --- a/productmd/compose.py +++ b/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 diff --git a/tests/test_compose.py b/tests/test_compose.py index 2540a49..5e0cee3 100755 --- a/tests/test_compose.py +++ b/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: /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() diff --git a/tests/test_images.py b/tests/test_images.py index 16e23da..12bd500 100755 --- a/tests/test_images.py +++ b/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()