Code cleanup, added FIXMEs for bigger issues.

This commit is contained in:
akwizgran
2016-07-30 16:37:10 +01:00
parent d8272d875b
commit 1bfa1016b4
5 changed files with 33 additions and 21 deletions

View File

@@ -21,7 +21,6 @@ import org.briarproject.api.sync.GroupId;
import org.briarproject.api.sync.MessageId; import org.briarproject.api.sync.MessageId;
import java.util.Collection; import java.util.Collection;
import java.util.logging.Logger;
import javax.inject.Inject; import javax.inject.Inject;
@@ -38,8 +37,6 @@ public class BlogActivity extends BriarActivity implements BlogPostListener,
static final String IS_NEW_BLOG = "briar.IS_NEW_BLOG"; static final String IS_NEW_BLOG = "briar.IS_NEW_BLOG";
private static final String BLOG_PAGER_ADAPTER = "briar.BLOG_PAGER_ADAPTER"; private static final String BLOG_PAGER_ADAPTER = "briar.BLOG_PAGER_ADAPTER";
private static final Logger LOG =
Logger.getLogger(BlogActivity.class.getName());
private ProgressBar progressBar; private ProgressBar progressBar;
private ViewPager pager; private ViewPager pager;

View File

@@ -7,14 +7,14 @@ import org.briarproject.android.controller.handler.ResultHandler;
import org.briarproject.api.sync.GroupId; import org.briarproject.api.sync.GroupId;
import org.briarproject.api.sync.MessageId; import org.briarproject.api.sync.MessageId;
import java.util.TreeSet; import java.util.SortedSet;
public interface BlogController extends ActivityLifecycleController { public interface BlogController extends ActivityLifecycleController {
void loadBlog(final GroupId groupId, final boolean reload, void loadBlog(GroupId groupId, boolean reload,
final ResultHandler<Boolean> resultHandler); ResultHandler<Boolean> resultHandler);
TreeSet<BlogPostItem> getBlogPosts(); SortedSet<BlogPostItem> getBlogPosts();
@Nullable @Nullable
BlogPostItem getBlogPost(MessageId postId); BlogPostItem getBlogPost(MessageId postId);
@@ -22,10 +22,10 @@ public interface BlogController extends ActivityLifecycleController {
@Nullable @Nullable
MessageId getBlogPostId(int position); MessageId getBlogPostId(int position);
void deleteBlog(final ResultHandler<Boolean> resultHandler); void deleteBlog(ResultHandler<Boolean> resultHandler);
interface BlogPostListener { interface BlogPostListener {
void onBlogPostAdded(final BlogPostItem post, final boolean local); void onBlogPostAdded(BlogPostItem post, boolean local);
} }
} }

View File

@@ -19,6 +19,7 @@ import org.briarproject.api.sync.MessageId;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.SortedSet;
import java.util.TreeSet; import java.util.TreeSet;
import java.util.logging.Logger; import java.util.logging.Logger;
@@ -42,6 +43,7 @@ public class BlogControllerImpl extends DbControllerImpl
private volatile BlogPostListener listener; private volatile BlogPostListener listener;
private volatile GroupId groupId = null; private volatile GroupId groupId = null;
// FIXME: This collection isn't thread-safe, isn't updated atomically
private volatile TreeSet<BlogPostItem> posts = null; private volatile TreeSet<BlogPostItem> posts = null;
@Inject @Inject
@@ -81,9 +83,11 @@ public class BlogControllerImpl extends DbControllerImpl
LOG.info("New blog post added"); LOG.info("New blog post added");
if (posts == null) { if (posts == null) {
LOG.info("Posts have not loaded, yet"); LOG.info("Posts have not loaded, yet");
// FIXME: Race condition, new post may not get loaded
return; return;
} }
final BlogPostHeader header = m.getHeader(); final BlogPostHeader header = m.getHeader();
// FIXME: Don't make blocking calls in event handlers
try { try {
final byte[] body = blogManager.getPostBody(header.getId()); final byte[] body = blogManager.getPostBody(header.getId());
final BlogPostItem post = new BlogPostItem(header, body); final BlogPostItem post = new BlogPostItem(header, body);
@@ -112,7 +116,6 @@ public class BlogControllerImpl extends DbControllerImpl
public void loadBlog(final GroupId g, final boolean reload, public void loadBlog(final GroupId g, final boolean reload,
final ResultHandler<Boolean> resultHandler) { final ResultHandler<Boolean> resultHandler) {
LOG.info("Loading blog...");
runOnDbThread(new Runnable() { runOnDbThread(new Runnable() {
@Override @Override
public void run() { public void run() {
@@ -132,8 +135,7 @@ public class BlogControllerImpl extends DbControllerImpl
posts.addAll(newPosts); posts.addAll(newPosts);
long duration = System.currentTimeMillis() - now; long duration = System.currentTimeMillis() - now;
if (LOG.isLoggable(INFO)) if (LOG.isLoggable(INFO))
LOG.info("Post header load took " + duration + LOG.info("Loading blog took " + duration + " ms");
" ms");
} }
resultHandler.onResult(true); resultHandler.onResult(true);
} catch (DbException e) { } catch (DbException e) {
@@ -147,7 +149,7 @@ public class BlogControllerImpl extends DbControllerImpl
@Override @Override
@Nullable @Nullable
public TreeSet<BlogPostItem> getBlogPosts() { public SortedSet<BlogPostItem> getBlogPosts() {
return posts; return posts;
} }

View File

@@ -4,7 +4,6 @@ import android.support.annotation.Nullable;
import org.briarproject.android.controller.ActivityLifecycleController; import org.briarproject.android.controller.ActivityLifecycleController;
import org.briarproject.android.controller.handler.ResultHandler; import org.briarproject.android.controller.handler.ResultHandler;
import org.briarproject.android.controller.handler.UiResultHandler;
import org.briarproject.api.forum.Forum; import org.briarproject.api.forum.Forum;
import org.briarproject.api.sync.GroupId; import org.briarproject.api.sync.GroupId;
import org.briarproject.api.sync.MessageId; import org.briarproject.api.sync.MessageId;
@@ -15,17 +14,25 @@ import java.util.List;
public interface ForumController extends ActivityLifecycleController { public interface ForumController extends ActivityLifecycleController {
void loadForum(GroupId groupId, ResultHandler<Boolean> resultHandler); void loadForum(GroupId groupId, ResultHandler<Boolean> resultHandler);
@Nullable @Nullable
Forum getForum(); Forum getForum();
List<ForumEntry> getForumEntries(); List<ForumEntry> getForumEntries();
void unsubscribe(UiResultHandler<Boolean> resultHandler);
void unsubscribe(ResultHandler<Boolean> resultHandler);
void entryRead(ForumEntry forumEntry); void entryRead(ForumEntry forumEntry);
void entriesRead(Collection<ForumEntry> messageIds); void entriesRead(Collection<ForumEntry> messageIds);
void createPost(byte[] body); void createPost(byte[] body);
void createPost(byte[] body, MessageId parentId); void createPost(byte[] body, MessageId parentId);
interface ForumPostListener { interface ForumPostListener {
void addLocalEntry(int index, ForumEntry entry); void addLocalEntry(int index, ForumEntry entry);
void addForeignEntry(int index, ForumEntry entry); void addForeignEntry(int index, ForumEntry entry);
} }

View File

@@ -5,7 +5,6 @@ import android.support.annotation.Nullable;
import org.briarproject.android.controller.DbControllerImpl; import org.briarproject.android.controller.DbControllerImpl;
import org.briarproject.android.controller.handler.ResultHandler; import org.briarproject.android.controller.handler.ResultHandler;
import org.briarproject.android.controller.handler.UiResultHandler;
import org.briarproject.api.FormatException; import org.briarproject.api.FormatException;
import org.briarproject.api.clients.MessageTree; import org.briarproject.api.clients.MessageTree;
import org.briarproject.api.crypto.CryptoComponent; import org.briarproject.api.crypto.CryptoComponent;
@@ -34,10 +33,10 @@ import java.security.GeneralSecurityException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Collections; import java.util.Collections;
import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Stack; import java.util.Stack;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Executor; import java.util.concurrent.Executor;
import java.util.logging.Logger; import java.util.logging.Logger;
@@ -69,11 +68,12 @@ public class ForumControllerImpl extends DbControllerImpl
@Inject @Inject
protected volatile IdentityManager identityManager; protected volatile IdentityManager identityManager;
private volatile MessageTree<ForumPostHeader> tree = private final Map<MessageId, byte[]> bodyCache = new ConcurrentHashMap<>();
new MessageTreeImpl<>(); private final MessageTree<ForumPostHeader> tree = new MessageTreeImpl<>();
private volatile Map<MessageId, byte[]> bodyCache = new HashMap<>();
private volatile LocalAuthor localAuthor = null; private volatile LocalAuthor localAuthor = null;
private volatile Forum forum = null; private volatile Forum forum = null;
// FIXME: This collection isn't thread-safe, isn't updated atomically
private volatile List<ForumEntry> forumEntries = null; private volatile List<ForumEntry> forumEntries = null;
private ForumPostListener listener; private ForumPostListener listener;
@@ -116,6 +116,7 @@ public class ForumControllerImpl extends DbControllerImpl
ForumPostReceivedEvent pe = (ForumPostReceivedEvent) e; ForumPostReceivedEvent pe = (ForumPostReceivedEvent) e;
if (pe.getGroupId().equals(forum.getId())) { if (pe.getGroupId().equals(forum.getId())) {
LOG.info("Forum Post received, adding..."); LOG.info("Forum Post received, adding...");
// FIXME: Don't make blocking calls in event handlers
addNewPost(pe.getForumPostHeader()); addNewPost(pe.getForumPostHeader());
} }
} else if (e instanceof GroupRemovedEvent) { } else if (e instanceof GroupRemovedEvent) {
@@ -272,9 +273,10 @@ public class ForumControllerImpl extends DbControllerImpl
} }
@Override @Override
public void unsubscribe(final UiResultHandler<Boolean> resultHandler) { public void unsubscribe(final ResultHandler<Boolean> resultHandler) {
if (forum == null) return; if (forum == null) return;
runOnDbThread(new Runnable() { runOnDbThread(new Runnable() {
@Override
public void run() { public void run() {
try { try {
long now = System.currentTimeMillis(); long now = System.currentTimeMillis();
@@ -300,6 +302,7 @@ public class ForumControllerImpl extends DbControllerImpl
@Override @Override
public void entriesRead(final Collection<ForumEntry> forumEntries) { public void entriesRead(final Collection<ForumEntry> forumEntries) {
runOnDbThread(new Runnable() { runOnDbThread(new Runnable() {
@Override
public void run() { public void run() {
try { try {
long now = System.currentTimeMillis(); long now = System.currentTimeMillis();
@@ -325,6 +328,7 @@ public class ForumControllerImpl extends DbControllerImpl
@Override @Override
public void createPost(final byte[] body, final MessageId parentId) { public void createPost(final byte[] body, final MessageId parentId) {
cryptoExecutor.execute(new Runnable() { cryptoExecutor.execute(new Runnable() {
@Override
public void run() { public void run() {
long timestamp = System.currentTimeMillis(); long timestamp = System.currentTimeMillis();
long newestTimeStamp = 0; long newestTimeStamp = 0;
@@ -353,6 +357,7 @@ public class ForumControllerImpl extends DbControllerImpl
} }
bodyCache.put(p.getMessage().getId(), body); bodyCache.put(p.getMessage().getId(), body);
storePost(p); storePost(p);
// FIXME: Don't make DB calls on the crypto executor
addNewPost(p); addNewPost(p);
} }
}); });
@@ -378,6 +383,7 @@ public class ForumControllerImpl extends DbControllerImpl
private void storePost(final ForumPost p) { private void storePost(final ForumPost p) {
runOnDbThread(new Runnable() { runOnDbThread(new Runnable() {
@Override
public void run() { public void run() {
try { try {
long now = System.currentTimeMillis(); long now = System.currentTimeMillis();