Add an option to only accept messages from contacts

data/src/main/java/com/message/ink/blocking/QksmsBlockingClient.kt

@@ -18,20 +18,55 @@
  */
 package com.message.ink.blocking
 
+import com.message.ink.manager.PermissionManager
 import com.message.ink.repository.BlockingRepository
+import com.message.ink.repository.ContactRepository
+import com.message.ink.util.Preferences
 import io.reactivex.Completable
 import io.reactivex.Single
 import javax.inject.Inject
 
+/**
+ * Decides whether a message should be blocked.
+ *
+ * Kept as a pure function so the truth table can be tested without Android, a content resolver or
+ * a preferences store. The permission argument matters: without it, a denied contacts permission
+ * makes every lookup fail, "not a contact" quietly becomes "everyone", and the rule blocks the
+ * whole inbox.
+ */
+internal fun shouldBlockMessage(
+    isBlacklisted: Boolean,
+    onlyAllowContacts: Boolean,
+    canReadContacts: Boolean,
+    isContact: Boolean,
+): Boolean = isBlacklisted || (onlyAllowContacts && canReadContacts && !isContact)
+
 class QksmsBlockingClient @Inject constructor(
-    private val blockingRepo: BlockingRepository
+    private val blockingRepo: BlockingRepository,
+    private val contactRepo: ContactRepository,
+    private val permissionManager: PermissionManager,
+    private val prefs: Preferences
 ) : BlockingClient {
 
     override fun isAvailable(): Boolean = true
 
     override fun getClientCapability() = BlockingClient.Capability.BLOCK_WITHOUT_PERMISSION
 
-    override fun shouldBlock(address: String): Single<BlockingClient.Action> = isBlacklisted(address)
+    override fun shouldBlock(address: String): Single<BlockingClient.Action> = Single.fromCallable {
+        val onlyAllowContacts = prefs.blockNonContacts.get()
+        val block = shouldBlockMessage(
+            isBlacklisted = blockingRepo.isBlocked(address),
+            onlyAllowContacts = onlyAllowContacts,
+            canReadContacts = permissionManager.hasContacts(),
+            // Only looked up when the setting is on, so the usual case stays one lookup rather
+            // than a contacts-provider query per incoming message.
+            isContact = onlyAllowContacts && contactRepo.isContact(address),
+        )
+        when (block) {
+            true -> BlockingClient.Action.Block()
+            false -> BlockingClient.Action.Unblock
+        }
+    }
 
     override fun isBlacklisted(address: String): Single<BlockingClient.Action> = Single.fromCallable {
         when (blockingRepo.isBlocked(address)) {

data/src/test/java/com/message/ink/blocking/QksmsBlockingClientTest.kt

@@ -0,0 +1,86 @@
+package com.message.ink.blocking
+
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class QksmsBlockingClientTest {
+
+    @Test
+    fun blacklistedNumbersAreBlocked() {
+        assertTrue(
+            shouldBlockMessage(
+                isBlacklisted = true,
+                onlyAllowContacts = false,
+                canReadContacts = true,
+                isContact = false,
+            )
+        )
+    }
+
+    @Test
+    fun strangersArriveNormallyWhenTheRuleIsOff() {
+        assertFalse(
+            shouldBlockMessage(
+                isBlacklisted = false,
+                onlyAllowContacts = false,
+                canReadContacts = true,
+                isContact = false,
+            )
+        )
+    }
+
+    @Test
+    fun strangersAreBlockedWhenTheRuleIsOn() {
+        assertTrue(
+            shouldBlockMessage(
+                isBlacklisted = false,
+                onlyAllowContacts = true,
+                canReadContacts = true,
+                isContact = false,
+            )
+        )
+    }
+
+    @Test
+    fun contactsStillGetThroughWhenTheRuleIsOn() {
+        assertFalse(
+            shouldBlockMessage(
+                isBlacklisted = false,
+                onlyAllowContacts = true,
+                canReadContacts = true,
+                isContact = true,
+            )
+        )
+    }
+
+    /**
+     * The one that matters. Without the contacts permission every lookup comes back "not a
+     * contact", so a rule that trusted it alone would block every incoming message - including
+     * from the people the phone's owner actually talks to - and it would look like the network
+     * had gone quiet rather than like a setting had misfired.
+     */
+    @Test
+    fun nothingIsBlockedWhenContactsCannotBeRead() {
+        assertFalse(
+            shouldBlockMessage(
+                isBlacklisted = false,
+                onlyAllowContacts = true,
+                canReadContacts = false,
+                isContact = false,
+            )
+        )
+    }
+
+    @Test
+    fun blacklistStillAppliesWhenContactsCannotBeRead() {
+        assertTrue(
+            shouldBlockMessage(
+                isBlacklisted = true,
+                onlyAllowContacts = true,
+                canReadContacts = false,
+                isContact = false,
+            )
+        )
+    }
+}

domain/src/main/java/com/message/ink/util/Preferences.kt

@@ -118,6 +118,7 @@ class Preferences @Inject constructor(
     val textSize = rxPrefs.getInteger("textSize", TEXT_SIZE_LARGE)
     val blockingManager = rxPrefs.getInteger("blockingManager", BLOCKING_MANAGER_QKSMS)
     val drop = rxPrefs.getBoolean("drop", false)
+    val blockNonContacts = rxPrefs.getBoolean("blockNonContacts", false)
     val silentNotContact = rxPrefs.getBoolean("silentNotContact", false)
     val notifAction1 = rxPrefs.getInteger("notifAction1", NOTIFICATION_ACTION_READ)
     val notifAction2 = rxPrefs.getInteger("notifAction2", NOTIFICATION_ACTION_REPLY)

presentation/src/main/java/com/message/ink/feature/blocking/BlockingController.kt

@@ -42,6 +42,7 @@ class BlockingController : QkController<BlockingView, BlockingState, BlockingPre
     override val messageContentFiltersIntent by lazy { messageContentFilters.clicks() }
     override val blockedMessagesIntent by lazy { blockedMessages.clicks() }
     override val dropClickedIntent by lazy { drop.clicks() }
+    override val blockNonContactsClickedIntent by lazy { blockNonContacts.clicks() }
 
     @Inject lateinit var colors: Colors
     @Inject override lateinit var presenter: BlockingPresenter
@@ -67,6 +68,14 @@ class BlockingController : QkController<BlockingView, BlockingState, BlockingPre
     override fun render(state: BlockingState) {
         blockingManager.summary = state.blockingManager
         drop.checkbox.isChecked = state.dropEnabled
+
+        blockNonContacts.checkbox.isChecked = state.blockNonContactsEnabled
+        blockNonContacts.isEnabled = state.usingBuiltInBlocking && state.canReadContacts
+        blockNonContacts.summary = when {
+            !state.usingBuiltInBlocking -> activity?.getString(R.string.blocking_manager_title)
+            !state.canReadContacts -> activity?.getString(R.string.blocking_non_contacts_no_permission)
+            else -> activity?.getString(R.string.blocking_non_contacts_summary)
+        }
         blockedMessages.isEnabled = !state.dropEnabled
     }
 

presentation/src/main/java/com/message/ink/feature/blocking/BlockingPresenter.kt

@@ -24,13 +24,15 @@ import com.uber.autodispose.autoDisposable
 import com.message.ink.R
 import com.message.ink.blocking.BlockingClient
 import com.message.ink.common.base.QkPresenter
+import com.message.ink.manager.PermissionManager
 import com.message.ink.util.Preferences
 import io.reactivex.rxkotlin.plusAssign
 import javax.inject.Inject
 
 class BlockingPresenter @Inject constructor(
     context: Context,
     private val blockingClient: BlockingClient,
+    private val permissionManager: PermissionManager,
     private val prefs: Preferences
 ) : QkPresenter<BlockingView, BlockingState>(BlockingState()) {
 
@@ -49,6 +51,15 @@ class BlockingPresenter @Inject constructor(
 
         disposables += prefs.drop.asObservable()
                 .subscribe { enabled -> newState { copy(dropEnabled = enabled) } }
+
+        disposables += prefs.blockNonContacts.asObservable()
+                .subscribe { enabled -> newState { copy(blockNonContactsEnabled = enabled) } }
+
+        disposables += prefs.blockingManager.asObservable()
+                .map { manager -> manager == Preferences.BLOCKING_MANAGER_QKSMS }
+                .subscribe { builtIn -> newState { copy(usingBuiltInBlocking = builtIn) } }
+
+        newState { copy(canReadContacts = permissionManager.hasContacts()) }
     }
 
     override fun bindIntents(view: BlockingView) {
@@ -80,6 +91,10 @@ class BlockingPresenter @Inject constructor(
         view.dropClickedIntent
                 .autoDisposable(view.scope())
                 .subscribe { prefs.drop.set(!prefs.drop.get()) }
+
+        view.blockNonContactsClickedIntent
+                .autoDisposable(view.scope())
+                .subscribe { prefs.blockNonContacts.set(!prefs.blockNonContacts.get()) }
     }
 
 }

presentation/src/main/java/com/message/ink/feature/blocking/BlockingState.kt

@@ -20,5 +20,10 @@ package com.message.ink.feature.blocking
 
 data class BlockingState(
     val blockingManager: String = "",
-    val dropEnabled: Boolean = false
+    val dropEnabled: Boolean = false,
+    val blockNonContactsEnabled: Boolean = false,
+    val canReadContacts: Boolean = true,
+    // The rule lives in the built-in blocking client, so it does nothing while a third-party
+    // manager is selected. Better to say so than to leave a switch that silently has no effect.
+    val usingBuiltInBlocking: Boolean = true
 )

presentation/src/main/java/com/message/ink/feature/blocking/BlockingView.kt

@@ -28,6 +28,7 @@ interface BlockingView : QkViewContract<BlockingState> {
     val messageContentFiltersIntent: Observable<*>
     val blockedMessagesIntent: Observable<*>
     val dropClickedIntent: Observable<*>
+    val blockNonContactsClickedIntent: Observable<*>
 
     fun openBlockingManager()
     fun openBlockedNumbers()

presentation/src/main/res/layout/blocking_controller.xml

@@ -60,6 +60,14 @@
             app:title="@string/blocking_drop_title"
             app:widget="@layout/settings_switch_widget" />
 
+        <com.message.ink.common.widget.PreferenceView
+            android:id="@+id/blockNonContacts"
+            android:layout_width="match_parent"
+            android:layout_height="wrap_content"
+            app:summary="@string/blocking_non_contacts_summary"
+            app:title="@string/blocking_non_contacts_title"
+            app:widget="@layout/settings_switch_widget" />
+
         <com.message.ink.common.widget.PreferenceView
             android:id="@+id/blockedMessages"
             android:layout_width="match_parent"

presentation/src/main/res/values/strings.xml

@@ -348,6 +348,9 @@
     <string name="blocking_drop_summary">Drop incoming messages from blocked senders instead of hiding them</string>
     <string name="blocking_conversations">Blocked conversations</string>
 
+    <string name="blocking_non_contacts_title">Only allow contacts</string>
+    <string name="blocking_non_contacts_summary">Block messages from anyone who is not in your contacts</string>
+    <string name="blocking_non_contacts_no_permission">Needs permission to read your contacts</string>
     <string name="blocking_manager_title">Blocking Manager</string>
     <string name="blocking_manager_qksms_summary">Built-in blocking functionality in QUIK</string>
     <string name="blocking_manager_call_blocker_title" translatable="false">Call Blocker - Incoming/Outgoing</string>