From 4b955809f7c8e1d9258909cc687231695233efd0 Mon Sep 17 00:00:00 2001 From: Torsten Grote <t@grobox.de> Date: Wed, 12 Apr 2017 09:30:08 -0300 Subject: [PATCH] Address review comments --- .../bramble/api/identity/Author.java | 4 +++- .../android/blog/BlogPostViewHolder.java | 2 +- .../briar/android/blog/RssFeedAdapter.java | 7 +------ .../briar/android/view/AuthorView.java | 14 ++++++++++--- briar-android/src/main/res/values/strings.xml | 2 +- .../briar/api/blog/BlogCommentHeader.java | 6 +++++- .../org/briarproject/briar/api/feed/Feed.java | 18 +++++++--------- .../briar/api/feed/FeedConstants.java | 1 - .../briar/blog/BlogFactoryImpl.java | 21 ++++++++++++++----- .../briar/blog/BlogManagerImpl.java | 2 +- .../briar/feed/FeedFactoryImpl.java | 17 ++++++++------- .../briar/feed/FeedManagerImpl.java | 5 ++--- .../blog/BlogManagerIntegrationTest.java | 4 +++- 13 files changed, 60 insertions(+), 43 deletions(-) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/identity/Author.java b/bramble-api/src/main/java/org/briarproject/bramble/api/identity/Author.java index 7e89a7a7bf..b7c208ff29 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/identity/Author.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/identity/Author.java @@ -13,7 +13,9 @@ import javax.annotation.concurrent.Immutable; @NotNullByDefault public class Author { - public enum Status {ANONYMOUS, UNKNOWN, UNVERIFIED, VERIFIED, OURSELVES} + public enum Status { + NONE, ANONYMOUS, UNKNOWN, UNVERIFIED, VERIFIED, OURSELVES + } private final AuthorId id; private final String name; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostViewHolder.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostViewHolder.java index b0f4187b40..76232fc575 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostViewHolder.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostViewHolder.java @@ -169,7 +169,7 @@ class BlogPostViewHolder extends RecyclerView.ViewHolder { reblogger.setVisibility(VISIBLE); reblogger.setPersona(AuthorView.REBLOGGER); - author.setPersona(item.getHeader().getParent().isRssFeed() ? + author.setPersona(item.getHeader().getRootPost().isRssFeed() ? AuthorView.RSS_FEED_REBLOGGED : AuthorView.COMMENTER); 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 e55dcef305..e4ed107b6a 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 @@ -39,12 +39,7 @@ class RssFeedAdapter extends BriarAdapter<Feed, RssFeedAdapter.FeedViewHolder> { if (item == null) return; // Feed Title - if (item.getTitle() != null) { - ui.title.setText(item.getTitle()); - ui.title.setVisibility(VISIBLE); - } else { - ui.title.setVisibility(GONE); - } + ui.title.setText(item.getTitle()); // Delete Button ui.delete.setOnClickListener(new OnClickListener() { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/view/AuthorView.java b/briar-android/src/main/java/org/briarproject/briar/android/view/AuthorView.java index 67aa5a3b92..f03a88ecd1 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/view/AuthorView.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/view/AuthorView.java @@ -30,6 +30,7 @@ import static android.content.Context.LAYOUT_INFLATER_SERVICE; import static android.content.Intent.FLAG_ACTIVITY_CLEAR_TOP; import static android.graphics.Typeface.BOLD; import static android.util.TypedValue.COMPLEX_UNIT_PX; +import static org.briarproject.bramble.api.identity.Author.Status.NONE; import static org.briarproject.bramble.api.identity.Author.Status.OURSELVES; import static org.briarproject.briar.android.activity.BriarActivity.GROUP_ID; @@ -85,7 +86,13 @@ public class AuthorView extends RelativeLayout { } public void setAuthorStatus(Status status) { - trustIndicator.setTrustLevel(status); + if (status != NONE) { + trustIndicator.setTrustLevel(status); + trustIndicator.setVisibility(VISIBLE); + } else { + trustIndicator.setVisibility(GONE); + } + if (status == OURSELVES) { authorName.setTypeface(authorNameTypeface, BOLD); } else { @@ -129,8 +136,9 @@ public class AuthorView extends RelativeLayout { /** * Styles this view for a different persona. * - * Attention: If used in a RecyclerView with RSS_FEED, - * call this after setAuthor() + * Attention: RSS_FEED and RSS_FEED_REBLOGGED change the avatar + * and override the one set by + * {@link AuthorView#setAuthor(Author)}. */ public void setPersona(int persona) { switch (persona) { diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index 5103e596b7..3c8a448830 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -305,7 +305,7 @@ <string name="blogs_rss_feeds_manage_author">Author:</string> <string name="blogs_rss_feeds_manage_updated">Last Updated:</string> <string name="blogs_rss_remove_feed">Remove Feed</string> - <string name="blogs_rss_remove_feed_dialog_message">Are you sure that you want to remove this feed and all its posts?\nNote that this will not remove the feed\'s blog from other people\'s devices.</string> + <string name="blogs_rss_remove_feed_dialog_message">Are you sure you want to remove this feed and all its posts?\nAny posts you have shared will not be removed from other people\'s devices.</string> <string name="blogs_rss_remove_feed_ok">Remove Feed</string> <string name="blogs_rss_feeds_manage_delete_error">The feed could not be deleted!</string> <string name="blogs_rss_feeds_manage_empty_state">You haven\'t imported any RSS feeds.\n\nWhy don\'t you click the plus in the top right screen corner to add your first?</string> diff --git a/briar-api/src/main/java/org/briarproject/briar/api/blog/BlogCommentHeader.java b/briar-api/src/main/java/org/briarproject/briar/api/blog/BlogCommentHeader.java index af28ca1a2e..f7914c9d55 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/blog/BlogCommentHeader.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/blog/BlogCommentHeader.java @@ -41,8 +41,12 @@ public class BlogCommentHeader extends BlogPostHeader { } public BlogPostHeader getParent() { + return parent; + } + + public BlogPostHeader getRootPost() { if (parent instanceof BlogCommentHeader) - return ((BlogCommentHeader) parent).getParent(); + return ((BlogCommentHeader) parent).getRootPost(); return parent; } 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 4932ecab75..f2fcaa3872 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 @@ -16,18 +16,16 @@ public class Feed { private final Blog blog; private final LocalAuthor localAuthor; @Nullable - private final String title, description, author; + private final String description, author; private final long added, updated, lastEntryTime; public Feed(String url, Blog blog, LocalAuthor localAuthor, - @Nullable String title, @Nullable String description, - @Nullable String author, long added, long updated, - long lastEntryTime) { + @Nullable String description, @Nullable String author, long added, + long updated, long lastEntryTime) { this.url = url; this.blog = blog; this.localAuthor = localAuthor; - this.title = title; this.description = description; this.author = author; this.added = added; @@ -36,13 +34,12 @@ public class Feed { } public Feed(String url, Blog blog, LocalAuthor localAuthor, - @Nullable String title, @Nullable String description, - @Nullable String author, long added) { - this(url, blog, localAuthor, title, description, author, added, 0L, 0L); + @Nullable String description, @Nullable String author, long added) { + this(url, blog, localAuthor, description, author, added, 0L, 0L); } public Feed(String url, Blog blog, LocalAuthor localAuthor, long added) { - this(url, blog, localAuthor, null, null, null, added, 0L, 0L); + this(url, blog, localAuthor, null, null, added, 0L, 0L); } public String getUrl() { @@ -61,9 +58,8 @@ public class Feed { return localAuthor; } - @Nullable public String getTitle() { - return title; + return blog.getName(); } @Nullable 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 9dc944585b..0fddb3aa73 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 @@ -21,7 +21,6 @@ public interface FeedConstants { String KEY_BLOG_TITLE = "blogTitle"; String KEY_PUBLIC_KEY = "publicKey"; String KEY_PRIVATE_KEY = "privateKey"; - String KEY_FEED_TITLE = "feedTitle"; String KEY_FEED_DESC = "feedDesc"; String KEY_FEED_AUTHOR = "feedAuthor"; String KEY_FEED_ADDED = "feedAdded"; diff --git a/briar-core/src/main/java/org/briarproject/briar/blog/BlogFactoryImpl.java b/briar-core/src/main/java/org/briarproject/briar/blog/BlogFactoryImpl.java index deaae2cfe6..c7d3a2001a 100644 --- a/briar-core/src/main/java/org/briarproject/briar/blog/BlogFactoryImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/blog/BlogFactoryImpl.java @@ -14,6 +14,9 @@ import org.briarproject.briar.api.blog.BlogFactory; import javax.annotation.concurrent.Immutable; import javax.inject.Inject; +import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH; +import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_PUBLIC_KEY_LENGTH; + @Immutable @NotNullByDefault class BlogFactoryImpl implements BlogFactory { @@ -58,13 +61,21 @@ class BlogFactoryImpl implements BlogFactory { } @Override - public Blog parseBlog(Group g) throws FormatException { - byte[] descriptor = g.getDescriptor(); + public Blog parseBlog(Group group) throws FormatException { + byte[] descriptor = group.getDescriptor(); // Author Name, Public Key BdfList blog = clientHelper.toList(descriptor); - Author a = - authorFactory.createAuthor(blog.getString(0), blog.getRaw(1)); - return new Blog(g, a, blog.getBoolean(2)); + String name = blog.getString(0); + if (name.length() > MAX_AUTHOR_NAME_LENGTH) + throw new IllegalArgumentException(); + byte[] publicKey = blog.getRaw(1); + if (publicKey.length > MAX_PUBLIC_KEY_LENGTH) + throw new IllegalArgumentException(); + + Author author = + authorFactory.createAuthor(name, publicKey); + boolean rssFeed = blog.getBoolean(2); + return new Blog(group, author, rssFeed); } } diff --git a/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java b/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java index 91945f3108..39ccb52410 100644 --- a/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/blog/BlogManagerImpl.java @@ -606,7 +606,7 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager, boolean isFeedPost = meta.getBoolean(KEY_RSS_FEED, false); Status authorStatus; if (isFeedPost) { - authorStatus = Status.UNKNOWN; + authorStatus = Status.NONE; } else if (authorStatuses.containsKey(authorId)) { authorStatus = authorStatuses.get(authorId); } else { 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 14aca15598..ec5d9c31b5 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 @@ -10,18 +10,19 @@ import org.briarproject.bramble.api.data.BdfEntry; 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; import javax.inject.Inject; +import static org.briarproject.bramble.api.identity.AuthorConstants.MAX_AUTHOR_NAME_LENGTH; import static org.briarproject.briar.api.feed.FeedConstants.KEY_BLOG_TITLE; 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_TITLE; 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.briarproject.briar.api.feed.FeedConstants.KEY_PRIVATE_KEY; @@ -45,11 +46,13 @@ class FeedFactoryImpl implements FeedFactory { @Override public Feed createFeed(String url, SyndFeed syndFeed) { - if (syndFeed.getTitle() == null) syndFeed.setTitle("RSS feed"); + String title = syndFeed.getTitle(); + if (title == null) title = "RSS"; + title = StringUtils.truncateUtf8(title, MAX_AUTHOR_NAME_LENGTH); KeyPair keyPair = cryptoComponent.generateSignatureKeyPair(); LocalAuthor localAuthor = authorFactory - .createLocalAuthor(syndFeed.getTitle(), + .createLocalAuthor(title, keyPair.getPublic().getEncoded(), keyPair.getPrivate().getEncoded()); Blog blog = blogFactory.createFeedBlog(localAuthor); @@ -62,8 +65,8 @@ class FeedFactoryImpl implements FeedFactory { public Feed createFeed(Feed feed, SyndFeed f, long lastEntryTime) { long updated = clock.currentTimeMillis(); return new Feed(feed.getUrl(), feed.getBlog(), feed.getLocalAuthor(), - f.getTitle(), f.getDescription(), f.getAuthor(), - feed.getAdded(), updated, lastEntryTime); + f.getDescription(), f.getAuthor(), feed.getAdded(), updated, + lastEntryTime); } @Override @@ -77,14 +80,13 @@ class FeedFactoryImpl implements FeedFactory { .createLocalAuthor(blogTitle, publicKey, privateKey); Blog blog = blogFactory.createFeedBlog(localAuthor); - String title = d.getOptionalString(KEY_FEED_TITLE); String desc = d.getOptionalString(KEY_FEED_DESC); String author = d.getOptionalString(KEY_FEED_AUTHOR); 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, title, desc, author, added, + return new Feed(url, blog, localAuthor, desc, author, added, updated, lastEntryTime); } @@ -101,7 +103,6 @@ class FeedFactoryImpl implements FeedFactory { new BdfEntry(KEY_FEED_UPDATED, feed.getUpdated()), new BdfEntry(KEY_FEED_LAST_ENTRY, feed.getLastEntryTime()) ); - if (feed.getTitle() != null) d.put(KEY_FEED_TITLE, feed.getTitle()); if (feed.getDescription() != null) d.put(KEY_FEED_DESC, feed.getDescription()); if (feed.getAuthor() != null) d.put(KEY_FEED_AUTHOR, feed.getAuthor()); 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 75d8977ea8..bb97bb0d48 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 @@ -433,9 +433,8 @@ class FeedManagerImpl implements FeedManager, Client, EventListener, // build post body StringBuilder b = new StringBuilder(); - if (feed.getTitle() != null) { - b.append("<h3>").append(feed.getTitle()).append("</h3>"); - } + b.append("<h3>").append(feed.getTitle()).append("</h3>"); + if (!StringUtils.isNullOrEmpty(entry.getTitle())) { b.append("<h1>").append(entry.getTitle()).append("</h1>"); } diff --git a/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerIntegrationTest.java index c2c00ea571..165c1fab52 100644 --- a/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/blog/BlogManagerIntegrationTest.java @@ -1,6 +1,7 @@ package org.briarproject.briar.blog; import org.briarproject.bramble.api.db.Transaction; +import org.briarproject.bramble.api.identity.Author; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.test.TestDatabaseModule; import org.briarproject.briar.api.blog.Blog; @@ -417,6 +418,7 @@ public class BlogManagerIntegrationTest assertEquals(1, headers.size()); BlogPostHeader header = headers.iterator().next(); assertEquals(POST, header.getType()); + assertEquals(Author.Status.NONE, header.getAuthorStatus()); assertTrue(header.isRssFeed()); } @@ -454,7 +456,7 @@ public class BlogManagerIntegrationTest for (BlogPostHeader h: headers) { assertTrue(h instanceof BlogCommentHeader); assertEquals(COMMENT, h.getType()); - assertTrue(((BlogCommentHeader) h).getParent().isRssFeed()); + assertTrue(((BlogCommentHeader) h).getRootPost().isRssFeed()); } } -- GitLab