From abb51ddb8ab1f1ec15e0ba648ba265fdccbff111 Mon Sep 17 00:00:00 2001
From: Andrey Meshkov <am@adguard.com>
Date: Mon, 29 Oct 2018 16:17:18 +0300
Subject: [PATCH] Add ErrAlreadyExists

---
 .gitignore                       | 7 +------
 control.go                       | 7 ++++---
 coredns_plugin/coredns_plugin.go | 6 ++++--
 dnsfilter/dnsfilter.go           | 8 +++++---
 dnsfilter/dnsfilter_test.go      | 6 ++++--
 5 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/.gitignore b/.gitignore
index a863c779..e22df4e9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -13,10 +13,5 @@ debug
 /querylog.json.1
 
 # Test output
-dnsfilter/dnsfilter.TestLotsOfRulesLotsOfHostsMemoryUsage1.pprof
-dnsfilter/dnsfilter.TestLotsOfRulesLotsOfHostsMemoryUsage2.pprof
-dnsfilter/dnsfilter.TestLotsOfRulesLotsOfHostsMemoryUsage3.pprof
-dnsfilter/dnsfilter.TestLotsOfRulesMemoryUsage1.pprof
-dnsfilter/dnsfilter.TestLotsOfRulesMemoryUsage2.pprof
-dnsfilter/dnsfilter.TestLotsOfRulesMemoryUsage3.pprof
+dnsfilter/dnsfilter.TestLotsOfRules*.pprof
 tests/top-1m.csv
diff --git a/control.go b/control.go
index 91eb8c68..1afd4e24 100644
--- a/control.go
+++ b/control.go
@@ -731,12 +731,13 @@ func (filter *filter) update(now time.Time) (bool, error) {
 			}
 		} else if len(line) != 0 {
 			err = d.AddRule(line, 0)
-			if err == dnsfilter.ErrInvalidSyntax {
+			if err == dnsfilter.ErrAlreadyExists || err == dnsfilter.ErrInvalidSyntax {
 				continue
 			}
 			if err != nil {
-				log.Printf("Couldn't add rule %s: %s", filter.URL, err)
-				return false, err
+				log.Printf("Cannot add rule %s from %s: %s", line, filter.URL, err)
+				// Just ignore invalid rules
+				continue
 			}
 			rulesCount++
 		}
diff --git a/coredns_plugin/coredns_plugin.go b/coredns_plugin/coredns_plugin.go
index d12031ae..31b147cf 100644
--- a/coredns_plugin/coredns_plugin.go
+++ b/coredns_plugin/coredns_plugin.go
@@ -149,11 +149,13 @@ func setupPlugin(c *caddy.Controller) (*plug, error) {
 			text := scanner.Text()
 
 			err = p.d.AddRule(text, uint32(i))
-			if err == dnsfilter.ErrInvalidSyntax {
+			if err == dnsfilter.ErrAlreadyExists || err == dnsfilter.ErrInvalidSyntax {
 				continue
 			}
 			if err != nil {
-				return nil, err
+				log.Printf("Cannot add rule %s: %s", text, err)
+				// Just ignore invalid rules
+				continue
 			}
 			count++
 		}
diff --git a/dnsfilter/dnsfilter.go b/dnsfilter/dnsfilter.go
index 49ee721c..1eaaeffa 100644
--- a/dnsfilter/dnsfilter.go
+++ b/dnsfilter/dnsfilter.go
@@ -33,9 +33,12 @@ const defaultSafebrowsingURL = "http://%s/safebrowsing-lookup-hash.html?prefixes
 const defaultParentalServer = "pctrl.adguard.com"
 const defaultParentalURL = "http://%s/check-parental-control-hash?prefixes=%s&sensitivity=%d"
 
-// ErrInvalidSyntax is returned by AddRule when rule is invalid
+// ErrInvalidSyntax is returned by AddRule when the rule is invalid
 var ErrInvalidSyntax = errors.New("dnsfilter: invalid rule syntax")
 
+// ErrInvalidSyntax is returned by AddRule when the rule was already added to the filter
+var ErrAlreadyExists = errors.New("dnsfilter: rule was already added")
+
 // ErrInvalidParental is returned by EnableParental when sensitivity is not a valid value
 var ErrInvalidParental = errors.New("dnsfilter: invalid parental sensitivity, must be either 3, 10, 13 or 17")
 
@@ -737,8 +740,7 @@ func (d *Dnsfilter) AddRule(input string, filterListID uint32) error {
 	d.storageMutex.RUnlock()
 	if exists {
 		// already added
-		// TODO: Change the error type
-		return ErrInvalidSyntax
+		return ErrAlreadyExists
 	}
 
 	if !isValidRule(input) {
diff --git a/dnsfilter/dnsfilter_test.go b/dnsfilter/dnsfilter_test.go
index 735d086e..f186ecce 100644
--- a/dnsfilter/dnsfilter_test.go
+++ b/dnsfilter/dnsfilter_test.go
@@ -261,7 +261,7 @@ func (d *Dnsfilter) checkAddRule(t *testing.T, rule string) {
 func (d *Dnsfilter) checkAddRuleFail(t *testing.T, rule string) {
 	t.Helper()
 	err := d.AddRule(rule, 0)
-	if err == ErrInvalidSyntax {
+	if err == ErrInvalidSyntax || err == ErrAlreadyExists {
 		return
 	}
 	if err != nil {
@@ -318,7 +318,7 @@ func loadTestRules(d *Dnsfilter) error {
 	for scanner.Scan() {
 		rule := scanner.Text()
 		err = d.AddRule(rule, 0)
-		if err == ErrInvalidSyntax {
+		if err == ErrInvalidSyntax || err == ErrAlreadyExists {
 			continue
 		}
 		if err != nil {
@@ -724,6 +724,7 @@ func BenchmarkAddRule(b *testing.B) {
 		err := d.AddRule(rule, 0)
 		switch err {
 		case nil:
+		case ErrAlreadyExists: // ignore rules which were already added
 		case ErrInvalidSyntax: // ignore invalid syntax
 		default:
 			b.Fatalf("Error while adding rule %s: %s", rule, err)
@@ -743,6 +744,7 @@ func BenchmarkAddRuleParallel(b *testing.B) {
 		}
 		switch err {
 		case nil:
+		case ErrAlreadyExists: // ignore rules which were already added
 		case ErrInvalidSyntax: // ignore invalid syntax
 		default:
 			b.Fatalf("Error while adding rule %s: %s", rule, err)