From 654cb102e78e9ccaee5539ec6d9472b2ef5df0c8 Mon Sep 17 00:00:00 2001 From: Sebastian Webber Date: Mon, 20 Jul 2026 15:19:14 -0300 Subject: [PATCH] refactor: route REST v1 through tuning Use the shared tuning boundary so REST and newer consumers share one calculation entry point. Regression coverage protects the established REST contract during the migration. Signed-off-by: Sebastian Webber --- cmd/api/handlers/v1/config.go | 4 +- cmd/api/handlers/v1/config_test.go | 240 +++++++++++++++++++++++++++++ pkg/rules/tuning.go | 24 ++- pkg/rules/tuning_test.go | 11 +- 4 files changed, 275 insertions(+), 4 deletions(-) diff --git a/cmd/api/handlers/v1/config.go b/cmd/api/handlers/v1/config.go index 4d28dcf..82fb5d4 100644 --- a/cmd/api/handlers/v1/config.go +++ b/cmd/api/handlers/v1/config.go @@ -108,13 +108,13 @@ func processConfig(c *fiber.Ctx, args *configArgs) ([]category.SliceOutput, erro args.maxConn, args.pgVersion) - tune, err := rules.Compute(input) + tuningResult, err := rules.TuneCompatibility(input) if err != nil { return nil, err } - output := tune.ToSlice(args.pgVersion, args.includePgbadger, args.logFormat) + output := tuningResult.CompatibilityProjection().ToSlice(args.pgVersion, args.includePgbadger, args.logFormat) if args.showDoc { doc := pgDocs.Documentation[docs.FormatVer(args.pgVersion)] diff --git a/cmd/api/handlers/v1/config_test.go b/cmd/api/handlers/v1/config_test.go index 0b7d5fd..7ad7bee 100644 --- a/cmd/api/handlers/v1/config_test.go +++ b/cmd/api/handlers/v1/config_test.go @@ -1,14 +1,254 @@ package v1 import ( + "encoding/json" + "io" + "math" + "net/http" "net/http/httptest" + "strings" "testing" "github.com/gofiber/fiber/v2" + "github.com/pgconfig/api/pkg/category" + "github.com/pgconfig/api/pkg/format" + "github.com/pgconfig/api/pkg/input" + "github.com/pgconfig/api/pkg/input/bytes" "github.com/pgconfig/api/pkg/input/profile" + "github.com/pgconfig/api/pkg/rules" . "github.com/pgconfig/api/pkg/tests" ) +func TestGetConfigPreservesLegacyResponses(t *testing.T) { + if err := LoadConfig("../../../../rules.yml", "../../../../pg-docs.yml"); err != nil { + t.Fatal(err) + } + + tests := []struct { + name string + query string + request input.Input + includePGBadger bool + logFormat string + }{ + { + name: "defaults", + query: "", + request: defaultRESTInput(), + logFormat: "stderr", + }, + { + name: "representative explicit request", + query: "?pg_version=18.4&total_ram=64GB&max_connections=250" + + "&environment_name=OLTP&os_type=windows&arch=386" + + "&drive_type=SSD&cpus=16&include_pgbadger=true&log_format=jsonlog", + request: input.Input{ + OS: "windows", Arch: "386", TotalRAM: 64 * bytes.GB, + Profile: profile.OLTP, DiskType: "SSD", MaxConnections: 250, + TotalCPU: 16, PostgresVersion: 18.4, + }, + includePGBadger: true, + logFormat: "jsonlog", + }, + { + name: "uppercase operating system keeps legacy tuning", + query: "?os_type=WINDOWS", + request: withRESTOperatingSystem(defaultRESTInput(), "WINDOWS"), + logFormat: "stderr", + }, + { + name: "lowercase storage keeps legacy tuning", + query: "?drive_type=ssd", + request: withRESTDiskType(defaultRESTInput(), "ssd"), + logFormat: "stderr", + }, + { + name: "zero PostgreSQL version keeps legacy response", + query: "?pg_version=0", + request: withRESTPostgreSQLVersion(defaultRESTInput(), 0), + logFormat: "stderr", + }, + { + name: "negative PostgreSQL version keeps legacy response", + query: "?pg_version=-1", + request: withRESTPostgreSQLVersion(defaultRESTInput(), -1), + logFormat: "stderr", + }, + { + name: "NaN PostgreSQL version keeps legacy response", + query: "?pg_version=NaN", + request: withRESTPostgreSQLVersion(defaultRESTInput(), float32(math.NaN())), + logFormat: "stderr", + }, + { + name: "infinite PostgreSQL version keeps legacy response", + query: "?pg_version=Inf", + request: withRESTPostgreSQLVersion(defaultRESTInput(), float32(math.Inf(1))), + logFormat: "stderr", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + legacy, err := rules.Compute(tt.request) + if err != nil { + t.Fatal(err) + } + expected, err := json.Marshal(legacy.ToSlice( + tt.request.PostgresVersion, + tt.includePGBadger, + tt.logFormat, + )) + if err != nil { + t.Fatal(err) + } + + response := getConfigResponse(t, "/v1/tuning/get-config"+tt.query) + if response.StatusCode != fiber.StatusOK { + t.Fatalf("status = %d, want %d", response.StatusCode, fiber.StatusOK) + } + + var body struct { + Data json.RawMessage `json:"data"` + JSONAPI struct { + Version string `json:"version"` + } `json:"jsonapi"` + } + if err := json.NewDecoder(response.Body).Decode(&body); err != nil { + t.Fatal(err) + } + if string(body.Data) != string(expected) { + t.Fatalf("REST v1 data changed:\n got: %s\nwant: %s", body.Data, expected) + } + if body.JSONAPI.Version != "1.0" { + t.Fatalf("JSON:API version = %q, want %q", body.JSONAPI.Version, "1.0") + } + if !containsParameter(body.Data, "listen_addresses") { + t.Fatal("REST v1 response no longer contains listen_addresses") + } + }) + } +} + +func TestGetConfigPreservesLegacyValidation(t *testing.T) { + response := getConfigResponse(t, "/v1/tuning/get-config?arch=AMD64") + if response.StatusCode != fiber.StatusInternalServerError { + t.Fatalf("status = %d, want %d", response.StatusCode, fiber.StatusInternalServerError) + } +} + +func TestGetConfigPreservesLegacyFormats(t *testing.T) { + request := defaultRESTInput() + legacy, err := rules.Compute(request) + if err != nil { + t.Fatal(err) + } + legacyOutput := legacy.ToSlice(request.PostgresVersion, false, "stderr") + + for _, outputFormat := range []format.ExportFormat{format.Config, format.AlterSystemFormat} { + t.Run(string(outputFormat), func(t *testing.T) { + path := "/v1/tuning/get-config?format=" + string(outputFormat) + expected := format.ExportConf( + outputFormat, + legacyOutput, + request.PostgresVersion, + []string{"http://example.com" + path + "\n"}, + ) + + response := getConfigResponse(t, path) + actual, err := io.ReadAll(response.Body) + if err != nil { + t.Fatal(err) + } + if string(actual) != expected { + t.Fatalf("REST v1 %s output changed:\n got: %s\nwant: %s", outputFormat, actual, expected) + } + if !strings.Contains(string(actual), "listen_addresses") { + t.Fatal("REST v1 response no longer contains listen_addresses") + } + }) + } +} + +func TestGetConfigPreservesDocumentationEnrichment(t *testing.T) { + if err := LoadConfig("../../../../rules.yml", "../../../../pg-docs.yml"); err != nil { + t.Fatal(err) + } + + response := getConfigResponse(t, "/v1/tuning/get-config?show_doc=true") + + var body struct { + Data []category.SliceOutput `json:"data"` + } + if err := json.NewDecoder(response.Body).Decode(&body); err != nil { + t.Fatal(err) + } + for _, group := range body.Data { + for _, parameter := range group.Parameters { + if parameter.Name == "shared_buffers" { + if parameter.Documentation == nil || parameter.Documentation.Title == "" { + t.Fatal("shared_buffers documentation was not enriched") + } + return + } + } + } + t.Fatal("shared_buffers was not returned") +} + +func defaultRESTInput() input.Input { + return input.Input{ + OS: "linux", Arch: "amd64", TotalRAM: 2 * bytes.GB, + Profile: profile.Web, DiskType: "HDD", MaxConnections: 100, + TotalCPU: 2, PostgresVersion: 18, + } +} + +func withRESTOperatingSystem(request input.Input, operatingSystem string) input.Input { + request.OS = operatingSystem + return request +} + +func withRESTDiskType(request input.Input, diskType string) input.Input { + request.DiskType = diskType + return request +} + +func withRESTPostgreSQLVersion(request input.Input, postgresVersion float32) input.Input { + request.PostgresVersion = postgresVersion + return request +} + +func getConfigResponse(t *testing.T, path string) *http.Response { + t.Helper() + app := fiber.New() + app.Get("/v1/tuning/get-config", GetConfig) + response, err := app.Test(httptest.NewRequest("GET", path, nil)) + if err != nil { + t.Fatal(err) + } + return response +} + +func containsParameter(data json.RawMessage, name string) bool { + var categories []struct { + Parameters []struct { + Name string `json:"name"` + } `json:"parameters"` + } + if err := json.Unmarshal(data, &categories); err != nil { + return false + } + for _, category := range categories { + for _, parameter := range category.Parameters { + if parameter.Name == name { + return true + } + } + } + return false +} + func TestParseConfigArgs_ProfileCaseInsensitive(t *testing.T) { Describe("parseConfigArgs", t, func() { Context("profile parsing (issue #37)", func() { diff --git a/pkg/rules/tuning.go b/pkg/rules/tuning.go index fdf9cec..36475b1 100644 --- a/pkg/rules/tuning.go +++ b/pkg/rules/tuning.go @@ -73,7 +73,8 @@ func Tune(request TuningRequest) (*TuningResult, error) { if err != nil { return nil, err } - legacy, adjustments, err := computeWithAdjustments(normalized.legacyInput(rulesVersion)) + calculationRequest := normalized.legacyInput(rulesVersion) + legacy, adjustments, err := computeWithAdjustments(calculationRequest) if err != nil { return nil, err } @@ -99,6 +100,27 @@ func Tune(request TuningRequest) (*TuningResult, error) { }, nil } +// TuneCompatibility uses the shared tuning operation while retaining the +// historical projection and errors expected by legacy consumers. +func TuneCompatibility(legacyRequest input.Input) (*TuningResult, error) { + canonicalRequest := NewTuningRequest(legacyRequest) + result, tuningErr := Tune(canonicalRequest) + compatibility, _, compatibilityErr := computeWithAdjustments(legacyRequest) + if compatibilityErr != nil { + return nil, compatibilityErr + } + if tuningErr != nil { + return &TuningResult{ + Request: normalizeRequest(canonicalRequest), + Recommendations: map[string]TuningRecommendation{}, + ApplicationVersion: version.Pretty(), + legacy: compatibility, + }, nil + } + result.legacy = compatibility + return result, nil +} + // CompatibilityProjection returns the legacy category model used by REST v1 // and the CLI while those consumers are migrated. func (r *TuningResult) CompatibilityProjection() *category.ExportCfg { diff --git a/pkg/rules/tuning_test.go b/pkg/rules/tuning_test.go index 2d435a3..48ef9a1 100644 --- a/pkg/rules/tuning_test.go +++ b/pkg/rules/tuning_test.go @@ -333,6 +333,15 @@ func TestTuneNormalizesPostgreSQLVersionWhitespace(t *testing.T) { } } +func TestTuneKeepsCanonicalVersionValidationStrict(t *testing.T) { + request := validTuningRequest() + request.PostgreSQLVersion = "0" + + if _, err := Tune(request); err == nil { + t.Fatal("canonical tuning accepted an invalid PostgreSQL Version") + } +} + func TestTuneIsDeterministic(t *testing.T) { request := NewTuningRequest(validLegacyInput()) @@ -361,7 +370,7 @@ func TestCompatibilityProjectionMatchesCompute(t *testing.T) { if err != nil { t.Fatal(err) } - result, err := Tune(NewTuningRequest(in)) + result, err := TuneCompatibility(in) if err != nil { t.Fatal(err) }