-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
When subscribed to a new_topic, send endpoints to all apps #70
base: main
Are you sure you want to change the base?
Changes from 6 commits
a09374b
9d38118
eb1c830
235f354
c7d0fb1
b341956
61f57dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -93,7 +93,12 @@ 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) | ||
/* 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. | ||
Unless the app sends registration twice. Then it'll get the endpoint.*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ^ Pointing this out, hopefully is fine There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, assuming the Android app is pushed out before the server changes, I'll have to leave the endpoint sending in for now. Otherwise, the new Android app will break for old servers. |
||
|
||
// Refresh (and maybe start) foreground service | ||
SubscriberServiceManager.refresh(app) | ||
|
@@ -143,5 +148,26 @@ class BroadcastReceiver : android.content.BroadcastReceiver() { | |
private const val TOPIC_RANDOM_ID_LENGTH = 12 | ||
|
||
val mutex = Mutex() // https://github.com/binwiederhier/ntfy/issues/230 | ||
|
||
// 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) | ||
GlobalScope.launch(Dispatchers.IO) { | ||
// We're doing all of this inside a critical section, because of possible races. | ||
// See https://github.com/binwiederhier/ntfy/issues/230 for details. | ||
|
||
mutex.withLock { | ||
val existingSubscription = repository.getSubscription(baseUrl, topic) ?: return@launch | ||
val appId = existingSubscription.upAppId ?: return@launch | ||
val connectorToken = existingSubscription.upConnectorToken ?: return@launch | ||
val endpoint = topicUrlUp(existingSubscription.baseUrl, existingSubscription.topic) | ||
distributor.sendEndpoint(appId, connectorToken, endpoint) | ||
Comment on lines
+153
to
+170
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where should I put this function? |
||
} | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should that work? Or do I need to make a whole chain of contexts being passed down and stored? Not really sure what context is used for when creating the repository downstream. It seems to work as is.