Design: layered transformations and the unmapped-key problem #14
Replies: 1 comment
Resolved — ADR-0011, merged in #17Decision: Layer B carries no key-taking public methods. A keyed capability becomes a That is close to the provisional lean above (C + E, with B) — refined by evidence: E's value What changed the answerEvery option in this discussion was tested rather than argued, and three of the four premises Option D was already rejected upstream, in The option list was missing the terminal fix. dol#18's recommended direction is is-a Option E's stated mechanism is false. A wrapper does not re-wrap a Mapping-valued Option C's cited prior art has the bug. And the escape ADR-0006 prescribed is itself unsafe. I then drafted "free functions everywhere", and an adversarial review refuted that too: with a Sibling stores need no key-resolution primitive at all, which is why they are the answer: The reframe worth keeping
Corrections to the framing above
Answers to the four questions
Follow-upsBlocking upstream (i2mint/dol#84 open): export Next: #15, credential and endpoint resolution. |
Uh oh!
There was an error while loading. Please reload this page.
Splitting this out of the v1 redesign (#11) because it is a
dol-wide problem, not an s3dol one, and because the v1 design only sidesteps it rather than solving it.The problem
dol's model is "wrap a base store with layers of transformations". Key transforms are applied by the wrapper's__getitem__/__setitem__/__delitem__/__contains__/__iter__. Every other method is invisible to the transformation layer —dol.base.Store.__getattr__returns the bound leaf method, so it receives the outer, unmapped key.For a backend adapter this is severe, because the interesting capabilities are exactly the non-dunder methods.
Measured (dol 0.3.58), store scoped to
logs/, in a bucket that also holdslogs2/leakand a roota.txt:Minimal repro:
Nothing raises, and
isinstance(w, SupportsUrlFor)staysTrue—@runtime_checkablechecks method presence only.There is a second, independent failure mode: pushdown is impossible.
Store.__iter__callsself.store.__iter__(), so there is no channel by which a key-wrapper hands its prefix to the leaf'sListObjectsV2(Prefix=…). Every prefix-scoped listing becomes a full-bucket scan — measured at 22 LIST requests where a leaf-owned prefix costs 1.Filed upstream as i2mint/dol#83 (delegation) and i2mint/dol#82 (the related prefix-corruption bug).
What v1 does, and why it isn't a solution
v1 sidesteps it: the prefix lives in the leaf, so there is no key-mapping seam between a capability method and the wire (ADR-0001 §Why the prefix lives in the leaf). That fixes the prefix case only. A user who wraps an s3dol store with any
dolkey codec still gets a silently wrongurl_for. Every*doladapter with a keyed capability method has the same latent bug.Options
A — status quo + documentation
Prefix in the leaf; document the trap; use
inner_most_key(wrapped_self(self), k)inside any capability method that might be wrapped.Cheap. Leaves the footgun loaded for users and sibling packages. Note the natural form is itself a trap:
inner_most_key(self, k)returnsNone(inside a delegated methodselfis the unwrapped leaf), so the URL becomeshttps://…/Nonewith no exception.B — declarative key-method registration
The leaf declares which methods take a key and where; dol's wrapper machinery generates mapped delegates.
General; fixes the family at once. But a method the author forgets to declare fails the same silent way. Mitigable with a reflective test that enumerates public methods and fails on any undeclared one — which is the kind of guard that actually holds.
C — free functions instead of methods
s3dol.url_for(store, k)rather thanstore.url_for(k). The function resolves the key through the whole wrapper chain once, then calls the leaf.This is already dol's own idiom —
dol.content_url(store, ref_or_key),get_content,put_content,add_contentall take the store as first argument, anddol/content.pyexplicitly frames it as solving the problem beside the interface rather than inside it.Composes at any wrapper depth; no delegation at all, so nothing to get wrong. Costs ergonomics, and
dol.SupportsUrlForcurrently expects a method — though both can coexist (method on the leaf for the unwrapped case, free function as the safe general form).D — bind delegated methods to the outer store
Change
Store.__getattr__to return a delegate bound to the wrapper, soself._id_of_key(k)inside a leaf method resolves through the full chain.Addresses the root cause rather than each symptom. But it is a behavioural change in dol's core with a wide blast radius (~42k downloads/month, many dependents), and it only works if leaf methods consistently call
self._id_of_key. Closely related to dol#18.E — capabilities as parallel Mappings
Turn each keyed capability into a Mapping view sharing the key space —
store.urls[k],store.info[k],store.handles[k]— so key transformation happens through the Mapping protocol the wrapper already handles correctly. Requires the wrapper to re-wrap Mapping-valued attributes with the same key codec (declarative, à la B:_key_mapped_attrs = ('urls', 'info', 'handles')).Most dol-native of the options: it turns delegation into composition. It also collapses three of the v1.x deferrals in #12 (presigned-URL store, ObjectInfo-from-LIST, handles) into one mechanism. Costs: more objects, and it doesn't cover non-keyed methods (
delete_many,prefixes).Provisional lean
C as the general safe form, E for capabilities that are naturally keyed, with B as the dol-level mechanism that makes E's re-wrapping declarative. D is the most principled but the riskiest to land in dol.
The pushdown half needs its own answer regardless — probably a hint protocol (
__iter__(self, *, prefix_hint=…)) or an explicititer_prefixon the leaf that the wrapper knows to route to. Worth deciding whether that belongs in this design or separately.Questions I'd want settled
All reactions