diff --git a/cmd/api.go b/cmd/api.go index 54ea766..12e443b 100644 --- a/cmd/api.go +++ b/cmd/api.go @@ -50,7 +50,7 @@ Examples: func init() { apiCmd.Flags().StringP("method", "X", "", "HTTP method (default: GET, or POST if --json or --file is provided)") apiCmd.Flags().StringArray("file", nil, "Send the request as multipart/form-data with a file part: --file @path, or --file field=@path to name the part (repeatable). --json then supplies the request's other form fields") - apiCmd.Flags().Bool("no-preflight", false, "Send the request even if the path is absent from the API spec") + apiCmd.Flags().Bool("no-preflight", false, "Send the request even if the path is absent from the API spec; it cannot make a missing endpoint exist") rootCmd.AddCommand(apiCmd) } @@ -315,13 +315,21 @@ func preflightPath(cmd *cobra.Command, c *client.Client, httpMethod, resolvedPat suggestions := idx.Suggest(resolvedPath, 5) hint := "run 'cio schema' to list resources" - if len(suggestions) > 0 && suggestions[0].Resource != "" { - hint = fmt.Sprintf("run 'cio schema %s' to list that resource's endpoints", suggestions[0].Resource) + // Suggest ranks by shared leading segments, so an unknown resource still + // gets a neighbour from under the same scope — a different resource. + if requested := routes.ResourceFor(resolvedPath); requested != "" { + for _, s := range suggestions { + if s.Resource == requested { + hint = fmt.Sprintf("run 'cio schema %s' to list that resource's endpoints", requested) + break + } + } } // The spec omits a few real endpoints, and for those `cio schema` cannot // list what it does not describe — so the way past a wrong rejection has to // be in the message, not only in the details below. - hint += ", or resend with --no-preflight if you know the endpoint exists" + hint += ". --no-preflight resends without this check, but it cannot make a missing endpoint exist: " + + "an unknown path returns the app's HTML page rather than JSON, so reserve it for an endpoint you know the spec omits" closest := make([]string, 0, len(suggestions)) for _, s := range suggestions { diff --git a/cmd/api_preflight_test.go b/cmd/api_preflight_test.go index 7fa7dae..cf39e64 100644 --- a/cmd/api_preflight_test.go +++ b/cmd/api_preflight_test.go @@ -10,12 +10,16 @@ import ( // preflightSpec is a Journeys OpenAPI fixture with enough of the campaigns // resource to exercise the pre-send path check: a collection with two verbs and -// a single-campaign read. +// a single-campaign read, plus an alphabetically earlier second resource the +// suggestion ranking would otherwise reach for. func preflightSpec() string { return `{ "openapi": "3.1.0", "info": {"title": "Test", "version": "1.0.0"}, "paths": { + "/v1/environments/{environment_id}/all_metrics": { + "get": {"summary": "List metrics"} + }, "/v1/environments/{environment_id}/campaigns": { "get": {"summary": "List campaigns"}, "post": {"summary": "Create campaign"} @@ -352,3 +356,36 @@ func TestAPIPreflight_FailsOpenWhenSpecUnavailable(t *testing.T) { t.Errorf("expected the request to be sent, got %v", got) } } + +// The nearest route under the same scope is some other resource entirely. +func TestAPIPreflight_DoesNotNameAnUnrelatedResource(t *testing.T) { + server := setupPreflightTest(t) + + _, _, err := executeCommand("api", "/v1/environments/456/topics", "--api-url", server.URL) + if err == nil { + t.Fatal("expected an unknown resource to be rejected before sending") + } + if strings.Contains(err.Error(), "cio schema all_metrics") { + t.Errorf("error must not point at a resource the path never named, got: %s", err.Error()) + } + if !strings.Contains(err.Error(), "run 'cio schema' to list resources") { + t.Errorf("error should fall back to listing resources, got: %s", err.Error()) + } +} + +// Without this, a rejected guess is just resent with the check off. +func TestAPIPreflight_CautionsAgainstForcingAGuessedPath(t *testing.T) { + server := setupPreflightTest(t) + + _, _, err := executeCommand("api", "/v1/environments/456/campaigns/48/actions", + "--api-url", server.URL) + if err == nil { + t.Fatal("expected an invented sub-path to be rejected before sending") + } + if !strings.Contains(err.Error(), "cannot make a missing endpoint exist") { + t.Errorf("error should say --no-preflight does not create the endpoint, got: %s", err.Error()) + } + if !strings.Contains(err.Error(), "HTML page rather than JSON") { + t.Errorf("error should name what forcing it returns, got: %s", err.Error()) + } +} diff --git a/cmd/prime_context.md b/cmd/prime_context.md index eff8666..bf0336a 100644 --- a/cmd/prime_context.md +++ b/cmd/prime_context.md @@ -202,8 +202,11 @@ real endpoints plus the `cio schema` command that lists them — so treat it as rather than trying another spelling. A path that exists only for another method reports that method and the `-X` flag to use. -A few real endpoints are absent from the spec. If you know the path exists, -resend it with `--no-preflight`. +`--no-preflight` skips the check; it does not make a missing endpoint exist. An +unknown path is answered by the web app with its HTML page and a 200, so forcing +a guessed path returns no data. Reserve the flag for the few real endpoints the +spec omits, where you already know the path; otherwise run `cio schema +` and use a documented endpoint. ## Retry Behavior diff --git a/internal/client/pagination.go b/internal/client/pagination.go index e688a34..30d7476 100644 --- a/internal/client/pagination.go +++ b/internal/client/pagination.go @@ -1,6 +1,7 @@ package client import ( + "bytes" "context" "encoding/json" "fmt" @@ -26,7 +27,10 @@ type paginationMeta struct { Total *int HasMore *bool EmptyData bool - DataKey string + // DataLen is the number of records on this page: the length of the + // collection array, taken as the largest top-level array when the key is + // not one of the well-known names. + DataLen int } const maxAutoPages = 10000 @@ -44,6 +48,11 @@ func (c *Client) PageAll(cfg PageAllConfig) error { } page := cfg.StartPage + // The page size for the total-based stop is what the server actually + // returned on the first page, not --limit: servers clamp oversized limits, + // and trusting the flag would end the walk early. A later, final page may + // be shorter, so only the first page is read. + pageSize := 0 for { params := copyParams(cfg.Params) @@ -62,13 +71,16 @@ func (c *Client) PageAll(cfg PageAllConfig) error { } meta := extractPaginationMeta(result, cfg.Limit) + if pageSize == 0 { + pageSize = meta.DataLen + } if meta.TotalPages != nil && page >= *meta.TotalPages { return nil } - if meta.Total != nil && cfg.Limit > 0 { - totalPages := int(math.Ceil(float64(*meta.Total) / float64(cfg.Limit))) + if meta.Total != nil && pageSize > 0 { + totalPages := int(math.Ceil(float64(*meta.Total) / float64(pageSize))) if totalPages == 0 { totalPages = 1 } @@ -101,17 +113,16 @@ func extractPaginationMeta(data json.RawMessage, limit int) paginationMeta { return meta } - if raw, ok := obj["total_pages"]; ok { - var v int - if json.Unmarshal(raw, &v) == nil { - meta.TotalPages = &v + // Totals live either at the top level or, on Journeys list endpoints, + // under meta.pagination. + meta.TotalPages = intField(obj, "total_pages") + meta.Total = intField(obj, "total") + if pagination := nestedObject(obj, "meta", "pagination"); pagination != nil { + if meta.TotalPages == nil { + meta.TotalPages = intField(pagination, "total_pages") } - } - - if raw, ok := obj["total"]; ok { - var v int - if json.Unmarshal(raw, &v) == nil { - meta.Total = &v + if meta.Total == nil { + meta.Total = intField(pagination, "total") } } @@ -123,21 +134,87 @@ func extractPaginationMeta(data json.RawMessage, limit int) paginationMeta { } for _, key := range []string{"data", "items", "results", "records", "entries", "campaigns"} { - if raw, ok := obj[key]; ok { - var arr []json.RawMessage - if json.Unmarshal(raw, &arr) == nil { - meta.DataKey = key - if len(arr) == 0 { - meta.EmptyData = true - } - break - } + raw, ok := obj[key] + if !ok { + continue + } + // A known collection key that is null is an empty page: Go backends + // emit null for a nil slice. + if isNull(raw) { + meta.EmptyData = true + return meta } + if arr, ok := arrayField(obj, key); ok { + meta.DataLen = len(arr) + meta.EmptyData = len(arr) == 0 + return meta + } + } + + // Endpoints name their collection after the resource (imports, segments, + // ...). Without a known key, take the largest top-level array as the + // collection, so a response like {"imports": [], "meta": {...}} still ends + // the walk instead of running to maxAutoPages. + arrays := 0 + for key := range obj { + arr, ok := arrayField(obj, key) + if !ok { + continue + } + arrays++ + meta.DataLen = max(meta.DataLen, len(arr)) } + meta.EmptyData = arrays > 0 && meta.DataLen == 0 return meta } +func intField(obj map[string]json.RawMessage, key string) *int { + raw, ok := obj[key] + if !ok { + return nil + } + var v int + if json.Unmarshal(raw, &v) != nil { + return nil + } + return &v +} + +func isNull(raw json.RawMessage) bool { + return bytes.Equal(bytes.TrimSpace(raw), []byte("null")) +} + +// arrayField returns the field only when it is syntactically an array. +// json.Unmarshal accepts null into a slice, so an unrelated null field +// ("next": null) would otherwise read as an empty collection. +func arrayField(obj map[string]json.RawMessage, key string) ([]json.RawMessage, bool) { + raw, ok := obj[key] + if !ok || !bytes.HasPrefix(bytes.TrimSpace(raw), []byte("[")) { + return nil, false + } + var arr []json.RawMessage + if json.Unmarshal(raw, &arr) != nil { + return nil, false + } + return arr, true +} + +func nestedObject(obj map[string]json.RawMessage, keys ...string) map[string]json.RawMessage { + for _, key := range keys { + raw, ok := obj[key] + if !ok { + return nil + } + var next map[string]json.RawMessage + if json.Unmarshal(raw, &next) != nil { + return nil + } + obj = next + } + return obj +} + func copyParams(params map[string]string) map[string]string { out := make(map[string]string, len(params)) for k, v := range params { diff --git a/internal/client/pagination_test.go b/internal/client/pagination_test.go new file mode 100644 index 0000000..8f7d382 --- /dev/null +++ b/internal/client/pagination_test.go @@ -0,0 +1,175 @@ +package client + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" +) + +// importsServer mimics a Journeys list endpoint: the collection is keyed by +// resource name and the total lives under meta.pagination. +func importsServer(t *testing.T, total int, defaultLimit int) (*httptest.Server, *[]string) { + t.Helper() + var requests []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests = append(requests, r.URL.RawQuery) + page, _ := strconv.Atoi(r.URL.Query().Get("page")) + limit, _ := strconv.Atoi(r.URL.Query().Get("limit")) + if page < 1 { + page = 1 + } + if limit < 1 { + limit = defaultLimit + } + start := (page - 1) * limit + items := make([]map[string]int, 0, limit) + for id := start + 1; id <= total && id <= start+limit; id++ { + items = append(items, map[string]int{"id": id}) + } + resp := map[string]any{ + "imports": items, + "meta": map[string]any{"pagination": map[string]int{ + "page": page, "size": len(items), "total": total, + }}, + } + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(resp); err != nil { + t.Errorf("encode: %v", err) + } + })) + t.Cleanup(server.Close) + return server, &requests +} + +func pageAllLines(t *testing.T, server *httptest.Server, limit int) []string { + t.Helper() + c := New(Config{ + BaseURL: server.URL, + AccessToken: "test-jwt", + RetryConfig: &RetryConfig{MaxRetries: 0, SleepFn: ContextSleep}, + }) + var buf bytes.Buffer + err := c.PageAll(PageAllConfig{ + Ctx: context.Background(), + Method: "GET", + Path: "/v1/environments/1/imports", + Limit: limit, + Writer: &buf, + }) + if err != nil { + t.Fatalf("PageAll: %v", err) + } + var lines []string + for _, line := range strings.Split(buf.String(), "\n") { + if strings.TrimSpace(line) != "" { + lines = append(lines, line) + } + } + return lines +} + +func TestPageAll_StopsOnMetaPaginationTotal(t *testing.T) { + server, requests := importsServer(t, 5, 50) + lines := pageAllLines(t, server, 2) + + if len(lines) != 3 { + t.Fatalf("expected 3 pages for total=5 limit=2, got %d: %v", len(lines), *requests) + } + if len(*requests) != 3 { + t.Errorf("expected exactly 3 requests, got %d: %v", len(*requests), *requests) + } +} + +func TestPageAll_DerivesPageSizeWithoutLimit(t *testing.T) { + server, requests := importsServer(t, 5, 2) + lines := pageAllLines(t, server, 0) + + if len(lines) != 3 { + t.Fatalf("expected 3 pages for total=5 server-default-limit=2, got %d: %v", len(lines), *requests) + } +} + +// The server clamps limit to 2; total=5 must still yield 3 pages even though +// --limit 100 would compute a single page. +func TestPageAll_UsesReturnedPageSizeWhenServerClampsLimit(t *testing.T) { + var requests int + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + page, _ := strconv.Atoi(r.URL.Query().Get("page")) + const total, clamp = 5, 2 + items := make([]int, 0, clamp) + for id := (page-1)*clamp + 1; id <= total && id <= page*clamp; id++ { + items = append(items, id) + } + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(map[string]any{ + "imports": items, + "meta": map[string]any{"pagination": map[string]int{"total": total}}, + }); err != nil { + t.Errorf("encode: %v", err) + } + })) + t.Cleanup(server.Close) + + lines := pageAllLines(t, server, 100) + if len(lines) != 3 || requests != 3 { + t.Fatalf("expected 3 pages and 3 requests, got %d pages, %d requests", len(lines), requests) + } +} + +func TestPageAll_StopsOnEmptyResourceKeyedArray(t *testing.T) { + var requests int + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"imports": [], "meta": {"other": true}}`) + })) + t.Cleanup(server.Close) + + lines := pageAllLines(t, server, 0) + if len(lines) != 1 || requests != 1 { + t.Fatalf("expected a single request and page, got %d requests, %d lines", requests, len(lines)) + } +} + +func TestExtractPaginationMeta(t *testing.T) { + tests := []struct { + name string + body string + total int + emptyData bool + dataLen int + }{ + {"top-level total", `{"total": 7, "data": [1, 2]}`, 7, false, 2}, + {"meta.pagination.total", `{"imports": [1], "meta": {"pagination": {"total": 9}}}`, 9, false, 1}, + {"top-level wins over meta", `{"total": 3, "items": [], "meta": {"pagination": {"total": 9}}}`, 3, true, 0}, + {"empty resource array", `{"segments": [], "meta": {}}`, -1, true, 0}, + {"largest array is the collection", `{"imports": [1, 2, 3], "warnings": ["w"]}`, -1, false, 3}, + {"null field is not a collection", `{"next": null, "id": 1}`, -1, false, 0}, + {"null known collection is empty", `{"data": null, "total": 0}`, 0, true, 0}, + {"no arrays is not empty", `{"id": 1}`, -1, false, 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + meta := extractPaginationMeta(json.RawMessage(tt.body), 0) + switch { + case tt.total < 0 && meta.Total != nil: + t.Errorf("expected no total, got %d", *meta.Total) + case tt.total >= 0 && (meta.Total == nil || *meta.Total != tt.total): + t.Errorf("expected total %d, got %v", tt.total, meta.Total) + } + if meta.EmptyData != tt.emptyData { + t.Errorf("EmptyData = %v, want %v", meta.EmptyData, tt.emptyData) + } + if meta.DataLen != tt.dataLen { + t.Errorf("DataLen = %d, want %d", meta.DataLen, tt.dataLen) + } + }) + } +} diff --git a/internal/routes/loader.go b/internal/routes/loader.go index d5d48c8..205716c 100644 --- a/internal/routes/loader.go +++ b/internal/routes/loader.go @@ -146,6 +146,14 @@ func LoadRegistryFromData(data []byte) (*Registry, error) { return reg, nil } +// ResourceFor returns the resource name a path belongs to, as +// `cio schema ` takes it, or "" for a path outside any scope. +// Concrete ids are accepted in place of templates. +func ResourceFor(path string) string { + resource, _ := deriveFromWalkedPath(path, "") + return resource +} + // deriveFromWalkedPath extracts the CLI resource name and method name from an // httprouter-style path. // diff --git a/internal/routes/loader_test.go b/internal/routes/loader_test.go index ac1b05e..a43412b 100644 --- a/internal/routes/loader_test.go +++ b/internal/routes/loader_test.go @@ -308,3 +308,19 @@ func TestStripScopePrefix(t *testing.T) { }) } } + +func TestResourceForReadsConcreteAndTemplatedPaths(t *testing.T) { + cases := map[string]string{ + "/v1/environments/456/topics": "topics", + "/v1/environments/456/campaigns/97/subjects": "campaigns", + "/v1/environments/{environment_id}/campaigns": "campaigns", + "/cdp/api/workspaces/12/sources": "sources", + "/v1/login": "", + "/health": "", + } + for path, want := range cases { + if got := ResourceFor(path); got != want { + t.Errorf("ResourceFor(%q) = %q, want %q", path, got, want) + } + } +}