|
| 1 | +# Separate metadata serialization from metadata class model but keep helpers |
| 2 | + |
| 3 | +Technical Story: https://github.com/theupdateframework/tuf/pull/1279 |
| 4 | + |
| 5 | +## Context and Problem Statement |
| 6 | +In the course of implementing a class-based role metadata model we have also |
| 7 | +reviewed options on how to design serialization infrastructure between wire |
| 8 | +formats and the class model. In an initial attempt we have implemented |
| 9 | +serialization on the metadata class (see option 1), but issues with inheritance |
| 10 | +and calls for more flexibility have caused us to rethink this approach. |
| 11 | + |
| 12 | +## Decision Drivers |
| 13 | +* A class-based role metadata model (see ADR4) requires serialization routines |
| 14 | + from and to wire formats |
| 15 | +* TUF integrators may require custom serialization implementations for custom |
| 16 | + wire formats |
| 17 | +* Readability and simplicity of implementation for users and maintainers |
| 18 | +* Recognizability of specification |
| 19 | + |
| 20 | +## Considered Options |
| 21 | +1. Serialization in metadata classes |
| 22 | +2. Serialization in metadata subclasses |
| 23 | +3. Serialization separated from metadata classes |
| 24 | +4. Compromise 1: Default serialization methods in metadata classes / |
| 25 | + non-default serialization separated |
| 26 | +5. Compromise 2: Serialization separated / dict conversion helper methods for |
| 27 | + default serialization in metadata classes |
| 28 | + |
| 29 | +## Decision Outcome |
| 30 | +Chosen option: "Compromise 2", because implementing dict conversion as methods |
| 31 | +on a corresponding class is idiomatic and allows for well-structured code. |
| 32 | +Together with a separated serialization interface, it provides both ease of use |
| 33 | +and maintenance, and full flexibility with regards to custom serialization |
| 34 | +implementations and wire formats. |
| 35 | + |
| 36 | +## Pros and Cons of the Options |
| 37 | + |
| 38 | +### Option 1: Serialization in metadata classes |
| 39 | + |
| 40 | +Serialization is implemented on metadata classes, e.g. |
| 41 | +`Metadata.serialize_as_json()`, etc. |
| 42 | + |
| 43 | +* Good, because serialization for any object is encapsulated within the |
| 44 | + corresponding class and thus structured in small code chunks, using the |
| 45 | + already existing hierarchical class model structure. |
| 46 | +* Good, because the TUF specification is heavily based on json, even if only |
| 47 | + for illustrative purposes, thus this option facilitates recognizability. |
| 48 | +* Bad, because it might suggest that TUF is limited to json alone. |
| 49 | +* Bad, because it does not facilitate custom serialization implementations. |
| 50 | +* Bad, because it can get complicated with inheritance in the class model. |
| 51 | + *NOTE: a workaround exists in #1279.* |
| 52 | + |
| 53 | +### Option 2: Serialization in metadata subclasses |
| 54 | +Serialization is implemented on metadata subclasses, e.g. |
| 55 | +`JsonMetadata.serialize()`, etc. |
| 56 | + |
| 57 | +* Good, because the wire format is decoupled from the base classes, not giving |
| 58 | + the impression that TUF is limited to json, and facilitating custom |
| 59 | + implementations. |
| 60 | +* Bad, because a user needs to decide on serialization ahead of time, when |
| 61 | + instantiating the metadata objects. |
| 62 | +* Bad, because the metadata model has many classes, which would all need to be |
| 63 | + subclassed accordingly. |
| 64 | + |
| 65 | +### Option 3: Serialization separated from metadata classes |
| 66 | +Serialization is implemented independently of the metadata class, e.g. by |
| 67 | +defining an abstract `Serializer` interface, which must be implemented in |
| 68 | +subclasses, e.g. `JsonSerializer`, etc. |
| 69 | + |
| 70 | +* Good, because the wire format is completely decoupled from the class model, |
| 71 | + not giving the impression that TUF is limited to json, and facilitating |
| 72 | + custom implementations. |
| 73 | +* Good, because it can serve as exact blueprint for custom implementations. |
| 74 | +* Bad, because a decoupled serialization implementation needs to "re-implement" |
| 75 | + the entire class hierarchy, likely in a procedural manner. |
| 76 | + |
| 77 | +### Option 4: Compromise 1 |
| 78 | +Default json serialization is implemented on the metadata class as described in |
| 79 | +(1), but can be overridden using an independent `Serializer` interface as |
| 80 | +described in (3). |
| 81 | + |
| 82 | +* Good, for the reasons outlined in options (1) and (3), i.e. encapsulation |
| 83 | + within classes but decoupled class model and wire format. |
| 84 | +* Bad, because it creates two different code paths for default and non-default |
| 85 | + wire formats making the code more complex and prone to deteriorate, |
| 86 | + especially on the non-default path. |
| 87 | +* Bad, because the on-the-class default implementation can not be used as |
| 88 | + blueprint for custom implementations. |
| 89 | + |
| 90 | +### Option 5: Compromise 2 |
| 91 | +Serialization is implemented independently of the metadata class as described |
| 92 | +in (3). However, the *meat* of the default `JsonSerializer`, i.e. conversion |
| 93 | +between metadata objects and dicts, is implemented on the metadata class, e.g. |
| 94 | +as `Metadata.to_dict()`, etc. |
| 95 | + |
| 96 | +* Good, for the reasons outlined in options (1) and (3), i.e. encapsulation |
| 97 | + within classes but decoupled class model and wire format, without the |
| 98 | + disadvantage in (4) of having two completely different code paths. |
| 99 | +* Good, because it makes the separate default serializer a minimal wrapper |
| 100 | + around the dict conversion methods. |
| 101 | +* Good, because other serialization implementations might also make use of dict |
| 102 | + conversion methods. |
| 103 | +* Good, because conversion between class objects and dicts is akin to type |
| 104 | + casting, which is idiomatic to implement on the class. |
| 105 | +* Bad, because the on-the-class default implementation can not be used as |
| 106 | + blueprint for custom implementations. |
| 107 | + |
| 108 | +## Links |
| 109 | +* [ADR4: Add classes for complex metadata attributes (decision driver)](/Users/lukp/tuf/tuf/docs/adr/0004-extent-of-OOP-in-metadata-model.md) |
| 110 | +* [PR: Add simple TUF role metadata model (implements option 1)](https://github.com/theupdateframework/tuf/pull/1112) |
| 111 | + - [details about separation of serialization and instantiation](https://github.com/theupdateframework/tuf/commit/f63dce6dddb9cfbf8986141340c6fac00a36d46e) |
| 112 | + - [code comment about issues with inheritance](https://github.com/theupdateframework/tuf/blob/9401059101b08a18abc5e3be4d60e18670693f62/tuf/api/metadata.py#L297-L306) |
| 113 | +* [PR: New metadata API: add MetadataInfo and TargetFile classes (recent ADR discussion impetus)](https://github.com/theupdateframework/tuf/pull/1223) |
| 114 | + - [more discussion about issues with inheritance](https://github.com/theupdateframework/tuf/pull/1223#issuecomment-737188686) |
| 115 | +* [SSLIB/Issue: Add metadata container classes (comparison of options 1 and 2)](https://github.com/secure-systems-lab/securesystemslib/issues/272) |
| 116 | +* [tuf-on-a-plane parser (implements option 3)](https://github.com/trishankatdatadog/tuf-on-a-plane/blob/master/src/tuf_on_a_plane/parsers/) |
0 commit comments