Skip to content

Commit 45222b5

Browse files
authored
Replace gorilla/mux with Go 1.22 stdlib ServeMux in test server (#4955)
## Summary - Remove the `gorilla/mux` dependency by switching `libs/testserver` to Go 1.22's enhanced `net/http.ServeMux`, which added method-based routing (`"GET /items/{id}"`), path wildcards (`{id}`), and rest wildcards (`{path...}`) — the same features we used gorilla for. See the [Go 1.22 release notes](https://go.dev/doc/go1.22#enhanced_routing_patterns). - Replace gorilla's `{path:.*}` regex wildcards with stdlib `{path...}` rest wildcards, and `mux.Vars(r)` with `r.PathValue()`. - Exact (non-wildcard) paths are dispatched via a map lookup before reaching ServeMux. This avoids a ServeMux limitation where registering method-specific exact paths alongside method-specific wildcard patterns panics due to Go's implicit GET→HEAD handling creating unresolvable precedence. When an exact path has no handler for the request method, it falls through to ServeMux where a wildcard handler can serve it (e.g., a stub registers `GET /exact/path`, but `HEAD` falls through to a wildcard `HEAD` handler). - Wildcard patterns in `test.toml` stubs must now use the same placeholder names as the default handlers they coexist with — ServeMux panics on structurally identical patterns with different wildcard names. Two stubs updated: `{name}` → `{full_name}`. - Extract the routing logic into a `Router` type with its own tests (`libs/testserver/router.go` + `router_test.go`); `Server` embeds it. The package doc on `Router` explains why the wrapper exists on top of stdlib ServeMux. ## Test plan - [x] Unit tests pass (`libs/testserver`, `bundle/direct/dresources`, `acceptance/internal`) - [x] Acceptance selftests pass - [x] Full CI This pull request was AI-assisted by Isaac.
1 parent 2156629 commit 45222b5

10 files changed

Lines changed: 308 additions & 58 deletions

File tree

NOTICE

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,6 @@ Copyright (c) 2013 Dario Castañé. All rights reserved.
7979
Copyright (c) 2012 The Go Authors. All rights reserved.
8080
License - https://github.com/darccio/mergo/blob/master/LICENSE
8181

82-
gorilla/mux - https://github.com/gorilla/mux
83-
Copyright (c) 2023 The Gorilla Authors. All rights reserved.
84-
License - https://github.com/gorilla/mux/blob/main/LICENSE
85-
8682
palantir/pkg - https://github.com/palantir/pkg
8783
Copyright (c) 2016, Palantir Technologies, Inc.
8884
License - https://github.com/palantir/pkg/blob/master/LICENSE

acceptance/bundle/invariant/test.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,5 @@ Pattern = "POST /api/2.0/sql/statements/"
8181
Response.Body = '{"status": {"state": "SUCCEEDED"}, "manifest": {"schema": {"columns": []}}}'
8282

8383
[[Server]]
84-
Pattern = "DELETE /api/2.1/unity-catalog/tables/{name}"
84+
Pattern = "DELETE /api/2.1/unity-catalog/tables/{full_name}"
8585
Response.Body = '{"status": "OK"}'

acceptance/bundle/resources/synced_database_tables/basic/test.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,5 @@ Pattern = "POST /api/2.0/sql/statements/"
2020
Response.Body = '{"status": {"state": "SUCCEEDED"}, "manifest": {"schema": {"columns": []}}}'
2121

2222
[[Server]]
23-
Pattern = "DELETE /api/2.1/unity-catalog/tables/{name}"
23+
Pattern = "DELETE /api/2.1/unity-catalog/tables/{full_name}"
2424
Response.Body = '{"status": "OK"}'

acceptance/internal/prepare_server.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,8 @@ func startLocalServer(t *testing.T,
188188
killCountersMu := &sync.Mutex{}
189189

190190
for ind := range stubs {
191-
// We want later stubs takes precedence, because then leaf configs take precedence over parent directory configs
192-
// In gorilla/mux earlier handlers take precedence, so we need to reverse the order
191+
// Later stubs take precedence over earlier ones (leaf configs override parent configs).
192+
// The first handler registered for a given pattern wins, so we reverse the order.
193193
stub := stubs[len(stubs)-1-ind]
194194
require.NotEmpty(t, stub.Pattern)
195195
items := strings.Split(stub.Pattern, " ")
@@ -226,7 +226,8 @@ func startLocalServer(t *testing.T,
226226
})
227227
}
228228

229-
// The earliest handlers take precedence, add default handlers last
229+
// The first handler registered for a given pattern wins, so default
230+
// handlers registered last serve as fallbacks.
230231
testserver.AddDefaultHandlers(s)
231232
return s.URL
232233
}

go.mod

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ require (
1515
github.com/databricks/databricks-sdk-go v0.128.0 // Apache-2.0
1616
github.com/google/jsonschema-go v0.4.3 // MIT
1717
github.com/google/uuid v1.6.0 // BSD-3-Clause
18-
github.com/gorilla/mux v1.8.1 // BSD-3-Clause
1918
github.com/gorilla/websocket v1.5.3 // BSD-2-Clause
2019
github.com/hashicorp/go-version v1.9.0 // MPL-2.0
2120
github.com/hashicorp/hc-install v0.9.3 // MPL-2.0

go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,6 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.11 h1:vAe81Msw+8tKUxi2Dq
124124
github.com/googleapis/enterprise-certificate-proxy v0.3.11/go.mod h1:RFV7MUdlb7AgEq2v7FmMCfeSMCllAzWxFgRdusoGks8=
125125
github.com/googleapis/gax-go/v2 v2.17.0 h1:RksgfBpxqff0EZkDWYuz9q/uWsTVz+kf43LsZ1J6SMc=
126126
github.com/googleapis/gax-go/v2 v2.17.0/go.mod h1:mzaqghpQp4JDh3HvADwrat+6M3MOIDp5YKHhb9PAgDY=
127-
github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY=
128-
github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ=
129127
github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg=
130128
github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
131129
github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=

libs/testserver/handlers.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ func AddDefaultHandlers(server *Server) {
109109
return ""
110110
})
111111

112-
server.Handle("POST", "/api/2.0/workspace-files/import-file/{path:.*}", func(req Request) any {
112+
server.Handle("POST", "/api/2.0/workspace-files/import-file/{path...}", func(req Request) any {
113113
path := req.Vars["path"]
114114
overwrite := req.URL.Query().Get("overwrite") == "true"
115115
return req.Workspace.WorkspaceFilesImportFile(path, req.Body, overwrite)
@@ -145,12 +145,12 @@ func AddDefaultHandlers(server *Server) {
145145
return req.Workspace.WorkspaceFilesImportFile(request.Path, decoded, request.Overwrite)
146146
})
147147

148-
server.Handle("GET", "/api/2.0/workspace-files/{path:.*}", func(req Request) any {
148+
server.Handle("GET", "/api/2.0/workspace-files/{path...}", func(req Request) any {
149149
path := req.Vars["path"]
150150
return req.Workspace.WorkspaceFilesExportFile(path)
151151
})
152152

153-
server.Handle("HEAD", "/api/2.0/fs/directories/{path:.*}", func(req Request) any {
153+
server.Handle("HEAD", "/api/2.0/fs/directories/{path...}", func(req Request) any {
154154
dirPath := req.Vars["path"]
155155
if !strings.HasPrefix(dirPath, "/") {
156156
dirPath = "/" + dirPath
@@ -165,15 +165,15 @@ func AddDefaultHandlers(server *Server) {
165165
return Response{StatusCode: 404}
166166
})
167167

168-
server.Handle("HEAD", "/api/2.0/fs/files/{path:.*}", func(req Request) any {
168+
server.Handle("HEAD", "/api/2.0/fs/files/{path...}", func(req Request) any {
169169
path := req.Vars["path"]
170170
if req.Workspace.FileExists(path) {
171171
return Response{StatusCode: 200}
172172
}
173173
return Response{StatusCode: 404}
174174
})
175175

176-
server.Handle("PUT", "/api/2.0/fs/directories/{path:.*}", func(req Request) any {
176+
server.Handle("PUT", "/api/2.0/fs/directories/{path...}", func(req Request) any {
177177
dirPath := req.Vars["path"]
178178
if !strings.HasPrefix(dirPath, "/") {
179179
dirPath = "/" + dirPath
@@ -194,13 +194,13 @@ func AddDefaultHandlers(server *Server) {
194194
return Response{}
195195
})
196196

197-
server.Handle("PUT", "/api/2.0/fs/files/{path:.*}", func(req Request) any {
197+
server.Handle("PUT", "/api/2.0/fs/files/{path...}", func(req Request) any {
198198
path := req.Vars["path"]
199199
overwrite := req.URL.Query().Get("overwrite") == "true"
200200
return req.Workspace.WorkspaceFilesImportFile(path, req.Body, overwrite)
201201
})
202202

203-
server.Handle("GET", "/api/2.0/fs/files/{path:.*}", func(req Request) any {
203+
server.Handle("GET", "/api/2.0/fs/files/{path...}", func(req Request) any {
204204
path := req.Vars["path"]
205205
data := req.Workspace.WorkspaceFilesExportFile(path)
206206
if data == nil {

libs/testserver/router.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package testserver
2+
3+
import (
4+
"net/http"
5+
"strings"
6+
)
7+
8+
// HandlerFunc is the test-server handler signature.
9+
type HandlerFunc func(req Request) any
10+
11+
// Router maps method+path to a HandlerFunc. Wildcards use Go 1.22 ServeMux
12+
// placeholder syntax ({name} or {name...}).
13+
//
14+
// # Why a custom router
15+
//
16+
// Go 1.22 added method matching ("GET /path") and {name}/{name...}
17+
// placeholders to http.ServeMux, covering most of what we previously used
18+
// gorilla/mux for. But two ServeMux behaviors make it inconvenient to use
19+
// directly in the test server:
20+
//
21+
// - Exact and wildcard patterns conflict when they cover the same
22+
// request under different methods. ServeMux treats `GET /x` as
23+
// matching both GET and HEAD, so it overlaps with `HEAD /{path...}`
24+
// and panics at registration. Test fixtures register both kinds of
25+
// routes side by side, so we keep exact paths in our own map and
26+
// only delegate wildcards to ServeMux. Exact lookup runs first;
27+
// misses fall through to ServeMux, which also lets a wildcard
28+
// handler serve methods that the exact registration doesn't cover.
29+
//
30+
// - ServeMux panics on duplicate pattern registration. Router silently
31+
// drops the later registration — first wins. Two callers rely on this:
32+
// AddDefaultHandlers (libs/testserver/handlers.go) installs fallback
33+
// handlers that any test stub for the same pattern can override, and
34+
// startLocalServer (acceptance/internal/prepare_server.go) iterates
35+
// test.toml stubs in reverse so leaf-directory configs register first
36+
// and win over inherited parent stubs.
37+
//
38+
// Router also clears req.URL.RawPath before dispatch so percent-encoded
39+
// slashes (%2F) match literal slashes in patterns; workspace file paths
40+
// in tests routinely contain encoded slashes.
41+
type Router struct {
42+
mux *http.ServeMux
43+
exact map[string]map[string]HandlerFunc
44+
wildcard map[string]bool
45+
46+
// Dispatch is invoked when a route matches. vars holds the path values for
47+
// wildcard routes and is nil for exact routes.
48+
Dispatch func(w http.ResponseWriter, r *http.Request, h HandlerFunc, vars map[string]string)
49+
// NotFound is invoked when no route matches.
50+
NotFound http.HandlerFunc
51+
}
52+
53+
func NewRouter() *Router {
54+
r := &Router{
55+
mux: http.NewServeMux(),
56+
exact: map[string]map[string]HandlerFunc{},
57+
wildcard: map[string]bool{},
58+
}
59+
r.mux.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) {
60+
if r.NotFound != nil {
61+
r.NotFound(w, req)
62+
}
63+
})
64+
return r
65+
}
66+
67+
// Handle registers a handler for method+path. First registration wins;
68+
// duplicate (method, path) registrations are ignored.
69+
func (r *Router) Handle(method, path string, handler HandlerFunc) {
70+
if !strings.Contains(path, "{") {
71+
if r.exact[path] == nil {
72+
r.exact[path] = map[string]HandlerFunc{}
73+
}
74+
if _, ok := r.exact[path][method]; !ok {
75+
r.exact[path][method] = handler
76+
}
77+
return
78+
}
79+
pattern := method + " " + path
80+
if r.wildcard[pattern] {
81+
return
82+
}
83+
r.wildcard[pattern] = true
84+
names := wildcardNames(path)
85+
r.mux.HandleFunc(pattern, func(w http.ResponseWriter, req *http.Request) {
86+
vars := make(map[string]string, len(names))
87+
for _, name := range names {
88+
vars[name] = req.PathValue(name)
89+
}
90+
if r.Dispatch != nil {
91+
r.Dispatch(w, req, handler, vars)
92+
}
93+
})
94+
}
95+
96+
// ServeHTTP routes a request to the registered handler, falling back to
97+
// NotFound if no route matches.
98+
func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) {
99+
// Force ServeMux to match against the decoded path; see the type doc.
100+
req.URL.RawPath = ""
101+
if methods, ok := r.exact[req.URL.Path]; ok {
102+
if h, ok := methods[req.Method]; ok {
103+
if r.Dispatch != nil {
104+
r.Dispatch(w, req, h, nil)
105+
}
106+
return
107+
}
108+
}
109+
r.mux.ServeHTTP(w, req)
110+
}
111+
112+
// wildcardNames extracts wildcard parameter names from a path pattern,
113+
// e.g. "/api/{id}/files/{path...}" returns ["id", "path"].
114+
func wildcardNames(path string) []string {
115+
var names []string
116+
for part := range strings.SplitSeq(path, "/") {
117+
if strings.HasPrefix(part, "{") && strings.HasSuffix(part, "}") {
118+
name := part[1 : len(part)-1]
119+
name = strings.TrimSuffix(name, "...")
120+
names = append(names, name)
121+
}
122+
}
123+
return names
124+
}

libs/testserver/router_test.go

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
package testserver_test
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
8+
"github.com/databricks/cli/libs/testserver"
9+
"github.com/stretchr/testify/assert"
10+
)
11+
12+
type capture struct {
13+
handler string
14+
vars map[string]string
15+
notFound bool
16+
}
17+
18+
func newRouter(t *testing.T) (*testserver.Router, *capture) {
19+
t.Helper()
20+
c := &capture{}
21+
r := testserver.NewRouter()
22+
r.Dispatch = func(w http.ResponseWriter, req *http.Request, h testserver.HandlerFunc, vars map[string]string) {
23+
c.vars = vars
24+
c.handler = h(testserver.Request{}).(string)
25+
}
26+
r.NotFound = func(w http.ResponseWriter, req *http.Request) {
27+
c.notFound = true
28+
}
29+
return r, c
30+
}
31+
32+
func handlerNamed(name string) testserver.HandlerFunc {
33+
return func(req testserver.Request) any { return name }
34+
}
35+
36+
func TestRouterExactMatch(t *testing.T) {
37+
r, c := newRouter(t)
38+
r.Handle("GET", "/foo", handlerNamed("foo-get"))
39+
40+
r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/foo", nil))
41+
assert.Equal(t, "foo-get", c.handler)
42+
assert.Nil(t, c.vars)
43+
}
44+
45+
func TestRouterWildcardMatch(t *testing.T) {
46+
r, c := newRouter(t)
47+
r.Handle("GET", "/items/{id}", handlerNamed("item-get"))
48+
49+
r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/items/42", nil))
50+
assert.Equal(t, "item-get", c.handler)
51+
assert.Equal(t, map[string]string{"id": "42"}, c.vars)
52+
}
53+
54+
func TestRouterCatchAllWildcard(t *testing.T) {
55+
r, c := newRouter(t)
56+
r.Handle("GET", "/files/{path...}", handlerNamed("files-get"))
57+
58+
r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/files/a/b/c", nil))
59+
assert.Equal(t, "files-get", c.handler)
60+
assert.Equal(t, map[string]string{"path": "a/b/c"}, c.vars)
61+
}
62+
63+
func TestRouterMultipleWildcards(t *testing.T) {
64+
r, c := newRouter(t)
65+
r.Handle("GET", "/items/{id}/files/{path...}", handlerNamed("nested"))
66+
67+
r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/items/42/files/a/b", nil))
68+
assert.Equal(t, "nested", c.handler)
69+
assert.Equal(t, map[string]string{"id": "42", "path": "a/b"}, c.vars)
70+
}
71+
72+
func TestRouterExactBeforeWildcard(t *testing.T) {
73+
r, c := newRouter(t)
74+
r.Handle("GET", "/foo", handlerNamed("exact"))
75+
r.Handle("HEAD", "/{path...}", handlerNamed("wildcard-head"))
76+
77+
r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/foo", nil))
78+
assert.Equal(t, "exact", c.handler)
79+
80+
c.handler = ""
81+
r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodHead, "/foo", nil))
82+
assert.Equal(t, "wildcard-head", c.handler)
83+
}
84+
85+
func TestRouterFirstRegistrationWins(t *testing.T) {
86+
t.Run("exact", func(t *testing.T) {
87+
r, c := newRouter(t)
88+
r.Handle("GET", "/foo", handlerNamed("first"))
89+
r.Handle("GET", "/foo", handlerNamed("second"))
90+
91+
r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/foo", nil))
92+
assert.Equal(t, "first", c.handler)
93+
})
94+
95+
t.Run("wildcard", func(t *testing.T) {
96+
r, c := newRouter(t)
97+
r.Handle("GET", "/items/{id}", handlerNamed("first"))
98+
r.Handle("GET", "/items/{id}", handlerNamed("second"))
99+
100+
r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/items/42", nil))
101+
assert.Equal(t, "first", c.handler)
102+
})
103+
}
104+
105+
func TestRouterNotFound(t *testing.T) {
106+
r, c := newRouter(t)
107+
r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/missing", nil))
108+
assert.True(t, c.notFound)
109+
}
110+
111+
func TestRouterMethodNotAllowed(t *testing.T) {
112+
t.Run("exact", func(t *testing.T) {
113+
r, c := newRouter(t)
114+
r.Handle("GET", "/foo", handlerNamed("foo-get"))
115+
r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/foo", nil))
116+
assert.True(t, c.notFound, "wrong method on exact path should hit NotFound")
117+
assert.Empty(t, c.handler)
118+
})
119+
120+
t.Run("wildcard", func(t *testing.T) {
121+
r, c := newRouter(t)
122+
r.Handle("GET", "/items/{id}", handlerNamed("item-get"))
123+
r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/items/42", nil))
124+
assert.True(t, c.notFound, "wrong method on wildcard path should hit NotFound")
125+
assert.Empty(t, c.handler)
126+
})
127+
}
128+
129+
func TestRouterPercentEncodedSlash(t *testing.T) {
130+
r, c := newRouter(t)
131+
r.Handle("GET", "/files/{path...}", handlerNamed("files-get"))
132+
133+
req := httptest.NewRequest(http.MethodGet, "/files/a%2Fb%2Fc", nil)
134+
r.ServeHTTP(httptest.NewRecorder(), req)
135+
assert.Equal(t, "files-get", c.handler)
136+
assert.Equal(t, "a/b/c", c.vars["path"])
137+
}

0 commit comments

Comments
 (0)