From d54022baa6c8a3d0d3c308a9b6b1a6a9dc6ac7b6 Mon Sep 17 00:00:00 2001
From: Stanislav Chzhen <s.chzhen@adguard.com>
Date: Tue, 28 Feb 2023 16:16:40 +0300
Subject: [PATCH] all: imp code; fix chlog

---
 CHANGELOG.md                  | 12 ++++--------
 internal/home/upgrade.go      |  2 +-
 internal/querylog/http.go     | 10 ++++++----
 internal/querylog/qlog.go     | 14 ++++++++++++++
 internal/querylog/querylog.go |  7 ++++---
 internal/stats/http.go        |  8 ++++----
 internal/stats/http_test.go   |  8 ++++----
 internal/stats/stats.go       | 21 ++++++++++++++++++---
 openapi/CHANGELOG.md          | 10 ++++++++++
 9 files changed, 65 insertions(+), 27 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 36d7ed90..d783d063 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -53,19 +53,15 @@ In this release, the schema version has changed from 16 to 17.
 - The `GET /control/stats_info` HTTP API; use the new `GET
   /control/stats/config` API instead.
 
-  **NOTE:** If `interval` was configured by editing configuration file or new
-  HTTP API call `PUT /control/stats/config/update` and it's not equal to
-  previous allowed enum values then it will be equal to `90` days for
-  compatibility reasons.
+  **NOTE:** If interval is custom then it will be equal to `90` days for
+  compatibility reasons.  See openapi/openapi.yaml and `openapi/CHANGELOG.md`.
 - The `POST /control/stats_config` HTTP API; use the new `PUT
   /control/stats/config/update` API instead.
 - The `GET /control/querylog_info` HTTP API; use the new `GET
   /control/querylog/config` API instead.
 
-  **NOTE:** If `interval` was configured by editing configuration file or new
-  HTTP API call `PUT /control/querylog/config/update` and it's not equal to
-  previous allowed enum values then it will be equal to `90` days for
-  compatibility reasons.
+  **NOTE:** If interval is custom then it will be equal to `90` days for
+  compatibility reasons.  See openapi/openapi.yaml and `openapi/CHANGELOG.md`.
 - The `POST /control/querylog_config` HTTP API; use the new `PUT
   /control/querylog/config/update` API instead.
 
diff --git a/internal/home/upgrade.go b/internal/home/upgrade.go
index b979e957..ed06ae91 100644
--- a/internal/home/upgrade.go
+++ b/internal/home/upgrade.go
@@ -932,7 +932,7 @@ func upgradeSchema16to17(diskConf yobj) (err error) {
 
 	const field = "interval"
 
-	// Set the initial value from home.initConfig function.
+	// Set the initial value from the global configuration structure.
 	statsIvl := 1
 	statsIvlVal, ok := stats[field]
 	if ok {
diff --git a/internal/querylog/http.go b/internal/querylog/http.go
index a7af1050..e78f54fd 100644
--- a/internal/querylog/http.go
+++ b/internal/querylog/http.go
@@ -164,7 +164,8 @@ func (l *queryLog) handleQueryLogConfig(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	ivl := time.Duration(float64(timeutil.Day) * newConf.Interval)
+	ivl := time.Duration(newConf.Interval) * timeutil.Day
+
 	hasIvl := !math.IsNaN(newConf.Interval)
 	if hasIvl && !checkInterval(ivl) {
 		aghhttp.Error(r, w, http.StatusBadRequest, "unsupported interval")
@@ -214,8 +215,9 @@ func (l *queryLog) handlePutQueryLogConfig(w http.ResponseWriter, r *http.Reques
 
 	ivl := time.Duration(float64(time.Millisecond) * newConf.Interval)
 
-	if ivl < time.Hour || ivl > timeutil.Day*365 {
-		aghhttp.Error(r, w, http.StatusUnprocessableEntity, "unsupported interval")
+	err = validateIvl(ivl)
+	if err != nil {
+		aghhttp.Error(r, w, http.StatusUnprocessableEntity, "unsupported interval: %s", err)
 
 		return
 	}
@@ -254,7 +256,7 @@ func (l *queryLog) handlePutQueryLogConfig(w http.ResponseWriter, r *http.Reques
 	if len(newConf.Ignored) > 0 {
 		set, serr := aghnet.NewDomainNameSet(newConf.Ignored)
 		if serr != nil {
-			aghhttp.Error(r, w, http.StatusUnprocessableEntity, "ignored: duplicate or empty host")
+			aghhttp.Error(r, w, http.StatusUnprocessableEntity, "ignored: %s", serr)
 
 			return
 		}
diff --git a/internal/querylog/qlog.go b/internal/querylog/qlog.go
index d2e64df6..c46200c5 100644
--- a/internal/querylog/qlog.go
+++ b/internal/querylog/qlog.go
@@ -132,6 +132,20 @@ func checkInterval(ivl time.Duration) (ok bool) {
 	return ivl == quarterDay || ivl == day || ivl == week || ivl == month || ivl == threeMonths
 }
 
+// validateIvl returns an error if ivl is less than an hour or more than a
+// year.
+func validateIvl(ivl time.Duration) (err error) {
+	if ivl < time.Hour {
+		return errors.Error("less than an hour")
+	}
+
+	if ivl > timeutil.Day*365 {
+		return errors.Error("more than a year")
+	}
+
+	return nil
+}
+
 func (l *queryLog) WriteDiskConfig(c *Config) {
 	*c = *l.conf
 }
diff --git a/internal/querylog/querylog.go b/internal/querylog/querylog.go
index f52b30f9..fb1fb9b0 100644
--- a/internal/querylog/querylog.go
+++ b/internal/querylog/querylog.go
@@ -1,6 +1,7 @@
 package querylog
 
 import (
+	"fmt"
 	"net"
 	"path/filepath"
 	"time"
@@ -10,7 +11,6 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/golibs/errors"
 	"github.com/AdguardTeam/golibs/stringutil"
-	"github.com/AdguardTeam/golibs/timeutil"
 	"github.com/miekg/dns"
 )
 
@@ -157,8 +157,9 @@ func newQueryLog(conf Config) (l *queryLog, err error) {
 	l.conf = &Config{}
 	*l.conf = conf
 
-	if conf.RotationIvl < time.Hour || conf.RotationIvl > timeutil.Day*365 {
-		return nil, errors.Error("unsupported interval")
+	err = validateIvl(conf.RotationIvl)
+	if err != nil {
+		return nil, fmt.Errorf("unsupported interval: %w", err)
 	}
 
 	return l, nil
diff --git a/internal/stats/http.go b/internal/stats/http.go
index 7e882380..db4ffa8e 100644
--- a/internal/stats/http.go
+++ b/internal/stats/http.go
@@ -11,7 +11,6 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/aghhttp"
 	"github.com/AdguardTeam/AdGuardHome/internal/aghnet"
 	"github.com/AdguardTeam/golibs/log"
-	"github.com/AdguardTeam/golibs/timeutil"
 	"golang.org/x/exp/slices"
 )
 
@@ -163,8 +162,9 @@ func (s *StatsCtx) handlePutStatsConfig(w http.ResponseWriter, r *http.Request)
 
 	ivl := time.Duration(float64(time.Millisecond) * reqData.Interval)
 
-	if ivl < time.Hour || ivl > timeutil.Day*365 {
-		aghhttp.Error(r, w, http.StatusUnprocessableEntity, "unsupported interval")
+	err = validateIvl(ivl)
+	if err != nil {
+		aghhttp.Error(r, w, http.StatusUnprocessableEntity, "unsupported interval: %s", err)
 
 		return
 	}
@@ -187,7 +187,7 @@ func (s *StatsCtx) handlePutStatsConfig(w http.ResponseWriter, r *http.Request)
 	if len(reqData.Ignored) > 0 {
 		set, serr := aghnet.NewDomainNameSet(reqData.Ignored)
 		if serr != nil {
-			aghhttp.Error(r, w, http.StatusUnprocessableEntity, "ignored: duplicate or empty host")
+			aghhttp.Error(r, w, http.StatusUnprocessableEntity, "ignored: %s", serr)
 
 			return
 		}
diff --git a/internal/stats/http_test.go b/internal/stats/http_test.go
index df8e11a9..916168dc 100644
--- a/internal/stats/http_test.go
+++ b/internal/stats/http_test.go
@@ -53,7 +53,7 @@ func TestHandleStatsConfig(t *testing.T) {
 			Ignored:  []string{},
 		},
 		wantCode: http.StatusUnprocessableEntity,
-		wantErr:  "unsupported interval\n",
+		wantErr:  "unsupported interval: less than an hour\n",
 	}, {
 		name: "big_interval",
 		body: getConfigResp{
@@ -62,7 +62,7 @@ func TestHandleStatsConfig(t *testing.T) {
 			Ignored:  []string{},
 		},
 		wantCode: http.StatusUnprocessableEntity,
-		wantErr:  "unsupported interval\n",
+		wantErr:  "unsupported interval: more than a year\n",
 	}, {
 		name: "set_ignored_ivl_1_maxIvl",
 		body: getConfigResp{
@@ -85,7 +85,7 @@ func TestHandleStatsConfig(t *testing.T) {
 			},
 		},
 		wantCode: http.StatusUnprocessableEntity,
-		wantErr:  "ignored: duplicate or empty host\n",
+		wantErr:  "ignored: duplicate host name \"ignor.ed\" at index 1\n",
 	}, {
 		name: "ignored_empty",
 		body: getConfigResp{
@@ -96,7 +96,7 @@ func TestHandleStatsConfig(t *testing.T) {
 			},
 		},
 		wantCode: http.StatusUnprocessableEntity,
-		wantErr:  "ignored: duplicate or empty host\n",
+		wantErr:  "ignored: host name is empty\n",
 	}, {
 		name: "enabled_is_null",
 		body: getConfigResp{
diff --git a/internal/stats/stats.go b/internal/stats/stats.go
index 82a14374..dd192dcb 100644
--- a/internal/stats/stats.go
+++ b/internal/stats/stats.go
@@ -26,6 +26,20 @@ func checkInterval(days uint32) (ok bool) {
 	return days == 0 || days == 1 || days == 7 || days == 30 || days == 90
 }
 
+// validateIvl returns an error if ivl is less than an hour or more than a
+// year.
+func validateIvl(ivl time.Duration) (err error) {
+	if ivl < time.Hour {
+		return errors.Error("less than an hour")
+	}
+
+	if ivl > timeutil.Day*365 {
+		return errors.Error("more than a year")
+	}
+
+	return nil
+}
+
 // Config is the configuration structure for the statistics collecting.
 type Config struct {
 	// UnitID is the function to generate the identifier for current unit.  If
@@ -127,8 +141,9 @@ func New(conf Config) (s *StatsCtx, err error) {
 		ignored:        conf.Ignored,
 	}
 
-	if conf.Limit < time.Hour || conf.Limit > timeutil.Day*365 {
-		return nil, errors.Error("unsupported interval")
+	err = validateIvl(conf.Limit)
+	if err != nil {
+		return nil, fmt.Errorf("unsupported interval: %w", err)
 	}
 
 	s.limit = conf.Limit
@@ -445,7 +460,7 @@ func (s *StatsCtx) periodicFlush() {
 func (s *StatsCtx) setLimitLocked(limitDays int) {
 	if limitDays != 0 {
 		s.enabled = true
-		s.limit = time.Duration(int(timeutil.Day) * limitDays)
+		s.limit = time.Duration(limitDays) * timeutil.Day
 		log.Debug("stats: set limit: %d days", limitDays)
 
 		return
diff --git a/openapi/CHANGELOG.md b/openapi/CHANGELOG.md
index f32771de..442718f7 100644
--- a/openapi/CHANGELOG.md
+++ b/openapi/CHANGELOG.md
@@ -13,6 +13,11 @@
 * The `GET /control/stats_info` HTTP API; use the new `GET
   /control/stats/config` API instead.
 
+  **NOTE:** If `interval` was configured by editing configuration file or new
+  HTTP API call `PUT /control/stats/config/update` and it's not equal to
+  previous allowed enum values then it will be equal to `90` days for
+  compatibility reasons.
+
 * The `POST /control/stats_config` HTTP API; use the new `PUT
   /control/stats/config/update` API instead.
 
@@ -38,6 +43,11 @@ return a JSON object with the following format:
 * The `GET /control/querylog_info` HTTP API; use the new `GET
   /control/querylog/config` API instead.
 
+  **NOTE:** If `interval` was configured by editing configuration file or new
+  HTTP API call `PUT /control/querylog/config/update` and it's not equal to
+  previous allowed enum values then it will be equal to `90` days for
+  compatibility reasons.
+
 * The `POST /control/querylog_config` HTTP API; use the new `PUT
   /control/querylog/config/update` API instead.