Match newly added RSS feeds to existing feeds.

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"
 

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<OkHttpClient> 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<OkHttpClient> 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<Feed> 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<Feed> 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<Feed> 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<SyndEntry> entries)
+	private long postFeedEntries(Feed feed, List<SyndEntry> entries)
 			throws DbException {
 
 		return db.transactionWithResult(false, txn -> {

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<Feed> feeds) {

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<OkHttpClient> httpClientProvider =
 			new WeakSingletonProvider<OkHttpClient>() {
 				@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<SyndEntry> entries = new ArrayList<>();
-		entries.add(new SyndEntryImpl());
-		SyndEntry entry = new SyndEntryImpl();
-		entry.setUpdatedDate(new Date());
-		entries.add(entry);
-		String text = "<p>(" + entry.getUpdatedDate().toString() + ")</p>";
-		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 =
+				"<item><pubDate>" + pubDateString + "</pubDate></item>";
+		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("<rss version='2.0'><channel>");
+		for (String entry : entries) sb.append(entry);
+		sb.append("</channel></rss>");
+		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 = "<p>(" + new Date(pubDate) + ")</p>";
+
+		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));
+		}});
+	}
 }