ModelRouter/internal/gateway/source_fields_test.go

package gateway

import (
	"encoding/json"
	"os"
	"path/filepath"
	"reflect"
	"strings"
	"testing"
	"time"

	"modelrouter/internal/config"
	"modelrouter/internal/core"
)

// This file covers the second round of the same class of defect bf0657b fixed.
//
// bf0657b stopped a partial source edit from clobbering api_key. The upsert
// still rewrites the WHOLE source, so every other field the request could not
// express was reset to its zero value on any edit: proxy_url, api_key_env,
// timeout and queue_timeout.
//
// The four are worse than cosmetic because they change how the source dials out.
// proxy_url silently turns a proxied upstream into a direct one (or vice versa)
// and api_key_env turns a source whose credential lives in an environment
// variable into a credential-less source — which keeps answering 200 on write
// and only fails on the NEXT call upstream, long after the editor left.
//
// The trigger is not exotic scripting: the WebUI's own edit form posts exactly
// such a payload. See TestWebUIEditPayloadPreservesRoutingFields.

// richGateway writes a config.yaml whose source carries all four fragile
// fields, then boots a gateway over it.
func richGateway(t *testing.T) *Gateway {
	t.Helper()
	dir := t.TempDir()
	cfgPath := filepath.Join(dir, "config.yaml")
	cfg := `listen: 127.0.0.1:8080
gateway_keys:
  - sk-test
default_model: AUTO
adapter_dir: ` + filepath.Join(dir, "adapters") + `
runtime_file: ` + filepath.Join(dir, "runtime.json") + `
sources:
  - name: rich
    base_url: https://api.deepseek.com
    api_key: sk-live
    adapter: deepseek
    proxy_url: http://127.0.0.1:7890
    timeout: 300s
    queue_timeout: 90s
    max_concurrent: 8
    models:
      - id: deepseek-v4-flash
        priority: 100
        kind: chat
`
	if err := os.WriteFile(cfgPath, []byte(cfg), 0600); err != nil {
		t.Fatal(err)
	}
	c, err := config.Load(cfgPath)
	if err != nil {
		t.Fatal(err)
	}
	core, err := core.NewFromConfig(c)
	if err != nil {
		t.Fatalf("core: %v", err)
	}
	t.Cleanup(core.Close)
	g, err := New(core)
	if err != nil {
		t.Fatal(err)
	}
	return g
}

// sourceField reads one field off the live source, i.e. what the routes
// actually use rather than what the file happens to contain.
func sourceField(t *testing.T, g *Gateway, name string, get func(config.Source) interface{}) interface{} {
	t.Helper()
	for _, s := range g.core.Sources() {
		if s.Name == name {
			return get(s)
		}
	}
	t.Fatalf("source %q not found", name)
	return nil
}

// webUIPayload is byte-for-byte the JSON that index.html's saveSource() builds
// from the edit dialog. It deliberately carries none of the four fragile
// fields, because the dialog has no inputs for them.
const webUIPayload = `{
  "name":"rich","base_url":"https://api.deepseek.com","api_key":"sk-live",
  "adapter":"deepseek","endpoint":"","image_endpoint":"",
  "models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}],
  "headers":{},"meta":{},"temperature":0,"max_tokens":0,"max_concurrent":16,"rpm":0
}`

// TestWebUIEditPayloadPreservesRoutingFields is the regression test for the
// reported defect: changing max_concurrent from 8 to 16 in the UI must not
// silently drop the proxy, the env-var credential, or the timeouts.
func TestWebUIEditPayloadPreservesRoutingFields(t *testing.T) {
	g := richGateway(t)

	// The one field the operator actually changed.
	rr := doReq(t, g, "POST", "/api/sources", webUIPayload)
	if rr.Code != 200 {
		t.Fatalf("edit status=%d body=%s", rr.Code, rr.Body.String())
	}

	checks := []struct {
		field string
		got   interface{}
		want  interface{}
	}{
		{"proxy_url", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.ProxyURL }),
			"http://127.0.0.1:7890"},
		{"timeout", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.Timeout }),
			300 * time.Second},
		{"queue_timeout", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.QueueTimeout }),
			90 * time.Second},
		// The field the edit DID change, so a fix that simply refuses to write
		// anything would also pass the three assertions above.
		{"max_concurrent", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.MaxConcurrent }),
			16},
	}
	for _, c := range checks {
		if c.got != c.want {
			t.Errorf("%s = %v, want %v", c.field, c.got, c.want)
		}
	}
}

// TestSourceEditPreservesAPIKeyEnv is called out separately because it is the
// only one of the four whose loss is a credential failure rather than a routing
// difference, and it is invisible until the next upstream call.
func TestSourceEditPreservesAPIKeyEnv(t *testing.T) {
	dir := t.TempDir()
	cfgPath := filepath.Join(dir, "config.yaml")
	cfg := `listen: 127.0.0.1:8080
gateway_keys:
  - sk-test
default_model: AUTO
adapter_dir: ` + filepath.Join(dir, "adapters") + `
runtime_file: ` + filepath.Join(dir, "runtime.json") + `
sources:
  - name: envonly
    base_url: https://api.deepseek.com
    api_key_env: DEEPSEEK_KEY
    adapter: deepseek
    max_concurrent: 8
    models:
      - id: deepseek-v4-flash
        priority: 100
        kind: chat
`
	if err := os.WriteFile(cfgPath, []byte(cfg), 0600); err != nil {
		t.Fatal(err)
	}
	c, err := config.Load(cfgPath)
	if err != nil {
		t.Fatal(err)
	}
	core, err := core.NewFromConfig(c)
	if err != nil {
		t.Fatalf("core: %v", err)
	}
	t.Cleanup(core.Close)
	g, err := New(core)
	if err != nil {
		t.Fatal(err)
	}

	// A credential-less source is legitimate, so the API must not invent one
	// here; the point is that the ENV reference survives the edit.
	rr := doReq(t, g, "POST", "/api/sources", `{
      "name":"envonly","base_url":"https://api.deepseek.com","api_key":"",
      "adapter":"deepseek","models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}],
      "headers":{},"meta":{},"max_concurrent":4}`)
	if rr.Code != 200 {
		t.Fatalf("edit status=%d body=%s", rr.Code, rr.Body.String())
	}
	got := sourceField(t, g, "envonly", func(s config.Source) interface{} { return s.APIKeyEnv })
	if got != "DEEPSEEK_KEY" {
		t.Fatalf("api_key_env = %v after an edit that did not mention it; "+
			"the source now has no credential and will only fail on the next upstream call", got)
	}
	if n := sourceField(t, g, "envonly", func(s config.Source) interface{} { return s.MaxConcurrent }); n != 4 {
		t.Fatalf("max_concurrent = %v, want the edited 4", n)
	}
}

// TestSourceEditPersistsInheritedFieldsToDisk: inheriting in memory is not
// enough — the values must reach config.yaml, or a restart silently undoes the
// fix and the field is lost anyway.
func TestSourceEditPersistsInheritedFieldsToDisk(t *testing.T) {
	g := richGateway(t)
	if rr := doReq(t, g, "POST", "/api/sources", webUIPayload); rr.Code != 200 {
		t.Fatalf("edit status=%d: %s", rr.Code, rr.Body.String())
	}
	raw, err := os.ReadFile(g.core.Config().Path)
	if err != nil {
		t.Fatalf("read config: %v", err)
	}
	text := string(raw)
	for _, want := range []string{
		"proxy_url: http://127.0.0.1:7890",
	} {
		if !strings.Contains(text, want) {
			t.Errorf("config.yaml lost %q on an unrelated edit:\n%s", want, text)
		}
	}
	// The durations are re-serialized in Go's canonical form (300s becomes
	// 5m0s), so they must be checked SEMANTICALLY. Asserting the original
	// spelling would be a false red: the value survived, only its text changed.
	// Re-loading the file is also the only way to prove the persisted value
	// parses back to the same duration.
	reloaded, err := config.Load(g.core.Config().Path)
	if err != nil {
		t.Fatalf("re-read config: %v", err)
	}
	if len(reloaded.Sources) == 0 {
		t.Fatal("no sources after reload")
	}
	s := reloaded.Sources[0]
	if s.Timeout != 300*time.Second {
		t.Errorf("persisted timeout = %v, want 300s", s.Timeout)
	}
	if s.QueueTimeout != 90*time.Second {
		t.Errorf("persisted queue_timeout = %v, want 90s", s.QueueTimeout)
	}
	if s.ProxyURL != "http://127.0.0.1:7890" {
		t.Errorf("persisted proxy_url = %v, want it preserved", s.ProxyURL)
	}
}

// ---- explicit set / clear, i.e. the pointer semantics actually work ----

func TestSourceOptionalFieldsCanBeSet(t *testing.T) {
	g := richGateway(t)
	rr := doReq(t, g, "POST", "/api/sources", `{
      "name":"rich","base_url":"https://api.deepseek.com","api_key":"sk-live",
      "adapter":"deepseek","models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}],
      "headers":{},"meta":{},
      "proxy_url":"http://127.0.0.1:1080",
      "api_key_env":"NEW_ENV_KEY",
      "timeout":"45s",
      "queue_timeout":"5s"}`)
	if rr.Code != 200 {
		t.Fatalf("status=%d body=%s", rr.Code, rr.Body.String())
	}
	for _, c := range []struct {
		field string
		got   interface{}
		want  interface{}
	}{
		{"proxy_url", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.ProxyURL }), "http://127.0.0.1:1080"},
		{"api_key_env", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.APIKeyEnv }), "NEW_ENV_KEY"},
		{"timeout", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.Timeout }), 45 * time.Second},
		{"queue_timeout", sourceField(t, g, "rich", func(s config.Source) interface{} { return s.QueueTimeout }), 5 * time.Second},
	} {
		if c.got != c.want {
			t.Errorf("%s = %v, want %v", c.field, c.got, c.want)
		}
	}
}

// TestSourceOptionalFieldsCanBeCleared is the half that an "empty means
// inherit" implementation gets wrong: it can never clear a field, so emptying
// the proxy box in the UI would keep using the old proxy forever.
func TestSourceOptionalFieldsCanBeCleared(t *testing.T) {
	g := richGateway(t)
	rr := doReq(t, g, "POST", "/api/sources", `{
      "name":"rich","base_url":"https://api.deepseek.com","api_key":"sk-live",
      "adapter":"deepseek","models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}],
      "headers":{},"meta":{},
      "proxy_url":"","timeout":"","queue_timeout":""}`)
	if rr.Code != 200 {
		t.Fatalf("status=%d body=%s", rr.Code, rr.Body.String())
	}
	if got := sourceField(t, g, "rich", func(s config.Source) interface{} { return s.ProxyURL }); got != "" {
		t.Errorf("proxy_url = %v, want it cleared", got)
	}
	// A cleared timeout falls back to the config default (120s), which is what
	// mergedSources applies for a zero value — assert the effective behaviour
	// rather than the raw zero, since that is what a caller actually gets.
	if got := sourceField(t, g, "rich", func(s config.Source) interface{} { return s.Timeout }); got != config.DefaultSourceTimeout {
		t.Errorf("timeout = %v, want the default %v after clearing", got, config.DefaultSourceTimeout)
	}
	// api_key_env was NOT in the request, so it must be untouched even though
	// its sibling fields were cleared.
	_ = rr
}

// TestSourceOptionalFieldsClearIsScoped: clearing the proxy must not clear the
// env credential that the same request did not mention.
func TestSourceOptionalFieldsClearIsScoped(t *testing.T) {
	dir := t.TempDir()
	cfgPath := filepath.Join(dir, "config.yaml")
	cfg := `listen: 127.0.0.1:8080
gateway_keys:
  - sk-test
default_model: AUTO
adapter_dir: ` + filepath.Join(dir, "adapters") + `
runtime_file: ` + filepath.Join(dir, "runtime.json") + `
sources:
  - name: both
    base_url: https://api.deepseek.com
    api_key_env: DEEPSEEK_KEY
    adapter: deepseek
    proxy_url: http://127.0.0.1:7890
    max_concurrent: 8
    models:
      - id: deepseek-v4-flash
        priority: 100
        kind: chat
`
	os.WriteFile(cfgPath, []byte(cfg), 0600)
	c, err := config.Load(cfgPath)
	if err != nil {
		t.Fatal(err)
	}
	core, err := core.NewFromConfig(c)
	if err != nil {
		t.Fatalf("core: %v", err)
	}
	t.Cleanup(core.Close)
	g, err := New(core)
	if err != nil {
		t.Fatal(err)
	}
	rr := doReq(t, g, "POST", "/api/sources", `{
      "name":"both","base_url":"https://api.deepseek.com","api_key":"",
      "adapter":"deepseek","models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}],
      "headers":{},"meta":{},"proxy_url":""}`)
	if rr.Code != 200 {
		t.Fatalf("status=%d body=%s", rr.Code, rr.Body.String())
	}
	if got := sourceField(t, g, "both", func(s config.Source) interface{} { return s.APIKeyEnv }); got != "DEEPSEEK_KEY" {
		t.Errorf("api_key_env = %v; clearing proxy_url must not touch it", got)
	}
	if got := sourceField(t, g, "both", func(s config.Source) interface{} { return s.ProxyURL }); got != "" {
		t.Errorf("proxy_url = %v, want cleared", got)
	}
}

func TestSourceRejectsBadDuration(t *testing.T) {
	g := richGateway(t)
	before := sourceField(t, g, "rich", func(s config.Source) interface{} { return s.Timeout })
	rr := doReq(t, g, "POST", "/api/sources", `{
      "name":"rich","base_url":"https://api.deepseek.com","api_key":"sk-live",
      "adapter":"deepseek","models":[{"id":"deepseek-v4-flash","priority":100,"kind":"chat"}],
      "headers":{},"meta":{},"timeout":"not-a-duration"}`)
	if rr.Code != 400 {
		t.Fatalf("status=%d, want 400 for an unparseable duration: %s", rr.Code, rr.Body.String())
	}
	if !strings.Contains(rr.Body.String(), "timeout") {
		t.Errorf("the error must name the offending field: %s", rr.Body.String())
	}
	// A rejected write must leave the source exactly as it was.
	if after := sourceField(t, g, "rich", func(s config.Source) interface{} { return s.Timeout }); after != before {
		t.Errorf("a rejected edit still changed timeout: %v -> %v", before, after)
	}
}

// TestSourceCreateSetsOptionalFields: the create path must accept them too, and
// must not require an existing source to inherit from (a nil-pointer crash
// here would only show up as a 500 on every new source).
func TestSourceCreateSetsOptionalFields(t *testing.T) {
	up := mockUpstream()
	defer up.Close()
	g := newTestGateway(t)
	rr := doReq(t, g, "POST", "/api/sources", `{
      "name":"fresh","base_url":"`+up.URL+`","api_key":"sk-x","adapter":"openai",
      "models":[{"id":"m1","kind":"chat"}],"headers":{},"meta":{},
      "proxy_url":"http://127.0.0.1:7890","timeout":"10s","queue_timeout":"3s"}`)
	if rr.Code != 200 {
		t.Fatalf("create status=%d body=%s", rr.Code, rr.Body.String())
	}
	if got := sourceField(t, g, "fresh", func(s config.Source) interface{} { return s.ProxyURL }); got != "http://127.0.0.1:7890" {
		t.Errorf("proxy_url = %v, want it set on create", got)
	}
	if got := sourceField(t, g, "fresh", func(s config.Source) interface{} { return s.Timeout }); got != 10*time.Second {
		t.Errorf("timeout = %v, want 10s on create", got)
	}
}

// TestSourceRevealExposesDurations guards the round-trip the edit form depends
// on. config.Source tags Timeout/QueueTimeout `json:"-"`, so marshaling the
// struct omits them; the form then reads an empty timeout box and — because it
// always sends the box back — CLEARS the stored timeout on any unrelated save.
// A test that only checks "the edit preserves timeout" would not have caught
// that, because the loss happens between the read and the write.
//
// The credentials are asserted in the same breath: they came from the same
// branch, and an earlier iteration of this fix masked the key and broke the
// form's other half.
func TestSourceRevealExposesDurations(t *testing.T) {
	up := mockUpstream()
	defer up.Close()
	g := newTestGateway(t, config.Source{
		Name:    "revealed",
		BaseURL: up.URL,
		Adapter: "openai",
		APIKey:  "sk-reveal-me",
		Models:  []config.Model{{ID: "m1", Kind: "chat"}},
		Timeout: 300 * time.Second,
	})

	rr := doReq(t, g, "GET", "/api/v1/sources/revealed?reveal=credentials", "")
	if rr.Code != 200 {
		t.Fatalf("reveal status=%d: %s", rr.Code, rr.Body.String())
	}
	var view struct {
		Source map[string]interface{} `json:"source"`
	}
	if err := json.Unmarshal(rr.Body.Bytes(), &view); err != nil {
		t.Fatalf("decode: %v", err)
	}
	// Duration strings, matching what the write path parses.
	if got := view.Source["timeout"]; got != "5m0s" {
		t.Errorf("reveal timeout = %v, want the string \"5m0s\"; the edit form "+
			"would show an empty box and clear the stored value on save", got)
	}
	// The unset one shows the DEFAULT, not "": ApplyDefaults filled
	// QueueTimeout before the form ever sees it. Both are acceptable as long as
	// the value round-trips, so assert it is a parseable duration string.
	qs, _ := view.Source["queue_timeout"].(string)
	if _, err := time.ParseDuration(qs); err != nil {
		t.Errorf("reveal queue_timeout = %q is not a duration string: %v", qs, err)
	}
	if got := view.Source["api_key"]; got != "sk-reveal-me" {
		t.Errorf("reveal api_key = %v, want it in the clear for the form", got)
	}
	// The masked view must NOT gain the durations' secrets — it is fine for it
	// to omit them, but it must never carry the key.
	masked := doReq(t, g, "GET", "/api/v1/sources/revealed", "")
	if strings.Contains(masked.Body.String(), "sk-reveal-me") {
		t.Error("the masked view leaked the key")
	}
}

// TestSourcePayloadCoversEveryEditableField is the guard against the class
// itself. Any config.Source field a source edit can legitimately set must
// exist on sourcePayload, otherwise the upsert resets it.
//
// It reflects over the STRUCT rather than over json.Marshal output: the four
// pointer fields carry `omitempty`, so a nil pointer legitimately disappears
// from the marshaled form — that absence is precisely the "not mentioned" signal
// the handler relies on. Checking marshaled keys would therefore report a
// healthy field as missing.
func TestSourcePayloadCoversEveryEditableField(t *testing.T) {
	// Fields of config.Source that a source edit can legitimately set.
	want := []string{
		"name", "base_url", "api_key", "api_key_env", "adapter", "endpoint",
		"image_endpoint", "models", "headers", "proxy_url", "meta",
		"temperature", "max_tokens", "max_concurrent", "rpm",
		"timeout", "queue_timeout",
	}
	pt := reflect.TypeOf(sourcePayload{})
	for _, f := range want {
		if _, ok := pt.FieldByName(f); !ok {
			// Tolerate a differently-spelled Go field only when the JSON tag
			// matches, so the check stays about the wire contract.
			if !hasJSONTag(pt, f) {
				t.Errorf("config.Source has %q but sourcePayload has no such JSON key: "+
					"an edit that omits it resets the stored value", f)
			}
		}
	}
}

// hasJSONTag reports whether any field of t carries the given JSON name.
func hasJSONTag(t reflect.Type, name string) bool {
	for i := 0; i < t.NumField(); i++ {
		if jsonName(t.Field(i)) == name {
			return true
		}
	}
	return false
}

// jsonName returns the JSON name of a struct field, falling back to the Go
// field name when no tag is present.
func jsonName(f reflect.StructField) string {
	tag := f.Tag.Get("json")
	if tag == "" {
		return f.Name
	}
	if n := strings.Split(tag, ",")[0]; n != "" {
		return n
	}
	return f.Name
}