Rename cleanup config variables

The config parser logs a warning when the user uses a deprecated variable. It also ignores the value from a deprecated variable if it has already been set using the corresponding non-deprecated variable (and logs another warning).

- CLEANUP_FREQUENCY_HOURS instead of CLEANUP_FREQUENCY
- CLEANUP_ARCHIVE_READ_DAYS instead of ARCHIVE_READ_DAYS

3 files changed, 193 insertions, 119 deletions

diff --git a/config/config_test.go b/config/config_test.go
index 353b87c..c1c9e2a 100644
--- a/config/config_test.go
+++ b/config/config_test.go
@@ -444,7 +444,7 @@ func TestDefaultCertCacheValue(t *testing.T) {
 	}
 }
 
-func TestDefaultCleanupFrequencyValue(t *testing.T) {
+func TestDefaultCleanupFrequencyHoursValue(t *testing.T) {
 	os.Clearenv()
 
 	parser := NewParser()
@@ -453,15 +453,34 @@ func TestDefaultCleanupFrequencyValue(t *testing.T) {
 		t.Fatalf(`Parsing failure: %v`, err)
 	}
 
-	expected := defaultCleanupFrequency
-	result := opts.CleanupFrequency()
+	expected := defaultCleanupFrequencyHours
+	result := opts.CleanupFrequencyHours()
 
 	if result != expected {
-		t.Fatalf(`Unexpected CLEANUP_FREQUENCY value, got %v instead of %v`, result, expected)
+		t.Fatalf(`Unexpected CLEANUP_FREQUENCY_HOURS value, got %v instead of %v`, result, expected)
 	}
 }
 
-func TestCleanupFrequency(t *testing.T) {
+func TestCleanupFrequencyHours(t *testing.T) {
+	os.Clearenv()
+	os.Setenv("CLEANUP_FREQUENCY_HOURS", "42")
+	os.Setenv("CLEANUP_FREQUENCY", "19")
+
+	parser := NewParser()
+	opts, err := parser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Parsing failure: %v`, err)
+	}
+
+	expected := 42
+	result := opts.CleanupFrequencyHours()
+
+	if result != expected {
+		t.Fatalf(`Unexpected CLEANUP_FREQUENCY_HOURS value, got %v instead of %v`, result, expected)
+	}
+}
+
+func TestDeprecatedCleanupFrequencyHoursVar(t *testing.T) {
 	os.Clearenv()
 	os.Setenv("CLEANUP_FREQUENCY", "42")
 
@@ -472,13 +491,102 @@ func TestCleanupFrequency(t *testing.T) {
 	}
 
 	expected := 42
-	result := opts.CleanupFrequency()
+	result := opts.CleanupFrequencyHours()
 
 	if result != expected {
 		t.Fatalf(`Unexpected CLEANUP_FREQUENCY value, got %v instead of %v`, result, expected)
 	}
 }
 
+func TestDefaultCleanupArchiveReadDaysValue(t *testing.T) {
+	os.Clearenv()
+
+	parser := NewParser()
+	opts, err := parser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Parsing failure: %v`, err)
+	}
+
+	expected := 60
+	result := opts.CleanupArchiveReadDays()
+
+	if result != expected {
+		t.Fatalf(`Unexpected CLEANUP_ARCHIVE_READ_DAYS value, got %v instead of %v`, result, expected)
+	}
+}
+
+func TestCleanupArchiveReadDays(t *testing.T) {
+	os.Clearenv()
+	os.Setenv("CLEANUP_ARCHIVE_READ_DAYS", "7")
+	os.Setenv("ARCHIVE_READ_DAYS", "19")
+
+	parser := NewParser()
+	opts, err := parser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Parsing failure: %v`, err)
+	}
+
+	expected := 7
+	result := opts.CleanupArchiveReadDays()
+
+	if result != expected {
+		t.Fatalf(`Unexpected CLEANUP_ARCHIVE_READ_DAYS value, got %v instead of %v`, result, expected)
+	}
+}
+
+func TestDeprecatedCleanupArchiveReadDaysVar(t *testing.T) {
+	os.Clearenv()
+	os.Setenv("ARCHIVE_READ_DAYS", "7")
+
+	parser := NewParser()
+	opts, err := parser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Parsing failure: %v`, err)
+	}
+
+	expected := 7
+	result := opts.CleanupArchiveReadDays()
+
+	if result != expected {
+		t.Fatalf(`Unexpected ARCHIVE_READ_DAYS value, got %v instead of %v`, result, expected)
+	}
+}
+
+func TestDefaultCleanupRemoveSessionsDaysValue(t *testing.T) {
+	os.Clearenv()
+
+	parser := NewParser()
+	opts, err := parser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Parsing failure: %v`, err)
+	}
+
+	expected := 30
+	result := opts.CleanupRemoveSessionsDays()
+
+	if result != expected {
+		t.Fatalf(`Unexpected CLEANUP_REMOVE_SESSIONS_DAYS value, got %v instead of %v`, result, expected)
+	}
+}
+
+func TestCleanupRemoveSessionsDays(t *testing.T) {
+	os.Clearenv()
+	os.Setenv("CLEANUP_REMOVE_SESSIONS_DAYS", "7")
+
+	parser := NewParser()
+	opts, err := parser.ParseEnvironmentVariables()
+	if err != nil {
+		t.Fatalf(`Parsing failure: %v`, err)
+	}
+
+	expected := 7
+	result := opts.CleanupRemoveSessionsDays()
+
+	if result != expected {
+		t.Fatalf(`Unexpected CLEANUP_REMOVE_SESSIONS_DAYS value, got %v instead of %v`, result, expected)
+	}
+}
+
 func TestDefaultWorkerPoolSizeValue(t *testing.T) {
 	os.Clearenv()
 
@@ -864,59 +972,6 @@ func TestDisableSchedulerService(t *testing.T) {
 	}
 }
 
-func TestArchiveReadDays(t *testing.T) {
-	os.Clearenv()
-	os.Setenv("ARCHIVE_READ_DAYS", "7")
-
-	parser := NewParser()
-	opts, err := parser.ParseEnvironmentVariables()
-	if err != nil {
-		t.Fatalf(`Parsing failure: %v`, err)
-	}
-
-	expected := 7
-	result := opts.ArchiveReadDays()
-
-	if result != expected {
-		t.Fatalf(`Unexpected ARCHIVE_READ_DAYS value, got %v instead of %v`, result, expected)
-	}
-}
-
-func TestRemoveSessionsDays(t *testing.T) {
-	os.Clearenv()
-	os.Setenv("REMOVE_SESSIONS_DAYS", "7")
-
-	parser := NewParser()
-	opts, err := parser.ParseEnvironmentVariables()
-	if err != nil {
-		t.Fatalf(`Parsing failure: %v`, err)
-	}
-
-	expected := 7
-	result := opts.RemoveSessionsDays()
-
-	if result != expected {
-		t.Fatalf(`Unexpected REMOVE_SESSIONS_DAYS value, got %v instead of %v`, result, expected)
-	}
-}
-
-func TestDefaultRemoveSessionsDays(t *testing.T) {
-	os.Clearenv()
-
-	parser := NewParser()
-	opts, err := parser.ParseEnvironmentVariables()
-	if err != nil {
-		t.Fatalf(`Parsing failure: %v`, err)
-	}
-
-	expected := 30
-	result := opts.RemoveSessionsDays()
-
-	if result != expected {
-		t.Fatalf(`Unexpected REMOVE_SESSIONS_DAYS value, got %v instead of %v`, result, expected)
-	}
-}
-
 func TestRunMigrationsWhenUnset(t *testing.T) {
 	os.Clearenv()
 
diff --git a/config/options.go b/config/options.go
index a5c75b8..27387a2 100644
--- a/config/options.go
+++ b/config/options.go
@@ -10,40 +10,40 @@ import (
 )
 
 const (
-	defaultHTTPS                 = false
-	defaultLogDateTime           = false
-	defaultHSTS                  = true
-	defaultHTTPService           = true
-	defaultSchedulerService      = true
-	defaultDebug                 = false
-	defaultBaseURL               = "http://localhost"
-	defaultRootURL               = "http://localhost"
-	defaultBasePath              = ""
-	defaultWorkerPoolSize        = 5
-	defaultPollingFrequency      = 60
-	defaultBatchSize             = 10
-	defaultRunMigrations         = false
-	defaultDatabaseURL           = "user=postgres password=postgres dbname=miniflux2 sslmode=disable"
-	defaultDatabaseMaxConns      = 20
-	defaultDatabaseMinConns      = 1
-	defaultArchiveReadDays       = 60
-	defaultRemoveSessionsDays    = 30
-	defaultListenAddr            = "127.0.0.1:8080"
-	defaultCertFile              = ""
-	defaultKeyFile               = ""
-	defaultCertDomain            = ""
-	defaultCertCache             = "/tmp/cert_cache"
-	defaultCleanupFrequency      = 24
-	defaultProxyImages           = "http-only"
-	defaultCreateAdmin           = false
-	defaultOAuth2UserCreation    = false
-	defaultOAuth2ClientID        = ""
-	defaultOAuth2ClientSecret    = ""
-	defaultOAuth2RedirectURL     = ""
-	defaultOAuth2Provider        = ""
-	defaultPocketConsumerKey     = ""
-	defaultHTTPClientTimeout     = 20
-	defaultHTTPClientMaxBodySize = 15
+	defaultHTTPS                     = false
+	defaultLogDateTime               = false
+	defaultHSTS                      = true
+	defaultHTTPService               = true
+	defaultSchedulerService          = true
+	defaultDebug                     = false
+	defaultBaseURL                   = "http://localhost"
+	defaultRootURL                   = "http://localhost"
+	defaultBasePath                  = ""
+	defaultWorkerPoolSize            = 5
+	defaultPollingFrequency          = 60
+	defaultBatchSize                 = 10
+	defaultRunMigrations             = false
+	defaultDatabaseURL               = "user=postgres password=postgres dbname=miniflux2 sslmode=disable"
+	defaultDatabaseMaxConns          = 20
+	defaultDatabaseMinConns          = 1
+	defaultListenAddr                = "127.0.0.1:8080"
+	defaultCertFile                  = ""
+	defaultKeyFile                   = ""
+	defaultCertDomain                = ""
+	defaultCertCache                 = "/tmp/cert_cache"
+	defaultCleanupFrequencyHours     = 24
+	defaultCleanupArchiveReadDays    = 60
+	defaultCleanupRemoveSessionsDays = 30
+	defaultProxyImages               = "http-only"
+	defaultCreateAdmin               = false
+	defaultOAuth2UserCreation        = false
+	defaultOAuth2ClientID            = ""
+	defaultOAuth2ClientSecret        = ""
+	defaultOAuth2RedirectURL         = ""
+	defaultOAuth2Provider            = ""
+	defaultPocketConsumerKey         = ""
+	defaultHTTPClientTimeout         = 20
+	defaultHTTPClientMaxBodySize     = 15
 )
 
 // Options contains configuration options.
@@ -66,9 +66,9 @@ type Options struct {
 	certDomain                string
 	certCache                 string
 	certKeyFile               string
-	cleanupFrequency          int
-	archiveReadDays           int
-	removeSessionsDays        int
+	cleanupFrequencyHours     int
+	cleanupArchiveReadDays    int
+	cleanupRemoveSessionsDays int
 	pollingFrequency          int
 	batchSize                 int
 	workerPoolSize            int
@@ -105,9 +105,9 @@ func NewOptions() *Options {
 		certDomain:                defaultCertDomain,
 		certCache:                 defaultCertCache,
 		certKeyFile:               defaultKeyFile,
-		cleanupFrequency:          defaultCleanupFrequency,
-		archiveReadDays:           defaultArchiveReadDays,
-		removeSessionsDays:        defaultRemoveSessionsDays,
+		cleanupFrequencyHours:     defaultCleanupFrequencyHours,
+		cleanupArchiveReadDays:    defaultCleanupArchiveReadDays,
+		cleanupRemoveSessionsDays: defaultCleanupRemoveSessionsDays,
 		pollingFrequency:          defaultPollingFrequency,
 		batchSize:                 defaultBatchSize,
 		workerPoolSize:            defaultWorkerPoolSize,
@@ -194,9 +194,19 @@ func (o *Options) CertCache() string {
 	return o.certCache
 }
 
-// CleanupFrequency returns the interval for cleanup jobs.
-func (o *Options) CleanupFrequency() int {
-	return o.cleanupFrequency
+// CleanupFrequencyHours returns the interval in hours for cleanup jobs.
+func (o *Options) CleanupFrequencyHours() int {
+	return o.cleanupFrequencyHours
+}
+
+// CleanupArchiveReadDays returns the number of days after which marking read items as removed.
+func (o *Options) CleanupArchiveReadDays() int {
+	return o.cleanupArchiveReadDays
+}
+
+// CleanupRemoveSessionsDays returns the number of days after which to remove sessions.
+func (o *Options) CleanupRemoveSessionsDays() int {
+	return o.cleanupRemoveSessionsDays
 }
 
 // WorkerPoolSize returns the number of background worker.
@@ -269,16 +279,6 @@ func (o *Options) HasSchedulerService() bool {
 	return o.schedulerService
 }
 
-// ArchiveReadDays returns the number of days after which marking read items as removed.
-func (o *Options) ArchiveReadDays() int {
-	return o.archiveReadDays
-}
-
-// RemoveSessionsDays returns the number of days after which to remove sessions.
-func (o *Options) RemoveSessionsDays() int {
-	return o.removeSessionsDays
-}
-
 // PocketConsumerKey returns the Pocket Consumer Key if configured.
 func (o *Options) PocketConsumerKey(defaultValue string) string {
 	if o.pocketConsumerKey != "" {
@@ -317,11 +317,12 @@ func (o *Options) String() string {
 	builder.WriteString(fmt.Sprintf("KEY_FILE: %v\n", o.certKeyFile))
 	builder.WriteString(fmt.Sprintf("CERT_DOMAIN: %v\n", o.certDomain))
 	builder.WriteString(fmt.Sprintf("CERT_CACHE: %v\n", o.certCache))
-	builder.WriteString(fmt.Sprintf("CLEANUP_FREQUENCY: %v\n", o.cleanupFrequency))
+	builder.WriteString(fmt.Sprintf("CLEANUP_FREQUENCY_HOURS: %v\n", o.cleanupFrequencyHours))
+	builder.WriteString(fmt.Sprintf("CLEANUP_ARCHIVE_READ_DAYS: %v\n", o.cleanupArchiveReadDays))
+	builder.WriteString(fmt.Sprintf("CLEANUP_REMOVE_SESSIONS_DAYS: %v\n", o.cleanupRemoveSessionsDays))
 	builder.WriteString(fmt.Sprintf("WORKER_POOL_SIZE: %v\n", o.workerPoolSize))
 	builder.WriteString(fmt.Sprintf("POLLING_FREQUENCY: %v\n", o.pollingFrequency))
 	builder.WriteString(fmt.Sprintf("BATCH_SIZE: %v\n", o.batchSize))
-	builder.WriteString(fmt.Sprintf("ARCHIVE_READ_DAYS: %v\n", o.archiveReadDays))
 	builder.WriteString(fmt.Sprintf("PROXY_IMAGES: %v\n", o.proxyImages))
 	builder.WriteString(fmt.Sprintf("CREATE_ADMIN: %v\n", o.createAdmin))
 	builder.WriteString(fmt.Sprintf("POCKET_CONSUMER_KEY: %v\n", o.pocketConsumerKey))
diff --git a/config/parser.go b/config/parser.go
index e151c59..2e8f92d 100644
--- a/config/parser.go
+++ b/config/parser.go
@@ -13,6 +13,8 @@ import (
 	"os"
 	"strconv"
 	"strings"
+
+	"miniflux.app/logger"
 )
 
 // Parser handles configuration parsing.
@@ -108,18 +110,34 @@ func (p *Parser) parseLines(lines []string) (err error) {
 			p.opts.certDomain = parseString(value, defaultCertDomain)
 		case "CERT_CACHE":
 			p.opts.certCache = parseString(value, defaultCertCache)
+		case "CLEANUP_FREQUENCY_HOURS":
+			p.opts.cleanupFrequencyHours = parseInt(value, defaultCleanupFrequencyHours)
+		case "CLEANUP_ARCHIVE_READ_DAYS":
+			p.opts.cleanupArchiveReadDays = parseInt(value, defaultCleanupArchiveReadDays)
+		case "CLEANUP_REMOVE_SESSIONS_DAYS":
+			p.opts.cleanupRemoveSessionsDays = parseInt(value, defaultCleanupRemoveSessionsDays)
 		case "CLEANUP_FREQUENCY":
-			p.opts.cleanupFrequency = parseInt(value, defaultCleanupFrequency)
+			logger.Error("[Config] CLEANUP_FREQUENCY has been deprecated in favor of CLEANUP_FREQUENCY_HOURS.")
+
+			if p.opts.cleanupFrequencyHours != defaultCleanupFrequencyHours {
+				logger.Error("[Config] Ignoring CLEANUP_FREQUENCY as CLEANUP_FREQUENCY_HOURS is already specified.")
+			} else {
+				p.opts.cleanupFrequencyHours = parseInt(value, defaultCleanupFrequencyHours)
+			}
+		case "ARCHIVE_READ_DAYS":
+			logger.Error("[Config] ARCHIVE_READ_DAYS has been deprecated in favor of CLEANUP_ARCHIVE_READ_DAYS.")
+
+			if p.opts.cleanupArchiveReadDays != defaultCleanupArchiveReadDays {
+				logger.Error("[Config] Ignoring ARCHIVE_READ_DAYS as CLEANUP_ARCHIVE_READ_DAYS is already specified.")
+			} else {
+				p.opts.cleanupArchiveReadDays = parseInt(value, defaultCleanupArchiveReadDays)
+			}
 		case "WORKER_POOL_SIZE":
 			p.opts.workerPoolSize = parseInt(value, defaultWorkerPoolSize)
 		case "POLLING_FREQUENCY":
 			p.opts.pollingFrequency = parseInt(value, defaultPollingFrequency)
 		case "BATCH_SIZE":
 			p.opts.batchSize = parseInt(value, defaultBatchSize)
-		case "ARCHIVE_READ_DAYS":
-			p.opts.archiveReadDays = parseInt(value, defaultArchiveReadDays)
-		case "REMOVE_SESSIONS_DAYS":
-			p.opts.removeSessionsDays = parseInt(value, defaultRemoveSessionsDays)
 		case "PROXY_IMAGES":
 			p.opts.proxyImages = parseString(value, defaultProxyImages)
 		case "CREATE_ADMIN":