Fixes crashes while swiping on messages

First crash was caused by passing a null location id to one of the swipe
actions handlers which are all in kotlin now and all take non nullable type.
The second crash was caused by passing a label repository to the jobs
directly instead of using DI for it- jobs are written to disk and need
to be serializable, which in turn means repository would need to be
serializable.

MAILAND-2439
This commit is contained in:
msurmacz 2021-10-14 10:01:40 +02:00 committed by stefanija
parent cdb287c392
commit 7e34de551e
14 changed files with 56 additions and 85 deletions

View File

@ -23,27 +23,24 @@ import ch.protonmail.android.core.Constants
import ch.protonmail.android.jobs.MoveToFolderJob
import ch.protonmail.android.jobs.PostArchiveJob
import ch.protonmail.android.jobs.PostInboxJob
import ch.protonmail.android.labels.domain.LabelRepository
import com.birbit.android.jobqueue.Job
class ArchiveSwipeHandler : ISwipeHandler {
override fun handleSwipe(
message: SimpleMessage,
currentLocation: String,
labelRepository: LabelRepository
currentLocation: String
): Job = PostArchiveJob(listOf(message.messageId), listOf(currentLocation))
override fun handleUndo(
message: SimpleMessage,
messageLocation: Constants.MessageLocationType,
currentLocation: String,
labelRepository: LabelRepository
currentLocation: String
): Job {
return if (messageLocation == Constants.MessageLocationType.LABEL_FOLDER) {
MoveToFolderJob(listOf(message.messageId), currentLocation, labelRepository)
MoveToFolderJob(listOf(message.messageId), currentLocation)
} else {
PostInboxJob(listOf(message.messageId), labelRepository)
PostInboxJob(listOf(message.messageId))
}
}
}

View File

@ -20,13 +20,18 @@ package ch.protonmail.android.adapters.swipe;
import com.birbit.android.jobqueue.Job;
import org.jetbrains.annotations.NotNull;
import ch.protonmail.android.api.models.SimpleMessage;
import ch.protonmail.android.core.Constants;
import ch.protonmail.android.labels.domain.LabelRepository;
public interface ISwipeHandler {
Job handleSwipe(SimpleMessage message, String currentLocation, LabelRepository labelRepository);
@NotNull
Job handleSwipe(@NotNull SimpleMessage message, @NotNull String currentLocation);
Job handleUndo(SimpleMessage message, Constants.MessageLocationType messageLocation, String currentLocation, LabelRepository labelRepository);
@NotNull
Job handleUndo(@NotNull SimpleMessage message,
@NotNull Constants.MessageLocationType messageLocation,
@NotNull String currentLocation);
}

View File

@ -22,21 +22,18 @@ import ch.protonmail.android.api.models.SimpleMessage
import ch.protonmail.android.core.Constants
import ch.protonmail.android.jobs.PostReadJob
import ch.protonmail.android.jobs.PostUnreadJob
import ch.protonmail.android.labels.domain.LabelRepository
import com.birbit.android.jobqueue.Job
class MarkReadSwipeHandler : ISwipeHandler {
override fun handleSwipe(
message: SimpleMessage,
currentLocation: String,
labelRepository: LabelRepository
currentLocation: String
): Job = PostReadJob(listOf(message.messageId))
override fun handleUndo(
message: SimpleMessage,
messageLocation: Constants.MessageLocationType,
currentLocation: String,
labelRepository: LabelRepository
currentLocation: String
): Job = PostUnreadJob(listOf(message.messageId))
}

View File

@ -23,28 +23,25 @@ import ch.protonmail.android.core.Constants
import ch.protonmail.android.jobs.MoveToFolderJob
import ch.protonmail.android.jobs.PostInboxJob
import ch.protonmail.android.jobs.PostSpamJob
import ch.protonmail.android.labels.domain.LabelRepository
import com.birbit.android.jobqueue.Job
class SpamSwipeHandler : ISwipeHandler {
override fun handleSwipe(
message: SimpleMessage,
currentLocation: String,
labelRepository: LabelRepository
currentLocation: String
): Job =
PostSpamJob(listOf(message.messageId), currentLocation)
override fun handleUndo(
message: SimpleMessage,
messageLocation: Constants.MessageLocationType,
currentLocation: String,
labelRepository: LabelRepository
currentLocation: String
): Job {
return if (messageLocation == Constants.MessageLocationType.LABEL_FOLDER) {
MoveToFolderJob(listOf(message.messageId), currentLocation, labelRepository)
MoveToFolderJob(listOf(message.messageId), currentLocation)
} else {
PostInboxJob(listOf(message.messageId), labelRepository)
PostInboxJob(listOf(message.messageId))
}
}
}

View File

@ -22,15 +22,13 @@ import ch.protonmail.android.api.models.SimpleMessage
import ch.protonmail.android.core.Constants
import ch.protonmail.android.jobs.PostStarJob
import ch.protonmail.android.jobs.PostUnstarJob
import ch.protonmail.android.labels.domain.LabelRepository
import com.birbit.android.jobqueue.Job
class StarSwipeHandler : ISwipeHandler {
override fun handleSwipe(
message: SimpleMessage,
currentLocation: String,
labelRepository: LabelRepository
currentLocation: String
): Job {
return if (!message.isStarred) {
PostStarJob(listOf(message.messageId))
@ -42,8 +40,7 @@ class StarSwipeHandler : ISwipeHandler {
override fun handleUndo(
message: SimpleMessage,
messageLocation: Constants.MessageLocationType,
currentLocation: String,
labelRepository: LabelRepository
currentLocation: String
): Job {
return if (message.isStarred) {
PostStarJob(listOf(message.messageId))

View File

@ -21,6 +21,8 @@ package ch.protonmail.android.adapters.swipe;
import com.birbit.android.jobqueue.Job;
import com.birbit.android.jobqueue.JobManager;
import org.jetbrains.annotations.NotNull;
import java.util.EnumMap;
import java.util.Map;
@ -29,12 +31,11 @@ import javax.inject.Singleton;
import ch.protonmail.android.api.models.SimpleMessage;
import ch.protonmail.android.core.Constants;
import ch.protonmail.android.labels.domain.LabelRepository;
@Singleton
public class SwipeProcessor {
private Map<SwipeAction, ISwipeHandler> handlers = new EnumMap<>(SwipeAction.class);
private final Map<SwipeAction, ISwipeHandler> handlers = new EnumMap<>(SwipeAction.class);
@Inject
public SwipeProcessor() {
@ -45,18 +46,25 @@ public class SwipeProcessor {
handlers.put(swipeAction, swipeHandler);
}
public void handleSwipe(SwipeAction swipeAction, SimpleMessage message, JobManager jobManager, String currentLocation, LabelRepository labelRepository) {
public void handleSwipe(@NotNull SwipeAction swipeAction,
@NotNull SimpleMessage message,
@NotNull JobManager jobManager,
@NotNull String currentLocation) {
ISwipeHandler handler = handlers.get(swipeAction);
if (handler != null) {
Job job = handler.handleSwipe(message, currentLocation, labelRepository);
Job job = handler.handleSwipe(message, currentLocation);
jobManager.addJobInBackground(job);
}
}
public void handleUndo(SwipeAction swipeAction, SimpleMessage message, JobManager jobManager, Constants.MessageLocationType messageLocation, String currentLocation, LabelRepository labelRepository) {
public void handleUndo(@NotNull SwipeAction swipeAction,
@NotNull SimpleMessage message,
@NotNull JobManager jobManager,
@NotNull Constants.MessageLocationType messageLocation,
@NotNull String currentLocation) {
ISwipeHandler handler = handlers.get(swipeAction);
if (handler != null) {
Job job = handler.handleUndo(message, messageLocation, currentLocation, labelRepository);
Job job = handler.handleUndo(message, messageLocation, currentLocation);
jobManager.addJobInBackground(job);
}
}

View File

@ -25,28 +25,25 @@ import ch.protonmail.android.jobs.PostArchiveJob
import ch.protonmail.android.jobs.PostDraftJob
import ch.protonmail.android.jobs.PostInboxJob
import ch.protonmail.android.jobs.PostTrashJobV2
import ch.protonmail.android.labels.domain.LabelRepository
import com.birbit.android.jobqueue.Job
class TrashSwipeHandler : ISwipeHandler {
override fun handleSwipe(
message: SimpleMessage,
currentLocation: String,
labelRepository: LabelRepository
currentLocation: String
): Job =
PostTrashJobV2(listOf(message.messageId), currentLocation, labelRepository)
PostTrashJobV2(listOf(message.messageId), currentLocation)
override fun handleUndo(
message: SimpleMessage,
messageLocation: Constants.MessageLocationType,
currentLocation: String,
labelRepository: LabelRepository
currentLocation: String
): Job = when (messageLocation) {
Constants.MessageLocationType.INBOX -> PostInboxJob(listOf(message.messageId), labelRepository)
Constants.MessageLocationType.INBOX -> PostInboxJob(listOf(message.messageId))
Constants.MessageLocationType.ARCHIVE -> PostArchiveJob(listOf(message.messageId))
Constants.MessageLocationType.LABEL_FOLDER -> MoveToFolderJob(
listOf(message.messageId), currentLocation, labelRepository
listOf(message.messageId), currentLocation
)
else -> PostDraftJob(listOf(message.messageId))
}

View File

@ -24,6 +24,7 @@ import ch.protonmail.android.api.ProtonMailApiManager
import ch.protonmail.android.core.ProtonMailApplication
import ch.protonmail.android.core.QueueNetworkUtil
import ch.protonmail.android.core.UserManager
import ch.protonmail.android.labels.domain.LabelRepository
import ch.protonmail.android.utils.Logger
import ch.protonmail.android.worker.FetchContactsDataWorker
import ch.protonmail.android.worker.FetchMailSettingsWorker
@ -36,6 +37,7 @@ import dagger.hilt.EntryPoint
import dagger.hilt.InstallIn
import dagger.hilt.components.SingletonComponent
import me.proton.core.accountmanager.domain.AccountManager
import me.proton.core.user.domain.UserAddressManager
import timber.log.Timber
import javax.inject.Singleton
@ -95,6 +97,7 @@ interface JobEntryPoint {
fun messageDetailsRepository(): MessageDetailsRepository
fun queueNetworkUtil(): QueueNetworkUtil
fun userManager(): UserManager
fun userAddressManager(): me.proton.core.user.domain.UserAddressManager
fun userAddressManager(): UserAddressManager
fun fetchMailSettingsWorkerEnqueuer(): FetchMailSettingsWorker.Enqueuer
fun labelRepository(): LabelRepository
}

View File

@ -32,7 +32,6 @@ import ch.protonmail.android.data.local.CounterDao;
import ch.protonmail.android.data.local.CounterDatabase;
import ch.protonmail.android.data.local.model.Message;
import ch.protonmail.android.data.local.model.UnreadLocationCounter;
import ch.protonmail.android.labels.domain.LabelRepository;
import ch.protonmail.android.labels.domain.model.Label;
import ch.protonmail.android.labels.domain.model.LabelId;
import ch.protonmail.android.labels.domain.model.LabelType;
@ -42,13 +41,11 @@ import timber.log.Timber;
public class MoveToFolderJob extends ProtonMailBaseJob {
private List<String> mMessageIds;
private String mLabelId;
private final LabelRepository labelRepository;
public MoveToFolderJob(List<String> messageIds, String labelId, LabelRepository labelRepository) {
public MoveToFolderJob(List<String> messageIds, String labelId) {
super(new Params(Priority.MEDIUM).requireNetwork().persist().groupBy(Constants.JOB_GROUP_LABEL));
this.mMessageIds = messageIds;
this.mLabelId = labelId;
this.labelRepository = labelRepository;
}
@Override
@ -97,7 +94,7 @@ public class MoveToFolderJob extends ProtonMailBaseJob {
message.setLocation(Constants.MessageLocationType.LABEL_FOLDER.getMessageLocationTypeValue());
}
message.setFolderLocation(labelRepository);
message.setFolderLocation(getLabelRepository());
Timber.d("Move message id: %s, location: %s, labels: %s", message.getMessageId(), message.getLocation(), message.getAllLabelIDs());
getMessageDetailsRepository().saveMessageBlocking(message);
return unreadIncrease;
@ -108,7 +105,7 @@ public class MoveToFolderJob extends ProtonMailBaseJob {
ArrayList<String> labelsToRemove = new ArrayList<>();
for (String labelId : oldLabels) {
Label label = labelRepository.findLabelBlocking(new LabelId(labelId));
Label label = getLabelRepository().findLabelBlocking(new LabelId(labelId));
// find folders
if (label != null && (label.getType() == LabelType.FOLDER) && !label.getId().equals(mLabelId)) {
labelsToRemove.add(labelId);

View File

@ -32,7 +32,6 @@ import ch.protonmail.android.data.local.CounterDao;
import ch.protonmail.android.data.local.CounterDatabase;
import ch.protonmail.android.data.local.model.Message;
import ch.protonmail.android.data.local.model.UnreadLocationCounter;
import ch.protonmail.android.labels.domain.LabelRepository;
import ch.protonmail.android.labels.domain.model.Label;
import ch.protonmail.android.labels.domain.model.LabelId;
import ch.protonmail.android.labels.domain.model.LabelType;
@ -43,22 +42,13 @@ public class PostInboxJob extends ProtonMailCounterJob {
private final List<String> mMessageIds;
private final List<String> mFolderIds;
private final LabelRepository labelRepository;
public PostInboxJob(final List<String> messageIds, LabelRepository labelRepository) {
public PostInboxJob(final List<String> messageIds) {
super(new Params(Priority.MEDIUM).requireNetwork().persist());
mMessageIds = messageIds;
this.labelRepository = labelRepository;
mFolderIds = null;
}
public PostInboxJob(final List<String> messageIds, List<String> folderIds, LabelRepository labelRepository) {
super(new Params(Priority.MEDIUM).requireNetwork().persist());
mMessageIds = messageIds;
mFolderIds = folderIds;
this.labelRepository = labelRepository;
}
@Override
public void onAdded() {
final CounterDao counterDao = CounterDatabase.Companion
@ -104,7 +94,7 @@ public class PostInboxJob extends ProtonMailCounterJob {
ArrayList<String> labelsToRemove = new ArrayList<>();
for (String labelId : oldLabels) {
Label label = labelRepository.findLabelBlocking(new LabelId(labelId));
Label label = getLabelRepository().findLabelBlocking(new LabelId(labelId));
// find folders
if (label != null &&
(label.getType() == LabelType.FOLDER) &&

View File

@ -32,7 +32,6 @@ import ch.protonmail.android.data.local.CounterDao;
import ch.protonmail.android.data.local.CounterDatabase;
import ch.protonmail.android.data.local.model.Message;
import ch.protonmail.android.data.local.model.UnreadLocationCounter;
import ch.protonmail.android.labels.domain.LabelRepository;
import ch.protonmail.android.labels.domain.model.Label;
import ch.protonmail.android.labels.domain.model.LabelId;
import ch.protonmail.android.labels.domain.model.LabelType;
@ -44,24 +43,14 @@ public class PostTrashJobV2 extends ProtonMailCounterJob {
private final List<String> mMessageIds;
private final List<String> mFolderIds;
private final String mLabelId;
private final LabelRepository labelRepository;
public PostTrashJobV2(final List<String> messageIds, String labelId, LabelRepository labelRepo) {
public PostTrashJobV2(final List<String> messageIds, String labelId) {
super(new Params(Priority.HIGH).requireNetwork().persist().groupBy(Constants.JOB_GROUP_MESSAGE));
mMessageIds = messageIds;
labelRepository = labelRepo;
mFolderIds = null;
mLabelId = labelId;
}
public PostTrashJobV2(final List<String> messageIds, List<String> folderIds, String labelId, LabelRepository labelRepo) {
super(new Params(Priority.HIGH).requireNetwork().persist().groupBy(Constants.JOB_GROUP_MESSAGE));
mMessageIds = messageIds;
mFolderIds = folderIds;
mLabelId = labelId;
labelRepository = labelRepo;
}
@Override
public void onAdded() {
Timber.v("Post to Trash ids: %s onAdded", mMessageIds);
@ -117,7 +106,7 @@ public class PostTrashJobV2 extends ProtonMailCounterJob {
ArrayList<String> labelsToRemove = new ArrayList<>();
for (String labelId : oldLabels) {
Label label = labelRepository.findLabelBlocking(new LabelId(labelId));
Label label = getLabelRepository().findLabelBlocking(new LabelId(labelId));
// find folders
if (label != null && (label.getType() == LabelType.FOLDER) && !label.getId().equals(String.valueOf(Constants.MessageLocationType.TRASH.getMessageLocationTypeValue()))) {
labelsToRemove.add(labelId);

View File

@ -53,6 +53,7 @@ abstract class ProtonMailBaseJob @JvmOverloads protected constructor(
protected fun getQueueNetworkUtil() = entryPoint.queueNetworkUtil()
protected fun getUserManager() = entryPoint.userManager()
protected fun getUserAddressManager() = entryPoint.userAddressManager()
protected fun getLabelRepository() = entryPoint.labelRepository()
override fun onAdded() {}

View File

@ -102,7 +102,6 @@ import ch.protonmail.android.fcm.RegisterDeviceWorker
import ch.protonmail.android.fcm.model.FirebaseToken
import ch.protonmail.android.feature.account.AccountStateManager
import ch.protonmail.android.jobs.EmptyFolderJob
import ch.protonmail.android.labels.domain.LabelRepository
import ch.protonmail.android.labels.domain.model.Label
import ch.protonmail.android.labels.domain.model.LabelType
import ch.protonmail.android.labels.presentation.ui.LabelsActionSheet
@ -188,9 +187,6 @@ internal class MailboxActivity :
@Inject
lateinit var isConversationModeEnabled: ConversationModeEnabled
@Inject
lateinit var labelRepository: LabelRepository
@Inject
@DefaultSharedPreferences
lateinit var defaultSharedPreferences: SharedPreferences
@ -1484,15 +1480,16 @@ internal class MailboxActivity :
else -> throw IllegalArgumentException("Unrecognised direction: $direction")
}
val swipeAction = normalise(SwipeAction.values()[swipeActionOrdinal], currentMailboxLocation)
val currentLocationId = mailboxLabelId ?: mailboxLocation.messageLocationTypeValue.toString()
if (isConversationModeEnabled(mailboxLocation)) {
mailboxViewModel.handleConversationSwipe(
swipeAction,
mailboxItem.itemId,
mailboxLocation,
mailboxLabelId ?: mailboxLocation.messageLocationTypeValue.toString()
currentLocationId
)
} else {
mSwipeProcessor.handleSwipe(swipeAction, messageSwiped, mJobManager, mailboxLabelId, labelRepository)
mSwipeProcessor.handleSwipe(swipeAction, messageSwiped, mJobManager, currentLocationId)
}
if (undoSnack != null && undoSnack!!.isShownOrQueued) {
undoSnack!!.dismiss()
@ -1503,7 +1500,7 @@ internal class MailboxActivity :
getString(swipeAction.actionDescription),
{
mSwipeProcessor.handleUndo(
swipeAction, messageSwiped, mJobManager, mailboxLocation, mailboxLabelId, labelRepository
swipeAction, messageSwiped, mJobManager, mailboxLocation, currentLocationId
)
mailboxAdapter.notifyDataSetChanged()
},

View File

@ -25,7 +25,6 @@ import ch.protonmail.android.activities.messageDetails.repository.MessageDetails
import ch.protonmail.android.api.segments.event.AlarmReceiver
import ch.protonmail.android.jobs.PostArchiveJob
import ch.protonmail.android.jobs.PostTrashJobV2
import ch.protonmail.android.labels.domain.LabelRepository
import ch.protonmail.android.utils.AppUtil
import com.birbit.android.jobqueue.Job
import com.birbit.android.jobqueue.JobManager
@ -53,9 +52,6 @@ class NotificationReceiver : BroadcastReceiver() {
@Inject
lateinit var messageDetailsRepository: MessageDetailsRepository
@Inject
lateinit var labelRepository: LabelRepository
private val coroutineScope = CoroutineScope(Dispatchers.Default)
override fun onReceive(context: Context, intent: Intent) {
@ -88,7 +84,7 @@ class NotificationReceiver : BroadcastReceiver() {
withContext(Dispatchers.Default) {
val message = messageDetailsRepository.findMessageByIdBlocking(messageId)
if (message != null) {
val job: Job = PostTrashJobV2(listOf(message.messageId), null, labelRepository)
val job: Job = PostTrashJobV2(listOf(message.messageId), null)
jobManager.addJobInBackground(job)
}
}