Skip to content

Commit 9befda1

Browse files
committed
frontend: address router review feedback
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
1 parent b64e347 commit 9befda1

3 files changed

Lines changed: 33 additions & 23 deletions

File tree

frontend/client.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ const (
2020
// for the builder.
2121
KeyDefaultPlatform = "frontend.dalec.defaultPlatform"
2222
KeyJSONSchema = "frontend.dalec.schema"
23+
24+
resultJSONMetaKey = "result.json"
25+
resultTextMetaKey = "result.txt"
2326
)
2427

2528
const keyTarget = "target"
@@ -38,9 +41,7 @@ func (err *noSuchHandlerError) Error() string {
3841
return fmt.Sprintf("no such handler for target %q: available targets: %s", err.Target, strings.Join(err.Available, ", "))
3942
}
4043

41-
var (
42-
_ gwclient.Client = (*clientWithCustomOpts)(nil)
43-
)
44+
var _ gwclient.Client = (*clientWithCustomOpts)(nil)
4445

4546
type clientWithCustomOpts struct {
4647
opts gwclient.BuildOpts
@@ -112,10 +113,10 @@ func handleResolveSpec(ctx context.Context, client gwclient.Client) (*gwclient.R
112113
}
113114

114115
res := gwclient.NewResult()
115-
res.AddMeta("result.json", dtJSON)
116+
res.AddMeta(resultJSONMetaKey, dtJSON)
116117
// result.txt here so that `docker buildx build --print` will output it directly.
117118
// Otherwise it prints a go object.
118-
res.AddMeta("result.txt", dtYaml)
119+
res.AddMeta(resultTextMetaKey, dtYaml)
119120

120121
return res, nil
121122
}
@@ -130,17 +131,17 @@ func handleDefaultPlatform() (*gwclient.Result, error) {
130131
return nil, err
131132
}
132133

133-
res.AddMeta("result.json", dt)
134-
res.AddMeta("result.txt", []byte(platforms.Format(p)))
134+
res.AddMeta(resultJSONMetaKey, dt)
135+
res.AddMeta(resultTextMetaKey, []byte(platforms.Format(p)))
135136

136137
return res, nil
137138
}
138139

139140
func handleJSONSchema() (*gwclient.Result, error) {
140141
res := gwclient.NewResult()
141142
jsonSchema := dalec.GetJSONSchema()
142-
res.AddMeta("result.json", jsonSchema)
143-
res.AddMeta("result.txt", jsonSchema)
143+
res.AddMeta(resultJSONMetaKey, jsonSchema)
144+
res.AddMeta(resultTextMetaKey, jsonSchema)
144145

145146
return res, nil
146147
}

frontend/router.go

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -204,8 +204,8 @@ func (r *Router) describe() (*gwclient.Result, error) {
204204

205205
res := gwclient.NewResult()
206206
res.Metadata = map[string][]byte{
207-
"result.txt": buf.Bytes(),
208-
"version": []byte(subrequests.SubrequestsDescribeDefinition.Version),
207+
resultTextMetaKey: buf.Bytes(),
208+
"version": []byte(subrequests.SubrequestsDescribeDefinition.Version),
209209
}
210210
return res, nil
211211
}
@@ -273,9 +273,9 @@ func queryForwardedTargets(ctx context.Context, client gwclient.Client, prefix s
273273
return nil, err
274274
}
275275

276-
dt, ok := res.Metadata["result.json"]
276+
dt, ok := res.Metadata[resultJSONMetaKey]
277277
if !ok {
278-
return nil, errors.New("forwarded frontend did not return result.json")
278+
return nil, errors.New("forwarded frontend did not return " + resultJSONMetaKey)
279279
}
280280

281281
var remote bktargets.List
@@ -300,13 +300,13 @@ func dalecTargetListToResult(ls TargetList) (*gwclient.Result, error) {
300300
if err != nil {
301301
return nil, errors.Wrap(err, "error marshalling target list to json")
302302
}
303-
res.AddMeta("result.json", dtJSON)
303+
res.AddMeta(resultJSONMetaKey, dtJSON)
304304

305305
buf := bytes.NewBuffer(nil)
306306
if err := printTargets(ls, buf); err != nil {
307307
return nil, err
308308
}
309-
res.AddMeta("result.txt", buf.Bytes())
309+
res.AddMeta(resultTextMetaKey, buf.Bytes())
310310

311311
res.AddMeta("version", []byte(bktargets.SubrequestsTargetsDefinition.Version))
312312
return res, nil
@@ -336,20 +336,17 @@ func printTargets(ls TargetList, w io.Writer) error {
336336
}
337337

338338
// lookupTarget finds the route matching the given target string.
339-
// It tries exact match first, then longest prefix match.
340339
// An empty target is always an error — callers must specify a target key.
340+
// Otherwise it tries exact match first, then longest prefix match.
341341
func (r *Router) lookupTarget(ctx context.Context, target string) (matchedPath string, _ *Route, _ error) {
342-
// 1. Exact match
343-
if route, ok := r.routes[target]; ok {
344-
return target, &route, nil
345-
}
346-
347-
// 2. Empty target — no global default; prompt the user.
348342
if target == "" {
349343
return "", nil, handlerNotFound(target, maps.Keys(r.routes))
350344
}
351345

352-
// 3. Longest prefix match
346+
if route, ok := r.routes[target]; ok {
347+
return target, &route, nil
348+
}
349+
353350
var candidates []string
354351
for k := range r.routes {
355352
if strings.HasPrefix(target, k+"/") {

frontend/router_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,15 @@ func TestRouterPrefixMatch(t *testing.T) {
144144
func TestRouterEmptyTargetReturnsError(t *testing.T) {
145145
ctx := context.Background()
146146
var r Router
147+
var emptyRouteCalled bool
148+
149+
r.Add(ctx, Route{
150+
FullPath: "",
151+
Handler: func(context.Context, gwclient.Client) (*gwclient.Result, error) {
152+
emptyRouteCalled = true
153+
return nil, nil
154+
},
155+
})
147156

148157
r.Add(ctx, Route{
149158
FullPath: "mydefault",
@@ -163,6 +172,9 @@ func TestRouterEmptyTargetReturnsError(t *testing.T) {
163172
if !errors.As(err, &nsh) {
164173
t.Fatalf("expected noSuchHandlerError, got %T: %v", err, err)
165174
}
175+
if emptyRouteCalled {
176+
t.Fatal("expected empty target to error before matching empty route")
177+
}
166178
}
167179

168180
func TestRouterNotFound(t *testing.T) {

0 commit comments

Comments
 (0)