fix(filters): release download counts (#1739)

* fix(database): count multiple actions per filter as one download

* fix(database): change queries and add tests

* fix(releases): add additional testdata

---------

Co-authored-by: ze0s <ze0s@riseup.net>

internal/database/filter.go

@@ -1320,13 +1320,13 @@ func (r *FilterRepo) GetDownloadsByFilterId(ctx context.Context, filterID int) (
 
 func (r *FilterRepo) downloadsByFilterSqlite(ctx context.Context, filterID int) (*domain.FilterDownloads, error) {
 	query := `SELECT
-	COUNT(CASE WHEN CAST(strftime('%s', datetime(release_action_status.timestamp, 'localtime')) AS INTEGER) >= CAST(strftime('%s', strftime('%Y-%m-%dT%H:00:00', datetime('now','localtime'))) AS INTEGER) THEN 1 END) as "hour_count",
+	COUNT(DISTINCT CASE WHEN CAST(strftime('%s', datetime(timestamp, 'localtime')) AS INTEGER) >= CAST(strftime('%s', strftime('%Y-%m-%dT%H:00:00', datetime('now','localtime'))) AS INTEGER) THEN release_id END) as "hour_count",
-	COUNT(CASE WHEN CAST(strftime('%s', datetime(release_action_status.timestamp, 'localtime')) AS INTEGER) >= CAST(strftime('%s', datetime('now', 'localtime', 'start of day')) AS INTEGER) THEN 1 END) as "day_count",
+	COUNT(DISTINCT CASE WHEN CAST(strftime('%s', datetime(timestamp, 'localtime')) AS INTEGER) >= CAST(strftime('%s', datetime('now', 'localtime', 'start of day')) AS INTEGER) THEN release_id END) as "day_count",
-	COUNT(CASE WHEN CAST(strftime('%s', datetime(release_action_status.timestamp, 'localtime')) AS INTEGER) >= CAST(strftime('%s', datetime('now', 'localtime', 'weekday 0', '-7 days', 'start of day')) AS INTEGER) THEN 1 END) as "week_count",
+	COUNT(DISTINCT CASE WHEN CAST(strftime('%s', datetime(timestamp, 'localtime')) AS INTEGER) >= CAST(strftime('%s', datetime('now', 'localtime', 'weekday 0', '-7 days', 'start of day')) AS INTEGER) THEN release_id END) as "week_count",
-	COUNT(CASE WHEN CAST(strftime('%s', datetime(release_action_status.timestamp, 'localtime')) AS INTEGER) >= CAST(strftime('%s', datetime('now', 'localtime', 'start of month')) AS INTEGER) THEN 1 END) as "month_count",
+	COUNT(DISTINCT CASE WHEN CAST(strftime('%s', datetime(timestamp, 'localtime')) AS INTEGER) >= CAST(strftime('%s', datetime('now', 'localtime', 'start of month')) AS INTEGER) THEN release_id END) as "month_count",
-	COUNT(*) as "total_count"
+	COUNT(DISTINCT release_id) as "total_count"
 FROM release_action_status
-WHERE (release_action_status.status = 'PUSH_APPROVED' OR release_action_status.status = 'PENDING') AND release_action_status.filter_id = ?;`
+WHERE status IN ('PUSH_APPROVED', 'PUSH_PENDING') AND filter_id = ?;`
 
 	row := r.db.handler.QueryRowContext(ctx, query, filterID)
 	if err := row.Err(); err != nil {
@@ -1350,13 +1350,13 @@ WHERE (release_action_status.status = 'PUSH_APPROVED' OR release_action_status.s
 
 func (r *FilterRepo) downloadsByFilterPostgres(ctx context.Context, filterID int) (*domain.FilterDownloads, error) {
 	query := `SELECT
-    COALESCE(SUM(CASE WHEN release_action_status.timestamp >= date_trunc('hour', CURRENT_TIMESTAMP) THEN 1 ELSE 0 END),0) as "hour_count",
+    COUNT(DISTINCT CASE WHEN timestamp >= date_trunc('hour', CURRENT_TIMESTAMP) THEN release_id END) as "hour_count",
-    COALESCE(SUM(CASE WHEN release_action_status.timestamp >= date_trunc('day', CURRENT_DATE) THEN 1 ELSE 0 END),0) as "day_count",
+    COUNT(DISTINCT CASE WHEN timestamp >= date_trunc('day', CURRENT_DATE) THEN release_id END) as "day_count",
-    COALESCE(SUM(CASE WHEN release_action_status.timestamp >= date_trunc('week', CURRENT_DATE) THEN 1 ELSE 0 END),0) as "week_count",
+    COUNT(DISTINCT CASE WHEN timestamp >= date_trunc('week', CURRENT_DATE) THEN release_id END) as "week_count",
-    COALESCE(SUM(CASE WHEN release_action_status.timestamp >= date_trunc('month', CURRENT_DATE) THEN 1 ELSE 0 END),0) as "month_count",
+    COUNT(DISTINCT CASE WHEN timestamp >= date_trunc('month', CURRENT_DATE) THEN release_id END) as "month_count",
-    count(*) as "total_count"
+    COUNT(DISTINCT release_id) as "total_count"
 FROM release_action_status
-WHERE (release_action_status.status = 'PUSH_APPROVED' OR release_action_status.status = 'PENDING') AND release_action_status.filter_id = $1;`
+WHERE status IN ('PUSH_APPROVED', 'PUSH_PENDING') AND filter_id = $1;`
 
 	row := r.db.handler.QueryRowContext(ctx, query, filterID)
 	if err := row.Err(); err != nil {

internal/database/filter_test.go

@@ -846,5 +846,154 @@ func TestFilterRepo_GetDownloadsByFilterId(t *testing.T) {
 			})
 		})
 
+		t.Run(fmt.Sprintf("GetDownloadsByFilterId_Multiple_Actions [%s]", dbType), func(t *testing.T) {
+			// Setup
+			err := repo.Store(context.Background(), mockData)
+			assert.NoError(t, err)
+
+			mockClient := getMockDownloadClient()
+
+			err = downloadClientRepo.Store(context.Background(), &mockClient)
+			assert.NoError(t, err)
+			assert.NotNil(t, mockClient)
+
+			mockAction1 := getMockAction()
+			mockAction1.FilterID = mockData.ID
+			mockAction1.ClientID = mockClient.ID
+
+			action1, err := actionRepo.Store(context.Background(), mockAction1)
+
+			mockAction2 := getMockAction()
+			mockAction2.FilterID = mockData.ID
+			mockAction2.ClientID = mockClient.ID
+
+			action2, err := actionRepo.Store(context.Background(), mockAction2)
+
+			mockRelease.FilterID = mockData.ID
+
+			err = releaseRepo.Store(context.Background(), mockRelease)
+			assert.NoError(t, err)
+
+			mockReleaseActionStatus1 := getMockReleaseActionStatus()
+			mockReleaseActionStatus1.ActionID = int64(action1.ID)
+			mockReleaseActionStatus1.FilterID = int64(mockData.ID)
+			mockReleaseActionStatus1.ReleaseID = mockRelease.ID
+
+			err = releaseRepo.StoreReleaseActionStatus(context.Background(), mockReleaseActionStatus1)
+			assert.NoError(t, err)
+
+			mockReleaseActionStatus2 := getMockReleaseActionStatus()
+			mockReleaseActionStatus2.ActionID = int64(action2.ID)
+			mockReleaseActionStatus2.FilterID = int64(mockData.ID)
+			mockReleaseActionStatus2.ReleaseID = mockRelease.ID
+
+			err = releaseRepo.StoreReleaseActionStatus(context.Background(), mockReleaseActionStatus2)
+			assert.NoError(t, err)
+
+			// Execute
+			downloads, err := repo.GetDownloadsByFilterId(context.Background(), mockData.ID)
+			assert.NoError(t, err)
+			assert.NotNil(t, downloads)
+			assert.Equal(t, downloads, &domain.FilterDownloads{
+				HourCount:  1,
+				DayCount:   1,
+				WeekCount:  1,
+				MonthCount: 1,
+				TotalCount: 1,
+			})
+
+			// Cleanup
+			_ = actionRepo.Delete(context.Background(), &domain.DeleteActionRequest{ActionId: action1.ID})
+			_ = actionRepo.Delete(context.Background(), &domain.DeleteActionRequest{ActionId: action2.ID})
+			_ = repo.Delete(context.Background(), mockData.ID)
+			_ = downloadClientRepo.Delete(context.Background(), mockClient.ID)
+			_ = releaseRepo.Delete(context.Background(), &domain.DeleteReleaseRequest{OlderThan: 0})
+		})
+
+		t.Run(fmt.Sprintf("GetDownloadsByFilterId_Old_Release [%s]", dbType), func(t *testing.T) {
+			// Setup
+			err := repo.Store(context.Background(), mockData)
+			assert.NoError(t, err)
+
+			mockClient := getMockDownloadClient()
+
+			err = downloadClientRepo.Store(context.Background(), &mockClient)
+			assert.NoError(t, err)
+			assert.NotNil(t, mockClient)
+
+			mockAction.FilterID = mockData.ID
+			mockAction.ClientID = mockClient.ID
+
+			action, err := actionRepo.Store(context.Background(), mockAction)
+
+			mockAction2 := getMockAction()
+			mockAction2.FilterID = mockData.ID
+			mockAction2.ClientID = mockClient.ID
+
+			action2, err := actionRepo.Store(context.Background(), mockAction2)
+
+			mockRelease.FilterID = mockData.ID
+
+			err = releaseRepo.Store(context.Background(), mockRelease)
+			assert.NoError(t, err)
+
+			mockReleaseActionStatus = getMockReleaseActionStatus()
+			mockReleaseActionStatus.ActionID = int64(action.ID)
+			mockReleaseActionStatus.FilterID = int64(mockData.ID)
+			mockReleaseActionStatus.ReleaseID = mockRelease.ID
+			mockReleaseActionStatus.Timestamp = mockReleaseActionStatus.Timestamp.AddDate(0, -1, 0)
+
+			err = releaseRepo.StoreReleaseActionStatus(context.Background(), mockReleaseActionStatus)
+			assert.NoError(t, err)
+
+			mockReleaseActionStatus2 := getMockReleaseActionStatus()
+			mockReleaseActionStatus2.ActionID = int64(action2.ID)
+			mockReleaseActionStatus2.FilterID = int64(mockData.ID)
+			mockReleaseActionStatus2.ReleaseID = mockRelease.ID
+			mockReleaseActionStatus2.Timestamp = mockReleaseActionStatus2.Timestamp.AddDate(0, -1, 0)
+
+			err = releaseRepo.StoreReleaseActionStatus(context.Background(), mockReleaseActionStatus2)
+			assert.NoError(t, err)
+
+			// Execute
+			downloads, err := repo.GetDownloadsByFilterId(context.Background(), mockData.ID)
+			assert.NoError(t, err)
+			assert.NotNil(t, downloads)
+			assert.Equal(t, downloads, &domain.FilterDownloads{
+				HourCount:  0,
+				DayCount:   0,
+				WeekCount:  0,
+				MonthCount: 0,
+				TotalCount: 1,
+			})
+
+			// Cleanup
+			_ = actionRepo.Delete(context.Background(), &domain.DeleteActionRequest{ActionId: action.ID})
+			_ = repo.Delete(context.Background(), mockData.ID)
+			_ = downloadClientRepo.Delete(context.Background(), mockClient.ID)
+			_ = releaseRepo.Delete(context.Background(), &domain.DeleteReleaseRequest{OlderThan: 0})
+		})
+
+		t.Run(fmt.Sprintf("GetDownloadsByFilterId_No_Releases [%s]", dbType), func(t *testing.T) {
+			// Setup
+			err := repo.Store(context.Background(), mockData)
+			assert.NoError(t, err)
+
+			// Execute
+			downloads, err := repo.GetDownloadsByFilterId(context.Background(), mockData.ID)
+			assert.NoError(t, err)
+			assert.NotNil(t, downloads)
+			assert.Equal(t, downloads, &domain.FilterDownloads{
+				HourCount:  0,
+				DayCount:   0,
+				WeekCount:  0,
+				MonthCount: 0,
+				TotalCount: 0,
+			})
+
+			// Cleanup
+			_ = repo.Delete(context.Background(), mockData.ID)
+		})
+
 	}
 }