Setting NextCheckAt due to TTL of a feed in feed.go.

Add unit tests.
miniflux · Nov 28, 2023 · 093a5ed · 093a5ed
1 parent bcb01b5
commit 093a5ed
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 31 deletions.
diff --git a/internal/model/feed.go b/internal/model/feed.go
@@ -107,21 +107,27 @@ func (f *Feed) CheckedNow() {
 }
 
 // ScheduleNextCheck set "next_check_at" of a feed based on the scheduler selected from the configuration.
-func (f *Feed) ScheduleNextCheck(weeklyCount int) {
+func (f *Feed) ScheduleNextCheck(weeklyCount int, newTTL int) {
+	f.TTL = newTTL
+	// Default to the global config Polling Frequency.
+	var intervalMinutes int
 	switch config.Opts.PollingScheduler() {
 	case SchedulerEntryFrequency:
-		var intervalMinutes int
-		if weeklyCount == 0 {
+		if weeklyCount <= 0 {
 			intervalMinutes = config.Opts.SchedulerEntryFrequencyMaxInterval()
 		} else {
 			intervalMinutes = int(math.Round(float64(7*24*60) / float64(weeklyCount*config.Opts.SchedulerEntryFrequencyFactor())))
 			intervalMinutes = int(math.Min(float64(intervalMinutes), float64(config.Opts.SchedulerEntryFrequencyMaxInterval())))
 			intervalMinutes = int(math.Max(float64(intervalMinutes), float64(config.Opts.SchedulerEntryFrequencyMinInterval())))
 		}
-		f.NextCheckAt = time.Now().Add(time.Minute * time.Duration(intervalMinutes))
 	default:
-		f.NextCheckAt = time.Now().Add(time.Minute * time.Duration(config.Opts.PollingFrequency()))
+		intervalMinutes = config.Opts.PollingFrequency()
 	}
+	// If the feed has a TTL defined, we use it to make sure we don't check it too often.
+	if newTTL > intervalMinutes && newTTL > 0 {
+		intervalMinutes = newTTL
+	}
+	f.NextCheckAt = time.Now().Add(time.Minute * time.Duration(intervalMinutes))
 }
 
 // FeedCreationRequest represents the request to create a feed.

diff --git a/internal/model/feed_test.go b/internal/model/feed_test.go
@@ -72,7 +72,8 @@ func TestFeedScheduleNextCheckDefault(t *testing.T) {
 
 	feed := &Feed{}
 	weeklyCount := 10
-	feed.ScheduleNextCheck(weeklyCount)
+	newTTL := 0
+	feed.ScheduleNextCheck(weeklyCount, newTTL)
 
 	if feed.NextCheckAt.IsZero() {
 		t.Error(`The next_check_at must be set`)
@@ -99,7 +100,8 @@ func TestFeedScheduleNextCheckEntryCountBasedMaxInterval(t *testing.T) {
 	}
 	feed := &Feed{}
 	weeklyCount := maxInterval * 100
-	feed.ScheduleNextCheck(weeklyCount)
+	newTTL := 0
+	feed.ScheduleNextCheck(weeklyCount, newTTL)
 
 	if feed.NextCheckAt.IsZero() {
 		t.Error(`The next_check_at must be set`)
@@ -126,7 +128,8 @@ func TestFeedScheduleNextCheckEntryCountBasedMinInterval(t *testing.T) {
 	}
 	feed := &Feed{}
 	weeklyCount := minInterval / 2
-	feed.ScheduleNextCheck(weeklyCount)
+	newTTL := 0
+	feed.ScheduleNextCheck(weeklyCount, newTTL)
 
 	if feed.NextCheckAt.IsZero() {
 		t.Error(`The next_check_at must be set`)
@@ -151,7 +154,8 @@ func TestFeedScheduleNextCheckEntryFrequencyFactor(t *testing.T) {
 	}
 	feed := &Feed{}
 	weeklyCount := 7
-	feed.ScheduleNextCheck(weeklyCount)
+	newTTL := 0
+	feed.ScheduleNextCheck(weeklyCount, newTTL)
 
 	if feed.NextCheckAt.IsZero() {
 		t.Error(`The next_check_at must be set`)
@@ -161,3 +165,69 @@ func TestFeedScheduleNextCheckEntryFrequencyFactor(t *testing.T) {
 		t.Error(`The next_check_at should not be after the now + factor * count`)
 	}
 }
+
+func TestFeedScheduleNextCheckEntryFrequencySmallNewTTL(t *testing.T) {
+	// If the feed has a TTL defined, we use it to make sure we don't check it too often.
+	maxInterval := 500
+	minInterval := 100
+	os.Clearenv()
+	os.Setenv("POLLING_SCHEDULER", "entry_frequency")
+	os.Setenv("SCHEDULER_ENTRY_FREQUENCY_MAX_INTERVAL", fmt.Sprintf("%d", maxInterval))
+	os.Setenv("SCHEDULER_ENTRY_FREQUENCY_MIN_INTERVAL", fmt.Sprintf("%d", minInterval))
+
+	var err error
+	parser := config.NewParser()
+	config.Opts, err = parser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Parsing failure: %v`, err)
+	}
+	feed := &Feed{}
+	weeklyCount := minInterval / 2
+	// TTL is smaller than minInterval.
+	newTTL := minInterval / 2
+	feed.ScheduleNextCheck(weeklyCount, newTTL)
+
+	if feed.NextCheckAt.IsZero() {
+		t.Error(`The next_check_at must be set`)
+	}
+
+	if feed.NextCheckAt.Before(time.Now().Add(time.Minute * time.Duration(minInterval))) {
+		t.Error(`The next_check_at should not be before the now + min interval`)
+	}
+	if feed.NextCheckAt.Before(time.Now().Add(time.Minute * time.Duration(newTTL))) {
+		t.Error(`The next_check_at should not be before the now + TTL`)
+	}
+}
+
+func TestFeedScheduleNextCheckEntryFrequencyLargeNewTTL(t *testing.T) {
+	// If the feed has a TTL defined, we use it to make sure we don't check it too often.
+	maxInterval := 500
+	minInterval := 100
+	os.Clearenv()
+	os.Setenv("POLLING_SCHEDULER", "entry_frequency")
+	os.Setenv("SCHEDULER_ENTRY_FREQUENCY_MAX_INTERVAL", fmt.Sprintf("%d", maxInterval))
+	os.Setenv("SCHEDULER_ENTRY_FREQUENCY_MIN_INTERVAL", fmt.Sprintf("%d", minInterval))
+
+	var err error
+	parser := config.NewParser()
+	config.Opts, err = parser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Parsing failure: %v`, err)
+	}
+	feed := &Feed{}
+	// TTL is larger than minInterval.
+	weeklyCount := minInterval / 2
+	newTTL := minInterval * 2
+	feed.ScheduleNextCheck(weeklyCount, newTTL)
+
+	if feed.NextCheckAt.IsZero() {
+		t.Error(`The next_check_at must be set`)
+	}
+
+	if feed.NextCheckAt.Before(time.Now().Add(time.Minute * time.Duration(minInterval))) {
+		t.Error(`The next_check_at should not be before the now + min interval`)
+	}
+	if feed.NextCheckAt.Before(time.Now().Add(time.Minute * time.Duration(newTTL))) {
+		t.Error(`The next_check_at should not be before the now + TTL`)
+	}
+}
diff --git a/internal/reader/handler/handler.go b/internal/reader/handler/handler.go
@@ -7,7 +7,6 @@ import (
 	"bytes"
 	"errors"
 	"log/slog"
-	"time"
 
 	"miniflux.app/v2/internal/config"
 	"miniflux.app/v2/internal/integration"
@@ -217,6 +216,7 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
 	}
 
 	weeklyEntryCount := 0
+	newTTL := 0
 	if config.Opts.PollingScheduler() == model.SchedulerEntryFrequency {
 		var weeklyCountErr error
 		weeklyEntryCount, weeklyCountErr = store.WeeklyFeedEntryCount(userID, feedID)
@@ -226,7 +226,20 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
 	}
 
 	originalFeed.CheckedNow()
-	originalFeed.ScheduleNextCheck(weeklyEntryCount)
+	// We are calling ScheduleNextCheck at the end of this function, thus we have a single place to update the "next check at".
+	// However this is done after processing all entires. In case of processing entries takes some time, we do not want to add
+	// the feed again to the worker queue during the process. To sovle this problem, we are calling ScheduleNextCheck
+	// at the beginning of the function. This "next check at" value would be overwritten by the call inside the defer function.
+	originalFeed.ScheduleNextCheck(0, 0)
+	defer func() {
+		originalFeed.ScheduleNextCheck(weeklyEntryCount, newTTL)
+		slog.Debug("Updating next check date",
+			slog.Int64("user_id", userID),
+			slog.Int64("feed_id", feedID),
+			slog.Time("new_next_check_at", originalFeed.NextCheckAt),
+			slog.Int("ttl", newTTL),
+		)
+	}()
 
 	requestBuilder := fetcher.NewRequestBuilder()
 	requestBuilder.WithUsernameAndPassword(originalFeed.Username, originalFeed.Password)
@@ -280,26 +293,7 @@ func RefreshFeed(store *storage.Storage, userID, feedID int64, forceRefresh bool
 		}
 
 		// If the feed has a TTL defined, we use it to make sure we don't check it too often.
-		if updatedFeed.TTL > 0 {
-			minNextCheckAt := time.Now().Add(time.Minute * time.Duration(updatedFeed.TTL))
-			slog.Debug("Feed TTL",
-				slog.Int64("user_id", userID),
-				slog.Int64("feed_id", feedID),
-				slog.Int("ttl", updatedFeed.TTL),
-				slog.Time("next_check_at", originalFeed.NextCheckAt),
-			)
-
-			if originalFeed.NextCheckAt.IsZero() || originalFeed.NextCheckAt.Before(minNextCheckAt) {
-				slog.Debug("Updating next check date based on TTL",
-					slog.Int64("user_id", userID),
-					slog.Int64("feed_id", feedID),
-					slog.Int("ttl", updatedFeed.TTL),
-					slog.Time("new_next_check_at", minNextCheckAt),
-					slog.Time("old_next_check_at", originalFeed.NextCheckAt),
-				)
-				originalFeed.NextCheckAt = minNextCheckAt
-			}
-		}
+		newTTL = updatedFeed.TTL
 
 		originalFeed.Entries = updatedFeed.Entries
 		processor.ProcessFeedEntries(store, originalFeed, user, forceRefresh)