diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseViewModel.java index e61ae3c38..95e859ff6 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseViewModel.java @@ -127,41 +127,34 @@ abstract class BaseViewModel extends DbViewModel implements EventListener { LiveData> loadBlogPost(GroupId g, MessageId m) { MutableLiveData> result = new MutableLiveData<>(); - runOnDbThread(() -> { - try { - long start = now(); - BlogPostHeader header = blogManager.getPostHeader(g, m); - BlogPostItem item = db.transactionWithResult(true, txn -> - getItem(txn, header) - ); - logDuration(LOG, "Loading post", start); - result.postValue(new LiveResult<>(item)); - } catch (DbException e) { - logException(LOG, WARNING, e); - result.postValue(new LiveResult<>(e)); - } + runOnDbThread(true, txn -> { + long start = now(); + BlogPostHeader header = blogManager.getPostHeader(txn, g, m); + BlogPostItem item = getItem(txn, header); + logDuration(LOG, "Loading post", start); + result.postValue(new LiveResult<>(item)); + }, e -> { + logException(LOG, WARNING, e); + result.postValue(new LiveResult<>(e)); }); return result; } protected void onBlogPostAdded(BlogPostHeader header, boolean local) { - runOnDbThread(() -> { - try { - db.transaction(true, txn -> { - BlogPostItem item = getItem(txn, header); - txn.attach(() -> { - List items = addListItem(blogPosts, item); - if (items != null) { - Collections.sort(items); - postAddedWasLocal = local; - blogPosts.setValue(new LiveResult<>(items)); - } - }); - }); - } catch (DbException e) { - logException(LOG, WARNING, e); - } - }); + runOnDbThread(true, txn -> { + BlogPostItem item = getItem(txn, header); + txn.attach(() -> onBlogPostItemAdded(item, local)); + }, e -> logException(LOG, WARNING, e)); + } + + @UiThread + private void onBlogPostItemAdded(BlogPostItem item, boolean local) { + List items = addListItem(blogPosts, item); + if (items != null) { + Collections.sort(items); + postAddedWasLocal = local; + blogPosts.setValue(new LiveResult<>(items)); + } } void repeatPost(BlogPostItem item, @Nullable String comment) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogFragment.java index 63ce4954d..079410e1c 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogFragment.java @@ -106,8 +106,7 @@ public class BlogFragment extends BaseFragment @Override public void onStart() { super.onStart(); - viewModel.blockNotifications(); - viewModel.clearBlogPostNotifications(); + viewModel.blockAndClearNotifications(); list.startPeriodicUpdate(); } @@ -134,23 +133,22 @@ public class BlogFragment extends BaseFragment public boolean onOptionsItemSelected(MenuItem item) { int itemId = item.getItemId(); if (itemId == R.id.action_write_blog_post) { - Intent i = new Intent(getActivity(), - WriteBlogPostActivity.class); + Intent i = new Intent(getActivity(), WriteBlogPostActivity.class); i.putExtra(GROUP_ID, groupId.getBytes()); startActivityForResult(i, REQUEST_WRITE_BLOG_POST); return true; } else if (itemId == R.id.action_blog_share) { - Intent i2 = new Intent(getActivity(), ShareBlogActivity.class); - i2.setFlags(FLAG_ACTIVITY_CLEAR_TOP); - i2.putExtra(GROUP_ID, groupId.getBytes()); - startActivityForResult(i2, REQUEST_SHARE_BLOG); + Intent i = new Intent(getActivity(), ShareBlogActivity.class); + i.setFlags(FLAG_ACTIVITY_CLEAR_TOP); + i.putExtra(GROUP_ID, groupId.getBytes()); + startActivityForResult(i, REQUEST_SHARE_BLOG); return true; } else if (itemId == R.id.action_blog_sharing_status) { - Intent i3 = new Intent(getActivity(), - BlogSharingStatusActivity.class); - i3.setFlags(FLAG_ACTIVITY_CLEAR_TOP); - i3.putExtra(GROUP_ID, groupId.getBytes()); - startActivity(i3); + Intent i = + new Intent(getActivity(), BlogSharingStatusActivity.class); + i.setFlags(FLAG_ACTIVITY_CLEAR_TOP); + i.putExtra(GROUP_ID, groupId.getBytes()); + startActivity(i); return true; } else if (itemId == R.id.action_blog_delete) { showDeleteDialog(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostFragment.java index 75001f45b..77043d276 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostFragment.java @@ -23,7 +23,6 @@ import java.util.logging.Logger; import javax.annotation.Nullable; import javax.inject.Inject; -import androidx.annotation.CallSuper; import androidx.annotation.UiThread; import androidx.lifecycle.LifecycleOwner; import androidx.lifecycle.ViewModelProvider; @@ -36,7 +35,6 @@ import static java.util.logging.Logger.getLogger; import static org.briarproject.briar.android.activity.BriarActivity.GROUP_ID; import static org.briarproject.briar.android.util.UiUtils.MIN_DATE_RESOLUTION; -@UiThread @MethodsNotNullByDefault @ParametersNotNullByDefault public class BlogPostFragment extends BaseFragment @@ -82,7 +80,8 @@ public class BlogPostFragment extends BaseFragment Bundle args = requireArguments(); GroupId groupId = new GroupId(requireNonNull(args.getByteArray(GROUP_ID))); - MessageId postId = new MessageId(args.getByteArray(POST_ID)); + MessageId postId = + new MessageId(requireNonNull(args.getByteArray(POST_ID))); View view = inflater.inflate(R.layout.fragment_blog_post, container, false); @@ -97,14 +96,12 @@ public class BlogPostFragment extends BaseFragment return view; } - @CallSuper @Override public void onStart() { super.onStart(); startPeriodicUpdate(); } - @CallSuper @Override public void onStop() { super.onStop(); @@ -128,7 +125,7 @@ public class BlogPostFragment extends BaseFragment Intent i = new Intent(requireContext(), BlogActivity.class); i.putExtra(GROUP_ID, post.getGroupId().getBytes()); i.setFlags(FLAG_ACTIVITY_CLEAR_TOP); - getContext().startActivity(i); + requireContext().startActivity(i); } @Override 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 1e948af57..a2d883f9f 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 @@ -9,6 +9,7 @@ import android.view.ViewGroup; import android.widget.ImageButton; import android.widget.TextView; +import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.briar.R; import org.briarproject.briar.android.view.AuthorView; @@ -17,7 +18,6 @@ import org.briarproject.briar.api.blog.BlogPostHeader; import javax.annotation.Nullable; -import androidx.annotation.NonNull; import androidx.annotation.UiThread; import androidx.core.view.ViewCompat; import androidx.recyclerview.widget.RecyclerView; @@ -36,6 +36,7 @@ import static org.briarproject.briar.android.view.AuthorView.RSS_FEED_REBLOGGED; import static org.briarproject.briar.api.blog.MessageType.POST; @UiThread +@NotNullByDefault class BlogPostViewHolder extends RecyclerView.ViewHolder { private final Context ctx; @@ -47,11 +48,10 @@ class BlogPostViewHolder extends RecyclerView.ViewHolder { private final ViewGroup commentContainer; private final boolean fullText; - @NonNull private final OnBlogPostClickListener listener; BlogPostViewHolder(View v, boolean fullText, - @NonNull OnBlogPostClickListener listener) { + OnBlogPostClickListener listener) { super(v); this.fullText = fullText; this.listener = listener; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogViewModel.java index 173a627cc..5ac2c4143 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogViewModel.java @@ -15,12 +15,10 @@ import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; import org.briarproject.bramble.api.sync.GroupId; -import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.api.sync.event.GroupRemovedEvent; import org.briarproject.bramble.api.system.AndroidExecutor; import org.briarproject.briar.android.sharing.SharingController; import org.briarproject.briar.android.sharing.SharingController.SharingInfo; -import org.briarproject.briar.android.viewmodel.LiveResult; import org.briarproject.briar.api.android.AndroidNotificationManager; import org.briarproject.briar.api.blog.Blog; import org.briarproject.briar.api.blog.BlogInvitationResponse; @@ -37,6 +35,7 @@ import java.util.logging.Logger; import javax.inject.Inject; +import androidx.annotation.UiThread; import androidx.lifecycle.LiveData; import androidx.lifecycle.MutableLiveData; @@ -136,11 +135,8 @@ class BlogViewModel extends BaseViewModel { }); } - void blockNotifications() { + void blockAndClearNotifications() { notificationManager.blockNotification(groupId); - } - - void clearBlogPostNotifications() { notificationManager.clearBlogPostNotification(groupId); } @@ -153,18 +149,18 @@ class BlogViewModel extends BaseViewModel { } private void loadSharingContacts(GroupId groupId) { - runOnDbThread(() -> { - try { - Collection contacts = - blogSharingManager.getSharedWith(groupId); - Collection contactIds = - new ArrayList<>(contacts.size()); - for (Contact c : contacts) contactIds.add(c.getId()); - sharingController.addAll(contactIds); - } catch (DbException e) { - logException(LOG, WARNING, e); - } - }); + runOnDbThread(true, txn -> { + Collection contacts = + blogSharingManager.getSharedWith(txn, groupId); + txn.attach(() -> onSharingContactsLoaded(contacts)); + }, e -> logException(LOG, WARNING, e)); + } + + @UiThread + private void onSharingContactsLoaded(Collection contacts) { + Collection contactIds = new ArrayList<>(contacts.size()); + for (Contact c : contacts) contactIds.add(c.getId()); + sharingController.addAll(contactIds); } void deleteBlog() { @@ -180,10 +176,6 @@ class BlogViewModel extends BaseViewModel { }); } - LiveData> loadBlogPost(MessageId m) { - return loadBlogPost(groupId, m); - } - LiveData getBlog() { return blog; } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedFragment.java index 8ef8d1b96..6e4718576 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedFragment.java @@ -24,7 +24,6 @@ import java.util.List; import javax.inject.Inject; import androidx.annotation.Nullable; -import androidx.annotation.UiThread; import androidx.lifecycle.ViewModelProvider; import androidx.recyclerview.widget.LinearLayoutManager; @@ -32,7 +31,6 @@ import static android.content.Intent.FLAG_ACTIVITY_CLEAR_TOP; import static com.google.android.material.snackbar.Snackbar.LENGTH_LONG; import static org.briarproject.briar.android.activity.BriarActivity.GROUP_ID; -@UiThread @MethodsNotNullByDefault @ParametersNotNullByDefault public class FeedFragment extends BaseFragment @@ -82,8 +80,7 @@ public class FeedFragment extends BaseFragment list.setEmptyAction(R.string.blogs_feed_empty_state_action); viewModel.getBlogPosts().observe(getViewLifecycleOwner(), result -> - result - .onError(this::handleException) + result.onError(this::handleException) .onSuccess(this::onBlogPostsLoaded) ); @@ -93,8 +90,7 @@ public class FeedFragment extends BaseFragment @Override public void onStart() { super.onStart(); - viewModel.blockAllBlogPostNotifications(); - viewModel.clearAllBlogPostNotifications(); + viewModel.blockAndClearAllBlogPostNotifications(); list.startPeriodicUpdate(); } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedViewModel.java index 320afa7cd..d5374a7bc 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedViewModel.java @@ -4,7 +4,6 @@ import android.app.Application; import org.briarproject.bramble.api.db.DatabaseExecutor; import org.briarproject.bramble.api.db.DbException; -import org.briarproject.bramble.api.db.NoSuchMessageException; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.db.TransactionManager; import org.briarproject.bramble.api.event.Event; @@ -31,6 +30,7 @@ import java.util.logging.Logger; import javax.inject.Inject; +import androidx.annotation.UiThread; import androidx.lifecycle.LiveData; import androidx.lifecycle.MutableLiveData; @@ -87,11 +87,8 @@ class FeedViewModel extends BaseViewModel { } } - void blockAllBlogPostNotifications() { + void blockAndClearAllBlogPostNotifications() { notificationManager.blockAllBlogPostNotifications(); - } - - void clearAllBlogPostNotifications() { notificationManager.clearAllBlogPostNotifications(); } @@ -127,11 +124,7 @@ class FeedViewModel extends BaseViewModel { long start = now(); List posts = new ArrayList<>(); for (GroupId g : blogManager.getBlogIds(txn)) { - try { - posts.addAll(loadBlogPosts(txn, g)); - } catch (NoSuchMessageException e) { - logException(LOG, WARNING, e); - } + posts.addAll(loadBlogPosts(txn, g)); } Collections.sort(posts); logDuration(LOG, "Loading all posts", start); @@ -139,23 +132,19 @@ class FeedViewModel extends BaseViewModel { } private void onBlogAdded(GroupId g) { - runOnDbThread(() -> { - try { - db.transaction(true, txn -> { - List posts = loadBlogPosts(txn, g); - txn.attach(() -> { - List items = - addListItems(blogPosts, posts); - if (items != null) { - Collections.sort(items); - blogPosts.setValue(new LiveResult<>(items)); - } - }); - }); - } catch (DbException e) { - logException(LOG, WARNING, e); - } - }); + runOnDbThread(true, txn -> { + List posts = loadBlogPosts(txn, g); + txn.attach(() -> onBlogPostItemsAdded(posts)); + }, e -> logException(LOG, WARNING, e)); + } + + @UiThread + private void onBlogPostItemsAdded(List posts) { + List items = addListItems(blogPosts, posts); + if (items != null) { + Collections.sort(items); + blogPosts.setValue(new LiveResult<>(items)); + } } private void onBlogRemoved(GroupId g) { diff --git a/briar-api/src/main/java/org/briarproject/briar/api/blog/BlogManager.java b/briar-api/src/main/java/org/briarproject/briar/api/blog/BlogManager.java index b2d76357e..9953cbac4 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/blog/BlogManager.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/blog/BlogManager.java @@ -107,7 +107,8 @@ public interface BlogManager { /** * Returns the header of the blog post with the given ID. */ - BlogPostHeader getPostHeader(GroupId g, MessageId m) throws DbException; + BlogPostHeader getPostHeader(Transaction txn, GroupId g, MessageId m) + throws DbException; /** * Returns the text of the blog post with the given ID. 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 157668e88..68ea45863 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 @@ -454,19 +454,14 @@ class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager, } @Override - public BlogPostHeader getPostHeader(GroupId g, MessageId m) + public BlogPostHeader getPostHeader(Transaction txn, GroupId g, MessageId m) throws DbException { - Transaction txn = db.startTransaction(true); try { BdfDictionary meta = clientHelper.getMessageMetadataAsDictionary(txn, m); - BlogPostHeader h = getPostHeaderFromMetadata(txn, g, m, meta); - db.commitTransaction(txn); - return h; + return getPostHeaderFromMetadata(txn, g, m, meta); } catch (FormatException e) { throw new DbException(e); - } finally { - db.endTransaction(txn); } } diff --git a/briar-headless/src/main/java/org/briarproject/briar/headless/blogs/BlogControllerImpl.kt b/briar-headless/src/main/java/org/briarproject/briar/headless/blogs/BlogControllerImpl.kt index ba4068050..2df0d6dcc 100644 --- a/briar-headless/src/main/java/org/briarproject/briar/headless/blogs/BlogControllerImpl.kt +++ b/briar-headless/src/main/java/org/briarproject/briar/headless/blogs/BlogControllerImpl.kt @@ -3,12 +3,15 @@ package org.briarproject.briar.headless.blogs import com.fasterxml.jackson.databind.ObjectMapper import io.javalin.http.BadRequestResponse import io.javalin.http.Context +import org.briarproject.bramble.api.db.DbException +import org.briarproject.bramble.api.db.TransactionManager import org.briarproject.bramble.api.identity.IdentityManager import org.briarproject.bramble.api.system.Clock import org.briarproject.bramble.util.StringUtils.utf8IsTooLong import org.briarproject.briar.api.blog.BlogConstants.MAX_BLOG_POST_TEXT_LENGTH import org.briarproject.briar.api.blog.BlogManager import org.briarproject.briar.api.blog.BlogPostFactory +import org.briarproject.briar.api.blog.BlogPostHeader import org.briarproject.briar.headless.getFromJson import javax.annotation.concurrent.Immutable import javax.inject.Inject @@ -21,6 +24,7 @@ internal class BlogControllerImpl constructor( private val blogManager: BlogManager, private val blogPostFactory: BlogPostFactory, + private val db: TransactionManager, private val identityManager: IdentityManager, private val objectMapper: ObjectMapper, private val clock: Clock @@ -45,8 +49,10 @@ constructor( val blog = blogManager.getPersonalBlog(author) val now = clock.currentTimeMillis() val post = blogPostFactory.createBlogPost(blog.id, now, null, author, text) - blogManager.addLocalPost(post) - val header = blogManager.getPostHeader(blog.id, post.message.id) + val header = db.transactionWithResult(true) { txn -> + blogManager.addLocalPost(txn, post) + return@transactionWithResult blogManager.getPostHeader(txn, blog.id, post.message.id) + } return ctx.json(header.output(text)) } diff --git a/briar-headless/src/test/java/org/briarproject/briar/headless/ControllerTest.kt b/briar-headless/src/test/java/org/briarproject/briar/headless/ControllerTest.kt index ed4aa8384..c7d2a6aaf 100644 --- a/briar-headless/src/test/java/org/briarproject/briar/headless/ControllerTest.kt +++ b/briar-headless/src/test/java/org/briarproject/briar/headless/ControllerTest.kt @@ -7,6 +7,7 @@ import io.mockk.mockk import org.briarproject.bramble.api.connection.ConnectionRegistry import org.briarproject.bramble.api.contact.Contact import org.briarproject.bramble.api.contact.ContactManager +import org.briarproject.bramble.api.db.TransactionManager import org.briarproject.bramble.api.identity.Author import org.briarproject.bramble.api.identity.IdentityManager import org.briarproject.bramble.api.identity.LocalAuthor @@ -24,6 +25,7 @@ import javax.servlet.http.HttpServletResponse abstract class ControllerTest { + protected val db = mockk() protected val contactManager = mockk() protected val conversationManager = mockk() protected val identityManager = mockk() diff --git a/briar-headless/src/test/java/org/briarproject/briar/headless/blogs/BlogControllerTest.kt b/briar-headless/src/test/java/org/briarproject/briar/headless/blogs/BlogControllerTest.kt index aae2aa100..94c6b0dae 100644 --- a/briar-headless/src/test/java/org/briarproject/briar/headless/blogs/BlogControllerTest.kt +++ b/briar-headless/src/test/java/org/briarproject/briar/headless/blogs/BlogControllerTest.kt @@ -6,14 +6,22 @@ import io.mockk.Runs import io.mockk.every import io.mockk.just import io.mockk.mockk -import org.briarproject.briar.api.identity.AuthorInfo -import org.briarproject.briar.api.identity.AuthorInfo.Status.OURSELVES +import io.mockk.slot +import org.briarproject.bramble.api.db.DbCallable +import org.briarproject.bramble.api.db.DbException +import org.briarproject.bramble.api.db.Transaction import org.briarproject.bramble.api.sync.MessageId import org.briarproject.bramble.identity.output import org.briarproject.bramble.util.StringUtils.getRandomString -import org.briarproject.briar.api.blog.* +import org.briarproject.briar.api.blog.Blog import org.briarproject.briar.api.blog.BlogConstants.MAX_BLOG_POST_TEXT_LENGTH +import org.briarproject.briar.api.blog.BlogManager +import org.briarproject.briar.api.blog.BlogPost +import org.briarproject.briar.api.blog.BlogPostFactory +import org.briarproject.briar.api.blog.BlogPostHeader import org.briarproject.briar.api.blog.MessageType.POST +import org.briarproject.briar.api.identity.AuthorInfo +import org.briarproject.briar.api.identity.AuthorInfo.Status.OURSELVES import org.briarproject.briar.headless.ControllerTest import org.junit.jupiter.api.Assertions.assertThrows import org.junit.jupiter.api.Test @@ -24,7 +32,7 @@ internal class BlogControllerTest : ControllerTest() { private val blogPostFactory = mockk() private val controller = - BlogControllerImpl(blogManager, blogPostFactory, identityManager, objectMapper, clock) + BlogControllerImpl(blogManager, blogPostFactory, db, identityManager, objectMapper, clock) private val blog = Blog(group, author, false) private val parentId: MessageId? = null @@ -46,6 +54,8 @@ internal class BlogControllerTest : ControllerTest() { @Test fun testCreate() { val post = BlogPost(message, null, localAuthor) + val dbSlot = slot>() + val txn = Transaction(Object(), true) every { ctx.body() } returns """{"text": "$text"}""" every { identityManager.localAuthor } returns localAuthor @@ -60,8 +70,13 @@ internal class BlogControllerTest : ControllerTest() { text ) } returns post - every { blogManager.addLocalPost(post) } just Runs - every { blogManager.getPostHeader(post.message.groupId, post.message.id) } returns header + every { db.transactionWithResult(true, capture(dbSlot)) } answers { + dbSlot.captured.call(txn) + } + every { blogManager.addLocalPost(txn, post) } just Runs + every { + blogManager.getPostHeader(txn, post.message.groupId, post.message.id) + } returns header every { ctx.json(header.output(text)) } returns ctx controller.createPost(ctx)