From 760e3596b6072d93421d3b8358cf85fbe706c2c3 Mon Sep 17 00:00:00 2001
From: Andrey Meshkov <am@adguard.com>
Date: Tue, 30 Oct 2018 11:01:09 +0300
Subject: [PATCH] Fix review comments Fixed coredns plugin tests Check that
 user filter is not empty

---
 app.go                                |  3 ++-
 config.go                             |  7 +++----
 coredns_plugin/coredns_plugin.go      |  2 +-
 coredns_plugin/coredns_plugin_test.go | 22 ++++++++++++++++------
 4 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/app.go b/app.go
index 78a995e7..e1d0abdf 100644
--- a/app.go
+++ b/app.go
@@ -149,6 +149,7 @@ func main() {
 		filter := &config.Filters[i]
 		err = filter.load()
 		if err != nil {
+			// This is okay for the first start, the filter will be loaded later
 			log.Printf("Couldn't load filter %d contents due to %s", filter.ID, err)
 		}
 	}
@@ -264,7 +265,7 @@ func upgradeConfig() error {
 	}
 
 	if config.SchemaVersion > SchemaVersion {
-		// Unexpected -- config file is newer than the
+		// Unexpected -- the config file is newer than we expect
 		return fmt.Errorf("configuration file is supposed to be used with a newer version of AdGuard Home, schema=%d", config.SchemaVersion)
 	}
 
diff --git a/config.go b/config.go
index 5ac55015..36337fd8 100644
--- a/config.go
+++ b/config.go
@@ -244,10 +244,9 @@ func generateCoreDNSConfigText() (string, error) {
 	// first of all, append the user filter
 	userFilter := getUserFilter()
 
-	// TODO: Don't add if empty
-	//if len(userFilter.contents) > 0 {
-	filters = append(filters, coreDnsFilter{ID: userFilter.ID, Path: userFilter.getFilterFilePath()})
-	//}
+	if len(userFilter.contents) > 0 {
+		filters = append(filters, coreDnsFilter{ID: userFilter.ID, Path: userFilter.getFilterFilePath()})
+	}
 
 	// then go through other filters
 	for i := range config.Filters {
diff --git a/coredns_plugin/coredns_plugin.go b/coredns_plugin/coredns_plugin.go
index e2d3f821..58c19604 100644
--- a/coredns_plugin/coredns_plugin.go
+++ b/coredns_plugin/coredns_plugin.go
@@ -185,7 +185,7 @@ func setupPlugin(c *caddy.Controller) (*plug, error) {
 			}
 			count++
 		}
-		log.Printf("Added %d rules from %d", count, filter.ID)
+		log.Printf("Added %d rules from filter ID=%d", count, filter.ID)
 
 		if err = scanner.Err(); err != nil {
 			return nil, err
diff --git a/coredns_plugin/coredns_plugin_test.go b/coredns_plugin/coredns_plugin_test.go
index f6b85e9d..1733fd6f 100644
--- a/coredns_plugin/coredns_plugin_test.go
+++ b/coredns_plugin/coredns_plugin_test.go
@@ -15,17 +15,26 @@ import (
 	"github.com/miekg/dns"
 )
 
-// TODO: Change tests -- there's new config template now
 func TestSetup(t *testing.T) {
 	for i, testcase := range []struct {
 		config  string
 		failing bool
 	}{
 		{`dnsfilter`, false},
-		{`dnsfilter /dev/nonexistent/abcdef`, true},
-		{`dnsfilter ../tests/dns.txt`, false},
-		{`dnsfilter ../tests/dns.txt { safebrowsing }`, false},
-		{`dnsfilter ../tests/dns.txt { parental }`, true},
+		{`dnsfilter { 
+					filter 0 /dev/nonexistent/abcdef
+				}`, true},
+		{`dnsfilter { 
+					filter 0 ../tests/dns.txt
+				}`, false},
+		{`dnsfilter { 
+					safebrowsing
+					filter 0 ../tests/dns.txt 
+				}`, false},
+		{`dnsfilter { 
+					parental
+					filter 0 ../tests/dns.txt
+				}`, true},
 	} {
 		c := caddy.NewTestController("dns", testcase.config)
 		err := setup(c)
@@ -56,7 +65,8 @@ func TestEtcHostsFilter(t *testing.T) {
 
 	defer os.Remove(tmpfile.Name())
 
-	c := caddy.NewTestController("dns", fmt.Sprintf("dnsfilter %s", tmpfile.Name()))
+	configText := fmt.Sprintf("dnsfilter {\nfilter 0 %s\n}", tmpfile.Name())
+	c := caddy.NewTestController("dns", configText)
 	p, err := setupPlugin(c)
 	if err != nil {
 		t.Fatal(err)