From 72fb4e9bc73aa71e9f1d4ea624df56798938d67e Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Wed, 24 Aug 2016 12:39:04 -0300
Subject: [PATCH] Post RSS entries within one transaction

This also fixes a bug where new feeds was not added properly.
---
 .../briarproject/api/blogs/BlogManager.java   |  3 +
 .../src/org/briarproject/api/feed/Feed.java   | 21 ++++++
 .../briarproject/blogs/BlogManagerImpl.java   | 30 ++++----
 .../briarproject/feed/FeedManagerImpl.java    | 73 ++++++++++++-------
 .../blogs/BlogManagerImplTest.java            |  5 +-
 5 files changed, 89 insertions(+), 43 deletions(-)

diff --git a/briar-api/src/org/briarproject/api/blogs/BlogManager.java b/briar-api/src/org/briarproject/api/blogs/BlogManager.java
index 368069f11f..f0f4bc0a12 100644
--- a/briar-api/src/org/briarproject/api/blogs/BlogManager.java
+++ b/briar-api/src/org/briarproject/api/blogs/BlogManager.java
@@ -29,6 +29,9 @@ public interface BlogManager {
 	/** Stores a local blog post. */
 	void addLocalPost(BlogPost p) throws DbException;
 
+	/** Stores a local blog post. */
+	void addLocalPost(Transaction txn, BlogPost p) throws DbException;
+
 	/** Returns the blog with the given ID. */
 	Blog getBlog(GroupId g) throws DbException;
 
diff --git a/briar-api/src/org/briarproject/api/feed/Feed.java b/briar-api/src/org/briarproject/api/feed/Feed.java
index 2dd1eb9b15..502923c4b0 100644
--- a/briar-api/src/org/briarproject/api/feed/Feed.java
+++ b/briar-api/src/org/briarproject/api/feed/Feed.java
@@ -110,4 +110,25 @@ public class Feed {
 		return lastEntryTime;
 	}
 
+	@Override
+	public boolean equals(Object o) {
+		if (this == o) return true;
+		if (o instanceof Feed) {
+			Feed f = (Feed) o;
+			return url.equals(f.url) && blogId.equals(f.getBlogId()) &&
+					equalsWithNull(title, f.getTitle()) &&
+					equalsWithNull(description, f.getDescription()) &&
+					equalsWithNull(author, f.getAuthor()) &&
+					added == f.getAdded() &&
+					updated == f.getUpdated() &&
+					lastEntryTime == f.getLastEntryTime();
+		}
+		return false;
+	}
+
+	private boolean equalsWithNull(Object a, Object b) {
+		if (a == b) return true;
+		if (a == null || b==null) return false;
+		return a.equals(b);
+	}
 }
diff --git a/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java b/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java
index 543b97e0a1..c58a2d0d84 100644
--- a/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java
+++ b/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java
@@ -234,9 +234,20 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 
 	@Override
 	public void addLocalPost(BlogPost p) throws DbException {
-		BdfDictionary meta;
+		Transaction txn = db.startTransaction(false);
+		try {
+			addLocalPost(txn, p);
+			txn.setComplete();
+		} finally {
+			//noinspection ThrowFromFinallyBlock
+			db.endTransaction(txn);
+		}
+	}
+
+	@Override
+	public void addLocalPost(Transaction txn, BlogPost p) throws DbException {
 		try {
-			meta = new BdfDictionary();
+			BdfDictionary meta = new BdfDictionary();
 			if (p.getTitle() != null) meta.put(KEY_TITLE, p.getTitle());
 			meta.put(KEY_TIMESTAMP, p.getMessage().getTimestamp());
 
@@ -249,25 +260,18 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
 
 			meta.put(KEY_CONTENT_TYPE, p.getContentType());
 			meta.put(KEY_READ, true);
-			clientHelper.addLocalMessage(p.getMessage(), CLIENT_ID, meta, true);
-		} catch (FormatException e) {
-			throw new RuntimeException(e);
-		}
+			clientHelper.addLocalMessage(txn, p.getMessage(), CLIENT_ID, meta,
+					true);
 
-		// broadcast event about new post
-		Transaction txn = db.startTransaction(true);
-		try {
+			// broadcast event about new post
 			GroupId groupId = p.getMessage().getGroupId();
 			MessageId postId = p.getMessage().getId();
 			BlogPostHeader h = getPostHeaderFromMetadata(txn, postId, meta);
 			BlogPostAddedEvent event =
 					new BlogPostAddedEvent(groupId, h, true);
 			txn.attach(event);
-			txn.setComplete();
 		} catch (FormatException e) {
-			if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
-		} finally {
-			db.endTransaction(txn);
+			throw new DbException(e);
 		}
 	}
 
diff --git a/briar-core/src/org/briarproject/feed/FeedManagerImpl.java b/briar-core/src/org/briarproject/feed/FeedManagerImpl.java
index a1331a78c6..9379840da7 100644
--- a/briar-core/src/org/briarproject/feed/FeedManagerImpl.java
+++ b/briar-core/src/org/briarproject/feed/FeedManagerImpl.java
@@ -165,13 +165,13 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 			storeFeeds(txn, feeds);
 			txn.setComplete();
 		} finally {
-			//noinspection ThrowFromFinallyBlock
 			db.endTransaction(txn);
 		}
 
 		// fetch feed again, post entries this time
+		Feed updatedFeed;
 		try {
-			feed = fetchFeed(feed, true);
+			updatedFeed = fetchFeed(feed, true);
 		} catch (FeedException e) {
 			throw new IOException(e);
 		}
@@ -180,11 +180,11 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 		txn = db.startTransaction(false);
 		try {
 			List<Feed> feeds = getFeeds(txn);
-			feeds.add(feed);
+			feeds.remove(feed);
+			feeds.add(updatedFeed);
 			storeFeeds(txn, feeds);
 			txn.setComplete();
 		} finally {
-			//noinspection ThrowFromFinallyBlock
 			db.endTransaction(txn);
 		}
 	}
@@ -294,6 +294,9 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 			} catch (IOException e) {
 				if (LOG.isLoggable(WARNING))
 					LOG.log(WARNING, e.toString(), e);
+			} catch (DbException e) {
+				if (LOG.isLoggable(WARNING))
+					LOG.log(WARNING, e.toString(), e);
 			}
 		}
 
@@ -308,7 +311,7 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 	}
 
 	private Feed fetchFeed(Feed feed, boolean post)
-			throws FeedException, IOException {
+			throws FeedException, IOException, DbException {
 		if (LOG.isLoggable(INFO))
 			LOG.info("Fetching feed from " + feed.getUrl());
 
@@ -331,24 +334,7 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 
 		// sort and add new entries
 		if (post) {
-			Collections.sort(f.getEntries(), getEntryComparator());
-			for (SyndEntry entry : f.getEntries()) {
-				long entryTime;
-				if (entry.getPublishedDate() != null) {
-					entryTime = entry.getPublishedDate().getTime();
-				} else if (entry.getUpdatedDate() != null) {
-					entryTime = entry.getUpdatedDate().getTime();
-				} else {
-					// no time information available, ignore this entry
-					if (LOG.isLoggable(WARNING))
-						LOG.warning("Entry has no date: " + entry.getTitle());
-					continue;
-				}
-				if (entryTime > feed.getLastEntryTime()) {
-					postEntry(feed, entry);
-					if (entryTime > lastEntryTime) lastEntryTime = entryTime;
-				}
-			}
+			lastEntryTime = postFeedEntries(feed, f.getEntries());
 		}
 		return new Feed(feed.getUrl(), feed.getBlogId(), title, description,
 				author, feed.getAdded(), updated, lastEntryTime);
@@ -384,9 +370,40 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 		return input.build(new XmlReader(stream));
 	}
 
-	private void postEntry(Feed feed, SyndEntry entry) {
+	private long postFeedEntries(Feed feed, List<SyndEntry> entries)
+			throws DbException {
+
+		long lastEntryTime = feed.getLastEntryTime();
+		Transaction txn = db.startTransaction(false);
+		try {
+			Collections.sort(entries, getEntryComparator());
+			for (SyndEntry entry : entries) {
+				long entryTime;
+				if (entry.getPublishedDate() != null) {
+					entryTime = entry.getPublishedDate().getTime();
+				} else if (entry.getUpdatedDate() != null) {
+					entryTime = entry.getUpdatedDate().getTime();
+				} else {
+					// no time information available, ignore this entry
+					if (LOG.isLoggable(WARNING))
+						LOG.warning("Entry has no date: " + entry.getTitle());
+					continue;
+				}
+				if (entryTime > feed.getLastEntryTime()) {
+					postEntry(txn, feed, entry);
+					if (entryTime > lastEntryTime) lastEntryTime = entryTime;
+				}
+			}
+			txn.setComplete();
+		} finally {
+			db.endTransaction(txn);
+		}
+		return lastEntryTime;
+	}
+
+	private void postEntry(Transaction txn, Feed feed, SyndEntry entry)
+			throws DbException {
 		LOG.info("Adding new entry...");
-		// TODO do this within one database transaction?
 
 		// build post body
 		StringBuilder b = new StringBuilder();
@@ -426,13 +443,13 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 		byte[] body = getPostBody(b.toString());
 		try {
 			// create and store post
-			Blog blog = blogManager.getBlog(groupId);
+			Blog blog = blogManager.getBlog(txn, groupId);
 			AuthorId authorId = blog.getAuthor().getId();
-			LocalAuthor author = identityManager.getLocalAuthor(authorId);
+			LocalAuthor author = identityManager.getLocalAuthor(txn, authorId);
 			BlogPost post = blogPostFactory
 					.createBlogPost(groupId, null, time, null, author,
 							"text/plain", body);
-			blogManager.addLocalPost(post);
+			blogManager.addLocalPost(txn, post);
 		} catch (DbException e) {
 			if (LOG.isLoggable(WARNING))
 				LOG.log(WARNING, e.toString(), e);
diff --git a/briar-tests/src/org/briarproject/blogs/BlogManagerImplTest.java b/briar-tests/src/org/briarproject/blogs/BlogManagerImplTest.java
index 5b380bdda0..9171103303 100644
--- a/briar-tests/src/org/briarproject/blogs/BlogManagerImplTest.java
+++ b/briar-tests/src/org/briarproject/blogs/BlogManagerImplTest.java
@@ -284,9 +284,10 @@ public class BlogManagerImplTest extends BriarTestCase {
 		);
 
 		context.checking(new Expectations() {{
-			oneOf(clientHelper).addLocalMessage(message, CLIENT_ID, meta, true);
-			oneOf(db).startTransaction(true);
+			oneOf(db).startTransaction(false);
 			will(returnValue(txn));
+			oneOf(clientHelper)
+					.addLocalMessage(txn, message, CLIENT_ID, meta, true);
 			oneOf(identityManager)
 					.getAuthorStatus(txn, blog1.getAuthor().getId());
 			will(returnValue(VERIFIED));
-- 
GitLab