From 1a2f85f701522055f358cfda3cdd3ce580d5e403 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Wed, 7 Dec 2022 10:51:33 +0000 Subject: [PATCH 01/13] Small code cleanups for feed manager, don't fetch new feeds twice. --- .../briar/android/blog/BaseViewModel.java | 3 +- .../briar/feed/FeedManagerImpl.java | 97 ++++++++----------- .../briarproject/briar/util/HtmlUtils.java | 11 ++- .../briar/feed/FeedManagerImplTest.java | 6 +- 4 files changed, 53 insertions(+), 64 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseViewModel.java index e4360d81c..a1c44865d 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseViewModel.java @@ -41,7 +41,6 @@ import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.now; -import static org.briarproject.briar.util.HtmlUtils.ARTICLE; @NotNullByDefault abstract class BaseViewModel extends DbViewModel implements EventListener { @@ -115,7 +114,7 @@ abstract class BaseViewModel extends DbViewModel implements EventListener { @DatabaseExecutor private String getPostText(Transaction txn, MessageId m) throws DbException { - return HtmlUtils.clean(blogManager.getPostText(txn, m), ARTICLE); + return HtmlUtils.cleanArticle(blogManager.getPostText(txn, m)); } LiveData> loadBlogPost(GroupId g, MessageId m) { diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java index b200ec451..dd1bada2e 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java @@ -31,7 +31,6 @@ import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.bramble.api.system.Clock; import org.briarproject.bramble.api.system.TaskScheduler; import org.briarproject.bramble.api.system.Wakeful; -import org.briarproject.bramble.util.StringUtils; import org.briarproject.briar.api.blog.Blog; import org.briarproject.briar.api.blog.BlogManager; import org.briarproject.briar.api.blog.BlogManager.RemoveBlogHook; @@ -47,12 +46,12 @@ import java.security.GeneralSecurityException; import java.util.ArrayList; import java.util.Comparator; import java.util.Date; +import java.util.Iterator; import java.util.List; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Logger; -import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; import javax.inject.Inject; @@ -64,14 +63,15 @@ import okhttp3.ResponseBody; import static java.util.Collections.sort; import static java.util.logging.Level.WARNING; import static org.briarproject.bramble.util.LogUtils.logException; +import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty; +import static org.briarproject.bramble.util.StringUtils.truncateUtf8; import static org.briarproject.briar.api.blog.BlogConstants.MAX_BLOG_POST_TEXT_LENGTH; import static org.briarproject.briar.api.feed.FeedConstants.FETCH_DELAY_INITIAL; import static org.briarproject.briar.api.feed.FeedConstants.FETCH_INTERVAL; import static org.briarproject.briar.api.feed.FeedConstants.FETCH_UNIT; import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEEDS; -import static org.briarproject.briar.util.HtmlUtils.ARTICLE; -import static org.briarproject.briar.util.HtmlUtils.STRIP_ALL; -import static org.briarproject.briar.util.HtmlUtils.clean; +import static org.briarproject.briar.util.HtmlUtils.cleanAll; +import static org.briarproject.briar.util.HtmlUtils.cleanArticle; @ThreadSafe @NotNullByDefault @@ -160,12 +160,12 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, @Override public Feed addFeed(String url) throws DbException, IOException { - // fetch syndication feed to get its metadata - SyndFeed f = fetchSyndFeed(url); + // fetch feed to get posts and metadata + SyndFeed sf = fetchSyndFeed(url); - Feed feed = feedFactory.createFeed(url, f); + Feed feed = feedFactory.createFeed(url, sf); - // store feed and new blog + // store feed metadata and new blog Transaction txn = db.startTransaction(false); try { blogManager.addBlog(txn, feed.getBlog()); @@ -177,10 +177,11 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, db.endTransaction(txn); } - // fetch feed again and post entries - Feed updatedFeed = fetchFeed(feed); + // post entries + long lastEntryTime = postFeedEntries(feed, sf.getEntries()); + Feed updatedFeed = feedFactory.createFeed(feed, sf, lastEntryTime); - // store feed again to also store last added entry + // store feed metadata again to also store last entry time txn = db.startTransaction(false); try { List feeds = getFeeds(txn); @@ -215,10 +216,12 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, // delete blog's RSS feed if we have it boolean found = false; List feeds = getFeeds(txn); - for (Feed f : feeds) { + Iterator it = feeds.iterator(); + while (it.hasNext()) { + Feed f = it.next(); if (f.getBlogId().equals(b.getId())) { + it.remove(); found = true; - feeds.remove(f); break; } } @@ -248,7 +251,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, return feeds; } - private void storeFeeds(@Nullable Transaction txn, List feeds) + private void storeFeeds(Transaction txn, List feeds) throws DbException { BdfList feedList = new BdfList(); @@ -257,19 +260,14 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, } BdfDictionary gm = BdfDictionary.of(new BdfEntry(KEY_FEEDS, feedList)); try { - if (txn == null) { - clientHelper.mergeGroupMetadata(getLocalGroup().getId(), gm); - } else { - clientHelper.mergeGroupMetadata(txn, getLocalGroup().getId(), - gm); - } + clientHelper.mergeGroupMetadata(txn, getLocalGroup().getId(), gm); } catch (FormatException e) { throw new DbException(e); } } private void storeFeeds(List feeds) throws DbException { - storeFeeds(null, feeds); + db.transaction(false, txn -> storeFeeds(txn, feeds)); } /** @@ -299,7 +297,11 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, List newFeeds = new ArrayList<>(feeds.size()); for (Feed feed : feeds) { try { - newFeeds.add(fetchFeed(feed)); + // fetch and clean feed + SyndFeed sf = fetchSyndFeed(feed.getUrl()); + // sort and add new entries + long lastEntryTime = postFeedEntries(feed, sf.getEntries()); + newFeeds.add(feedFactory.createFeed(feed, sf, lastEntryTime)); } catch (IOException | DbException e) { logException(LOG, WARNING, e); newFeeds.add(feed); @@ -318,42 +320,25 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, private SyndFeed fetchSyndFeed(String url) throws IOException { // fetch feed InputStream stream = getFeedInputStream(url); - SyndFeed f = getSyndFeed(stream); + SyndFeed sf = getSyndFeed(stream); stream.close(); - if (f.getEntries().size() == 0) - throw new IOException("Feed has no entries"); - // clean title - String title = - StringUtils.isNullOrEmpty(f.getTitle()) ? null : f.getTitle(); - if (title != null) title = clean(title, STRIP_ALL); - f.setTitle(title); + String title = sf.getTitle(); + if (title != null) title = cleanAll(title); + sf.setTitle(isNullOrEmpty(title) ? null : title); // clean description - String description = - StringUtils.isNullOrEmpty(f.getDescription()) ? null : - f.getDescription(); - if (description != null) description = clean(description, STRIP_ALL); - f.setDescription(description); + String description = sf.getDescription(); + if (description != null) description = cleanAll(description); + sf.setDescription(isNullOrEmpty(description) ? null : description); // clean author - String author = - StringUtils.isNullOrEmpty(f.getAuthor()) ? null : f.getAuthor(); - if (author != null) author = clean(author, STRIP_ALL); - f.setAuthor(author); + String author = sf.getAuthor(); + if (author != null) author = cleanAll(author); + sf.setAuthor(isNullOrEmpty(author) ? null : author); - return f; - } - - private Feed fetchFeed(Feed feed) throws IOException, DbException { - // fetch and clean feed - SyndFeed f = fetchSyndFeed(feed.getUrl()); - - // sort and add new entries - long lastEntryTime = postFeedEntries(feed, f.getEntries()); - - return feedFactory.createFeed(feed, f, lastEntryTime); + return sf; } private InputStream getFeedInputStream(String url) throws IOException { @@ -417,7 +402,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, // build post text StringBuilder b = new StringBuilder(); - if (!StringUtils.isNullOrEmpty(entry.getTitle())) { + if (!isNullOrEmpty(entry.getTitle())) { b.append("

").append(entry.getTitle()).append("

"); } for (SyndContent content : entry.getContents()) { @@ -430,7 +415,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, b.append(entry.getDescription().getValue()); } b.append("

"); - if (!StringUtils.isNullOrEmpty(entry.getAuthor())) { + if (!isNullOrEmpty(entry.getAuthor())) { b.append("-- ").append(entry.getAuthor()); } if (entry.getPublishedDate() != null) { @@ -442,7 +427,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, } b.append("

"); String link = entry.getLink(); - if (!StringUtils.isNullOrEmpty(link)) { + if (!isNullOrEmpty(link)) { b.append("").append(link) .append(""); } @@ -472,8 +457,8 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, } private String getPostText(String text) { - text = clean(text, ARTICLE); - return StringUtils.truncateUtf8(text, MAX_BLOG_POST_TEXT_LENGTH); + text = cleanArticle(text); + return truncateUtf8(text, MAX_BLOG_POST_TEXT_LENGTH); } private Comparator getEntryComparator() { diff --git a/briar-core/src/main/java/org/briarproject/briar/util/HtmlUtils.java b/briar-core/src/main/java/org/briarproject/briar/util/HtmlUtils.java index 37c5775a2..fb01ca1b2 100644 --- a/briar-core/src/main/java/org/briarproject/briar/util/HtmlUtils.java +++ b/briar-core/src/main/java/org/briarproject/briar/util/HtmlUtils.java @@ -7,12 +7,15 @@ import org.jsoup.safety.Safelist; @NotNullByDefault public class HtmlUtils { - public static Safelist STRIP_ALL = Safelist.none(); - public static Safelist ARTICLE = Safelist.basic() + private static final Safelist STRIP_ALL = Safelist.none(); + private static final Safelist ARTICLE = Safelist.basic() .addTags("h1", "h2", "h3", "h4", "h5", "h6"); - public static String clean(String s, Safelist list) { - return Jsoup.clean(s, list); + public static String cleanAll(String s) { + return Jsoup.clean(s, STRIP_ALL); } + public static String cleanArticle(String s) { + return Jsoup.clean(s, ARTICLE); + } } diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java index 099ae1921..984ca0d56 100644 --- a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java @@ -175,11 +175,13 @@ public class FeedManagerImplTest extends BrambleMockTestCase { } private void expectStoreFeed(BdfList feedList) throws Exception { + Transaction txn = new Transaction(null, false); BdfDictionary feedDict = BdfDictionary.of(new BdfEntry(KEY_FEEDS, feedList)); expectGetLocalGroup(); - context.checking(new Expectations() {{ - oneOf(clientHelper).mergeGroupMetadata(localGroupId, feedDict); + context.checking(new DbExpectations() {{ + oneOf(db).transaction(with(false), withDbRunnable(txn)); + oneOf(clientHelper).mergeGroupMetadata(txn, localGroupId, feedDict); if (feedList.size() == 1) { oneOf(feedFactory).feedToBdfDictionary(feed); will(returnValue(feedList.getDictionary(0))); From b920382e44ee8f5240a227056007bc3bc492c255 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 9 Dec 2022 17:57:20 +0000 Subject: [PATCH 02/13] Store additional properties of RSS feed in metadata. --- .../briar/android/blog/RssFeedAdapter.java | 11 ++- .../briar/android/blog/RssFeedViewModel.java | 4 +- .../org/briarproject/briar/api/feed/Feed.java | 40 ++------- .../briar/api/feed/FeedConstants.java | 3 + .../briar/api/feed/RssProperties.java | 86 +++++++++++++++++++ .../briar/feed/FeedFactoryImpl.java | 45 +++++++--- .../briar/feed/FeedManagerImpl.java | 4 +- .../briar/feed/FeedManagerImplTest.java | 6 +- .../feed/FeedManagerIntegrationTest.java | 2 +- 9 files changed, 146 insertions(+), 55 deletions(-) create mode 100644 briar-api/src/main/java/org/briarproject/briar/api/feed/RssProperties.java diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedAdapter.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedAdapter.java index 4ef04ffe0..a3b6c7790 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedAdapter.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedAdapter.java @@ -28,8 +28,7 @@ class RssFeedAdapter extends ListAdapter { super(new DiffUtil.ItemCallback() { @Override public boolean areItemsTheSame(Feed a, Feed b) { - return a.getUrl().equals(b.getUrl()) && - a.getBlogId().equals(b.getBlogId()) && + return a.getBlogId().equals(b.getBlogId()) && a.getAdded() == b.getAdded(); } @@ -86,8 +85,8 @@ class RssFeedAdapter extends ListAdapter { delete.setOnClickListener(v -> listener.onDeleteClick(item)); // Author - if (item.getRssAuthor() != null) { - author.setText(item.getRssAuthor()); + if (item.getProperties().getAuthor() != null) { + author.setText(item.getProperties().getAuthor()); author.setVisibility(VISIBLE); authorLabel.setVisibility(VISIBLE); } else { @@ -100,8 +99,8 @@ class RssFeedAdapter extends ListAdapter { updated.setText(formatDate(ctx, item.getUpdated())); // Description - if (item.getDescription() != null) { - description.setText(item.getDescription()); + if (item.getProperties().getDescription() != null) { + description.setText(item.getProperties().getDescription()); description.setVisibility(VISIBLE); } else { description.setVisibility(GONE); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java index 785d34c46..ddd673c01 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java @@ -168,7 +168,9 @@ class RssFeedViewModel extends DbViewModel { List list = getList(feeds); if (list != null) { for (Feed feed : list) { - if (url.equals(feed.getUrl())) { + // TODO: Fetch the feed and also match it against feeds that + // were imported from files? + if (url.equals(feed.getProperties().getUrl())) { return true; } } diff --git a/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java b/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java index 18a469c72..f743a8221 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java @@ -5,47 +5,27 @@ import org.briarproject.bramble.api.sync.GroupId; import org.briarproject.briar.api.blog.Blog; import org.briarproject.nullsafety.NotNullByDefault; -import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @Immutable @NotNullByDefault public class Feed implements Comparable { - private final String url; private final Blog blog; private final LocalAuthor localAuthor; - @Nullable - private final String description, rssAuthor; + private final RssProperties properties; private final long added, updated, lastEntryTime; - public Feed(String url, Blog blog, LocalAuthor localAuthor, - @Nullable String description, @Nullable String rssAuthor, + public Feed(Blog blog, LocalAuthor localAuthor, RssProperties properties, long added, long updated, long lastEntryTime) { - this.url = url; this.blog = blog; this.localAuthor = localAuthor; - this.description = description; - this.rssAuthor = rssAuthor; + this.properties = properties; this.added = added; this.updated = updated; this.lastEntryTime = lastEntryTime; } - public Feed(String url, Blog blog, LocalAuthor localAuthor, - @Nullable String description, @Nullable String rssAuthor, - long added) { - this(url, blog, localAuthor, description, rssAuthor, added, 0L, 0L); - } - - public Feed(String url, Blog blog, LocalAuthor localAuthor, long added) { - this(url, blog, localAuthor, null, null, added, 0L, 0L); - } - - public String getUrl() { - return url; - } - public GroupId getBlogId() { return blog.getId(); } @@ -62,14 +42,8 @@ public class Feed implements Comparable { return blog.getName(); } - @Nullable - public String getDescription() { - return description; - } - - @Nullable - public String getRssAuthor() { - return rssAuthor; + public RssProperties getProperties() { + return properties; } public long getAdded() { @@ -103,4 +77,8 @@ public class Feed implements Comparable { return 0; } + @Override + public int hashCode() { + return blog.hashCode(); + } } diff --git a/briar-api/src/main/java/org/briarproject/briar/api/feed/FeedConstants.java b/briar-api/src/main/java/org/briarproject/briar/api/feed/FeedConstants.java index b165c2a02..7bf4c9ee3 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/feed/FeedConstants.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/feed/FeedConstants.java @@ -22,6 +22,9 @@ public interface FeedConstants { String KEY_FEED_PRIVATE_KEY = "feedPrivateKey"; String KEY_FEED_DESC = "feedDesc"; String KEY_FEED_RSS_AUTHOR = "feedRssAuthor"; + String KEY_FEED_RSS_TITLE = "feedRssTitle"; + String KEY_FEED_RSS_LINK = "feedRssLink"; + String KEY_FEED_RSS_URI = "feedRssUri"; String KEY_FEED_ADDED = "feedAdded"; String KEY_FEED_UPDATED = "feedUpdated"; String KEY_FEED_LAST_ENTRY = "feedLastEntryTime"; diff --git a/briar-api/src/main/java/org/briarproject/briar/api/feed/RssProperties.java b/briar-api/src/main/java/org/briarproject/briar/api/feed/RssProperties.java new file mode 100644 index 000000000..b5ce9664c --- /dev/null +++ b/briar-api/src/main/java/org/briarproject/briar/api/feed/RssProperties.java @@ -0,0 +1,86 @@ +package org.briarproject.briar.api.feed; + +import org.briarproject.nullsafety.NotNullByDefault; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; + +/** + * The properties of an RSS feed, which may have been imported from a URL + * or a file. + */ +@Immutable +@NotNullByDefault +public class RssProperties { + + @Nullable + private final String url, title, description, author, link, uri; + + public RssProperties(@Nullable String url, @Nullable String title, + @Nullable String description, @Nullable String author, + @Nullable String link, @Nullable String uri) { + this.url = url; + this.title = title; + this.description = description; + this.author = author; + this.link = link; + this.uri = uri; + } + + /** + * Returns the URL from which the RSS feed was imported, or null if the + * feed was imported from a file. + */ + @Nullable + public String getUrl() { + return url; + } + + /** + * Returns the title property of the RSS feed, or null if no title was + * specified. + */ + @Nullable + public String getTitle() { + return title; + } + + /** + * Returns the description property of the RSS feed, or null if no + * description was specified. + */ + @Nullable + public String getDescription() { + return description; + } + + /** + * Returns the author property of the RSS feed, or null if no author was + * specified. + */ + @Nullable + public String getAuthor() { + return author; + } + + /** + * Returns the link property of the RSS feed, or null if no link was + * specified. This is usually the URL of a webpage where the equivalent + * content can be viewed in a browser. + */ + @Nullable + public String getLink() { + return link; + } + + /** + * Returns the URI property of the RSS feed, or null if no URI was + * specified. This may be a URL from which the feed can be downloaded, + * or it may be an opaque identifier such as a number that serves to + * distinguish this feed from other feeds produced by the same creator. + */ + @Nullable + public String getUri() { + return uri; + } +} diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java index b8e9f2740..b688fab96 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java @@ -17,6 +17,7 @@ import org.briarproject.bramble.util.StringUtils; import org.briarproject.briar.api.blog.Blog; import org.briarproject.briar.api.blog.BlogFactory; import org.briarproject.briar.api.feed.Feed; +import org.briarproject.briar.api.feed.RssProperties; import javax.inject.Inject; @@ -27,6 +28,9 @@ import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_DESC; import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_LAST_ENTRY; import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_PRIVATE_KEY; import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_RSS_AUTHOR; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_RSS_LINK; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_RSS_TITLE; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_RSS_URI; import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_UPDATED; import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_URL; @@ -56,20 +60,20 @@ class FeedFactoryImpl implements FeedFactory { Blog blog = blogFactory.createFeedBlog(localAuthor); long added = clock.currentTimeMillis(); - return new Feed(url, blog, localAuthor, added); + RssProperties properties = + new RssProperties(url, null, null, null, null, null); + return new Feed(blog, localAuthor, properties, added, 0, 0); } @Override public Feed createFeed(Feed feed, SyndFeed f, long lastEntryTime) { long updated = clock.currentTimeMillis(); - return new Feed(feed.getUrl(), feed.getBlog(), feed.getLocalAuthor(), - f.getDescription(), f.getAuthor(), feed.getAdded(), updated, - lastEntryTime); + return new Feed(feed.getBlog(), feed.getLocalAuthor(), + feed.getProperties(), feed.getAdded(), updated, lastEntryTime); } @Override public Feed createFeed(BdfDictionary d) throws FormatException { - String url = d.getString(KEY_FEED_URL); BdfList authorList = d.getList(KEY_FEED_AUTHOR); PrivateKey privateKey = @@ -80,14 +84,21 @@ class FeedFactoryImpl implements FeedFactory { author.getPublicKey(), privateKey); Blog blog = blogFactory.createFeedBlog(localAuthor); - String desc = d.getOptionalString(KEY_FEED_DESC); + String url = d.getOptionalString(KEY_FEED_URL); + String description = d.getOptionalString(KEY_FEED_DESC); String rssAuthor = d.getOptionalString(KEY_FEED_RSS_AUTHOR); + String title = d.getOptionalString(KEY_FEED_RSS_TITLE); + String link = d.getOptionalString(KEY_FEED_RSS_LINK); + String uri = d.getOptionalString(KEY_FEED_RSS_URI); + RssProperties properties = new RssProperties(url, title, description, + rssAuthor, link, uri); + long added = d.getLong(KEY_FEED_ADDED, 0L); long updated = d.getLong(KEY_FEED_UPDATED, 0L); long lastEntryTime = d.getLong(KEY_FEED_LAST_ENTRY, 0L); - return new Feed(url, blog, localAuthor, desc, rssAuthor, added, - updated, lastEntryTime); + return new Feed(blog, localAuthor, properties, added, updated, + lastEntryTime); } @Override @@ -95,17 +106,25 @@ class FeedFactoryImpl implements FeedFactory { LocalAuthor localAuthor = feed.getLocalAuthor(); BdfList authorList = clientHelper.toList(localAuthor); BdfDictionary d = BdfDictionary.of( - new BdfEntry(KEY_FEED_URL, feed.getUrl()), new BdfEntry(KEY_FEED_AUTHOR, authorList), new BdfEntry(KEY_FEED_PRIVATE_KEY, localAuthor.getPrivateKey()), new BdfEntry(KEY_FEED_ADDED, feed.getAdded()), new BdfEntry(KEY_FEED_UPDATED, feed.getUpdated()), new BdfEntry(KEY_FEED_LAST_ENTRY, feed.getLastEntryTime()) ); - if (feed.getDescription() != null) - d.put(KEY_FEED_DESC, feed.getDescription()); - if (feed.getRssAuthor() != null) - d.put(KEY_FEED_RSS_AUTHOR, feed.getRssAuthor()); + RssProperties properties = feed.getProperties(); + if (properties.getUrl() != null) + d.put(KEY_FEED_URL, properties.getUrl()); + if (properties.getDescription() != null) + d.put(KEY_FEED_DESC, properties.getDescription()); + if (properties.getAuthor() != null) + d.put(KEY_FEED_RSS_AUTHOR, properties.getAuthor()); + if (properties.getTitle() != null) + d.put(KEY_FEED_RSS_TITLE, properties.getTitle()); + if (properties.getLink() != null) + d.put(KEY_FEED_RSS_LINK, properties.getLink()); + if (properties.getUri() != null) + d.put(KEY_FEED_RSS_URI, properties.getUri()); return d; } diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java index dd1bada2e..2a2895f85 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java @@ -297,8 +297,10 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, List newFeeds = new ArrayList<>(feeds.size()); for (Feed feed : feeds) { try { + String url = feed.getProperties().getUrl(); + if (url == null) continue; // fetch and clean feed - SyndFeed sf = fetchSyndFeed(feed.getUrl()); + SyndFeed sf = fetchSyndFeed(url); // sort and add new entries long lastEntryTime = postFeedEntries(feed, sf.getEntries()); newFeeds.add(feedFactory.createFeed(feed, sf, lastEntryTime)); diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java index 984ca0d56..5116c4a9b 100644 --- a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java @@ -25,6 +25,7 @@ import org.briarproject.briar.api.blog.BlogManager; import org.briarproject.briar.api.blog.BlogPost; import org.briarproject.briar.api.blog.BlogPostFactory; import org.briarproject.briar.api.feed.Feed; +import org.briarproject.briar.api.feed.RssProperties; import org.jmock.Expectations; import org.junit.Test; @@ -83,8 +84,9 @@ public class FeedManagerImplTest extends BrambleMockTestCase { private final GroupId blogGroupId = blogGroup.getId(); private final LocalAuthor localAuthor = getLocalAuthor(); private final Blog blog = new Blog(blogGroup, localAuthor, true); - private final Feed feed = - new Feed("http://example.org", blog, localAuthor, 0); + private final RssProperties properties = new RssProperties( + "http://example.org", null, null, null, null, null); + private final Feed feed = new Feed(blog, localAuthor, properties, 0, 0, 0); private final BdfDictionary feedDict = new BdfDictionary(); private final FeedManagerImpl feedManager = diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTest.java index 5e00e3b9e..56aa1637a 100644 --- a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTest.java @@ -80,7 +80,7 @@ public class FeedManagerIntegrationTest extends BrambleTestCase { assertTrue(feed.getLastEntryTime() > 0); assertTrue(feed.getAdded() > 0); assertTrue(feed.getUpdated() > 0); - assertEquals(url, feed.getUrl()); + assertEquals(url, feed.getProperties().getUrl()); assertEquals(feedBlog, feed.getBlog()); assertEquals("Schneier on Security", feed.getTitle()); assertEquals(feed.getTitle(), feed.getBlog().getName()); From 33d01aac8c8c65b009e51b8bce7b3f28b2cf956e Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 9 Dec 2022 17:57:53 +0000 Subject: [PATCH 03/13] Add matcher for matching an imported feed against existing feeds. --- .../org/briarproject/briar/api/feed/Feed.java | 1 + .../briarproject/briar/feed/FeedFactory.java | 4 +- .../briar/feed/FeedFactoryImpl.java | 26 +-- .../briar/feed/FeedManagerImpl.java | 10 +- .../briarproject/briar/feed/FeedMatcher.java | 22 +++ .../briar/feed/FeedMatcherImpl.java | 64 ++++++ .../briarproject/briar/feed/FeedModule.java | 5 + .../briar/feed/FeedMatcherImplTest.java | 183 ++++++++++++++++++ 8 files changed, 299 insertions(+), 16 deletions(-) create mode 100644 briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcher.java create mode 100644 briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcherImpl.java create mode 100644 briar-core/src/test/java/org/briarproject/briar/feed/FeedMatcherImplTest.java diff --git a/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java b/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java index f743a8221..e1467bd1a 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java @@ -68,6 +68,7 @@ public class Feed implements Comparable { return false; } + // FIXME: compareTo() is inconsistent with equals() @Override public int compareTo(Feed o) { if (this == o) return 0; diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactory.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactory.java index 192c4855e..cd9447f72 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactory.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactory.java @@ -12,14 +12,14 @@ interface FeedFactory { * Create a new feed based on the feed url * and the metadata of an existing {@link SyndFeed}. */ - Feed createFeed(String url, SyndFeed feed); + Feed createFeed(String url, SyndFeed sf); /** * Creates a new updated feed, based on the given existing feed, * new metadata from the given {@link SyndFeed} * and the time of the last feed entry. */ - Feed createFeed(Feed feed, SyndFeed f, long lastEntryTime); + Feed updateFeed(Feed feed, SyndFeed sf, long lastEntryTime); /** * De-serializes a {@link BdfDictionary} into a {@link Feed}. diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java index b688fab96..bd062faf7 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java @@ -13,7 +13,6 @@ import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.identity.AuthorFactory; import org.briarproject.bramble.api.identity.LocalAuthor; import org.briarproject.bramble.api.system.Clock; -import org.briarproject.bramble.util.StringUtils; import org.briarproject.briar.api.blog.Blog; import org.briarproject.briar.api.blog.BlogFactory; import org.briarproject.briar.api.feed.Feed; @@ -22,6 +21,7 @@ import org.briarproject.briar.api.feed.RssProperties; import javax.inject.Inject; import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH; +import static org.briarproject.bramble.util.StringUtils.truncateUtf8; import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_ADDED; import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_AUTHOR; import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_DESC; @@ -51,25 +51,29 @@ class FeedFactoryImpl implements FeedFactory { } @Override - public Feed createFeed(String url, SyndFeed syndFeed) { - String title = syndFeed.getTitle(); + public Feed createFeed(String url, SyndFeed sf) { + String title = sf.getTitle(); if (title == null) title = "RSS"; - else title = StringUtils.truncateUtf8(title, MAX_AUTHOR_NAME_LENGTH); + else title = truncateUtf8(title, MAX_AUTHOR_NAME_LENGTH); LocalAuthor localAuthor = authorFactory.createLocalAuthor(title); Blog blog = blogFactory.createFeedBlog(localAuthor); long added = clock.currentTimeMillis(); - RssProperties properties = - new RssProperties(url, null, null, null, null, null); + RssProperties properties = new RssProperties(url, sf.getTitle(), + sf.getDescription(), sf.getAuthor(), sf.getLink(), sf.getUri()); return new Feed(blog, localAuthor, properties, added, 0, 0); } @Override - public Feed createFeed(Feed feed, SyndFeed f, long lastEntryTime) { + public Feed updateFeed(Feed feed, SyndFeed sf, long lastEntryTime) { long updated = clock.currentTimeMillis(); - return new Feed(feed.getBlog(), feed.getLocalAuthor(), - feed.getProperties(), feed.getAdded(), updated, lastEntryTime); + String url = feed.getProperties().getUrl(); + // Update the RSS properties + RssProperties properties = new RssProperties(url, sf.getTitle(), + sf.getDescription(), sf.getAuthor(), sf.getLink(), sf.getUri()); + return new Feed(feed.getBlog(), feed.getLocalAuthor(), properties, + feed.getAdded(), updated, lastEntryTime); } @Override @@ -115,12 +119,12 @@ class FeedFactoryImpl implements FeedFactory { RssProperties properties = feed.getProperties(); if (properties.getUrl() != null) d.put(KEY_FEED_URL, properties.getUrl()); + if (properties.getTitle() != null) + d.put(KEY_FEED_RSS_TITLE, properties.getTitle()); if (properties.getDescription() != null) d.put(KEY_FEED_DESC, properties.getDescription()); if (properties.getAuthor() != null) d.put(KEY_FEED_RSS_AUTHOR, properties.getAuthor()); - if (properties.getTitle() != null) - d.put(KEY_FEED_RSS_TITLE, properties.getTitle()); if (properties.getLink() != null) d.put(KEY_FEED_RSS_LINK, properties.getLink()); if (properties.getUri() != null) diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java index 2a2895f85..c3b56ac6f 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java @@ -179,7 +179,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, // post entries long lastEntryTime = postFeedEntries(feed, sf.getEntries()); - Feed updatedFeed = feedFactory.createFeed(feed, sf, lastEntryTime); + Feed updatedFeed = feedFactory.updateFeed(feed, sf, lastEntryTime); // store feed metadata again to also store last entry time txn = db.startTransaction(false); @@ -303,7 +303,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, SyndFeed sf = fetchSyndFeed(url); // sort and add new entries long lastEntryTime = postFeedEntries(feed, sf.getEntries()); - newFeeds.add(feedFactory.createFeed(feed, sf, lastEntryTime)); + newFeeds.add(feedFactory.updateFeed(feed, sf, lastEntryTime)); } catch (IOException | DbException e) { logException(LOG, WARNING, e); newFeeds.add(feed); @@ -328,7 +328,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, // clean title String title = sf.getTitle(); if (title != null) title = cleanAll(title); - sf.setTitle(isNullOrEmpty(title) ? null : title); + sf.setTitle(isNullOrEmpty(title) ? "RSS" : title); // clean description String description = sf.getDescription(); @@ -340,6 +340,10 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, if (author != null) author = cleanAll(author); sf.setAuthor(isNullOrEmpty(author) ? null : author); + // set other relevant fields to null if empty + if ("".equals(sf.getLink())) sf.setLink(null); + if ("".equals(sf.getUri())) sf.setUri(null); + return sf; } diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcher.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcher.java new file mode 100644 index 000000000..db31c6792 --- /dev/null +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcher.java @@ -0,0 +1,22 @@ +package org.briarproject.briar.feed; + +import org.briarproject.briar.api.feed.Feed; +import org.briarproject.briar.api.feed.RssProperties; +import org.briarproject.nullsafety.NotNullByDefault; + +import java.util.List; + +import javax.annotation.Nullable; +import javax.annotation.concurrent.ThreadSafe; + +@ThreadSafe +@NotNullByDefault +public interface FeedMatcher { + + /** + * Returns the best match for the given candidate from the given list of + * feeds, or null if there are no matches. + */ + @Nullable + Feed findMatchingFeed(RssProperties candidate, List feeds); +} diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcherImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcherImpl.java new file mode 100644 index 000000000..61ccfbad4 --- /dev/null +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcherImpl.java @@ -0,0 +1,64 @@ +package org.briarproject.briar.feed; + +import org.briarproject.briar.api.feed.Feed; +import org.briarproject.briar.api.feed.RssProperties; +import org.briarproject.nullsafety.NotNullByDefault; + +import java.util.List; + +import javax.annotation.Nullable; + +@NotNullByDefault +class FeedMatcherImpl implements FeedMatcher { + + private static final int MIN_MATCHING_FIELDS = 2; + + @Nullable + @Override + public Feed findMatchingFeed(RssProperties candidate, List feeds) { + // First pass: if the candidate was imported from a URL and we have + // a feed that was imported from the same URL then it's a match + String url = candidate.getUrl(); + if (url != null) { + for (Feed f : feeds) { + if (url.equals(f.getProperties().getUrl())) return f; + } + } + // Second pass: if the candidate matches at least MIN_MATCHING_FIELDS + // out of the title, description, author, link and URI, then return the + // feed with the highest number of matching fields + int bestScore = 0; + Feed bestFeed = null; + String title = candidate.getTitle(); + String description = candidate.getDescription(); + String author = candidate.getAuthor(); + String link = candidate.getLink(); + String uri = candidate.getUri(); + for (Feed f : feeds) { + int score = 0; + RssProperties p = f.getProperties(); + if (title != null && title.equals(p.getTitle())) { + score++; + } + if (description != null && description.equals(p.getDescription())) { + score++; + } + if (author != null && author.equals(p.getAuthor())) { + score++; + } + if (link != null && link.equals(p.getLink())) { + score++; + } + if (uri != null && uri.equals(p.getUri())) { + score++; + } + if (score > bestScore) { + bestScore = score; + bestFeed = f; + } + } + if (bestScore >= MIN_MATCHING_FIELDS) return bestFeed; + // No match + return null; + } +} diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedModule.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedModule.java index cbb47e474..63c2e75b8 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedModule.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedModule.java @@ -38,4 +38,9 @@ public class FeedModule { FeedFactory provideFeedFactory(FeedFactoryImpl feedFactory) { return feedFactory; } + + @Provides + FeedMatcher provideFeedMatcher(FeedMatcherImpl feedMatcher) { + return feedMatcher; + } } diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedMatcherImplTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedMatcherImplTest.java new file mode 100644 index 000000000..e244a997e --- /dev/null +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedMatcherImplTest.java @@ -0,0 +1,183 @@ +package org.briarproject.briar.feed; + +import org.briarproject.bramble.api.identity.LocalAuthor; +import org.briarproject.bramble.api.sync.ClientId; +import org.briarproject.bramble.test.BrambleTestCase; +import org.briarproject.briar.api.blog.Blog; +import org.briarproject.briar.api.feed.Feed; +import org.briarproject.briar.api.feed.RssProperties; +import org.junit.Test; + +import java.util.Random; + +import static java.util.Arrays.asList; +import static org.briarproject.bramble.test.TestUtils.getClientId; +import static org.briarproject.bramble.test.TestUtils.getGroup; +import static org.briarproject.bramble.test.TestUtils.getLocalAuthor; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; + +public class FeedMatcherImplTest extends BrambleTestCase { + + private static final String URL = "url"; + private static final String TITLE = "title"; + private static final String DESCRIPTION = "description"; + private static final String AUTHOR = "author"; + private static final String LINK = "link"; + private static final String URI = "uri"; + + private final Random random = new Random(); + private final ClientId clientId = getClientId(); + private final LocalAuthor localAuthor = getLocalAuthor(); + private final FeedMatcher matcher = new FeedMatcherImpl(); + + @Test + public void testFeedWithMatchingUrlIsChosen() { + RssProperties candidate = new RssProperties(URL, + TITLE, DESCRIPTION, AUTHOR, LINK, URI); + // The first feed has a different/null URL but matching RSS fields + Feed feed1 = createFeed(new RssProperties(nope(), + TITLE, DESCRIPTION, AUTHOR, LINK, URI)); + // The second feed has a matching URL but different/null RSS fields + Feed feed2 = createFeed(new RssProperties(URL, + nope(), nope(), nope(), nope(), nope())); + + Feed match = matcher.findMatchingFeed(candidate, asList(feed1, feed2)); + + // The matcher should choose the feed with the matching URL + assertNotNull(match); + assertSame(feed2, match); + } + + @Test + public void testNullUrlIsNotMatched() { + // The candidate has a null URL + RssProperties candidate = new RssProperties(null, + TITLE, DESCRIPTION, AUTHOR, LINK, URI); + // The first feed has a non-null URL and matching RSS fields + Feed feed1 = createFeed(new RssProperties(URL, + TITLE, DESCRIPTION, AUTHOR, LINK, URI)); + // The second feed has a null URL and different/null RSS fields + Feed feed2 = createFeed(new RssProperties(null, + nope(), nope(), nope(), nope(), nope())); + + Feed match = matcher.findMatchingFeed(candidate, asList(feed1, feed2)); + + // The matcher should choose the feed with the matching RSS fields + assertNotNull(match); + assertSame(feed1, match); + } + + @Test + public void testDoesNotMatchOneRssField() { + testDoesNotMatchRssFields(TITLE, nope(), nope(), nope(), nope()); + testDoesNotMatchRssFields(nope(), DESCRIPTION, nope(), nope(), nope()); + testDoesNotMatchRssFields(nope(), nope(), AUTHOR, nope(), nope()); + testDoesNotMatchRssFields(nope(), nope(), nope(), LINK, nope()); + testDoesNotMatchRssFields(nope(), nope(), nope(), nope(), URL); + } + + private void testDoesNotMatchRssFields(String title, String description, + String author, String link, String uri) { + RssProperties candidate = new RssProperties(null, + TITLE, DESCRIPTION, AUTHOR, LINK, URL); + // The first feed has no matching RSS fields + Feed feed1 = createFeed(new RssProperties(null, + nope(), nope(), nope(), nope(), nope())); + // The second feed has the given RSS fields + Feed feed2 = createFeed(new RssProperties(null, + title, description, author, link, uri)); + // The third feed has no matching RSS fields + Feed feed3 = createFeed(new RssProperties(null, + nope(), nope(), nope(), nope(), nope())); + + Feed match = matcher.findMatchingFeed(candidate, + asList(feed1, feed2, feed3)); + + // The matcher should not choose any of the feeds + assertNull(match); + } + + @Test + public void testMatchesTwoRssFields() { + testMatchesRssFields(TITLE, DESCRIPTION, nope(), nope(), nope()); + testMatchesRssFields(nope(), DESCRIPTION, AUTHOR, nope(), nope()); + testMatchesRssFields(nope(), nope(), AUTHOR, LINK, nope()); + testMatchesRssFields(nope(), nope(), nope(), LINK, URI); + } + + private void testMatchesRssFields(String title, String description, + String author, String link, String uri) { + RssProperties candidate = new RssProperties(null, + TITLE, DESCRIPTION, AUTHOR, LINK, URI); + // The first feed has no matching RSS fields + Feed feed1 = createFeed(new RssProperties(null, + nope(), nope(), nope(), nope(), nope())); + // The second feed has the given RSS fields + Feed feed2 = createFeed(new RssProperties(null, + title, description, author, link, uri)); + // The third feed has one matching RSS field + Feed feed3 = createFeed(new RssProperties(null, + TITLE, nope(), nope(), nope(), nope())); + + FeedMatcher matcher = new FeedMatcherImpl(); + Feed match = matcher.findMatchingFeed(candidate, + asList(feed1, feed2, feed3)); + + // The matcher should choose the second feed + assertSame(feed2, match); + } + + @Test + public void testFeedWithMostMatchingRssFieldsIsChosen() { + RssProperties candidate = new RssProperties(null, + TITLE, DESCRIPTION, AUTHOR, LINK, URI); + // The first feed has no matching RSS fields + Feed feed1 = createFeed(new RssProperties(null, + nope(), nope(), nope(), nope(), nope())); + // The second feed has three matching RSS fields + Feed feed2 = createFeed(new RssProperties(null, + TITLE, DESCRIPTION, AUTHOR, nope(), nope())); + // The third feed has two matching RSS fields + Feed feed3 = createFeed(new RssProperties(null, + TITLE, DESCRIPTION, nope(), nope(), nope())); + + Feed match = matcher.findMatchingFeed(candidate, + asList(feed1, feed2, feed3)); + + // The matcher should choose the second feed + assertSame(feed2, match); + } + + @Test + public void testNullRssFieldsAreNotMatched() { + // The candidate has a null URL and null RSS fields + RssProperties candidate = new RssProperties(null, + null, null, null, null, null); + // The first feed has a null URL and non-null RSS fields + Feed feed1 = createFeed(new RssProperties(null, + TITLE, DESCRIPTION, AUTHOR, LINK, URI)); + // The second feed has a non-null URL and null RSS fields + Feed feed2 = createFeed(new RssProperties(URL, + null, null, null, null, null)); + + Feed match = matcher.findMatchingFeed(candidate, asList(feed1, feed2)); + + // The matcher should not choose either of the feeds + assertNull(match); + } + + /** + * Returns an RSS field that doesn't match the default, either because it's + * null or because it's a different non-null value. + */ + private String nope() { + return random.nextBoolean() ? null : "x"; + } + + private Feed createFeed(RssProperties properties) { + Blog blog = new Blog(getGroup(clientId, 123), localAuthor, true); + return new Feed(blog, localAuthor, properties, 0, 0, 0); + } +} From e52250f1e4c27ddb2eb717cc789eb2eba84a23a7 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Mon, 12 Dec 2022 17:31:45 +0000 Subject: [PATCH 04/13] Don't sort list of RSS feeds in UI. --- .../briar/android/blog/RssFeedViewModel.java | 3 --- .../java/org/briarproject/briar/api/feed/Feed.java | 12 +----------- .../org/briarproject/briar/feed/FeedManagerImpl.java | 3 ++- 3 files changed, 3 insertions(+), 15 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java index ddd673c01..37b6b9c44 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java @@ -22,7 +22,6 @@ import org.briarproject.nullsafety.NotNullByDefault; import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; -import java.util.Collections; import java.util.List; import java.util.concurrent.Executor; import java.util.logging.Logger; @@ -101,7 +100,6 @@ class RssFeedViewModel extends DbViewModel { private List loadFeeds(Transaction txn) throws DbException { long start = now(); List feeds = feedManager.getFeeds(txn); - Collections.sort(feeds); logDuration(LOG, "Loading feeds", start); return feeds; } @@ -145,7 +143,6 @@ class RssFeedViewModel extends DbViewModel { Feed feed = feedManager.addFeed(url); List updated = addListItem(getList(feeds), feed); if (updated != null) { - Collections.sort(updated); feeds.postValue(new LiveResult<>(updated)); } importResult.postEvent(IMPORTED); diff --git a/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java b/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java index e1467bd1a..848e27531 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/feed/Feed.java @@ -9,7 +9,7 @@ import javax.annotation.concurrent.Immutable; @Immutable @NotNullByDefault -public class Feed implements Comparable { +public class Feed { private final Blog blog; private final LocalAuthor localAuthor; @@ -68,16 +68,6 @@ public class Feed implements Comparable { return false; } - // FIXME: compareTo() is inconsistent with equals() - @Override - public int compareTo(Feed o) { - if (this == o) return 0; - long aTime = getAdded(), bTime = o.getAdded(); - if (aTime > bTime) return -1; - if (aTime < bTime) return 1; - return 0; - } - @Override public int hashCode() { return blog.hashCode(); diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java index c3b56ac6f..adeb3aed3 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java @@ -62,6 +62,7 @@ import okhttp3.ResponseBody; import static java.util.Collections.sort; import static java.util.logging.Level.WARNING; +import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty; import static org.briarproject.bramble.util.StringUtils.truncateUtf8; @@ -79,7 +80,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, RemoveBlogHook { private static final Logger LOG = - Logger.getLogger(FeedManagerImpl.class.getName()); + getLogger(FeedManagerImpl.class.getName()); private final TaskScheduler scheduler; private final Executor ioExecutor; From 2eef34f4241a71955a50f2ba835970a494c98c1f Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 20 Jan 2023 13:35:12 +0000 Subject: [PATCH 05/13] Use new transaction wrappers. --- .../briar/feed/FeedManagerImpl.java | 39 ++++++------------- .../briar/feed/FeedManagerImplTest.java | 7 +--- 2 files changed, 13 insertions(+), 33 deletions(-) diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java index adeb3aed3..419a95780 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java @@ -167,32 +167,24 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, Feed feed = feedFactory.createFeed(url, sf); // store feed metadata and new blog - Transaction txn = db.startTransaction(false); - try { + db.transaction(false, txn -> { blogManager.addBlog(txn, feed.getBlog()); List feeds = getFeeds(txn); feeds.add(feed); storeFeeds(txn, feeds); - db.commitTransaction(txn); - } finally { - db.endTransaction(txn); - } + }); // post entries long lastEntryTime = postFeedEntries(feed, sf.getEntries()); Feed updatedFeed = feedFactory.updateFeed(feed, sf, lastEntryTime); // store feed metadata again to also store last entry time - txn = db.startTransaction(false); - try { + db.transaction(false, txn -> { List feeds = getFeeds(txn); feeds.remove(feed); feeds.add(updatedFeed); storeFeeds(txn, feeds); - db.commitTransaction(txn); - } finally { - db.endTransaction(txn); - } + }); return updatedFeed; } @@ -200,14 +192,9 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, @Override public void removeFeed(Feed feed) throws DbException { LOG.info("Removing RSS feed..."); - Transaction txn = db.startTransaction(false); - try { - // this will call removingBlog() where the feed itself gets removed - blogManager.removeBlog(txn, feed.getBlog()); - db.commitTransaction(txn); - } finally { - db.endTransaction(txn); - } + // this will call removingBlog() where the feed itself gets removed + db.transaction(false, txn -> + blogManager.removeBlog(txn, feed.getBlog())); } @Override @@ -375,9 +362,8 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, long postFeedEntries(Feed feed, List entries) throws DbException { - long lastEntryTime = feed.getLastEntryTime(); - Transaction txn = db.startTransaction(false); - try { + return db.transactionWithResult(false, txn -> { + long lastEntryTime = feed.getLastEntryTime(); //noinspection Java8ListSort sort(entries, getEntryComparator()); for (SyndEntry entry : entries) { @@ -396,11 +382,8 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, if (entryTime > lastEntryTime) lastEntryTime = entryTime; } } - db.commitTransaction(txn); - } finally { - db.endTransaction(txn); - } - return lastEntryTime; + return lastEntryTime; + }); } private void postEntry(Transaction txn, Feed feed, SyndEntry entry) { diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java index 5116c4a9b..3b2ee973c 100644 --- a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java @@ -137,17 +137,14 @@ public class FeedManagerImplTest extends BrambleMockTestCase { Message msg = getMessage(blogGroupId); BlogPost post = new BlogPost(msg, null, localAuthor); - context.checking(new Expectations() {{ - oneOf(db).startTransaction(false); - will(returnValue(txn)); + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(false), withDbCallable(txn)); oneOf(clock).currentTimeMillis(); will(returnValue(42L)); oneOf(blogPostFactory).createBlogPost(feed.getBlogId(), 42L, null, localAuthor, text); will(returnValue(post)); oneOf(blogManager).addLocalPost(txn, post); - oneOf(db).commitTransaction(txn); - oneOf(db).endTransaction(txn); }}); feedManager.postFeedEntries(feed, entries); } From dc220200b6541679a732cec2717fab374569d7c0 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 24 Jan 2023 12:43:14 +0000 Subject: [PATCH 06/13] Match newly added RSS feeds to existing feeds. --- bramble-core/build.gradle | 2 +- .../briar/android/blog/RssFeedActivity.java | 14 +- .../briar/android/blog/RssFeedViewModel.java | 44 +-- briar-android/src/main/res/values/strings.xml | 1 - briar-core/build.gradle | 1 + .../briar/feed/FeedManagerImpl.java | 34 +- .../briar/feed/FeedMatcherImpl.java | 5 + .../briar/feed/FeedManagerImplTest.java | 296 ++++++++++++++---- build.gradle | 1 + 9 files changed, 286 insertions(+), 112 deletions(-) diff --git a/bramble-core/build.gradle b/bramble-core/build.gradle index f758c08a4..a067dd390 100644 --- a/bramble-core/build.gradle +++ b/bramble-core/build.gradle @@ -33,7 +33,7 @@ dependencies { testImplementation "org.jmock:jmock:$jmock_version" testImplementation "org.jmock:jmock-junit4:$jmock_version" testImplementation "org.jmock:jmock-imposters:$jmock_version" - testImplementation "com.squareup.okhttp3:mockwebserver:4.9.3" + testImplementation "com.squareup.okhttp3:mockwebserver:$mockwebserver_version" testAnnotationProcessor "com.google.dagger:dagger-compiler:$dagger_version" diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedActivity.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedActivity.java index e64af07d6..6b50fed44 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedActivity.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedActivity.java @@ -1,7 +1,6 @@ package org.briarproject.briar.android.blog; import android.os.Bundle; -import android.widget.Toast; import org.briarproject.briar.R; import org.briarproject.briar.android.activity.ActivityComponent; @@ -16,10 +15,6 @@ import androidx.annotation.Nullable; import androidx.fragment.app.FragmentManager; import androidx.lifecycle.ViewModelProvider; -import static org.briarproject.briar.android.blog.RssFeedViewModel.ImportResult.EXISTS; -import static org.briarproject.briar.android.blog.RssFeedViewModel.ImportResult.FAILED; -import static org.briarproject.briar.android.blog.RssFeedViewModel.ImportResult.IMPORTED; - @MethodsNotNullByDefault @ParametersNotNullByDefault public class RssFeedActivity extends BriarActivity @@ -50,13 +45,13 @@ public class RssFeedActivity extends BriarActivity viewModel.getImportResult().observeEvent(this, this::onImportResult); } - private void onImportResult(RssFeedViewModel.ImportResult result) { - if (result == IMPORTED) { + private void onImportResult(boolean result) { + if (result) { FragmentManager fm = getSupportFragmentManager(); if (fm.findFragmentByTag(RssFeedImportFragment.TAG) != null) { onBackPressed(); } - } else if (result == FAILED) { + } else { String url = viewModel.getUrlFailedImport(); if (url == null) { throw new AssertionError(); @@ -65,9 +60,6 @@ public class RssFeedActivity extends BriarActivity RssFeedImportFailedDialogFragment.newInstance(url); dialog.show(getSupportFragmentManager(), RssFeedImportFailedDialogFragment.TAG); - } else if (result == EXISTS) { - Toast.makeText(this, R.string.blogs_rss_feeds_import_exists, - Toast.LENGTH_LONG).show(); } } } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java index 37b6b9c44..c9f050c77 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/RssFeedViewModel.java @@ -22,6 +22,7 @@ import org.briarproject.nullsafety.NotNullByDefault; import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; +import java.util.ArrayList; import java.util.List; import java.util.concurrent.Executor; import java.util.logging.Logger; @@ -37,13 +38,9 @@ import static java.util.logging.Logger.getLogger; import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.now; -import static org.briarproject.briar.android.blog.RssFeedViewModel.ImportResult.EXISTS; -import static org.briarproject.briar.android.blog.RssFeedViewModel.ImportResult.FAILED; -import static org.briarproject.briar.android.blog.RssFeedViewModel.ImportResult.IMPORTED; @NotNullByDefault class RssFeedViewModel extends DbViewModel { - enum ImportResult {IMPORTED, FAILED, EXISTS} private static final Logger LOG = getLogger(RssFeedViewModel.class.getName()); @@ -59,7 +56,7 @@ class RssFeedViewModel extends DbViewModel { private volatile String urlFailedImport = null; private final MutableLiveData isImporting = new MutableLiveData<>(false); - private final MutableLiveEvent importResult = + private final MutableLiveEvent importResult = new MutableLiveEvent<>(); @Inject @@ -123,7 +120,7 @@ class RssFeedViewModel extends DbViewModel { }); } - LiveEvent getImportResult() { + LiveEvent getImportResult() { return importResult; } @@ -136,20 +133,23 @@ class RssFeedViewModel extends DbViewModel { urlFailedImport = null; ioExecutor.execute(() -> { try { - if (exists(url)) { - importResult.postEvent(EXISTS); - return; - } Feed feed = feedManager.addFeed(url); - List updated = addListItem(getList(feeds), feed); - if (updated != null) { - feeds.postValue(new LiveResult<>(updated)); + // Update the feed if it was already present + List feedList = getList(feeds); + if (feedList == null) feedList = new ArrayList<>(); + List updated = updateListItems(feedList, + f -> f.equals(feed), f -> feed); + // Add the feed if it wasn't already present + if (updated == null) { + feedList.add(feed); + updated = feedList; } - importResult.postEvent(IMPORTED); + feeds.postValue(new LiveResult<>(updated)); + importResult.postEvent(true); } catch (DbException | IOException e) { logException(LOG, WARNING, e); urlFailedImport = url; - importResult.postEvent(FAILED); + importResult.postEvent(false); } finally { isImporting.postValue(false); } @@ -160,18 +160,4 @@ class RssFeedViewModel extends DbViewModel { String getUrlFailedImport() { return urlFailedImport; } - - private boolean exists(String url) { - List list = getList(feeds); - if (list != null) { - for (Feed feed : list) { - // TODO: Fetch the feed and also match it against feeds that - // were imported from files? - if (url.equals(feed.getProperties().getUrl())) { - return true; - } - } - } - return false; - } } diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index 44c9adf03..aeb890d77 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -516,7 +516,6 @@ Import Enter the URL of the RSS feed We are sorry! There was an error importing your feed. - That feed is already imported. RSS Feeds Imported: Author: diff --git a/briar-core/build.gradle b/briar-core/build.gradle index 181d642b9..df3dacb2c 100644 --- a/briar-core/build.gradle +++ b/briar-core/build.gradle @@ -28,6 +28,7 @@ dependencies { testImplementation "org.jmock:jmock:$jmock_version" testImplementation "org.jmock:jmock-junit4:$jmock_version" testImplementation "org.jmock:jmock-imposters:$jmock_version" + testImplementation "com.squareup.okhttp3:mockwebserver:$mockwebserver_version" testAnnotationProcessor "com.google.dagger:dagger-compiler:$dagger_version" diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java index 419a95780..e39f4b62d 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java @@ -38,6 +38,7 @@ import org.briarproject.briar.api.blog.BlogPost; import org.briarproject.briar.api.blog.BlogPostFactory; import org.briarproject.briar.api.feed.Feed; import org.briarproject.briar.api.feed.FeedManager; +import org.briarproject.briar.api.feed.RssProperties; import org.briarproject.nullsafety.NotNullByDefault; import java.io.IOException; @@ -90,6 +91,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, private final BlogManager blogManager; private final BlogPostFactory blogPostFactory; private final FeedFactory feedFactory; + private final FeedMatcher feedMatcher; private final Clock clock; private final WeakSingletonProvider httpClientProvider; private final AtomicBoolean fetcherStarted = new AtomicBoolean(false); @@ -105,6 +107,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, BlogManager blogManager, BlogPostFactory blogPostFactory, FeedFactory feedFactory, + FeedMatcher feedMatcher, WeakSingletonProvider httpClientProvider, Clock clock) { this.scheduler = scheduler; @@ -115,6 +118,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, this.blogManager = blogManager; this.blogPostFactory = blogPostFactory; this.feedFactory = feedFactory; + this.feedMatcher = feedMatcher; this.httpClientProvider = httpClientProvider; this.clock = clock; } @@ -163,16 +167,28 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, public Feed addFeed(String url) throws DbException, IOException { // fetch feed to get posts and metadata SyndFeed sf = fetchSyndFeed(url); + RssProperties properties = new RssProperties(url, sf.getTitle(), + sf.getDescription(), sf.getAuthor(), sf.getLink(), sf.getUri()); - Feed feed = feedFactory.createFeed(url, sf); + // check whether the properties match an existing feed + List candidates = db.transactionWithResult(true, this::getFeeds); + Feed matched = feedMatcher.findMatchingFeed(properties, candidates); - // store feed metadata and new blog - db.transaction(false, txn -> { - blogManager.addBlog(txn, feed.getBlog()); - List feeds = getFeeds(txn); - feeds.add(feed); - storeFeeds(txn, feeds); - }); + Feed feed; + if (matched == null) { + LOG.info("Adding new feed"); + feed = feedFactory.createFeed(url, sf); + // store feed metadata and new blog + db.transaction(false, txn -> { + blogManager.addBlog(txn, feed.getBlog()); + List feeds = getFeeds(txn); + feeds.add(feed); + storeFeeds(txn, feeds); + }); + } else { + LOG.info("New feed matches an existing feed"); + feed = matched; + } // post entries long lastEntryTime = postFeedEntries(feed, sf.getEntries()); @@ -359,7 +375,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, } } - long postFeedEntries(Feed feed, List entries) + private long postFeedEntries(Feed feed, List entries) throws DbException { return db.transactionWithResult(false, txn -> { diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcherImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcherImpl.java index 61ccfbad4..8acaed39e 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcherImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcherImpl.java @@ -7,12 +7,17 @@ import org.briarproject.nullsafety.NotNullByDefault; import java.util.List; import javax.annotation.Nullable; +import javax.inject.Inject; @NotNullByDefault class FeedMatcherImpl implements FeedMatcher { private static final int MIN_MATCHING_FIELDS = 2; + @Inject + FeedMatcherImpl() { + } + @Nullable @Override public Feed findMatchingFeed(RssProperties candidate, List feeds) { diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java index 3b2ee973c..cf7e38104 100644 --- a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java @@ -1,7 +1,6 @@ package org.briarproject.briar.feed; -import com.rometools.rome.feed.synd.SyndEntry; -import com.rometools.rome.feed.synd.SyndEntryImpl; +import com.rometools.rome.feed.synd.SyndFeed; import org.briarproject.bramble.api.WeakSingletonProvider; import org.briarproject.bramble.api.client.ClientHelper; @@ -29,19 +28,18 @@ import org.briarproject.briar.api.feed.RssProperties; import org.jmock.Expectations; import org.junit.Test; -import java.net.UnknownHostException; -import java.util.ArrayList; +import java.text.SimpleDateFormat; import java.util.Date; -import java.util.List; import java.util.concurrent.Executor; import javax.annotation.Nonnull; -import javax.net.SocketFactory; -import okhttp3.Dns; import okhttp3.OkHttpClient; +import okhttp3.mockwebserver.MockResponse; +import okhttp3.mockwebserver.MockWebServer; -import static java.util.concurrent.TimeUnit.MILLISECONDS; +import static java.util.Collections.singletonList; +import static okhttp3.mockwebserver.SocketPolicy.DISCONNECT_DURING_RESPONSE_BODY; import static org.briarproject.bramble.test.TestUtils.getGroup; import static org.briarproject.bramble.test.TestUtils.getLocalAuthor; import static org.briarproject.bramble.test.TestUtils.getMessage; @@ -61,14 +59,10 @@ public class FeedManagerImplTest extends BrambleMockTestCase { private final BlogPostFactory blogPostFactory = context.mock(BlogPostFactory.class); private final FeedFactory feedFactory = context.mock(FeedFactory.class); + private final FeedMatcher feedMatcher = context.mock(FeedMatcher.class); private final Clock clock = context.mock(Clock.class); - private final Dns noDnsLookups = context.mock(Dns.class); - private final OkHttpClient client = new OkHttpClient.Builder() - .socketFactory(SocketFactory.getDefault()) - .dns(noDnsLookups) - .connectTimeout(60_000, MILLISECONDS) - .build(); + private final OkHttpClient client = new OkHttpClient.Builder().build(); private final WeakSingletonProvider httpClientProvider = new WeakSingletonProvider() { @Override @@ -84,15 +78,20 @@ public class FeedManagerImplTest extends BrambleMockTestCase { private final GroupId blogGroupId = blogGroup.getId(); private final LocalAuthor localAuthor = getLocalAuthor(); private final Blog blog = new Blog(blogGroup, localAuthor, true); - private final RssProperties properties = new RssProperties( - "http://example.org", null, null, null, null, null); - private final Feed feed = new Feed(blog, localAuthor, properties, 0, 0, 0); - private final BdfDictionary feedDict = new BdfDictionary(); + private final Message message = getMessage(blogGroupId); + private final BlogPost blogPost = new BlogPost(message, null, localAuthor); + + private final long now = System.currentTimeMillis(); + // Round the publication date to a whole second to avoid rounding errors + private final long pubDate = now / 1000 * 1000 - 1000; + private final SimpleDateFormat sdf = + new SimpleDateFormat("EEE, dd MMM yy HH:mm:ss Z"); + private final String pubDateString = sdf.format(new Date(pubDate)); private final FeedManagerImpl feedManager = new FeedManagerImpl(scheduler, ioExecutor, db, contactGroupFactory, clientHelper, blogManager, blogPostFactory, feedFactory, - httpClientProvider, clock); + feedMatcher, httpClientProvider, clock); @Test public void testFetchFeedsReturnsEarlyIfTorIsNotActive() { @@ -101,52 +100,166 @@ public class FeedManagerImplTest extends BrambleMockTestCase { } @Test - public void testEmptyFetchFeeds() throws Exception { - BdfList feedList = new BdfList(); - expectGetFeeds(feedList); - expectStoreFeed(feedList); + public void testFetchFeedsEmptyList() throws Exception { + // The list of feeds is empty + expectGetFeeds(); + expectStoreFeeds(); feedManager.setTorActive(true); feedManager.fetchFeeds(); } @Test public void testFetchFeedsIoException() throws Exception { - BdfDictionary feedDict = new BdfDictionary(); - BdfList feedList = BdfList.of(feedDict); + // Fetching the feed will fail + MockWebServer server = new MockWebServer(); + String url = server.url("/").toString(); + server.enqueue(new MockResponse() + .setBody(" ") + .setSocketPolicy(DISCONNECT_DURING_RESPONSE_BODY)); - expectGetFeeds(feedList); - context.checking(new Expectations() {{ - oneOf(noDnsLookups).lookup("example.org"); - will(throwException(new UnknownHostException())); - }}); - expectStoreFeed(feedList); + Feed feed = createFeed(url, blog); + + expectGetFeeds(feed); + expectStoreFeeds(feed); feedManager.setTorActive(true); feedManager.fetchFeeds(); } @Test - public void testPostFeedEntriesEmptyDate() throws Exception { - Transaction txn = new Transaction(null, false); - List entries = new ArrayList<>(); - entries.add(new SyndEntryImpl()); - SyndEntry entry = new SyndEntryImpl(); - entry.setUpdatedDate(new Date()); - entries.add(entry); - String text = "

(" + entry.getUpdatedDate().toString() + ")

"; - Message msg = getMessage(blogGroupId); - BlogPost post = new BlogPost(msg, null, localAuthor); + public void testFetchFeedsEmptyResponseBody() throws Exception { + // Fetching the feed will succeed, but parsing the empty body will fail + MockWebServer server = new MockWebServer(); + String url = server.url("/").toString(); + server.enqueue(new MockResponse()); + + Feed feed = createFeed(url, blog); + + expectGetFeeds(feed); + expectStoreFeeds(feed); + + feedManager.setTorActive(true); + feedManager.fetchFeeds(); + } + + @Test + public void testFetchFeedsNoEntries() throws Exception { + // Fetching and parsing the feed will succeed; there are no entries + String feedXml = createRssFeedXml(); + + MockWebServer server = new MockWebServer(); + String url = server.url("/").toString(); + server.enqueue(new MockResponse().setBody(feedXml)); + + Feed feed = createFeed(url, blog); + + expectGetFeeds(feed); + expectUpdateFeedNoEntries(feed); + expectStoreFeeds(feed); + + feedManager.setTorActive(true); + feedManager.fetchFeeds(); + } + + @Test + public void testFetchFeedsOneEntry() throws Exception { + // Fetching and parsing the feed will succeed; there is one entry + String entryXml = + "" + pubDateString + ""; + String feedXml = createRssFeedXml(entryXml); + + MockWebServer server = new MockWebServer(); + String url = server.url("/").toString(); + server.enqueue(new MockResponse().setBody(feedXml)); + + Feed feed = createFeed(url, blog); + + expectGetFeeds(feed); + expectUpdateFeedOneEntry(feed); + expectStoreFeeds(feed); + + feedManager.setTorActive(true); + feedManager.fetchFeeds(); + } + + @Test + public void testAddNewFeed() throws Exception { + // Fetching and parsing the feed will succeed; there are no entries + String feedXml = createRssFeedXml(); + + MockWebServer server = new MockWebServer(); + String url = server.url("/").toString(); + server.enqueue(new MockResponse().setBody(feedXml)); + + Feed newFeed = createFeed(url, blog); + + Group existingBlogGroup = getGroup(BlogManager.CLIENT_ID, + BlogManager.MAJOR_VERSION); + Blog existingBlog = new Blog(existingBlogGroup, localAuthor, true); + Feed existingFeed = createFeed("http://example.com", existingBlog); + + expectGetFeeds(existingFeed); context.checking(new DbExpectations() {{ - oneOf(db).transactionWithResult(with(false), withDbCallable(txn)); - oneOf(clock).currentTimeMillis(); - will(returnValue(42L)); - oneOf(blogPostFactory).createBlogPost(feed.getBlogId(), 42L, null, - localAuthor, text); - will(returnValue(post)); - oneOf(blogManager).addLocalPost(txn, post); + // The added feed doesn't match any existing feed + oneOf(feedMatcher).findMatchingFeed(with(any(RssProperties.class)), + with(singletonList(existingFeed))); + will(returnValue(null)); + // Create the new feed + oneOf(feedFactory).createFeed(with(url), with(any(SyndFeed.class))); + will(returnValue(newFeed)); + // Add the new feed to the list of feeds + Transaction txn = new Transaction(null, false); + oneOf(db).transaction(with(false), withDbRunnable(txn)); + oneOf(blogManager).addBlog(txn, blog); + expectGetFeeds(txn, existingFeed); + expectStoreFeeds(txn, existingFeed, newFeed); }}); - feedManager.postFeedEntries(feed, entries); + + expectUpdateFeedNoEntries(newFeed); + expectGetAndStoreFeeds(existingFeed, newFeed); + + feedManager.addFeed(url); + } + + @Test + public void testAddExistingFeed() throws Exception { + // Fetching and parsing the feed will succeed; there are no entries + String feedXml = createRssFeedXml(); + + MockWebServer server = new MockWebServer(); + String url = server.url("/").toString(); + server.enqueue(new MockResponse().setBody(feedXml)); + + Feed newFeed = createFeed(url, blog); + + expectGetFeeds(newFeed); + + context.checking(new DbExpectations() {{ + // The added feed matches an existing feed + oneOf(feedMatcher).findMatchingFeed(with(any(RssProperties.class)), + with(singletonList(newFeed))); + will(returnValue(newFeed)); + }}); + + expectUpdateFeedNoEntries(newFeed); + expectGetAndStoreFeeds(newFeed); + + feedManager.addFeed(url); + } + + private Feed createFeed(String url, Blog blog) { + RssProperties properties = new RssProperties(url, + null, null, null, null, null); + return new Feed(blog, localAuthor, properties, 0, 0, 0); + } + + private String createRssFeedXml(String... entries) { + StringBuilder sb = new StringBuilder(); + sb.append(""); + for (String entry : entries) sb.append(entry); + sb.append(""); + return sb.toString(); } private void expectGetLocalGroup() { @@ -157,35 +270,96 @@ public class FeedManagerImplTest extends BrambleMockTestCase { }}); } - private void expectGetFeeds(BdfList feedList) throws Exception { + private void expectGetFeeds(Feed... feeds) throws Exception { Transaction txn = new Transaction(null, true); + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + }}); + expectGetFeeds(txn, feeds); + } + + private void expectGetFeeds(Transaction txn, Feed... feeds) + throws Exception { + BdfList feedList = new BdfList(); + for (int i = 0; i < feeds.length; i++) { + feedList.add(new BdfDictionary()); + } BdfDictionary feedsDict = BdfDictionary.of(new BdfEntry(KEY_FEEDS, feedList)); expectGetLocalGroup(); - context.checking(new DbExpectations() {{ - oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + + context.checking(new Expectations() {{ oneOf(clientHelper).getGroupMetadataAsDictionary(txn, localGroupId); will(returnValue(feedsDict)); - if (feedList.size() == 1) { - oneOf(feedFactory).createFeed(feedDict); - will(returnValue(feed)); + for (int i = 0; i < feeds.length; i++) { + oneOf(feedFactory).createFeed(feedList.getDictionary(i)); + will(returnValue(feeds[i])); } }}); } - private void expectStoreFeed(BdfList feedList) throws Exception { + private void expectStoreFeeds(Feed... feeds) throws Exception { Transaction txn = new Transaction(null, false); + context.checking(new DbExpectations() {{ + oneOf(db).transaction(with(false), withDbRunnable(txn)); + }}); + expectStoreFeeds(txn, feeds); + } + + private void expectStoreFeeds(Transaction txn, Feed... feeds) + throws Exception { + BdfList feedList = new BdfList(); + for (int i = 0; i < feeds.length; i++) { + feedList.add(new BdfDictionary()); + } BdfDictionary feedDict = BdfDictionary.of(new BdfEntry(KEY_FEEDS, feedList)); expectGetLocalGroup(); - context.checking(new DbExpectations() {{ - oneOf(db).transaction(with(false), withDbRunnable(txn)); + + context.checking(new Expectations() {{ oneOf(clientHelper).mergeGroupMetadata(txn, localGroupId, feedDict); - if (feedList.size() == 1) { - oneOf(feedFactory).feedToBdfDictionary(feed); - will(returnValue(feedList.getDictionary(0))); + for (int i = 0; i < feeds.length; i++) { + oneOf(feedFactory).feedToBdfDictionary(feeds[i]); + will(returnValue(feedList.getDictionary(i))); } }}); } + private void expectGetAndStoreFeeds(Feed... feeds) throws Exception { + context.checking(new DbExpectations() {{ + Transaction txn = new Transaction(null, false); + oneOf(db).transaction(with(false), withDbRunnable(txn)); + expectGetFeeds(txn, feeds); + expectStoreFeeds(txn, feeds); + }}); + } + + private void expectUpdateFeedNoEntries(Feed feed) throws Exception { + Transaction txn = new Transaction(null, false); + + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(false), withDbCallable(txn)); + oneOf(feedFactory).updateFeed(with(feed), with(any(SyndFeed.class)), + with(0L)); + will(returnValue(feed)); + }}); + } + + private void expectUpdateFeedOneEntry(Feed feed) throws Exception { + Transaction txn = new Transaction(null, false); + String body = "

(" + new Date(pubDate) + ")

"; + + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(false), withDbCallable(txn)); + oneOf(clock).currentTimeMillis(); + will(returnValue(now)); + oneOf(blogPostFactory).createBlogPost(blogGroupId, pubDate, null, + localAuthor, body); + will(returnValue(blogPost)); + oneOf(blogManager).addLocalPost(txn, blogPost); + oneOf(feedFactory).updateFeed(with(feed), with(any(SyndFeed.class)), + with(pubDate)); + will(returnValue(feed)); + }}); + } } diff --git a/build.gradle b/build.gradle index bcc33bfc4..ac3337266 100644 --- a/build.gradle +++ b/build.gradle @@ -39,6 +39,7 @@ buildscript { bouncy_castle_version = '1.71' junit_version = "4.13.2" jmock_version = '2.12.0' + mockwebserver_version = '4.9.3' } dependencies { classpath 'com.android.tools.build:gradle:7.2.2' From 8f7bb9d26b7215effb5cccf15a6f756f51d3c8dd Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 24 Jan 2023 13:28:22 +0000 Subject: [PATCH 07/13] Don't overwrite the list of feeds after fetching. --- .../briar/feed/FeedManagerImpl.java | 43 +++++++++++++------ .../briar/feed/FeedManagerImplTest.java | 18 +++----- 2 files changed, 36 insertions(+), 25 deletions(-) diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java index e39f4b62d..7c0be7856 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java @@ -47,8 +47,11 @@ import java.security.GeneralSecurityException; import java.util.ArrayList; import java.util.Comparator; import java.util.Date; +import java.util.HashMap; import java.util.Iterator; import java.util.List; +import java.util.ListIterator; +import java.util.Map; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Logger; @@ -61,6 +64,7 @@ import okhttp3.Request; import okhttp3.Response; import okhttp3.ResponseBody; +import static java.util.Collections.singletonList; import static java.util.Collections.sort; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; @@ -195,12 +199,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, Feed updatedFeed = feedFactory.updateFeed(feed, sf, lastEntryTime); // store feed metadata again to also store last entry time - db.transaction(false, txn -> { - List feeds = getFeeds(txn); - feeds.remove(feed); - feeds.add(updatedFeed); - storeFeeds(txn, feeds); - }); + updateFeeds(singletonList(updatedFeed)); return updatedFeed; } @@ -270,8 +269,23 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, } } - private void storeFeeds(List feeds) throws DbException { - db.transaction(false, txn -> storeFeeds(txn, feeds)); + /** + * Updates the given feeds in the stored list of feeds, without affecting + * any other feeds in the list or re-adding any of the given feeds that + * have been removed from the list. + */ + private void updateFeeds(List updatedFeeds) throws DbException { + Map updatedMap = new HashMap<>(); + for (Feed feed : updatedFeeds) updatedMap.put(feed.getBlogId(), feed); + db.transaction(false, txn -> { + List feeds = getFeeds(txn); + ListIterator it = feeds.listIterator(); + while (it.hasNext()) { + Feed updated = updatedMap.get(it.next().getBlogId()); + if (updated != null) it.set(updated); + } + storeFeeds(txn, feeds); + }); } /** @@ -297,8 +311,13 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, return; } + if (feeds.isEmpty()) { + LOG.info("No RSS feeds to update"); + return; + } + // Fetch and update all feeds - List newFeeds = new ArrayList<>(feeds.size()); + List updatedFeeds = new ArrayList<>(feeds.size()); for (Feed feed : feeds) { try { String url = feed.getProperties().getUrl(); @@ -307,16 +326,16 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, SyndFeed sf = fetchSyndFeed(url); // sort and add new entries long lastEntryTime = postFeedEntries(feed, sf.getEntries()); - newFeeds.add(feedFactory.updateFeed(feed, sf, lastEntryTime)); + updatedFeeds.add( + feedFactory.updateFeed(feed, sf, lastEntryTime)); } catch (IOException | DbException e) { logException(LOG, WARNING, e); - newFeeds.add(feed); } } // Store updated feeds try { - storeFeeds(newFeeds); + updateFeeds(updatedFeeds); } catch (DbException e) { logException(LOG, WARNING, e); } diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java index cf7e38104..f7d011149 100644 --- a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java @@ -103,7 +103,7 @@ public class FeedManagerImplTest extends BrambleMockTestCase { public void testFetchFeedsEmptyList() throws Exception { // The list of feeds is empty expectGetFeeds(); - expectStoreFeeds(); + feedManager.setTorActive(true); feedManager.fetchFeeds(); } @@ -120,7 +120,7 @@ public class FeedManagerImplTest extends BrambleMockTestCase { Feed feed = createFeed(url, blog); expectGetFeeds(feed); - expectStoreFeeds(feed); + expectGetAndStoreFeeds(feed); feedManager.setTorActive(true); feedManager.fetchFeeds(); @@ -136,7 +136,7 @@ public class FeedManagerImplTest extends BrambleMockTestCase { Feed feed = createFeed(url, blog); expectGetFeeds(feed); - expectStoreFeeds(feed); + expectGetAndStoreFeeds(feed); feedManager.setTorActive(true); feedManager.fetchFeeds(); @@ -155,7 +155,7 @@ public class FeedManagerImplTest extends BrambleMockTestCase { expectGetFeeds(feed); expectUpdateFeedNoEntries(feed); - expectStoreFeeds(feed); + expectGetAndStoreFeeds(feed); feedManager.setTorActive(true); feedManager.fetchFeeds(); @@ -176,7 +176,7 @@ public class FeedManagerImplTest extends BrambleMockTestCase { expectGetFeeds(feed); expectUpdateFeedOneEntry(feed); - expectStoreFeeds(feed); + expectGetAndStoreFeeds(feed); feedManager.setTorActive(true); feedManager.fetchFeeds(); @@ -298,14 +298,6 @@ public class FeedManagerImplTest extends BrambleMockTestCase { }}); } - private void expectStoreFeeds(Feed... feeds) throws Exception { - Transaction txn = new Transaction(null, false); - context.checking(new DbExpectations() {{ - oneOf(db).transaction(with(false), withDbRunnable(txn)); - }}); - expectStoreFeeds(txn, feeds); - } - private void expectStoreFeeds(Transaction txn, Feed... feeds) throws Exception { BdfList feedList = new BdfList(); From 28a747f7f3c6d100f9e2a4e3b24817a8e04ac83f Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 24 Jan 2023 13:57:44 +0000 Subject: [PATCH 08/13] Add method for adding an RSS feed from an input stream. --- .../briar/api/feed/FeedManager.java | 10 ++- .../briarproject/briar/feed/FeedFactory.java | 4 +- .../briar/feed/FeedFactoryImpl.java | 3 +- .../briar/feed/FeedManagerImpl.java | 34 ++++++++-- .../briar/feed/FeedManagerImplTest.java | 64 ++++++++++++++++++- 5 files changed, 103 insertions(+), 12 deletions(-) diff --git a/briar-api/src/main/java/org/briarproject/briar/api/feed/FeedManager.java b/briar-api/src/main/java/org/briarproject/briar/api/feed/FeedManager.java index d51c2b001..0778e5901 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/feed/FeedManager.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/feed/FeedManager.java @@ -6,6 +6,7 @@ import org.briarproject.bramble.api.sync.ClientId; import org.briarproject.nullsafety.NotNullByDefault; import java.io.IOException; +import java.io.InputStream; import java.util.List; @NotNullByDefault @@ -22,10 +23,17 @@ public interface FeedManager { int MAJOR_VERSION = 0; /** - * Adds an RSS feed as a new dedicated blog. + * Adds an RSS feed as a new dedicated blog, or updates the existing blog + * if a blog for the feed already exists. */ Feed addFeed(String url) throws DbException, IOException; + /** + * Adds an RSS feed as a new dedicated blog, or updates the existing blog + * if a blog for the feed already exists. + */ + Feed addFeed(InputStream in) throws DbException, IOException; + /** * Removes an RSS feed. */ diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactory.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactory.java index cd9447f72..6abef67eb 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactory.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactory.java @@ -6,13 +6,15 @@ import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.data.BdfDictionary; import org.briarproject.briar.api.feed.Feed; +import javax.annotation.Nullable; + interface FeedFactory { /** * Create a new feed based on the feed url * and the metadata of an existing {@link SyndFeed}. */ - Feed createFeed(String url, SyndFeed sf); + Feed createFeed(@Nullable String url, SyndFeed sf); /** * Creates a new updated feed, based on the given existing feed, diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java index bd062faf7..430364cfb 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedFactoryImpl.java @@ -18,6 +18,7 @@ import org.briarproject.briar.api.blog.BlogFactory; import org.briarproject.briar.api.feed.Feed; import org.briarproject.briar.api.feed.RssProperties; +import javax.annotation.Nullable; import javax.inject.Inject; import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH; @@ -51,7 +52,7 @@ class FeedFactoryImpl implements FeedFactory { } @Override - public Feed createFeed(String url, SyndFeed sf) { + public Feed createFeed(@Nullable String url, SyndFeed sf) { String title = sf.getTitle(); if (title == null) title = "RSS"; else title = truncateUtf8(title, MAX_AUTHOR_NAME_LENGTH); diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java index 7c0be7856..ac3d1054b 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedManagerImpl.java @@ -56,6 +56,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; import java.util.logging.Logger; +import javax.annotation.Nullable; import javax.annotation.concurrent.ThreadSafe; import javax.inject.Inject; @@ -68,6 +69,7 @@ import static java.util.Collections.singletonList; import static java.util.Collections.sort; import static java.util.logging.Level.WARNING; import static java.util.logging.Logger.getLogger; +import static org.briarproject.bramble.util.IoUtils.tryToClose; import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.StringUtils.isNullOrEmpty; import static org.briarproject.bramble.util.StringUtils.truncateUtf8; @@ -170,7 +172,19 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, @Override public Feed addFeed(String url) throws DbException, IOException { // fetch feed to get posts and metadata - SyndFeed sf = fetchSyndFeed(url); + SyndFeed sf = fetchAndCleanFeed(url); + return addFeed(url, sf); + } + + @Override + public Feed addFeed(InputStream in) throws DbException, IOException { + // fetch feed to get posts and metadata + SyndFeed sf = fetchAndCleanFeed(in); + return addFeed(null, sf); + } + + private Feed addFeed(@Nullable String url, SyndFeed sf) throws DbException { + // extract properties from the feed RssProperties properties = new RssProperties(url, sf.getTitle(), sf.getDescription(), sf.getAuthor(), sf.getLink(), sf.getUri()); @@ -323,7 +337,7 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, String url = feed.getProperties().getUrl(); if (url == null) continue; // fetch and clean feed - SyndFeed sf = fetchSyndFeed(url); + SyndFeed sf = fetchAndCleanFeed(url); // sort and add new entries long lastEntryTime = postFeedEntries(feed, sf.getEntries()); updatedFeeds.add( @@ -342,11 +356,17 @@ class FeedManagerImpl implements FeedManager, EventListener, OpenDatabaseHook, LOG.info("Done updating RSS feeds"); } - private SyndFeed fetchSyndFeed(String url) throws IOException { - // fetch feed - InputStream stream = getFeedInputStream(url); - SyndFeed sf = getSyndFeed(stream); - stream.close(); + private SyndFeed fetchAndCleanFeed(String url) throws IOException { + return fetchAndCleanFeed(getFeedInputStream(url)); + } + + private SyndFeed fetchAndCleanFeed(InputStream in) throws IOException { + SyndFeed sf; + try { + sf = getSyndFeed(in); + } finally { + tryToClose(in, LOG, WARNING); + } // clean title String title = sf.getTitle(); diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java index f7d011149..382e486b3 100644 --- a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java @@ -28,6 +28,7 @@ import org.briarproject.briar.api.feed.RssProperties; import org.jmock.Expectations; import org.junit.Test; +import java.io.ByteArrayInputStream; import java.text.SimpleDateFormat; import java.util.Date; import java.util.concurrent.Executor; @@ -38,6 +39,7 @@ import okhttp3.OkHttpClient; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; +import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.singletonList; import static okhttp3.mockwebserver.SocketPolicy.DISCONNECT_DURING_RESPONSE_BODY; import static org.briarproject.bramble.test.TestUtils.getGroup; @@ -46,6 +48,7 @@ import static org.briarproject.bramble.test.TestUtils.getMessage; import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEEDS; import static org.briarproject.briar.api.feed.FeedManager.CLIENT_ID; import static org.briarproject.briar.api.feed.FeedManager.MAJOR_VERSION; +import static org.hamcrest.Matchers.nullValue; public class FeedManagerImplTest extends BrambleMockTestCase { @@ -183,7 +186,7 @@ public class FeedManagerImplTest extends BrambleMockTestCase { } @Test - public void testAddNewFeed() throws Exception { + public void testAddNewFeedFromUrl() throws Exception { // Fetching and parsing the feed will succeed; there are no entries String feedXml = createRssFeedXml(); @@ -223,7 +226,7 @@ public class FeedManagerImplTest extends BrambleMockTestCase { } @Test - public void testAddExistingFeed() throws Exception { + public void testAddExistingFeedFromUrl() throws Exception { // Fetching and parsing the feed will succeed; there are no entries String feedXml = createRssFeedXml(); @@ -248,6 +251,63 @@ public class FeedManagerImplTest extends BrambleMockTestCase { feedManager.addFeed(url); } + @Test + public void testAddNewFeedFromInputStream() throws Exception { + // Reading and parsing the feed will succeed; there are no entries + String feedXml = createRssFeedXml(); + Feed newFeed = createFeed(null, blog); + + Group existingBlogGroup = getGroup(BlogManager.CLIENT_ID, + BlogManager.MAJOR_VERSION); + Blog existingBlog = new Blog(existingBlogGroup, localAuthor, true); + Feed existingFeed = createFeed(null, existingBlog); + + expectGetFeeds(existingFeed); + + context.checking(new DbExpectations() {{ + // The added feed doesn't match any existing feed + oneOf(feedMatcher).findMatchingFeed(with(any(RssProperties.class)), + with(singletonList(existingFeed))); + will(returnValue(null)); + // Create the new feed + oneOf(feedFactory).createFeed(with(nullValue(String.class)), + with(any(SyndFeed.class))); + will(returnValue(newFeed)); + // Add the new feed to the list of feeds + Transaction txn = new Transaction(null, false); + oneOf(db).transaction(with(false), withDbRunnable(txn)); + oneOf(blogManager).addBlog(txn, blog); + expectGetFeeds(txn, existingFeed); + expectStoreFeeds(txn, existingFeed, newFeed); + }}); + + expectUpdateFeedNoEntries(newFeed); + expectGetAndStoreFeeds(existingFeed, newFeed); + + feedManager.addFeed(new ByteArrayInputStream(feedXml.getBytes(UTF_8))); + } + + @Test + public void testAddExistingFeedFromInputStream() throws Exception { + // Reading and parsing the feed will succeed; there are no entries + String feedXml = createRssFeedXml(); + Feed newFeed = createFeed(null, blog); + + expectGetFeeds(newFeed); + + context.checking(new DbExpectations() {{ + // The added feed matches an existing feed + oneOf(feedMatcher).findMatchingFeed(with(any(RssProperties.class)), + with(singletonList(newFeed))); + will(returnValue(newFeed)); + }}); + + expectUpdateFeedNoEntries(newFeed); + expectGetAndStoreFeeds(newFeed); + + feedManager.addFeed(new ByteArrayInputStream(feedXml.getBytes(UTF_8))); + } + private Feed createFeed(String url, Blog blog) { RssProperties properties = new RssProperties(url, null, null, null, null, null); From 4007fca6689317fc18c8578b2e0643421d6b2ac0 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 24 Jan 2023 14:13:38 +0000 Subject: [PATCH 09/13] Add integration tests for importing an RSS feed from a file. --- .../feed/FeedManagerIntegrationTest.java | 48 ++++++++-- .../resources/briarproject.org_news_index.xml | 95 +++++++++++++++++++ 2 files changed, 133 insertions(+), 10 deletions(-) create mode 100644 briar-core/src/test/resources/briarproject.org_news_index.xml diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTest.java index 56aa1637a..42480694c 100644 --- a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerIntegrationTest.java @@ -5,30 +5,43 @@ import org.briarproject.bramble.api.identity.IdentityManager; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.test.BrambleTestCase; import org.briarproject.bramble.test.TestDatabaseConfigModule; -import org.briarproject.bramble.test.TestUtils; import org.briarproject.briar.api.blog.Blog; import org.briarproject.briar.api.blog.BlogManager; import org.briarproject.briar.api.blog.BlogPostHeader; import org.briarproject.briar.api.feed.Feed; import org.briarproject.briar.api.feed.FeedManager; +import org.briarproject.nullsafety.NullSafety; import org.junit.After; import org.junit.Before; import org.junit.Test; import java.io.File; +import java.io.FileInputStream; import java.util.Collection; +import javax.annotation.Nullable; + +import static org.briarproject.bramble.test.TestUtils.deleteTestDirectory; import static org.briarproject.bramble.test.TestUtils.getSecretKey; +import static org.briarproject.bramble.test.TestUtils.getTestDirectory; +import static org.briarproject.nullsafety.NullSafety.requireNonNull; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; public class FeedManagerIntegrationTest extends BrambleTestCase { + private static final String FEED_PATH = + "src/test/resources/briarproject.org_news_index.xml"; + private static final String FEED_URL = + "https://briarproject.org/news/index.xml"; + private static final String FEED_TITLE = "News - Briar"; + private LifecycleManager lifecycleManager; private FeedManager feedManager; private BlogManager blogManager; - private final File testDir = TestUtils.getTestDirectory(); + private final File testDir = getTestDirectory(); private final File testFile = new File(testDir, "feedTest"); @Before @@ -38,7 +51,8 @@ public class FeedManagerIntegrationTest extends BrambleTestCase { DaggerFeedManagerIntegrationTestComponent.builder() .testDatabaseConfigModule( new TestDatabaseConfigModule(testFile)).build(); - FeedManagerIntegrationTestComponent.Helper.injectEagerSingletons(component); + FeedManagerIntegrationTestComponent.Helper + .injectEagerSingletons(component); component.inject(this); IdentityManager identityManager = component.getIdentityManager(); @@ -54,17 +68,30 @@ public class FeedManagerIntegrationTest extends BrambleTestCase { } @Test - public void testFeedImportAndRemoval() throws Exception { + public void testFeedImportAndRemovalFromUrl() throws Exception { + testFeedImportAndRemoval(FEED_URL, null); + } + + @Test + public void testFeedImportAndRemovalFromFile() throws Exception { + testFeedImportAndRemoval(null, FEED_PATH); + } + + private void testFeedImportAndRemoval(@Nullable String url, + @Nullable String path) throws Exception { // initially, there's only the one personal blog Collection blogs = blogManager.getBlogs(); assertEquals(1, blogs.size()); Blog personalBlog = blogs.iterator().next(); // add feed into a dedicated blog - String url = "https://www.schneier.com/blog/atom.xml"; - feedManager.addFeed(url); + if (url == null) { + feedManager.addFeed(new FileInputStream(requireNonNull(path))); + } else { + feedManager.addFeed(url); + } - // then there's the feed's blog now + // now there's the feed's blog too blogs = blogManager.getBlogs(); assertEquals(2, blogs.size()); Blog feedBlog = null; @@ -80,15 +107,16 @@ public class FeedManagerIntegrationTest extends BrambleTestCase { assertTrue(feed.getLastEntryTime() > 0); assertTrue(feed.getAdded() > 0); assertTrue(feed.getUpdated() > 0); - assertEquals(url, feed.getProperties().getUrl()); + assertTrue(NullSafety.equals(url, feed.getProperties().getUrl())); assertEquals(feedBlog, feed.getBlog()); - assertEquals("Schneier on Security", feed.getTitle()); + assertEquals(FEED_TITLE, feed.getTitle()); assertEquals(feed.getTitle(), feed.getBlog().getName()); assertEquals(feed.getTitle(), feed.getLocalAuthor().getName()); // check the feed entries have been added to the blog as expected Collection headers = blogManager.getPostHeaders(feedBlog.getId()); + assertFalse(headers.isEmpty()); for (BlogPostHeader header : headers) { assertTrue(header.isRssFeed()); } @@ -105,6 +133,6 @@ public class FeedManagerIntegrationTest extends BrambleTestCase { public void tearDown() throws Exception { lifecycleManager.stopServices(); lifecycleManager.waitForShutdown(); - TestUtils.deleteTestDirectory(testDir); + deleteTestDirectory(testDir); } } diff --git a/briar-core/src/test/resources/briarproject.org_news_index.xml b/briar-core/src/test/resources/briarproject.org_news_index.xml new file mode 100644 index 000000000..7d27dfbc1 --- /dev/null +++ b/briar-core/src/test/resources/briarproject.org_news_index.xml @@ -0,0 +1,95 @@ + + + News - Briar + + + + 2022-12-05T12:00:00+00:00 + https://briarproject.org/news/ + + The Briar Team + + Hugo -- gohugo.io + + <![CDATA[Briar Desktop got another round of funding]]> + + https://briarproject.org/news/2022-briar-desktop-nlnet-funding/ + 2022-12-05T12:00:00+00:00 + 2022-12-05T12:00:00+00:00 + + + + + <![CDATA[Briar is available on Google Play again]]> + + https://briarproject.org/news/2022-briar-removed-from-google-play/ + 2022-02-28T13:20:00+00:00 + 2022-02-28T13:20:00+00:00 + + + + + <![CDATA[Briar 1.4 released - offline app sharing, message transfer via SD cards and USB sticks]]> + + https://briarproject.org/news/2021-briar-1.4-released/ + 2021-11-15T00:00:00+00:00 + 2021-11-15T00:00:00+00:00 + + + + + <![CDATA[Briar 1.3 released - image attachments, profile images and disappearing messages]]> + + https://briarproject.org/news/2021-briar-1.3-released/ + 2021-06-07T00:00:00+00:00 + 2021-06-07T00:00:00+00:00 + + + + + <![CDATA[Briar 1.2 released, contacts can now be added by exchanging links]]> + + https://briarproject.org/news/2019-briar-1.2-released-remote-contacts/ + 2019-11-06T00:00:00+00:00 + 2019-11-06T00:00:00+00:00 + + + + + <![CDATA[Briar 1.1 released with dark theme, new emoji and more]]> + + https://briarproject.org/news/2018-briar-1.1-released/ + 2018-09-14T00:00:00+00:00 + 2018-09-14T00:00:00+00:00 + + + + + <![CDATA[Briar - Secure P2P Messenger Releases First Version, Receives New Funding]]> + + https://briarproject.org/news/2018-1.0-released-new-funding/ + 2018-05-09T00:00:00+00:00 + 2018-05-09T00:00:00+00:00 + + + + + <![CDATA[Briar - Darknet Messenger Releases Beta, Passes Security Audit]]> + + https://briarproject.org/news/2017-beta-released-security-audit/ + 2017-07-21T00:00:00+00:00 + 2017-07-21T00:00:00+00:00 + + + + From 6faa095dfb364dd1562eb28ae63d95068401d7cc Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 24 Jan 2023 14:48:55 +0000 Subject: [PATCH 10/13] FeedMatcher interface doesn't need to be public. --- .../src/main/java/org/briarproject/briar/feed/FeedMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcher.java b/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcher.java index db31c6792..400398bb3 100644 --- a/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcher.java +++ b/briar-core/src/main/java/org/briarproject/briar/feed/FeedMatcher.java @@ -11,7 +11,7 @@ import javax.annotation.concurrent.ThreadSafe; @ThreadSafe @NotNullByDefault -public interface FeedMatcher { +interface FeedMatcher { /** * Returns the best match for the given candidate from the given list of From 6eda2f6d136074f3661921548f3a33abde0eba50 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 24 Jan 2023 14:50:40 +0000 Subject: [PATCH 11/13] AnimalSniffer doesn't allow StandardCharsets in tests. --- .../java/org/briarproject/briar/feed/FeedManagerImplTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java index 382e486b3..2de44e1d7 100644 --- a/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedManagerImplTest.java @@ -39,12 +39,12 @@ import okhttp3.OkHttpClient; import okhttp3.mockwebserver.MockResponse; import okhttp3.mockwebserver.MockWebServer; -import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Collections.singletonList; import static okhttp3.mockwebserver.SocketPolicy.DISCONNECT_DURING_RESPONSE_BODY; import static org.briarproject.bramble.test.TestUtils.getGroup; import static org.briarproject.bramble.test.TestUtils.getLocalAuthor; import static org.briarproject.bramble.test.TestUtils.getMessage; +import static org.briarproject.bramble.util.StringUtils.UTF_8; import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEEDS; import static org.briarproject.briar.api.feed.FeedManager.CLIENT_ID; import static org.briarproject.briar.api.feed.FeedManager.MAJOR_VERSION; From cc5365eaf046bebfbdcdce94b92a196fa7453f70 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 24 Jan 2023 17:27:34 +0000 Subject: [PATCH 12/13] Remove redundant comparison from test. --- .../org/briarproject/briar/feed/FeedMatcherImplTest.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedMatcherImplTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedMatcherImplTest.java index e244a997e..955ecb7b9 100644 --- a/briar-core/src/test/java/org/briarproject/briar/feed/FeedMatcherImplTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedMatcherImplTest.java @@ -88,14 +88,10 @@ public class FeedMatcherImplTest extends BrambleTestCase { // The second feed has the given RSS fields Feed feed2 = createFeed(new RssProperties(null, title, description, author, link, uri)); - // The third feed has no matching RSS fields - Feed feed3 = createFeed(new RssProperties(null, - nope(), nope(), nope(), nope(), nope())); - Feed match = matcher.findMatchingFeed(candidate, - asList(feed1, feed2, feed3)); + Feed match = matcher.findMatchingFeed(candidate, asList(feed1, feed2)); - // The matcher should not choose any of the feeds + // The matcher should not choose either of the feeds assertNull(match); } From abd04ee7f5528e8afef52db4eece4e4f2439645a Mon Sep 17 00:00:00 2001 From: akwizgran Date: Tue, 24 Jan 2023 17:27:52 +0000 Subject: [PATCH 13/13] Add tests for feed serialisation/deserialisation. --- .../briar/feed/FeedFactoryImplTest.java | 159 ++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 briar-core/src/test/java/org/briarproject/briar/feed/FeedFactoryImplTest.java diff --git a/briar-core/src/test/java/org/briarproject/briar/feed/FeedFactoryImplTest.java b/briar-core/src/test/java/org/briarproject/briar/feed/FeedFactoryImplTest.java new file mode 100644 index 000000000..5d5bd2b98 --- /dev/null +++ b/briar-core/src/test/java/org/briarproject/briar/feed/FeedFactoryImplTest.java @@ -0,0 +1,159 @@ +package org.briarproject.briar.feed; + +import org.briarproject.bramble.api.client.ClientHelper; +import org.briarproject.bramble.api.data.BdfDictionary; +import org.briarproject.bramble.api.data.BdfEntry; +import org.briarproject.bramble.api.data.BdfList; +import org.briarproject.bramble.api.identity.AuthorFactory; +import org.briarproject.bramble.api.identity.LocalAuthor; +import org.briarproject.bramble.api.sync.Group; +import org.briarproject.bramble.api.system.Clock; +import org.briarproject.bramble.test.BrambleMockTestCase; +import org.briarproject.briar.api.blog.Blog; +import org.briarproject.briar.api.blog.BlogFactory; +import org.briarproject.briar.api.feed.Feed; +import org.briarproject.briar.api.feed.RssProperties; +import org.jmock.Expectations; +import org.junit.Test; + +import static org.briarproject.bramble.test.TestUtils.getGroup; +import static org.briarproject.bramble.test.TestUtils.getLocalAuthor; +import static org.briarproject.bramble.util.StringUtils.getRandomString; +import static org.briarproject.briar.api.blog.BlogManager.CLIENT_ID; +import static org.briarproject.briar.api.blog.BlogManager.MAJOR_VERSION; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_ADDED; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_AUTHOR; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_DESC; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_LAST_ENTRY; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_PRIVATE_KEY; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_RSS_AUTHOR; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_RSS_LINK; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_RSS_TITLE; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_RSS_URI; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_UPDATED; +import static org.briarproject.briar.api.feed.FeedConstants.KEY_FEED_URL; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +public class FeedFactoryImplTest extends BrambleMockTestCase { + + private final AuthorFactory authorFactory = + context.mock(AuthorFactory.class); + private final BlogFactory blogFactory = context.mock(BlogFactory.class); + private final ClientHelper clientHelper = context.mock(ClientHelper.class); + private final Clock clock = context.mock(Clock.class); + + private final LocalAuthor localAuthor = getLocalAuthor(); + private final Group blogGroup = getGroup(CLIENT_ID, MAJOR_VERSION); + private final Blog blog = new Blog(blogGroup, localAuthor, true); + private final BdfList authorList = BdfList.of("foo"); + private final long added = 123, updated = 234, lastEntryTime = 345; + + private final String url = getRandomString(123); + private final String description = getRandomString(123); + private final String rssAuthor = getRandomString(123); + private final String title = getRandomString(123); + private final String link = getRandomString(123); + private final String uri = getRandomString(123); + + private final FeedFactoryImpl feedFactory = new FeedFactoryImpl( + authorFactory, blogFactory, clientHelper, clock); + + @Test + public void testSerialiseAndDeserialiseWithoutOptionalFields() + throws Exception { + RssProperties propertiesBefore = new RssProperties(null, null, null, + null, null, null); + Feed before = new Feed(blog, localAuthor, propertiesBefore, added, + updated, lastEntryTime); + + + context.checking(new Expectations() {{ + oneOf(clientHelper).toList(localAuthor); + will(returnValue(authorList)); + }}); + + BdfDictionary dict = feedFactory.feedToBdfDictionary(before); + + BdfDictionary expectedDict = BdfDictionary.of( + new BdfEntry(KEY_FEED_AUTHOR, authorList), + new BdfEntry(KEY_FEED_PRIVATE_KEY, localAuthor.getPrivateKey()), + new BdfEntry(KEY_FEED_ADDED, added), + new BdfEntry(KEY_FEED_UPDATED, updated), + new BdfEntry(KEY_FEED_LAST_ENTRY, lastEntryTime) + ); + assertEquals(expectedDict, dict); + + context.checking(new Expectations() {{ + oneOf(clientHelper).parseAndValidateAuthor(authorList); + will(returnValue(localAuthor)); + oneOf(blogFactory).createFeedBlog(localAuthor); + will(returnValue(blog)); + }}); + + Feed after = feedFactory.createFeed(dict); + RssProperties afterProperties = after.getProperties(); + + assertNull(afterProperties.getUrl()); + assertNull(afterProperties.getTitle()); + assertNull(afterProperties.getDescription()); + assertNull(afterProperties.getAuthor()); + assertNull(afterProperties.getLink()); + assertNull(afterProperties.getUri()); + assertEquals(added, after.getAdded()); + assertEquals(updated, after.getUpdated()); + assertEquals(lastEntryTime, after.getLastEntryTime()); + } + + @Test + public void testSerialiseAndDeserialiseWithOptionalFields() + throws Exception { + RssProperties propertiesBefore = new RssProperties(url, title, + description, rssAuthor, link, uri); + Feed before = new Feed(blog, localAuthor, propertiesBefore, added, + updated, lastEntryTime); + + + context.checking(new Expectations() {{ + oneOf(clientHelper).toList(localAuthor); + will(returnValue(authorList)); + }}); + + BdfDictionary dict = feedFactory.feedToBdfDictionary(before); + + BdfDictionary expectedDict = BdfDictionary.of( + new BdfEntry(KEY_FEED_AUTHOR, authorList), + new BdfEntry(KEY_FEED_PRIVATE_KEY, localAuthor.getPrivateKey()), + new BdfEntry(KEY_FEED_ADDED, added), + new BdfEntry(KEY_FEED_UPDATED, updated), + new BdfEntry(KEY_FEED_LAST_ENTRY, lastEntryTime), + new BdfEntry(KEY_FEED_URL, url), + new BdfEntry(KEY_FEED_RSS_TITLE, title), + new BdfEntry(KEY_FEED_DESC, description), + new BdfEntry(KEY_FEED_RSS_AUTHOR, rssAuthor), + new BdfEntry(KEY_FEED_RSS_LINK, link), + new BdfEntry(KEY_FEED_RSS_URI, uri) + ); + assertEquals(expectedDict, dict); + + context.checking(new Expectations() {{ + oneOf(clientHelper).parseAndValidateAuthor(authorList); + will(returnValue(localAuthor)); + oneOf(blogFactory).createFeedBlog(localAuthor); + will(returnValue(blog)); + }}); + + Feed after = feedFactory.createFeed(dict); + RssProperties afterProperties = after.getProperties(); + + assertEquals(url, afterProperties.getUrl()); + assertEquals(title, afterProperties.getTitle()); + assertEquals(description, afterProperties.getDescription()); + assertEquals(rssAuthor, afterProperties.getAuthor()); + assertEquals(link, afterProperties.getLink()); + assertEquals(uri, afterProperties.getUri()); + assertEquals(added, after.getAdded()); + assertEquals(updated, after.getUpdated()); + assertEquals(lastEntryTime, after.getLastEntryTime()); + } +}