Fix review comments

Fixed coredns plugin tests
Check that user filter is not empty

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)
 	}
 

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 {
-	//if len(userFilter.contents) > 0 {
+		filters = append(filters, coreDnsFilter{ID: userFilter.ID, Path: userFilter.getFilterFilePath()})
-	filters = append(filters, coreDnsFilter{ID: userFilter.ID, Path: userFilter.getFilterFilePath()})
+	}
-	//}
 
 	// then go through other filters
 	for i := range config.Filters {

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

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 { 
-		{`dnsfilter ../tests/dns.txt`, false},
+					filter 0 /dev/nonexistent/abcdef
-		{`dnsfilter ../tests/dns.txt { safebrowsing }`, false},
+				}`, true},
-		{`dnsfilter ../tests/dns.txt { parental }`, 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)