Pull request 2110: AG-27492-client-persistent

Squashed commit of the following:

commit 6605cd17a2e5137cf69c853c2a956b2443e81ce9
Merge: 5b294a268 bec3cab56
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Thu Dec 28 16:51:39 2023 +0300

    Merge branch 'master' into AG-27492-client-persistent

commit 5b294a26848e173b26eb9496b3c380b847a9fa1b
Merge: afe4d5f16 d75712bb9
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Dec 27 19:13:50 2023 +0300

    Merge branch 'master' into AG-27492-client-persistent

commit afe4d5f1659c474173139ed4c841d72306ed27ac
Merge: 05dc0bfda ad147ac7b
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Tue Dec 26 14:59:10 2023 +0300

    Merge branch 'master' into AG-27492-client-persistent

commit 05dc0bfda2001cececc37c040f0cee632921aae4
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Wed Dec 20 19:44:04 2023 +0300

    home: imp err msg

commit c3b21c739ccb4436e9606579e3fb46cc32821a81
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Mon Dec 18 15:05:29 2023 +0300

    all: imp docs

commit a2118f5aedd93c16ffeb35d14dbcd2b3a1cdb4a4
Author: Stanislav Chzhen <s.chzhen@adguard.com>
Date:   Fri Dec 15 18:27:50 2023 +0300

    all: add persistent client uid

CHANGELOG.md

@@ -40,6 +40,8 @@ NOTE: Add new changes BELOW THIS COMMENT.
 
 #### Configuration changes
 
+- The new property `clients.persistent.*.uid`, which is unique identifier of the
+  persistent client.
 - The properties `dns.'all_servers` and `dns.fastest_addr` were removed, their
   values migrated to newly added field `dns.upstream_mode` that describes the
   logic through which upstreams will be used.

internal/home/client.go

@@ -1,6 +1,7 @@
 package home
 
 import (
+	"encoding"
 	"fmt"
 	"time"
 
@@ -8,10 +9,38 @@ import (
 	"github.com/AdguardTeam/AdGuardHome/internal/filtering/safesearch"
 	"github.com/AdguardTeam/dnsproxy/proxy"
 	"github.com/AdguardTeam/golibs/stringutil"
+	"github.com/google/uuid"
 )
 
-// Client contains information about persistent clients.
+// UID is the type for the unique IDs of persistent clients.
-type Client struct {
+type UID uuid.UUID
+
+// NewUID returns a new persistent client UID.  Any error returned is an error
+// from the cryptographic randomness reader.
+func NewUID() (uid UID, err error) {
+	uuidv7, err := uuid.NewV7()
+
+	return UID(uuidv7), err
+}
+
+// type check
+var _ encoding.TextMarshaler = UID{}
+
+// MarshalText implements the [encoding.TextMarshaler] for UID.
+func (uid UID) MarshalText() ([]byte, error) {
+	return uuid.UUID(uid).MarshalText()
+}
+
+// type check
+var _ encoding.TextUnmarshaler = (*UID)(nil)
+
+// UnmarshalText implements the [encoding.TextUnmarshaler] interface for UID.
+func (uid *UID) UnmarshalText(data []byte) error {
+	return (*uuid.UUID)(uid).UnmarshalText(data)
+}
+
+// persistentClient contains information about persistent clients.
+type persistentClient struct {
 	// upstreamConfig is the custom upstream configuration for this client.  If
 	// it's nil, it has not been initialized yet.  If it's non-nil and empty,
 	// there are no valid upstreams.  If it's non-nil and non-empty, these
@@ -31,6 +60,9 @@ type Client struct {
 	Tags      []string
 	Upstreams []string
 
+	// UID is the unique identifier of the persistent client.
+	UID UID
+
 	UpstreamsCacheSize    uint32
 	UpstreamsCacheEnabled bool
 
@@ -45,7 +77,7 @@ type Client struct {
 
 // ShallowClone returns a deep copy of the client, except upstreamConfig,
 // safeSearchConf, SafeSearch fields, because it's difficult to copy them.
-func (c *Client) ShallowClone() (sh *Client) {
+func (c *persistentClient) ShallowClone() (sh *persistentClient) {
 	clone := *c
 
 	clone.BlockedServices = c.BlockedServices.Clone()
@@ -57,7 +89,7 @@ func (c *Client) ShallowClone() (sh *Client) {
 }
 
 // closeUpstreams closes the client-specific upstream config of c if any.
-func (c *Client) closeUpstreams() (err error) {
+func (c *persistentClient) closeUpstreams() (err error) {
 	if c.upstreamConfig != nil {
 		if err = c.upstreamConfig.Close(); err != nil {
 			return fmt.Errorf("closing upstreams of client %q: %w", c.Name, err)
@@ -68,7 +100,7 @@ func (c *Client) closeUpstreams() (err error) {
 }
 
 // setSafeSearch initializes and sets the safe search filter for this client.
-func (c *Client) setSafeSearch(
+func (c *persistentClient) setSafeSearch(
 	conf filtering.SafeSearchConfig,
 	cacheSize uint,
 	cacheTTL time.Duration,

internal/home/clients.go

@@ -48,8 +48,8 @@ type DHCP interface {
 type clientsContainer struct {
 	// TODO(a.garipov): Perhaps use a number of separate indices for different
 	// types (string, netip.Addr, and so on).
-	list    map[string]*Client // name -> client
+	list    map[string]*persistentClient // name -> client
-	idIndex map[string]*Client // ID -> client
+	idIndex map[string]*persistentClient // ID -> client
 
 	// ipToRC maps IP addresses to runtime client information.
 	ipToRC map[netip.Addr]*client.Runtime
@@ -103,8 +103,8 @@ func (clients *clientsContainer) Init(
 		log.Fatal("clients.list != nil")
 	}
 
-	clients.list = map[string]*Client{}
+	clients.list = map[string]*persistentClient{}
-	clients.idIndex = map[string]*Client{}
+	clients.idIndex = map[string]*persistentClient{}
 	clients.ipToRC = map[netip.Addr]*client.Runtime{}
 
 	clients.allTags = stringutil.NewSet(clientTags...)
@@ -189,6 +189,9 @@ type clientObject struct {
 	Tags      []string `yaml:"tags"`
 	Upstreams []string `yaml:"upstreams"`
 
+	// UID is the unique identifier of the persistent client.
+	UID UID `yaml:"uid"`
+
 	// UpstreamsCacheSize is the DNS cache size (in bytes).
 	//
 	// TODO(d.kolyshev): Use [datasize.Bytesize].
@@ -207,66 +210,87 @@ type clientObject struct {
 	IgnoreStatistics bool `yaml:"ignore_statistics"`
 }
 
+// toPersistent returns an initialized persistent client if there are no errors.
+func (o *clientObject) toPersistent(
+	filteringConf *filtering.Config,
+	allTags *stringutil.Set,
+) (cli *persistentClient, err error) {
+	cli = &persistentClient{
+		Name: o.Name,
+
+		IDs:       o.IDs,
+		Upstreams: o.Upstreams,
+
+		UID: o.UID,
+
+		UseOwnSettings:        !o.UseGlobalSettings,
+		FilteringEnabled:      o.FilteringEnabled,
+		ParentalEnabled:       o.ParentalEnabled,
+		safeSearchConf:        o.SafeSearchConf,
+		SafeBrowsingEnabled:   o.SafeBrowsingEnabled,
+		UseOwnBlockedServices: !o.UseGlobalBlockedServices,
+		IgnoreQueryLog:        o.IgnoreQueryLog,
+		IgnoreStatistics:      o.IgnoreStatistics,
+		UpstreamsCacheEnabled: o.UpstreamsCacheEnabled,
+		UpstreamsCacheSize:    o.UpstreamsCacheSize,
+	}
+
+	if (cli.UID == UID{}) {
+		cli.UID, err = NewUID()
+		if err != nil {
+			return nil, fmt.Errorf("generating uid: %w", err)
+		}
+	}
+
+	if o.SafeSearchConf.Enabled {
+		o.SafeSearchConf.CustomResolver = safeSearchResolver{}
+
+		err = cli.setSafeSearch(
+			o.SafeSearchConf,
+			filteringConf.SafeSearchCacheSize,
+			time.Minute*time.Duration(filteringConf.CacheTime),
+		)
+		if err != nil {
+			return nil, fmt.Errorf("init safesearch %q: %w", cli.Name, err)
+		}
+	}
+
+	err = o.BlockedServices.Validate()
+	if err != nil {
+		return nil, fmt.Errorf("init blocked services %q: %w", cli.Name, err)
+	}
+
+	cli.BlockedServices = o.BlockedServices.Clone()
+
+	for _, t := range o.Tags {
+		if allTags.Has(t) {
+			cli.Tags = append(cli.Tags, t)
+		} else {
+			log.Info("skipping unknown tag %q", t)
+		}
+	}
+
+	slices.Sort(cli.Tags)
+
+	return cli, nil
+}
+
 // addFromConfig initializes the clients container with objects from the
 // configuration file.
 func (clients *clientsContainer) addFromConfig(
 	objects []*clientObject,
 	filteringConf *filtering.Config,
 ) (err error) {
-	for _, o := range objects {
+	for i, o := range objects {
-		cli := &Client{
+		var cli *persistentClient
-			Name: o.Name,
+		cli, err = o.toPersistent(filteringConf, clients.allTags)
-
-			IDs:       o.IDs,
-			Upstreams: o.Upstreams,
-
-			UseOwnSettings:        !o.UseGlobalSettings,
-			FilteringEnabled:      o.FilteringEnabled,
-			ParentalEnabled:       o.ParentalEnabled,
-			safeSearchConf:        o.SafeSearchConf,
-			SafeBrowsingEnabled:   o.SafeBrowsingEnabled,
-			UseOwnBlockedServices: !o.UseGlobalBlockedServices,
-			IgnoreQueryLog:        o.IgnoreQueryLog,
-			IgnoreStatistics:      o.IgnoreStatistics,
-			UpstreamsCacheEnabled: o.UpstreamsCacheEnabled,
-			UpstreamsCacheSize:    o.UpstreamsCacheSize,
-		}
-
-		if o.SafeSearchConf.Enabled {
-			o.SafeSearchConf.CustomResolver = safeSearchResolver{}
-
-			err = cli.setSafeSearch(
-				o.SafeSearchConf,
-				filteringConf.SafeSearchCacheSize,
-				time.Minute*time.Duration(filteringConf.CacheTime),
-			)
-			if err != nil {
-				log.Error("clients: init client safesearch %q: %s", cli.Name, err)
-
-				continue
-			}
-		}
-
-		err = o.BlockedServices.Validate()
 		if err != nil {
-			return fmt.Errorf("clients: init client blocked services %q: %w", cli.Name, err)
+			return fmt.Errorf("clients: init persistent client at index %d: %w", i, err)
 		}
 
-		cli.BlockedServices = o.BlockedServices.Clone()
+		_, err = clients.add(cli)
-
-		for _, t := range o.Tags {
-			if clients.allTags.Has(t) {
-				cli.Tags = append(cli.Tags, t)
-			} else {
-				log.Info("clients: skipping unknown tag %q", t)
-			}
-		}
-
-		slices.Sort(cli.Tags)
-
-		_, err = clients.Add(cli)
 		if err != nil {
-			log.Error("clients: adding clients %s: %s", cli.Name, err)
+			log.Error("clients: adding client at index %d %s: %s", i, cli.Name, err)
 		}
 	}
 
@@ -290,6 +314,8 @@ func (clients *clientsContainer) forConfig() (objs []*clientObject) {
 			Tags:      stringutil.CloneSlice(cli.Tags),
 			Upstreams: stringutil.CloneSlice(cli.Upstreams),
 
+			UID: cli.UID,
+
 			UseGlobalSettings:        !cli.UseOwnSettings,
 			FilteringEnabled:         cli.FilteringEnabled,
 			ParentalEnabled:          cli.ParentalEnabled,
@@ -352,10 +378,10 @@ func (clients *clientsContainer) clientSource(ip netip.Addr) (src client.Source)
 	return src
 }
 
-// findMultiple is a wrapper around Find to make it a valid client finder for
+// findMultiple is a wrapper around [clientsContainer.find] to make it a valid
-// the query log.  c is never nil; if no information about the client is found,
+// client finder for the query log.  c is never nil; if no information about the
-// it returns an artificial client record by only setting the blocking-related
+// client is found, it returns an artificial client record by only setting the
-// fields.  err is always nil.
+// blocking-related fields.  err is always nil.
 func (clients *clientsContainer) findMultiple(ids []string) (c *querylog.Client, err error) {
 	var artClient *querylog.Client
 	var art bool
@@ -389,7 +415,7 @@ func (clients *clientsContainer) clientOrArtificial(
 		}
 	}()
 
-	cli, ok := clients.Find(id)
+	cli, ok := clients.find(id)
 	if ok {
 		return &querylog.Client{
 			Name:           cli.Name,
@@ -413,8 +439,8 @@ func (clients *clientsContainer) clientOrArtificial(
 	}, true
 }
 
-// Find returns a shallow copy of the client if there is one found.
+// find returns a shallow copy of the client if there is one found.
-func (clients *clientsContainer) Find(id string) (c *Client, ok bool) {
+func (clients *clientsContainer) find(id string) (c *persistentClient, ok bool) {
 	clients.lock.Lock()
 	defer clients.lock.Unlock()
 
@@ -426,9 +452,9 @@ func (clients *clientsContainer) Find(id string) (c *Client, ok bool) {
 	return c.ShallowClone(), true
 }
 
-// shouldCountClient is a wrapper around Find to make it a valid client
+// shouldCountClient is a wrapper around [clientsContainer.find] to make it a
-// information finder for the statistics.  If no information about the client
+// valid client information finder for the statistics.  If no information about
-// is found, it returns true.
+// the client is found, it returns true.
 func (clients *clientsContainer) shouldCountClient(ids []string) (y bool) {
 	clients.lock.Lock()
 	defer clients.lock.Unlock()
@@ -496,7 +522,7 @@ func (clients *clientsContainer) UpstreamConfigByID(
 
 // findLocked searches for a client by its ID.  clients.lock is expected to be
 // locked.
-func (clients *clientsContainer) findLocked(id string) (c *Client, ok bool) {
+func (clients *clientsContainer) findLocked(id string) (c *persistentClient, ok bool) {
 	c, ok = clients.idIndex[id]
 	if ok {
 		return c, true
@@ -527,7 +553,7 @@ func (clients *clientsContainer) findLocked(id string) (c *Client, ok bool) {
 
 // findDHCP searches for a client by its MAC, if the DHCP server is active and
 // there is such client.  clients.lock is expected to be locked.
-func (clients *clientsContainer) findDHCP(ip netip.Addr) (c *Client, ok bool) {
+func (clients *clientsContainer) findDHCP(ip netip.Addr) (c *persistentClient, ok bool) {
 	foundMAC := clients.dhcp.MACByIP(ip)
 	if foundMAC == nil {
 		return nil, false
@@ -583,7 +609,7 @@ func (clients *clientsContainer) findRuntimeClient(ip netip.Addr) (rc *client.Ru
 }
 
 // check validates the client.
-func (clients *clientsContainer) check(c *Client) (err error) {
+func (clients *clientsContainer) check(c *persistentClient) (err error) {
 	switch {
 	case c == nil:
 		return errors.Error("client is nil")
@@ -650,9 +676,9 @@ func normalizeClientIdentifier(idStr string) (norm string, err error) {
 	return "", fmt.Errorf("bad client identifier %q", idStr)
 }
 
-// Add adds a new client object.  ok is false if such client already exists or
+// add adds a new client object.  ok is false if such client already exists or
 // if an error occurred.
-func (clients *clientsContainer) Add(c *Client) (ok bool, err error) {
+func (clients *clientsContainer) add(c *persistentClient) (ok bool, err error) {
 	err = clients.check(c)
 	if err != nil {
 		return false, err
@@ -669,22 +695,22 @@ func (clients *clientsContainer) Add(c *Client) (ok bool, err error) {
 
 	// check ID index
 	for _, id := range c.IDs {
-		var c2 *Client
+		var c2 *persistentClient
 		c2, ok = clients.idIndex[id]
 		if ok {
 			return false, fmt.Errorf("another client uses the same ID (%q): %q", id, c2.Name)
 		}
 	}
 
-	clients.add(c)
+	clients.addLocked(c)
 
 	log.Debug("clients: added %q: ID:%q [%d]", c.Name, c.IDs, len(clients.list))
 
 	return true, nil
 }
 
-// add c to the indexes. clients.lock is expected to be locked.
+// addLocked c to the indexes.  clients.lock is expected to be locked.
-func (clients *clientsContainer) add(c *Client) {
+func (clients *clientsContainer) addLocked(c *persistentClient) {
 	// update Name index
 	clients.list[c.Name] = c
 
@@ -694,24 +720,25 @@ func (clients *clientsContainer) add(c *Client) {
 	}
 }
 
-// Del removes a client.  ok is false if there is no such client.
+// remove removes a client.  ok is false if there is no such client.
-func (clients *clientsContainer) Del(name string) (ok bool) {
+func (clients *clientsContainer) remove(name string) (ok bool) {
 	clients.lock.Lock()
 	defer clients.lock.Unlock()
 
-	var c *Client
+	var c *persistentClient
 	c, ok = clients.list[name]
 	if !ok {
 		return false
 	}
 
-	clients.del(c)
+	clients.removeLocked(c)
 
 	return true
 }
 
-// del removes c from the indexes. clients.lock is expected to be locked.
+// removeLocked removes c from the indexes.  clients.lock is expected to be
-func (clients *clientsContainer) del(c *Client) {
+// locked.
+func (clients *clientsContainer) removeLocked(c *persistentClient) {
 	if err := c.closeUpstreams(); err != nil {
 		log.Error("client container: removing client %s: %s", c.Name, err)
 	}
@@ -725,8 +752,8 @@ func (clients *clientsContainer) del(c *Client) {
 	}
 }
 
-// Update updates a client by its name.
+// update updates a client by its name.
-func (clients *clientsContainer) Update(prev, c *Client) (err error) {
+func (clients *clientsContainer) update(prev, c *persistentClient) (err error) {
 	err = clients.check(c)
 	if err != nil {
 		// Don't wrap the error since it's informative enough as is.
@@ -754,8 +781,8 @@ func (clients *clientsContainer) Update(prev, c *Client) (err error) {
 		}
 	}
 
-	clients.del(prev)
+	clients.removeLocked(prev)
-	clients.add(c)
+	clients.addLocked(c)
 
 	return nil
 }
@@ -928,7 +955,7 @@ func (clients *clientsContainer) addFromSystemARP() {
 // the persistent clients.
 func (clients *clientsContainer) close() (err error) {
 	persistent := maps.Values(clients.list)
-	slices.SortFunc(persistent, func(a, b *Client) (res int) {
+	slices.SortFunc(persistent, func(a, b *persistentClient) (res int) {
 		return strings.Compare(a.Name, b.Name)
 	})
 

internal/home/clients_internal_test.go

@@ -64,42 +64,42 @@ func TestClients(t *testing.T) {
 			cli2IP = netip.MustParseAddr(cli2)
 		)
 
-		c := &Client{
+		c := &persistentClient{
 			IDs:  []string{cli1, "1:2:3::4", "aa:aa:aa:aa:aa:aa"},
 			Name: "client1",
 		}
 
-		ok, err := clients.Add(c)
+		ok, err := clients.add(c)
 		require.NoError(t, err)
 
 		assert.True(t, ok)
 
-		c = &Client{
+		c = &persistentClient{
 			IDs:  []string{cli2},
 			Name: "client2",
 		}
 
-		ok, err = clients.Add(c)
+		ok, err = clients.add(c)
 		require.NoError(t, err)
 
 		assert.True(t, ok)
 
-		c, ok = clients.Find(cli1)
+		c, ok = clients.find(cli1)
 		require.True(t, ok)
 
 		assert.Equal(t, "client1", c.Name)
 
-		c, ok = clients.Find("1:2:3::4")
+		c, ok = clients.find("1:2:3::4")
 		require.True(t, ok)
 
 		assert.Equal(t, "client1", c.Name)
 
-		c, ok = clients.Find(cli2)
+		c, ok = clients.find(cli2)
 		require.True(t, ok)
 
 		assert.Equal(t, "client2", c.Name)
 
-		_, ok = clients.Find(cliNone)
+		_, ok = clients.find(cliNone)
 		assert.False(t, ok)
 
 		assert.Equal(t, clients.clientSource(cli1IP), client.SourcePersistent)
@@ -107,7 +107,7 @@ func TestClients(t *testing.T) {
 	})
 
 	t.Run("add_fail_name", func(t *testing.T) {
-		ok, err := clients.Add(&Client{
+		ok, err := clients.add(&persistentClient{
 			IDs:  []string{"1.2.3.5"},
 			Name: "client1",
 		})
@@ -116,7 +116,7 @@ func TestClients(t *testing.T) {
 	})
 
 	t.Run("add_fail_ip", func(t *testing.T) {
-		ok, err := clients.Add(&Client{
+		ok, err := clients.add(&persistentClient{
 			IDs:  []string{"2.2.2.2"},
 			Name: "client3",
 		})
@@ -125,7 +125,7 @@ func TestClients(t *testing.T) {
 	})
 
 	t.Run("update_fail_ip", func(t *testing.T) {
-		err := clients.Update(&Client{Name: "client1"}, &Client{
+		err := clients.update(&persistentClient{Name: "client1"}, &persistentClient{
 			IDs:  []string{"2.2.2.2"},
 			Name: "client1",
 		})
@@ -143,13 +143,13 @@ func TestClients(t *testing.T) {
 		prev, ok := clients.list["client1"]
 		require.True(t, ok)
 
-		err := clients.Update(prev, &Client{
+		err := clients.update(prev, &persistentClient{
 			IDs:  []string{cliNew},
 			Name: "client1",
 		})
 		require.NoError(t, err)
 
-		_, ok = clients.Find(cliOld)
+		_, ok = clients.find(cliOld)
 		assert.False(t, ok)
 
 		assert.Equal(t, clients.clientSource(cliNewIP), client.SourcePersistent)
@@ -157,14 +157,14 @@ func TestClients(t *testing.T) {
 		prev, ok = clients.list["client1"]
 		require.True(t, ok)
 
-		err = clients.Update(prev, &Client{
+		err = clients.update(prev, &persistentClient{
 			IDs:            []string{cliNew},
 			Name:           "client1-renamed",
 			UseOwnSettings: true,
 		})
 		require.NoError(t, err)
 
-		c, ok := clients.Find(cliNew)
+		c, ok := clients.find(cliNew)
 		require.True(t, ok)
 
 		assert.Equal(t, "client1-renamed", c.Name)
@@ -181,15 +181,15 @@ func TestClients(t *testing.T) {
 	})
 
 	t.Run("del_success", func(t *testing.T) {
-		ok := clients.Del("client1-renamed")
+		ok := clients.remove("client1-renamed")
 		require.True(t, ok)
 
-		_, ok = clients.Find("1.1.1.2")
+		_, ok = clients.find("1.1.1.2")
 		assert.False(t, ok)
 	})
 
 	t.Run("del_fail", func(t *testing.T) {
-		ok := clients.Del("client3")
+		ok := clients.remove("client3")
 		assert.False(t, ok)
 	})
 
@@ -258,7 +258,7 @@ func TestClientsWHOIS(t *testing.T) {
 	t.Run("can't_set_manually-added", func(t *testing.T) {
 		ip := netip.MustParseAddr("1.1.1.2")
 
-		ok, err := clients.Add(&Client{
+		ok, err := clients.add(&persistentClient{
 			IDs:  []string{"1.1.1.2"},
 			Name: "client1",
 		})
@@ -269,7 +269,7 @@ func TestClientsWHOIS(t *testing.T) {
 		rc := clients.ipToRC[ip]
 		require.Nil(t, rc)
 
-		assert.True(t, clients.Del("client1"))
+		assert.True(t, clients.remove("client1"))
 	})
 }
 
@@ -280,7 +280,7 @@ func TestClientsAddExisting(t *testing.T) {
 		ip := netip.MustParseAddr("1.1.1.1")
 
 		// Add a client.
-		ok, err := clients.Add(&Client{
+		ok, err := clients.add(&persistentClient{
 			IDs:  []string{ip.String(), "1:2:3::4", "aa:aa:aa:aa:aa:aa", "2.2.2.0/24"},
 			Name: "client1",
 		})
@@ -328,7 +328,7 @@ func TestClientsAddExisting(t *testing.T) {
 		require.NoError(t, err)
 
 		// Add a new client with the same IP as for a client with MAC.
-		ok, err := clients.Add(&Client{
+		ok, err := clients.add(&persistentClient{
 			IDs:  []string{ip.String()},
 			Name: "client2",
 		})
@@ -336,7 +336,7 @@ func TestClientsAddExisting(t *testing.T) {
 		assert.True(t, ok)
 
 		// Add a new client with the IP from the first client's IP range.
-		ok, err = clients.Add(&Client{
+		ok, err = clients.add(&persistentClient{
 			IDs:  []string{"2.2.2.2"},
 			Name: "client3",
 		})
@@ -349,7 +349,7 @@ func TestClientsCustomUpstream(t *testing.T) {
 	clients := newClientsContainer(t)
 
 	// Add client with upstreams.
-	ok, err := clients.Add(&Client{
+	ok, err := clients.add(&persistentClient{
 		IDs:  []string{"1.1.1.1", "1:2:3::4", "aa:aa:aa:aa:aa:aa"},
 		Name: "client1",
 		Upstreams: []string{

internal/home/clientshttp.go

@@ -61,6 +61,7 @@ type clientJSON struct {
 	UpstreamsCacheEnabled aghalg.NullBool `json:"upstreams_cache_enabled"`
 }
 
+// runtimeClientJSON is a JSON representation of the [client.Runtime].
 type runtimeClientJSON struct {
 	WHOIS *whois.Info `json:"whois_info"`
 
@@ -69,6 +70,8 @@ type runtimeClientJSON struct {
 	Source client.Source `json:"source"`
 }
 
+// clientListJSON contains lists of persistent clients, runtime clients and also
+// supported tags.
 type clientListJSON struct {
 	Clients        []*clientJSON       `json:"clients"`
 	RuntimeClients []runtimeClientJSON `json:"auto_clients"`
@@ -126,32 +129,36 @@ func (clients *clientsContainer) handleGetClients(w http.ResponseWriter, r *http
 	aghhttp.WriteJSONResponseOK(w, r, data)
 }
 
-// jsonToClient converts JSON object to Client object.
+// initPrev initializes the persistent client with the default or previous
-func (clients *clientsContainer) jsonToClient(cj clientJSON, prev *Client) (c *Client, err error) {
+// client properties.
-	safeSearchConf := copySafeSearch(cj.SafeSearchConf, cj.SafeSearchEnabled)
+func initPrev(cj clientJSON, prev *persistentClient) (c *persistentClient, err error) {
+	var (
+		uid              UID
+		ignoreQueryLog   bool
+		ignoreStatistics bool
+		upsCacheEnabled  bool
+		upsCacheSize     uint32
+	)
+
+	if prev != nil {
+		uid = prev.UID
+		ignoreQueryLog = prev.IgnoreQueryLog
+		ignoreStatistics = prev.IgnoreStatistics
+		upsCacheEnabled = prev.UpstreamsCacheEnabled
+		upsCacheSize = prev.UpstreamsCacheSize
+	}
 
-	var ignoreQueryLog bool
 	if cj.IgnoreQueryLog != aghalg.NBNull {
 		ignoreQueryLog = cj.IgnoreQueryLog == aghalg.NBTrue
-	} else if prev != nil {
-		ignoreQueryLog = prev.IgnoreQueryLog
 	}
 
-	var ignoreStatistics bool
 	if cj.IgnoreStatistics != aghalg.NBNull {
 		ignoreStatistics = cj.IgnoreStatistics == aghalg.NBTrue
-	} else if prev != nil {
-		ignoreStatistics = prev.IgnoreStatistics
 	}
 
-	var upsCacheEnabled bool
-	var upsCacheSize uint32
 	if cj.UpstreamsCacheEnabled != aghalg.NBNull {
 		upsCacheEnabled = cj.UpstreamsCacheEnabled == aghalg.NBTrue
 		upsCacheSize = cj.UpstreamsCacheSize
-	} else if prev != nil {
-		upsCacheEnabled = prev.UpstreamsCacheEnabled
-		upsCacheSize = prev.UpstreamsCacheSize
 	}
 
 	svcs, err := copyBlockedServices(cj.Schedule, cj.BlockedServices, prev)
@@ -159,31 +166,49 @@ func (clients *clientsContainer) jsonToClient(cj clientJSON, prev *Client) (c *C
 		return nil, fmt.Errorf("invalid blocked services: %w", err)
 	}
 
-	c = &Client{
+	if (uid == UID{}) {
-		safeSearchConf: safeSearchConf,
+		uid, err = NewUID()
+		if err != nil {
+			return nil, fmt.Errorf("generating uid: %w", err)
+		}
+	}
 
-		Name: cj.Name,
+	return &persistentClient{
-
+		BlockedServices:       svcs,
-		BlockedServices: svcs,
+		UID:                   uid,
-
-		IDs:       cj.IDs,
-		Tags:      cj.Tags,
-		Upstreams: cj.Upstreams,
-
-		UseOwnSettings:        !cj.UseGlobalSettings,
-		FilteringEnabled:      cj.FilteringEnabled,
-		ParentalEnabled:       cj.ParentalEnabled,
-		SafeBrowsingEnabled:   cj.SafeBrowsingEnabled,
-		UseOwnBlockedServices: !cj.UseGlobalBlockedServices,
 		IgnoreQueryLog:        ignoreQueryLog,
 		IgnoreStatistics:      ignoreStatistics,
 		UpstreamsCacheEnabled: upsCacheEnabled,
 		UpstreamsCacheSize:    upsCacheSize,
+	}, nil
+}
+
+// jsonToClient converts JSON object to persistent client object if there are no
+// errors.
+func (clients *clientsContainer) jsonToClient(
+	cj clientJSON,
+	prev *persistentClient,
+) (c *persistentClient, err error) {
+	c, err = initPrev(cj, prev)
+	if err != nil {
+		// Don't wrap the error since it's informative enough as is.
+		return nil, err
 	}
 
-	if safeSearchConf.Enabled {
+	c.safeSearchConf = copySafeSearch(cj.SafeSearchConf, cj.SafeSearchEnabled)
+	c.Name = cj.Name
+	c.IDs = cj.IDs
+	c.Tags = cj.Tags
+	c.Upstreams = cj.Upstreams
+	c.UseOwnSettings = !cj.UseGlobalSettings
+	c.FilteringEnabled = cj.FilteringEnabled
+	c.ParentalEnabled = cj.ParentalEnabled
+	c.SafeBrowsingEnabled = cj.SafeBrowsingEnabled
+	c.UseOwnBlockedServices = !cj.UseGlobalBlockedServices
+
+	if c.safeSearchConf.Enabled {
 		err = c.setSafeSearch(
-			safeSearchConf,
+			c.safeSearchConf,
 			clients.safeSearchCacheSize,
 			clients.safeSearchCacheTTL,
 		)
@@ -228,7 +253,7 @@ func copySafeSearch(
 func copyBlockedServices(
 	sch *schedule.Weekly,
 	svcStrs []string,
-	prev *Client,
+	prev *persistentClient,
 ) (svcs *filtering.BlockedServices, err error) {
 	var weekly *schedule.Weekly
 	if sch != nil {
@@ -252,8 +277,8 @@ func copyBlockedServices(
 	return svcs, nil
 }
 
-// clientToJSON converts Client object to JSON.
+// clientToJSON converts persistent client object to JSON object.
-func clientToJSON(c *Client) (cj *clientJSON) {
+func clientToJSON(c *persistentClient) (cj *clientJSON) {
 	// TODO(d.kolyshev): Remove after cleaning the deprecated
 	// [clientJSON.SafeSearchEnabled] field.
 	cloneVal := c.safeSearchConf
@@ -302,7 +327,7 @@ func (clients *clientsContainer) handleAddClient(w http.ResponseWriter, r *http.
 		return
 	}
 
-	ok, err := clients.Add(c)
+	ok, err := clients.add(c)
 	if err != nil {
 		aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)
 
@@ -334,7 +359,7 @@ func (clients *clientsContainer) handleDelClient(w http.ResponseWriter, r *http.
 		return
 	}
 
-	if !clients.Del(cj.Name) {
+	if !clients.remove(cj.Name) {
 		aghhttp.Error(r, w, http.StatusBadRequest, "Client not found")
 
 		return
@@ -343,6 +368,7 @@ func (clients *clientsContainer) handleDelClient(w http.ResponseWriter, r *http.
 	onConfigModified()
 }
 
+// updateJSON contains the name and data of the updated persistent client.
 type updateJSON struct {
 	Name string     `json:"name"`
 	Data clientJSON `json:"data"`
@@ -366,7 +392,7 @@ func (clients *clientsContainer) handleUpdateClient(w http.ResponseWriter, r *ht
 		return
 	}
 
-	var prev *Client
+	var prev *persistentClient
 	var ok bool
 
 	func() {
@@ -389,7 +415,7 @@ func (clients *clientsContainer) handleUpdateClient(w http.ResponseWriter, r *ht
 		return
 	}
 
-	err = clients.Update(prev, c)
+	err = clients.update(prev, c)
 	if err != nil {
 		aghhttp.Error(r, w, http.StatusBadRequest, "%s", err)
 
@@ -410,7 +436,7 @@ func (clients *clientsContainer) handleFindClient(w http.ResponseWriter, r *http
 		}
 
 		ip, _ := netip.ParseAddr(idStr)
-		c, ok := clients.Find(idStr)
+		c, ok := clients.find(idStr)
 		var cj *clientJSON
 		if !ok {
 			cj = clients.findRuntime(ip, idStr)

internal/home/dns.go

@@ -411,9 +411,9 @@ func applyAdditionalFiltering(clientIP netip.Addr, clientID string, setts *filte
 
 	setts.ClientIP = clientIP
 
-	c, ok := Context.clients.Find(clientID)
+	c, ok := Context.clients.find(clientID)
 	if !ok {
-		c, ok = Context.clients.Find(clientIP.String())
+		c, ok = Context.clients.find(clientIP.String())
 		if !ok {
 			log.Debug("%s: no clients with ip %s and clientid %q", pref, clientIP, clientID)
 

internal/home/dns_internal_test.go

@@ -22,7 +22,7 @@ func TestApplyAdditionalFiltering(t *testing.T) {
 	}, nil)
 	require.NoError(t, err)
 
-	Context.clients.idIndex = map[string]*Client{
+	Context.clients.idIndex = map[string]*persistentClient{
 		"default": {
 			UseOwnSettings:      false,
 			safeSearchConf:      filtering.SafeSearchConfig{Enabled: false},
@@ -108,7 +108,7 @@ func TestApplyAdditionalFiltering_blockedServices(t *testing.T) {
 	}, nil)
 	require.NoError(t, err)
 
-	Context.clients.idIndex = map[string]*Client{
+	Context.clients.idIndex = map[string]*persistentClient{
 		"default": {
 			UseOwnBlockedServices: false,
 		},