From eb1c8300ab123d0673a03213ef8da6aa58d16601 Mon Sep 17 00:00:00 2001 From: Karmanyaah Malhotra Date: Sat, 16 Dec 2023 22:45:26 -0600 Subject: [PATCH] make things slightly nicer? --- .../java/io/heckel/ntfy/msg/ApiService.kt | 3 +- .../io/heckel/ntfy/msg/NotificationParser.kt | 12 +++---- .../io/heckel/ntfy/service/JsonConnection.kt | 2 +- .../heckel/ntfy/service/SubscriberService.kt | 31 +++++++++---------- .../io/heckel/ntfy/service/WsConnection.kt | 4 +-- .../io/heckel/ntfy/up/BroadcastReceiver.kt | 17 ++++++---- 6 files changed, 33 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/io/heckel/ntfy/msg/ApiService.kt b/app/src/main/java/io/heckel/ntfy/msg/ApiService.kt index 708ca71..25385d6 100644 --- a/app/src/main/java/io/heckel/ntfy/msg/ApiService.kt +++ b/app/src/main/java/io/heckel/ntfy/msg/ApiService.kt @@ -11,7 +11,6 @@ import java.io.IOException import java.net.URLEncoder import java.nio.charset.StandardCharsets.UTF_8 import java.util.concurrent.TimeUnit -import kotlin.random.Random class ApiService { private val client = OkHttpClient.Builder() @@ -97,7 +96,7 @@ class ApiService { val body = response.body?.string()?.trim() if (body.isNullOrEmpty()) return emptyList() val notifications = body.lines().mapNotNull { line -> - parser.parse(line, subscriptionId = subscriptionId, notificationId = 0) // No notification when we poll + parser.parseNotification(line, subscriptionId = subscriptionId, notificationId = 0) // No notification when we poll } Log.d(TAG, "Notifications: $notifications") diff --git a/app/src/main/java/io/heckel/ntfy/msg/NotificationParser.kt b/app/src/main/java/io/heckel/ntfy/msg/NotificationParser.kt index be7d07a..84a4097 100644 --- a/app/src/main/java/io/heckel/ntfy/msg/NotificationParser.kt +++ b/app/src/main/java/io/heckel/ntfy/msg/NotificationParser.kt @@ -6,7 +6,6 @@ import io.heckel.ntfy.db.Action import io.heckel.ntfy.db.Attachment import io.heckel.ntfy.db.Icon import io.heckel.ntfy.db.Notification -import io.heckel.ntfy.util.Log import io.heckel.ntfy.util.joinTags import io.heckel.ntfy.util.toPriority import java.lang.reflect.Type @@ -15,19 +14,16 @@ class NotificationParser { private val gson = Gson() fun parseMessage(s: String) : Message? { - - val message= gson.fromJson(s, Message::class.java) - Log.d("HITAGME", message.toString()) - return message + return gson.fromJson(s, Message::class.java) } - fun parse(s: String, subscriptionId: Long = 0, notificationId: Int = 0): Notification? { + fun parseNotification(s: String, subscriptionId: Long = 0, notificationId: Int = 0): Notification? { val message = parseMessage(s) ?: return null - val notificationWithTopic = parseWithTopic(message, subscriptionId = subscriptionId, notificationId = notificationId) + val notificationWithTopic = parseNotificationWithTopic(message, subscriptionId = subscriptionId, notificationId = notificationId) return notificationWithTopic?.notification } - fun parseWithTopic(message: Message, subscriptionId: Long = 0, notificationId: Int = 0): NotificationWithTopic? { + fun parseNotificationWithTopic(message: Message, subscriptionId: Long = 0, notificationId: Int = 0): NotificationWithTopic? { if (message.event != ApiService.EVENT_MESSAGE) { return null } diff --git a/app/src/main/java/io/heckel/ntfy/service/JsonConnection.kt b/app/src/main/java/io/heckel/ntfy/service/JsonConnection.kt index cccab41..4d3a951 100644 --- a/app/src/main/java/io/heckel/ntfy/service/JsonConnection.kt +++ b/app/src/main/java/io/heckel/ntfy/service/JsonConnection.kt @@ -43,7 +43,7 @@ class JsonConnection( while (isActive && serviceActive()) { Log.d(TAG, "[$url] (Re-)starting connection for subscriptions: $topicsToSubscriptionIds") val startTime = System.currentTimeMillis() - val notify = notify@ { message : Message -> + val notify = { message : Message -> since = notificationListener(ConnectionId(baseUrl, topicsToSubscriptionIds, topicIsUnifiedPush), message)?: since } val failed = AtomicBoolean(false) diff --git a/app/src/main/java/io/heckel/ntfy/service/SubscriberService.kt b/app/src/main/java/io/heckel/ntfy/service/SubscriberService.kt index 3d711f8..f68f38f 100644 --- a/app/src/main/java/io/heckel/ntfy/service/SubscriberService.kt +++ b/app/src/main/java/io/heckel/ntfy/service/SubscriberService.kt @@ -15,7 +15,6 @@ import io.heckel.ntfy.R import io.heckel.ntfy.app.Application import io.heckel.ntfy.db.ConnectionState import io.heckel.ntfy.db.Repository -import io.heckel.ntfy.db.Subscription import io.heckel.ntfy.msg.ApiService import io.heckel.ntfy.msg.Message import io.heckel.ntfy.msg.NotificationDispatcher @@ -252,18 +251,18 @@ class SubscriberService : Service() { } private fun onConnectionOpen(connectionId: ConnectionId, message: String?) { - Log.d(TAG, "Received open from connection to ${connectionId.baseUrl} with message: $message") - // TODO extract constant + Log.d(TAG, "Received open from connection ${connectionId.baseUrl} with message: $message") + // this check is sufficient for now, and the message can be upgraded to include other parameters in the future if (message?.contains(ApiService.EVENT_OPEN_PARAM_NEW_TOPIC) == true) { - val connectionId = connectionId.copy() // TODO does this deep copy GlobalScope.launch(Dispatchers.IO) { - for (topic in connectionId.topicsToSubscriptionIds.keys) - if (connectionId.topicIsUnifiedPush[topic] == true){ - io.heckel.ntfy.up.BroadcastReceiver.sendRegistration(baseContext, connectionId.baseUrl, topic) + for (topic in connectionId.topicsToSubscriptionIds.keys) { + if (connectionId.topicIsUnifiedPush[topic] == true) { + io.heckel.ntfy.up.BroadcastReceiver.sendRegistration(baseContext, connectionId.baseUrl, topic) + // TODO is that the right context + // looks like it works??? Log.d(TAG, "Attempting to re-register ${connectionId.baseUrl}/$topic") } - // TODO is that the right context - // looks like it works??? + } } } } @@ -271,21 +270,19 @@ class SubscriberService : Service() { repository.updateState(subscriptionIds, state) } - // return successfully processed ID, else null + // Process messages received from the server, and dispatch a notification if required. + // Return the ID of the notification if successfully processed, else null. private fun onNotificationReceived(connectionId: ConnectionId, message: Message) : String? { if (message.event == ApiService.EVENT_OPEN) { onConnectionOpen(connectionId, message.message) return null } - val notificationWithTopic = parser.parseWithTopic(message, notificationId = Random.nextInt(), subscriptionId = 0 - ) ?: return null// subscriptionId to be set downstream - val (topic, notificationWoId) = notificationWithTopic + val (topic, notificationWithoutId) = parser.parseNotificationWithTopic(message, notificationId = Random.nextInt(), subscriptionId = 0) + ?: return null // subscriptionId to be set downstream val subscriptionId = connectionId.topicsToSubscriptionIds[topic] ?: return null - val subscription = - repository.getSubscription(subscriptionId) ?: return null - val notification = - notificationWoId.copy(subscriptionId = subscription.id) + val subscription = repository.getSubscription(subscriptionId) ?: return null + val notification = notificationWithoutId.copy(subscriptionId = subscription.id) // Wakelock while notifications are being dispatched // Wakelocks are reference counted by default so that should work neatly here diff --git a/app/src/main/java/io/heckel/ntfy/service/WsConnection.kt b/app/src/main/java/io/heckel/ntfy/service/WsConnection.kt index 179f4f9..9827c6c 100644 --- a/app/src/main/java/io/heckel/ntfy/service/WsConnection.kt +++ b/app/src/main/java/io/heckel/ntfy/service/WsConnection.kt @@ -61,8 +61,8 @@ class WsConnection( private val topicIsUnifiedPush = connectionId.topicIsUnifiedPush private val subscriptionIds = topicsToSubscriptionIds.values private val topicsStr = topicsToSubscriptionIds.keys.joinToString(separator = ",") - private val unifiedPushTopicsStr = - topicIsUnifiedPush.filter { entry -> entry.value }.keys.joinToString(separator = ",") + private val unifiedPushTopicsStr = topicIsUnifiedPush.filter { entry -> entry.value +}.keys.joinToString(separator = ",") private val shortUrl = topicShortUrl(baseUrl, topicsStr) init { diff --git a/app/src/main/java/io/heckel/ntfy/up/BroadcastReceiver.kt b/app/src/main/java/io/heckel/ntfy/up/BroadcastReceiver.kt index 359b757..34c8fc7 100644 --- a/app/src/main/java/io/heckel/ntfy/up/BroadcastReceiver.kt +++ b/app/src/main/java/io/heckel/ntfy/up/BroadcastReceiver.kt @@ -93,10 +93,13 @@ class BroadcastReceiver : android.content.BroadcastReceiver() { try { // Note, this may fail due to a SQL constraint exception, see https://github.com/binwiederhier/ntfy/issues/185 repository.addSubscription(subscription) - //distributor.sendEndpoint(appId, connectorToken, endpoint) -//lets wait to subscribe, THEN, send + /* We don't send the endpoint here anymore, the foreground service will do that after + registering with the push server. This avoids a race condition where the application server + is rejected before ntfy even establishes that this topic exists. + This is fine from an application perspective, because other distributors can't even register + without a connection to the push server. */ - // Refresh (and maybe start) foreground service + //Refresh (and maybe start) foreground service SubscriberServiceManager.refresh(app) } catch (e: Exception) { Log.w(TAG, "Failed to add subscription", e) @@ -144,7 +147,11 @@ class BroadcastReceiver : android.content.BroadcastReceiver() { private const val TOPIC_RANDOM_ID_LENGTH = 12 val mutex = Mutex() // https://github.com/binwiederhier/ntfy/issues/230 - public fun sendRegistration(context: Context, baseUrl : String, topic : String) : Boolean { + + // TODO Where's the best place to put this function? This seems to be the only place + // with the access to the locks, but also globally accessible + // but also, broadcast receiver is for *receiving Android broadcasts* + public fun sendRegistration(context: Context, baseUrl : String, topic : String) { val app = context.applicationContext as Application val repository = app.repository val distributor = Distributor(app) @@ -161,8 +168,6 @@ class BroadcastReceiver : android.content.BroadcastReceiver() { distributor.sendEndpoint(appId, connectorToken, endpoint) } } - return false // todo return result async somehow } - } }