Request permissions for older versions; filename things; polishing

app/src/main/AndroidManifest.xml

@@ -13,7 +13,7 @@
     <uses-permission android:name="android.permission.WAKE_LOCK"/>
     <uses-permission android:name="android.permission.RECEIVE_BOOT_COMPLETED"/>
     <uses-permission android:name="android.permission.VIBRATE"/>
-    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE"/> <!-- Only required on SDK <= 28 -->
+    <uses-permission android:name="android.permission.WRITE_EXTERNAL_STORAGE" android:maxSdkVersion="28"/> <!-- Only required on SDK <= 28 -->
     <uses-permission android:name="android.permission.REQUEST_INSTALL_PACKAGES"/> <!-- Required to install packages downloaded through ntfy; craazyy! -->
 
     <application
@@ -95,6 +95,16 @@
             </intent-filter>
         </receiver>
 
+        <!-- Broadcast receiver for the "Download" attachment action in the notification popup -->
+        <receiver
+                android:name=".msg.NotificationService$DownloadBroadcastReceiver"
+                android:enabled="true"
+                android:exported="false">
+            <intent-filter>
+                <action android:name="io.heckel.ntfy.DOWNLOAD_ATTACHMENT"/>
+            </intent-filter>
+        </receiver>
+
         <!-- Firebase messaging (note that this is empty in the F-Droid flavor) -->
         <service
                 android:name=".firebase.FirebaseService"
@@ -110,5 +120,16 @@
                 android:name="com.google.firebase.messaging.default_notification_icon"
                 android:resource="@drawable/ic_notification"/>
 
+        <!-- FileProvider required for older Android versions (<= P), to allow passing the file URI in the open intent.
+             Avoids "exposed beyong app through Intent.getData" exception, see see https://stackoverflow.com/a/57288352/1440785 -->
+        <provider
+                android:name="androidx.core.content.FileProvider"
+                android:authorities="${applicationId}.provider"
+                android:exported="false"
+                android:grantUriPermissions="true">
+            <meta-data
+                    android:name="android.support.FILE_PROVIDER_PATHS"
+                    android:resource="@xml/file_paths"/>
+        </provider>
     </application>
 </manifest>

app/src/main/java/io/heckel/ntfy/data/Database.kt

@@ -73,6 +73,7 @@ data class Attachment(
 
 const val PROGRESS_NONE = -1
 const val PROGRESS_INDETERMINATE = -2
+const val PROGRESS_FAILED = -3
 const val PROGRESS_DONE = 100
 
 @androidx.room.Database(entities = [Subscription::class, Notification::class], version = 6)

app/src/main/java/io/heckel/ntfy/data/Repository.kt

@@ -1,6 +1,7 @@
 package io.heckel.ntfy.data
 
 import android.content.SharedPreferences
+import android.os.Build
 import android.util.Log
 import androidx.annotation.WorkerThread
 import androidx.lifecycle.*
@@ -162,7 +163,8 @@ class Repository(private val sharedPrefs: SharedPreferences, private val subscri
     }
 
     fun getAutoDownloadEnabled(): Boolean {
-        return sharedPrefs.getBoolean(SHARED_PREFS_AUTO_DOWNLOAD_ENABLED, true) // Enabled by default
+        val defaultEnabled = Build.VERSION.SDK_INT > Build.VERSION_CODES.P // Need to request permission on older versions
+        return sharedPrefs.getBoolean(SHARED_PREFS_AUTO_DOWNLOAD_ENABLED, defaultEnabled)
     }
 
     fun setAutoDownloadEnabled(enabled: Boolean) {

app/src/main/java/io/heckel/ntfy/msg/AttachmentDownloaderWorker.kt

@@ -2,15 +2,24 @@ package io.heckel.ntfy.msg
 
 import android.content.ContentValues
 import android.content.Context
+import android.os.Build
 import android.os.Environment
+import android.os.Handler
+import android.os.Looper
 import android.provider.MediaStore
 import android.util.Log
+import android.widget.Toast
+import androidx.core.content.FileProvider
 import androidx.work.Worker
 import androidx.work.WorkerParameters
+import io.heckel.ntfy.BuildConfig
+import io.heckel.ntfy.R
 import io.heckel.ntfy.app.Application
 import io.heckel.ntfy.data.*
+import io.heckel.ntfy.util.queryFilename
 import okhttp3.OkHttpClient
 import okhttp3.Request
+import java.io.File
 import java.util.concurrent.TimeUnit
 
 class AttachmentDownloadWorker(private val context: Context, params: WorkerParameters) : Worker(context, params) {
@@ -37,6 +46,7 @@ class AttachmentDownloadWorker(private val context: Context, params: WorkerParam
         val attachment = notification.attachment ?: return
         Log.d(TAG, "Downloading attachment from ${attachment.url}")
 
+        try {
             val request = Request.Builder()
                 .url(attachment.url)
                 .addHeader("User-Agent", ApiService.USER_AGENT)
@@ -48,6 +58,10 @@ class AttachmentDownloadWorker(private val context: Context, params: WorkerParam
                 val name = attachment.name
                 val size = attachment.size ?: 0
                 val resolver = applicationContext.contentResolver
+                val uri = if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) {
+                    val file = ensureSafeNewFile(Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS), name)
+                    FileProvider.getUriForFile(context, FILE_PROVIDER_AUTHORITY, file)
+                } else {
                     val details = ContentValues().apply {
                         put(MediaStore.MediaColumns.DISPLAY_NAME, name)
                         if (attachment.type != null) {
@@ -56,8 +70,9 @@ class AttachmentDownloadWorker(private val context: Context, params: WorkerParam
                         put(MediaStore.MediaColumns.RELATIVE_PATH, Environment.DIRECTORY_DOWNLOADS)
                         put(MediaStore.MediaColumns.IS_DOWNLOAD, 1)
                     }
-            val uri = resolver.insert(MediaStore.Files.getContentUri(MediaStore.VOLUME_EXTERNAL), details)
+                    resolver.insert(MediaStore.Files.getContentUri(MediaStore.VOLUME_EXTERNAL), details)
                         ?: throw Exception("Cannot get content URI")
+                }
                 Log.d(TAG, "Starting download to content URI: $uri")
                 var bytesCopied: Long = 0
                 val out = resolver.openOutputStream(uri) ?: throw Exception("Cannot open output stream")
@@ -81,14 +96,56 @@ class AttachmentDownloadWorker(private val context: Context, params: WorkerParam
                     }
                 }
                 Log.d(TAG, "Attachment download: successful response, proceeding with download")
-            val newAttachment = attachment.copy(contentUri = uri.toString(), size = bytesCopied, progress = PROGRESS_DONE)
+                val actualName = queryFilename(context, uri.toString(), name)
+                val newAttachment = attachment.copy(
+                    name = actualName,
+                    size = bytesCopied,
+                    contentUri = uri.toString(),
+                    progress = PROGRESS_DONE
+                )
                 val newNotification = notification.copy(attachment = newAttachment)
                 repository.updateNotification(newNotification)
                 notifier.update(subscription, newNotification)
             }
+        } catch (e: Exception) {
+            Log.w(TAG, "Attachment download failed", e)
+
+            val newAttachment = attachment.copy(progress = PROGRESS_FAILED)
+            val newNotification = notification.copy(attachment = newAttachment)
+            notifier.update(subscription, newNotification)
+            repository.updateNotification(newNotification)
+
+            // Toast in a Worker: https://stackoverflow.com/a/56428145/1440785
+            val handler = Handler(Looper.getMainLooper())
+            handler.postDelayed({
+                Toast
+                    .makeText(context, context.getString(R.string.detail_item_download_failed, e.message), Toast.LENGTH_LONG)
+                    .show()
+            }, 200)
+        }
+    }
+
+    private fun ensureSafeNewFile(dir: File, name: String): File {
+        val safeName = name.replace("[^-_.()\\w]+".toRegex(), "_");
+        val file = File(dir, safeName)
+        if (!file.exists()) {
+            return file
+        }
+        (1..1000).forEach { i ->
+            val newFile = File(dir, if (file.extension == "") {
+                "${file.nameWithoutExtension} ($i)"
+            } else {
+                "${file.nameWithoutExtension} ($i).${file.extension}"
+            })
+            if (!newFile.exists()) {
+                return newFile
+            }
+        }
+        throw Exception("Cannot find safe file")
     }
 
     companion object {
         private const val TAG = "NtfyAttachDownload"
+        private const val FILE_PROVIDER_AUTHORITY = BuildConfig.APPLICATION_ID + ".provider" // See AndroidManifest.xml
     }
 }

app/src/main/java/io/heckel/ntfy/msg/NotificationDispatcher.kt

@@ -25,6 +25,8 @@ class NotificationDispatcher(val context: Context, val repository: Repository) {
     }
 
     fun dispatch(subscription: Subscription, notification: Notification) {
+        Log.d(TAG, "Dispatching $notification for subscription $subscription")
+
         val muted = getMuted(subscription)
         val notify = shouldNotify(subscription, notification, muted)
         val broadcast = shouldBroadcast(subscription)

app/src/main/java/io/heckel/ntfy/msg/NotificationService.kt

@@ -9,9 +9,12 @@ import android.os.Build
 import android.util.Log
 import androidx.core.app.NotificationCompat
 import androidx.core.content.ContextCompat
+import androidx.work.OneTimeWorkRequest
+import androidx.work.WorkManager
+import androidx.work.workDataOf
 import io.heckel.ntfy.R
+import io.heckel.ntfy.data.*
 import io.heckel.ntfy.data.Notification
-import io.heckel.ntfy.data.Subscription
 import io.heckel.ntfy.ui.DetailActivity
 import io.heckel.ntfy.ui.MainActivity
 import io.heckel.ntfy.util.*
@@ -49,40 +52,25 @@ class NotificationService(val context: Context) {
 
     private fun displayInternal(subscription: Subscription, notification: Notification, update: Boolean = false) {
         val title = formatTitle(subscription, notification)
-        val message = maybeWithAttachmentInfo(formatMessage(notification), notification)
         val channelId = toChannelId(notification.priority)
         val builder = NotificationCompat.Builder(context, channelId)
             .setSmallIcon(R.drawable.ic_notification)
             .setColor(ContextCompat.getColor(context, R.color.primaryColor))
             .setContentTitle(title)
-            .setContentText(message)
             .setOnlyAlertOnce(true) // Do not vibrate or play sound if already showing (updates!)
             .setAutoCancel(true) // Cancel when notification is clicked
-        setStyle(builder, notification, message) // Preview picture or big text style
+        setStyleAndText(builder, notification) // Preview picture or big text style
-        setContentIntent(builder, subscription, notification)
+        setClickAction(builder, subscription, notification)
         maybeSetSound(builder, update)
         maybeSetProgress(builder, notification)
         maybeAddOpenAction(builder, notification)
         maybeAddBrowseAction(builder, notification)
+        maybeAddDownloadAction(builder, notification)
 
         maybeCreateNotificationChannel(notification.priority)
         notificationManager.notify(notification.notificationId, builder.build())
     }
 
-    // FIXME duplicate code
-    private fun maybeWithAttachmentInfo(message: String, notification: Notification): String {
-        val att = notification.attachment ?: return message
-        if (att.progress < 0) return message
-        val infos = mutableListOf<String>()
-        if (att.name != null) infos.add(att.name)
-        if (att.size != null) infos.add(formatBytes(att.size))
-        //if (att.expires != null && att.expires != 0L) infos.add(formatDateShort(att.expires))
-        if (att.progress in 0..99) infos.add("${att.progress}%")
-        if (infos.size == 0) return message
-        if (att.progress < 100) return "Downloading ${infos.joinToString(", ")}\n${message}"
-        return "${message}\nFile: ${infos.joinToString(", ")}"
-    }
-
     private fun maybeSetSound(builder: NotificationCompat.Builder, update: Boolean) {
         if (!update) {
             val defaultSoundUri = RingtoneManager.getDefaultUri(RingtoneManager.TYPE_NOTIFICATION)
@@ -92,7 +80,7 @@ class NotificationService(val context: Context) {
         }
     }
 
-    private fun setStyle(builder: NotificationCompat.Builder, notification: Notification, message: String) {
+    private fun setStyleAndText(builder: NotificationCompat.Builder, notification: Notification) {
         val contentUri = notification.attachment?.contentUri
         val isSupportedImage = supportedImage(notification.attachment?.type)
         if (contentUri != null && isSupportedImage) {
@@ -101,19 +89,46 @@ class NotificationService(val context: Context) {
                 val bitmapStream = resolver.openInputStream(Uri.parse(contentUri))
                 val bitmap = BitmapFactory.decodeStream(bitmapStream)
                 builder
+                    .setContentText(formatMessage(notification))
                     .setLargeIcon(bitmap)
                     .setStyle(NotificationCompat.BigPictureStyle()
                         .bigPicture(bitmap)
                         .bigLargeIcon(null))
             } catch (_: Exception) {
-                builder.setStyle(NotificationCompat.BigTextStyle().bigText(message))
+                val message = formatMessageMaybeWithAttachmentInfo(notification)
+                builder
+                    .setContentText(message)
+                    .setStyle(NotificationCompat.BigTextStyle().bigText(message))
             }
         } else {
-            builder.setStyle(NotificationCompat.BigTextStyle().bigText(message))
+            val message = formatMessageMaybeWithAttachmentInfo(notification)
+            builder
+                .setContentText(message)
+                .setStyle(NotificationCompat.BigTextStyle().bigText(message))
         }
     }
 
-    private fun setContentIntent(builder: NotificationCompat.Builder, subscription: Subscription, notification: Notification) {
+    private fun formatMessageMaybeWithAttachmentInfo(notification: Notification): String {
+        val message = formatMessage(notification)
+        val attachment = notification.attachment ?: return message
+        val infos = if (attachment.size != null) {
+            "${attachment.name}, ${formatBytes(attachment.size)}"
+        } else {
+            attachment.name
+        }
+        if (attachment.progress in 0..99) {
+            return context.getString(R.string.notification_popup_file_downloading, infos, attachment.progress, message)
+        }
+        if (attachment.progress == PROGRESS_DONE) {
+            return context.getString(R.string.notification_popup_file_download_successful, message, infos)
+        }
+        if (attachment.progress == PROGRESS_FAILED) {
+            return context.getString(R.string.notification_popup_file_download_failed, message, infos)
+        }
+        return context.getString(R.string.notification_popup_file, message, infos)
+    }
+
+    private fun setClickAction(builder: NotificationCompat.Builder, subscription: Subscription, notification: Notification) {
         if (notification.click == "") {
             builder.setContentIntent(detailActivityIntent(subscription))
         } else {
@@ -140,6 +155,8 @@ class NotificationService(val context: Context) {
         if (notification.attachment?.contentUri != null) {
             val contentUri = Uri.parse(notification.attachment.contentUri)
             val intent = Intent(Intent.ACTION_VIEW, contentUri)
+            intent.setDataAndType(contentUri, notification.attachment.type ?: "application/octet-stream") // Required for Android <= P
+            intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
             val pendingIntent = PendingIntent.getActivity(context, 0, intent, 0)
             builder.addAction(NotificationCompat.Action.Builder(0, context.getString(R.string.notification_popup_action_open), pendingIntent).build())
         }
@@ -148,11 +165,33 @@ class NotificationService(val context: Context) {
     private fun maybeAddBrowseAction(builder: NotificationCompat.Builder, notification: Notification) {
         if (notification.attachment?.contentUri != null) {
             val intent = Intent(DownloadManager.ACTION_VIEW_DOWNLOADS)
+            intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
             val pendingIntent = PendingIntent.getActivity(context, 0, intent, 0)
             builder.addAction(NotificationCompat.Action.Builder(0, context.getString(R.string.notification_popup_action_browse), pendingIntent).build())
         }
     }
 
+    private fun maybeAddDownloadAction(builder: NotificationCompat.Builder, notification: Notification) {
+        if (notification.attachment?.contentUri == null && listOf(PROGRESS_NONE, PROGRESS_FAILED).contains(notification.attachment?.progress)) {
+            val intent = Intent(context, DownloadBroadcastReceiver::class.java)
+            intent.putExtra("id", notification.id)
+            val pendingIntent = PendingIntent.getBroadcast(context, 0, intent, PendingIntent.FLAG_UPDATE_CURRENT)
+            builder.addAction(NotificationCompat.Action.Builder(0, context.getString(R.string.notification_popup_action_download), pendingIntent).build())
+        }
+    }
+
+    class DownloadBroadcastReceiver : android.content.BroadcastReceiver() {
+        override fun onReceive(context: Context, intent: Intent) {
+            val id = intent.getStringExtra("id") ?: return
+            Log.d(TAG, "Enqueuing work to download attachment for notification $id")
+            val workManager = WorkManager.getInstance(context)
+            val workRequest = OneTimeWorkRequest.Builder(AttachmentDownloadWorker::class.java)
+                .setInputData(workDataOf("id" to id))
+                .build()
+            workManager.enqueue(workRequest)
+        }
+    }
+
     private fun detailActivityIntent(subscription: Subscription): PendingIntent? {
         val intent = Intent(context, DetailActivity::class.java)
         intent.putExtra(MainActivity.EXTRA_SUBSCRIPTION_ID, subscription.id)
@@ -215,6 +254,7 @@ class NotificationService(val context: Context) {
 
     companion object {
         private const val TAG = "NtfyNotifService"
+        private const val DOWNLOAD_ATTACHMENT_ACTION = "io.heckel.ntfy.DOWNLOAD_ATTACHMENT"
 
         private const val CHANNEL_ID_MIN = "ntfy-min"
         private const val CHANNEL_ID_LOW = "ntfy-low"

app/src/main/java/io/heckel/ntfy/service/SubscriberConnection.kt

@@ -3,6 +3,7 @@ package io.heckel.ntfy.service
 import android.util.Log
 import io.heckel.ntfy.data.ConnectionState
 import io.heckel.ntfy.data.Notification
+import io.heckel.ntfy.data.Repository
 import io.heckel.ntfy.data.Subscription
 import io.heckel.ntfy.msg.ApiService
 import io.heckel.ntfy.util.topicUrl
@@ -11,15 +12,17 @@ import okhttp3.Call
 import java.util.concurrent.atomic.AtomicBoolean
 
 class SubscriberConnection(
+    private val repository: Repository,
     private val api: ApiService,
     private val baseUrl: String,
     private val sinceTime: Long,
-    private val subscriptions: Map<Long, Subscription>,
+    private val topicsToSubscriptionIds: Map<String, Long>, // Topic -> Subscription ID
-    private val stateChangeListener: (Collection<Subscription>, ConnectionState) -> Unit,
+    private val stateChangeListener: (Collection<Long>, ConnectionState) -> Unit,
     private val notificationListener: (Subscription, Notification) -> Unit,
     private val serviceActive: () -> Boolean
 ) {
-    private val topicsStr = subscriptions.values.joinToString(separator = ",") { s -> s.topic }
+    private val subscriptionIds = topicsToSubscriptionIds.values
+    private val topicsStr = topicsToSubscriptionIds.keys.joinToString(separator = ",")
     private val url = topicUrl(baseUrl, topicsStr)
 
     private var since: Long = sinceTime
@@ -28,16 +31,17 @@ class SubscriberConnection(
 
     fun start(scope: CoroutineScope) {
         job = scope.launch(Dispatchers.IO) {
-            Log.d(TAG, "[$url] Starting connection for subscriptions: $subscriptions")
+            Log.d(TAG, "[$url] Starting connection for subscriptions: $topicsToSubscriptionIds")
 
             // Retry-loop: if the connection fails, we retry unless the job or service is cancelled/stopped
             var retryMillis = 0L
             while (isActive && serviceActive()) {
-                Log.d(TAG, "[$url] (Re-)starting connection for subscriptions: $subscriptions")
+                Log.d(TAG, "[$url] (Re-)starting connection for subscriptions: $topicsToSubscriptionIds")
                 val startTime = System.currentTimeMillis()
-                val notify = { topic: String, notification: Notification ->
+                val notify = notify@ { topic: String, notification: Notification ->
                     since = notification.timestamp
-                    val subscription = subscriptions.values.first { it.topic == topic }
+                    val subscriptionId = topicsToSubscriptionIds[topic] ?: return@notify
+                    val subscription = repository.getSubscription(subscriptionId) ?: return@notify
                     val notificationWithSubscriptionId = notification.copy(subscriptionId = subscription.id)
                     notificationListener(subscription, notificationWithSubscriptionId)
                 }
@@ -45,7 +49,7 @@ class SubscriberConnection(
                 val fail = { e: Exception ->
                     failed.set(true)
                     if (isActive && serviceActive()) { // Avoid UI update races if we're restarting a connection
-                        stateChangeListener(subscriptions.values, ConnectionState.CONNECTING)
+                        stateChangeListener(subscriptionIds, ConnectionState.CONNECTING)
                     }
                 }
 
@@ -54,14 +58,14 @@ class SubscriberConnection(
                 try {
                     call = api.subscribe(baseUrl, topicsStr, since, notify, fail)
                     while (!failed.get() && !call.isCanceled() && isActive && serviceActive()) {
-                        stateChangeListener(subscriptions.values, ConnectionState.CONNECTED)
+                        stateChangeListener(subscriptionIds, ConnectionState.CONNECTED)
                         Log.d(TAG,"[$url] Connection is active (failed=$failed, callCanceled=${call.isCanceled()}, jobActive=$isActive, serviceStarted=${serviceActive()}")
                         delay(CONNECTION_LOOP_DELAY_MILLIS) // Resumes immediately if job is cancelled
                     }
                 } catch (e: Exception) {
                     Log.e(TAG, "[$url] Connection failed: ${e.message}", e)
                     if (isActive && serviceActive()) { // Avoid UI update races if we're restarting a connection
-                        stateChangeListener(subscriptions.values, ConnectionState.CONNECTING)
+                        stateChangeListener(subscriptionIds, ConnectionState.CONNECTING)
                     }
                 }
 
@@ -77,10 +81,6 @@ class SubscriberConnection(
         }
     }
 
-    fun matches(otherSubscriptions: Map<Long, Subscription>): Boolean {
-        return subscriptions.keys == otherSubscriptions.keys
-    }
-
     fun since(): Long {
         return since
     }
@@ -91,6 +91,10 @@ class SubscriberConnection(
         if (this::call.isInitialized) call?.cancel()
     }
 
+    fun matches(otherSubscriptionIds: Collection<Long>): Boolean {
+        return subscriptionIds.toSet() == otherSubscriptionIds.toSet()
+    }
+
     private fun nextRetryMillis(retryMillis: Long, startTime: Long): Long {
         val connectionDurationMillis = System.currentTimeMillis() - startTime
         if (connectionDurationMillis > RETRY_RESET_AFTER_MILLIS) {

app/src/main/java/io/heckel/ntfy/service/SubscriberService.kt

@@ -11,8 +11,6 @@ import android.os.SystemClock
 import android.util.Log
 import androidx.core.app.NotificationCompat
 import androidx.core.content.ContextCompat
-import androidx.work.Worker
-import androidx.work.WorkerParameters
 import io.heckel.ntfy.BuildConfig
 import io.heckel.ntfy.R
 import io.heckel.ntfy.app.Application
@@ -140,37 +138,38 @@ class SubscriberService : Service() {
 
     private fun refreshConnections() =
         GlobalScope.launch(Dispatchers.IO) {
-            // Group subscriptions by base URL (Base URL -> Map<SubId -> Sub>.
+            // Group INSTANT subscriptions by base URL, there is only one connection per base URL
-            // There is only one connection per base URL.
+            val instantSubscriptions = repository.getSubscriptions()
-            val subscriptions = repository.getSubscriptions()
                 .filter { s -> s.instant }
-            val subscriptionsByBaseUrl = subscriptions
+            val instantSubscriptionsByBaseUrl = instantSubscriptions // BaseUrl->Map[Topic->SubscriptionId]
                 .groupBy { s -> s.baseUrl }
-                .mapValues { entry -> entry.value.associateBy { it.id } }
+                .mapValues { entry ->
+                    entry.value.associate { subscription -> subscription.topic to subscription.id }
+                }
 
             Log.d(TAG, "Refreshing subscriptions")
-            Log.d(TAG, "- Subscriptions: $subscriptionsByBaseUrl")
+            Log.d(TAG, "- Subscriptions: $instantSubscriptionsByBaseUrl")
             Log.d(TAG, "- Active connections: $connections")
 
             // Start new connections and restart connections (if subscriptions have changed)
-            subscriptionsByBaseUrl.forEach { (baseUrl, subscriptions) ->
+            instantSubscriptionsByBaseUrl.forEach { (baseUrl, subscriptions) ->
                 val connection = connections[baseUrl]
                 var since = 0L
-                if (connection != null && !connection.matches(subscriptions)) {
+                if (connection != null && !connection.matches(subscriptions.values)) {
                     since = connection.since()
                     connections.remove(baseUrl)
                     connection.cancel()
                 }
                 if (!connections.containsKey(baseUrl)) {
                     val serviceActive = { -> isServiceStarted }
-                    val connection = SubscriberConnection(api, baseUrl, since, subscriptions, ::onStateChanged, ::onNotificationReceived, serviceActive)
+                    val connection = SubscriberConnection(repository, api, baseUrl, since, subscriptions, ::onStateChanged, ::onNotificationReceived, serviceActive)
                     connections[baseUrl] = connection
                     connection.start(this)
                 }
             }
 
             // Close connections without subscriptions
-            val baseUrls = subscriptionsByBaseUrl.keys
+            val baseUrls = instantSubscriptionsByBaseUrl.keys
             connections.keys().toList().forEach { baseUrl ->
                 if (!baseUrls.contains(baseUrl)) {
                     val connection = connections.remove(baseUrl)
@@ -182,12 +181,12 @@ class SubscriberService : Service() {
             if (connections.size > 0) {
                 synchronized(this) {
                     val title = getString(R.string.channel_subscriber_notification_title)
-                    val text = when (subscriptions.size) {
+                    val text = when (instantSubscriptions.size) {
                         1 -> getString(R.string.channel_subscriber_notification_text_one)
                         2 -> getString(R.string.channel_subscriber_notification_text_two)
                         3 -> getString(R.string.channel_subscriber_notification_text_three)
                         4 -> getString(R.string.channel_subscriber_notification_text_four)
-                        else -> getString(R.string.channel_subscriber_notification_text_more, subscriptions.size)
+                        else -> getString(R.string.channel_subscriber_notification_text_more, instantSubscriptions.size)
                     }
                     serviceNotification = createNotification(title, text)
                     notificationManager?.notify(NOTIFICATION_SERVICE_ID, serviceNotification)
@@ -195,8 +194,7 @@ class SubscriberService : Service() {
             }
         }
 
-    private fun onStateChanged(subscriptions: Collection<Subscription>, state: ConnectionState) {
+    private fun onStateChanged(subscriptionIds: Collection<Long>, state: ConnectionState) {
-        val subscriptionIds = subscriptions.map { it.id }
         repository.updateState(subscriptionIds, state)
     }
 

app/src/main/java/io/heckel/ntfy/ui/DetailActivity.kt

@@ -111,7 +111,7 @@ class DetailActivity : AppCompatActivity(), ActionMode.Callback, NotificationFra
         val onNotificationClick = { n: Notification -> onNotificationClick(n) }
         val onNotificationLongClick = { n: Notification -> onNotificationLongClick(n) }
 
-        adapter = DetailAdapter(onNotificationClick, onNotificationLongClick)
+        adapter = DetailAdapter(this, onNotificationClick, onNotificationLongClick)
         mainList = findViewById(R.id.detail_notification_list)
         mainList.adapter = adapter
 
@@ -298,6 +298,7 @@ class DetailActivity : AppCompatActivity(), ActionMode.Callback, NotificationFra
 
     override fun onNotificationMutedUntilChanged(mutedUntilTimestamp: Long) {
         lifecycleScope.launch(Dispatchers.IO) {
+            Log.d(TAG, "Setting subscription 'muted until' to $mutedUntilTimestamp")
             val subscription = repository.getSubscription(subscriptionId)
             val newSubscription = subscription?.copy(mutedUntil = mutedUntilTimestamp)
             newSubscription?.let { repository.updateSubscription(newSubscription) }

app/src/main/java/io/heckel/ntfy/ui/DetailAdapter.kt

@@ -1,16 +1,20 @@
 package io.heckel.ntfy.ui
 
+import android.Manifest
+import android.app.Activity
 import android.app.DownloadManager
 import android.content.*
+import android.content.pm.PackageManager
 import android.graphics.Bitmap
 import android.graphics.BitmapFactory
 import android.net.Uri
-import android.provider.OpenableColumns
+import android.os.Build
 import android.util.Log
 import android.view.LayoutInflater
 import android.view.View
 import android.view.ViewGroup
 import android.widget.*
+import androidx.core.app.ActivityCompat
 import androidx.core.content.ContextCompat
 import androidx.recyclerview.widget.DiffUtil
 import androidx.recyclerview.widget.ListAdapter
@@ -20,16 +24,12 @@ import androidx.work.WorkManager
 import androidx.work.workDataOf
 import com.stfalcon.imageviewer.StfalconImageViewer
 import io.heckel.ntfy.R
-import io.heckel.ntfy.data.Attachment
+import io.heckel.ntfy.data.*
-import io.heckel.ntfy.data.Notification
-import io.heckel.ntfy.data.PROGRESS_DONE
-import io.heckel.ntfy.data.PROGRESS_NONE
 import io.heckel.ntfy.msg.AttachmentDownloadWorker
 import io.heckel.ntfy.util.*
 import java.util.*
 
-
+class DetailAdapter(private val activity: Activity, private val onClick: (Notification) -> Unit, private val onLongClick: (Notification) -> Unit) :
-class DetailAdapter(private val onClick: (Notification) -> Unit, private val onLongClick: (Notification) -> Unit) :
     ListAdapter<Notification, DetailAdapter.DetailViewHolder>(TopicDiffCallback) {
     val selected = mutableSetOf<String>() // Notification IDs
 
@@ -37,7 +37,7 @@ class DetailAdapter(private val onClick: (Notification) -> Unit, private val onL
     override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): DetailViewHolder {
         val view = LayoutInflater.from(parent.context)
             .inflate(R.layout.fragment_detail_item, parent, false)
-        return DetailViewHolder(view, selected, onClick, onLongClick)
+        return DetailViewHolder(activity, view, selected, onClick, onLongClick)
     }
 
     /* Gets current topic and uses it to bind view. */
@@ -54,7 +54,7 @@ class DetailAdapter(private val onClick: (Notification) -> Unit, private val onL
     }
 
     /* ViewHolder for Topic, takes in the inflated view and the onClick behavior. */
-    class DetailViewHolder(itemView: View, private val selected: Set<String>, val onClick: (Notification) -> Unit, val onLongClick: (Notification) -> Unit) :
+    class DetailViewHolder(private val activity: Activity, itemView: View, private val selected: Set<String>, val onClick: (Notification) -> Unit, val onLongClick: (Notification) -> Unit) :
         RecyclerView.ViewHolder(itemView) {
         private var notification: Notification? = null
         private val priorityImageView: ImageView = itemView.findViewById(R.id.detail_item_priority_image)
@@ -189,19 +189,27 @@ class DetailAdapter(private val onClick: (Notification) -> Unit, private val onL
             if (attachment.contentUri != null) {
                 openItem.setOnMenuItemClickListener {
                     try {
-                        context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(attachment.contentUri)))
+                        val contentUri = Uri.parse(attachment.contentUri)
+                        val intent = Intent(Intent.ACTION_VIEW, contentUri)
+                        intent.setDataAndType(contentUri, attachment.type ?: "application/octet-stream") // Required for Android <= P
+                        intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
+                        context.startActivity(intent)
                     } catch (e: ActivityNotFoundException) {
                         Toast
-                            .makeText(context, context.getString(R.string.detail_item_cannot_open), Toast.LENGTH_LONG)
+                            .makeText(context, context.getString(R.string.detail_item_cannot_open_not_found), Toast.LENGTH_LONG)
+                            .show()
+                    } catch (e: Exception) {
+                        Toast
+                            .makeText(context, context.getString(R.string.detail_item_cannot_open, e.message), Toast.LENGTH_LONG)
                             .show()
-                    } catch (_: Exception) {
-                        // URI parse exception and others; we don't care!
                     }
                     true
                 }
             }
             browseItem.setOnMenuItemClickListener {
-                context.startActivity(Intent(DownloadManager.ACTION_VIEW_DOWNLOADS))
+                val intent = Intent(DownloadManager.ACTION_VIEW_DOWNLOADS)
+                intent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION)
+                context.startActivity(intent)
                 true
             }
             copyUrlItem.setOnMenuItemClickListener {
@@ -214,6 +222,11 @@ class DetailAdapter(private val onClick: (Notification) -> Unit, private val onL
                 true
             }
             downloadItem.setOnMenuItemClickListener {
+                val requiresPermission = Build.VERSION.SDK_INT <= Build.VERSION_CODES.P && ContextCompat.checkSelfPermission(context, Manifest.permission.WRITE_EXTERNAL_STORAGE) == PackageManager.PERMISSION_DENIED
+                if (requiresPermission) {
+                    ActivityCompat.requestPermissions(activity, arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE), REQUEST_CODE_WRITE_STORAGE_PERMISSION_FOR_DOWNLOAD)
+                    return@setOnMenuItemClickListener true
+                }
                 scheduleAttachmentDownload(context, notification)
                 true
             }
@@ -229,10 +242,11 @@ class DetailAdapter(private val onClick: (Notification) -> Unit, private val onL
         }
 
         private fun formatAttachmentDetails(context: Context, attachment: Attachment, exists: Boolean): String {
-            val name = queryAttachmentFilename(context, attachment)
+            val name = queryFilename(context, attachment.contentUri, attachment.name)
             val notYetDownloaded = !exists && attachment.progress == PROGRESS_NONE
             val downloading = !exists && attachment.progress in 0..99
             val deleted = !exists && attachment.progress == PROGRESS_DONE
+            val failed = !exists && attachment.progress == PROGRESS_FAILED
             val expired = attachment.expires != null && attachment.expires < System.currentTimeMillis()/1000
             val expires = attachment.expires != null && attachment.expires > System.currentTimeMillis()/1000
             val infos = mutableListOf<String>()
@@ -241,22 +255,24 @@ class DetailAdapter(private val onClick: (Notification) -> Unit, private val onL
             }
             if (notYetDownloaded) {
                 if (expired) {
-                    infos.add("not downloaded, link expired")
+                    infos.add(context.getString(R.string.detail_item_download_info_not_downloaded_expired))
                 } else if (expires) {
-                    infos.add("not downloaded, expires ${formatDateShort(attachment.expires!!)}")
+                    infos.add(context.getString(R.string.detail_item_download_info_not_downloaded_expires_x, formatDateShort(attachment.expires!!)))
                 } else {
-                    infos.add("not downloaded")
+                    infos.add(context.getString(R.string.detail_item_download_info_not_downloaded))
                 }
             } else if (downloading) {
-                infos.add("${attachment.progress}% downloaded")
+                infos.add(context.getString(R.string.detail_item_download_info_downloading_x_percent, attachment.progress))
             } else if (deleted) {
                 if (expired) {
-                    infos.add("deleted, link expired")
+                    infos.add(context.getString(R.string.detail_item_download_info_deleted_expired))
                 } else if (expires) {
-                    infos.add("deleted, link expires ${formatDateShort(attachment.expires!!)}")
+                    infos.add(context.getString(R.string.detail_item_download_info_deleted_expires_x, formatDateShort(attachment.expires!!)))
                 } else {
-                    infos.add("deleted")
+                    infos.add(context.getString(R.string.detail_item_download_info_deleted))
                 }
+            } else if (failed) {
+                infos.add(context.getString(R.string.detail_item_download_info_download_failed))
             }
             return if (infos.size > 0) {
                 "$name\n${infos.joinToString(", ")}"
@@ -265,23 +281,6 @@ class DetailAdapter(private val onClick: (Notification) -> Unit, private val onL
             }
         }
 
-        private fun queryAttachmentFilename(context: Context, attachment: Attachment): String {
-            if (attachment.contentUri == null) {
-                return attachment.name
-            }
-            try {
-                val resolver = context.applicationContext.contentResolver
-                val cursor = resolver.query(Uri.parse(attachment.contentUri), null, null, null, null) ?: return attachment.name
-                return cursor.use { c ->
-                    val nameIndex = c.getColumnIndexOrThrow(OpenableColumns.DISPLAY_NAME)
-                    c.moveToFirst()
-                    c.getString(nameIndex)
-                }
-            } catch (_: Exception) {
-                return attachment.name
-            }
-        }
-
         private fun maybeRenderAttachmentImage(context: Context, attachment: Attachment, image: Boolean) {
             if (!image) {
                 attachmentImageView.visibility = View.GONE
@@ -328,5 +327,6 @@ class DetailAdapter(private val onClick: (Notification) -> Unit, private val onL
 
     companion object {
         const val TAG = "NtfyDetailAdapter"
+        const val REQUEST_CODE_WRITE_STORAGE_PERMISSION_FOR_DOWNLOAD = 9876
     }
 }

app/src/main/java/io/heckel/ntfy/ui/MainActivity.kt

@@ -117,44 +117,8 @@ class MainActivity : AppCompatActivity(), ActionMode.Callback, AddFragment.Subsc
         // Background things
         startPeriodicPollWorker()
         startPeriodicServiceRestartWorker()
+    }
 
-        /*if (ContextCompat.checkSelfPermission(this, Manifest.permission.WRITE_EXTERNAL_STORAGE) == PackageManager.PERMISSION_DENIED) {
-            ActivityCompat.requestPermissions(this, arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE), 1234);
-        }
-        else {
-            Toast.makeText(this, "Permission already granted", Toast.LENGTH_SHORT).show();
-        }*/
-    }
-    override fun onRequestPermissionsResult(requestCode: Int,
-                                            permissions: Array<String>,
-                                            grantResults: IntArray) {
-        super.onRequestPermissionsResult(requestCode, permissions, grantResults)
-        if (requestCode == 1234) { // FIXME
-            if (grantResults.isNotEmpty() && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
-                Toast.makeText(this@MainActivity, "Camera Permission Granted", Toast.LENGTH_SHORT).show()
-            } else {
-                Toast.makeText(this@MainActivity, "Camera Permission Denied", Toast.LENGTH_SHORT).show()
-            }
-        }
-    }
-/*
-    public static final int REQUEST_WRITE_STORAGE = 112;
-
-    fun requestPermission(Activity context) {
-        boolean hasPermission = (ContextCompat.checkSelfPermission(context, Manifest.permission.WRITE_EXTERNAL_STORAGE) == PackageManager.PERMISSION_GRANTED);
-        if (!hasPermission) {
-            ActivityCompat.requestPermissions(context,
-                new String[]{Manifest.permission.WRITE_EXTERNAL_STORAGE},
-                REQUEST_WRITE_STORAGE);
-        } else {
-            // You are allowed to write external storage:
-            String path = Environment.getExternalStorageDirectory().getAbsolutePath() + "/new_folder";
-            File storageDir = new File(path);
-            if (!storageDir.exists() && !storageDir.mkdirs()) {
-                // This should never happen - log handled exception!
-            }
-        }
-*/
     override fun onResume() {
         super.onResume()
         showHideNotificationMenuItems()

app/src/main/java/io/heckel/ntfy/ui/SettingsActivity.kt

@@ -1,13 +1,18 @@
 package io.heckel.ntfy.ui
 
+import android.Manifest
 import android.content.ClipData
 import android.content.ClipboardManager
 import android.content.Context
+import android.content.pm.PackageManager
+import android.os.Build
 import android.os.Bundle
 import android.text.TextUtils
 import android.util.Log
 import android.widget.Toast
 import androidx.appcompat.app.AppCompatActivity
+import androidx.core.app.ActivityCompat
+import androidx.core.content.ContextCompat
 import androidx.fragment.app.FragmentManager
 import androidx.preference.*
 import androidx.preference.Preference.OnPreferenceClickListener
@@ -20,6 +25,7 @@ import io.heckel.ntfy.util.toPriorityString
 
 class SettingsActivity : AppCompatActivity() {
     private val repository by lazy { (application as Application).repository }
+    private lateinit var fragment: SettingsFragment
 
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
@@ -28,9 +34,10 @@ class SettingsActivity : AppCompatActivity() {
         Log.d(TAG, "Create $this")
 
         if (savedInstanceState == null) {
+            fragment = SettingsFragment(repository, supportFragmentManager)
             supportFragmentManager
                 .beginTransaction()
-                .replace(R.id.settings_layout, SettingsFragment(repository, supportFragmentManager))
+                .replace(R.id.settings_layout, fragment)
                 .commit()
         }
 
@@ -125,6 +132,16 @@ class SettingsActivity : AppCompatActivity() {
                     getString(R.string.settings_notifications_auto_download_summary_off)
                 }
             }
+            if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.P) {
+                autoDownload?.setOnPreferenceChangeListener { _, v ->
+                    if (ContextCompat.checkSelfPermission(requireContext(), Manifest.permission.WRITE_EXTERNAL_STORAGE) == PackageManager.PERMISSION_DENIED) {
+                        ActivityCompat.requestPermissions(requireActivity(), arrayOf(Manifest.permission.WRITE_EXTERNAL_STORAGE), REQUEST_CODE_WRITE_EXTERNAL_STORAGE_PERMISSION_FOR_AUTO_DOWNLOAD)
+                        false // If permission is granted, auto-download will be enabled in onRequestPermissionsResult()
+                    } else {
+                        true
+                    }
+                }
+            }
 
             // Broadcast enabled
             val broadcastEnabledPrefId = context?.getString(R.string.settings_advanced_broadcast_key) ?: return
@@ -204,9 +221,31 @@ class SettingsActivity : AppCompatActivity() {
                 true
             }
         }
+
+        fun enableAutoDownload() {
+            val autoDownloadPrefId = context?.getString(R.string.settings_notifications_auto_download_key) ?: return
+            val autoDownload: SwitchPreference? = findPreference(autoDownloadPrefId)
+            autoDownload?.isChecked = true
+        }
+    }
+
+    override fun onRequestPermissionsResult(requestCode: Int, permissions: Array<String>, grantResults: IntArray) {
+        super.onRequestPermissionsResult(requestCode, permissions, grantResults)
+        if (requestCode == REQUEST_CODE_WRITE_EXTERNAL_STORAGE_PERMISSION_FOR_AUTO_DOWNLOAD) {
+            if (grantResults.isNotEmpty() && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
+                enableAutoDownload()
+                repository.setAutoDownloadEnabled(true)
+            }
+        }
+    }
+
+    private fun enableAutoDownload() {
+        if (!this::fragment.isInitialized) return
+        fragment.enableAutoDownload()
     }
 
     companion object {
-        const val TAG = "NtfySettingsActivity"
+        private const val TAG = "NtfySettingsActivity"
+        private const val REQUEST_CODE_WRITE_EXTERNAL_STORAGE_PERMISSION_FOR_AUTO_DOWNLOAD = 2586
     }
 }

app/src/main/java/io/heckel/ntfy/util/Util.kt

@@ -4,6 +4,7 @@ import android.animation.ArgbEvaluator
 import android.animation.ValueAnimator
 import android.content.Context
 import android.net.Uri
+import android.provider.OpenableColumns
 import android.view.Window
 import io.heckel.ntfy.data.Notification
 import io.heckel.ntfy.data.PROGRESS_NONE
@@ -110,7 +111,8 @@ fun formatTitle(notification: Notification): String {
 }
 
 // Checks in the most horrible way if a content URI exists; I couldn't find a better way
-fun fileExists(context: Context, uri: String): Boolean {
+fun fileExists(context: Context, uri: String?): Boolean {
+    if (uri == null) return false
     val resolver = context.applicationContext.contentResolver
     return try {
         val fileIS = resolver.openInputStream(Uri.parse(uri))
@@ -121,6 +123,24 @@ fun fileExists(context: Context, uri: String): Boolean {
     }
 }
 
+// Queries the filename of a content URI
+fun queryFilename(context: Context, contentUri: String?, fallbackName: String): String {
+    if (contentUri == null) {
+        return fallbackName
+    }
+    try {
+        val resolver = context.applicationContext.contentResolver
+        val cursor = resolver.query(Uri.parse(contentUri), null, null, null, null) ?: return fallbackName
+        return cursor.use { c ->
+            val nameIndex = c.getColumnIndexOrThrow(OpenableColumns.DISPLAY_NAME)
+            c.moveToFirst()
+            c.getString(nameIndex)
+        }
+    } catch (_: Exception) {
+        return fallbackName
+    }
+}
+
 // Status bar color fading to match action bar, see https://stackoverflow.com/q/51150077/1440785
 fun fadeStatusBarColor(window: Window, fromColor: Int, toColor: Int) {
     val statusBarColorAnimation = ValueAnimator.ofObject(ArgbEvaluator(), fromColor, toColor)

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

@@ -115,7 +115,17 @@
     <string name="detail_item_menu_copy_url">Copy URL</string>
     <string name="detail_item_menu_copy_url_copied">Copied URL to clipboard</string>
     <string name="detail_item_cannot_download">Cannot open or download attachment. Link expired and no local file found.</string>
-    <string name="detail_item_cannot_open">Cannot open attachment: File may have been deleted, or there is no app to open the file.</string>
+    <string name="detail_item_cannot_open">Cannot open attachment: %1$s</string>
+    <string name="detail_item_cannot_open_not_found">Cannot open attachment: File may have been deleted, or there is no app to open the file.</string>
+    <string name="detail_item_download_failed">Attachment download failed: %1$s</string>
+    <string name="detail_item_download_info_not_downloaded">not downloaded</string>
+    <string name="detail_item_download_info_not_downloaded_expired">not downloaded, link expired</string>
+    <string name="detail_item_download_info_not_downloaded_expires_x">not downloaded, expires %1$s</string>
+    <string name="detail_item_download_info_downloading_x_percent">%1$d%% downloaded</string>
+    <string name="detail_item_download_info_deleted">deleted</string>
+    <string name="detail_item_download_info_deleted_expired">deleted, link expired</string>
+    <string name="detail_item_download_info_deleted_expires_x">deleted, link expires %1$s</string>
+    <string name="detail_item_download_info_download_failed">download failed</string>
 
     <!-- Detail activity: Action bar -->
     <string name="detail_menu_notifications_enabled">Notifications enabled</string>
@@ -155,6 +165,11 @@
     <!-- Notification popup -->
     <string name="notification_popup_action_open">Open</string>
     <string name="notification_popup_action_browse">Browse</string>
+    <string name="notification_popup_action_download">Download</string>
+    <string name="notification_popup_file">%1$s\nFile: %2$s</string>
+    <string name="notification_popup_file_downloading">Downloading %1$s, %2$d%%\n%3$s</string>
+    <string name="notification_popup_file_download_successful">%1$s\nFile: %2$s, download successful</string>
+    <string name="notification_popup_file_download_failed">%1$s\nFile: %2$s, download failed</string>
 
     <!-- Settings -->
     <string name="settings_title">Settings</string>

app/src/main/res/xml/file_paths.xml

@@ -0,0 +1,4 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<paths xmlns:android="http://schemas.android.com/apk/res/android">
+    <external-path name="external_files" path="."/>
+</paths>