Merge branch '483-design-ux-for-importing-an-rss-feed' into 'master'

Add feed title to imported entries

Also fixes one bug where a new feed was not saved and improved HTML stripping a bit.

Closes #483

See merge request !287

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;
 

briar-api/src/org/briarproject/api/feed/Feed.java

@@ -83,14 +83,17 @@ public class Feed {
 				lastEntryTime);
 	}
 
+	@Nullable
 	public String getTitle() {
 		return title;
 	}
 
+	@Nullable
 	public String getDescription() {
 		return description;
 	}
 
+	@Nullable
 	public String getAuthor() {
 		return author;
 	}
@@ -107,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);
+	}
 }

briar-api/src/org/briarproject/api/identity/IdentityManager.java

@@ -20,6 +20,9 @@ public interface IdentityManager {
 	/** Returns the local pseudonym with the given ID. */
 	LocalAuthor getLocalAuthor(AuthorId a) throws DbException;
 
+	/** Returns the local pseudonym with the given ID. */
+	LocalAuthor getLocalAuthor(Transaction txn, AuthorId a) throws DbException;
+
 	/** Returns the main local identity. */
 	LocalAuthor getLocalAuthor() throws DbException;
 

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 {
-			meta = new BdfDictionary();
+			addLocalPost(txn, p);
+			txn.setComplete();
+		} finally {
+			//noinspection ThrowFromFinallyBlock
+			db.endTransaction(txn);
+		}
+	}
+
+	@Override
+	public void addLocalPost(Transaction txn, BlogPost p) throws DbException {
+		try {
+			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);
 		}
 	}
 

briar-core/src/org/briarproject/feed/FeedManagerImpl.java

@@ -149,18 +149,41 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 		LOG.info("Adding new RSS feed...");
 
 		// TODO check for existing feed?
+		// fetch feed to get its metadata
 		Feed feed = new Feed(url, g, clock.currentTimeMillis());
 		try {
-			feed = fetchFeed(feed);
+			feed = fetchFeed(feed, false);
 		} catch (FeedException e) {
 			throw new IOException(e);
 		}
 
+		// store feed
 		Transaction txn = db.startTransaction(false);
 		try {
 			List<Feed> feeds = getFeeds(txn);
 			feeds.add(feed);
 			storeFeeds(txn, feeds);
+			txn.setComplete();
+		} finally {
+			db.endTransaction(txn);
+		}
+
+		// fetch feed again, post entries this time
+		Feed updatedFeed;
+		try {
+			updatedFeed = fetchFeed(feed, true);
+		} catch (FeedException e) {
+			throw new IOException(e);
+		}
+
+		// store feed again to also store last added entry
+		txn = db.startTransaction(false);
+		try {
+			List<Feed> feeds = getFeeds(txn);
+			feeds.remove(feed);
+			feeds.add(updatedFeed);
+			storeFeeds(txn, feeds);
+			txn.setComplete();
 		} finally {
 			db.endTransaction(txn);
 		}
@@ -182,6 +205,7 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 			}
 			if (!found) throw new DbException();
 			storeFeeds(txn, feeds);
+			txn.setComplete();
 		} finally {
 			db.endTransaction(txn);
 		}
@@ -263,13 +287,16 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 		List<Feed> newFeeds = new ArrayList<Feed>(feeds.size());
 		for (Feed feed : feeds) {
 			try {
-				newFeeds.add(fetchFeed(feed));
+				newFeeds.add(fetchFeed(feed, true));
 			} catch (FeedException e) {
 				if (LOG.isLoggable(WARNING))
 					LOG.log(WARNING, e.toString(), e);
 			} 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);
 			}
 		}
 
@@ -283,7 +310,8 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 		LOG.info("Done updating RSS feeds");
 	}
 
-	private Feed fetchFeed(Feed feed) throws FeedException, IOException {
+	private Feed fetchFeed(Feed feed, boolean post)
+			throws FeedException, IOException, DbException {
 		if (LOG.isLoggable(INFO))
 			LOG.info("Fetching feed from " + feed.getUrl());
 
@@ -301,24 +329,12 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 				StringUtils.isNullOrEmpty(f.getAuthor()) ? null : f.getAuthor();
 		if (author != null) author = stripHTML(author);
 
+		if (f.getEntries().size() == 0)
+			throw new FeedException("Feed has no entries");
+
 		// sort and add new entries
-		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;
-			}
+		if (post) {
+			lastEntryTime = postFeedEntries(feed, f.getEntries());
 		}
 		return new Feed(feed.getUrl(), feed.getBlogId(), title, description,
 				author, feed.getAdded(), updated, lastEntryTime);
@@ -354,11 +370,47 @@ 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...");
 
 		// build post body
 		StringBuilder b = new StringBuilder();
+		if (feed.getTitle() != null) {
+			// HTML in feed title was already stripped
+			b.append(feed.getTitle()).append("\n\n");
+		}
 		if (!StringUtils.isNullOrEmpty(entry.getTitle())) {
 			b.append(stripHTML(entry.getTitle())).append("\n\n");
 		}
@@ -391,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);
@@ -416,7 +468,8 @@ class FeedManagerImpl implements FeedManager, Service, Client {
 	}
 
 	private String stripHTML(String s) {
-		return StringUtils.trim(s.replaceAll("<.*?>", ""));
+		s = s.replaceAll("<script.*?>(?s).*?</script>", "");
+		return StringUtils.trim(s.replaceAll("<(?s).*?>", ""));
 	}
 
 	private byte[] getPostBody(String text) {

briar-core/src/org/briarproject/identity/IdentityManagerImpl.java

@@ -60,7 +60,7 @@ class IdentityManagerImpl implements IdentityManager {
 		LocalAuthor author;
 		Transaction txn = db.startTransaction(true);
 		try {
-			author = db.getLocalAuthor(txn, a);
+			author = getLocalAuthor(txn, a);
 			txn.setComplete();
 		} finally {
 			db.endTransaction(txn);
@@ -68,6 +68,12 @@ class IdentityManagerImpl implements IdentityManager {
 		return author;
 	}
 
+	@Override
+	public LocalAuthor getLocalAuthor(Transaction txn, AuthorId a)
+			throws DbException {
+		return db.getLocalAuthor(txn, a);
+	}
+
 	@Override
 	public LocalAuthor getLocalAuthor() throws DbException {
 		return getLocalAuthors().iterator().next();

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));