From 8991762b0cd75871733c97643a4ff5105c9c3624 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Thu, 15 Apr 2021 16:12:38 +0100 Subject: [PATCH] Final code review nitpicks. --- .../ConversationSettingsDialog.java | 5 ++++- .../conversation/ConversationViewModel.java | 2 +- .../briar/android/util/UiUtils.java | 3 +-- briar-android/src/main/res/values/strings.xml | 4 ++-- .../api/autodelete/AutoDeleteManager.java | 3 ++- .../AutoDeleteIntegrationTest.java | 5 ++--- .../AbstractAutoDeleteIntegrationTest.java | 7 +++++-- .../AutoDeleteBlogIntegrationTest.java | 15 ++------------ .../AutoDeleteForumIntegrationTest.java | 20 ++++--------------- 9 files changed, 23 insertions(+), 41 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationSettingsDialog.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationSettingsDialog.java index 4806a4e7a..f55d4c6d9 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationSettingsDialog.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationSettingsDialog.java @@ -26,6 +26,7 @@ import androidx.fragment.app.DialogFragment; import androidx.fragment.app.FragmentActivity; import androidx.lifecycle.ViewModelProvider; +import static java.util.logging.Level.INFO; import static org.briarproject.briar.android.conversation.ConversationActivity.CONTACT_ID; import static org.briarproject.briar.api.autodelete.AutoDeleteConstants.NO_AUTO_DELETE_TIMER; @@ -102,7 +103,9 @@ public class ConversationSettingsDialog extends DialogFragment { viewModel.getAutoDeleteTimer() .observe(getViewLifecycleOwner(), timer -> { - LOG.info("Received auto delete timer: " + timer); + if (LOG.isLoggable(INFO)) { + LOG.info("Received auto delete timer: " + timer); + } boolean disappearingMessages = timer != NO_AUTO_DELETE_TIMER; switchDisappearingMessages diff --git a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java index 8bbd5c325..1d477da11 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/conversation/ConversationViewModel.java @@ -176,7 +176,7 @@ public class ConversationViewModel extends DbViewModel } else if (e instanceof AutoDeleteTimerMirroredEvent) { AutoDeleteTimerMirroredEvent a = (AutoDeleteTimerMirroredEvent) e; if (a.getContactId().equals(contactId)) { - autoDeleteTimer.postValue(a.getNewTimer()); + autoDeleteTimer.setValue(a.getNewTimer()); } } else if (e instanceof AvatarUpdatedEvent) { AvatarUpdatedEvent a = (AvatarUpdatedEvent) e; diff --git a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java index c8b223ab1..5193caa21 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/util/UiUtils.java @@ -170,8 +170,7 @@ public class UiUtils { /** * Returns the given duration in a human-friendly format. For example, - * "7 days" or "1 hour". Returns only the largest meaningful unit of time, - * from days up to minutes. + * "7 days" or "1 hour 3 minutes". */ public static String formatDuration(Context ctx, long millis) { Resources r = ctx.getResources(); diff --git a/briar-android/src/main/res/values/strings.xml b/briar-android/src/main/res/values/strings.xml index edce11614..67431f249 100644 --- a/briar-android/src/main/res/values/strings.xml +++ b/briar-android/src/main/res/values/strings.xml @@ -172,7 +172,7 @@ Your messages will disappear after %1$s. %2$s Your messages will not disappear. %1$s - + %1$s\'s messages will disappear after %2$s. %3$s %d minute @@ -186,7 +186,7 @@ %d day %d days - + %1$s\'s messages will not disappear. %2$s Tap to learn more. Disappearing messages changed diff --git a/briar-api/src/main/java/org/briarproject/briar/api/autodelete/AutoDeleteManager.java b/briar-api/src/main/java/org/briarproject/briar/api/autodelete/AutoDeleteManager.java index 2f2bccaa9..b5e2bc7f3 100644 --- a/briar-api/src/main/java/org/briarproject/briar/api/autodelete/AutoDeleteManager.java +++ b/briar-api/src/main/java/org/briarproject/briar/api/autodelete/AutoDeleteManager.java @@ -40,7 +40,8 @@ public interface AutoDeleteManager { /** * Returns the auto-delete timer duration for the given contact, for use in - * a message with the given timestamp. The timestamp is stored. + * a message with the given timestamp. The timestamp is stored. This method + * requires a read-write transaction. */ long getAutoDeleteTimer(Transaction txn, ContactId c, long timestamp) throws DbException; diff --git a/briar-core/src/test/java/org/briarproject/briar/introduction/AutoDeleteIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/introduction/AutoDeleteIntegrationTest.java index 63e17436a..fac110dd6 100644 --- a/briar-core/src/test/java/org/briarproject/briar/introduction/AutoDeleteIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/introduction/AutoDeleteIntegrationTest.java @@ -17,7 +17,6 @@ import org.briarproject.briar.api.introduction.IntroductionResponse; import org.briarproject.briar.api.introduction.event.IntroductionResponseReceivedEvent; import org.briarproject.briar.autodelete.AbstractAutoDeleteTest; import org.briarproject.briar.test.BriarIntegrationTestComponent; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -391,13 +390,13 @@ public class AutoDeleteIntegrationTest extends AbstractAutoDeleteTest { // FIRST CYCLE introduceAndAutoDecline(); - Assert.assertTrue(c0.getIntroductionManager() + assertTrue(c0.getIntroductionManager() .canIntroduce(contact1From0, contact2From0)); // SECOND CYCLE introduceAndAutoDecline(); - Assert.assertTrue(c0.getIntroductionManager() + assertTrue(c0.getIntroductionManager() .canIntroduce(contact1From0, contact2From0)); } diff --git a/briar-core/src/test/java/org/briarproject/briar/sharing/AbstractAutoDeleteIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/sharing/AbstractAutoDeleteIntegrationTest.java index 19a01fbf3..9de9d5421 100644 --- a/briar-core/src/test/java/org/briarproject/briar/sharing/AbstractAutoDeleteIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/sharing/AbstractAutoDeleteIntegrationTest.java @@ -13,6 +13,7 @@ import org.briarproject.briar.api.sharing.Shareable; import org.briarproject.briar.api.sharing.SharingInvitationItem; import org.briarproject.briar.api.sharing.SharingManager; import org.briarproject.briar.autodelete.AbstractAutoDeleteTest; +import org.junit.Test; import java.util.Collection; @@ -42,7 +43,8 @@ public abstract class AbstractAutoDeleteIntegrationTest protected abstract Class> getResponseReceivedEventClass(); - protected void testAutoDeclinedSharing() throws Exception { + @Test + public void testAutoDeclinedSharing() throws Exception { setAutoDeleteTimer(c0, contactId1From0, MIN_AUTO_DELETE_TIMER_MS); // Send invitation @@ -191,7 +193,8 @@ public abstract class AbstractAutoDeleteIntegrationTest assertGroupCount(c1, contactId0From1, 1, 1); } - protected void testRespondAfterSenderDeletedInvitation() throws Exception { + @Test + public void testRespondAfterSenderDeletedInvitation() throws Exception { setAutoDeleteTimer(c0, contactId1From0, MIN_AUTO_DELETE_TIMER_MS); assertTrue(subscriptions0().contains(getShareable())); diff --git a/briar-core/src/test/java/org/briarproject/briar/sharing/AutoDeleteBlogIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/sharing/AutoDeleteBlogIntegrationTest.java index 0dcc7c450..dda1800f8 100644 --- a/briar-core/src/test/java/org/briarproject/briar/sharing/AutoDeleteBlogIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/sharing/AutoDeleteBlogIntegrationTest.java @@ -4,14 +4,13 @@ import org.briarproject.bramble.api.db.DbException; import org.briarproject.briar.api.blog.Blog; import org.briarproject.briar.api.blog.BlogManager; import org.briarproject.briar.api.blog.event.BlogInvitationResponseReceivedEvent; -import org.briarproject.briar.api.conversation.ConversationManager; +import org.briarproject.briar.api.conversation.ConversationManager.ConversationClient; import org.briarproject.briar.api.conversation.event.ConversationMessageReceivedEvent; import org.briarproject.briar.api.sharing.InvitationResponse; import org.briarproject.briar.api.sharing.Shareable; import org.briarproject.briar.api.sharing.SharingManager; import org.briarproject.briar.test.BriarIntegrationTestComponent; import org.junit.Before; -import org.junit.Test; import java.util.Collection; @@ -40,7 +39,7 @@ public class AutoDeleteBlogIntegrationTest } @Override - protected ConversationManager.ConversationClient getConversationClient( + protected ConversationClient getConversationClient( BriarIntegrationTestComponent component) { return component.getBlogSharingManager(); } @@ -74,14 +73,4 @@ public class AutoDeleteBlogIntegrationTest protected Class> getResponseReceivedEventClass() { return responseReceivedEventClass; } - - @Test - public void testAutoDeclinedBlogSharing() throws Exception { - testAutoDeclinedSharing(); - } - - @Test - public void testRespondAfterSenderDeletedBlogInvitation() throws Exception { - testRespondAfterSenderDeletedInvitation(); - } } diff --git a/briar-core/src/test/java/org/briarproject/briar/sharing/AutoDeleteForumIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/sharing/AutoDeleteForumIntegrationTest.java index b5f87e79f..b8bfdef24 100644 --- a/briar-core/src/test/java/org/briarproject/briar/sharing/AutoDeleteForumIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/sharing/AutoDeleteForumIntegrationTest.java @@ -1,7 +1,7 @@ package org.briarproject.briar.sharing; import org.briarproject.bramble.api.db.DbException; -import org.briarproject.briar.api.conversation.ConversationManager; +import org.briarproject.briar.api.conversation.ConversationManager.ConversationClient; import org.briarproject.briar.api.conversation.event.ConversationMessageReceivedEvent; import org.briarproject.briar.api.forum.Forum; import org.briarproject.briar.api.forum.ForumManager; @@ -11,7 +11,6 @@ import org.briarproject.briar.api.sharing.Shareable; import org.briarproject.briar.api.sharing.SharingManager; import org.briarproject.briar.test.BriarIntegrationTestComponent; import org.junit.Before; -import org.junit.Test; import java.util.Collection; @@ -20,10 +19,10 @@ public class AutoDeleteForumIntegrationTest private SharingManager sharingManager0; private SharingManager sharingManager1; - protected Forum shareable; + private Forum shareable; private ForumManager manager0; private ForumManager manager1; - protected Class + private Class responseReceivedEventClass; @Before @@ -39,7 +38,7 @@ public class AutoDeleteForumIntegrationTest } @Override - protected ConversationManager.ConversationClient getConversationClient( + protected ConversationClient getConversationClient( BriarIntegrationTestComponent component) { return component.getForumSharingManager(); } @@ -73,15 +72,4 @@ public class AutoDeleteForumIntegrationTest protected Class> getResponseReceivedEventClass() { return responseReceivedEventClass; } - - @Test - public void testAutoDeclinedForumSharing() throws Exception { - testAutoDeclinedSharing(); - } - - @Test - public void testRespondAfterSenderDeletedForumInvitation() - throws Exception { - testRespondAfterSenderDeletedInvitation(); - } }