Merge remote-tracking branch 'origin/master' into AG-28961-upd-golibs

CHANGELOG.md

@@ -71,6 +71,7 @@ In this release, the schema version has changed from 27 to 28.
 
 ### Fixed
 
+- Schedule display in the client settings after creating or updating.
 - Zero value in `querylog.size_memory` disables logging ([#6570]).
 - Non-anonymized IP addresses on the dashboard ([#6584]).
 - Maximum cache TTL requirement when editing minimum cache TTL in the Web UI

client/src/components/Filters/Services/ScheduleForm/index.js

@@ -1,4 +1,4 @@
-import React, { useState, useMemo } from 'react';
+import React, { useState } from 'react';
 import { useTranslation } from 'react-i18next';
 import PropTypes from 'prop-types';
 import cn from 'classnames';
@@ -21,10 +21,7 @@ export const ScheduleForm = ({
     const onModalOpen = () => setModalOpen(true);
     const onModalClose = () => setModalOpen(false);
 
-    const filteredScheduleKeys = useMemo(() => (
-        schedule ? Object.keys(schedule).filter((v) => v !== 'time_zone') : []
-    ), [schedule]);
-
+    const filteredScheduleKeys = schedule ? Object.keys(schedule).filter((v) => v !== 'time_zone') : [];
     const scheduleMap = new Map();
     filteredScheduleKeys.forEach((day) => scheduleMap.set(day, schedule[day]));
 

client/src/components/Settings/Clients/Form.js

@@ -162,7 +162,7 @@ let Form = (props) => {
     const [activeTabLabel, setActiveTabLabel] = useState('settings');
 
     const handleScheduleSubmit = (values) => {
-        change('blocked_services_schedule', values);
+        change('blocked_services_schedule', { ...values });
     };
 
     const tabs = {

internal/home/client.go

@@ -3,13 +3,20 @@ package home
 import (
 	"encoding"
 	"fmt"
+	"net"
+	"net/netip"
+	"strings"
 	"time"
 
+	"github.com/AdguardTeam/AdGuardHome/internal/dnsforward"
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering"
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering/safesearch"
 	"github.com/AdguardTeam/dnsproxy/proxy"
+	"github.com/AdguardTeam/golibs/errors"
+	"github.com/AdguardTeam/golibs/log"
 	"github.com/AdguardTeam/golibs/stringutil"
 	"github.com/google/uuid"
+	"golang.org/x/exp/slices"
 )
 
 // UID is the type for the unique IDs of persistent clients.
@@ -56,10 +63,15 @@ type persistentClient struct {
 
 	Name string
 
-	IDs       []string
 	Tags      []string
 	Upstreams []string
 
+	IPs []netip.Addr
+	// TODO(s.chzhen):  Use netutil.Prefix.
+	Subnets   []netip.Prefix
+	MACs      []net.HardwareAddr
+	ClientIDs []string
+
 	// UID is the unique identifier of the persistent client.
 	UID UID
 
@@ -75,17 +87,149 @@ type persistentClient struct {
 	IgnoreStatistics      bool
 }
 
-// ShallowClone returns a deep copy of the client, except upstreamConfig,
+// setTags sets the tags if they are known, otherwise logs an unknown tag.
+func (c *persistentClient) setTags(tags []string, known *stringutil.Set) {
+	for _, t := range tags {
+		if !known.Has(t) {
+			log.Info("skipping unknown tag %q", t)
+
+			continue
+		}
+
+		c.Tags = append(c.Tags, t)
+	}
+
+	slices.Sort(c.Tags)
+}
+
+// setIDs parses a list of strings into typed fields and returns an error if
+// there is one.
+func (c *persistentClient) setIDs(ids []string) (err error) {
+	for _, id := range ids {
+		err = c.setID(id)
+		if err != nil {
+			return err
+		}
+	}
+
+	slices.SortFunc(c.IPs, netip.Addr.Compare)
+
+	// TODO(s.chzhen):  Use netip.PrefixCompare in Go 1.23.
+	slices.SortFunc(c.Subnets, subnetCompare)
+	slices.SortFunc(c.MACs, slices.Compare[net.HardwareAddr])
+	slices.Sort(c.ClientIDs)
+
+	return nil
+}
+
+// subnetCompare is a comparison function for the two subnets.  It returns -1 if
+// x sorts before y, 1 if x sorts after y, and 0 if their relative sorting
+// position is the same.
+func subnetCompare(x, y netip.Prefix) (cmp int) {
+	if x == y {
+		return 0
+	}
+
+	xAddr, xBits := x.Addr(), x.Bits()
+	yAddr, yBits := y.Addr(), y.Bits()
+	if xBits == yBits {
+		return xAddr.Compare(yAddr)
+	}
+
+	if xBits > yBits {
+		return -1
+	} else {
+		return 1
+	}
+}
+
+// setID parses id into typed field if there is no error.
+func (c *persistentClient) setID(id string) (err error) {
+	if id == "" {
+		return errors.Error("clientid is empty")
+	}
+
+	var ip netip.Addr
+	if ip, err = netip.ParseAddr(id); err == nil {
+		c.IPs = append(c.IPs, ip)
+
+		return nil
+	}
+
+	var subnet netip.Prefix
+	if subnet, err = netip.ParsePrefix(id); err == nil {
+		c.Subnets = append(c.Subnets, subnet)
+
+		return nil
+	}
+
+	var mac net.HardwareAddr
+	if mac, err = net.ParseMAC(id); err == nil {
+		c.MACs = append(c.MACs, mac)
+
+		return nil
+	}
+
+	err = dnsforward.ValidateClientID(id)
+	if err != nil {
+		// Don't wrap the error, because it's informative enough as is.
+		return err
+	}
+
+	c.ClientIDs = append(c.ClientIDs, strings.ToLower(id))
+
+	return nil
+}
+
+// ids returns a list of client ids containing at least one element.
+func (c *persistentClient) ids() (ids []string) {
+	ids = make([]string, 0, c.idsLen())
+
+	for _, ip := range c.IPs {
+		ids = append(ids, ip.String())
+	}
+
+	for _, subnet := range c.Subnets {
+		ids = append(ids, subnet.String())
+	}
+
+	for _, mac := range c.MACs {
+		ids = append(ids, mac.String())
+	}
+
+	return append(ids, c.ClientIDs...)
+}
+
+// idsLen returns a length of client ids.
+func (c *persistentClient) idsLen() (n int) {
+	return len(c.IPs) + len(c.Subnets) + len(c.MACs) + len(c.ClientIDs)
+}
+
+// equalIDs returns true if the ids of the current and previous clients are the
+// same.
+func (c *persistentClient) equalIDs(prev *persistentClient) (equal bool) {
+	return slices.Equal(c.IPs, prev.IPs) &&
+		slices.Equal(c.Subnets, prev.Subnets) &&
+		slices.EqualFunc(c.MACs, prev.MACs, slices.Equal[net.HardwareAddr]) &&
+		slices.Equal(c.ClientIDs, prev.ClientIDs)
+}
+
+// shallowClone returns a deep copy of the client, except upstreamConfig,
 // safeSearchConf, SafeSearch fields, because it's difficult to copy them.
-func (c *persistentClient) ShallowClone() (sh *persistentClient) {
-	clone := *c
+func (c *persistentClient) shallowClone() (clone *persistentClient) {
+	clone = &persistentClient{}
+	*clone = *c
 
 	clone.BlockedServices = c.BlockedServices.Clone()
-	clone.IDs = stringutil.CloneSlice(c.IDs)
-	clone.Tags = stringutil.CloneSlice(c.Tags)
-	clone.Upstreams = stringutil.CloneSlice(c.Upstreams)
+	clone.Tags = slices.Clone(c.Tags)
+	clone.Upstreams = slices.Clone(c.Upstreams)
+
+	clone.IPs = slices.Clone(c.IPs)
+	clone.Subnets = slices.Clone(c.Subnets)
+	clone.MACs = slices.Clone(c.MACs)
+	clone.ClientIDs = slices.Clone(c.ClientIDs)
 
-	return &clone
+	return clone
 }
 
 // closeUpstreams closes the client-specific upstream config of c if any.

internal/home/client_internal_test.go

@@ -0,0 +1,124 @@
+package home
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestPersistentClient_EqualIDs(t *testing.T) {
+	const (
+		ip  = "0.0.0.0"
+		ip1 = "1.1.1.1"
+		ip2 = "2.2.2.2"
+
+		cidr  = "0.0.0.0/0"
+		cidr1 = "1.1.1.1/11"
+		cidr2 = "2.2.2.2/22"
+
+		mac  = "00-00-00-00-00-00"
+		mac1 = "11-11-11-11-11-11"
+		mac2 = "22-22-22-22-22-22"
+
+		cli  = "client0"
+		cli1 = "client1"
+		cli2 = "client2"
+	)
+
+	testCases := []struct {
+		name    string
+		ids     []string
+		prevIDs []string
+		want    assert.BoolAssertionFunc
+	}{{
+		name:    "single_ip",
+		ids:     []string{ip1},
+		prevIDs: []string{ip1},
+		want:    assert.True,
+	}, {
+		name:    "single_ip_not_equal",
+		ids:     []string{ip1},
+		prevIDs: []string{ip2},
+		want:    assert.False,
+	}, {
+		name:    "ips_not_equal",
+		ids:     []string{ip1, ip2},
+		prevIDs: []string{ip1, ip},
+		want:    assert.False,
+	}, {
+		name:    "ips_mixed_equal",
+		ids:     []string{ip1, ip2},
+		prevIDs: []string{ip2, ip1},
+		want:    assert.True,
+	}, {
+		name:    "single_subnet",
+		ids:     []string{cidr1},
+		prevIDs: []string{cidr1},
+		want:    assert.True,
+	}, {
+		name:    "subnets_not_equal",
+		ids:     []string{ip1, ip2, cidr1, cidr2},
+		prevIDs: []string{ip1, ip2, cidr1, cidr},
+		want:    assert.False,
+	}, {
+		name:    "subnets_mixed_equal",
+		ids:     []string{ip1, ip2, cidr1, cidr2},
+		prevIDs: []string{cidr2, cidr1, ip2, ip1},
+		want:    assert.True,
+	}, {
+		name:    "single_mac",
+		ids:     []string{mac1},
+		prevIDs: []string{mac1},
+		want:    assert.True,
+	}, {
+		name:    "single_mac_not_equal",
+		ids:     []string{mac1},
+		prevIDs: []string{mac2},
+		want:    assert.False,
+	}, {
+		name:    "macs_not_equal",
+		ids:     []string{ip1, ip2, cidr1, cidr2, mac1, mac2},
+		prevIDs: []string{ip1, ip2, cidr1, cidr2, mac1, mac},
+		want:    assert.False,
+	}, {
+		name:    "macs_mixed_equal",
+		ids:     []string{ip1, ip2, cidr1, cidr2, mac1, mac2},
+		prevIDs: []string{mac2, mac1, cidr2, cidr1, ip2, ip1},
+		want:    assert.True,
+	}, {
+		name:    "single_client_id",
+		ids:     []string{cli1},
+		prevIDs: []string{cli1},
+		want:    assert.True,
+	}, {
+		name:    "single_client_id_not_equal",
+		ids:     []string{cli1},
+		prevIDs: []string{cli2},
+		want:    assert.False,
+	}, {
+		name:    "client_ids_not_equal",
+		ids:     []string{ip1, ip2, cidr1, cidr2, mac1, mac2, cli1, cli2},
+		prevIDs: []string{ip1, ip2, cidr1, cidr2, mac1, mac2, cli1, cli},
+		want:    assert.False,
+	}, {
+		name:    "client_ids_mixed_equal",
+		ids:     []string{ip1, ip2, cidr1, cidr2, mac1, mac2, cli1, cli2},
+		prevIDs: []string{cli2, cli1, mac2, mac1, cidr2, cidr1, ip2, ip1},
+		want:    assert.True,
+	}}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			c := &persistentClient{}
+			err := c.setIDs(tc.ids)
+			require.NoError(t, err)
+
+			prev := &persistentClient{}
+			err = prev.setIDs(tc.prevIDs)
+			require.NoError(t, err)
+
+			tc.want(t, c.equalIDs(prev))
+		})
+	}
+}