Remove job adds from database transactions.

This commit is contained in:
Greyson Parrelli 2020-12-03 17:33:04 -05:00
parent 31960b53a0
commit 97047bccde
6 changed files with 63 additions and 43 deletions

View file

@ -1302,7 +1302,6 @@ public class MmsDatabase extends MessageDatabase {
} }
notifyConversationListeners(threadId); notifyConversationListeners(threadId);
ApplicationDependencies.getJobManager().add(new TrimThreadJob(threadId));
return Optional.of(new InsertResult(messageId, threadId)); return Optional.of(new InsertResult(messageId, threadId));
} }

View file

@ -866,13 +866,16 @@ public class SmsDatabase extends MessageDatabase {
db.insert(TABLE_NAME, null, values); db.insert(TABLE_NAME, null, values);
notifyConversationListeners(threadId); notifyConversationListeners(threadId);
ApplicationDependencies.getJobManager().add(new TrimThreadJob(threadId));
}); });
db.setTransactionSuccessful(); db.setTransactionSuccessful();
} finally { } finally {
db.endTransaction(); db.endTransaction();
} }
Stream.of(threadIdsToUpdate)
.withoutNulls()
.forEach(threadId -> ApplicationDependencies.getJobManager().add(new TrimThreadJob(threadId)));
} }
@Override @Override

View file

@ -9,6 +9,7 @@ import androidx.annotation.NonNull;
import androidx.annotation.Nullable; import androidx.annotation.Nullable;
import androidx.annotation.WorkerThread; import androidx.annotation.WorkerThread;
import org.thoughtcrime.securesms.database.DatabaseFactory;
import org.thoughtcrime.securesms.jobmanager.impl.DefaultExecutorFactory; import org.thoughtcrime.securesms.jobmanager.impl.DefaultExecutorFactory;
import org.thoughtcrime.securesms.jobmanager.impl.JsonDataSerializer; import org.thoughtcrime.securesms.jobmanager.impl.JsonDataSerializer;
import org.thoughtcrime.securesms.jobmanager.workmanager.WorkManagerMigrator; import org.thoughtcrime.securesms.jobmanager.workmanager.WorkManagerMigrator;
@ -17,7 +18,7 @@ import org.thoughtcrime.securesms.logging.Log;
import org.thoughtcrime.securesms.util.Debouncer; import org.thoughtcrime.securesms.util.Debouncer;
import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.thoughtcrime.securesms.util.TextSecurePreferences;
import org.thoughtcrime.securesms.util.Util; import org.thoughtcrime.securesms.util.Util;
import org.thoughtcrime.securesms.util.concurrent.NonMainThreadExecutor; import org.thoughtcrime.securesms.util.concurrent.FilteredExecutor;
import org.whispersystems.libsignal.util.guava.Optional; import org.whispersystems.libsignal.util.guava.Optional;
import java.util.ArrayList; import java.util.ArrayList;
@ -57,7 +58,17 @@ public class JobManager implements ConstraintObserver.Notifier {
public JobManager(@NonNull Application application, @NonNull Configuration configuration) { public JobManager(@NonNull Application application, @NonNull Configuration configuration) {
this.application = application; this.application = application;
this.configuration = configuration; this.configuration = configuration;
this.executor = new NonMainThreadExecutor(configuration.getExecutorFactory().newSingleThreadExecutor("signal-JobManager")); this.executor = new FilteredExecutor(configuration.getExecutorFactory().newSingleThreadExecutor("signal-JobManager"),
() -> {
if (Util.isMainThread()) {
return true;
} else if (DatabaseFactory.inTransaction(application)) {
Log.w(TAG, "Tried to add a job while in a transaction!", new Throwable());
return true;
} else {
return false;
}
});
this.jobTracker = configuration.getJobTracker(); this.jobTracker = configuration.getJobTracker();
this.jobController = new JobController(application, this.jobController = new JobController(application,
configuration.getJobStorage(), configuration.getJobStorage(),

View file

@ -1156,16 +1156,6 @@ public final class PushProcessMessageJob extends BaseJob {
insertResult = database.insertSecureDecryptedMessageInbox(mediaMessage, -1); insertResult = database.insertSecureDecryptedMessageInbox(mediaMessage, -1);
if (insertResult.isPresent()) { if (insertResult.isPresent()) {
List<DatabaseAttachment> allAttachments = DatabaseFactory.getAttachmentDatabase(context).getAttachmentsForMessage(insertResult.get().getMessageId());
List<DatabaseAttachment> stickerAttachments = Stream.of(allAttachments).filter(Attachment::isSticker).toList();
List<DatabaseAttachment> attachments = Stream.of(allAttachments).filterNot(Attachment::isSticker).toList();
forceStickerDownloadIfNecessary(stickerAttachments);
for (DatabaseAttachment attachment : attachments) {
ApplicationDependencies.getJobManager().add(new AttachmentDownloadJob(insertResult.get().getMessageId(), attachment.getAttachmentId(), false));
}
if (smsMessageId.isPresent()) { if (smsMessageId.isPresent()) {
DatabaseFactory.getSmsDatabase(context).deleteMessage(smsMessageId.get()); DatabaseFactory.getSmsDatabase(context).deleteMessage(smsMessageId.get());
} }
@ -1179,7 +1169,18 @@ public final class PushProcessMessageJob extends BaseJob {
} }
if (insertResult.isPresent()) { if (insertResult.isPresent()) {
List<DatabaseAttachment> allAttachments = DatabaseFactory.getAttachmentDatabase(context).getAttachmentsForMessage(insertResult.get().getMessageId());
List<DatabaseAttachment> stickerAttachments = Stream.of(allAttachments).filter(Attachment::isSticker).toList();
List<DatabaseAttachment> attachments = Stream.of(allAttachments).filterNot(Attachment::isSticker).toList();
forceStickerDownloadIfNecessary(stickerAttachments);
for (DatabaseAttachment attachment : attachments) {
ApplicationDependencies.getJobManager().add(new AttachmentDownloadJob(insertResult.get().getMessageId(), attachment.getAttachmentId(), false));
}
ApplicationDependencies.getMessageNotifier().updateNotification(context, insertResult.get().getThreadId()); ApplicationDependencies.getMessageNotifier().updateNotification(context, insertResult.get().getThreadId());
ApplicationDependencies.getJobManager().add(new TrimThreadJob(insertResult.get().getThreadId()));
if (message.isViewOnce()) { if (message.isViewOnce()) {
ApplicationContext.getInstance(context).getViewOnceMessageManager().scheduleIfNecessary(); ApplicationContext.getInstance(context).getViewOnceMessageManager().scheduleIfNecessary();

View file

@ -0,0 +1,35 @@
package org.thoughtcrime.securesms.util.concurrent;
import androidx.annotation.NonNull;
import org.thoughtcrime.securesms.util.Util;
import java.util.concurrent.Executor;
/**
* Allows you to specify a filter upon which a job will be executed on the provided executor. If
* it doesn't match the filter, it will be run on the calling thread.
*/
public final class FilteredExecutor implements Executor {
private final Executor backgroundExecutor;
private final Filter filter;
public FilteredExecutor(@NonNull Executor backgroundExecutor, @NonNull Filter filter) {
this.backgroundExecutor = backgroundExecutor;
this.filter = filter;
}
@Override
public void execute(@NonNull Runnable runnable) {
if (filter.shouldRunOnExecutor()) {
backgroundExecutor.execute(runnable);
} else {
runnable.run();
}
}
public interface Filter {
boolean shouldRunOnExecutor();
}
}

View file

@ -1,29 +0,0 @@
package org.thoughtcrime.securesms.util.concurrent;
import androidx.annotation.NonNull;
import org.thoughtcrime.securesms.util.Util;
import java.util.concurrent.Executor;
/**
* If submitted on the main thread, the task will be executed on the provided background executor.
* Otherwise, the task will be run on the calling thread.
*/
public final class NonMainThreadExecutor implements Executor {
private final Executor backgroundExecutor;
public NonMainThreadExecutor(@NonNull Executor backgroundExecutor) {
this.backgroundExecutor = backgroundExecutor;
}
@Override
public void execute(@NonNull Runnable runnable) {
if (Util.isMainThread()) {
backgroundExecutor.execute(runnable);
} else {
runnable.run();
}
}
}