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

ADR0006: Where to implement model serialization #1270

Merged
merged 1 commit into from
Jul 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
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
116 changes: 116 additions & 0 deletions docs/adr/0006-where-to-implemenent-model-serialization.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
# Separate metadata serialization from metadata class model but keep helpers

Technical Story: https://github.com/theupdateframework/tuf/pull/1279

## Context and Problem Statement
In the course of implementing a class-based role metadata model we have also
reviewed options on how to design serialization infrastructure between wire
formats and the class model. In an initial attempt we have implemented
serialization on the metadata class (see option 1), but issues with inheritance
and calls for more flexibility have caused us to rethink this approach.

## Decision Drivers
* A class-based role metadata model (see ADR4) requires serialization routines
from and to wire formats
* TUF integrators may require custom serialization implementations for custom
wire formats
* Readability and simplicity of implementation for users and maintainers
* Recognizability of specification

## Considered Options
1. Serialization in metadata classes
2. Serialization in metadata subclasses
3. Serialization separated from metadata classes
4. Compromise 1: Default serialization methods in metadata classes /
non-default serialization separated
5. Compromise 2: Serialization separated / dict conversion helper methods for
default serialization in metadata classes

## Decision Outcome
Chosen option: "Compromise 2", because implementing dict conversion as methods
on a corresponding class is idiomatic and allows for well-structured code.
Together with a separated serialization interface, it provides both ease of use
and maintenance, and full flexibility with regards to custom serialization
implementations and wire formats.

## Pros and Cons of the Options

### Option 1: Serialization in metadata classes

Serialization is implemented on metadata classes, e.g.
`Metadata.serialize_as_json()`, etc.

* Good, because serialization for any object is encapsulated within the
corresponding class and thus structured in small code chunks, using the
already existing hierarchical class model structure.
* Good, because the TUF specification is heavily based on json, even if only
for illustrative purposes, thus this option facilitates recognizability.
* Bad, because it might suggest that TUF is limited to json alone.
* Bad, because it does not facilitate custom serialization implementations.
* Bad, because it can get complicated with inheritance in the class model.
*NOTE: a workaround exists in #1279.*

### Option 2: Serialization in metadata subclasses
Serialization is implemented on metadata subclasses, e.g.
`JsonMetadata.serialize()`, etc.

* Good, because the wire format is decoupled from the base classes, not giving
the impression that TUF is limited to json, and facilitating custom
implementations.
* Bad, because a user needs to decide on serialization ahead of time, when
instantiating the metadata objects.
* Bad, because the metadata model has many classes, which would all need to be
subclassed accordingly.

### Option 3: Serialization separated from metadata classes
Serialization is implemented independently of the metadata class, e.g. by
defining an abstract `Serializer` interface, which must be implemented in
subclasses, e.g. `JsonSerializer`, etc.

* Good, because the wire format is completely decoupled from the class model,
not giving the impression that TUF is limited to json, and facilitating
custom implementations.
* Good, because it can serve as exact blueprint for custom implementations.
* Bad, because a decoupled serialization implementation needs to "re-implement"
the entire class hierarchy, likely in a procedural manner.

### Option 4: Compromise 1
Default json serialization is implemented on the metadata class as described in
(1), but can be overridden using an independent `Serializer` interface as
described in (3).

* Good, for the reasons outlined in options (1) and (3), i.e. encapsulation
within classes but decoupled class model and wire format.
* Bad, because it creates two different code paths for default and non-default
wire formats making the code more complex and prone to deteriorate,
especially on the non-default path.
* Bad, because the on-the-class default implementation can not be used as
blueprint for custom implementations.

### Option 5: Compromise 2
Serialization is implemented independently of the metadata class as described
in (3). However, the *meat* of the default `JsonSerializer`, i.e. conversion
between metadata objects and dicts, is implemented on the metadata class, e.g.
as `Metadata.to_dict()`, etc.

* Good, for the reasons outlined in options (1) and (3), i.e. encapsulation
within classes but decoupled class model and wire format, without the
disadvantage in (4) of having two completely different code paths.
* Good, because it makes the separate default serializer a minimal wrapper
around the dict conversion methods.
* Good, because other serialization implementations might also make use of dict
conversion methods.
* Good, because conversion between class objects and dicts is akin to type
casting, which is idiomatic to implement on the class.
* Bad, because the on-the-class default implementation can not be used as
blueprint for custom implementations.

## Links
* [ADR4: Add classes for complex metadata attributes (decision driver)](/Users/lukp/tuf/tuf/docs/adr/0004-extent-of-OOP-in-metadata-model.md)
* [PR: Add simple TUF role metadata model (implements option 1)](https://github.com/theupdateframework/tuf/pull/1112)
- [details about separation of serialization and instantiation](https://github.com/theupdateframework/tuf/commit/f63dce6dddb9cfbf8986141340c6fac00a36d46e)
- [code comment about issues with inheritance](https://github.com/theupdateframework/tuf/blob/9401059101b08a18abc5e3be4d60e18670693f62/tuf/api/metadata.py#L297-L306)
* [PR: New metadata API: add MetadataInfo and TargetFile classes (recent ADR discussion impetus)](https://github.com/theupdateframework/tuf/pull/1223)
- [more discussion about issues with inheritance](https://github.com/theupdateframework/tuf/pull/1223#issuecomment-737188686)
* [SSLIB/Issue: Add metadata container classes (comparison of options 1 and 2)](https://github.com/secure-systems-lab/securesystemslib/issues/272)
* [tuf-on-a-plane parser (implements option 3)](https://github.com/trishankatdatadog/tuf-on-a-plane/blob/master/src/tuf_on_a_plane/parsers/)
1 change: 1 addition & 0 deletions docs/adr/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ This log lists the architectural decisions for tuf.
- [ADR-0003](0003-where-to-develop-TUF-1-0-0.md) - Develop TUF 1.0.0 in a subdirectory of the current TUF implementation
- [ADR-0004](0004-extent-of-OOP-in-metadata-model.md) - Add classes for complex metadata attributes
- [ADR-0005](0005-use-google-python-style-guide.md) - Use Google Python style guide with minimal refinements
- [ADR-0006](0006-where-to-implemenent-model-serialization.md) - Separate metadata serialization from metadata class model but keep helpers

<!-- adrlogstop -->

Expand Down