-
Notifications
You must be signed in to change notification settings - Fork 292
Closed
Labels
experimental-clientItems related to the development of a new client (see milestone/8 and theexperimental-client branch)Items related to the development of a new client (see milestone/8 and theexperimental-client branch)
Milestone
Description
I find it a bit paradox that MetadataWrapper is another container on top of Metadata, adding another level of nesting, when most of the methods/properties are actually shortcuts to attributes contained somewhere in Metadata. If we want a client-variant of Metadata, I'd maybe use inheritance instead of composition. 🤷
But maybe we don't even need it. Let's quickly go through the individual methods/properties of here:
On MetadataWrapper:
from_json_file-> available asMetadata.from_filewith json as default (Move metadata class model de/serialization to sub-package #1279)from_json_object-> could be a format-agnostic method onMetadatajust likefrom_file. Implement from_object/from_string method in Metadata class #1336persist-> available asMetadata.to_filewith json as default (Move metadata class model de/serialization to sub-package #1279)expires-> could be a method onSigned, I'd call it 'is_expired' Add 'expired' method to Signed class #1305verify-> This is definitely needed for the repository tool as well. So it should be somewhere where both client and repository tool can access it. Maybe even onMetadatain addition to the vanilla one-signatureverifywe currently have. See A simple, per-file metadata CRUD API #1060 (comment) for thoughts about that distinction. Implement verification by a threshold of keys #1306
All the other methods here are either pure shortcuts or filters of attributes contained in Metadata objects (or contained objects).
- Re shortcuts: https://github.com/theupdateframework/tuf/pull/1060/files#r452906629 has arguments against shortcuts.
- Re filters: Maybe it will become easier to access individual properties once we have classes for all complex attributes as planned in Add classes for complex metadata fields #1139? But similarly to the arguments against shortcuts, I'd be careful when implementing behavior that relates to contained classes on container classes.
Originally posted by @lukpueh in #1291 (comment)
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
experimental-clientItems related to the development of a new client (see milestone/8 and theexperimental-client branch)Items related to the development of a new client (see milestone/8 and theexperimental-client branch)