all: imp code; fix notes

CHANGELOG.md

@@ -62,14 +62,18 @@ 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 is custom then it will be equal to `90` days for
+  **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.
 - The `GET /control/querylog_info` HTTP API; use the new `GET
   /control/querylog/config` API instead.
 
-  **NOTE:** If interval is custom then it will be equal to `90` days for
+  **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.

internal/aghnet/hostgen.go

@@ -71,16 +71,16 @@ func GenerateHostname(ip net.IP) (hostname string) {
 func NewDomainNameSet(list []string) (set *stringutil.Set, err error) {
 	set = stringutil.NewSet()
 
-	for _, v := range list {
+	for i, v := range list {
 		host := strings.ToLower(strings.TrimSuffix(v, "."))
-		// TODO(a.garipov): Think about ignoring empty (".") names in
+		// TODO(a.garipov): Think about ignoring empty (".") names in the
-		// the future.
+		// future.
 		if host == "" {
 			return nil, errors.Error("host name is empty")
 		}
 
 		if set.Has(host) {
-			return nil, fmt.Errorf("duplicate host name %q", host)
+			return nil, fmt.Errorf("duplicate host name %q at index %d", host, i)
 		}
 
 		set.Add(host)

internal/home/config.go

@@ -218,33 +218,32 @@ type tlsConfigSettings struct {
 }
 
 type queryLogConfig struct {
+	// Ignored is the list of host names, which should not be written to log.
+	Ignored []string `yaml:"ignored"`
+
+	// Interval is the interval for query log's files rotation.
+	Interval timeutil.Duration `yaml:"interval"`
+
+	// MemSize is the number of entries kept in memory before they are flushed
+	// to disk.
+	MemSize uint32 `yaml:"size_memory"`
+
 	// Enabled defines if the query log is enabled.
 	Enabled bool `yaml:"enabled"`
 
 	// FileEnabled defines, if the query log is written to the file.
 	FileEnabled bool `yaml:"file_enabled"`
-
-	// Interval is the interval for query log's files rotation.
-	Interval timeutil.Duration `yaml:"interval"`
-
-	// MemSize is the number of entries kept in memory before they are
-	// flushed to disk.
-	MemSize uint32 `yaml:"size_memory"`
-
-	// Ignored is the list of host names, which should not be written to
-	// log.
-	Ignored []string `yaml:"ignored"`
 }
 
 type statsConfig struct {
-	// Enabled defines if the statistics are enabled.
-	Enabled bool `yaml:"enabled"`
-
-	// Interval is the time interval for flushing statistics to the disk.
-	Interval timeutil.Duration `yaml:"interval"`
-
 	// Ignored is the list of host names, which should not be counted.
 	Ignored []string `yaml:"ignored"`
+
+	// Interval is the retention interval for statistics.
+	Interval timeutil.Duration `yaml:"interval"`
+
+	// Enabled defines if the statistics are enabled.
+	Enabled bool `yaml:"enabled"`
 }
 
 // config is the global configuration structure.

internal/querylog/http.go

@@ -7,6 +7,7 @@ import (
 	"net"
 	"net/http"
 	"net/url"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@@ -26,8 +27,8 @@ type configJSON struct {
 	// fractional numbers and not mess the API users by changing the units.
 	Interval float64 `json:"interval"`
 
-	// Enabled shows if the querylog is enabled.  It is an [aghalg.NullBool]
+	// Enabled shows if the querylog is enabled.  It is an aghalg.NullBool to
-	// to be able to tell when it's set without using pointers.
+	// be able to tell when it's set without using pointers.
 	Enabled aghalg.NullBool `json:"enabled"`
 
 	// AnonymizeClientIP shows if the clients' IP addresses must be anonymized.
@@ -38,24 +39,22 @@ type configJSON struct {
 
 // getConfigResp is the JSON structure for the querylog configuration.
 type getConfigResp struct {
-	// Enabled shows if the querylog is enabled.  It is an
+	// Ignored is the list of host names, which should not be written to log.
-	// [aghalg.NullBool] to be able to tell when it's set without using
+	Ignored []string `json:"ignored"`
-	// pointers.
-	Enabled aghalg.NullBool `json:"enabled"`
-
-	// AnonymizeClientIP shows if the clients' IP addresses must be
-	// anonymized.  It is an [aghalg.NullBool] to be able to tell when it's
-	// set without using pointers.
-	//
-	// TODO(a.garipov): Consider using separate setting for statistics.
-	AnonymizeClientIP aghalg.NullBool `json:"anonymize_client_ip"`
 
 	// Interval is the querylog rotation interval in milliseconds
 	Interval float64 `json:"interval"`
 
-	// Ignored is the list of host names, which should not be written to
+	// Enabled shows if the querylog is enabled.  It is an aghalg.NullBool to
-	// log.
+	// be able to tell when it's set without using pointers.
-	Ignored []string `json:"ignored"`
+	Enabled aghalg.NullBool `json:"enabled"`
+
+	// AnonymizeClientIP shows if the clients' IP addresses must be anonymized.
+	// It is an aghalg.NullBool to be able to tell when it's set without using
+	// pointers.
+	//
+	// TODO(a.garipov): Consider using separate setting for statistics.
+	AnonymizeClientIP aghalg.NullBool `json:"anonymize_client_ip"`
 }
 
 // Register web handlers
@@ -109,8 +108,8 @@ func (l *queryLog) handleQueryLogInfo(w http.ResponseWriter, r *http.Request) {
 
 	ok := checkInterval(ivl)
 	if !ok {
-		// NOTE: If interval is custom we set it to 90 days for
+		// NOTE: If interval is custom we set it to 90 days for compatibility
-		// compatibility with old API.
+		// with old API.
 		ivl = timeutil.Day * 90
 	}
 
@@ -128,6 +127,7 @@ func (l *queryLog) handleGetQueryLogConfig(w http.ResponseWriter, r *http.Reques
 	defer l.lock.Unlock()
 
 	ignored := l.conf.Ignored.Values()
+	sort.Strings(ignored)
 	_ = aghhttp.WriteJSONResponse(w, r, getConfigResp{
 		Enabled:           aghalg.BoolToNullBool(l.conf.Enabled),
 		Interval:          float64(l.conf.RotationIvl.Milliseconds()),
@@ -257,7 +257,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: %s", serr)
+			aghhttp.Error(r, w, http.StatusUnprocessableEntity, "ignored: duplicate or empty host")
 
 			return
 		}

internal/querylog/querylog.go

@@ -157,11 +157,8 @@ func newQueryLog(conf Config) (l *queryLog, err error) {
 	l.conf = &Config{}
 	*l.conf = conf
 
-	if conf.RotationIvl < time.Hour {
+	if conf.RotationIvl < time.Hour || conf.RotationIvl > timeutil.Day*365 {
-		return nil, errors.Error("interval: less than an hour")
+		return nil, errors.Error("unsupported interval")
-	}
-	if conf.RotationIvl > timeutil.Day*365 {
-		return nil, errors.Error("interval: more than a year")
 	}
 
 	return l, nil

internal/stats/http.go

@@ -5,6 +5,7 @@ package stats
 import (
 	"encoding/json"
 	"net/http"
+	"sort"
 	"time"
 
 	"github.com/AdguardTeam/AdGuardHome/internal/aghalg"
@@ -68,15 +69,15 @@ type configResp struct {
 
 // getConfigResp is the response to the GET /control/stats_info.
 type getConfigResp struct {
-	// Enabled shows if statistics are enabled.  It is an [aghalg.NullBool]
+	// Ignored is the list of host names, which should not be counted.
-	// to be able to tell when it's set without using pointers.
+	Ignored []string `json:"ignored"`
-	Enabled aghalg.NullBool `json:"enabled"`
 
 	// Interval is the statistics rotation interval in milliseconds.
 	Interval float64 `json:"interval"`
 
-	// Ignored is the list of host names, which should not be counted.
+	// Enabled shows if statistics are enabled.  It is an aghalg.NullBool to be
-	Ignored []string `json:"ignored"`
+	// able to tell when it's set without using pointers.
+	Enabled aghalg.NullBool `json:"enabled"`
 }
 
 // handleStatsInfo handles requests to the GET /control/stats_info endpoint.
@@ -90,8 +91,8 @@ func (s *StatsCtx) handleStatsInfo(w http.ResponseWriter, r *http.Request) {
 
 	ok := checkInterval(days)
 	if !ok || (s.enabled && days == 0) {
-		// NOTE: If interval is custom we set it to 90 days for
+		// NOTE: If interval is custom we set it to 90 days for compatibility
-		// compatibility with old API.
+		// with old API.
 		days = 90
 	}
 
@@ -102,15 +103,19 @@ func (s *StatsCtx) handleStatsInfo(w http.ResponseWriter, r *http.Request) {
 	_ = aghhttp.WriteJSONResponse(w, r, resp)
 }
 
-// handleGetStatsConfig handles requests to the GET /control/stats_info endpoint.
+// handleGetStatsConfig handles requests to the GET /control/stats_info
+// endpoint.
 func (s *StatsCtx) handleGetStatsConfig(w http.ResponseWriter, r *http.Request) {
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
+	ignored := s.ignored.Values()
+	sort.Strings(ignored)
+
 	resp := getConfigResp{
 		Enabled:  aghalg.BoolToNullBool(s.enabled),
 		Interval: float64(s.limit.Milliseconds()),
-		Ignored:  s.ignored.Values(),
+		Ignored:  ignored,
 	}
 	err := aghhttp.WriteJSONResponse(w, r, resp)
 	if err != nil {
@@ -182,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: %s", serr)
+			aghhttp.Error(r, w, http.StatusUnprocessableEntity, "ignored: duplicate or empty host")
 
 			return
 		}

internal/stats/http_test.go

@@ -17,22 +17,30 @@ import (
 )
 
 func TestHandleStatsConfig(t *testing.T) {
-	var (
+	const (
-		halfHour = time.Hour / 2
+		smallIvl = 1 * time.Minute
-		hour     = time.Hour
+		minIvl   = 1 * time.Hour
-		year     = timeutil.Day * 365
+		maxIvl   = 365 * timeutil.Day
 	)
 
+	conf := Config{
+		Filename:       filepath.Join(t.TempDir(), "stats.db"),
+		Limit:          time.Hour * 24,
+		Enabled:        true,
+		UnitID:         func() (id uint32) { return 0 },
+		ConfigModified: func() {},
+	}
+
 	testCases := []struct {
 		name     string
 		body     getConfigResp
 		wantCode int
 		wantErr  string
 	}{{
-		name: "set_ivl_1_hour",
+		name: "set_ivl_1_minIvl",
 		body: getConfigResp{
 			Enabled:  aghalg.NBTrue,
-			Interval: float64(hour.Milliseconds()),
+			Interval: float64(minIvl.Milliseconds()),
 			Ignored:  nil,
 		},
 		wantCode: http.StatusOK,
@@ -41,7 +49,7 @@ func TestHandleStatsConfig(t *testing.T) {
 		name: "small_interval",
 		body: getConfigResp{
 			Enabled:  aghalg.NBTrue,
-			Interval: float64(halfHour.Milliseconds()),
+			Interval: float64(smallIvl.Milliseconds()),
 			Ignored:  []string{},
 		},
 		wantCode: http.StatusUnprocessableEntity,
@@ -50,16 +58,16 @@ func TestHandleStatsConfig(t *testing.T) {
 		name: "big_interval",
 		body: getConfigResp{
 			Enabled:  aghalg.NBTrue,
-			Interval: float64(year.Milliseconds() + hour.Milliseconds()),
+			Interval: float64(maxIvl.Milliseconds() + minIvl.Milliseconds()),
 			Ignored:  []string{},
 		},
 		wantCode: http.StatusUnprocessableEntity,
 		wantErr:  "unsupported interval\n",
 	}, {
-		name: "set_ignored_ivl_1_year",
+		name: "set_ignored_ivl_1_maxIvl",
 		body: getConfigResp{
 			Enabled:  aghalg.NBTrue,
-			Interval: float64(year.Milliseconds()),
+			Interval: float64(maxIvl.Milliseconds()),
 			Ignored: []string{
 				"ignor.ed",
 			},
@@ -70,30 +78,30 @@ func TestHandleStatsConfig(t *testing.T) {
 		name: "ignored_duplicate",
 		body: getConfigResp{
 			Enabled:  aghalg.NBTrue,
-			Interval: float64(hour.Milliseconds()),
+			Interval: float64(minIvl.Milliseconds()),
 			Ignored: []string{
 				"ignor.ed",
 				"ignor.ed",
 			},
 		},
 		wantCode: http.StatusUnprocessableEntity,
-		wantErr:  "ignored: duplicate host name \"ignor.ed\"\n",
+		wantErr:  "ignored: duplicate or empty host\n",
 	}, {
 		name: "ignored_empty",
 		body: getConfigResp{
 			Enabled:  aghalg.NBTrue,
-			Interval: float64(hour.Milliseconds()),
+			Interval: float64(minIvl.Milliseconds()),
 			Ignored: []string{
 				"",
 			},
 		},
 		wantCode: http.StatusUnprocessableEntity,
-		wantErr:  "ignored: host name is empty\n",
+		wantErr:  "ignored: duplicate or empty host\n",
 	}, {
 		name: "enabled_is_null",
 		body: getConfigResp{
 			Enabled:  aghalg.NBNull,
-			Interval: float64(hour.Milliseconds()),
+			Interval: float64(minIvl.Milliseconds()),
 			Ignored:  []string{},
 		},
 		wantCode: http.StatusUnprocessableEntity,
@@ -102,23 +110,6 @@ func TestHandleStatsConfig(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			handlers := map[string]http.Handler{}
-
-			conf := Config{
-				Filename: filepath.Join(t.TempDir(), "stats.db"),
-				Limit:    time.Hour * 24,
-				Enabled:  true,
-				UnitID:   func() (id uint32) { return 0 },
-				HTTPRegister: func(
-					_ string,
-					url string,
-					handler http.HandlerFunc,
-				) {
-					handlers[url] = handler
-				},
-				ConfigModified: func() {},
-			}
-
 			s, err := New(conf)
 			require.NoError(t, err)
 
@@ -136,11 +127,10 @@ func TestHandleStatsConfig(t *testing.T) {
 			req := httptest.NewRequest(http.MethodPut, configPut, bytes.NewReader(buf))
 			rw := httptest.NewRecorder()
 
-			handlers[configPut].ServeHTTP(rw, req)
+			s.handlePutStatsConfig(rw, req)
 			require.Equal(t, tc.wantCode, rw.Code)
 
 			if tc.wantCode != http.StatusOK {
-				assert.Equal(t, tc.wantCode, rw.Code)
 				assert.Equal(t, tc.wantErr, rw.Body.String())
 
 				return
@@ -149,12 +139,12 @@ func TestHandleStatsConfig(t *testing.T) {
 			resp := httptest.NewRequest(http.MethodGet, configGet, nil)
 			rw = httptest.NewRecorder()
 
-			handlers[configGet].ServeHTTP(rw, resp)
+			s.handleGetStatsConfig(rw, resp)
 			require.Equal(t, http.StatusOK, rw.Code)
 
 			ans := getConfigResp{}
-			jerr := json.Unmarshal(rw.Body.Bytes(), &ans)
+			err = json.Unmarshal(rw.Body.Bytes(), &ans)
-			require.NoError(t, jerr)
+			require.NoError(t, err)
 
 			assert.Equal(t, tc.body, ans)
 		})

internal/stats/stats.go

@@ -128,12 +128,8 @@ func New(conf Config) (s *StatsCtx, err error) {
 		ignored:        conf.Ignored,
 	}
 
-	if conf.Limit < time.Hour {
+	if conf.Limit < time.Hour || conf.Limit > timeutil.Day*365 {
-		return nil, errors.Error("interval: less than an hour")
+		return nil, errors.Error("unsupported interval")
-	}
-
-	if conf.Limit > timeutil.Day*365 {
-		return nil, errors.Error("interval: more than a year")
 	}
 
 	s.limit = conf.Limit

openapi/CHANGELOG.md

@@ -6,18 +6,20 @@
 
 
 
-## v0.107.25: API changes
+## v0.107.26: API changes
 
 ### Deprecated statistics APIs
 
 * The `GET /control/stats_info` HTTP API; use the new `GET
   /control/stats/config` API instead.
+
 * The `POST /control/stats_config` HTTP API; use the new `PUT
   /control/stats/config/update` API instead.
 
 ### New statistics APIs
 
 * The new `GET /control/stats/config` HTTP API.
+
 * The new `PUT /control/stats/config/update` HTTP API allows config updates.
 
 These `control/stats/config/update` and `control/stats/config` APIs accept and
@@ -35,12 +37,14 @@ return a JSON object with the following format:
 
 * The `GET /control/querylog_info` HTTP API; use the new `GET
   /control/querylog/config` API instead.
+
 * The `POST /control/querylog_config` HTTP API; use the new `PUT
   /control/querylog/config/update` API instead.
 
 ### New query log APIs
 
 * The new `GET /control/querylog/config` HTTP API.
+
 * The new `PUT /control/querylog/config/update` HTTP API allows config updates.
 
 These `control/querylog/config/update` and `control/querylog/config` APIs

openapi/openapi.yaml

@@ -227,10 +227,12 @@
   '/querylog_info':
     'get':
       'deprecated': true
-      'description': >
+      'description': |
         Deprecated: Use `GET /querylog/config` instead.
 
-        NOTE: If interval is custom then it will be equal to `90` days for
+        NOTE: If `interval` was configured by editing configuration file or new
+        HTTP API call `PUT /querylog/config/update` and it's not equal to
+        previous allowed enum values then it will be equal to `90` days for
         compatibility reasons.
       'tags':
       - 'log'
@@ -322,10 +324,12 @@
   '/stats_info':
     'get':
       'deprecated': true
-      'description': >
+      'description': |
         Deprecated: Use `GET /stats/config` instead.
 
-        NOTE: If interval is custom then it will be equal to `90` days for
+        NOTE: If `interval` was configured by editing configuration file or new
+        HTTP API call `PUT /stats/config/update` and it's not equal to
+        previous allowed enum values then it will be equal to `90` days for
         compatibility reasons.
       'tags':
       - 'stats'
@@ -1748,24 +1752,7 @@
           'items':
             'type': 'string'
     'PutStatsConfigUpdateRequest':
-      'type': 'object'
+      '$ref': '#/components/schemas/GetStatsConfigResponse'
-      'description': 'Statistics configuration'
-      'required':
-      - 'enabled'
-      - 'interval'
-      - 'ignored'
-      'properties':
-        'enabled':
-          'description': 'Are statistics enabled'
-          'type': 'boolean'
-        'interval':
-          'description': 'Statistics rotation interval'
-          'type': 'number'
-        'ignored':
-          'description': 'List of host names, which should not be counted'
-          'type': 'array'
-          'items':
-            'type': 'string'
     'DhcpConfig':
       'type': 'object'
       'properties':
@@ -2194,29 +2181,7 @@
           'items':
             'type': 'string'
     'PutQueryLogConfigUpdateRequest':
-      'type': 'object'
+      '$ref': '#/components/schemas/GetQueryLogConfigResponse'
-      'description': 'Query log configuration'
-      'required':
-      - 'enabled'
-      - 'interval'
-      - 'anonymize_client_ip'
-      - 'ignored'
-      'properties':
-        'enabled':
-          'type': 'boolean'
-          'description': 'Is query log enabled'
-        'interval':
-          'description': >
-            Time period for query log rotation.
-          'type': 'number'
-        'anonymize_client_ip':
-          'type': 'boolean'
-          'description': "Anonymize clients' IP addresses"
-        'ignored':
-          'description': 'List of host names, which should not be written to log'
-          'type': 'array'
-          'items':
-            'type': 'string'
     'ResultRule':
       'description': 'Applied rule.'
       'properties':