From f22f3579340d7261fcb08b68ca00084d5fd75178 Mon Sep 17 00:00:00 2001 From: Lukas Puehringer Date: Mon, 20 Dec 2021 12:13:40 +0100 Subject: [PATCH] Metadata API: Remove 3 'update' methods + tests Remove ambiguous, unspecific, opinionated and trivial 'update' methods, which can be replaced by feasible one-liners that assign values directly to the object attribute to be *updated*. (see #1627 for details). Reasons to have these methods would be increased usability in terms of - reduced work - immediate feedback on invalid assignments However, given above described issues, the reasons against the methods as they are now seem to outweigh the reasons for them. Furthermore, it seems easier to re-add similar methods, which addressed these issues, after the upcoming 1.0.0 release than to remove or modify them. This patch also removes the corresponding tests as they become irrelevant (there is no need to test object assignment). In the case of the timestamp test, the removal also includes redundant test logic, which is already tested in `test_metadata_base`. Signed-off-by: Lukas Puehringer --- tests/test_api.py | 75 --------------------------------------------- tuf/api/metadata.py | 16 ---------- 2 files changed, 91 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 2ef44ed7..6accd680 100755 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -32,7 +32,6 @@ Delegations, Key, Metadata, - MetaFile, Root, Snapshot, TargetFile, @@ -297,56 +296,6 @@ def test_metadata_base(self) -> None: with self.assertRaises(ValueError): Metadata.from_dict(data) - def test_metadata_snapshot(self) -> None: - snapshot_path = os.path.join(self.repo_dir, "metadata", "snapshot.json") - snapshot = Metadata[Snapshot].from_file(snapshot_path) - - # Create a MetaFile instance representing what we expect - # the updated data to be. - hashes = { - "sha256": "c2986576f5fdfd43944e2b19e775453b96748ec4fe2638a6d2f32f1310967095" - } - fileinfo = MetaFile(2, 123, hashes) - - self.assertNotEqual( - snapshot.signed.meta["role1.json"].to_dict(), fileinfo.to_dict() - ) - snapshot.signed.update("role1", fileinfo) - self.assertEqual( - snapshot.signed.meta["role1.json"].to_dict(), fileinfo.to_dict() - ) - - def test_metadata_timestamp(self) -> None: - timestamp_path = os.path.join( - self.repo_dir, "metadata", "timestamp.json" - ) - timestamp = Metadata[Timestamp].from_file(timestamp_path) - - self.assertEqual(timestamp.signed.version, 1) - timestamp.signed.bump_version() - self.assertEqual(timestamp.signed.version, 2) - - self.assertEqual(timestamp.signed.expires, datetime(2030, 1, 1, 0, 0)) - timestamp.signed.bump_expiration() - self.assertEqual(timestamp.signed.expires, datetime(2030, 1, 2, 0, 0)) - timestamp.signed.bump_expiration(timedelta(days=365)) - self.assertEqual(timestamp.signed.expires, datetime(2031, 1, 2, 0, 0)) - - # Create a MetaFile instance representing what we expect - # the updated data to be. - hashes = { - "sha256": "0ae9664468150a9aa1e7f11feecb32341658eb84292851367fea2da88e8a58dc" - } - fileinfo = MetaFile(2, 520, hashes) - - self.assertNotEqual( - timestamp.signed.snapshot_meta.to_dict(), fileinfo.to_dict() - ) - timestamp.signed.update(fileinfo) - self.assertEqual( - timestamp.signed.snapshot_meta.to_dict(), fileinfo.to_dict() - ) - def test_metadata_verify_delegate(self) -> None: root_path = os.path.join(self.repo_dir, "metadata", "root.json") root = Metadata[Root].from_file(root_path) @@ -505,30 +454,6 @@ def test_is_target_in_pathpattern(self) -> None: DelegatedRole._is_target_in_pathpattern(targetpath, pathpattern) ) - def test_metadata_targets(self) -> None: - targets_path = os.path.join(self.repo_dir, "metadata", "targets.json") - targets = Metadata[Targets].from_file(targets_path) - - # Create a fileinfo dict representing the expected updated data. - filename = "file2.txt" - hashes = { - "sha256": "141f740f53781d1ca54b8a50af22cbf74e44c21a998fa2a8a05aaac2c002886b", - "sha512": "ef5beafa16041bcdd2937140afebd485296cd54f7348ecd5a4d035c09759608de467a7ac0eb58753d0242df873c305e8bffad2454aa48f44480f15efae1cacd0", - } - - fileinfo = TargetFile(length=28, hashes=hashes, path=filename) - - # Assert that data is not aleady equal - self.assertNotEqual( - targets.signed.targets[filename].to_dict(), fileinfo.to_dict() - ) - # Update an already existing fileinfo - targets.signed.update(fileinfo) - # Verify that data is updated - self.assertEqual( - targets.signed.targets[filename].to_dict(), fileinfo.to_dict() - ) - def test_targets_key_api(self) -> None: targets_path = os.path.join(self.repo_dir, "metadata", "targets.json") targets: Targets = Metadata[Targets].from_file(targets_path).signed diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 6adcf412..dadeafe4 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -993,11 +993,6 @@ def to_dict(self) -> Dict[str, Any]: res_dict["meta"] = {"snapshot.json": self.snapshot_meta.to_dict()} return res_dict - # Modification. - def update(self, snapshot_meta: MetaFile) -> None: - """Assigns passed info about snapshot metadata.""" - self.snapshot_meta = snapshot_meta - class Snapshot(Signed): """A container for the signed part of snapshot metadata. @@ -1049,12 +1044,6 @@ def to_dict(self) -> Dict[str, Any]: snapshot_dict["meta"] = meta_dict return snapshot_dict - # Modification. - def update(self, rolename: str, role_info: MetaFile) -> None: - """Assigns passed (delegated) targets role info to meta dict.""" - metadata_fn = f"{rolename}.json" - self.meta[metadata_fn] = role_info - class DelegatedRole(Role): """A container with information about a delegated role. @@ -1463,11 +1452,6 @@ def to_dict(self) -> Dict[str, Any]: targets_dict["delegations"] = self.delegations.to_dict() return targets_dict - # Modification. - def update(self, fileinfo: TargetFile) -> None: - """Assigns passed target file info to meta dict.""" - self.targets[fileinfo.path] = fileinfo - def add_key(self, role: str, key: Key) -> None: """Adds new signing key for delegated role 'role'.