-
Notifications
You must be signed in to change notification settings - Fork 743
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
Fallback keys implementation #3970
Merged
Merged
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
4ac90f1
Fallback keys implementation.
bmarty c603135
Code review fixes.
10671a5
Quick refactor to use same mechanism as updateOneTimeKeyCount
BillCarsonFr f843ddd
Cleaning
BillCarsonFr 5d35f02
Support using unpublished fallback key instead of generating
BillCarsonFr 01b8b7d
Code review
BillCarsonFr a026137
code review
BillCarsonFr 38a8e8b
Update matrix-sdk-android/src/main/java/org/matrix/android/sdk/intern…
BillCarsonFr 76960f8
Update matrix-sdk-android/src/main/java/org/matrix/android/sdk/intern…
BillCarsonFr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,9 @@ import javax.inject.Inject | |
import kotlin.math.floor | ||
import kotlin.math.min | ||
|
||
const val FIVE_MINUTES = 5 * 60_000L | ||
// THe spec recommend a 5mn delay, but due to federation | ||
BillCarsonFr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// or server down we give it a bit more time (1 hour) | ||
BillCarsonFr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const val FALLBACK_KEY_FORGET_DELAY = 60 * 60_000L | ||
|
||
@SessionScope | ||
internal class OneTimeKeysUploader @Inject constructor( | ||
|
@@ -59,8 +61,13 @@ internal class OneTimeKeysUploader @Inject constructor( | |
oneTimeKeyCount = currentCount | ||
} | ||
|
||
fun setNeedsNewFallback() { | ||
needNewFallbackKey = true | ||
fun needsNewFallback() { | ||
if (olmDevice.generateFallbackKeyIfNeeded()) { | ||
// As we generated a new one, it's already forgetting one | ||
// so we can clear the last publish time | ||
// (in case the network calls fails after to avoid calling forgetKey) | ||
saveLastFallbackKeyPublishTime(0L) | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -120,9 +127,9 @@ internal class OneTimeKeysUploader @Inject constructor( | |
|
||
// Check if we need to forget a fallback key | ||
val latestPublishedTime = getLastFallbackKeyPublishTime() | ||
if (latestPublishedTime != 0L && System.currentTimeMillis() - latestPublishedTime > FIVE_MINUTES) { | ||
if (latestPublishedTime != 0L && System.currentTimeMillis() - latestPublishedTime > FALLBACK_KEY_FORGET_DELAY) { | ||
// This should be called once you are reasonably certain that you will not receive any more messages | ||
// that use the old fallback key (e.g. 5 minutes after the new fallback key has been published) | ||
// that use the old fallback key | ||
Timber.d("## forgetFallbackKey()") | ||
olmDevice.forgetFallbackKey() | ||
} | ||
|
@@ -155,20 +162,9 @@ internal class OneTimeKeysUploader @Inject constructor( | |
keysThisLoop = min(keyLimit - keyCount, ONE_TIME_KEY_GENERATION_MAX_NUMBER) | ||
olmDevice.generateOneTimeKeys(keysThisLoop) | ||
} | ||
if (needNewFallbackKey && !hasUnpublishedFallbackKey()) { | ||
// if there is already fallback key, but that hasn't been published yet, we | ||
// can use that instead of generating a new one | ||
olmDevice.generateFallbackKey() | ||
Timber.d("maybeUploadOneTimeKeys: Fallback key generated") | ||
// As we generated a new one, it's already forgetting one | ||
// so we can clear the last publish time | ||
// (in case the network calls fails after to avoid calling forgetKey) | ||
saveLastFallbackKeyPublishTime(0L) | ||
} | ||
|
||
// not copy paste error we check before sending if there is | ||
// an unpublished key in order to saveLastFallbackKeyPublishTime if needed | ||
val hadUnpublishedFallbackKey = hasUnpublishedFallbackKey() | ||
// We check before sending if there is an unpublished key in order to saveLastFallbackKeyPublishTime if needed | ||
val hadUnpublishedFallbackKey = olmDevice.hasUnpublishedFallbackKey() | ||
val response = uploadOneTimeKeys(olmDevice.getOneTimeKeys()) | ||
olmDevice.markKeysAsPublished() | ||
if (hadUnpublishedFallbackKey) { | ||
|
@@ -189,10 +185,6 @@ internal class OneTimeKeysUploader @Inject constructor( | |
} | ||
} | ||
|
||
private fun hasUnpublishedFallbackKey(): Boolean { | ||
return olmDevice.getFallbackKey()?.get(OlmAccount.JSON_KEY_ONE_TIME_KEY).orEmpty().isNotEmpty() | ||
} | ||
|
||
private fun saveLastFallbackKeyPublishTime(timeMillis: Long) { | ||
storage.edit().putLong("last_fb_key_publish", timeMillis).apply() | ||
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. Nit: use |
||
} | ||
|
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
orEmpty().isNotEmpty()
is a bit weird :/