feat: enhance cloud pagination and retries

This commit is contained in:
avivsinai
2025-10-27 15:24:43 +02:00
parent 9d9e6cca38
commit e49525476b
8 changed files with 12950 additions and 35 deletions
@@ -0,0 +1,9 @@
# bitbucket-cli-full preset
# Generated: 2025-10-27T07:14:50.243Z
# Source: patterns preserved (1)
# Patterns preserved as provided: 1
**/*
!**/node_modules/**
!**/dist/**
!**/build/**
+38 -9
View File
@@ -139,17 +139,46 @@ func (c *Client) ListPipelines(ctx context.Context, workspace, repoSlug string,
pageLen,
)
req, err := c.http.NewRequest(ctx, "GET", path, nil)
if err != nil {
return nil, err
var pipelines []Pipeline
for path != "" {
req, err := c.http.NewRequest(ctx, "GET", path, nil)
if err != nil {
return nil, err
}
var page PipelinePage
if err := c.http.Do(req, &page); err != nil {
return nil, err
}
pipelines = append(pipelines, page.Values...)
if limit > 0 && len(pipelines) >= limit {
pipelines = pipelines[:limit]
break
}
if page.Next == "" {
break
}
nextURL, err := url.Parse(page.Next)
if err != nil {
return nil, err
}
if nextURL.IsAbs() {
if uri := nextURL.RequestURI(); uri != "" {
path = uri
} else {
path = nextURL.String()
}
} else {
path = nextURL.String()
}
}
var page PipelinePage
if err := c.http.Do(req, &page); err != nil {
return nil, err
}
return page.Values, nil
return pipelines, nil
}
// RepositoryListPage encapsulates paginated repository responses.
+100
View File
@@ -0,0 +1,100 @@
package bbcloud
import (
"context"
"encoding/json"
"net/http"
"net/http/httptest"
"sync/atomic"
"testing"
)
func TestListPipelinesPaginates(t *testing.T) {
var hits int32
var serverURL string
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
count := atomic.AddInt32(&hits, 1)
w.Header().Set("Content-Type", "application/json")
switch count {
case 1:
if r.URL.Query().Get("pagelen") == "" {
t.Fatalf("expected pagelen query in first request")
}
payload := PipelinePage{
Values: []Pipeline{{UUID: "1"}, {UUID: "2"}},
Next: serverURL + "/repositories/work/repo/pipelines/?pagelen=20&page=2",
}
_ = json.NewEncoder(w).Encode(payload)
case 2:
payload := PipelinePage{
Values: []Pipeline{{UUID: "3"}},
}
_ = json.NewEncoder(w).Encode(payload)
default:
t.Fatalf("unexpected extra request %d", count)
}
}))
serverURL = server.URL
t.Cleanup(server.Close)
client, err := New(Options{BaseURL: server.URL})
if err != nil {
t.Fatalf("New: %v", err)
}
ctx := context.Background()
pipelines, err := client.ListPipelines(ctx, "work", "repo", 0)
if err != nil {
t.Fatalf("ListPipelines: %v", err)
}
if len(pipelines) != 3 {
t.Fatalf("expected 3 pipelines, got %d", len(pipelines))
}
if hits != 2 {
t.Fatalf("expected 2 requests, got %d", hits)
}
}
func TestListPipelinesRespectsLimit(t *testing.T) {
var hits int32
var serverURL string
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
count := atomic.AddInt32(&hits, 1)
w.Header().Set("Content-Type", "application/json")
if count == 1 {
payload := PipelinePage{
Values: []Pipeline{{UUID: "1"}, {UUID: "2"}},
Next: serverURL + "/repositories/work/repo/pipelines/?pagelen=20&page=2",
}
_ = json.NewEncoder(w).Encode(payload)
return
}
t.Fatalf("unexpected second request when limit satisfied")
}))
serverURL = server.URL
t.Cleanup(server.Close)
client, err := New(Options{BaseURL: server.URL})
if err != nil {
t.Fatalf("New: %v", err)
}
ctx := context.Background()
pipelines, err := client.ListPipelines(ctx, "work", "repo", 1)
if err != nil {
t.Fatalf("ListPipelines: %v", err)
}
if len(pipelines) != 1 {
t.Fatalf("expected 1 pipeline, got %d", len(pipelines))
}
if hits != 1 {
t.Fatalf("expected 1 request, got %d", hits)
}
}
+55 -9
View File
@@ -127,11 +127,33 @@ func New(opts Options) (*Client, error) {
// NewRequest builds an HTTP request relative to the base URL. Body values are
// JSON encoded when non-nil.
func (c *Client) NewRequest(ctx context.Context, method, path string, body any) (*http.Request, error) {
if !strings.HasPrefix(path, "/") {
path = "/" + path
if strings.TrimSpace(path) == "" {
return nil, fmt.Errorf("path is required")
}
u := c.baseURL.ResolveReference(&url.URL{Path: path})
var rel *url.URL
var err error
if strings.HasPrefix(path, "http://") || strings.HasPrefix(path, "https://") {
rel, err = url.Parse(path)
if err != nil {
return nil, fmt.Errorf("parse request URL: %w", err)
}
} else {
if !strings.HasPrefix(path, "/") {
path = "/" + path
}
rel, err = url.Parse(path)
if err != nil {
return nil, fmt.Errorf("parse request path: %w", err)
}
}
if rel.Path == "" {
rel.Path = "/"
}
u := c.baseURL.ResolveReference(rel)
var payload []byte
if body != nil {
@@ -202,7 +224,11 @@ func (c *Client) Do(req *http.Request, v any) error {
return err
}
attempts++
if !c.backoff(attempts, resp) {
continueRetry, waitErr := c.backoff(req.Context(), attempts, resp)
if waitErr != nil {
return waitErr
}
if !continueRetry {
if c.debug {
fmt.Fprintf(os.Stderr, "<-- retry abort after error: %v\n", err)
}
@@ -236,7 +262,11 @@ func (c *Client) Do(req *http.Request, v any) error {
return decodeError(resp)
}
attempts++
if !c.backoff(attempts, resp) {
continueRetry, waitErr := c.backoff(req.Context(), attempts, resp)
if waitErr != nil {
return waitErr
}
if !continueRetry {
if len(bodyBytes) > 0 {
resp.Body = io.NopCloser(bytes.NewReader(bodyBytes))
}
@@ -337,9 +367,9 @@ func (c *Client) shouldRetry(attempts int, status int) bool {
return attempts+1 < c.retry.MaxAttempts
}
func (c *Client) backoff(attempts int, resp *http.Response) bool {
func (c *Client) backoff(ctx context.Context, attempts int, resp *http.Response) (bool, error) {
if attempts >= c.retry.MaxAttempts {
return false
return false, nil
}
delay := c.retry.InitialBackoff
@@ -358,8 +388,24 @@ func (c *Client) backoff(attempts int, resp *http.Response) bool {
}
}
time.Sleep(delay)
return true
if delay <= 0 {
select {
case <-ctx.Done():
return false, ctx.Err()
default:
return true, nil
}
}
timer := time.NewTimer(delay)
defer timer.Stop()
select {
case <-ctx.Done():
return false, ctx.Err()
case <-timer.C:
return true, nil
}
}
func (c *Client) cacheKey(req *http.Request) string {
+86
View File
@@ -3,8 +3,10 @@ package httpx
import (
"context"
"encoding/json"
"errors"
"net/http"
"net/http/httptest"
"sync"
"sync/atomic"
"testing"
"time"
@@ -114,3 +116,87 @@ func TestClientRetriesOnServerError(t *testing.T) {
t.Fatalf("expected 2 attempts, got %d", hits)
}
}
func TestClientNewRequestPreservesQuery(t *testing.T) {
client, err := New(Options{BaseURL: "https://example.com/api"})
if err != nil {
t.Fatalf("New: %v", err)
}
req, err := client.NewRequest(context.Background(), http.MethodGet, "/rest/projects?limit=25&start=0", nil)
if err != nil {
t.Fatalf("NewRequest: %v", err)
}
if got := req.URL.String(); got != "https://example.com/rest/projects?limit=25&start=0" {
t.Fatalf("unexpected URL: %s", got)
}
if req.URL.RawQuery != "limit=25&start=0" {
t.Fatalf("expected raw query preserved, got %q", req.URL.RawQuery)
}
}
func TestClientNewRequestHandlesRelativeWithoutSlash(t *testing.T) {
client, err := New(Options{BaseURL: "https://example.com/api"})
if err != nil {
t.Fatalf("New: %v", err)
}
req, err := client.NewRequest(context.Background(), http.MethodGet, "rest/repos", nil)
if err != nil {
t.Fatalf("NewRequest: %v", err)
}
if got := req.URL.String(); got != "https://example.com/rest/repos" {
t.Fatalf("unexpected URL: %s", got)
}
}
func TestClientBackoffRespectsContextCancellation(t *testing.T) {
var hits int32
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
atomic.AddInt32(&hits, 1)
w.WriteHeader(http.StatusInternalServerError)
}))
t.Cleanup(server.Close)
client, err := New(Options{
BaseURL: server.URL,
Retry: RetryPolicy{
MaxAttempts: 3,
InitialBackoff: 500 * time.Millisecond,
MaxBackoff: time.Second,
},
})
if err != nil {
t.Fatalf("New: %v", err)
}
ctx, cancel := context.WithCancel(context.Background())
req, err := client.NewRequest(ctx, http.MethodGet, "/fail", nil)
if err != nil {
t.Fatalf("NewRequest: %v", err)
}
var once sync.Once
time.AfterFunc(50*time.Millisecond, func() {
once.Do(cancel)
})
start := time.Now()
err = client.Do(req, nil)
elapsed := time.Since(start)
if err == nil {
t.Fatalf("expected error from cancelled context")
}
if !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) {
t.Fatalf("expected context cancellation error, got %v", err)
}
if elapsed >= 400*time.Millisecond {
t.Fatalf("expected cancellation to interrupt backoff, took %v", elapsed)
}
if hits != 1 {
t.Fatalf("expected single request, got %d", hits)
}
}
+18 -17
View File
@@ -73,24 +73,25 @@ func (p *system) Confirm(prompt string, defaultYes bool) (bool, error) {
suffix = "[y/N]"
}
if _, err := fmt.Fprintf(p.ios.Out, "%s %s: ", prompt, suffix); err != nil {
return false, err
}
for {
if _, err := fmt.Fprintf(p.ios.Out, "%s %s: ", prompt, suffix); err != nil {
return false, err
}
line, err := r.ReadString('\n')
if err != nil {
return false, err
}
line, err := r.ReadString('\n')
if err != nil {
return false, err
}
switch strings.ToLower(strings.TrimSpace(line)) {
case "y", "yes":
return true, nil
case "n", "no":
return false, nil
case "":
return defaultYes, nil
default:
fmt.Fprintln(p.ios.ErrOut, "Please respond with 'y' or 'n'.")
return p.Confirm(prompt, defaultYes)
switch strings.ToLower(strings.TrimSpace(line)) {
case "y", "yes":
return true, nil
case "n", "no":
return false, nil
case "":
return defaultYes, nil
default:
fmt.Fprintln(p.ios.ErrOut, "Please respond with 'y' or 'n'.")
}
}
}
+48
View File
@@ -0,0 +1,48 @@
package prompter
import (
"bytes"
"io"
"reflect"
"strings"
"testing"
"unsafe"
"github.com/avivsinai/bitbucket-cli/pkg/iostreams"
)
func TestConfirmRetriesOnInvalidInput(t *testing.T) {
input := "maybe\ny\n"
ios := &iostreams.IOStreams{
In: io.NopCloser(strings.NewReader(input)),
Out: &bytes.Buffer{},
ErrOut: &bytes.Buffer{},
}
forceTTY(ios)
prompt := New(ios)
got, err := prompt.Confirm("Proceed?", false)
if err != nil {
t.Fatalf("Confirm returned error: %v", err)
}
if !got {
t.Fatalf("expected confirmation to be true after invalid input")
}
if !strings.Contains(ios.ErrOut.(*bytes.Buffer).String(), "Please respond") {
t.Fatalf("expected error prompt after invalid input")
}
}
func forceTTY(ios *iostreams.IOStreams) {
setBoolField := func(name string) {
field := reflect.ValueOf(ios).Elem().FieldByName(name)
ptr := unsafe.Pointer(field.UnsafeAddr())
reflect.NewAt(field.Type(), ptr).Elem().SetBool(true)
}
setBoolField("isStdinTTY")
setBoolField("isStdoutTTY")
setBoolField("isStderrTTY")
}
File diff suppressed because it is too large Load Diff