From 79a3f5f816fffec4646099be6ca8f58a427e7ade Mon Sep 17 00:00:00 2001 From: Amit Vijapur Date: Tue, 21 Jul 2026 23:16:27 +0800 Subject: [PATCH] feat(search): match server description as well as name The ?search= query on GET /v0/servers only matched server_name, so servers were undiscoverable by what they do unless the search term happened to appear in the name too (#135 added the name-only version of this). Add SubstringDescription to ServerFilter and have the handler set it alongside SubstringName from the same search term. buildFilterConditions combines the two into a single OR'd condition (server_name ILIKE ... OR value ->> 'description' ILIKE ...) rather than the AND every other filter pair uses, since requiring both would silently break the description half of the search. The description column lives inside the JSONB value blob (there's no dedicated column, unlike server_name), so the condition reads it via value ->> 'description'. Fixes #1453. --- internal/api/handlers/v0/servers.go | 5 +- internal/api/handlers/v0/servers_test.go | 14 ++- internal/database/database.go | 15 +-- internal/database/postgres.go | 37 +++++-- internal/database/postgres_internal_test.go | 104 ++++++++++++++++++++ internal/database/postgres_test.go | 44 ++++++++- 6 files changed, 201 insertions(+), 18 deletions(-) create mode 100644 internal/database/postgres_internal_test.go diff --git a/internal/api/handlers/v0/servers.go b/internal/api/handlers/v0/servers.go index 059095d28..60d0d2b98 100644 --- a/internal/api/handlers/v0/servers.go +++ b/internal/api/handlers/v0/servers.go @@ -58,7 +58,7 @@ type ListServersInput struct { Cursor string `query:"cursor" doc:"Pagination cursor" required:"false" example:"server-cursor-123"` Limit int `query:"limit" doc:"Number of items per page" default:"30" minimum:"1" maximum:"100" example:"50"` UpdatedSince string `query:"updated_since" doc:"Filter servers updated since timestamp (RFC3339 datetime)" required:"false" example:"2025-08-07T13:15:04.280Z"` - Search string `query:"search" doc:"Search servers by name (substring match)" required:"false" example:"filesystem"` + Search string `query:"search" doc:"Search servers by name or description (substring match)" required:"false" example:"filesystem"` Version string `query:"version" doc:"Filter by version ('latest' for latest version, or an exact version like '1.2.3')" required:"false" example:"latest"` IncludeDeleted OptionalBool `query:"include_deleted" doc:"Include deleted servers in results (default: false, but always true when updated_since is provided)" required:"false"` } @@ -105,9 +105,10 @@ func RegisterServersEndpoints(api huma.API, pathPrefix string, registry service. } } - // Handle search parameter + // Handle search parameter - matches against both name and description if input.Search != "" { filter.SubstringName = &input.Search + filter.SubstringDescription = &input.Search } // Handle version parameter diff --git a/internal/api/handlers/v0/servers_test.go b/internal/api/handlers/v0/servers_test.go index 16c763c4e..b611b7203 100644 --- a/internal/api/handlers/v0/servers_test.go +++ b/internal/api/handlers/v0/servers_test.go @@ -36,7 +36,7 @@ func TestListServersEndpoint(t *testing.T) { _, err = registryService.CreateServer(ctx, &apiv0.ServerJSON{ Schema: model.CurrentSchemaURL, Name: "com.example/server-beta", - Description: "Beta test server", + Description: "Beta test server for weather forecasting", Version: "2.0.0", }) require.NoError(t, err) @@ -71,6 +71,18 @@ func TestListServersEndpoint(t *testing.T) { expectedStatus: http.StatusOK, expectedCount: 1, }, + { + name: "search matches description when name does not match", + queryParams: "?search=weather", + expectedStatus: http.StatusOK, + expectedCount: 1, + }, + { + name: "search term absent from both name and description matches nothing", + queryParams: "?search=nonexistent-term-xyz", + expectedStatus: http.StatusOK, + expectedCount: 0, + }, { name: "filter latest only", queryParams: "?version=latest", diff --git a/internal/database/database.go b/internal/database/database.go index b4a797fae..a45a9537b 100644 --- a/internal/database/database.go +++ b/internal/database/database.go @@ -22,13 +22,14 @@ var ( // ServerFilter defines filtering options for server queries type ServerFilter struct { - Name *string // for finding versions of same server - RemoteURL *string // for duplicate URL detection - UpdatedSince *time.Time // for incremental sync filtering - SubstringName *string // for substring search on name - Version *string // for exact version matching - IsLatest *bool // for filtering latest versions only - IncludeDeleted *bool // for including deleted packages in results (default: exclude) + Name *string // for finding versions of same server + RemoteURL *string // for duplicate URL detection + UpdatedSince *time.Time // for incremental sync filtering + SubstringName *string // for substring search on name + SubstringDescription *string // for substring search on description + Version *string // for exact version matching + IsLatest *bool // for filtering latest versions only + IncludeDeleted *bool // for including deleted packages in results (default: exclude) } // Database defines the interface for database operations diff --git a/internal/database/postgres.go b/internal/database/postgres.go index b0b2d983b..efd04d7a3 100644 --- a/internal/database/postgres.go +++ b/internal/database/postgres.go @@ -117,14 +117,26 @@ func buildFilterConditions(filter *ServerFilter, argIndex int) ([]string, []any, args = append(args, *filter.UpdatedSince) argIndex++ } - if filter.SubstringName != nil { - // Escape LIKE metacharacters so that user input cannot expand into - // wildcard matches (e.g. `?search=_` matching every single-char name, - // `?search=%` matching everything). Order matters: backslashes must be - // escaped first so subsequent escape backslashes are not double-escaped. - escaped := strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`).Replace(*filter.SubstringName) + switch { + case filter.SubstringName != nil && filter.SubstringDescription != nil: + // The ?search= query param sets both fields to the same term so that + // a server matches on name OR description. These must be combined into + // a single OR condition here rather than left as two independent + // per-field checks below (which the outer AND-join would turn into + // "name matches AND description matches"). + conditions = append(conditions, fmt.Sprintf( + "(server_name ILIKE $%d ESCAPE '\\' OR value ->> 'description' ILIKE $%d ESCAPE '\\')", + argIndex, argIndex+1, + )) + args = append(args, likePattern(*filter.SubstringName), likePattern(*filter.SubstringDescription)) + argIndex += 2 + case filter.SubstringName != nil: conditions = append(conditions, fmt.Sprintf("server_name ILIKE $%d ESCAPE '\\'", argIndex)) - args = append(args, "%"+escaped+"%") + args = append(args, likePattern(*filter.SubstringName)) + argIndex++ + case filter.SubstringDescription != nil: + conditions = append(conditions, fmt.Sprintf("value ->> 'description' ILIKE $%d ESCAPE '\\'", argIndex)) + args = append(args, likePattern(*filter.SubstringDescription)) argIndex++ } if filter.Version != nil { @@ -144,6 +156,17 @@ func buildFilterConditions(filter *ServerFilter, argIndex int) ([]string, []any, return conditions, args, argIndex } +// likePattern escapes LIKE/ILIKE metacharacters in s and wraps it for a +// substring match, so that user input cannot expand into unintended wildcard +// matches (e.g. `?search=_` matching every single-char name, `?search=%` +// matching everything). Order matters: backslashes must be escaped first so +// subsequent escape backslashes are not double-escaped. Callers must pair +// this with `ESCAPE '\\'` in the ILIKE clause. +func likePattern(s string) string { + escaped := strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`).Replace(s) + return "%" + escaped + "%" +} + // addCursorCondition adds pagination cursor condition to WHERE clause. // // The compound cursor uses a row-constructor comparison so PostgreSQL can seek diff --git a/internal/database/postgres_internal_test.go b/internal/database/postgres_internal_test.go new file mode 100644 index 000000000..3f0879ac8 --- /dev/null +++ b/internal/database/postgres_internal_test.go @@ -0,0 +1,104 @@ +package database + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func ptr[T any](v T) *T { return &v } + +// TestBuildFilterConditions_SearchMatchesNameOrDescription covers the +// SubstringName/SubstringDescription combination directly against the SQL +// fragments buildFilterConditions produces. This is white-box on purpose: +// ListServers-level tests in postgres_test.go require a live PostgreSQL +// instance and can only assert on results, not on the generated WHERE +// clause. The behavior under test here is specifically that the ?search= +// query param (which the handler maps to both SubstringName and +// SubstringDescription set to the same term) must be OR'd together - every +// other pair of filter fields in ServerFilter is combined with AND, so this +// is the one case buildFilterConditions has to special-case to avoid +// silently requiring a match on both name and description at once. +func TestBuildFilterConditions_SearchMatchesNameOrDescription(t *testing.T) { + tests := []struct { + name string + filter *ServerFilter + wantConditions []string + wantArgs []any + }{ + { + name: "nil filter produces no conditions", + filter: nil, + wantConditions: nil, + wantArgs: nil, + }, + { + name: "SubstringName and SubstringDescription both set (the ?search= case) are OR'd", + filter: &ServerFilter{ + SubstringName: ptr("weather"), + SubstringDescription: ptr("weather"), + }, + wantConditions: []string{ + "(server_name ILIKE $1 ESCAPE '\\' OR value ->> 'description' ILIKE $2 ESCAPE '\\')", + "status != 'deleted'", + }, + wantArgs: []any{"%weather%", "%weather%"}, + }, + { + name: "SubstringName alone still matches only the name column", + filter: &ServerFilter{ + SubstringName: ptr("weather"), + }, + wantConditions: []string{ + "server_name ILIKE $1 ESCAPE '\\'", + "status != 'deleted'", + }, + wantArgs: []any{"%weather%"}, + }, + { + name: "SubstringDescription alone matches only the JSONB description field", + filter: &ServerFilter{ + SubstringDescription: ptr("weather"), + }, + wantConditions: []string{ + "value ->> 'description' ILIKE $1 ESCAPE '\\'", + "status != 'deleted'", + }, + wantArgs: []any{"%weather%"}, + }, + { + name: "LIKE metacharacters in the search term are escaped on both sides of the OR", + filter: &ServerFilter{ + SubstringName: ptr("100%_free"), + SubstringDescription: ptr("100%_free"), + }, + wantConditions: []string{ + "(server_name ILIKE $1 ESCAPE '\\' OR value ->> 'description' ILIKE $2 ESCAPE '\\')", + "status != 'deleted'", + }, + wantArgs: []any{`%100\%\_free%`, `%100\%\_free%`}, + }, + { + name: "combined search alongside other filters keeps AND semantics for the rest", + filter: &ServerFilter{ + SubstringName: ptr("weather"), + SubstringDescription: ptr("weather"), + IsLatest: ptr(true), + }, + wantConditions: []string{ + "(server_name ILIKE $1 ESCAPE '\\' OR value ->> 'description' ILIKE $2 ESCAPE '\\')", + "is_latest = $3", + "status != 'deleted'", + }, + wantArgs: []any{"%weather%", "%weather%", true}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotConditions, gotArgs, _ := buildFilterConditions(tt.filter, 1) + assert.Equal(t, tt.wantConditions, gotConditions) + assert.Equal(t, tt.wantArgs, gotArgs) + }) + } +} diff --git a/internal/database/postgres_test.go b/internal/database/postgres_test.go index 6784ae52d..e9904d9dd 100644 --- a/internal/database/postgres_test.go +++ b/internal/database/postgres_test.go @@ -246,6 +246,7 @@ func TestPostgreSQL_ListServers(t *testing.T) { remoteURL string isLatest bool publishedAt time.Time + description string }{ { name: "com.example/server-a", @@ -254,6 +255,7 @@ func TestPostgreSQL_ListServers(t *testing.T) { remoteURL: "https://api-a.example.com/mcp", isLatest: true, publishedAt: time.Now().Add(-2 * time.Hour), + description: "Provides real-time weather forecasts", }, { name: "com.example/server-b", @@ -262,6 +264,7 @@ func TestPostgreSQL_ListServers(t *testing.T) { remoteURL: "https://api-b.example.com/mcp", isLatest: true, publishedAt: time.Now().Add(-1 * time.Hour), + description: "Translates text between languages", }, { name: "com.example/server-c", @@ -270,6 +273,7 @@ func TestPostgreSQL_ListServers(t *testing.T) { remoteURL: "https://api-c.example.com/mcp", isLatest: true, publishedAt: time.Now().Add(-30 * time.Minute), + description: "A deprecated utility server", }, } @@ -277,7 +281,7 @@ func TestPostgreSQL_ListServers(t *testing.T) { for _, server := range testServers { serverJSON := &apiv0.ServerJSON{ Name: server.name, - Description: "Test server for listing", + Description: server.description, Version: server.version, Remotes: []model.Transport{ {Type: "http", URL: server.remoteURL}, @@ -337,6 +341,44 @@ func TestPostgreSQL_ListServers(t *testing.T) { limit: 10, expectedCount: 3, }, + { + name: "filter by substring description", + filter: &database.ServerFilter{ + SubstringDescription: stringPtr("weather"), + }, + limit: 10, + expectedCount: 1, + expectedNames: []string{"com.example/server-a"}, + }, + { + name: "substring description filter is case-insensitive substring, not exact match", + filter: &database.ServerFilter{ + SubstringDescription: stringPtr("LANGUAGES"), + }, + limit: 10, + expectedCount: 1, + expectedNames: []string{"com.example/server-b"}, + }, + { + name: "combined name+description search (the ?search= case) matches on description alone", + filter: &database.ServerFilter{ + SubstringName: stringPtr("weather"), // does not match any server_name + SubstringDescription: stringPtr("weather"), // matches server-a's description + }, + limit: 10, + expectedCount: 1, + expectedNames: []string{"com.example/server-a"}, + }, + { + name: "combined name+description search matches on name alone", + filter: &database.ServerFilter{ + SubstringName: stringPtr("server-b"), // matches server-b's name + SubstringDescription: stringPtr("no-such-phrase"), // matches nothing + }, + limit: 10, + expectedCount: 1, + expectedNames: []string{"com.example/server-b"}, + }, { name: "filter by version", filter: &database.ServerFilter{