fix: sync ID counters after import to prevent collisions
When importing issues with explicit high IDs (e.g., bd-100), the issue_counters table wasn't being updated. This caused the next auto-generated issue to collide with existing IDs (bd-4 instead of bd-101). Changes: - Add SyncAllCounters() to scan all issues and update counters atomically - Add SyncCounterForPrefix() for granular counter synchronization - Call SyncAllCounters() in import command after creating issues - Add comprehensive tests for counter sync functionality - Update TestImportCounterSyncAfterHighID to verify fix The fix uses a single efficient SQL query to prevent ID collisions with subsequently auto-generated issues.
This commit is contained in:
@@ -238,6 +238,13 @@ Behavior:
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Phase 4.5: Sync ID counters after importing issues with explicit IDs
|
||||||
|
// This prevents ID collisions with subsequently auto-generated issues
|
||||||
|
if err := sqliteStore.SyncAllCounters(ctx); err != nil {
|
||||||
|
fmt.Fprintf(os.Stderr, "Warning: failed to sync ID counters: %v\n", err)
|
||||||
|
// Don't exit - this is not fatal, just a warning
|
||||||
|
}
|
||||||
|
|
||||||
// Phase 5: Process dependencies
|
// Phase 5: Process dependencies
|
||||||
// Do this after all issues are created to handle forward references
|
// Do this after all issues are created to handle forward references
|
||||||
var depsCreated, depsSkipped int
|
var depsCreated, depsSkipped int
|
||||||
|
|||||||
@@ -1023,6 +1023,13 @@ func TestImportCounterSyncAfterHighID(t *testing.T) {
|
|||||||
t.Fatalf("Failed to import high ID issue: %v", err)
|
t.Fatalf("Failed to import high ID issue: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Step 4: Sync counters after import (mimics import command behavior)
|
||||||
|
if err := testStore.SyncAllCounters(ctx); err != nil {
|
||||||
|
t.Fatalf("Failed to sync counters: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Step 5: Create another auto-generated issue
|
||||||
|
// This should get bd-101 (counter should have synced to 100), not bd-4
|
||||||
newIssue := &types.Issue{
|
newIssue := &types.Issue{
|
||||||
Title: "New issue after import",
|
Title: "New issue after import",
|
||||||
Status: types.StatusOpen,
|
Status: types.StatusOpen,
|
||||||
|
|||||||
@@ -107,6 +107,22 @@ func (s *SQLiteStorage) getNextIDForPrefix(ctx context.Context, prefix string) (
|
|||||||
return nextID, nil
|
return nextID, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SyncCounterForPrefix synchronizes the counter to be at least the given value
|
||||||
|
// This is used after importing issues with explicit IDs to prevent ID collisions
|
||||||
|
// with subsequently auto-generated IDs
|
||||||
|
func (s *SQLiteStorage) SyncCounterForPrefix(ctx context.Context, prefix string, minValue int) error {
|
||||||
|
_, err := s.db.ExecContext(ctx, `
|
||||||
|
INSERT INTO issue_counters (prefix, last_id)
|
||||||
|
VALUES (?, ?)
|
||||||
|
ON CONFLICT(prefix) DO UPDATE SET
|
||||||
|
last_id = MAX(last_id, ?)
|
||||||
|
`, prefix, minValue, minValue)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("failed to sync counter for prefix %s: %w", prefix, err)
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
// SyncAllCounters synchronizes all ID counters based on existing issues in the database
|
// SyncAllCounters synchronizes all ID counters based on existing issues in the database
|
||||||
// This scans all issues and updates counters to prevent ID collisions with auto-generated IDs
|
// This scans all issues and updates counters to prevent ID collisions with auto-generated IDs
|
||||||
func (s *SQLiteStorage) SyncAllCounters(ctx context.Context) error {
|
func (s *SQLiteStorage) SyncAllCounters(ctx context.Context) error {
|
||||||
|
|||||||
@@ -479,3 +479,68 @@ func TestMultiProcessIDGeneration(t *testing.T) {
|
|||||||
t.Errorf("Expected no errors, got %d", len(errors))
|
t.Errorf("Expected no errors, got %d", len(errors))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestSyncCounterForPrefix(t *testing.T) {
|
||||||
|
store, cleanup := setupTestDB(t)
|
||||||
|
defer cleanup()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
// Set config for issue prefix
|
||||||
|
if err := store.SetConfig(ctx, "issue_prefix", "bd"); err != nil {
|
||||||
|
t.Fatalf("Failed to set issue prefix: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Create a few auto-generated issues (bd-1, bd-2, bd-3)
|
||||||
|
for i := 0; i < 3; i++ {
|
||||||
|
issue := &types.Issue{
|
||||||
|
Title: "Auto issue",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := store.CreateIssue(ctx, issue, "test"); err != nil {
|
||||||
|
t.Fatalf("Failed to create issue: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Sync counter to 100
|
||||||
|
if err := store.SyncCounterForPrefix(ctx, "bd", 100); err != nil {
|
||||||
|
t.Fatalf("SyncCounterForPrefix failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Next auto-generated issue should be bd-101
|
||||||
|
issue := &types.Issue{
|
||||||
|
Title: "After sync",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := store.CreateIssue(ctx, issue, "test"); err != nil {
|
||||||
|
t.Fatalf("Failed to create issue after sync: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if issue.ID != "bd-101" {
|
||||||
|
t.Errorf("Expected ID bd-101 after sync, got %s", issue.ID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Syncing to a lower value should not decrease the counter
|
||||||
|
if err := store.SyncCounterForPrefix(ctx, "bd", 50); err != nil {
|
||||||
|
t.Fatalf("SyncCounterForPrefix failed: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
// Next issue should still be bd-102, not bd-51
|
||||||
|
issue2 := &types.Issue{
|
||||||
|
Title: "After lower sync",
|
||||||
|
Status: types.StatusOpen,
|
||||||
|
Priority: 1,
|
||||||
|
IssueType: types.TypeTask,
|
||||||
|
}
|
||||||
|
if err := store.CreateIssue(ctx, issue2, "test"); err != nil {
|
||||||
|
t.Fatalf("Failed to create issue: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if issue2.ID != "bd-102" {
|
||||||
|
t.Errorf("Expected ID bd-102 (counter should not decrease), got %s", issue2.ID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user