Merge pull request #1681 from entireio/merge-mirror-list-show-available

refactor: one repo mirror list for mirrored and mirrorable repos, per-cluster detail via mirror get

Author: pjbgf

cmd/entire/cli/corecmd.go

@@ -1,11 +1,13 @@
 package cli
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
+	"strconv"
 	"strings"
 
 	"charm.land/huh/v2"
@@ -199,32 +201,142 @@ func renderCoreList[T any](cmd *cobra.Command, empty string, headers []string, r
 	}
 }
 
+// coreListFetchBudget bounds how many entries a bounded list command fetches
+// by default. The control plane pages but cannot filter or sort these lists,
+// so without a bound every call would walk the entire collection — thousands
+// of requests on a large org. Commands that stop at the budget must disclose
+// the partial window on stderr and offer --all.
+const coreListFetchBudget = 1000
+
 // fetchAllPages drives a keyset-paginated list endpoint to completion: it
 // calls fetch with an empty cursor, then re-calls it with each returned
 // nextPageToken until the cursor comes back empty, concatenating every page.
 // The control plane caps the page size (and may cap it further than a caller
 // requests), so a single call only returns one page — list commands must loop
-// or they silently truncate. The next==cursor guard turns a misbehaving server
-// that fails to advance the cursor into an error instead of an infinite loop.
+// or they silently truncate.
 func fetchAllPages[T any](ctx context.Context, fetch func(ctx context.Context, cursor string) (items []T, next string, err error)) ([]T, error) {
+	items, _, err := fetchPagesBounded(ctx, 0, fetch)
+	return items, err
+}
+
+// fetchPagesBounded is fetchAllPages with a fetch budget: the cursor walk
+// stops once at least budget entries have been fetched (a page is never split,
+// so the result can overshoot by up to one page). partial reports that the
+// walk stopped with a cursor remaining — entries exist beyond the returned
+// slice and the caller must disclose that. budget <= 0 means unbounded. The
+// next==cursor guard turns a misbehaving server that fails to advance the
+// cursor into an error instead of an infinite loop.
+func fetchPagesBounded[T any](ctx context.Context, budget int, fetch func(ctx context.Context, cursor string) (items []T, next string, err error)) (items []T, partial bool, err error) {
 	var all []T
 	cursor := ""
 	for {
-		items, next, err := fetch(ctx, cursor)
+		page, next, err := fetch(ctx, cursor)
 		if err != nil {
-			return nil, err
+			return nil, false, err
 		}
-		all = append(all, items...)
+		all = append(all, page...)
 		if next == "" {
-			return all, nil
+			return all, false, nil
+		}
+		if budget > 0 && len(all) >= budget {
+			return all, true, nil
 		}
 		if next == cursor {
-			return nil, fmt.Errorf("pagination did not advance (cursor %q repeated)", next)
+			return nil, false, fmt.Errorf("pagination did not advance (cursor %q repeated)", next)
 		}
 		cursor = next
 	}
 }
 
+// listPage is the --json envelope for single-page (cursor passthrough) list
+// output: rows plus the cursor to resume from, omitted on the last page. Page
+// mode cannot emit the bare array the walk modes use — the caller needs the
+// cursor to continue, and stdout is the only machine-readable channel.
+type listPage[T any] struct {
+	Items         []T    `json:"items"`
+	NextPageToken string `json:"nextPageToken,omitempty"`
+}
+
+// renderCoreListPage renders one fetched page of a list command: --json emits
+// the listPage envelope; the table view prints the usual table, preceded by a
+// stderr resume hint carrying the cursor when more entries exist.
+func renderCoreListPage[T any](cmd *cobra.Command, empty string, headers []string, row func(T) []string, items []T, next string) error {
+	if jsonRequested(cmd) {
+		if items == nil {
+			items = []T{} // a nil slice encodes as null; scripts expect []
+		}
+		return printJSON(cmd.OutOrStdout(), listPage[T]{Items: items, NextPageToken: next})
+	}
+	if next != "" {
+		fmt.Fprintf(cmd.ErrOrStderr(), "More entries available: resume with --page-token %s\n", next)
+	}
+	if len(items) == 0 {
+		fmt.Fprintln(cmd.OutOrStdout(), empty)
+		return nil
+	}
+	return printTable(cmd.OutOrStdout(), headers, items, row)
+}
+
+// pageModeFlags wires the single-page cursor-passthrough flags onto a list
+// command and excludes them from the walk flags (--all, --limit): one call =
+// one request, so a walk bound makes no sense alongside them. Callers validate
+// pageSize positivity in PreRunE via validatePageSize.
+func pageModeFlags(cmd *cobra.Command, pageSize *int, pageToken *string) {
+	cmd.Flags().IntVar(pageSize, "page-size", 0, "Fetch a single page of at most N entries (1-"+strconv.Itoa(coreListPageSizeMax)+"; the server may cap N further) and print the resume cursor")
+	cmd.Flags().StringVar(pageToken, "page-token", "", "Fetch the single page at this cursor (from a previous run's nextPageToken)")
+	cmd.MarkFlagsMutuallyExclusive("page-size", "all")
+	cmd.MarkFlagsMutuallyExclusive("page-token", "all")
+	cmd.MarkFlagsMutuallyExclusive("page-size", "limit")
+	cmd.MarkFlagsMutuallyExclusive("page-token", "limit")
+}
+
+// pageModeRequested reports whether the caller opted into single-page mode:
+// either page flag was explicitly set. Checked by Changed, not value — a
+// script's resume loop naturally passes --page-token "" for its first page,
+// and a value check would silently reroute that call to the multi-page walk,
+// flipping the --json shape from the {items, nextPageToken} envelope to a
+// bare array (and the request count from one to many).
+func pageModeRequested(cmd *cobra.Command) bool {
+	return cmd.Flags().Changed("page-size") || cmd.Flags().Changed("page-token")
+}
+
+// coreListPageSizeMax mirrors the OpenAPI `maximum: 500` on the list
+// endpoints' pageSize param (see internal/coreapi/spec). The generated client
+// does not validate params, so without this local bound an oversized
+// --page-size goes on the wire and comes back as a server 4xx naming the wire
+// param instead of the flag.
+const coreListPageSizeMax = 500
+
+// validatePageSize rejects an explicitly set out-of-range --page-size; an
+// unset flag passes.
+func validatePageSize(cmd *cobra.Command, pageSize int) error {
+	if cmd.Flags().Changed("page-size") && (pageSize <= 0 || pageSize > coreListPageSizeMax) {
+		return fmt.Errorf("--page-size must be between 1 and %d, got %d", coreListPageSizeMax, pageSize)
+	}
+	return nil
+}
+
+// flushThroughPager runs run with the command's stdout captured, then flushes
+// the captured output — through a pager when stdout is a real terminal and the
+// content is taller than the screen (see outputWithPager), directly otherwise.
+// --json output never pages: a machine consumer driving a PTY would hang
+// waiting on the pager's keyboard, and JSON is not for reading. Output is
+// flushed even when run errors, so partial renders are not swallowed.
+func flushThroughPager(cmd *cobra.Command, noPager bool, run func() error) error {
+	finalOut := cmd.OutOrStdout()
+	var buf bytes.Buffer
+	cmd.SetOut(&buf)
+	err := run()
+	cmd.SetOut(finalOut)
+	content := buf.String()
+	if noPager || jsonRequested(cmd) {
+		fmt.Fprint(finalOut, content)
+	} else {
+		outputWithPager(finalOut, content)
+	}
+	return err
+}
+
 // runCoreObject fetches a single value via fn and renders it as a vertical
 // field/value list (default) or raw JSON (--json), reusing the same column
 // definition as the matching list view.
@@ -321,6 +433,43 @@ func printTable[T any](w io.Writer, headers []string, items []T, row func(T) []s
 	return nil
 }
 
+// preStyleTable pre-colors table headers and a row function against w's color
+// capability, so a command that renders its table into a pager buffer keeps
+// its color. printTable/renderCoreListPage decide color from the writer they
+// render into; under flushThroughPager that writer is an in-memory buffer,
+// which never looks like a TTY, so a straight render there is always plain.
+// Pre-styling against the real output writer here and letting the buffered
+// render pass the ANSI through unchanged (its own color gate is off, so it
+// never re-styles) restores it — the same approach the mirror-list view takes.
+// Identity (no wrapping) when color is off, so pipes, tests, and NO_COLOR see
+// bare text byte for byte.
+func preStyleTable[T any](w io.Writer, headers []string, row func(T) []string) ([]string, func(T) []string) {
+	return styleTableWith(newTableStyles(w), headers, row)
+}
+
+// styleTableWith is the pure core of preStyleTable: it applies st's header and
+// per-column styles to the headers and row cells, matching how printTable
+// colors a direct render. Split out from the writer-facing wrapper so the
+// enabled path is unit-testable without a real terminal. Identity when st is
+// disabled, so plain output stays byte-for-byte unchanged.
+func styleTableWith[T any](st tableStyles, headers []string, row func(T) []string) ([]string, func(T) []string) {
+	if !st.enabled {
+		return headers, row
+	}
+	styledHeaders := make([]string, len(headers))
+	for i, h := range headers {
+		styledHeaders[i] = st.style(st.header, h)
+	}
+	styledRow := func(t T) []string {
+		cells := row(t)
+		for i := range cells {
+			cells[i] = st.style(st.columnStyle(i), cells[i])
+		}
+		return cells
+	}
+	return styledHeaders, styledRow
+}
+
 // printFields writes a single record as aligned "FIELD  value" lines: the
 // label in header gray, the value in the same primary/secondary color the
 // list view would give that column.

cmd/entire/cli/corecmd_test.go

@@ -7,6 +7,10 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+
+	"charm.land/lipgloss/v2"
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/require"
 )
 
 // TestConfirmControlPlaneDeletion covers the non-TTY decision paths of the
@@ -113,6 +117,133 @@ func TestFetchAllPages(t *testing.T) {
 	})
 }
 
+// TestFetchPagesBounded covers the budget branch fetchAllPages delegates to:
+// the walk stops once the budget is reached (never splitting a page, so it can
+// overshoot) and reports that entries remain; budget<=0 walks to the end.
+func TestFetchPagesBounded(t *testing.T) {
+	t.Parallel()
+
+	// Three pages keyed by the cursor the previous page returned.
+	pages := map[string][]string{"": {"a", "b"}, "c1": {"c", "d"}, "c2": {"e"}}
+	nexts := map[string]string{"": "c1", "c1": "c2", "c2": ""}
+
+	t.Run("stops at the budget and reports the partial walk", func(t *testing.T) {
+		t.Parallel()
+		got, partial, err := fetchPagesBounded(context.Background(), 3, func(_ context.Context, cursor string) ([]string, string, error) {
+			return pages[cursor], nexts[cursor], nil
+		})
+		require.NoError(t, err)
+		// Budget 3 is reached after the second page (4 items), which is not
+		// split — so the result overshoots to 4 and the walk stops there.
+		require.Equal(t, []string{"a", "b", "c", "d"}, got)
+		require.True(t, partial, "a cursor still remained, so entries are unseen")
+	})
+
+	t.Run("a zero budget walks to the empty cursor", func(t *testing.T) {
+		t.Parallel()
+		got, partial, err := fetchPagesBounded(context.Background(), 0, func(_ context.Context, cursor string) ([]string, string, error) {
+			return pages[cursor], nexts[cursor], nil
+		})
+		require.NoError(t, err)
+		require.Equal(t, []string{"a", "b", "c", "d", "e"}, got)
+		require.False(t, partial, "the chain ended, nothing left unseen")
+	})
+}
+
+// newPageModeTestCmd wires the walk (--all/--limit) and single-page
+// (--page-size/--page-token) flags the way the real list commands do, so the
+// shared flag helpers can be unit-tested without a command surface.
+func newPageModeTestCmd() *cobra.Command {
+	var pageSize, limit int
+	var pageToken string
+	var all bool
+	cmd := &cobra.Command{Use: "x", RunE: func(*cobra.Command, []string) error { return nil }}
+	cmd.Flags().IntVar(&limit, "limit", 0, "")
+	cmd.Flags().BoolVar(&all, "all", false, "")
+	pageModeFlags(cmd, &pageSize, &pageToken)
+	return cmd
+}
+
+// TestValidatePageSize covers the local bound the list commands enforce in
+// PreRunE: an unset flag passes, and an explicit value outside 1..max fails
+// naming the flag (and the max), turning a would-be server 4xx into a
+// flag-named error.
+func TestValidatePageSize(t *testing.T) {
+	t.Parallel()
+	check := func(args ...string) error {
+		cmd := newPageModeTestCmd()
+		require.NoError(t, cmd.Flags().Parse(args))
+		ps, err := cmd.Flags().GetInt("page-size")
+		require.NoError(t, err)
+		return validatePageSize(cmd, ps)
+	}
+	require.NoError(t, check(), "unset --page-size passes")
+	require.NoError(t, check("--page-size", "1"))
+	require.NoError(t, check("--page-size", "500"))
+
+	err := check("--page-size", "0")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "--page-size")
+
+	err = check("--page-size", "501")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "500")
+}
+
+// TestPageModeRequested pins that page mode is opted into by SETTING either
+// page flag, not by its value: an explicitly empty --page-token (a resume
+// loop's natural first call) still selects page mode, so the output shape does
+// not flip to the walk's bare array on an empty cursor.
+func TestPageModeRequested(t *testing.T) {
+	t.Parallel()
+	mode := func(args ...string) bool {
+		cmd := newPageModeTestCmd()
+		require.NoError(t, cmd.Flags().Parse(args))
+		return pageModeRequested(cmd)
+	}
+	require.False(t, mode(), "no page flag → walk mode")
+	require.True(t, mode("--page-size", "5"))
+	require.True(t, mode("--page-token", "p2"))
+	require.True(t, mode("--page-token", ""), "an explicit empty cursor still selects page mode")
+}
+
+// TestStyleTableWith covers the pre-styling that keeps a paged list command's
+// table colored: the render inside flushThroughPager targets a buffer that
+// never looks like a TTY, so color is decided against the real writer up front
+// and applied here. The enabled path must color the header row and route each
+// data cell through its column style (first column primary, rest secondary);
+// the disabled path must be an exact identity so pipes, tests, and NO_COLOR
+// see bare text byte for byte.
+func TestStyleTableWith(t *testing.T) {
+	t.Parallel()
+
+	headers := []string{"ID", "NAME"}
+	row := func(r []string) []string { return r }
+	item := []string{"a", "b"}
+
+	t.Run("enabled path colors headers and routes cells by column", func(t *testing.T) {
+		t.Parallel()
+		st := tableStyles{
+			enabled: true,
+			header:  lipgloss.NewStyle().Bold(true),
+			primary: lipgloss.NewStyle().Underline(true),
+			cell:    lipgloss.NewStyle().Faint(true),
+		}
+		gotHeaders, gotRow := styleTableWith(st, headers, row)
+		require.Equal(t, []string{st.header.Render("ID"), st.header.Render("NAME")}, gotHeaders)
+		// Column 0 is the primary identifier, the rest secondary — the same
+		// split printTable applies when it colors a direct render.
+		require.Equal(t, []string{st.primary.Render("a"), st.cell.Render("b")}, gotRow(item))
+	})
+
+	t.Run("disabled path is an exact identity", func(t *testing.T) {
+		t.Parallel()
+		gotHeaders, gotRow := styleTableWith(tableStyles{}, headers, row)
+		require.Equal(t, headers, gotHeaders)
+		require.Equal(t, item, gotRow(item))
+	})
+}
+
 // printTable/printFields render plain (no color/escape) when the writer
 // isn't a TTY — which a bytes.Buffer never is — so these assert the plain
 // layout directly.