From 45e55acfcd9c5e3f07e8653087db93086e55f659 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 13 Apr 2026 22:52:43 +0200 Subject: [PATCH 1/3] Replace gorilla/mux with Go 1.22 stdlib ServeMux in test server MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Go 1.22 added method-based routing ("GET /items/{id}"), path wildcards ({id}), and rest wildcards ({path...}) to net/http.ServeMux, which directly replaces the features we used gorilla/mux for. This removes the gorilla/mux dependency from go.mod and NOTICE. Exact (non-wildcard) paths are dispatched via a map lookup before reaching ServeMux. This avoids a ServeMux limitation where registering e.g. "GET /exact/path" alongside "HEAD /prefix/{path...}" panics because Go's implicit GET→HEAD handling creates an unresolvable precedence conflict. When an exact path is registered for one method but a request arrives for a different method, it falls through to ServeMux where the wildcard handler serves it. Wildcard patterns in test stubs (test.toml) must use the same placeholder names as the default handlers they coexist with. ServeMux panics on structurally identical patterns with different names (e.g. {name} vs {full_name}). Two test stubs are updated accordingly. Co-authored-by: Isaac --- NOTICE | 4 - acceptance/bundle/invariant/test.toml | 2 +- .../synced_database_tables/basic/test.toml | 2 +- acceptance/internal/prepare_server.go | 7 +- go.mod | 1 - go.sum | 2 - libs/testserver/handlers.go | 14 +- libs/testserver/server.go | 235 ++++++++++++------ 8 files changed, 169 insertions(+), 98 deletions(-) diff --git a/NOTICE b/NOTICE index 1e2aac0728b..2c90b58f2d3 100644 --- a/NOTICE +++ b/NOTICE @@ -79,10 +79,6 @@ Copyright (c) 2013 Dario Castañé. All rights reserved. Copyright (c) 2012 The Go Authors. All rights reserved. License - https://github.com/darccio/mergo/blob/master/LICENSE -gorilla/mux - https://github.com/gorilla/mux -Copyright (c) 2023 The Gorilla Authors. All rights reserved. -License - https://github.com/gorilla/mux/blob/main/LICENSE - palantir/pkg - https://github.com/palantir/pkg Copyright (c) 2016, Palantir Technologies, Inc. License - https://github.com/palantir/pkg/blob/master/LICENSE diff --git a/acceptance/bundle/invariant/test.toml b/acceptance/bundle/invariant/test.toml index beabef5ef1e..257e33005a3 100644 --- a/acceptance/bundle/invariant/test.toml +++ b/acceptance/bundle/invariant/test.toml @@ -81,5 +81,5 @@ Pattern = "POST /api/2.0/sql/statements/" Response.Body = '{"status": {"state": "SUCCEEDED"}, "manifest": {"schema": {"columns": []}}}' [[Server]] -Pattern = "DELETE /api/2.1/unity-catalog/tables/{name}" +Pattern = "DELETE /api/2.1/unity-catalog/tables/{full_name}" Response.Body = '{"status": "OK"}' diff --git a/acceptance/bundle/resources/synced_database_tables/basic/test.toml b/acceptance/bundle/resources/synced_database_tables/basic/test.toml index d41d9b917cc..191670590b5 100644 --- a/acceptance/bundle/resources/synced_database_tables/basic/test.toml +++ b/acceptance/bundle/resources/synced_database_tables/basic/test.toml @@ -20,5 +20,5 @@ Pattern = "POST /api/2.0/sql/statements/" Response.Body = '{"status": {"state": "SUCCEEDED"}, "manifest": {"schema": {"columns": []}}}' [[Server]] -Pattern = "DELETE /api/2.1/unity-catalog/tables/{name}" +Pattern = "DELETE /api/2.1/unity-catalog/tables/{full_name}" Response.Body = '{"status": "OK"}' diff --git a/acceptance/internal/prepare_server.go b/acceptance/internal/prepare_server.go index 702b4e145ef..8f18d1c61bc 100644 --- a/acceptance/internal/prepare_server.go +++ b/acceptance/internal/prepare_server.go @@ -188,8 +188,8 @@ func startLocalServer(t *testing.T, killCountersMu := &sync.Mutex{} for ind := range stubs { - // We want later stubs takes precedence, because then leaf configs take precedence over parent directory configs - // In gorilla/mux earlier handlers take precedence, so we need to reverse the order + // Later stubs take precedence over earlier ones (leaf configs override parent configs). + // The first handler registered for a given pattern wins, so we reverse the order. stub := stubs[len(stubs)-1-ind] require.NotEmpty(t, stub.Pattern) items := strings.Split(stub.Pattern, " ") @@ -226,7 +226,8 @@ func startLocalServer(t *testing.T, }) } - // The earliest handlers take precedence, add default handlers last + // The first handler registered for a given pattern wins, so default + // handlers registered last serve as fallbacks. testserver.AddDefaultHandlers(s) return s.URL } diff --git a/go.mod b/go.mod index 7b79b86e7f7..2e841c0d313 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,6 @@ require ( github.com/databricks/databricks-sdk-go v0.128.0 // Apache-2.0 github.com/google/jsonschema-go v0.4.3 // MIT github.com/google/uuid v1.6.0 // BSD-3-Clause - github.com/gorilla/mux v1.8.1 // BSD-3-Clause github.com/gorilla/websocket v1.5.3 // BSD-2-Clause github.com/hashicorp/go-version v1.9.0 // MPL-2.0 github.com/hashicorp/hc-install v0.9.3 // MPL-2.0 diff --git a/go.sum b/go.sum index 993ac401ffa..0d6543b5ceb 100644 --- a/go.sum +++ b/go.sum @@ -124,8 +124,6 @@ github.com/googleapis/enterprise-certificate-proxy v0.3.11 h1:vAe81Msw+8tKUxi2Dq github.com/googleapis/enterprise-certificate-proxy v0.3.11/go.mod h1:RFV7MUdlb7AgEq2v7FmMCfeSMCllAzWxFgRdusoGks8= github.com/googleapis/gax-go/v2 v2.17.0 h1:RksgfBpxqff0EZkDWYuz9q/uWsTVz+kf43LsZ1J6SMc= github.com/googleapis/gax-go/v2 v2.17.0/go.mod h1:mzaqghpQp4JDh3HvADwrat+6M3MOIDp5YKHhb9PAgDY= -github.com/gorilla/mux v1.8.1 h1:TuBL49tXwgrFYWhqrNgrUNEY92u81SPhu7sTdzQEiWY= -github.com/gorilla/mux v1.8.1/go.mod h1:AKf9I4AEqPTmMytcMc0KkNouC66V3BtZ4qD5fmWSiMQ= github.com/gorilla/websocket v1.5.3 h1:saDtZ6Pbx/0u+bgYQ3q96pZgCzfhKXGPqt7kZ72aNNg= github.com/gorilla/websocket v1.5.3/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ= diff --git a/libs/testserver/handlers.go b/libs/testserver/handlers.go index 8bd53391841..d98011fc7ba 100644 --- a/libs/testserver/handlers.go +++ b/libs/testserver/handlers.go @@ -109,7 +109,7 @@ func AddDefaultHandlers(server *Server) { return "" }) - server.Handle("POST", "/api/2.0/workspace-files/import-file/{path:.*}", func(req Request) any { + server.Handle("POST", "/api/2.0/workspace-files/import-file/{path...}", func(req Request) any { path := req.Vars["path"] overwrite := req.URL.Query().Get("overwrite") == "true" return req.Workspace.WorkspaceFilesImportFile(path, req.Body, overwrite) @@ -145,12 +145,12 @@ func AddDefaultHandlers(server *Server) { return req.Workspace.WorkspaceFilesImportFile(request.Path, decoded, request.Overwrite) }) - server.Handle("GET", "/api/2.0/workspace-files/{path:.*}", func(req Request) any { + server.Handle("GET", "/api/2.0/workspace-files/{path...}", func(req Request) any { path := req.Vars["path"] return req.Workspace.WorkspaceFilesExportFile(path) }) - server.Handle("HEAD", "/api/2.0/fs/directories/{path:.*}", func(req Request) any { + server.Handle("HEAD", "/api/2.0/fs/directories/{path...}", func(req Request) any { dirPath := req.Vars["path"] if !strings.HasPrefix(dirPath, "/") { dirPath = "/" + dirPath @@ -165,7 +165,7 @@ func AddDefaultHandlers(server *Server) { return Response{StatusCode: 404} }) - server.Handle("HEAD", "/api/2.0/fs/files/{path:.*}", func(req Request) any { + server.Handle("HEAD", "/api/2.0/fs/files/{path...}", func(req Request) any { path := req.Vars["path"] if req.Workspace.FileExists(path) { return Response{StatusCode: 200} @@ -173,7 +173,7 @@ func AddDefaultHandlers(server *Server) { return Response{StatusCode: 404} }) - server.Handle("PUT", "/api/2.0/fs/directories/{path:.*}", func(req Request) any { + server.Handle("PUT", "/api/2.0/fs/directories/{path...}", func(req Request) any { dirPath := req.Vars["path"] if !strings.HasPrefix(dirPath, "/") { dirPath = "/" + dirPath @@ -194,13 +194,13 @@ func AddDefaultHandlers(server *Server) { return Response{} }) - server.Handle("PUT", "/api/2.0/fs/files/{path:.*}", func(req Request) any { + server.Handle("PUT", "/api/2.0/fs/files/{path...}", func(req Request) any { path := req.Vars["path"] overwrite := req.URL.Query().Get("overwrite") == "true" return req.Workspace.WorkspaceFilesImportFile(path, req.Body, overwrite) }) - server.Handle("GET", "/api/2.0/fs/files/{path:.*}", func(req Request) any { + server.Handle("GET", "/api/2.0/fs/files/{path...}", func(req Request) any { path := req.Vars["path"] data := req.Workspace.WorkspaceFilesExportFile(path) if data == nil { diff --git a/libs/testserver/server.go b/libs/testserver/server.go index da354738682..cf3db41afdc 100644 --- a/libs/testserver/server.go +++ b/libs/testserver/server.go @@ -17,7 +17,6 @@ import ( "sync" "github.com/databricks/cli/internal/testutil" - "github.com/gorilla/mux" ) const testPidKey = "test-pid" @@ -39,10 +38,13 @@ func ExtractPidFromHeaders(headers http.Header) int { type Server struct { *httptest.Server - Router *mux.Router t testutil.TestingT + mux *http.ServeMux + wildcardMethods map[string]bool // "METHOD /pattern" -> registered + exactRoutes map[string]map[string]HandlerFunc // path -> method -> handler + fakeWorkspaces map[string]*FakeWorkspace fakeOidc *FakeOidc mu sync.Mutex @@ -84,7 +86,6 @@ func NewRequest(t testutil.TestingT, r *http.Request, fakeWorkspace *FakeWorkspa URL: r.URL, Headers: r.Header, Body: body, - Vars: mux.Vars(r), Workspace: fakeWorkspace, Context: r.Context(), } @@ -201,64 +202,52 @@ func getHeaders(value []byte) http.Header { } func New(t testutil.TestingT) *Server { - router := mux.NewRouter() - server := httptest.NewServer(router) - t.Cleanup(server.Close) + mux := http.NewServeMux() s := &Server{ - Server: server, - Router: router, - t: t, - fakeWorkspaces: map[string]*FakeWorkspace{}, - fakeOidc: &FakeOidc{url: server.URL}, + t: t, + mux: mux, + wildcardMethods: map[string]bool{}, + exactRoutes: map[string]map[string]HandlerFunc{}, + fakeWorkspaces: map[string]*FakeWorkspace{}, } - t.Cleanup(func() { - for _, ws := range s.fakeWorkspaces { - ws.Cleanup() + // Exact (non-wildcard) routes are kept out of ServeMux to avoid + // conflicts between method-specific exact paths and wildcard patterns + // (e.g., GET on an exact path vs HEAD on a wildcard that covers it). + // + // When an exact path is registered for one method but a request arrives + // for a different method, it intentionally falls through to ServeMux. + // This lets wildcard handlers serve methods not covered by the exact + // registration (e.g., a stub registers GET /exact, but HEAD /exact + // falls through to the wildcard HEAD handler). + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Clear RawPath so ServeMux matches decoded paths; without this, + // percent-encoded slashes (%2F) would not match literal slashes. + if r.URL.RawPath != "" { + r.URL.RawPath = "" } - }) - - // Set up the not found handler as fallback - notFoundFunc := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - pattern := r.Method + " " + r.URL.Path - bodyBytes, err := io.ReadAll(r.Body) - var body string - if err != nil { - body = fmt.Sprintf("failed to read the body: %s", err) - } else { - body = fmt.Sprintf("[%d bytes] %s", len(bodyBytes), bodyBytes) - } - - t.Errorf(`No handler for URL: %s -Body: %s - -For acceptance tests, add this to test.toml: -[[Server]] -Pattern = %q -Response.Body = '' -# Response.StatusCode = -`, r.URL, body, pattern) - - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusNotImplemented) - - resp := map[string]string{ - "message": "No stub found for pattern: " + pattern, + if methods, ok := s.exactRoutes[r.URL.Path]; ok { + if handler, ok := methods[r.Method]; ok { + s.serve(w, r, handler, nil) + return + } } + mux.ServeHTTP(w, r) + })) + t.Cleanup(server.Close) - respBytes, err := json.Marshal(resp) - if err != nil { - t.Errorf("JSON encoding error: %s", err) - respBytes = []byte("{\"message\": \"JSON encoding error\"}") - } + s.Server = server + s.fakeOidc = &FakeOidc{url: server.URL} - if _, err := w.Write(respBytes); err != nil { - t.Errorf("Response write error: %s", err) + t.Cleanup(func() { + for _, ws := range s.fakeWorkspaces { + ws.Cleanup() } }) - router.NotFoundHandler = notFoundFunc - router.MethodNotAllowedHandler = notFoundFunc + + // Register a catch-all handler as fallback for unmatched routes. + mux.HandleFunc("/", s.handleNotFound) // Register a default handler for the SDK's host metadata discovery endpoint. // The SDK resolves this during config initialization (as of v0.126.0) to @@ -292,46 +281,134 @@ func (s *Server) getWorkspaceForToken(token string) *FakeWorkspace { type HandlerFunc func(req Request) any +// Handle registers a handler for the given method and path pattern. +// First registration wins: subsequent calls with the same method+path are +// ignored. Exact paths are stored in a map checked before ServeMux; +// wildcard paths are registered with ServeMux using method-specific patterns. func (s *Server) Handle(method, path string, handler HandlerFunc) { - s.Router.HandleFunc(path, func(w http.ResponseWriter, r *http.Request) { - // Each test uses unique DATABRICKS_TOKEN, we simulate each token having - // it's own fake fakeWorkspace to avoid interference between tests. - fakeWorkspace := s.getWorkspaceForToken(getToken(r)) + if !strings.Contains(path, "{") { + s.handleExact(method, path, handler) + } else { + s.handleWildcard(method, path, handler) + } +} + +func (s *Server) handleExact(method, path string, handler HandlerFunc) { + if s.exactRoutes[path] == nil { + s.exactRoutes[path] = map[string]HandlerFunc{} + } + if _, exists := s.exactRoutes[path][method]; !exists { + s.exactRoutes[path][method] = handler + } +} - request := NewRequest(s.t, r, fakeWorkspace) +func (s *Server) handleWildcard(method, path string, handler HandlerFunc) { + pattern := method + " " + path + if s.wildcardMethods[pattern] { + return + } + s.wildcardMethods[pattern] = true - if s.RequestCallback != nil { - s.RequestCallback(&request) + names := wildcardNames(path) + s.mux.HandleFunc(pattern, func(w http.ResponseWriter, r *http.Request) { + vars := make(map[string]string, len(names)) + for _, name := range names { + vars[name] = r.PathValue(name) } + s.serve(w, r, handler, vars) + }) +} - var resp EncodedResponse - - if bytes.Contains(request.Body, []byte("INJECT_ERROR")) { - resp = EncodedResponse{ - StatusCode: 500, - Body: []byte("INJECTED"), - } - } else { - respAny := handler(request) - if respAny == nil && request.Context.Err() != nil { - return - } - resp = normalizeResponse(s.t, respAny) +// wildcardNames extracts wildcard parameter names from a path pattern, +// e.g. "/api/{id}/files/{path...}" returns ["id", "path"]. +func wildcardNames(path string) []string { + var names []string + for _, part := range strings.Split(path, "/") { + if strings.HasPrefix(part, "{") && strings.HasSuffix(part, "}") { + name := part[1 : len(part)-1] + name = strings.TrimSuffix(name, "...") + names = append(names, name) } + } + return names +} - maps.Copy(w.Header(), resp.Headers) +// serve is the common request handling logic for both exact and wildcard routes. +func (s *Server) serve(w http.ResponseWriter, r *http.Request, handler HandlerFunc, vars map[string]string) { + fakeWorkspace := s.getWorkspaceForToken(getToken(r)) - w.WriteHeader(resp.StatusCode) + request := NewRequest(s.t, r, fakeWorkspace) + request.Vars = vars - if s.ResponseCallback != nil { - s.ResponseCallback(&request, &resp) - } + if s.RequestCallback != nil { + s.RequestCallback(&request) + } + + var resp EncodedResponse - if _, err := w.Write(resp.Body); err != nil { - s.t.Errorf("Failed to write response: %s", err) + if bytes.Contains(request.Body, []byte("INJECT_ERROR")) { + resp = EncodedResponse{ + StatusCode: 500, + Body: []byte("INJECTED"), + } + } else { + respAny := handler(request) + if respAny == nil && request.Context.Err() != nil { return } - }).Methods(method) + resp = normalizeResponse(s.t, respAny) + } + + maps.Copy(w.Header(), resp.Headers) + + w.WriteHeader(resp.StatusCode) + + if s.ResponseCallback != nil { + s.ResponseCallback(&request, &resp) + } + + if _, err := w.Write(resp.Body); err != nil { + s.t.Errorf("Failed to write response: %s", err) + return + } +} + +func (s *Server) handleNotFound(w http.ResponseWriter, r *http.Request) { + pattern := r.Method + " " + r.URL.Path + bodyBytes, err := io.ReadAll(r.Body) + var body string + if err != nil { + body = fmt.Sprintf("failed to read the body: %s", err) + } else { + body = fmt.Sprintf("[%d bytes] %s", len(bodyBytes), bodyBytes) + } + + s.t.Errorf(`No handler for URL: %s +Body: %s + +For acceptance tests, add this to test.toml: +[[Server]] +Pattern = %q +Response.Body = '' +# Response.StatusCode = +`, r.URL, body, pattern) + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotImplemented) + + resp := map[string]string{ + "message": "No stub found for pattern: " + pattern, + } + + respBytes, err := json.Marshal(resp) + if err != nil { + s.t.Errorf("JSON encoding error: %s", err) + respBytes = []byte("{\"message\": \"JSON encoding error\"}") + } + + if _, err := w.Write(respBytes); err != nil { + s.t.Errorf("Response write error: %s", err) + } } func getToken(r *http.Request) string { From 2db96633e8f6d855a48823e59552b6b845bec27f Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 6 May 2026 10:37:53 +0200 Subject: [PATCH 2/3] Extract Router from testserver.Server Centralizes the method+path matching logic (exact routes, wildcard delegation to ServeMux, vars extraction, percent-encoded slash workaround) into a Router type with its own tests, and embeds it in Server. The package doc on Router explains why the wrapper exists on top of Go 1.22's ServeMux. Co-authored-by: Isaac --- libs/testserver/router.go | 124 ++++++++++++++++++++++ libs/testserver/router_test.go | 137 ++++++++++++++++++++++++ libs/testserver/server.go | 184 +++++++++------------------------ 3 files changed, 312 insertions(+), 133 deletions(-) create mode 100644 libs/testserver/router.go create mode 100644 libs/testserver/router_test.go diff --git a/libs/testserver/router.go b/libs/testserver/router.go new file mode 100644 index 00000000000..60c864ec209 --- /dev/null +++ b/libs/testserver/router.go @@ -0,0 +1,124 @@ +package testserver + +import ( + "net/http" + "strings" +) + +// HandlerFunc is the test-server handler signature. +type HandlerFunc func(req Request) any + +// Router maps method+path to a HandlerFunc. Wildcards use Go 1.22 ServeMux +// placeholder syntax ({name} or {name...}). +// +// # Why a custom router +// +// Go 1.22 added method matching ("GET /path") and {name}/{name...} +// placeholders to http.ServeMux, covering most of what we previously used +// gorilla/mux for. But two ServeMux behaviors make it inconvenient to use +// directly in the test server: +// +// - Exact and wildcard patterns conflict when they cover the same +// request under different methods. ServeMux treats `GET /x` as +// matching both GET and HEAD, so it overlaps with `HEAD /{path...}` +// and panics at registration. Test fixtures register both kinds of +// routes side by side, so we keep exact paths in our own map and +// only delegate wildcards to ServeMux. Exact lookup runs first; +// misses fall through to ServeMux, which also lets a wildcard +// handler serve methods that the exact registration doesn't cover. +// +// - ServeMux panics on duplicate pattern registration. Router silently +// drops the later registration — first wins. Two callers rely on this: +// AddDefaultHandlers (libs/testserver/handlers.go) installs fallback +// handlers that any test stub for the same pattern can override, and +// startLocalServer (acceptance/internal/prepare_server.go) iterates +// test.toml stubs in reverse so leaf-directory configs register first +// and win over inherited parent stubs. +// +// Router also clears req.URL.RawPath before dispatch so percent-encoded +// slashes (%2F) match literal slashes in patterns; workspace file paths +// in tests routinely contain encoded slashes. +type Router struct { + mux *http.ServeMux + exact map[string]map[string]HandlerFunc + wildcard map[string]bool + + // Dispatch is invoked when a route matches. vars holds the path values for + // wildcard routes and is nil for exact routes. + Dispatch func(w http.ResponseWriter, r *http.Request, h HandlerFunc, vars map[string]string) + // NotFound is invoked when no route matches. + NotFound http.HandlerFunc +} + +func NewRouter() *Router { + r := &Router{ + mux: http.NewServeMux(), + exact: map[string]map[string]HandlerFunc{}, + wildcard: map[string]bool{}, + } + r.mux.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) { + if r.NotFound != nil { + r.NotFound(w, req) + } + }) + return r +} + +// Handle registers a handler for method+path. First registration wins; +// duplicate (method, path) registrations are ignored. +func (r *Router) Handle(method, path string, handler HandlerFunc) { + if !strings.Contains(path, "{") { + if r.exact[path] == nil { + r.exact[path] = map[string]HandlerFunc{} + } + if _, ok := r.exact[path][method]; !ok { + r.exact[path][method] = handler + } + return + } + pattern := method + " " + path + if r.wildcard[pattern] { + return + } + r.wildcard[pattern] = true + names := wildcardNames(path) + r.mux.HandleFunc(pattern, func(w http.ResponseWriter, req *http.Request) { + vars := make(map[string]string, len(names)) + for _, name := range names { + vars[name] = req.PathValue(name) + } + if r.Dispatch != nil { + r.Dispatch(w, req, handler, vars) + } + }) +} + +// ServeHTTP routes a request to the registered handler, falling back to +// NotFound if no route matches. +func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { + // Force ServeMux to match against the decoded path; see the type doc. + req.URL.RawPath = "" + if methods, ok := r.exact[req.URL.Path]; ok { + if h, ok := methods[req.Method]; ok { + if r.Dispatch != nil { + r.Dispatch(w, req, h, nil) + } + return + } + } + r.mux.ServeHTTP(w, req) +} + +// wildcardNames extracts wildcard parameter names from a path pattern, +// e.g. "/api/{id}/files/{path...}" returns ["id", "path"]. +func wildcardNames(path string) []string { + var names []string + for _, part := range strings.Split(path, "/") { + if strings.HasPrefix(part, "{") && strings.HasSuffix(part, "}") { + name := part[1 : len(part)-1] + name = strings.TrimSuffix(name, "...") + names = append(names, name) + } + } + return names +} diff --git a/libs/testserver/router_test.go b/libs/testserver/router_test.go new file mode 100644 index 00000000000..f6b53d12e6e --- /dev/null +++ b/libs/testserver/router_test.go @@ -0,0 +1,137 @@ +package testserver_test + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/databricks/cli/libs/testserver" + "github.com/stretchr/testify/assert" +) + +type capture struct { + handler string + vars map[string]string + notFound bool +} + +func newRouter(t *testing.T) (*testserver.Router, *capture) { + t.Helper() + c := &capture{} + r := testserver.NewRouter() + r.Dispatch = func(w http.ResponseWriter, req *http.Request, h testserver.HandlerFunc, vars map[string]string) { + c.vars = vars + c.handler = h(testserver.Request{}).(string) + } + r.NotFound = func(w http.ResponseWriter, req *http.Request) { + c.notFound = true + } + return r, c +} + +func handlerNamed(name string) testserver.HandlerFunc { + return func(req testserver.Request) any { return name } +} + +func TestRouterExactMatch(t *testing.T) { + r, c := newRouter(t) + r.Handle("GET", "/foo", handlerNamed("foo-get")) + + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/foo", nil)) + assert.Equal(t, "foo-get", c.handler) + assert.Nil(t, c.vars) +} + +func TestRouterWildcardMatch(t *testing.T) { + r, c := newRouter(t) + r.Handle("GET", "/items/{id}", handlerNamed("item-get")) + + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/items/42", nil)) + assert.Equal(t, "item-get", c.handler) + assert.Equal(t, map[string]string{"id": "42"}, c.vars) +} + +func TestRouterCatchAllWildcard(t *testing.T) { + r, c := newRouter(t) + r.Handle("GET", "/files/{path...}", handlerNamed("files-get")) + + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/files/a/b/c", nil)) + assert.Equal(t, "files-get", c.handler) + assert.Equal(t, map[string]string{"path": "a/b/c"}, c.vars) +} + +func TestRouterMultipleWildcards(t *testing.T) { + r, c := newRouter(t) + r.Handle("GET", "/items/{id}/files/{path...}", handlerNamed("nested")) + + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/items/42/files/a/b", nil)) + assert.Equal(t, "nested", c.handler) + assert.Equal(t, map[string]string{"id": "42", "path": "a/b"}, c.vars) +} + +func TestRouterExactBeforeWildcard(t *testing.T) { + r, c := newRouter(t) + r.Handle("GET", "/foo", handlerNamed("exact")) + r.Handle("HEAD", "/{path...}", handlerNamed("wildcard-head")) + + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/foo", nil)) + assert.Equal(t, "exact", c.handler) + + c.handler = "" + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("HEAD", "/foo", nil)) + assert.Equal(t, "wildcard-head", c.handler) +} + +func TestRouterFirstRegistrationWins(t *testing.T) { + t.Run("exact", func(t *testing.T) { + r, c := newRouter(t) + r.Handle("GET", "/foo", handlerNamed("first")) + r.Handle("GET", "/foo", handlerNamed("second")) + + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/foo", nil)) + assert.Equal(t, "first", c.handler) + }) + + t.Run("wildcard", func(t *testing.T) { + r, c := newRouter(t) + r.Handle("GET", "/items/{id}", handlerNamed("first")) + r.Handle("GET", "/items/{id}", handlerNamed("second")) + + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/items/42", nil)) + assert.Equal(t, "first", c.handler) + }) +} + +func TestRouterNotFound(t *testing.T) { + r, c := newRouter(t) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/missing", nil)) + assert.True(t, c.notFound) +} + +func TestRouterMethodNotAllowed(t *testing.T) { + t.Run("exact", func(t *testing.T) { + r, c := newRouter(t) + r.Handle("GET", "/foo", handlerNamed("foo-get")) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("POST", "/foo", nil)) + assert.True(t, c.notFound, "wrong method on exact path should hit NotFound") + assert.Empty(t, c.handler) + }) + + t.Run("wildcard", func(t *testing.T) { + r, c := newRouter(t) + r.Handle("GET", "/items/{id}", handlerNamed("item-get")) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("POST", "/items/42", nil)) + assert.True(t, c.notFound, "wrong method on wildcard path should hit NotFound") + assert.Empty(t, c.handler) + }) +} + +func TestRouterPercentEncodedSlash(t *testing.T) { + r, c := newRouter(t) + r.Handle("GET", "/files/{path...}", handlerNamed("files-get")) + + req := httptest.NewRequest("GET", "/files/a%2Fb%2Fc", nil) + r.ServeHTTP(httptest.NewRecorder(), req) + assert.Equal(t, "files-get", c.handler) + assert.Equal(t, "a/b/c", c.vars["path"]) +} diff --git a/libs/testserver/server.go b/libs/testserver/server.go index cf3db41afdc..40556e55294 100644 --- a/libs/testserver/server.go +++ b/libs/testserver/server.go @@ -38,13 +38,10 @@ func ExtractPidFromHeaders(headers http.Header) int { type Server struct { *httptest.Server + *Router t testutil.TestingT - mux *http.ServeMux - wildcardMethods map[string]bool // "METHOD /pattern" -> registered - exactRoutes map[string]map[string]HandlerFunc // path -> method -> handler - fakeWorkspaces map[string]*FakeWorkspace fakeOidc *FakeOidc mu sync.Mutex @@ -202,43 +199,18 @@ func getHeaders(value []byte) http.Header { } func New(t testutil.TestingT) *Server { - mux := http.NewServeMux() + router := NewRouter() + server := httptest.NewServer(router) + t.Cleanup(server.Close) s := &Server{ - t: t, - mux: mux, - wildcardMethods: map[string]bool{}, - exactRoutes: map[string]map[string]HandlerFunc{}, - fakeWorkspaces: map[string]*FakeWorkspace{}, + Server: server, + Router: router, + t: t, + fakeWorkspaces: map[string]*FakeWorkspace{}, + fakeOidc: &FakeOidc{url: server.URL}, } - - // Exact (non-wildcard) routes are kept out of ServeMux to avoid - // conflicts between method-specific exact paths and wildcard patterns - // (e.g., GET on an exact path vs HEAD on a wildcard that covers it). - // - // When an exact path is registered for one method but a request arrives - // for a different method, it intentionally falls through to ServeMux. - // This lets wildcard handlers serve methods not covered by the exact - // registration (e.g., a stub registers GET /exact, but HEAD /exact - // falls through to the wildcard HEAD handler). - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Clear RawPath so ServeMux matches decoded paths; without this, - // percent-encoded slashes (%2F) would not match literal slashes. - if r.URL.RawPath != "" { - r.URL.RawPath = "" - } - if methods, ok := s.exactRoutes[r.URL.Path]; ok { - if handler, ok := methods[r.Method]; ok { - s.serve(w, r, handler, nil) - return - } - } - mux.ServeHTTP(w, r) - })) - t.Cleanup(server.Close) - - s.Server = server - s.fakeOidc = &FakeOidc{url: server.URL} + router.Dispatch = s.serve t.Cleanup(func() { for _, ws := range s.fakeWorkspaces { @@ -246,8 +218,45 @@ func New(t testutil.TestingT) *Server { } }) - // Register a catch-all handler as fallback for unmatched routes. - mux.HandleFunc("/", s.handleNotFound) + // Set up the not found handler as fallback + notFoundFunc := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + pattern := r.Method + " " + r.URL.Path + bodyBytes, err := io.ReadAll(r.Body) + var body string + if err != nil { + body = fmt.Sprintf("failed to read the body: %s", err) + } else { + body = fmt.Sprintf("[%d bytes] %s", len(bodyBytes), bodyBytes) + } + + t.Errorf(`No handler for URL: %s +Body: %s + +For acceptance tests, add this to test.toml: +[[Server]] +Pattern = %q +Response.Body = '' +# Response.StatusCode = +`, r.URL, body, pattern) + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotImplemented) + + resp := map[string]string{ + "message": "No stub found for pattern: " + pattern, + } + + respBytes, err := json.Marshal(resp) + if err != nil { + t.Errorf("JSON encoding error: %s", err) + respBytes = []byte("{\"message\": \"JSON encoding error\"}") + } + + if _, err := w.Write(respBytes); err != nil { + t.Errorf("Response write error: %s", err) + } + }) + router.NotFound = notFoundFunc // Register a default handler for the SDK's host metadata discovery endpoint. // The SDK resolves this during config initialization (as of v0.126.0) to @@ -279,62 +288,9 @@ func (s *Server) getWorkspaceForToken(token string) *FakeWorkspace { return s.fakeWorkspaces[token] } -type HandlerFunc func(req Request) any - -// Handle registers a handler for the given method and path pattern. -// First registration wins: subsequent calls with the same method+path are -// ignored. Exact paths are stored in a map checked before ServeMux; -// wildcard paths are registered with ServeMux using method-specific patterns. -func (s *Server) Handle(method, path string, handler HandlerFunc) { - if !strings.Contains(path, "{") { - s.handleExact(method, path, handler) - } else { - s.handleWildcard(method, path, handler) - } -} - -func (s *Server) handleExact(method, path string, handler HandlerFunc) { - if s.exactRoutes[path] == nil { - s.exactRoutes[path] = map[string]HandlerFunc{} - } - if _, exists := s.exactRoutes[path][method]; !exists { - s.exactRoutes[path][method] = handler - } -} - -func (s *Server) handleWildcard(method, path string, handler HandlerFunc) { - pattern := method + " " + path - if s.wildcardMethods[pattern] { - return - } - s.wildcardMethods[pattern] = true - - names := wildcardNames(path) - s.mux.HandleFunc(pattern, func(w http.ResponseWriter, r *http.Request) { - vars := make(map[string]string, len(names)) - for _, name := range names { - vars[name] = r.PathValue(name) - } - s.serve(w, r, handler, vars) - }) -} - -// wildcardNames extracts wildcard parameter names from a path pattern, -// e.g. "/api/{id}/files/{path...}" returns ["id", "path"]. -func wildcardNames(path string) []string { - var names []string - for _, part := range strings.Split(path, "/") { - if strings.HasPrefix(part, "{") && strings.HasSuffix(part, "}") { - name := part[1 : len(part)-1] - name = strings.TrimSuffix(name, "...") - names = append(names, name) - } - } - return names -} - -// serve is the common request handling logic for both exact and wildcard routes. func (s *Server) serve(w http.ResponseWriter, r *http.Request, handler HandlerFunc, vars map[string]string) { + // Each test uses unique DATABRICKS_TOKEN, we simulate each token having + // it's own fake fakeWorkspace to avoid interference between tests. fakeWorkspace := s.getWorkspaceForToken(getToken(r)) request := NewRequest(s.t, r, fakeWorkspace) @@ -373,44 +329,6 @@ func (s *Server) serve(w http.ResponseWriter, r *http.Request, handler HandlerFu } } -func (s *Server) handleNotFound(w http.ResponseWriter, r *http.Request) { - pattern := r.Method + " " + r.URL.Path - bodyBytes, err := io.ReadAll(r.Body) - var body string - if err != nil { - body = fmt.Sprintf("failed to read the body: %s", err) - } else { - body = fmt.Sprintf("[%d bytes] %s", len(bodyBytes), bodyBytes) - } - - s.t.Errorf(`No handler for URL: %s -Body: %s - -For acceptance tests, add this to test.toml: -[[Server]] -Pattern = %q -Response.Body = '' -# Response.StatusCode = -`, r.URL, body, pattern) - - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusNotImplemented) - - resp := map[string]string{ - "message": "No stub found for pattern: " + pattern, - } - - respBytes, err := json.Marshal(resp) - if err != nil { - s.t.Errorf("JSON encoding error: %s", err) - respBytes = []byte("{\"message\": \"JSON encoding error\"}") - } - - if _, err := w.Write(respBytes); err != nil { - s.t.Errorf("Response write error: %s", err) - } -} - func getToken(r *http.Request) string { header := r.Header.Get("Authorization") prefix := "Bearer " From 2b09c8889181c61335350f171067de90e352b4de Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Wed, 6 May 2026 11:26:56 +0200 Subject: [PATCH 3/3] Fix lint errors in testserver router Use strings.SplitSeq and http.MethodGet/Head/Post constants to satisfy the modernize and usestdlibvars linters. Co-authored-by: Isaac --- libs/testserver/router.go | 2 +- libs/testserver/router_test.go | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/libs/testserver/router.go b/libs/testserver/router.go index 60c864ec209..00381eae6a9 100644 --- a/libs/testserver/router.go +++ b/libs/testserver/router.go @@ -113,7 +113,7 @@ func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { // e.g. "/api/{id}/files/{path...}" returns ["id", "path"]. func wildcardNames(path string) []string { var names []string - for _, part := range strings.Split(path, "/") { + for part := range strings.SplitSeq(path, "/") { if strings.HasPrefix(part, "{") && strings.HasSuffix(part, "}") { name := part[1 : len(part)-1] name = strings.TrimSuffix(name, "...") diff --git a/libs/testserver/router_test.go b/libs/testserver/router_test.go index f6b53d12e6e..9d2c8c603e4 100644 --- a/libs/testserver/router_test.go +++ b/libs/testserver/router_test.go @@ -37,7 +37,7 @@ func TestRouterExactMatch(t *testing.T) { r, c := newRouter(t) r.Handle("GET", "/foo", handlerNamed("foo-get")) - r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/foo", nil)) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/foo", nil)) assert.Equal(t, "foo-get", c.handler) assert.Nil(t, c.vars) } @@ -46,7 +46,7 @@ func TestRouterWildcardMatch(t *testing.T) { r, c := newRouter(t) r.Handle("GET", "/items/{id}", handlerNamed("item-get")) - r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/items/42", nil)) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/items/42", nil)) assert.Equal(t, "item-get", c.handler) assert.Equal(t, map[string]string{"id": "42"}, c.vars) } @@ -55,7 +55,7 @@ func TestRouterCatchAllWildcard(t *testing.T) { r, c := newRouter(t) r.Handle("GET", "/files/{path...}", handlerNamed("files-get")) - r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/files/a/b/c", nil)) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/files/a/b/c", nil)) assert.Equal(t, "files-get", c.handler) assert.Equal(t, map[string]string{"path": "a/b/c"}, c.vars) } @@ -64,7 +64,7 @@ func TestRouterMultipleWildcards(t *testing.T) { r, c := newRouter(t) r.Handle("GET", "/items/{id}/files/{path...}", handlerNamed("nested")) - r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/items/42/files/a/b", nil)) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/items/42/files/a/b", nil)) assert.Equal(t, "nested", c.handler) assert.Equal(t, map[string]string{"id": "42", "path": "a/b"}, c.vars) } @@ -74,11 +74,11 @@ func TestRouterExactBeforeWildcard(t *testing.T) { r.Handle("GET", "/foo", handlerNamed("exact")) r.Handle("HEAD", "/{path...}", handlerNamed("wildcard-head")) - r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/foo", nil)) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/foo", nil)) assert.Equal(t, "exact", c.handler) c.handler = "" - r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("HEAD", "/foo", nil)) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodHead, "/foo", nil)) assert.Equal(t, "wildcard-head", c.handler) } @@ -88,7 +88,7 @@ func TestRouterFirstRegistrationWins(t *testing.T) { r.Handle("GET", "/foo", handlerNamed("first")) r.Handle("GET", "/foo", handlerNamed("second")) - r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/foo", nil)) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/foo", nil)) assert.Equal(t, "first", c.handler) }) @@ -97,14 +97,14 @@ func TestRouterFirstRegistrationWins(t *testing.T) { r.Handle("GET", "/items/{id}", handlerNamed("first")) r.Handle("GET", "/items/{id}", handlerNamed("second")) - r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/items/42", nil)) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/items/42", nil)) assert.Equal(t, "first", c.handler) }) } func TestRouterNotFound(t *testing.T) { r, c := newRouter(t) - r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("GET", "/missing", nil)) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodGet, "/missing", nil)) assert.True(t, c.notFound) } @@ -112,7 +112,7 @@ func TestRouterMethodNotAllowed(t *testing.T) { t.Run("exact", func(t *testing.T) { r, c := newRouter(t) r.Handle("GET", "/foo", handlerNamed("foo-get")) - r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("POST", "/foo", nil)) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/foo", nil)) assert.True(t, c.notFound, "wrong method on exact path should hit NotFound") assert.Empty(t, c.handler) }) @@ -120,7 +120,7 @@ func TestRouterMethodNotAllowed(t *testing.T) { t.Run("wildcard", func(t *testing.T) { r, c := newRouter(t) r.Handle("GET", "/items/{id}", handlerNamed("item-get")) - r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest("POST", "/items/42", nil)) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(http.MethodPost, "/items/42", nil)) assert.True(t, c.notFound, "wrong method on wildcard path should hit NotFound") assert.Empty(t, c.handler) }) @@ -130,7 +130,7 @@ func TestRouterPercentEncodedSlash(t *testing.T) { r, c := newRouter(t) r.Handle("GET", "/files/{path...}", handlerNamed("files-get")) - req := httptest.NewRequest("GET", "/files/a%2Fb%2Fc", nil) + req := httptest.NewRequest(http.MethodGet, "/files/a%2Fb%2Fc", nil) r.ServeHTTP(httptest.NewRecorder(), req) assert.Equal(t, "files-get", c.handler) assert.Equal(t, "a/b/c", c.vars["path"])