From b30a045a4e63d6fbb554ea003f201ba846c2180c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Guillot?= Date: Mon, 19 Oct 2020 22:07:35 -0700 Subject: [PATCH] Refactor entry filtering Avoid looping multiple times across entries --- model/feed.go | 4 +- model/feed_test.go | 28 +++++++- reader/processor/processor.go | 62 +++++++++--------- reader/processor/processor_test.go | 101 +++++++++-------------------- 4 files changed, 90 insertions(+), 105 deletions(-) diff --git a/model/feed.go b/model/feed.go index 4f17f5f8..df251e0b 100644 --- a/model/feed.go +++ b/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 } diff --git a/model/feed_test.go b/model/feed_test.go index e8abf8dd..ca585bdb 100644 --- a/model/feed_test.go +++ b/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) { diff --git a/reader/processor/processor.go b/reader/processor/processor.go index 6b48c737..c06435a1 100644 --- a/reader/processor/processor.go +++ b/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 +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 + } + } + return false +} - 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 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 } - } else { - filteredEntries = feed.Entries + return false } - 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++ - } - } - filteredEntries = filteredEntries[:k] - } - feed.Entries = filteredEntries + return true } // ProcessEntryWebPage downloads the entry web page and apply rewrite rules. diff --git a/reader/processor/processor_test.go b/reader/processor/processor_test.go index 1c72609d..5052fb6b 100644 --- a/reader/processor/processor_test.go +++ b/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" + + "miniflux.app/model" ) -func TestKeeplistRules(t *testing.T) { - data := ` - - - SomeGood News - http://foo.bar/ - - Kitten News - http://kitties.today/daily-kitten - Kitten picture of the day. - Tue, 03 Jun 2003 09:39:21 GMT - http://kitties.today - - - Daily Covid DoomScrolling News - http://covid.doom/daily-panic-dose - Did you know that you can get COVID IN YOUR DREAMS?. - Tue, 03 Jun 2020 09:39:21 GMT - http://covid.doom - - - ` - - 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 := ` - - - SomeGood News - http://foo.bar/ - - Kitten News - http://kitties.today/daily-kitten - Kitten picture of the day. - Tue, 03 Jun 2003 09:39:21 GMT - http://kitties.today - - - Daily Covid DoomScrolling News - http://covid.doom/daily-panic-dose - Did you know that you can get COVID IN YOUR DREAMS?. - Tue, 03 Jun 2020 09:39:21 GMT - http://covid.doom - - - ` - - 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) + } } }