From 3771a77ffefa10f0eae9fed89398ac81e57f2d27 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Tue, 30 Mar 2021 17:39:10 +0300 Subject: [PATCH 1/6] New API: Add MetaFile class In the top-level metadata classes, there are complex attributes such as "meta" in Targets and Snapshot, "key" and "roles" in Root etc. We want to represent those complex attributes with a class to allow easier verification and support for metadata with unrecognized fields. For more context read ADR 0004 and ADR 0008 in the docs/adr folder. Additionally, after adding the MetaFile class, when we create an object we are now calling from dict twice - one for the main class (Timestamp, Snapshot) and one for the pacticular complex attribute - MetaFile.from_dict(). Given that the "from_dict" methods have the side effect of destroying the given dictionary, we would need to start using deepcopy() for our tests. Signed-off-by: Martin Vrachev --- tests/test_api.py | 57 +++++++++++--------- tuf/api/metadata.py | 127 +++++++++++++++++++++++++++----------------- 2 files changed, 111 insertions(+), 73 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index c0c40ba405..71c4eee62d 100755 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -29,6 +29,7 @@ Targets, Key, Role, + MetaFile, Delegations, DelegatedRole, ) @@ -246,28 +247,31 @@ def test_metadata_snapshot(self): self.repo_dir, 'metadata', 'snapshot.json') snapshot = Metadata.from_file(snapshot_path) - # Create a dict representing what we expect the updated data to be - fileinfo = copy.deepcopy(snapshot.signed.meta) + # Create a MetaFile instance representing what we expect + # the updated data to be. hashes = {'sha256': 'c2986576f5fdfd43944e2b19e775453b96748ec4fe2638a6d2f32f1310967095'} - fileinfo['role1.json']['version'] = 2 - fileinfo['role1.json']['hashes'] = hashes - fileinfo['role1.json']['length'] = 123 + fileinfo = MetaFile(2, 123, hashes) - - self.assertNotEqual(snapshot.signed.meta, fileinfo) + self.assertNotEqual( + snapshot.signed.meta['role1.json'].to_dict(), fileinfo.to_dict() + ) snapshot.signed.update('role1', 2, 123, hashes) - self.assertEqual(snapshot.signed.meta, fileinfo) + self.assertEqual( + snapshot.signed.meta['role1.json'].to_dict(), fileinfo.to_dict() + ) # Update only version. Length and hashes are optional. snapshot.signed.update('role2', 3) - fileinfo['role2.json'] = {'version': 3} - self.assertEqual(snapshot.signed.meta, fileinfo) + fileinfo = MetaFile(3) + self.assertEqual( + snapshot.signed.meta['role2.json'].to_dict(), fileinfo.to_dict() + ) # Test from_dict and to_dict without hashes and length. snapshot_dict = snapshot.to_dict() - test_dict = snapshot_dict['signed'].copy() - del test_dict['meta']['role1.json']['length'] - del test_dict['meta']['role1.json']['hashes'] + del snapshot_dict['signed']['meta']['role1.json']['length'] + del snapshot_dict['signed']['meta']['role1.json']['hashes'] + test_dict = copy.deepcopy(snapshot_dict['signed']) snapshot = Snapshot.from_dict(test_dict) self.assertEqual(snapshot_dict['signed'], snapshot.to_dict()) @@ -295,28 +299,33 @@ def test_metadata_timestamp(self): timestamp.signed.bump_expiration(delta) self.assertEqual(timestamp.signed.expires, datetime(2036, 1, 3, 0, 0)) + # Create a MetaFile instance representing what we expect + # the updated data to be. hashes = {'sha256': '0ae9664468150a9aa1e7f11feecb32341658eb84292851367fea2da88e8a58dc'} - fileinfo = copy.deepcopy(timestamp.signed.meta['snapshot.json']) - fileinfo['hashes'] = hashes - fileinfo['version'] = 2 - fileinfo['length'] = 520 + fileinfo = MetaFile(2, 520, hashes) - self.assertNotEqual(timestamp.signed.meta['snapshot.json'], fileinfo) + self.assertNotEqual( + timestamp.signed.meta['snapshot.json'].to_dict(), fileinfo.to_dict() + ) timestamp.signed.update(2, 520, hashes) - self.assertEqual(timestamp.signed.meta['snapshot.json'], fileinfo) + self.assertEqual( + timestamp.signed.meta['snapshot.json'].to_dict(), fileinfo.to_dict() + ) # Test from_dict and to_dict without hashes and length. timestamp_dict = timestamp.to_dict() - test_dict = timestamp_dict['signed'].copy() - del test_dict['meta']['snapshot.json']['length'] - del test_dict['meta']['snapshot.json']['hashes'] + del timestamp_dict['signed']['meta']['snapshot.json']['length'] + del timestamp_dict['signed']['meta']['snapshot.json']['hashes'] + test_dict = copy.deepcopy(timestamp_dict['signed']) timestamp_test = Timestamp.from_dict(test_dict) self.assertEqual(timestamp_dict['signed'], timestamp_test.to_dict()) # Update only version. Length and hashes are optional. timestamp.signed.update(3) - fileinfo = {'version': 3} - self.assertEqual(timestamp.signed.meta['snapshot.json'], fileinfo) + fileinfo = MetaFile(version=3) + self.assertEqual( + timestamp.signed.meta['snapshot.json'].to_dict(), fileinfo.to_dict() + ) def test_key_class(self): keys = { diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 47e01ba63f..0cd44d76c6 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -598,6 +598,60 @@ def remove_key(self, role: str, keyid: str) -> None: del self.keys[keyid] +class MetaFile: + """A container with information about a particular metadata file. + + Attributes: + version: An integer indicating the version of the metadata file. + length: An optional integer indicating the length of the metadata file. + hashes: An optional dictionary mapping hash algorithms to the + hashes resulting from applying them over the metadata file + contents.:: + + 'hashes': { + '': '', + '': '', + ... + } + + unrecognized_fields: Dictionary of all unrecognized fields. + + """ + + def __init__( + self, + version: int, + length: Optional[int] = None, + hashes: Optional[Dict[str, str]] = None, + unrecognized_fields: Optional[Mapping[str, Any]] = None, + ) -> None: + self.version = version + self.length = length + self.hashes = hashes + self.unrecognized_fields: Mapping[str, Any] = unrecognized_fields or {} + + @classmethod + def from_dict(cls, meta_dict: Dict[str, Any]) -> "MetaFile": + """Creates MetaFile object from its dict representation.""" + version = meta_dict.pop("version") + length = meta_dict.pop("length", None) + hashes = meta_dict.pop("hashes", None) + # All fields left in the meta_dict are unrecognized. + return cls(version, length, hashes, meta_dict) + + def to_dict(self) -> Dict[str, Any]: + """Returns the dictionary representation of self.""" + res_dict = {"version": self.version, **self.unrecognized_fields} + + if self.length is not None: + res_dict["length"] = self.length + + if self.hashes is not None: + res_dict["hashes"] = self.hashes + + return res_dict + + class Timestamp(Signed): """A container for the signed part of timestamp metadata. @@ -605,15 +659,7 @@ class Timestamp(Signed): meta: A dictionary that contains information about snapshot metadata:: { - 'snapshot.json': { - 'version': , - 'length': , // optional - 'hashes': { - '': '', - '': '', - ... - } // optional - } + 'snapshot.json': } """ @@ -625,26 +671,28 @@ def __init__( version: int, spec_version: str, expires: datetime, - meta: Dict[str, Any], + meta: Dict[str, MetaFile], unrecognized_fields: Optional[Mapping[str, Any]] = None, ) -> None: super().__init__(version, spec_version, expires, unrecognized_fields) - # TODO: Add class for meta self.meta = meta @classmethod def from_dict(cls, timestamp_dict: Dict[str, Any]) -> "Timestamp": """Creates Timestamp object from its dict representation.""" common_args = cls._common_fields_from_dict(timestamp_dict) - meta = timestamp_dict.pop("meta") + meta_dict = timestamp_dict.pop("meta") + meta = {"snapshot.json": MetaFile.from_dict(meta_dict["snapshot.json"])} # All fields left in the timestamp_dict are unrecognized. return cls(*common_args, meta, timestamp_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self.""" - timestamp_dict = self._common_fields_to_dict() - timestamp_dict.update({"meta": self.meta}) - return timestamp_dict + res_dict = self._common_fields_to_dict() + res_dict["meta"] = { + "snapshot.json": self.meta["snapshot.json"].to_dict() + } + return res_dict # Modification. def update( @@ -654,13 +702,7 @@ def update( hashes: Optional[Dict[str, Any]] = None, ) -> None: """Assigns passed info about snapshot metadata to meta dict.""" - self.meta["snapshot.json"] = {"version": version} - - if length is not None: - self.meta["snapshot.json"]["length"] = length - - if hashes is not None: - self.meta["snapshot.json"]["hashes"] = hashes + self.meta["snapshot.json"] = MetaFile(version, length, hashes) class Snapshot(Signed): @@ -670,22 +712,9 @@ class Snapshot(Signed): meta: A dictionary that contains information about targets metadata:: { - 'targets.json': { - 'version': , - 'length': , // optional - 'hashes': { - '': '', - '': '', - ... - } // optional - }, - '.json': { - ... - }, - '.json': { - ... - }, - ... + 'targets.json': , + '.json': , + '.json': , } """ @@ -697,25 +726,31 @@ def __init__( version: int, spec_version: str, expires: datetime, - meta: Dict[str, Any], + meta: Dict[str, MetaFile], unrecognized_fields: Optional[Mapping[str, Any]] = None, ) -> None: super().__init__(version, spec_version, expires, unrecognized_fields) - # TODO: Add class for meta self.meta = meta @classmethod def from_dict(cls, snapshot_dict: Dict[str, Any]) -> "Snapshot": """Creates Snapshot object from its dict representation.""" common_args = cls._common_fields_from_dict(snapshot_dict) - meta = snapshot_dict.pop("meta") + meta_dicts = snapshot_dict.pop("meta") + meta = {} + for meta_path, meta_dict in meta_dicts.items(): + meta[meta_path] = MetaFile.from_dict(meta_dict) # All fields left in the snapshot_dict are unrecognized. return cls(*common_args, meta, snapshot_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self.""" snapshot_dict = self._common_fields_to_dict() - snapshot_dict.update({"meta": self.meta}) + meta_dict = {} + for meta_path, meta_info in self.meta.items(): + meta_dict[meta_path] = meta_info.to_dict() + + snapshot_dict["meta"] = meta_dict return snapshot_dict # Modification. @@ -728,13 +763,7 @@ def update( ) -> None: """Assigns passed (delegated) targets role info to meta dict.""" metadata_fn = f"{rolename}.json" - - self.meta[metadata_fn] = {"version": version} - if length is not None: - self.meta[metadata_fn]["length"] = length - - if hashes is not None: - self.meta[metadata_fn]["hashes"] = hashes + self.meta[metadata_fn] = MetaFile(version, length, hashes) class DelegatedRole(Role): From 60bbb165a885bbcec387d35135b17f0d705c8163 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Tue, 30 Mar 2021 18:13:16 +0300 Subject: [PATCH 2/6] New API: Add TargetFile class In the top-level metadata classes, there are complex attributes such as "meta" in Targets and Snapshot, "key" and "roles" in Root etc. We want to represent those complex attributes with a class to allow easier verification and support for metadata with unrecognized fields. For more context read ADR 0004 and ADR 0008 in the docs/adr folder. As written in the spec "targets" in "targets.json" has defined the "custom" field serving the same purpose as "unrecognized_fields" in the implementation. That's why to conform against the spec and support "custom" and allow "unrecognized_fields" everywhere where it's not sensitive we can define custom as property which actually access data stored in unrecognized_fields. For context read ADR 8 in tuf/docs/adr. Additionally, after adding the TargetFile class, when we create a Targets an object we are now calling from dict twice - one for the main Targets class and one for each of the complex attributes TargetFile.from_dict() and Delegations.from_dict(). Given that the "from_dict" methods have the side effect of destroying the given dictionary, we would need to start using deepcopy() for our tests. Signed-off-by: Martin Vrachev --- tests/test_api.py | 18 ++++++----- tuf/api/metadata.py | 76 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 72 insertions(+), 22 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 71c4eee62d..77a3edcc86 100755 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -30,6 +30,7 @@ Key, Role, MetaFile, + TargetFile, Delegations, DelegatedRole, ) @@ -540,22 +541,23 @@ def test_metadata_targets(self): "sha512": "ef5beafa16041bcdd2937140afebd485296cd54f7348ecd5a4d035c09759608de467a7ac0eb58753d0242df873c305e8bffad2454aa48f44480f15efae1cacd0" }, - fileinfo = { - 'hashes': hashes, - 'length': 28 - } + fileinfo = TargetFile(length=28, hashes=hashes) # Assert that data is not aleady equal - self.assertNotEqual(targets.signed.targets[filename], fileinfo) + self.assertNotEqual( + targets.signed.targets[filename].to_dict(), fileinfo.to_dict() + ) # Update an already existing fileinfo - targets.signed.update(filename, fileinfo) + targets.signed.update(filename, fileinfo.to_dict()) # Verify that data is updated - self.assertEqual(targets.signed.targets[filename], fileinfo) + self.assertEqual( + targets.signed.targets[filename].to_dict(), fileinfo.to_dict() + ) # Test from_dict/to_dict Targets without delegations targets_dict = targets.to_dict() del targets_dict["signed"]["delegations"] - tmp_dict = targets_dict["signed"].copy() + tmp_dict = copy.deepcopy(targets_dict["signed"]) targets_obj = Targets.from_dict(tmp_dict) self.assertEqual(targets_dict["signed"], targets_obj.to_dict()) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 0cd44d76c6..544d2221e8 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -889,6 +889,57 @@ def to_dict(self) -> Dict[str, Any]: } +class TargetFile: + """A container with information about a particular target file. + + Attributes: + length: An integer indicating the length of the target file. + hashes: A dictionary mapping hash algorithms to the + hashes resulting from applying them over the metadata file + contents:: + + 'hashes': { + '': '', + '': '', + ... + } + unrecognized_fields: Dictionary of all unrecognized fields. + + """ + + @property + def custom(self): + if self.unrecognized_fields is None: + return None + return self.unrecognized_fields.get("custom", None) + + def __init__( + self, + length: int, + hashes: Dict[str, str], + unrecognized_fields: Optional[Mapping[str, Any]] = None, + ) -> None: + self.length = length + self.hashes = hashes + self.unrecognized_fields = unrecognized_fields or {} + + @classmethod + def from_dict(cls, target_dict: Dict[str, Any]) -> "TargetFile": + """Creates TargetFile object from its dict representation.""" + length = target_dict.pop("length") + hashes = target_dict.pop("hashes") + # All fields left in the target_dict are unrecognized. + return cls(length, hashes, target_dict) + + def to_dict(self) -> Dict[str, Any]: + """Returns the JSON-serializable dictionary representation of self.""" + return { + "length": self.length, + "hashes": self.hashes, + **self.unrecognized_fields, + } + + class Targets(Signed): """A container for the signed part of targets metadata. @@ -896,15 +947,7 @@ class Targets(Signed): targets: A dictionary that contains information about target files:: { - '': { - 'length': , - 'hashes': { - '': '', - '': '', - ... - }, - 'custom': // optional - }, + '': , ... } @@ -925,12 +968,11 @@ def __init__( version: int, spec_version: str, expires: datetime, - targets: Dict[str, Any], + targets: Dict[str, TargetFile], delegations: Optional[Delegations] = None, unrecognized_fields: Optional[Mapping[str, Any]] = None, ) -> None: super().__init__(version, spec_version, expires, unrecognized_fields) - # TODO: Add class for meta self.targets = targets self.delegations = delegations @@ -945,13 +987,19 @@ def from_dict(cls, targets_dict: Dict[str, Any]) -> "Targets": delegations = None else: delegations = Delegations.from_dict(delegations_dict) + res_targets = {} + for target_path, target_info in targets.items(): + res_targets[target_path] = TargetFile.from_dict(target_info) # All fields left in the targets_dict are unrecognized. - return cls(*common_args, targets, delegations, targets_dict) + return cls(*common_args, res_targets, delegations, targets_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self.""" targets_dict = self._common_fields_to_dict() - targets_dict["targets"] = self.targets + targets = {} + for target_path, target_file_obj in self.targets.items(): + targets[target_path] = target_file_obj.to_dict() + targets_dict["targets"] = targets if self.delegations is not None: targets_dict["delegations"] = self.delegations.to_dict() return targets_dict @@ -959,4 +1007,4 @@ def to_dict(self) -> Dict[str, Any]: # Modification. def update(self, filename: str, fileinfo: Dict[str, Any]) -> None: """Assigns passed target file info to meta dict.""" - self.targets[filename] = fileinfo + self.targets[filename] = TargetFile.from_dict(fileinfo) From 408732f4ff4b1639f27855600d59c5fa77e7965c Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Wed, 12 May 2021 15:43:39 +0300 Subject: [PATCH 3/6] Add MetaFile/TargetFile specific tests Signed-off-by: Martin Vrachev --- tests/test_api.py | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/test_api.py b/tests/test_api.py index 77a3edcc86..a9e418aaf9 100755 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -243,6 +243,51 @@ def test_metadata_base(self): self.assertFalse(is_expired) md.signed.expires = expires + + def test_metafile_class(self): + # Test from_dict and to_dict with all attributes. + data = { + "hashes": { + "sha256": "8f88e2ba48b412c3843e9bb26e1b6f8fc9e98aceb0fbaa97ba37b4c98717d7ab" + }, + "length": 515, + "version": 1 + } + metafile_obj = MetaFile.from_dict(copy.copy(data)) + self.assertEqual(metafile_obj.to_dict(), data) + + # Test from_dict and to_dict without length. + del data["length"] + metafile_obj = MetaFile.from_dict(copy.copy(data)) + self.assertEqual(metafile_obj.to_dict(), data) + + # Test from_dict and to_dict without length and hashes. + del data["hashes"] + metafile_obj = MetaFile.from_dict(copy.copy(data)) + self.assertEqual(metafile_obj.to_dict(), data) + + + def test_targetfile_class(self): + # Test from_dict and to_dict with all attributes. + data = { + "custom": { + "file_permissions": "0644" + }, + "hashes": { + "sha256": "65b8c67f51c993d898250f40aa57a317d854900b3a04895464313e48785440da", + "sha512": "467430a68afae8e9f9c0771ea5d78bf0b3a0d79a2d3d3b40c69fde4dd42c461448aef76fcef4f5284931a1ffd0ac096d138ba3a0d6ca83fa8d7285a47a296f77" + }, + "length": 31 + } + targetfile_obj = TargetFile.from_dict(copy.copy(data)) + self.assertEqual(targetfile_obj.to_dict(), data) + + # Test from_dict and to_dict without custom. + del data["custom"] + targetfile_obj = TargetFile.from_dict(copy.copy(data)) + self.assertEqual(targetfile_obj.to_dict(), data) + + def test_metadata_snapshot(self): snapshot_path = os.path.join( self.repo_dir, 'metadata', 'snapshot.json') From aaa5bb4fc06f5ed9a255ff58fdb9d58a7ba048e2 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Wed, 12 May 2021 16:34:33 +0300 Subject: [PATCH 4/6] Disable "C0302" for tuf/api/metadata.py Disable the "C0302: Too many lines in module" warning which warns for modules with more 1000 lines, because all of the code here is logically connected and currently, we are above 1000 lines by a small margin. Signed-off-by: Martin Vrachev --- tuf/api/metadata.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 544d2221e8..a68cd3c3cb 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -31,6 +31,11 @@ SignedSerializer, ) +# Disable the "C0302: Too many lines in module" warning which warns for modules +# with more 1000 lines, because all of the code here is logically connected +# and currently, we are above 1000 lines by a small margin. +# pylint: disable=C0302 + class Metadata: """A container for signed TUF metadata. From 37de69050a9e28186febbeb21afe1d1f5717ad17 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Fri, 14 May 2021 17:54:31 +0300 Subject: [PATCH 5/6] Change "update()" argument types Currently, when we call Targets/Snapshot/Timestamp.update() we are passing all of the necessary values to create MetaFile/Targets File respectively. This is not needed, given that one of the reasons we have created MetaFile and TargetFile is to make the API easier to use. Signed-off-by: Martin Vrachev --- tests/test_api.py | 10 +++++----- tuf/api/metadata.py | 23 ++++++----------------- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index a9e418aaf9..18682b283c 100755 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -301,14 +301,14 @@ def test_metadata_snapshot(self): self.assertNotEqual( snapshot.signed.meta['role1.json'].to_dict(), fileinfo.to_dict() ) - snapshot.signed.update('role1', 2, 123, hashes) + snapshot.signed.update('role1', fileinfo) self.assertEqual( snapshot.signed.meta['role1.json'].to_dict(), fileinfo.to_dict() ) # Update only version. Length and hashes are optional. - snapshot.signed.update('role2', 3) fileinfo = MetaFile(3) + snapshot.signed.update('role2', fileinfo) self.assertEqual( snapshot.signed.meta['role2.json'].to_dict(), fileinfo.to_dict() ) @@ -353,7 +353,7 @@ def test_metadata_timestamp(self): self.assertNotEqual( timestamp.signed.meta['snapshot.json'].to_dict(), fileinfo.to_dict() ) - timestamp.signed.update(2, 520, hashes) + timestamp.signed.update(fileinfo) self.assertEqual( timestamp.signed.meta['snapshot.json'].to_dict(), fileinfo.to_dict() ) @@ -367,8 +367,8 @@ def test_metadata_timestamp(self): self.assertEqual(timestamp_dict['signed'], timestamp_test.to_dict()) # Update only version. Length and hashes are optional. - timestamp.signed.update(3) fileinfo = MetaFile(version=3) + timestamp.signed.update(fileinfo) self.assertEqual( timestamp.signed.meta['snapshot.json'].to_dict(), fileinfo.to_dict() ) @@ -593,7 +593,7 @@ def test_metadata_targets(self): targets.signed.targets[filename].to_dict(), fileinfo.to_dict() ) # Update an already existing fileinfo - targets.signed.update(filename, fileinfo.to_dict()) + targets.signed.update(filename, fileinfo) # Verify that data is updated self.assertEqual( targets.signed.targets[filename].to_dict(), fileinfo.to_dict() diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index a68cd3c3cb..034eeceec9 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -700,14 +700,9 @@ def to_dict(self) -> Dict[str, Any]: return res_dict # Modification. - def update( - self, - version: int, - length: Optional[int] = None, - hashes: Optional[Dict[str, Any]] = None, - ) -> None: + def update(self, snapshot_meta: MetaFile) -> None: """Assigns passed info about snapshot metadata to meta dict.""" - self.meta["snapshot.json"] = MetaFile(version, length, hashes) + self.meta["snapshot.json"] = snapshot_meta class Snapshot(Signed): @@ -759,16 +754,10 @@ def to_dict(self) -> Dict[str, Any]: return snapshot_dict # Modification. - def update( - self, - rolename: str, - version: int, - length: Optional[int] = None, - hashes: Optional[Dict[str, Any]] = None, - ) -> None: + 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] = MetaFile(version, length, hashes) + self.meta[metadata_fn] = role_info class DelegatedRole(Role): @@ -1010,6 +999,6 @@ def to_dict(self) -> Dict[str, Any]: return targets_dict # Modification. - def update(self, filename: str, fileinfo: Dict[str, Any]) -> None: + def update(self, filename: str, fileinfo: TargetFile) -> None: """Assigns passed target file info to meta dict.""" - self.targets[filename] = TargetFile.from_dict(fileinfo) + self.targets[filename] = fileinfo From 15bf88231d68bf772ad4891833305a7464abea83 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Mon, 17 May 2021 20:22:00 +0300 Subject: [PATCH 6/6] Remove testing "update()" with "version" only We have tests which make sure we can use `Timestamp.update()` and `Snapshot.update()` with MetaFile instance storing only version (because length and hashes are optional). Those tests were created to make sure that we are actually supporting optional hashes and length when we call `update` for those classes, but after we changed the `update()` signature to accept `MetaFile` instance the tests are obsolete. The reason is that length and hashes can be optional because of the MetaFile implementation, no the update function itself and we have other tests validating creating a MetaFie instance without hashes and length. Signed-off-by: Martin Vrachev --- tests/test_api.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 18682b283c..2a2ebc997f 100755 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -306,13 +306,6 @@ def test_metadata_snapshot(self): snapshot.signed.meta['role1.json'].to_dict(), fileinfo.to_dict() ) - # Update only version. Length and hashes are optional. - fileinfo = MetaFile(3) - snapshot.signed.update('role2', fileinfo) - self.assertEqual( - snapshot.signed.meta['role2.json'].to_dict(), fileinfo.to_dict() - ) - # Test from_dict and to_dict without hashes and length. snapshot_dict = snapshot.to_dict() del snapshot_dict['signed']['meta']['role1.json']['length'] @@ -366,13 +359,6 @@ def test_metadata_timestamp(self): timestamp_test = Timestamp.from_dict(test_dict) self.assertEqual(timestamp_dict['signed'], timestamp_test.to_dict()) - # Update only version. Length and hashes are optional. - fileinfo = MetaFile(version=3) - timestamp.signed.update(fileinfo) - self.assertEqual( - timestamp.signed.meta['snapshot.json'].to_dict(), fileinfo.to_dict() - ) - def test_key_class(self): keys = { "59a4df8af818e9ed7abe0764c0b47b4240952aa0d179b5b78346c470ac30278d":{