Skip to content

Commit 1e8c4e1

Browse files
fredbiclaude
andauthored
refactor(mediatype): extract findByCanonical from Lookup (#443)
Pulls the alias-aware walk pattern out of Lookup into a small helper. The same three steps — direct map hit, alias-canonical hit, O(n) walk returning any map entry whose canonical agrees — appeared in both the alias-bridge tier and the suffix-base tier of the original Lookup. The helper collapses the duplication and makes the structural model clear: "for each target (parsed key, then optionally the suffix base), find an entry whose canonical agrees." No behavior change. Every existing test stays green without modification. Minor trade-off: the original code skipped the m[key] lookup when key == mediaType (i.e. the caller already passed a canonical string). The refactored version always tries m[target] first inside findByCanonical, so an already-canonical input now pays one extra map miss after the fast-path. For codec maps of <10 entries this is invisible — the simpler shape is the better trade. Signed-off-by: Frederic BIDON <fredbi@yahoo.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 853ccde commit 1e8c4e1

1 file changed

Lines changed: 53 additions & 67 deletions

File tree

server-middleware/mediatype/lookup.go

Lines changed: 53 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,18 @@ package mediatype
1414
// 1. mediaType verbatim (fast path for callers that already pass a
1515
// canonical, parameter-free string and store map keys in the
1616
// same form).
17-
// 2. The canonical "type/subtype" form derived by parsing
18-
// mediaType (strips parameters and lowercases — recovers the
19-
// match when mediaType carries "; charset=...").
20-
// 3. The alias-canonicalized form from the package-internal alias
21-
// table — for example, a request for "application/yaml" finds
22-
// an entry registered under "application/x-yaml".
23-
// 4. Walks m and returns the first entry whose own key
24-
// alias-canonicalizes to the same target as mediaType. This
25-
// covers the "map keyed by one alias, query uses another alias
26-
// of the same canonical" case (e.g. registered under text/yaml,
27-
// queried as application/x-yaml).
28-
// 5. When [AllowSuffix] is passed in opts: the RFC 6839
29-
// structured-syntax suffix base on the query side (plus its own
30-
// alias canonical). Catches the "spec/traffic divergence" case
31-
// (request for application/vnd.api+json finds a JSON consumer
32-
// registered under application/json). Query-side only — no
17+
// 2. An alias-aware walk against the parsed "type/subtype" form:
18+
// a direct map hit on the parsed key, on its alias canonical
19+
// if any, and finally an O(len(m)) scan returning any map
20+
// entry whose key alias-canonicalizes to the same target.
21+
// Catches both "map keyed by canonical, query uses alias" and
22+
// "map keyed by one alias, query uses another alias of the
23+
// same canonical".
24+
// 3. When [AllowSuffix] is passed in opts: the same alias-aware
25+
// walk against the RFC 6839 structured-syntax suffix base.
26+
// Catches the "spec/traffic divergence" case (request for
27+
// application/vnd.api+json finds a JSON consumer registered
28+
// under application/json). Query-side suffix fold only — no
3329
// map-side suffix folding.
3430
//
3531
// Lookup does NOT fall back to "*/*". Callers that want wildcard
@@ -45,7 +41,7 @@ package mediatype
4541
//
4642
// - m is empty;
4743
// - mediaType fails to parse and is not present verbatim;
48-
// - none of the active tiers hits.
44+
// - none of the active steps hits.
4945
//
5046
// The malformed-vs-not-found distinction is intentionally elided:
5147
// codec-lookup callers treat both as the same "no codec" error path.
@@ -55,7 +51,9 @@ func Lookup[T any](m map[string]T, mediaType string, opts ...MatchOption) (T, bo
5551
return zero, false
5652
}
5753
o := applyMatchOptions(opts)
58-
// Tier 1: raw key.
54+
// Fast path: raw key (preserves any caller behaviour that stored
55+
// non-canonical strings as map keys, and skips parsing in the
56+
// common already-canonical case).
5957
if v, ok := m[mediaType]; ok {
6058
return v, true
6159
}
@@ -64,67 +62,55 @@ func Lookup[T any](m map[string]T, mediaType string, opts ...MatchOption) (T, bo
6462
return zero, false
6563
}
6664
key := mt.Type + "/" + mt.Subtype
67-
// Tier 2: parsed canonical form (strips params, lowercases).
68-
if key != mediaType {
69-
if v, ok := m[key]; ok {
70-
return v, true
65+
if v, ok := findByCanonical(m, key); ok {
66+
return v, true
67+
}
68+
if o.allowSuffix && mt.Suffix != "" {
69+
base := mt.Base()
70+
if baseKey := base.Type + "/" + base.Subtype; baseKey != key {
71+
if v, ok := findByCanonical(m, baseKey); ok {
72+
return v, true
73+
}
7174
}
7275
}
73-
// Tier 3: alias bridge from the query side — fast path when the
74-
// map is keyed by the canonical form.
75-
target := key
76-
if canon, ok := aliases[key]; ok {
77-
target = canon
78-
if v, ok := m[target]; ok {
76+
return zero, false
77+
}
78+
79+
// findByCanonical returns the first entry in m whose key
80+
// alias-canonicalizes to the same value as target.
81+
//
82+
// Tries direct hits before the O(len(m)) walk:
83+
//
84+
// 1. m[target] — map keyed by the same string.
85+
// 2. m[aliases[target]] — map keyed by the canonical when target
86+
// is an alias.
87+
// 3. Walk m: return any entry where canonical(k) == canonical(target).
88+
// Catches the "map keyed by an alias different from the query
89+
// side" case (e.g. registered under text/yaml, queried as
90+
// application/x-yaml — both canonicalize to application/yaml).
91+
//
92+
// Map size is single-digit for the runtime's codec maps, so the
93+
// walk is negligible.
94+
func findByCanonical[T any](m map[string]T, target string) (T, bool) {
95+
if v, ok := m[target]; ok {
96+
return v, true
97+
}
98+
canonTarget := target
99+
if canon, ok := aliases[target]; ok {
100+
canonTarget = canon
101+
if v, ok := m[canonTarget]; ok {
79102
return v, true
80103
}
81104
}
82-
// Tier 4: alias bridge from the map side. Catches the case where
83-
// the map is keyed by an alias (or by a different alias than the
84-
// query). O(len(m)) but codec maps are tiny (<10 entries) so this
85-
// is negligible.
86105
for k, v := range m {
87106
kCanon := k
88107
if c, ok := aliases[k]; ok {
89108
kCanon = c
90109
}
91-
if kCanon == target {
110+
if kCanon == canonTarget {
92111
return v, true
93112
}
94113
}
95-
// Tier 5 (opt-in): RFC 6839 structured-syntax suffix base. Only
96-
// fires when the query carries a known suffix and the suffix
97-
// resolves to a base type in the package-internal suffix→base
98-
// table. Query-side suffix fold only — we still walk the map
99-
// applying alias canonicalization (same as tier 4), but never
100-
// fold a map key's suffix.
101-
if o.allowSuffix && mt.Suffix != "" {
102-
base := mt.Base()
103-
if base.Type != mt.Type || base.Subtype != mt.Subtype {
104-
baseKey := base.Type + "/" + base.Subtype
105-
baseTarget := baseKey
106-
if v, ok := m[baseKey]; ok {
107-
return v, true
108-
}
109-
if canon, ok := aliases[baseKey]; ok {
110-
baseTarget = canon
111-
if v, ok := m[canon]; ok {
112-
return v, true
113-
}
114-
}
115-
// Catches the "map keyed by an alias of the suffix base"
116-
// case (e.g. base resolves to application/yaml, map is
117-
// keyed by application/x-yaml).
118-
for k, v := range m {
119-
kCanon := k
120-
if c, ok := aliases[k]; ok {
121-
kCanon = c
122-
}
123-
if kCanon == baseTarget {
124-
return v, true
125-
}
126-
}
127-
}
128-
}
114+
var zero T
129115
return zero, false
130116
}

0 commit comments

Comments
 (0)