Revert "Move to/from_dict metadata API methods..."

Revert an earlier commit that moved to/from_dict metadata class
model methods to a util module of the serialization sub-package.

We keep to/from_dict methods on the metadata classes because:
- It seems **idiomatic** (see e.g. 3rd-party libaries such as attrs,
pydantic, marshmallow, or built-ins that provide default or
customizable dict representation for higher-level objects).
The idiomatic choice should make usage more intuitive.
- It feels better **structured** when each method is encapsulated
within the corresponding class, which in turn should make
maintaining/modifying/extending the class model easier.
- It allows us to remove function-scope imports (see subsequent
commit).

Caveat:
Now that "the meat" of the sub-packaged JSON serializer is
implemented on the class, it might make it harder to create a
non-dict based serializer by copy-paste-amending the JSON
serializer.

However, the benefits from above seem to outweigh the disadvantage.

See option 5 of ADR0006 for further details (#1270).

Signed-off-by: Lukas Puehringer <[email protected]>

Author: lukpueh

tests/test_api.py

@@ -38,8 +38,6 @@
     CanonicalJSONSerializer
 )
 
-from tuf.api.serialization.util import metadata_to_dict
-
 from securesystemslib.interface import (
     import_ed25519_publickey_from_file,
     import_ed25519_privatekey_from_file
@@ -49,7 +47,6 @@
     format_keyval_to_metadata
 )
 
-
 logger = logging.getLogger(__name__)
 
 
@@ -114,9 +111,9 @@ def test_generic_read(self):
             self.assertTrue(
                     isinstance(metadata_obj2.signed, inner_metadata_cls))
 
-            # ... and are equal (compared by dict representation)
-            self.assertDictEqual(metadata_to_dict(metadata_obj),
-                                 metadata_to_dict(metadata_obj2))
+            # ... and return the same object (compared by dict representation)
+            self.assertDictEqual(
+                    metadata_obj.to_dict(), metadata_obj2.to_dict())
 
 
         # Assert that it chokes correctly on an unknown metadata type
@@ -148,8 +145,9 @@ def test_read_write_read_compare(self):
             metadata_obj.to_file(path_2)
             metadata_obj_2 = Metadata.from_file(path_2)
 
-            self.assertDictEqual(metadata_to_dict(metadata_obj),
-                                 metadata_to_dict(metadata_obj_2))
+            self.assertDictEqual(
+                    metadata_obj.to_dict(),
+                    metadata_obj_2.to_dict())
 
             os.remove(path_2)
 

tuf/api/metadata.py

@@ -23,6 +23,7 @@
 import tuf.exceptions
 
 
+
 # Types
 JsonDict = Dict[str, Any]
 
@@ -55,6 +56,49 @@ def __init__(self, signed: 'Signed', signatures: list) -> None:
         self.signatures = signatures
 
 
+    # Deserialization (factories).
+    @classmethod
+    def from_dict(cls, metadata: JsonDict) -> 'Metadata':
+        """Creates Metadata object from its JSON/dict representation.
+
+        Calls 'from_dict' for any complex metadata attribute represented by a
+        class also that has a 'from_dict' factory method. (Currently this is
+        only the signed attribute.)
+
+        Arguments:
+            metadata: TUF metadata in JSON/dict representation, as e.g.
+            returned by 'json.loads'.
+
+        Raises:
+            KeyError: The metadata dict format is invalid.
+            ValueError: The metadata has an unrecognized signed._type field.
+
+        Returns:
+            A TUF Metadata object.
+
+        """
+        # Dispatch to contained metadata class on metadata _type field.
+        _type = metadata['signed']['_type']
+
+        if _type == 'targets':
+            inner_cls = Targets
+        elif _type == 'snapshot':
+            inner_cls = Snapshot
+        elif _type == 'timestamp':
+            inner_cls = Timestamp
+        elif _type == 'root':
+            inner_cls = Root
+        else:
+            raise ValueError(f'unrecognized metadata type "{_type}"')
+
+        # NOTE: If Signature becomes a class, we should iterate over
+        # metadata['signatures'], call Signature.from_dict for each item, and
+        # pass a list of Signature objects to the Metadata constructor intead.
+        return cls(
+                signed=inner_cls.from_dict(metadata['signed']),
+                signatures=metadata['signatures'])
+
+
     @classmethod
     def from_file(
         cls, filename: str, deserializer: MetadataDeserializer = None,
@@ -95,6 +139,14 @@ def from_file(
         return deserializer.deserialize(raw_data)
 
 
+    # Serialization.
+    def to_dict(self) -> JsonDict:
+        """Returns the JSON-serializable dictionary representation of self. """
+        return {
+            'signatures': self.signatures,
+            'signed': self.signed.to_dict()
+        }
+
     def to_file(self, filename: str, serializer: MetadataSerializer = None,
                 storage_backend: StorageBackendInterface = None) -> None:
         """Writes TUF metadata to file storage.
@@ -250,6 +302,39 @@ def __init__(
         self.version = version
 
 
+    # Deserialization (factories).
+    @classmethod
+    def from_dict(cls, signed_dict: JsonDict) -> 'Signed':
+        """Creates Signed object from its JSON/dict representation. """
+
+        # 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(
+                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: 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)
+
+
+    def to_dict(self) -> JsonDict:
+        """Returns the JSON-serializable dictionary representation of self. """
+        return {
+            '_type': self._type,
+            'version': self.version,
+            'spec_version': self.spec_version,
+            'expires': self.expires.isoformat() + 'Z'
+        }
+
+
     # Modification.
     def bump_expiration(self, delta: timedelta = timedelta(days=1)) -> None:
         """Increments the expires attribute by the passed timedelta. """
@@ -261,7 +346,6 @@ def bump_version(self) -> None:
         self.version += 1
 
 
-
 class Root(Signed):
     """A container for the signed part of root metadata.
 
@@ -311,6 +395,18 @@ def __init__(
         self.roles = roles
 
 
+    # Serialization.
+    def to_dict(self) -> JsonDict:
+        """Returns the JSON-serializable dictionary representation of self. """
+        json_dict = super().to_dict()
+        json_dict.update({
+            'consistent_snapshot': self.consistent_snapshot,
+            'keys': self.keys,
+            'roles': self.roles
+        })
+        return json_dict
+
+
     # Update key for a role.
     def add_key(self, role: str, keyid: str, key_metadata: JsonDict) -> None:
         """Adds new key for 'role' and updates the key store. """
@@ -332,6 +428,7 @@ def remove_key(self, role: str, keyid: str) -> None:
 
 
 
+
 class Timestamp(Signed):
     """A container for the signed part of timestamp metadata.
 
@@ -359,6 +456,16 @@ def __init__(
         self.meta = meta
 
 
+    # Serialization.
+    def to_dict(self) -> JsonDict:
+        """Returns the JSON-serializable dictionary representation of self. """
+        json_dict = super().to_dict()
+        json_dict.update({
+            'meta': self.meta
+        })
+        return json_dict
+
+
     # Modification.
     def update(self, version: int, length: int, hashes: JsonDict) -> None:
         """Assigns passed info about snapshot metadata to meta dict. """
@@ -369,7 +476,6 @@ def update(self, version: int, length: int, hashes: JsonDict) -> None:
         }
 
 
-
 class Snapshot(Signed):
     """A container for the signed part of snapshot metadata.
 
@@ -403,6 +509,15 @@ def __init__(
         # TODO: Add class for meta
         self.meta = meta
 
+    # Serialization.
+    def to_dict(self) -> JsonDict:
+        """Returns the JSON-serializable dictionary representation of self. """
+        json_dict = super().to_dict()
+        json_dict.update({
+            'meta': self.meta
+        })
+        return json_dict
+
 
     # Modification.
     def update(
@@ -419,7 +534,6 @@ def update(
             self.meta[metadata_fn]['hashes'] = hashes
 
 
-
 class Targets(Signed):
     """A container for the signed part of targets metadata.
 
@@ -487,6 +601,16 @@ def __init__(
         self.delegations = delegations
 
 
+    # Serialization.
+    def to_dict(self) -> JsonDict:
+        """Returns the JSON-serializable dictionary representation of self. """
+        json_dict = super().to_dict()
+        json_dict.update({
+            'targets': self.targets,
+            'delegations': self.delegations,
+        })
+        return json_dict
+
     # Modification.
     def update(self, filename: str, fileinfo: JsonDict) -> None:
         """Assigns passed target file info to meta dict. """

tuf/api/pylintrc

@@ -8,8 +8,5 @@ good-names=e
 indent-string="    "
 max-line-length=79
 
-[CLASSES]
-exclude-protected:_type
-
 [DESIGN]
 min-public-methods=0

tuf/api/serialization/json.py

@@ -19,8 +19,7 @@
                                    MetadataDeserializer,
                                    SignedSerializer,
                                    SerializationError,
-                                   DeserializationError,
-                                   util)
+                                   DeserializationError)
 
 
 class JSONDeserializer(MetadataDeserializer):
@@ -30,7 +29,7 @@ def deserialize(self, raw_data: bytes) -> Metadata:
         """Deserialize utf-8 encoded JSON bytes into Metadata object. """
         try:
             _dict = json.loads(raw_data.decode("utf-8"))
-            return util.metadata_from_dict(_dict)
+            return Metadata.from_dict(_dict)
 
         except Exception as e: # pylint: disable=broad-except
             six.raise_from(DeserializationError, e)
@@ -52,7 +51,7 @@ def serialize(self, metadata_obj: Metadata) -> bytes:
         try:
             indent = (None if self.compact else 1)
             separators=((',', ':') if self.compact else (',', ': '))
-            return json.dumps(util.metadata_to_dict(metadata_obj),
+            return json.dumps(metadata_obj.to_dict(),
                               indent=indent,
                               separators=separators,
                               sort_keys=True).encode("utf-8")
@@ -67,7 +66,7 @@ class CanonicalJSONSerializer(SignedSerializer):
     def serialize(self, signed_obj: Signed) -> bytes:
         """Serialize Signed object into utf-8 encoded Canonical JSON bytes. """
         try:
-            signed_dict = util.signed_to_dict(signed_obj)
+            signed_dict = signed_obj.to_dict()
             return encode_canonical(signed_dict).encode("utf-8")
 
         except Exception as e: # pylint: disable=broad-except