Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move metadata class model de/serialization to sub-package #1279

Merged
merged 18 commits into from
Mar 10, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
Targets
)

from tuf.api.serialization import (
DeserializationError
)

from tuf.api.serialization.json import (
JSONSerializer,
JSONDeserializer,
Expand Down Expand Up @@ -118,7 +122,7 @@ def test_generic_read(self):
with open(bad_metadata_path, 'wb') as f:
f.write(json.dumps(bad_metadata).encode('utf-8'))

with self.assertRaises(ValueError):
with self.assertRaises(DeserializationError):
Metadata.from_file(bad_metadata_path)

os.remove(bad_metadata_path)
Expand Down
11 changes: 8 additions & 3 deletions tuf/api/metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,8 @@ def from_file(

Raises:
securesystemslib.exceptions.StorageError: The file cannot be read.
securesystemslib.exceptions.Error, ValueError, KeyError: The
metadata cannot be parsed.
tuf.api.serialization.DeserializationError:
The file cannot be deserialized.

Returns:
A TUF Metadata object.
Expand Down Expand Up @@ -161,6 +161,8 @@ def to_file(self, filename: str, serializer: MetadataSerializer = None,
a (local) FilesystemBackend is used.

Raises:
tuf.api.serialization.SerializationError:
The metadata object cannot be serialized.
securesystemslib.exceptions.StorageError:
The file cannot be written.

Expand Down Expand Up @@ -191,7 +193,8 @@ def sign(self, key: JsonDict, append: bool = False,
CanonicalJSONSerializer is used.

Raises:
securesystemslib.exceptions.FormatError: Key argument is malformed.
tuf.api.serialization.SerializationError:
'signed' cannot be serialized.
securesystemslib.exceptions.CryptoError, \
securesystemslib.exceptions.UnsupportedAlgorithmError:
Signing errors.
Expand Down Expand Up @@ -230,6 +233,8 @@ def verify(self, key: JsonDict,
# TODO: Revise exception taxonomy
tuf.exceptions.Error: None or multiple signatures found for key.
securesystemslib.exceptions.FormatError: Key argument is malformed.
tuf.api.serialization.SerializationError:
'signed' cannot be serialized.
securesystemslib.exceptions.CryptoError, \
securesystemslib.exceptions.UnsupportedAlgorithmError:
Signing errors.
Expand Down
3 changes: 3 additions & 0 deletions tuf/api/pylintrc
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
[MESSAGE_CONTROL]
disable=fixme

[BASIC]
good-names=e

[FORMAT]
indent-string=" "
max-line-length=79
Expand Down
8 changes: 8 additions & 0 deletions tuf/api/serialization/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@
"""
import abc

# TODO: Should these be in tuf.exceptions or inherit from tuf.exceptions.Error?
class SerializationError(Exception):
"""Error during serialization. """

class DeserializationError(Exception):
"""Error during deserialization. """


class MetadataDeserializer():
"""Abstract base class for deserialization of Metadata objects. """
__metaclass__ = abc.ABCMeta
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: now that we are only supporting Python3 we can switch to less confusing metaclass syntax, either

import abc

class MetadataDeserializer(ABC):

or

import abc

class MetadataDeserializer(metaclass=ABCMeta):

we can handle this in a future PR, though.

Expand Down
37 changes: 26 additions & 11 deletions tuf/api/serialization/json.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

"""
import json
import six
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we using six in code that was never intended to support Python2?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For raise_from below, which I will change to raise ... from.


from securesystemslib.formats import encode_canonical

Expand All @@ -16,16 +17,22 @@
from tuf.api.metadata import Metadata, Signed
from tuf.api.serialization import (MetadataSerializer,
MetadataDeserializer,
SignedSerializer)
SignedSerializer,
SerializationError,
DeserializationError)


class JSONDeserializer(MetadataDeserializer):
"""Provides JSON-to-Metadata deserialize method. """

def deserialize(self, raw_data: bytes) -> Metadata:
"""Deserialize utf-8 encoded JSON bytes into Metadata object. """
_dict = json.loads(raw_data.decode("utf-8"))
return Metadata.from_dict(_dict)
try:
_dict = json.loads(raw_data.decode("utf-8"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here, with the code moved, can we rename _dict? Possible alternatives include json_object and json_dict.

return Metadata.from_dict(_dict)

except Exception as e: # pylint: disable=broad-except
six.raise_from(DeserializationError, e)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python3 syntax for this appears to be

            raise SerializationError from e



class JSONSerializer(MetadataSerializer):
Expand All @@ -41,18 +48,26 @@ def __init__(self, compact: bool = False) -> None:

def serialize(self, metadata_obj: Metadata) -> bytes:
"""Serialize Metadata object into utf-8 encoded JSON bytes. """
indent = (None if self.compact else 1)
separators=((',', ':') if self.compact else (',', ': '))
return json.dumps(metadata_obj.to_dict(),
indent=indent,
separators=separators,
sort_keys=True).encode("utf-8")
try:
indent = (None if self.compact else 1)
separators=((',', ':') if self.compact else (',', ': '))
return json.dumps(metadata_obj.to_dict(),
indent=indent,
separators=separators,
sort_keys=True).encode("utf-8")

except Exception as e: # pylint: disable=broad-except
six.raise_from(SerializationError, e)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Python3 syntax for this appears to be

            raise SerializationError from e



class CanonicalJSONSerializer(SignedSerializer):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it is worth clarifying, via the class name, that this is the OLPC canonical JSON?
You mention it in the header above, but as we have had several adopters surprised that our Canonical JSON isn't compatible with go-tuf's Canonical JSON, I wonder whether it is worth being very explicit?

Counter-argument, I do not like OLPCCanonicalJSONSerializer as a class name...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with both arguments. :D

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another way to avoid ambiguity would be to not call it CanonicalJSONSerializer at all, but something like DefaultSignedSerializer or so? Then people are forced to read the docstring to get more info. At any rate, I will mention OLPC in the class and function docstrings.

"""A Signed-to-Canonical JSON 'serialize' method. """

def serialize(self, signed_obj: Signed) -> bytes:
"""Serialize Signed object into utf-8 encoded Canonical JSON bytes. """
signed_dict = signed_obj.to_dict()
return encode_canonical(signed_dict).encode("utf-8")
try:
signed_dict = signed_obj.to_dict()
return encode_canonical(signed_dict).encode("utf-8")

except Exception as e: # pylint: disable=broad-except
six.raise_from(SerializationError, e)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not use six here

            raise SerializationError from e