feat: replace static sleep with polling for datapoint verification after buzz add

beeminder.go

@@ -329,27 +329,35 @@ func GetLastDatapointValue(config *Config, goalSlug string) (float64, error) {
 	return result.LastDatapoint.Value, nil
 }
 
-// CreateDatapoint submits a new datapoint to a Beeminder goal
-func CreateDatapoint(config *Config, goalSlug, timestamp, value, comment string) error {
+// CreateDatapoint submits a new datapoint to a Beeminder goal and returns the created datapoint
+func CreateDatapoint(config *Config, goalSlug, timestamp, value, comment string) (*Datapoint, error) {
 	baseURL := getBaseURL(config)
-	url := fmt.Sprintf("%s/api/v1/users/%s/goals/%s/datapoints.json",
-		baseURL, config.Username, goalSlug)
+	apiURL := fmt.Sprintf("%s/api/v1/users/%s/goals/%s/datapoints.json",
+		baseURL, config.Username, url.PathEscape(goalSlug))
 
-	data := fmt.Sprintf("auth_token=%s&timestamp=%s&value=%s&comment=%s",
-		config.AuthToken, timestamp, value, comment)
+	data := url.Values{}
+	data.Set("auth_token", config.AuthToken)
+	data.Set("timestamp", timestamp)
+	data.Set("value", value)
+	data.Set("comment", comment)
 
-	resp, err := http.Post(url, "application/x-www-form-urlencoded",
-		strings.NewReader(data))
+	resp, err := http.Post(apiURL, "application/x-www-form-urlencoded",
+		strings.NewReader(data.Encode()))
 	if err != nil {
-		return fmt.Errorf("failed to create datapoint: %w", err)
+		return nil, fmt.Errorf("failed to create datapoint: %w", err)
 	}
 	defer resp.Body.Close()
 
 	if resp.StatusCode != http.StatusOK {
-		return fmt.Errorf("API returned status %d", resp.StatusCode)
+		return nil, fmt.Errorf("API returned status %d", resp.StatusCode)
+	}
+
+	var datapoint Datapoint
+	if err := json.NewDecoder(resp.Body).Decode(&datapoint); err != nil {
+		return nil, fmt.Errorf("failed to decode datapoint response: %w", err)
 	}
 
-	return nil
+	return &datapoint, nil
 }
 
 // CreateCharge creates a new charge for the authenticated user and returns it
@@ -419,10 +427,10 @@ func FetchGoal(config *Config, goalSlug string) (*Goal, error) {
 // FetchGoalWithDatapoints fetches goal details including recent datapoints
 func FetchGoalWithDatapoints(config *Config, goalSlug string) (*Goal, error) {
 	baseURL := getBaseURL(config)
-	url := fmt.Sprintf("%s/api/v1/users/%s/goals/%s.json?auth_token=%s&datapoints=true",
-		baseURL, config.Username, goalSlug, config.AuthToken)
+	apiURL := fmt.Sprintf("%s/api/v1/users/%s/goals/%s.json?auth_token=%s&datapoints=true",
+		baseURL, config.Username, url.PathEscape(goalSlug), config.AuthToken)
 
-	resp, err := http.Get(url)
+	resp, err := http.Get(apiURL)
 	if err != nil {
 		return nil, fmt.Errorf("failed to fetch goal details: %w", err)
 	}
@@ -533,3 +541,60 @@ func RefreshGoal(config *Config, goalSlug string) (bool, error) {
 
 	return result, nil
 }
+
+// WaitForDatapoint polls the goal's recent datapoints until the specified datapoint ID appears
+// or the timeout is reached. Returns true if the datapoint was found, false if timeout occurred.
+// The error return value is used to indicate permanent failures (e.g., authentication errors,
+// non-existent goals) where continuing to poll would be futile.
+// Note: A zero or negative timeout will result in at least one polling attempt before checking the deadline.
+func WaitForDatapoint(config *Config, goalSlug, datapointID string, timeout, pollInterval time.Duration) (bool, error) {
+	deadline := time.Now().Add(timeout)
+
+	for {
+		// Fetch the goal with datapoints
+		goal, err := FetchGoalWithDatapoints(config, goalSlug)
+		if err != nil {
+			// Check if this is a permanent error that we shouldn't retry
+			if isPermanentError(err) {
+				return false, err
+			}
+			// For transient errors, check if we should continue polling
+			if !time.Now().Before(deadline) {
+				// Timeout reached
+				return false, nil
+			}
+			// Wait before retry on transient errors
+			time.Sleep(pollInterval)
+			continue
+		}
+
+		// Check if the datapoint ID exists in the recent datapoints
+		for _, dp := range goal.Datapoints {
+			if dp.ID == datapointID {
+				return true, nil
+			}
+		}
+
+		// Check if we've exceeded the deadline
+		if !time.Now().Before(deadline) {
+			// Timeout reached without finding the datapoint
+			return false, nil
+		}
+
+		// Wait before next poll
+		time.Sleep(pollInterval)
+	}
+}
+
+// isPermanentError checks if an error indicates a permanent failure that shouldn't be retried
+func isPermanentError(err error) bool {
+	if err == nil {
+		return false
+	}
+	errMsg := err.Error()
+	// Check for HTTP status codes that indicate permanent failures
+	// 401 Unauthorized, 403 Forbidden, 404 Not Found
+	return strings.Contains(errMsg, "status 401") ||
+		strings.Contains(errMsg, "status 403") ||
+		strings.Contains(errMsg, "status 404")
+}

beeminder_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
@@ -1368,3 +1369,257 @@ func TestGoalTypeField(t *testing.T) {
 		})
 	}
 }
+
+// TestWaitForDatapoint tests the WaitForDatapoint polling function
+func TestWaitForDatapoint(t *testing.T) {
+	t.Run("finds datapoint immediately", func(t *testing.T) {
+		// Create a mock server that returns a goal with the target datapoint
+		mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			// Check for datapoints query parameter
+			if !strings.Contains(r.URL.RawQuery, "datapoints=true") {
+				t.Error("Expected datapoints=true query parameter")
+			}
+
+			goal := Goal{
+				Slug: "testgoal",
+				Datapoints: []Datapoint{
+					{ID: "target-id", Value: 1.0, Comment: "test"},
+					{ID: "other-id", Value: 2.0, Comment: "other"},
+				},
+			}
+			w.Header().Set("Content-Type", "application/json")
+			json.NewEncoder(w).Encode(goal)
+		}))
+		defer mockServer.Close()
+
+		config := &Config{
+			Username:  "testuser",
+			AuthToken: "testtoken",
+			BaseURL:   mockServer.URL,
+		}
+
+		// Should find the datapoint immediately
+		found, err := WaitForDatapoint(config, "testgoal", "target-id", 1*time.Second, 100*time.Millisecond)
+		if err != nil {
+			t.Errorf("Unexpected error: %v", err)
+		}
+		if !found {
+			t.Error("Expected to find datapoint immediately")
+		}
+	})
+
+	t.Run("times out when datapoint not found", func(t *testing.T) {
+		// Create a mock server that never returns the target datapoint
+		mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			goal := Goal{
+				Slug: "testgoal",
+				Datapoints: []Datapoint{
+					{ID: "other-id", Value: 2.0, Comment: "other"},
+				},
+			}
+			w.Header().Set("Content-Type", "application/json")
+			json.NewEncoder(w).Encode(goal)
+		}))
+		defer mockServer.Close()
+
+		config := &Config{
+			Username:  "testuser",
+			AuthToken: "testtoken",
+			BaseURL:   mockServer.URL,
+		}
+
+		// Should timeout without finding the datapoint
+		start := time.Now()
+		found, err := WaitForDatapoint(config, "testgoal", "missing-id", 300*time.Millisecond, 100*time.Millisecond)
+		elapsed := time.Since(start)
+
+		if err != nil {
+			t.Errorf("Unexpected error: %v", err)
+		}
+		if found {
+			t.Error("Expected not to find datapoint (timeout)")
+		}
+		// Verify it actually waited for approximately the timeout duration
+		if elapsed < 250*time.Millisecond {
+			t.Errorf("Expected to wait at least 250ms, but only waited %v", elapsed)
+		}
+	})
+
+	t.Run("continues polling on fetch errors", func(t *testing.T) {
+		callCount := 0
+		// Create a mock server that returns an error first, then success
+		mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			callCount++
+			if callCount == 1 {
+				// First call returns error
+				w.WriteHeader(http.StatusInternalServerError)
+				return
+			}
+			// Second call returns success with the datapoint
+			goal := Goal{
+				Slug: "testgoal",
+				Datapoints: []Datapoint{
+					{ID: "target-id", Value: 1.0, Comment: "test"},
+				},
+			}
+			w.Header().Set("Content-Type", "application/json")
+			json.NewEncoder(w).Encode(goal)
+		}))
+		defer mockServer.Close()
+
+		config := &Config{
+			Username:  "testuser",
+			AuthToken: "testtoken",
+			BaseURL:   mockServer.URL,
+		}
+
+		// Should retry after error and eventually find the datapoint
+		found, err := WaitForDatapoint(config, "testgoal", "target-id", 1*time.Second, 100*time.Millisecond)
+		if err != nil {
+			t.Errorf("Unexpected error: %v", err)
+		}
+		if !found {
+			t.Error("Expected to find datapoint after retrying")
+		}
+		if callCount < 2 {
+			t.Errorf("Expected at least 2 API calls, got %d", callCount)
+		}
+	})
+
+	t.Run("returns error on permanent failure", func(t *testing.T) {
+		// Create a mock server that always returns 404
+		mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusNotFound)
+		}))
+		defer mockServer.Close()
+
+		config := &Config{
+			Username:  "testuser",
+			AuthToken: "testtoken",
+			BaseURL:   mockServer.URL,
+		}
+
+		// Should return error immediately on permanent failure
+		start := time.Now()
+		found, err := WaitForDatapoint(config, "testgoal", "target-id", 5*time.Second, 100*time.Millisecond)
+		elapsed := time.Since(start)
+
+		if err == nil {
+			t.Error("Expected error on permanent failure, got nil")
+		}
+		if found {
+			t.Error("Expected not to find datapoint on error")
+		}
+		// Should fail fast, not wait for full timeout
+		if elapsed > 1*time.Second {
+			t.Errorf("Expected to fail fast (< 1s), but took %v", elapsed)
+		}
+	})
+
+	t.Run("handles zero timeout with at least one attempt", func(t *testing.T) {
+		callCount := 0
+		// Create a mock server that tracks calls
+		mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			callCount++
+			goal := Goal{
+				Slug: "testgoal",
+				Datapoints: []Datapoint{
+					{ID: "target-id", Value: 1.0, Comment: "test"},
+				},
+			}
+			w.Header().Set("Content-Type", "application/json")
+			json.NewEncoder(w).Encode(goal)
+		}))
+		defer mockServer.Close()
+
+		config := &Config{
+			Username:  "testuser",
+			AuthToken: "testtoken",
+			BaseURL:   mockServer.URL,
+		}
+
+		// Should make at least one attempt even with zero timeout
+		found, err := WaitForDatapoint(config, "testgoal", "target-id", 0, 100*time.Millisecond)
+		if err != nil {
+			t.Errorf("Unexpected error: %v", err)
+		}
+		if !found {
+			t.Error("Expected to find datapoint on first attempt")
+		}
+		if callCount < 1 {
+			t.Errorf("Expected at least 1 API call with zero timeout, got %d", callCount)
+		}
+	})
+}
+
+// TestIsPermanentError tests the isPermanentError function
+func TestIsPermanentError(t *testing.T) {
+	tests := []struct {
+		name     string
+		err      error
+		expected bool
+	}{
+		{"nil error", nil, false},
+		{"401 unauthorized", fmt.Errorf("API returned status 401"), true},
+		{"403 forbidden", fmt.Errorf("API returned status 403"), true},
+		{"404 not found", fmt.Errorf("API returned status 404"), true},
+		{"500 server error", fmt.Errorf("API returned status 500"), false},
+		{"network error", fmt.Errorf("connection refused"), false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := isPermanentError(tt.err)
+			if result != tt.expected {
+				t.Errorf("isPermanentError(%v) = %v, want %v", tt.err, result, tt.expected)
+			}
+		})
+	}
+}
+
+// TestCreateDatapointReturnsDatapoint tests that CreateDatapoint returns the created datapoint
+func TestCreateDatapointReturnsDatapoint(t *testing.T) {
+	// Create a mock server that returns a datapoint
+	mockServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method != http.MethodPost {
+			t.Errorf("Expected POST request, got %s", r.Method)
+		}
+
+		// Return a mock datapoint response
+		datapoint := Datapoint{
+			ID:        "12345",
+			Timestamp: 1234567890,
+			Value:     42.0,
+			Comment:   "test datapoint",
+		}
+		w.Header().Set("Content-Type", "application/json")
+		json.NewEncoder(w).Encode(datapoint)
+	}))
+	defer mockServer.Close()
+
+	config := &Config{
+		Username:  "testuser",
+		AuthToken: "testtoken",
+		BaseURL:   mockServer.URL,
+	}
+
+	// Call CreateDatapoint
+	datapoint, err := CreateDatapoint(config, "testgoal", "1234567890", "42", "test datapoint")
+	if err != nil {
+		t.Fatalf("CreateDatapoint failed: %v", err)
+	}
+
+	// Verify the returned datapoint
+	if datapoint == nil {
+		t.Fatal("Expected datapoint to be returned, got nil")
+	}
+	if datapoint.ID != "12345" {
+		t.Errorf("Expected ID '12345', got '%s'", datapoint.ID)
+	}
+	if datapoint.Value != 42.0 {
+		t.Errorf("Expected Value 42.0, got %v", datapoint.Value)
+	}
+	if datapoint.Comment != "test datapoint" {
+		t.Errorf("Expected Comment 'test datapoint', got '%s'", datapoint.Comment)
+	}
+}