Merge branch 'main' of https://github.com/steveyegge/beads
This commit is contained in:
@@ -59,28 +59,32 @@ func (s *Server) handleExport(req *Request) Response {
|
|||||||
issue.Dependencies = allDeps[issue.ID]
|
issue.Dependencies = allDeps[issue.ID]
|
||||||
}
|
}
|
||||||
|
|
||||||
// Populate labels for all issues
|
// Populate labels for all issues (avoid N+1)
|
||||||
for _, issue := range issues {
|
issueIDs := make([]string, len(issues))
|
||||||
labels, err := store.GetLabels(ctx, issue.ID)
|
for i, issue := range issues {
|
||||||
if err != nil {
|
issueIDs[i] = issue.ID
|
||||||
return Response{
|
}
|
||||||
Success: false,
|
allLabels, err := store.GetLabelsForIssues(ctx, issueIDs)
|
||||||
Error: fmt.Sprintf("failed to get labels for %s: %v", issue.ID, err),
|
if err != nil {
|
||||||
}
|
return Response{
|
||||||
|
Success: false,
|
||||||
|
Error: fmt.Sprintf("failed to get labels: %v", err),
|
||||||
}
|
}
|
||||||
issue.Labels = labels
|
}
|
||||||
|
for _, issue := range issues {
|
||||||
|
issue.Labels = allLabels[issue.ID]
|
||||||
}
|
}
|
||||||
|
|
||||||
// Populate comments for all issues
|
// Populate comments for all issues (avoid N+1)
|
||||||
for _, issue := range issues {
|
allComments, err := store.GetCommentsForIssues(ctx, issueIDs)
|
||||||
comments, err := store.GetIssueComments(ctx, issue.ID)
|
if err != nil {
|
||||||
if err != nil {
|
return Response{
|
||||||
return Response{
|
Success: false,
|
||||||
Success: false,
|
Error: fmt.Sprintf("failed to get comments: %v", err),
|
||||||
Error: fmt.Sprintf("failed to get comments for %s: %v", issue.ID, err),
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
issue.Comments = comments
|
}
|
||||||
|
for _, issue := range issues {
|
||||||
|
issue.Comments = allComments[issue.ID]
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create temp file for atomic write
|
// Create temp file for atomic write
|
||||||
@@ -387,7 +391,7 @@ func (s *Server) triggerExport(ctx context.Context, store storage.Storage, dbPat
|
|||||||
})
|
})
|
||||||
|
|
||||||
// CRITICAL: Populate all related data to prevent data loss
|
// CRITICAL: Populate all related data to prevent data loss
|
||||||
// This mirrors the logic in handleExport (lines 50-83)
|
// This mirrors the logic in handleExport
|
||||||
|
|
||||||
// Populate dependencies for all issues (avoid N+1 queries)
|
// Populate dependencies for all issues (avoid N+1 queries)
|
||||||
allDeps, err := store.GetAllDependencyRecords(ctx)
|
allDeps, err := store.GetAllDependencyRecords(ctx)
|
||||||
@@ -398,22 +402,26 @@ func (s *Server) triggerExport(ctx context.Context, store storage.Storage, dbPat
|
|||||||
issue.Dependencies = allDeps[issue.ID]
|
issue.Dependencies = allDeps[issue.ID]
|
||||||
}
|
}
|
||||||
|
|
||||||
// Populate labels for all issues
|
// Populate labels for all issues (avoid N+1 queries)
|
||||||
|
issueIDs := make([]string, len(allIssues))
|
||||||
|
for i, issue := range allIssues {
|
||||||
|
issueIDs[i] = issue.ID
|
||||||
|
}
|
||||||
|
allLabels, err := store.GetLabelsForIssues(ctx, issueIDs)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to get labels: %w", err)
|
||||||
|
}
|
||||||
for _, issue := range allIssues {
|
for _, issue := range allIssues {
|
||||||
labels, err := store.GetLabels(ctx, issue.ID)
|
issue.Labels = allLabels[issue.ID]
|
||||||
if err != nil {
|
|
||||||
return fmt.Errorf("failed to get labels for %s: %w", issue.ID, err)
|
|
||||||
}
|
|
||||||
issue.Labels = labels
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Populate comments for all issues
|
// Populate comments for all issues (avoid N+1 queries)
|
||||||
|
allComments, err := store.GetCommentsForIssues(ctx, issueIDs)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to get comments: %w", err)
|
||||||
|
}
|
||||||
for _, issue := range allIssues {
|
for _, issue := range allIssues {
|
||||||
comments, err := store.GetIssueComments(ctx, issue.ID)
|
issue.Comments = allComments[issue.ID]
|
||||||
if err != nil {
|
|
||||||
return fmt.Errorf("failed to get comments for %s: %w", issue.ID, err)
|
|
||||||
}
|
|
||||||
issue.Comments = comments
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Write to JSONL file with atomic replace (temp file + rename)
|
// Write to JSONL file with atomic replace (temp file + rename)
|
||||||
|
|||||||
@@ -912,6 +912,19 @@ func (m *MemoryStorage) GetIssueComments(ctx context.Context, issueID string) ([
|
|||||||
return m.comments[issueID], nil
|
return m.comments[issueID], nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func (m *MemoryStorage) GetCommentsForIssues(ctx context.Context, issueIDs []string) (map[string][]*types.Comment, error) {
|
||||||
|
m.mu.RLock()
|
||||||
|
defer m.mu.RUnlock()
|
||||||
|
|
||||||
|
result := make(map[string][]*types.Comment)
|
||||||
|
for _, issueID := range issueIDs {
|
||||||
|
if comments, exists := m.comments[issueID]; exists {
|
||||||
|
result[issueID] = comments
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return result, nil
|
||||||
|
}
|
||||||
|
|
||||||
func (m *MemoryStorage) GetStatistics(ctx context.Context) (*types.Statistics, error) {
|
func (m *MemoryStorage) GetStatistics(ctx context.Context) (*types.Statistics, error) {
|
||||||
m.mu.RLock()
|
m.mu.RLock()
|
||||||
defer m.mu.RUnlock()
|
defer m.mu.RUnlock()
|
||||||
|
|||||||
143
internal/storage/sqlite/batch_test.go
Normal file
143
internal/storage/sqlite/batch_test.go
Normal file
@@ -0,0 +1,143 @@
|
|||||||
|
package sqlite
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/steveyegge/beads/internal/types"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestBatchGetLabelsAndComments verifies that GetLabelsForIssues and GetCommentsForIssues
|
||||||
|
// correctly fetch data for multiple issues in a single query (avoiding N+1 pattern)
|
||||||
|
func TestBatchGetLabelsAndComments(t *testing.T) {
|
||||||
|
store, cleanup := setupTestDB(t)
|
||||||
|
defer cleanup()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Create test issues
|
||||||
|
issues := []*types.Issue{
|
||||||
|
{
|
||||||
|
ID: "bd-batch1",
|
||||||
|
Title: "Issue 1",
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
ID: "bd-batch2",
|
||||||
|
Title: "Issue 2",
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
ID: "bd-batch3",
|
||||||
|
Title: "Issue 3",
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, issue := range issues {
|
||||||
|
if err := store.CreateIssue(ctx, issue, "test-actor"); err != nil {
|
||||||
|
t.Fatalf("Failed to create issue %s: %v", issue.ID, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Add labels to issues
|
||||||
|
if err := store.AddLabel(ctx, "bd-batch1", "bug", "test-actor"); err != nil {
|
||||||
|
t.Fatalf("Failed to add label: %v", err)
|
||||||
|
}
|
||||||
|
if err := store.AddLabel(ctx, "bd-batch1", "urgent", "test-actor"); err != nil {
|
||||||
|
t.Fatalf("Failed to add label: %v", err)
|
||||||
|
}
|
||||||
|
if err := store.AddLabel(ctx, "bd-batch2", "feature", "test-actor"); err != nil {
|
||||||
|
t.Fatalf("Failed to add label: %v", err)
|
||||||
|
}
|
||||||
|
// bd-batch3 has no labels
|
||||||
|
|
||||||
|
// Add comments to issues
|
||||||
|
if _, err := store.AddIssueComment(ctx, "bd-batch1", "alice", "First comment"); err != nil {
|
||||||
|
t.Fatalf("Failed to add comment: %v", err)
|
||||||
|
}
|
||||||
|
if _, err := store.AddIssueComment(ctx, "bd-batch1", "bob", "Second comment"); err != nil {
|
||||||
|
t.Fatalf("Failed to add comment: %v", err)
|
||||||
|
}
|
||||||
|
if _, err := store.AddIssueComment(ctx, "bd-batch3", "charlie", "Comment on bd-batch3"); err != nil {
|
||||||
|
t.Fatalf("Failed to add comment: %v", err)
|
||||||
|
}
|
||||||
|
// bd-batch2 has no comments
|
||||||
|
|
||||||
|
// Test batch get labels
|
||||||
|
issueIDs := []string{"bd-batch1", "bd-batch2", "bd-batch3"}
|
||||||
|
allLabels, err := store.GetLabelsForIssues(ctx, issueIDs)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetLabelsForIssues failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify labels
|
||||||
|
if len(allLabels["bd-batch1"]) != 2 {
|
||||||
|
t.Errorf("Expected 2 labels for bd-batch1, got %d", len(allLabels["bd-batch1"]))
|
||||||
|
}
|
||||||
|
if len(allLabels["bd-batch2"]) != 1 {
|
||||||
|
t.Errorf("Expected 1 label for bd-batch2, got %d", len(allLabels["bd-batch2"]))
|
||||||
|
}
|
||||||
|
if len(allLabels["bd-batch3"]) != 0 {
|
||||||
|
t.Errorf("Expected 0 labels for bd-batch3, got %d", len(allLabels["bd-batch3"]))
|
||||||
|
}
|
||||||
|
|
||||||
|
// Test batch get comments
|
||||||
|
allComments, err := store.GetCommentsForIssues(ctx, issueIDs)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetCommentsForIssues failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Verify comments
|
||||||
|
if len(allComments["bd-batch1"]) != 2 {
|
||||||
|
t.Errorf("Expected 2 comments for bd-batch1, got %d", len(allComments["bd-batch1"]))
|
||||||
|
}
|
||||||
|
if allComments["bd-batch1"][0].Author != "alice" {
|
||||||
|
t.Errorf("Expected first comment author to be 'alice', got %s", allComments["bd-batch1"][0].Author)
|
||||||
|
}
|
||||||
|
if allComments["bd-batch1"][1].Author != "bob" {
|
||||||
|
t.Errorf("Expected second comment author to be 'bob', got %s", allComments["bd-batch1"][1].Author)
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(allComments["bd-batch2"]) != 0 {
|
||||||
|
t.Errorf("Expected 0 comments for bd-batch2, got %d", len(allComments["bd-batch2"]))
|
||||||
|
}
|
||||||
|
|
||||||
|
if len(allComments["bd-batch3"]) != 1 {
|
||||||
|
t.Errorf("Expected 1 comment for bd-batch3, got %d", len(allComments["bd-batch3"]))
|
||||||
|
}
|
||||||
|
if len(allComments["bd-batch3"]) > 0 && allComments["bd-batch3"][0].Author != "charlie" {
|
||||||
|
t.Errorf("Expected comment author to be 'charlie', got %s", allComments["bd-batch3"][0].Author)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestBatchGetEmptyIssueIDs verifies that batch methods handle empty issue ID lists
|
||||||
|
func TestBatchGetEmptyIssueIDs(t *testing.T) {
|
||||||
|
store, cleanup := setupTestDB(t)
|
||||||
|
defer cleanup()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Test with empty issue ID list
|
||||||
|
labels, err := store.GetLabelsForIssues(ctx, []string{})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetLabelsForIssues with empty list failed: %v", err)
|
||||||
|
}
|
||||||
|
if len(labels) != 0 {
|
||||||
|
t.Errorf("Expected empty map, got %d entries", len(labels))
|
||||||
|
}
|
||||||
|
|
||||||
|
comments, err := store.GetCommentsForIssues(ctx, []string{})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetCommentsForIssues with empty list failed: %v", err)
|
||||||
|
}
|
||||||
|
if len(comments) != 0 {
|
||||||
|
t.Errorf("Expected empty map, got %d entries", len(comments))
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -81,3 +81,46 @@ func (s *SQLiteStorage) GetIssueComments(ctx context.Context, issueID string) ([
|
|||||||
|
|
||||||
return comments, nil
|
return comments, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GetCommentsForIssues fetches comments for multiple issues in a single query
|
||||||
|
// Returns a map of issue_id -> []*Comment
|
||||||
|
func (s *SQLiteStorage) GetCommentsForIssues(ctx context.Context, issueIDs []string) (map[string][]*types.Comment, error) {
|
||||||
|
if len(issueIDs) == 0 {
|
||||||
|
return make(map[string][]*types.Comment), nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// Build placeholders for IN clause
|
||||||
|
placeholders := make([]interface{}, len(issueIDs))
|
||||||
|
for i, id := range issueIDs {
|
||||||
|
placeholders[i] = id
|
||||||
|
}
|
||||||
|
|
||||||
|
query := fmt.Sprintf(`
|
||||||
|
SELECT id, issue_id, author, text, created_at
|
||||||
|
FROM comments
|
||||||
|
WHERE issue_id IN (%s)
|
||||||
|
ORDER BY issue_id, created_at ASC
|
||||||
|
`, buildPlaceholders(len(issueIDs))) // #nosec G201 -- placeholders are generated internally
|
||||||
|
|
||||||
|
rows, err := s.db.QueryContext(ctx, query, placeholders...)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("failed to batch get comments: %w", err)
|
||||||
|
}
|
||||||
|
defer func() { _ = rows.Close() }()
|
||||||
|
|
||||||
|
result := make(map[string][]*types.Comment)
|
||||||
|
for rows.Next() {
|
||||||
|
comment := &types.Comment{}
|
||||||
|
err := rows.Scan(&comment.ID, &comment.IssueID, &comment.Author, &comment.Text, &comment.CreatedAt)
|
||||||
|
if err != nil {
|
||||||
|
return nil, fmt.Errorf("failed to scan comment: %w", err)
|
||||||
|
}
|
||||||
|
result[comment.IssueID] = append(result[comment.IssueID], comment)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := rows.Err(); err != nil {
|
||||||
|
return nil, fmt.Errorf("error iterating comments: %w", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
return result, nil
|
||||||
|
}
|
||||||
|
|||||||
@@ -51,6 +51,7 @@ type Storage interface {
|
|||||||
// Comments
|
// Comments
|
||||||
AddIssueComment(ctx context.Context, issueID, author, text string) (*types.Comment, error)
|
AddIssueComment(ctx context.Context, issueID, author, text string) (*types.Comment, error)
|
||||||
GetIssueComments(ctx context.Context, issueID string) ([]*types.Comment, error)
|
GetIssueComments(ctx context.Context, issueID string) ([]*types.Comment, error)
|
||||||
|
GetCommentsForIssues(ctx context.Context, issueIDs []string) (map[string][]*types.Comment, error)
|
||||||
|
|
||||||
// Statistics
|
// Statistics
|
||||||
GetStatistics(ctx context.Context) (*types.Statistics, error)
|
GetStatistics(ctx context.Context) (*types.Statistics, error)
|
||||||
|
|||||||
Reference in New Issue
Block a user