Anticipate review feedback for blog view models after re-basing

briar-android/src/main/java/org/briarproject/briar/android/blog/BaseViewModel.java

@@ -127,41 +127,34 @@ abstract class BaseViewModel extends DbViewModel implements EventListener {
 	LiveData<LiveResult<BlogPostItem>> loadBlogPost(GroupId g, MessageId m) {
 		MutableLiveData<LiveResult<BlogPostItem>> 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<BlogPostItem> 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<BlogPostItem> items = addListItem(blogPosts, item);
+		if (items != null) {
+			Collections.sort(items);
+			postAddedWasLocal = local;
+			blogPosts.setValue(new LiveResult<>(items));
+		}
 	}
 
 	void repeatPost(BlogPostItem item, @Nullable String comment) {

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

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

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;

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<Contact> contacts =
-						blogSharingManager.getSharedWith(groupId);
-				Collection<ContactId> 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<Contact> contacts =
+					blogSharingManager.getSharedWith(txn, groupId);
+			txn.attach(() -> onSharingContactsLoaded(contacts));
+		}, e -> logException(LOG, WARNING, e));
+	}
+
+	@UiThread
+	private void onSharingContactsLoaded(Collection<Contact> contacts) {
+		Collection<ContactId> 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<LiveResult<BlogPostItem>> loadBlogPost(MessageId m) {
-		return loadBlogPost(groupId, m);
-	}
-
 	LiveData<BlogItem> getBlog() {
 		return blog;
 	}

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

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<BlogPostItem> 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<BlogPostItem> posts = loadBlogPosts(txn, g);
-					txn.attach(() -> {
-						List<BlogPostItem> 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<BlogPostItem> posts = loadBlogPosts(txn, g);
+			txn.attach(() -> onBlogPostItemsAdded(posts));
+		}, e -> logException(LOG, WARNING, e));
+	}
+
+	@UiThread
+	private void onBlogPostItemsAdded(List<BlogPostItem> posts) {
+		List<BlogPostItem> items = addListItems(blogPosts, posts);
+		if (items != null) {
+			Collections.sort(items);
+			blogPosts.setValue(new LiveResult<>(items));
+		}
 	}
 
 	private void onBlogRemoved(GroupId g) {

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.

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

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<BlogPostHeader, DbException>(true) { txn ->
+            blogManager.addLocalPost(txn, post)
+            return@transactionWithResult blogManager.getPostHeader(txn, blog.id, post.message.id)
+        }
         return ctx.json(header.output(text))
     }
 

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<TransactionManager>()
     protected val contactManager = mockk<ContactManager>()
     protected val conversationManager = mockk<ConversationManager>()
     protected val identityManager = mockk<IdentityManager>()

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<BlogPostFactory>()
 
     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<DbCallable<BlogPostHeader, DbException>>()
+        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)