New API: Make sure we are not changing dict arg

In the Signed.from_dict function we are passing signed - a dictionary.
Dictionaries are passed by reference and thus we want to make sure
we are not changing the dictionary passed as a function argument.

I had to give default function arguments for all class arguments
assigned in __init__ for Root, Snapshot, Targets, and Timestamp classes.
I chose "None" as the easiest solution as a default argument, but
we definitely want to add proper validation which will ensure
we are not creating empty or partially populated objects.
I didn't want to create a discussion for sensible defaults
and argument validation here.
There is already existing issue for that:
theupdateframework#1140

Signed-off-by: Martin Vrachev <[email protected]>

tuf/api/metadata.py

@@ -279,8 +279,6 @@ def __init__(
         self.expires = expires
 
         # TODO: Should we separate data validation from constructor?
-        if version < 0:
-            raise ValueError(f'version must be < 0, got {version}')
         self.version = version
 
 
@@ -289,22 +287,24 @@ def __init__(
     def from_dict(cls, signed_dict: JsonDict) -> 'Signed':
         """Creates Signed object from its JSON/dict representation. """
 
+        # Create empty object with default or parametrized constructor with
+        # default arguments.
+        obj = cls()
+        obj._type = signed_dict['_type']
+        obj.version = signed_dict['version']
+        obj.spec_version = signed_dict['spec_version']
         # Convert 'expires' TUF metadata string to a datetime object, which is
         # what the constructor expects and what we store. The inverse operation
         # is implemented in 'to_dict'.
-        signed_dict['expires'] = tuf.formats.expiry_string_to_datetime(
+        obj.expires = tuf.formats.expiry_string_to_datetime(
                 signed_dict['expires'])
-        # NOTE: We write the converted 'expires' back into 'signed_dict' above
-        # so that we can pass it to the constructor as  '**signed_dict' below,
-        # along with other fields that belong to Signed subclasses.
-        # Any 'from_dict'(-like) conversions of fields that correspond to a
-        # subclass should be performed in the 'from_dict' method of that
-        # subclass and also be written back into 'signed_dict' before calling
-        # super().from_dict.
+        # NOTE: Any 'from_dict'(-like) conversions of fields that correspond
+        # to a subclass should be performed in the 'from_dict' method of that
+        # subclass and assigned to the below returned object.
 
         # NOTE: cls might be a subclass of Signed, if 'from_dict' was called on
         # that subclass (see e.g. Metadata.from_dict).
-        return cls(**signed_dict)
+        return obj
 
 
     # Serialization.
@@ -376,16 +376,28 @@ class Root(Signed):
     # default max-args value for pylint is 5
     # pylint: disable=too-many-arguments
     def __init__(
-            self, _type: str, version: int, spec_version: str,
-            expires: datetime, consistent_snapshot: bool,
-            keys: JsonDict, roles: JsonDict) -> None:
+            self, _type: str=None, version: int=None, spec_version: str=None,
+            expires: datetime=None, consistent_snapshot: bool=None,
+            keys: JsonDict=None, roles: JsonDict=None) -> None:
         super().__init__(_type, version, spec_version, expires)
         # TODO: Add classes for keys and roles
         self.consistent_snapshot = consistent_snapshot
         self.keys = keys
         self.roles = roles
 
 
+    @classmethod
+    def from_dict(cls, signed_dict: JsonDict) -> 'Root':
+        """Creates Root object from its JSON/dict representation. """
+
+        # Get a parent object with its attributes already assigned.
+        obj = super().from_dict(signed_dict)
+        obj.consistent_snapshot = signed_dict['consistent_snapshot']
+        obj.keys = signed_dict['keys']
+        obj.roles = signed_dict['roles']
+        return obj
+
+
     # Serialization.
     def to_dict(self) -> JsonDict:
         """Returns the JSON-serializable dictionary representation of self. """
@@ -476,8 +488,9 @@ class Timestamp(Signed):
 
     """
     def __init__(
-            self, _type: str, version: int, spec_version: str,
-            expires: datetime, meta: Dict[str, MetadataInfo]) -> None:
+            self, _type: str=None, version: int=None, spec_version: str=None,
+            expires: datetime=None, meta: Dict[str, MetadataInfo]=None
+            ) -> None:
         super().__init__(_type, version, spec_version, expires)
         self.meta = meta
 
@@ -498,12 +511,13 @@ def to_dict(self) -> JsonDict:
     def from_dict(cls, signed_dict: JsonDict) -> 'Timestamp':
         """Creates Timestamp object from its JSON/dict representation. """
 
-        # signed_dict is passed by reference, make sure we are not changing it.
-        changed_signed = signed_dict.copy()
-        changed_signed['meta']['snapshot.json'] = MetadataInfo(
+        # Get a parent object with its attributes already assigned.
+        obj = super().from_dict(signed_dict)
+        obj.meta = {}
+        obj.meta['snapshot.json'] = MetadataInfo(
                 **signed_dict['meta']['snapshot.json'])
 
-        return super().from_dict(changed_signed)
+        return obj
 
 
     # Modification.
@@ -529,8 +543,9 @@ class Snapshot(Signed):
 
     """
     def __init__(
-            self, _type: str, version: int, spec_version: str,
-            expires: datetime, meta: Dict[str, MetadataInfo]) -> None:
+            self, _type: str=None, version: int=None, spec_version: str=None,
+            expires: datetime=None, meta: Dict[str, MetadataInfo]=None
+            ) -> None:
         super().__init__(_type, version, spec_version, expires)
         self.meta = meta
 
@@ -539,13 +554,14 @@ def __init__(
     def from_dict(cls, signed_dict: JsonDict) -> 'Snapshot':
         """Creates Snapshot object from its JSON/dict representation. """
 
-        # signed_dict is passed by reference, make sure we are not changing it.
-        changed_signed = signed_dict.copy()
+        # Get a parent object with its attributes already assigned.
+        obj = super().from_dict(signed_dict)
+        obj.meta = {}
         for meta_path in signed_dict['meta'].keys():
-            changed_signed['meta'][meta_path] = MetadataInfo(
+            obj.meta[meta_path] = MetadataInfo(
                     **signed_dict['meta'][meta_path])
 
-        return super().from_dict(changed_signed)
+        return obj
 
 
     # Serialization.
@@ -674,9 +690,9 @@ class Targets(Signed):
     # default max-args value for pylint is 5
     # pylint: disable=too-many-arguments
     def __init__(
-            self, _type: str, version: int, spec_version: str,
-            expires: datetime, targets: Dict[str, TargetInfo],
-            delegations: JsonDict) -> None:
+            self, _type: str=None, version: int=None, spec_version: str=None,
+            expires: datetime=None, targets: Dict[str, TargetInfo]=None,
+            delegations: JsonDict=None) -> None:
         super().__init__(_type, version, spec_version, expires)
         self.targets = targets
 
@@ -688,13 +704,15 @@ def __init__(
     def from_dict(cls, signed_dict: JsonDict) -> 'Targets':
         """Creates Targets object from its JSON/dict representation. """
 
-        # signed_dict is passed by reference, make sure we are not changing it.
-        changed_signed = signed_dict.copy()
+        # Get a parent object with its attributes already assigned.
+        obj = super().from_dict(signed_dict)
+        obj.targets = {}
         for target_path in signed_dict['targets'].keys():
-            changed_signed['targets'][target_path] = TargetInfo(
+            obj.targets[target_path] = TargetInfo(
                     **signed_dict['targets'][target_path])
 
-        return super().from_dict(changed_signed)
+        obj.delegations = signed_dict['delegations']
+        return obj
 
 
     # Serialization.