Refactor tuf metadata to model deserializtion (WIP)

TODO:
- Clean up diff a bit more
- add code docs
- write proper commit message

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

Author: lukpueh

tests/test_api.py

@@ -16,7 +16,7 @@
 import tempfile
 import unittest
 
-from datetime import timedelta
+from datetime import datetime, timedelta
 from dateutil.relativedelta import relativedelta
 
 IS_PY_VERSION_SUPPORTED = sys.version_info >= (3, 6)
@@ -186,11 +186,11 @@ def test_metadata_base(self):
         self.assertEqual(md.signed.version, 1)
         md.signed.bump_version()
         self.assertEqual(md.signed.version, 2)
-        self.assertEqual(md.signed.expires, '2030-01-01T00:00:00Z')
+        self.assertEqual(md.signed.expires, datetime(2030, 1, 1, 0, 0))
         md.signed.bump_expiration()
-        self.assertEqual(md.signed.expires, '2030-01-02T00:00:00Z')
+        self.assertEqual(md.signed.expires, datetime(2030, 1, 2, 0, 0))
         md.signed.bump_expiration(timedelta(days=365))
-        self.assertEqual(md.signed.expires, '2031-01-02T00:00:00Z')
+        self.assertEqual(md.signed.expires, datetime(2031, 1, 2, 0, 0))
 
 
     def test_metadata_snapshot(self):
@@ -218,20 +218,20 @@ def test_metadata_timestamp(self):
         timestamp.signed.bump_version()
         self.assertEqual(timestamp.signed.version, 2)
 
-        self.assertEqual(timestamp.signed.expires, '2030-01-01T00:00:00Z')
+        self.assertEqual(timestamp.signed.expires, datetime(2030, 1, 1, 0, 0))
         timestamp.signed.bump_expiration()
-        self.assertEqual(timestamp.signed.expires, '2030-01-02T00:00:00Z')
+        self.assertEqual(timestamp.signed.expires, datetime(2030, 1, 2, 0, 0))
         timestamp.signed.bump_expiration(timedelta(days=365))
-        self.assertEqual(timestamp.signed.expires, '2031-01-02T00:00:00Z')
+        self.assertEqual(timestamp.signed.expires, datetime(2031, 1, 2, 0, 0))
 
         # Test whether dateutil.relativedelta works, this provides a much
         # easier to use interface for callers
         delta = relativedelta(days=1)
         timestamp.signed.bump_expiration(delta)
-        self.assertEqual(timestamp.signed.expires, '2031-01-03T00:00:00Z')
+        self.assertEqual(timestamp.signed.expires, datetime(2031, 1, 3, 0, 0))
         delta = relativedelta(years=5)
         timestamp.signed.bump_expiration(delta)
-        self.assertEqual(timestamp.signed.expires, '2036-01-03T00:00:00Z')
+        self.assertEqual(timestamp.signed.expires, datetime(2036, 1, 3, 0, 0))
 
         hashes = {'sha256': '0ae9664468150a9aa1e7f11feecb32341658eb84292851367fea2da88e8a58dc'}
         fileinfo = timestamp.signed.meta['snapshot.json']

tuf/api/metadata.py

@@ -29,7 +29,11 @@
 import tempfile
 
 from securesystemslib.formats import encode_canonical
-from securesystemslib.util import load_json_file, persist_temp_file
+from securesystemslib.util import (
+    load_json_file,
+    load_json_string,
+    persist_temp_file
+)
 from securesystemslib.storage import StorageBackendInterface
 from securesystemslib.keys import create_signature, verify_signature
 from tuf.repository_lib import (
@@ -69,12 +73,34 @@ class Metadata():
             ]
 
     """
-    def __init__(
-            self, signed: 'Signed' = None, signatures: list = None) -> None:
-        # TODO: How much init magic do we want?
+    def __init__(self, signed: 'Signed', signatures: list) -> None:
         self.signed = signed
         self.signatures = signatures
 
+    @classmethod
+    def from_dict(cls, metadata: JsonDict) -> 'Metadata':
+        _type = metadata['signed']['_type']
+
+        if _type == 'targets':
+            signed_cls = Targets
+        elif _type == 'snapshot':
+            signed_cls = Snapshot
+        elif _type == 'timestamp':
+            signed_cls = Timestamp
+        elif _type == 'root':
+            # TODO: implement Root class
+            raise NotImplementedError('Root not yet implemented')
+        else:
+            raise ValueError(f'unrecognized metadata type "{_type}"')
+
+        return Metadata(
+                signed=signed_cls.from_dict(metadata['signed']),
+                signatures=metadata['signatures'])
+
+    def from_json(metadata_json: str) -> 'Metadata':
+        """Deserialize a json string and pass to from_dict(). """
+        return Metadata.from_dict(load_json_string(metadata_json))
+
 
     def to_json(self, compact: bool = False) -> None:
         """Returns the optionally compacted JSON representation of self. """
@@ -174,27 +200,7 @@ def from_json_file(
             A TUF Metadata object.
 
         """
-        signable = load_json_file(filename, storage_backend)
-
-        # TODO: Should we use constants?
-        # And/or maybe a dispatch table? (<-- maybe too much magic)
-        _type = signable['signed']['_type']
-
-        if _type == 'targets':
-            inner_cls = Targets
-        elif _type == 'snapshot':
-            inner_cls = Snapshot
-        elif _type == 'timestamp':
-            inner_cls = Timestamp
-        elif _type == 'root':
-            # TODO: implement Root class
-            raise NotImplementedError('Root not yet implemented')
-        else:
-            raise ValueError(f'unrecognized metadata type "{_type}"')
-
-        return Metadata(
-                signed=inner_cls(**signable['signed']),
-                signatures=signable['signatures'])
+        return Metadata.from_dict(load_json_file(filename, storage_backend))
 
     def to_json_file(
             self, filename: str, compact: bool = False,
@@ -237,41 +243,42 @@ class Signed:
     # we keep it to match spec terminology (I often refer to this as "payload",
     # or "inner metadata")
 
-    # TODO: Re-think default values. It might be better to pass some things
-    # as args and not es kwargs. Then we'd need to pop those from
-    # signable["signed"] in read_from_json and pass them explicitly, which
-    # some say is better than implicit. :)
     def __init__(
-            self, _type: str = None, version: int = 0,
-            spec_version: str = None, expires: datetime = None
-        ) -> None:
-        # TODO: How much init magic do we want?
+            self, _type: str, version: int, spec_version: str,
+            expires: datetime) -> None:
 
         self._type = _type
+        self.version = version
         self.spec_version = spec_version
+        self.expires = expires
 
-        # We always intend times to be UTC
-        # NOTE: we could do this with datetime.fromisoformat() but that is not
-        # available in Python 2.7's datetime
-        # NOTE: Store as datetime object for convenient handling, use 'expires'
-        # property to get the TUF metadata format representation
-        self.__expiration = iso8601.parse_date(expires).replace(tzinfo=None)
-
+        # TODO: Should we separate data validation from constructor?
         if version < 0:
             raise ValueError(f'version must be < 0, got {version}')
+
         self.version = version
 
-    @property
-    def signed_bytes(self) -> bytes:
-        return encode_canonical(self.as_dict()).encode('UTF-8')
+    @classmethod
+    def from_dict(cls, signed_dict) -> 'Signed':
+        # NOTE: Convert 'expires' TUF metadata string representation 
+        # to datetime object, as the constructor expects it. See 'to_dict' for
+        # the inverse operation.
+        signed_dict['expires'] = iso8601.parse_date(
+                signed_dict['expires']).replace(tzinfo=None)
+
+        # NOTE: Any additional conversion of dict metadata should happen here
+        # or in the corresponding derived class of 'Signed'. E.g. if the
+        # delegations field  
+
+        return cls(**signed_dict)
 
     @property
-    def expires(self) -> str:
-        return self.__expiration.isoformat() + 'Z'
+    def signed_bytes(self) -> bytes:
+        return encode_canonical(self.to_dict()).encode('UTF-8')
 
     def bump_expiration(self, delta: timedelta = timedelta(days=1)) -> None:
         """Increments the expires attribute by the passed timedelta. """
-        self.__expiration = self.__expiration + delta
+        self.expires += delta
 
     def bump_version(self) -> None:
         """Increments the metadata version number by 1."""
@@ -283,7 +290,7 @@ def to_dict(self) -> JsonDict:
             '_type': self._type,
             'version': self.version,
             'spec_version': self.spec_version,
-            'expires': self.expires
+            'expires': self.expires.isoformat() + 'Z'
         }
 
 class Timestamp(Signed):
@@ -305,12 +312,17 @@ class Timestamp(Signed):
             }
 
     """
-    def __init__(self, meta: JsonDict = None, **kwargs) -> None:
-        super().__init__(**kwargs)
-        # TODO: How much init magic do we want?
-        # TODO: Is there merit in creating classes for dict fields?
+    def __init__(
+            self, _type: str, version: int, spec_version: str,
+            expires: datetime, meta: JsonDict) -> None:
+        super().__init__(_type, version, spec_version, expires)
+        # TODO: Add class for meta
         self.meta = meta
 
+    @classmethod
+    def from_dict(cls, timestamp_dict):
+        return super().from_dict(timestamp_dict) 
+
     def to_dict(self) -> JsonDict:
         """Returns the JSON-serializable dictionary representation of self. """
         json_dict = super().to_dict()
@@ -354,12 +366,17 @@ class Snapshot(Signed):
             }
 
     """
-    def __init__(self, meta: JsonDict = None, **kwargs) -> None:
-        # TODO: How much init magic do we want?
-        # TODO: Is there merit in creating classes for dict fields?
-        super().__init__(**kwargs)
+    def __init__(
+            self, _type: str, version: int, spec_version: str,
+            expires: datetime, meta: JsonDict) -> None:
+        super().__init__(_type, version, spec_version, expires)
+        # TODO: Add class for meta
         self.meta = meta
 
+    @classmethod
+    def from_dict(cls, snapshot_dict):
+        return super().from_dict(snapshot_dict) 
+
     def to_dict(self) -> JsonDict:
         """Returns the JSON-serializable dictionary representation of self. """
         json_dict = super().to_dict()
@@ -437,14 +454,18 @@ class Targets(Signed):
 
     """
     def __init__(
-            self, targets: JsonDict = None, delegations: JsonDict = None,
-            **kwargs) -> None:
-        # TODO: How much init magic do we want?
-        # TODO: Is there merit in creating classes for dict fields?
-        super().__init__(**kwargs)
+            self, _type: str, version: int, spec_version: str,
+            expires: datetime, targets: JsonDict, delegations: JsonDict
+            ) -> None:
+        super().__init__(_type, version, spec_version, expires)
+        # TODO: Add class for meta
         self.targets = targets
         self.delegations = delegations
 
+    @classmethod
+    def from_dict(cls, targets_dict):
+        return super().from_dict(targets_dict) 
+
     def to_dict(self) -> JsonDict:
         """Returns the JSON-serializable dictionary representation of self. """
         json_dict = super().to_dict()