Refactor entry filtering

Avoid looping multiple times across entries

model/feed.go

@@ -74,7 +74,7 @@ func (f *Feed) WithCategoryID(categoryID int64) {
 }
 
 // WithBrowsingParameters defines browsing parameters.
-func (f *Feed) WithBrowsingParameters(crawler bool, userAgent, username, password, scraperRules, rewriteRules, blacklistRules, keeplistRules string, fetchViaProxy bool) {
+func (f *Feed) WithBrowsingParameters(crawler bool, userAgent, username, password, scraperRules, rewriteRules, blocklistRules, keeplistRules string, fetchViaProxy bool) {
 	f.Crawler = crawler
 	f.UserAgent = userAgent
 	f.Username = username
@@ -82,7 +82,7 @@ func (f *Feed) WithBrowsingParameters(crawler bool, userAgent, username, passwor
 	f.ScraperRules = scraperRules
 	f.RewriteRules = rewriteRules
 	f.FetchViaProxy = fetchViaProxy
-	f.BlocklistRules = blacklistRules
+	f.BlocklistRules = blocklistRules
 	f.KeeplistRules = keeplistRules
 }
 

model/feed_test.go

@@ -48,7 +48,17 @@ func TestFeedCategorySetter(t *testing.T) {
 
 func TestFeedBrowsingParams(t *testing.T) {
 	feed := &Feed{}
-	feed.WithBrowsingParameters(true, "Custom User Agent", "Username", "Secret", "Some Rule", "Another Rule", "Look a Rule", "Oh wow another Rule", false)
+	feed.WithBrowsingParameters(
+		true,
+		"Custom User Agent",
+		"Username",
+		"Secret",
+		"Scraper Rule",
+		"Rewrite Rule",
+		"Block Rule",
+		"Allow Rule",
+		true,
+	)
 
 	if !feed.Crawler {
 		t.Error(`The crawler must be activated`)
@@ -66,13 +76,25 @@ func TestFeedBrowsingParams(t *testing.T) {
 		t.Error(`The password must be set`)
 	}
 
-	if feed.ScraperRules != "Some Rule" {
+	if feed.ScraperRules != "Scraper Rule" {
 		t.Errorf(`The scraper rules must be set`)
 	}
 
-	if feed.RewriteRules != "Another Rule" {
+	if feed.RewriteRules != "Rewrite Rule" {
 		t.Errorf(`The rewrite rules must be set`)
 	}
+
+	if feed.BlocklistRules != "Block Rule" {
+		t.Errorf(`The block list rules must be set`)
+	}
+
+	if feed.KeeplistRules != "Allow Rule" {
+		t.Errorf(`The keep list rules must be set`)
+	}
+
+	if !feed.FetchViaProxy {
+		t.Errorf(`The fetch via proxy is no set`)
+	}
 }
 
 func TestFeedErrorCounter(t *testing.T) {

reader/processor/processor.go

@@ -20,13 +20,19 @@ import (
 
 // ProcessFeedEntries downloads original web page for entries and apply filters.
 func ProcessFeedEntries(store *storage.Storage, feed *model.Feed) {
-
-	filterFeedEntries(feed)
+	var filteredEntries model.Entries
 
 	for _, entry := range feed.Entries {
-		logger.Debug("[Feed #%d] Processing entry %s", feed.ID, entry.URL)
+		logger.Debug("[Processor] Processing entry %q from feed %q", entry.URL, feed.FeedURL)
+
+		if isBlockedEntry(feed, entry) || !isAllowedEntry(feed, entry) {
+			continue
+		}
+
 		if feed.Crawler {
 			if !store.EntryURLExists(feed.ID, entry.URL) {
+				logger.Debug("[Processor] Crawling entry %q from feed %q", entry.URL, feed.FeedURL)
+
 				startTime := time.Now()
 				content, scraperErr := scraper.Fetch(entry.URL, feed.ScraperRules, feed.UserAgent)
 
@@ -39,7 +45,7 @@ func ProcessFeedEntries(store *storage.Storage, feed *model.Feed) {
 				}
 
 				if scraperErr != nil {
-					logger.Error(`[Filter] Unable to crawl this entry: %q => %v`, entry.URL, scraperErr)
+					logger.Error(`[Processor] Unable to crawl this entry: %q => %v`, entry.URL, scraperErr)
 				} else if content != "" {
 					// We replace the entry content only if the scraper doesn't return any error.
 					entry.Content = content
@@ -51,38 +57,34 @@ func ProcessFeedEntries(store *storage.Storage, feed *model.Feed) {
 
 		// The sanitizer should always run at the end of the process to make sure unsafe HTML is filtered.
 		entry.Content = sanitizer.Sanitize(entry.URL, entry.Content)
+
+		filteredEntries = append(filteredEntries, entry)
 	}
+
+	feed.Entries = filteredEntries
 }
 
-/*
-Filters feed entries based on regex rules
-First we filter based on our keep list, then we remove those entries that match the block list
-*/
-func filterFeedEntries(feed *model.Feed) {
-	var filteredEntries []*model.Entry
-
-	if len(feed.KeeplistRules) > 0 {
-		for _, entry := range feed.Entries {
-			match, _ := regexp.MatchString(feed.KeeplistRules, entry.Title)
-			if match == true {
-				filteredEntries = append(filteredEntries, entry)
-			}
+func isBlockedEntry(feed *model.Feed, entry *model.Entry) bool {
+	if feed.BlocklistRules != "" {
+		match, _ := regexp.MatchString(feed.BlocklistRules, entry.Title)
+		if match {
+			logger.Debug("[Processor] Blocking entry %q from feed %q based on rule %q", entry.Title, feed.FeedURL, feed.BlocklistRules)
+			return true
 		}
-	} else {
-		filteredEntries = feed.Entries
 	}
-	if len(feed.BlocklistRules) > 0 {
-		k := 0
-		for _, entry := range filteredEntries {
-			match, _ := regexp.MatchString(feed.BlocklistRules, entry.Title)
-			if match != true {
-				filteredEntries[k] = entry
-				k++
-			}
+	return false
+}
+
+func isAllowedEntry(feed *model.Feed, entry *model.Entry) bool {
+	if feed.KeeplistRules != "" {
+		match, _ := regexp.MatchString(feed.KeeplistRules, entry.Title)
+		if match {
+			logger.Debug("[Processor] Allow entry %q from feed %q based on rule %q", entry.Title, feed.FeedURL, feed.KeeplistRules)
+			return true
 		}
-		filteredEntries = filteredEntries[:k]
+		return false
 	}
-	feed.Entries = filteredEntries
+	return true
 }
 
 // ProcessEntryWebPage downloads the entry web page and apply rewrite rules.

reader/processor/processor_test.go

@@ -1,88 +1,49 @@
-// Copyright 2017 Frédéric Guillot. All rights reserved.
+// Copyright 2020 Frédéric Guillot. All rights reserved.
 // Use of this source code is governed by the Apache 2.0
 // license that can be found in the LICENSE file.
 
 package processor // import "miniflux.app/reader/processor"
 
 import (
-	"miniflux.app/reader/parser"
 	"testing"
-)
 
-func TestKeeplistRules(t *testing.T) {
-	data := `<?xml version="1.0"?>
-	<rss version="2.0">
-	<channel>
-		<title>SomeGood News</title>
-		<link>http://foo.bar/</link>
-		<item>
-			<title>Kitten News</title>
-			<link>http://kitties.today/daily-kitten</link>
-			<description>Kitten picture of the day.</description>
-			<pubDate>Tue, 03 Jun 2003 09:39:21 GMT</pubDate>
-			<guid>http://kitties.today</guid>
-		</item>
-		<item>
-			<title>Daily Covid DoomScrolling News</title>
-			<link>http://covid.doom/daily-panic-dose</link>
-			<description>Did you know that you can get COVID IN YOUR DREAMS?.</description>
-			<pubDate>Tue, 03 Jun 2020 09:39:21 GMT</pubDate>
-			<guid>http://covid.doom</guid>
-		</item>
-	</channel>
-	</rss>`
+	"miniflux.app/model"
+)
 
-	feed, err := parser.ParseFeed(data)
-	if err != nil {
-		t.Error(err)
-	}
-	if len(feed.Entries) != 2 {
-		t.Errorf("Error parsing feed")
+func TestBlockingEntries(t *testing.T) {
+	var scenarios = []struct {
+		feed     *model.Feed
+		entry    *model.Entry
+		expected bool
+	}{
+		{&model.Feed{ID: 1, BlocklistRules: "(?i)example"}, &model.Entry{Title: "Some Example"}, true},
+		{&model.Feed{ID: 1, BlocklistRules: "(?i)example"}, &model.Entry{Title: "Something different"}, false},
+		{&model.Feed{ID: 1}, &model.Entry{Title: "No rule defined"}, false},
 	}
 
-	//case insensitive
-	feed.KeeplistRules = "(?i)kitten"
-	filterFeedEntries(feed)
-	if len(feed.Entries) != 1 {
-		t.Errorf("Keeplist filter rule did not properly filter the feed")
+	for _, tc := range scenarios {
+		result := isBlockedEntry(tc.feed, tc.entry)
+		if tc.expected != result {
+			t.Errorf(`Unexpected result, got %v for entry %q`, result, tc.entry.Title)
+		}
 	}
 }
 
-func TestBlocklistRules(t *testing.T) {
-	data := `<?xml version="1.0"?>
-	<rss version="2.0">
-	<channel>
-		<title>SomeGood News</title>
-		<link>http://foo.bar/</link>
-		<item>
-			<title>Kitten News</title>
-			<link>http://kitties.today/daily-kitten</link>
-			<description>Kitten picture of the day.</description>
-			<pubDate>Tue, 03 Jun 2003 09:39:21 GMT</pubDate>
-			<guid>http://kitties.today</guid>
-		</item>
-		<item>
-			<title>Daily Covid DoomScrolling News</title>
-			<link>http://covid.doom/daily-panic-dose</link>
-			<description>Did you know that you can get COVID IN YOUR DREAMS?.</description>
-			<pubDate>Tue, 03 Jun 2020 09:39:21 GMT</pubDate>
-			<guid>http://covid.doom</guid>
-		</item>
-	</channel>
-	</rss>`
-
-	feed, err := parser.ParseFeed(data)
-	if err != nil {
-		t.Error(err)
-	}
-	if len(feed.Entries) != 2 {
-		t.Errorf("Error parsing feed")
+func TestAllowEntries(t *testing.T) {
+	var scenarios = []struct {
+		feed     *model.Feed
+		entry    *model.Entry
+		expected bool
+	}{
+		{&model.Feed{ID: 1, KeeplistRules: "(?i)example"}, &model.Entry{Title: "Some Example"}, true},
+		{&model.Feed{ID: 1, KeeplistRules: "(?i)example"}, &model.Entry{Title: "Something different"}, false},
+		{&model.Feed{ID: 1}, &model.Entry{Title: "No rule defined"}, true},
 	}
 
-	//case insensitive
-	feed.BlocklistRules = "(?i)covid"
-	filterFeedEntries(feed)
-	if len(feed.Entries) != 1 {
-		t.Errorf("Keeplist filter rule did not properly filter the feed")
+	for _, tc := range scenarios {
+		result := isAllowedEntry(tc.feed, tc.entry)
+		if tc.expected != result {
+			t.Errorf(`Unexpected result, got %v for entry %q`, result, tc.entry.Title)
+		}
 	}
 }