mirror of
https://github.com/signalapp/Signal-Android.git
synced 2025-12-22 20:18:36 +00:00
Do not upload long text attachments to the archive.
This commit is contained in:
@@ -44,7 +44,6 @@ import java.util.UUID
|
|||||||
import kotlin.random.Random
|
import kotlin.random.Random
|
||||||
import kotlin.time.Duration
|
import kotlin.time.Duration
|
||||||
import kotlin.time.Duration.Companion.days
|
import kotlin.time.Duration.Companion.days
|
||||||
import kotlin.time.Duration.Companion.hours
|
|
||||||
import kotlin.time.Duration.Companion.milliseconds
|
import kotlin.time.Duration.Companion.milliseconds
|
||||||
import kotlin.time.Duration.Companion.minutes
|
import kotlin.time.Duration.Companion.minutes
|
||||||
import kotlin.time.Duration.Companion.seconds
|
import kotlin.time.Duration.Companion.seconds
|
||||||
@@ -286,25 +285,6 @@ class AttachmentTableTest {
|
|||||||
assertThat(attachments).isNotEmpty()
|
assertThat(attachments).isNotEmpty()
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
|
||||||
fun givenAnAttachmentWithAMessageWithExpirationStartedThatExpiresIn5Minutes_whenIGetAttachmentsThatNeedArchiveUpload_thenIDoNotExpectThatAttachment() {
|
|
||||||
// GIVEN
|
|
||||||
val uncompressData = byteArrayOf(1, 2, 3, 4, 5)
|
|
||||||
val blobUncompressed = BlobProvider.getInstance().forData(uncompressData).createForSingleSessionInMemory()
|
|
||||||
val attachment = createAttachment(1, blobUncompressed, AttachmentTable.TransformProperties.empty())
|
|
||||||
val message = createIncomingMessage(serverTime = 0.days, attachment = attachment, expiresIn = 5.days)
|
|
||||||
val messageId = SignalDatabase.messages.insertMessageInbox(message).map { it.messageId }.get()
|
|
||||||
SignalDatabase.messages.markExpireStarted(messageId, startedTimestamp = System.currentTimeMillis() - (4.days + 12.hours).inWholeMilliseconds)
|
|
||||||
SignalDatabase.attachments.setArchiveTransferState(AttachmentId(1L), AttachmentTable.ArchiveTransferState.NONE)
|
|
||||||
SignalDatabase.attachments.setTransferState(messageId, AttachmentId(1L), AttachmentTable.TRANSFER_PROGRESS_DONE)
|
|
||||||
|
|
||||||
// WHEN
|
|
||||||
val attachments = SignalDatabase.attachments.getAttachmentsThatNeedArchiveUpload()
|
|
||||||
|
|
||||||
// THEN
|
|
||||||
assertThat(attachments).isEmpty()
|
|
||||||
}
|
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun givenAnAttachmentWithAMessageWithExpirationStartedThatExpiresIn5Days_whenIGetAttachmentsThatNeedArchiveUpload_thenIDoExpectThatAttachment() {
|
fun givenAnAttachmentWithAMessageWithExpirationStartedThatExpiresIn5Days_whenIGetAttachmentsThatNeedArchiveUpload_thenIDoExpectThatAttachment() {
|
||||||
// GIVEN
|
// GIVEN
|
||||||
@@ -324,6 +304,24 @@ class AttachmentTableTest {
|
|||||||
assertThat(attachments).isNotEmpty()
|
assertThat(attachments).isNotEmpty()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
fun givenAnAttachmentWithALongTextAttachment_whenIGetAttachmentsThatNeedArchiveUpload_thenIDoNotExpectThatAttachment() {
|
||||||
|
// GIVEN
|
||||||
|
val uncompressData = byteArrayOf(1, 2, 3, 4, 5)
|
||||||
|
val blobUncompressed = BlobProvider.getInstance().forData(uncompressData).createForSingleSessionInMemory()
|
||||||
|
val attachment = createAttachment(1, blobUncompressed, AttachmentTable.TransformProperties.empty(), contentType = MediaUtil.LONG_TEXT)
|
||||||
|
val message = createIncomingMessage(serverTime = 0.days, attachment = attachment)
|
||||||
|
val messageId = SignalDatabase.messages.insertMessageInbox(message).map { it.messageId }.get()
|
||||||
|
SignalDatabase.attachments.setArchiveTransferState(AttachmentId(1L), AttachmentTable.ArchiveTransferState.NONE)
|
||||||
|
SignalDatabase.attachments.setTransferState(messageId, AttachmentId(1L), AttachmentTable.TRANSFER_PROGRESS_DONE)
|
||||||
|
|
||||||
|
// WHEN
|
||||||
|
val attachments = SignalDatabase.attachments.getAttachmentsThatNeedArchiveUpload()
|
||||||
|
|
||||||
|
// THEN
|
||||||
|
assertThat(attachments).isEmpty()
|
||||||
|
}
|
||||||
|
|
||||||
private fun createIncomingMessage(
|
private fun createIncomingMessage(
|
||||||
serverTime: Duration,
|
serverTime: Duration,
|
||||||
attachment: Attachment,
|
attachment: Attachment,
|
||||||
@@ -395,11 +393,11 @@ class AttachmentTableTest {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun createAttachment(id: Long, uri: Uri, transformProperties: AttachmentTable.TransformProperties): UriAttachment {
|
private fun createAttachment(id: Long, uri: Uri, transformProperties: AttachmentTable.TransformProperties, contentType: String = MediaUtil.IMAGE_JPEG): UriAttachment {
|
||||||
return UriAttachmentBuilder.build(
|
return UriAttachmentBuilder.build(
|
||||||
id,
|
id,
|
||||||
uri = uri,
|
uri = uri,
|
||||||
contentType = MediaUtil.IMAGE_JPEG,
|
contentType = contentType,
|
||||||
transformProperties = transformProperties
|
transformProperties = transformProperties
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -836,16 +836,20 @@ object BackupRepository {
|
|||||||
var frameCount = 0L
|
var frameCount = 0L
|
||||||
|
|
||||||
writer.use {
|
writer.use {
|
||||||
|
val debugInfo = buildDebugInfo()
|
||||||
|
eventTimer.emit("debug-info")
|
||||||
|
|
||||||
writer.write(
|
writer.write(
|
||||||
BackupInfo(
|
BackupInfo(
|
||||||
version = VERSION,
|
version = VERSION,
|
||||||
backupTimeMs = exportState.backupTime,
|
backupTimeMs = exportState.backupTime,
|
||||||
mediaRootBackupKey = SignalStore.backup.mediaRootBackupKey.value.toByteString(),
|
mediaRootBackupKey = SignalStore.backup.mediaRootBackupKey.value.toByteString(),
|
||||||
firstAppVersion = SignalStore.backup.firstAppVersion,
|
firstAppVersion = SignalStore.backup.firstAppVersion,
|
||||||
debugInfo = buildDebugInfo()
|
debugInfo = debugInfo
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
frameCount++
|
frameCount++
|
||||||
|
eventTimer.emit("header")
|
||||||
|
|
||||||
// We're using a snapshot, so the transaction is more for perf than correctness
|
// We're using a snapshot, so the transaction is more for perf than correctness
|
||||||
dbSnapshot.rawWritableDatabase.withinTransaction {
|
dbSnapshot.rawWritableDatabase.withinTransaction {
|
||||||
|
|||||||
@@ -630,12 +630,6 @@ class AttachmentTable(
|
|||||||
.readToSingleLong()
|
.readToSingleLong()
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun getMessageDoesNotExpireWithinTimeoutClause(): String {
|
|
||||||
val messageHasExpiration = "${MessageTable.TABLE_NAME}.${MessageTable.EXPIRES_IN} > 0"
|
|
||||||
val messageExpiresInOneDayAfterViewing = "$messageHasExpiration AND ${MessageTable.TABLE_NAME}.${MessageTable.EXPIRES_IN} < ${1.days.inWholeMilliseconds}"
|
|
||||||
return "NOT $messageExpiresInOneDayAfterViewing"
|
|
||||||
}
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Finds all of the attachmentIds of attachments that need to be uploaded to the archive cdn.
|
* Finds all of the attachmentIds of attachments that need to be uploaded to the archive cdn.
|
||||||
*/
|
*/
|
||||||
@@ -643,11 +637,7 @@ class AttachmentTable(
|
|||||||
return readableDatabase
|
return readableDatabase
|
||||||
.select("$TABLE_NAME.$ID")
|
.select("$TABLE_NAME.$ID")
|
||||||
.from("$TABLE_NAME LEFT JOIN ${MessageTable.TABLE_NAME} ON $TABLE_NAME.$MESSAGE_ID = ${MessageTable.TABLE_NAME}.${MessageTable.ID}")
|
.from("$TABLE_NAME LEFT JOIN ${MessageTable.TABLE_NAME} ON $TABLE_NAME.$MESSAGE_ID = ${MessageTable.TABLE_NAME}.${MessageTable.ID}")
|
||||||
.where(
|
.where(buildAttachmentsThatNeedUploadQuery())
|
||||||
"($ARCHIVE_TRANSFER_STATE = ? or $ARCHIVE_TRANSFER_STATE = ?) AND $DATA_FILE NOT NULL AND $TRANSFER_STATE = $TRANSFER_PROGRESS_DONE AND (${MessageTable.STORY_TYPE} = 0 OR ${MessageTable.STORY_TYPE} IS NULL) AND ${getMessageDoesNotExpireWithinTimeoutClause()}",
|
|
||||||
ArchiveTransferState.NONE.value,
|
|
||||||
ArchiveTransferState.TEMPORARY_FAILURE.value
|
|
||||||
)
|
|
||||||
.orderBy("$TABLE_NAME.$ID DESC")
|
.orderBy("$TABLE_NAME.$ID DESC")
|
||||||
.run()
|
.run()
|
||||||
.readToList { AttachmentId(it.requireLong(ID)) }
|
.readToList { AttachmentId(it.requireLong(ID)) }
|
||||||
@@ -691,14 +681,10 @@ class AttachmentTable(
|
|||||||
/**
|
/**
|
||||||
* Similar to [getAttachmentsThatNeedArchiveUpload], but returns if the list would be non-null in a more efficient way.
|
* Similar to [getAttachmentsThatNeedArchiveUpload], but returns if the list would be non-null in a more efficient way.
|
||||||
*/
|
*/
|
||||||
fun doAnyAttachmentsNeedArchiveUpload(): Boolean {
|
fun doAnyAttachmentsNeedArchiveUpload(currentTime: Long): Boolean {
|
||||||
return readableDatabase
|
return readableDatabase
|
||||||
.exists("$TABLE_NAME LEFT JOIN ${MessageTable.TABLE_NAME} ON $TABLE_NAME.$MESSAGE_ID = ${MessageTable.TABLE_NAME}.${MessageTable.ID}")
|
.exists("$TABLE_NAME LEFT JOIN ${MessageTable.TABLE_NAME} ON $TABLE_NAME.$MESSAGE_ID = ${MessageTable.TABLE_NAME}.${MessageTable.ID}")
|
||||||
.where(
|
.where(buildAttachmentsThatNeedUploadQuery())
|
||||||
"($ARCHIVE_TRANSFER_STATE = ? OR $ARCHIVE_TRANSFER_STATE = ?) AND $DATA_FILE NOT NULL AND $TRANSFER_STATE = $TRANSFER_PROGRESS_DONE AND (${MessageTable.STORY_TYPE} = 0 OR ${MessageTable.STORY_TYPE} IS NULL) AND ${getMessageDoesNotExpireWithinTimeoutClause()}",
|
|
||||||
ArchiveTransferState.NONE.value,
|
|
||||||
ArchiveTransferState.TEMPORARY_FAILURE.value
|
|
||||||
)
|
|
||||||
.run()
|
.run()
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -856,7 +842,7 @@ class AttachmentTable(
|
|||||||
$REMOTE_KEY NOT NULL AND
|
$REMOTE_KEY NOT NULL AND
|
||||||
$ARCHIVE_TRANSFER_STATE NOT IN (${ArchiveTransferState.FINISHED.value}, ${ArchiveTransferState.PERMANENT_FAILURE.value}) AND
|
$ARCHIVE_TRANSFER_STATE NOT IN (${ArchiveTransferState.FINISHED.value}, ${ArchiveTransferState.PERMANENT_FAILURE.value}) AND
|
||||||
(${MessageTable.STORY_TYPE} = 0 OR ${MessageTable.STORY_TYPE} IS NULL) AND
|
(${MessageTable.STORY_TYPE} = 0 OR ${MessageTable.STORY_TYPE} IS NULL) AND
|
||||||
${getMessageDoesNotExpireWithinTimeoutClause()}
|
(${MessageTable.EXPIRES_IN} = 0 OR ${MessageTable.EXPIRES_IN} > ${ChatItemArchiveExporter.EXPIRATION_CUTOFF.inWholeMilliseconds})
|
||||||
)
|
)
|
||||||
""".trimIndent()
|
""".trimIndent()
|
||||||
)
|
)
|
||||||
@@ -2547,6 +2533,16 @@ class AttachmentTable(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private fun buildAttachmentsThatNeedUploadQuery(): String {
|
||||||
|
return """
|
||||||
|
$ARCHIVE_TRANSFER_STATE IN (${ArchiveTransferState.NONE.value}, ${ArchiveTransferState.TEMPORARY_FAILURE.value}) AND
|
||||||
|
$DATA_FILE NOT NULL AND
|
||||||
|
$TRANSFER_STATE = $TRANSFER_PROGRESS_DONE AND
|
||||||
|
(${MessageTable.STORY_TYPE} = 0 OR ${MessageTable.STORY_TYPE} IS NULL) AND
|
||||||
|
(${MessageTable.TABLE_NAME}.${MessageTable.EXPIRES_IN} <= 0 OR ${MessageTable.TABLE_NAME}.${MessageTable.EXPIRES_IN} > ${ChatItemArchiveExporter.EXPIRATION_CUTOFF.inWholeMilliseconds}) AND
|
||||||
|
$CONTENT_TYPE != '${MediaUtil.LONG_TEXT}'
|
||||||
|
"""
|
||||||
|
}
|
||||||
private fun getAttachment(cursor: Cursor): DatabaseAttachment {
|
private fun getAttachment(cursor: Cursor): DatabaseAttachment {
|
||||||
val contentType = cursor.requireString(CONTENT_TYPE)
|
val contentType = cursor.requireString(CONTENT_TYPE)
|
||||||
|
|
||||||
|
|||||||
@@ -198,10 +198,12 @@ class AttachmentUploadJob private constructor(
|
|||||||
messageId == AttachmentTable.PREUPLOAD_MESSAGE_ID -> {
|
messageId == AttachmentTable.PREUPLOAD_MESSAGE_ID -> {
|
||||||
Log.i(TAG, "[$attachmentId] Avoid uploading preuploaded attachments to archive. Skipping.")
|
Log.i(TAG, "[$attachmentId] Avoid uploading preuploaded attachments to archive. Skipping.")
|
||||||
}
|
}
|
||||||
|
|
||||||
SignalDatabase.messages.willMessageExpireBeforeCutoff(messageId) -> {
|
SignalDatabase.messages.willMessageExpireBeforeCutoff(messageId) -> {
|
||||||
Log.i(TAG, "[$attachmentId] Message will expire within 24hrs. Skipping.")
|
Log.i(TAG, "[$attachmentId] Message will expire within 24hrs. Skipping.")
|
||||||
}
|
}
|
||||||
|
databaseAttachment.contentType == MediaUtil.LONG_TEXT -> {
|
||||||
|
Log.i(TAG, "[$attachmentId] Long text attachment. Skipping.")
|
||||||
|
}
|
||||||
else -> {
|
else -> {
|
||||||
Log.i(TAG, "[$attachmentId] Enqueuing job to copy to archive.")
|
Log.i(TAG, "[$attachmentId] Enqueuing job to copy to archive.")
|
||||||
AppDependencies.jobManager.add(CopyAttachmentToArchiveJob(attachmentId))
|
AppDependencies.jobManager.add(CopyAttachmentToArchiveJob(attachmentId))
|
||||||
|
|||||||
@@ -240,7 +240,7 @@ class BackupMessagesJob private constructor(
|
|||||||
return Result.failure()
|
return Result.failure()
|
||||||
}
|
}
|
||||||
|
|
||||||
if (SignalStore.backup.backsUpMedia && SignalDatabase.attachments.doAnyAttachmentsNeedArchiveUpload()) {
|
if (SignalStore.backup.backsUpMedia && SignalDatabase.attachments.doAnyAttachmentsNeedArchiveUpload(System.currentTimeMillis())) {
|
||||||
Log.i(TAG, "Enqueuing attachment backfill job.")
|
Log.i(TAG, "Enqueuing attachment backfill job.")
|
||||||
AppDependencies.jobManager.add(ArchiveAttachmentBackfillJob())
|
AppDependencies.jobManager.add(ArchiveAttachmentBackfillJob())
|
||||||
} else {
|
} else {
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ import org.thoughtcrime.securesms.jobmanager.impl.NetworkConstraint
|
|||||||
import org.thoughtcrime.securesms.jobmanager.impl.NoRemoteArchiveGarbageCollectionPendingConstraint
|
import org.thoughtcrime.securesms.jobmanager.impl.NoRemoteArchiveGarbageCollectionPendingConstraint
|
||||||
import org.thoughtcrime.securesms.jobs.protos.CopyAttachmentToArchiveJobData
|
import org.thoughtcrime.securesms.jobs.protos.CopyAttachmentToArchiveJobData
|
||||||
import org.thoughtcrime.securesms.keyvalue.SignalStore
|
import org.thoughtcrime.securesms.keyvalue.SignalStore
|
||||||
|
import org.thoughtcrime.securesms.util.MediaUtil
|
||||||
import org.whispersystems.signalservice.api.NetworkResult
|
import org.whispersystems.signalservice.api.NetworkResult
|
||||||
import java.util.concurrent.TimeUnit
|
import java.util.concurrent.TimeUnit
|
||||||
|
|
||||||
@@ -102,6 +103,12 @@ class CopyAttachmentToArchiveJob private constructor(private val attachmentId: A
|
|||||||
return Result.success()
|
return Result.success()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (attachment.contentType == MediaUtil.LONG_TEXT) {
|
||||||
|
Log.i(TAG, "[$attachmentId] Attachment is long text. Resetting transfer state to none and skipping.")
|
||||||
|
SignalDatabase.attachments.setArchiveTransferState(attachmentId, AttachmentTable.ArchiveTransferState.NONE)
|
||||||
|
return Result.success()
|
||||||
|
}
|
||||||
|
|
||||||
if (isCanceled) {
|
if (isCanceled) {
|
||||||
Log.w(TAG, "[$attachmentId] Canceled. Refusing to proceed.")
|
Log.w(TAG, "[$attachmentId] Canceled. Refusing to proceed.")
|
||||||
return Result.failure()
|
return Result.failure()
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ import org.thoughtcrime.securesms.jobs.protos.UploadAttachmentToArchiveJobData
|
|||||||
import org.thoughtcrime.securesms.keyvalue.SignalStore
|
import org.thoughtcrime.securesms.keyvalue.SignalStore
|
||||||
import org.thoughtcrime.securesms.net.SignalNetwork
|
import org.thoughtcrime.securesms.net.SignalNetwork
|
||||||
import org.thoughtcrime.securesms.service.AttachmentProgressService
|
import org.thoughtcrime.securesms.service.AttachmentProgressService
|
||||||
|
import org.thoughtcrime.securesms.util.MediaUtil
|
||||||
import org.thoughtcrime.securesms.util.Util
|
import org.thoughtcrime.securesms.util.Util
|
||||||
import org.whispersystems.signalservice.api.NetworkResult
|
import org.whispersystems.signalservice.api.NetworkResult
|
||||||
import org.whispersystems.signalservice.api.archive.ArchiveMediaUploadFormStatusCodes
|
import org.whispersystems.signalservice.api.archive.ArchiveMediaUploadFormStatusCodes
|
||||||
@@ -137,6 +138,12 @@ class UploadAttachmentToArchiveJob private constructor(
|
|||||||
return Result.success()
|
return Result.success()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (attachment.contentType == MediaUtil.LONG_TEXT) {
|
||||||
|
Log.i(TAG, "[$attachmentId] Attachment is long text. Resetting transfer state to none and skipping.")
|
||||||
|
SignalDatabase.attachments.setArchiveTransferState(attachmentId, AttachmentTable.ArchiveTransferState.NONE)
|
||||||
|
return Result.success()
|
||||||
|
}
|
||||||
|
|
||||||
if (attachment.remoteKey == null) {
|
if (attachment.remoteKey == null) {
|
||||||
Log.w(TAG, "[$attachmentId] Attachment is missing remote key! Cannot upload.")
|
Log.w(TAG, "[$attachmentId] Attachment is missing remote key! Cannot upload.")
|
||||||
return Result.failure()
|
return Result.failure()
|
||||||
|
|||||||
Reference in New Issue
Block a user