Fixed a few issues related to the GroupAvatarDownloadJob

Added logic to avoid scheduling a GroupAvatarDownloadJob if there is already an existing one with the same parameters
Added logic to avoid trying to download an old avatar for a group if there is a new one
Added logic to prevent an old GroupAvatarDownloadJob from overriding a valid avatar with an old one

app/src/main/java/org/thoughtcrime/securesms/database/SessionJobDatabase.kt

@@ -83,11 +83,11 @@ class SessionJobDatabase(context: Context, helper: SQLCipherOpenHelper) : Databa
         }
     }
 
-    fun getGroupAvatarDownloadJob(server: String, room: String): GroupAvatarDownloadJob? {
+    fun getGroupAvatarDownloadJob(server: String, room: String, imageId: String?): GroupAvatarDownloadJob? {
         val database = databaseHelper.readableDatabase
         return database.getAll(sessionJobTable, "$jobType = ?", arrayOf(GroupAvatarDownloadJob.KEY)) {
             jobFromCursor(it) as GroupAvatarDownloadJob?
-        }.filterNotNull().find { it.server == server && it.room == room }
+        }.filterNotNull().find { it.server == server && it.room == room && (imageId == null || it.imageId == imageId) }
     }
 
     fun cancelPendingMessageSendJobs(threadID: Long) {

app/src/main/java/org/thoughtcrime/securesms/database/Storage.kt

@@ -226,8 +226,8 @@ class Storage(context: Context, helper: SQLCipherOpenHelper) : Database(context,
         return DatabaseComponent.get(context).sessionJobDatabase().getMessageReceiveJob(messageReceiveJobID)
     }
 
-    override fun getGroupAvatarDownloadJob(server: String, room: String): GroupAvatarDownloadJob? {
-        return DatabaseComponent.get(context).sessionJobDatabase().getGroupAvatarDownloadJob(server, room)
+    override fun getGroupAvatarDownloadJob(server: String, room: String, imageId: String?): GroupAvatarDownloadJob? {
+        return DatabaseComponent.get(context).sessionJobDatabase().getGroupAvatarDownloadJob(server, room, imageId)
     }
 
     override fun resumeMessageSendJobIfNeeded(messageSendJobID: String) {

libsession/src/main/java/org/session/libsession/database/StorageProtocol.kt

@@ -49,7 +49,7 @@ interface StorageProtocol {
     fun getAttachmentUploadJob(attachmentID: Long): AttachmentUploadJob?
     fun getMessageSendJob(messageSendJobID: String): MessageSendJob?
     fun getMessageReceiveJob(messageReceiveJobID: String): Job?
-    fun getGroupAvatarDownloadJob(server: String, room: String): Job?
+    fun getGroupAvatarDownloadJob(server: String, room: String, imageId: String?): Job?
     fun resumeMessageSendJobIfNeeded(messageSendJobID: String)
     fun isJobCanceled(job: Job): Boolean
 

libsession/src/main/java/org/session/libsession/messaging/jobs/BackgroundGroupAddJob.kt

@@ -43,8 +43,8 @@ class BackgroundGroupAddJob(val joinUrl: String): Job {
             storage.setOpenGroupPublicKey(openGroup.server, openGroup.serverPublicKey)
             val info = storage.addOpenGroup(openGroup.joinUrl())
             val imageId = info?.imageId
-            if (imageId != null) {
-                JobQueue.shared.add(GroupAvatarDownloadJob(openGroup.room, openGroup.server))
+            if (imageId != null && storage.getGroupAvatarDownloadJob(openGroup.server, openGroup.room, imageId) == null) {
+                JobQueue.shared.add(GroupAvatarDownloadJob(openGroup.server, openGroup.room, imageId))
             }
             Log.d(KEY, "onOpenGroupAdded(${openGroup.server})")
             storage.onOpenGroupAdded(openGroup.server)

libsession/src/main/java/org/session/libsession/messaging/jobs/GroupAvatarDownloadJob.kt

@@ -5,7 +5,7 @@ import org.session.libsession.messaging.open_groups.OpenGroupApi
 import org.session.libsession.messaging.utilities.Data
 import org.session.libsession.utilities.GroupUtil
 
-class GroupAvatarDownloadJob(val room: String, val server: String) : Job {
+class GroupAvatarDownloadJob(val server: String, val room: String, val imageId: String?) : Job {
 
     override var delegate: JobDelegate? = null
     override var id: String? = null
@@ -13,10 +13,30 @@ class GroupAvatarDownloadJob(val room: String, val server: String) : Job {
     override val maxFailureCount: Int = 10
 
     override fun execute(dispatcherName: String) {
+        if (imageId == null) {
+            delegate?.handleJobFailedPermanently(this, dispatcherName, Exception("GroupAvatarDownloadJob now requires imageId"))
+            return
+        }
+
         val storage = MessagingModuleConfiguration.shared.storage
-        val imageId = storage.getOpenGroup(room, server)?.imageId ?: return
+        val storedImageId = storage.getOpenGroup(room, server)?.imageId
+
+        if (storedImageId == null || storedImageId != imageId) {
+            delegate?.handleJobFailedPermanently(this, dispatcherName, Exception("GroupAvatarDownloadJob imageId does not match the OpenGroup"))
+            return
+        }
+
         try {
             val bytes = OpenGroupApi.downloadOpenGroupProfilePicture(server, room, imageId).get()
+
+            // Once the download is complete the imageId might no longer match, so we need to fetch it again just in case
+            val postDownloadStoredImageId = storage.getOpenGroup(room, server)?.imageId
+
+            if (postDownloadStoredImageId == null || postDownloadStoredImageId != imageId) {
+                delegate?.handleJobFailedPermanently(this, dispatcherName, Exception("GroupAvatarDownloadJob imageId no longer matches the OpenGroup"))
+                return
+            }
+
             val groupId = GroupUtil.getEncodedOpenGroupID("$server.$room".toByteArray())
             storage.updateProfilePicture(groupId, bytes)
             storage.updateTimestampUpdated(groupId, System.currentTimeMillis())
@@ -30,6 +50,7 @@ class GroupAvatarDownloadJob(val room: String, val server: String) : Job {
         return Data.Builder()
             .putString(ROOM, room)
             .putString(SERVER, server)
+            .putString(IMAGE_ID, imageId)
             .build()
     }
 
@@ -40,6 +61,7 @@ class GroupAvatarDownloadJob(val room: String, val server: String) : Job {
 
         private const val ROOM = "room"
         private const val SERVER = "server"
+        private const val IMAGE_ID = "imageId"
     }
 
     class Factory : Job.Factory<GroupAvatarDownloadJob> {
@@ -47,7 +69,8 @@ class GroupAvatarDownloadJob(val room: String, val server: String) : Job {
         override fun create(data: Data): GroupAvatarDownloadJob {
             return GroupAvatarDownloadJob(
                 data.getString(ROOM),
-                data.getString(SERVER)
+                data.getString(SERVER),
+                if (data.hasString(IMAGE_ID)) { data.getString(IMAGE_ID) } else { null }
             )
         }
     }

libsession/src/main/java/org/session/libsession/messaging/sending_receiving/pollers/OpenGroupPoller.kt

@@ -165,14 +165,16 @@ class OpenGroupPoller(private val server: String, private val executorService: S
                 pollInfo.details.imageId != null && (
                     pollInfo.details.imageId != existingOpenGroup.imageId ||
                     !storage.hasDownloadedProfilePicture(dbGroupId)
-                )
+                ) &&
+                storage.getGroupAvatarDownloadJob(openGroup.server, openGroup.room, pollInfo.details.imageId) == null
             ) || (
                 pollInfo.details == null &&
                 existingOpenGroup.imageId != null &&
-                !storage.hasDownloadedProfilePicture(dbGroupId)
+                !storage.hasDownloadedProfilePicture(dbGroupId) &&
+                storage.getGroupAvatarDownloadJob(openGroup.server, openGroup.room, existingOpenGroup.imageId) == null
             )
         ) {
-            JobQueue.shared.add(GroupAvatarDownloadJob(roomToken, server))
+            JobQueue.shared.add(GroupAvatarDownloadJob(roomToken, server, existingOpenGroup.imageId))
         }
     }