diff --git a/pkg/models/saved_filter_positions_test.go b/pkg/models/saved_filter_positions_test.go index 63220d383..d233460d2 100644 --- a/pkg/models/saved_filter_positions_test.go +++ b/pkg/models/saved_filter_positions_test.go @@ -24,7 +24,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "xorm.io/builder" ) func TestSavedFilterUpdateInsertsNonZeroPosition(t *testing.T) { @@ -305,41 +304,3 @@ func TestIssue724_SortingOnFilteredViews(t *testing.T) { assert.Zero(t, zeroCount, "No position=0 records should exist in database for view %d", view.ID) } - -func TestUpsertRelatedTaskPropertiesBatchesDeletes(t *testing.T) { - db.LoadAndAssertFixtures(t) - s := db.NewSession() - defer s.Close() - - viewID := int64(1) - - // Insert many task buckets and positions that we'll then delete - for i := int64(1); i <= 1200; i++ { - _, err := s.Insert(&TaskBucket{TaskID: 10000 + i, ProjectViewID: viewID, BucketID: 1}) - require.NoError(t, err) - _, err = s.Insert(&TaskPosition{TaskID: 10000 + i, ProjectViewID: viewID, Position: float64(i)}) - require.NoError(t, err) - } - - // Build 1200 delete conditions -- this would exceed SQLite's 1000-node limit - // if passed as a single builder.Or() - deleteCond := make([]builder.Cond, 0, 1200) - for i := int64(1); i <= 1200; i++ { - deleteCond = append(deleteCond, builder.And( - builder.Eq{"task_id": 10000 + i}, - builder.Eq{"project_view_id": viewID}, - )) - } - - // This should not panic or error -- it should batch the deletes - upsertRelatedTaskProperties(s, "[test] ", nil, nil, deleteCond) - - // Verify all were deleted - count, err := s.Where("project_view_id = ? AND task_id > 10000", viewID).Count(&TaskBucket{}) - require.NoError(t, err) - assert.Equal(t, int64(0), count) - - count, err = s.Where("project_view_id = ? AND task_id > 10000", viewID).Count(&TaskPosition{}) - require.NoError(t, err) - assert.Equal(t, int64(0), count) -} diff --git a/pkg/models/saved_filters.go b/pkg/models/saved_filters.go index 00ebc96b3..26f99ed48 100644 --- a/pkg/models/saved_filters.go +++ b/pkg/models/saved_filters.go @@ -429,7 +429,7 @@ func RegisterAddTaskToFilterViewCron() { filterTasksCache := make(map[int64][]*Task) newTaskBuckets := []*TaskBucket{} newTaskPositions := []*TaskPosition{} - deleteCond := []builder.Cond{} + viewsToRecalc := map[int64]struct { view *ProjectView ownerID int64 @@ -507,24 +507,10 @@ func RegisterAddTaskToFilterViewCron() { } // Remove tasks that should not be there - for taskID := range savedTaskBucketMap { - found := false - for _, task := range tasks { - if task.ID == taskID { - found = true - break - } - } - if !found { - deleteCond = append(deleteCond, builder.And( - builder.Eq{"task_id": taskID}, - builder.Eq{"project_view_id": view.ID}, - )) - } - } + deleteStaleFilterTasks(s, logPrefix, savedTaskBucketMap, tasks, view.ID) } - upsertRelatedTaskProperties(s, logPrefix, newTaskBuckets, newTaskPositions, deleteCond) + upsertRelatedTaskProperties(s, logPrefix, newTaskBuckets, newTaskPositions) for _, data := range viewsToRecalc { if err := RecalculateTaskPositions(s, data.view, &user.User{ID: data.ownerID}); err != nil { @@ -541,13 +527,7 @@ func RegisterAddTaskToFilterViewCron() { } } -// deleteCondBatchSize is the maximum number of OR conditions in a single delete query. -// SQLite has a hard limit of 1000 expression tree nodes; each builder.And(...) with two -// Eq conditions contributes ~3 nodes to the Or-tree, so 500 conditions stays safely -// under the limit on all supported databases. -const deleteCondBatchSize = 500 - -func upsertRelatedTaskProperties(s *xorm.Session, logPrefix string, newTaskBuckets []*TaskBucket, newTaskPositions []*TaskPosition, deleteCond []builder.Cond) { +func upsertRelatedTaskProperties(s *xorm.Session, logPrefix string, newTaskBuckets []*TaskBucket, newTaskPositions []*TaskPosition) { var err error if len(newTaskBuckets) > 0 { _, err = s.Insert(newTaskBuckets) @@ -561,22 +541,34 @@ func upsertRelatedTaskProperties(s *xorm.Session, logPrefix string, newTaskBucke log.Errorf("%sError inserting task positions: %s", logPrefix, err) } } - if len(deleteCond) > 0 { - for i := 0; i < len(deleteCond); i += deleteCondBatchSize { - end := i + deleteCondBatchSize - if end > len(deleteCond) { - end = len(deleteCond) - } - batch := deleteCond[i:end] +} - _, err = s.Where(builder.Or(batch...)).Delete(&TaskBucket{}) - if err != nil { - log.Errorf("%sError deleting task buckets: %s", logPrefix, err) - } - _, err = s.Where(builder.Or(batch...)).Delete(&TaskPosition{}) - if err != nil { - log.Errorf("%sError deleting task positions: %s", logPrefix, err) +func deleteStaleFilterTasks(s *xorm.Session, logPrefix string, savedTaskBucketMap map[int64]*TaskBucket, tasks []*Task, viewID int64) { + var taskIDsToDelete []int64 + for taskID := range savedTaskBucketMap { + found := false + for _, task := range tasks { + if task.ID == taskID { + found = true + break } } + if !found { + taskIDsToDelete = append(taskIDsToDelete, taskID) + } + } + if len(taskIDsToDelete) > 0 { + _, err := s.Where(builder.Eq{"project_view_id": viewID}). + And(builder.In("task_id", taskIDsToDelete)). + Delete(&TaskBucket{}) + if err != nil { + log.Errorf("%sError deleting task buckets: %s", logPrefix, err) + } + _, err = s.Where(builder.Eq{"project_view_id": viewID}). + And(builder.In("task_id", taskIDsToDelete)). + Delete(&TaskPosition{}) + if err != nil { + log.Errorf("%sError deleting task positions: %s", logPrefix, err) + } } }