From e461625da44af828a428cd048e0988e81d1e32b6 Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Wed, 14 Apr 2021 14:38:29 -0400 Subject: [PATCH] Add Log.internal() --- .../securesms/ApplicationContext.java | 2 +- .../notifications/v2/MessageNotifierV2.kt | 5 +- .../notifications/v2/NotificationFactory.kt | 9 +- .../core/util/logging/CompoundLogger.java | 65 ++++++++++ .../org/signal/core/util/logging/Log.java | 115 ++++++++++-------- .../signal/core/util/logging/NoopLogger.java | 27 ++++ 6 files changed, 162 insertions(+), 61 deletions(-) create mode 100644 core-util/src/main/java/org/signal/core/util/logging/CompoundLogger.java create mode 100644 core-util/src/main/java/org/signal/core/util/logging/NoopLogger.java diff --git a/app/src/main/java/org/thoughtcrime/securesms/ApplicationContext.java b/app/src/main/java/org/thoughtcrime/securesms/ApplicationContext.java index 80a31fe83..1f98cfd55 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/ApplicationContext.java +++ b/app/src/main/java/org/thoughtcrime/securesms/ApplicationContext.java @@ -229,7 +229,7 @@ public class ApplicationContext extends MultiDexApplication implements AppForegr private void initializeLogging() { persistentLogger = new PersistentLogger(this, LogSecretProvider.getOrCreateAttachmentSecret(this), BuildConfig.VERSION_NAME); - org.signal.core.util.logging.Log.initialize(new AndroidLogger(), persistentLogger); + org.signal.core.util.logging.Log.initialize(FeatureFlags::internalUser, new AndroidLogger(), persistentLogger); SignalProtocolLoggerProvider.setProvider(new CustomSignalProtocolLogger()); } diff --git a/app/src/main/java/org/thoughtcrime/securesms/notifications/v2/MessageNotifierV2.kt b/app/src/main/java/org/thoughtcrime/securesms/notifications/v2/MessageNotifierV2.kt index 27ddba024..1eb5fc89d 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/notifications/v2/MessageNotifierV2.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/notifications/v2/MessageNotifierV2.kt @@ -23,7 +23,6 @@ import org.thoughtcrime.securesms.notifications.NotificationIds import org.thoughtcrime.securesms.recipients.Recipient import org.thoughtcrime.securesms.service.KeyCachingService import org.thoughtcrime.securesms.util.BubbleUtil.BubbleState -import org.thoughtcrime.securesms.util.FeatureFlags import org.thoughtcrime.securesms.util.ServiceUtil import org.thoughtcrime.securesms.util.TextSecurePreferences import org.thoughtcrime.securesms.webrtc.CallNotificationBuilder @@ -111,9 +110,7 @@ class MessageNotifierV2 : MessageNotifier { val state: NotificationStateV2 = NotificationStateProvider.constructNotificationState(context) - if (FeatureFlags.internalUser()) { - Log.i(TAG, state.toString()) - } + Log.internal().i(TAG, state.toString()) if (state.isEmpty) { Log.i(TAG, "State is empty, cancelling all notifications") diff --git a/app/src/main/java/org/thoughtcrime/securesms/notifications/v2/NotificationFactory.kt b/app/src/main/java/org/thoughtcrime/securesms/notifications/v2/NotificationFactory.kt index b462a8c3c..a85c9cbd4 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/notifications/v2/NotificationFactory.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/notifications/v2/NotificationFactory.kt @@ -27,7 +27,6 @@ import org.thoughtcrime.securesms.notifications.NotificationIds import org.thoughtcrime.securesms.recipients.Recipient import org.thoughtcrime.securesms.util.BubbleUtil import org.thoughtcrime.securesms.util.ConversationUtil -import org.thoughtcrime.securesms.util.FeatureFlags import org.thoughtcrime.securesms.util.ServiceUtil import org.thoughtcrime.securesms.util.TextSecurePreferences @@ -56,9 +55,7 @@ object NotificationFactory { if (Build.VERSION.SDK_INT >= 23 || state.conversations.size == 1) { state.conversations.forEach { conversation -> if (conversation.threadId == visibleThreadId && conversation.hasNewNotifications()) { - if (FeatureFlags.internalUser()) { - Log.i(TAG, "Thread is visible, notifying in thread. notificationId: ${conversation.notificationId}") - } + Log.internal().i(TAG, "Thread is visible, notifying in thread. notificationId: ${conversation.notificationId}") notifyInThread(context, conversation.recipient, lastAudibleNotification) } else if (conversation.hasNewNotifications() || alertOverrides.contains(conversation.threadId)) { @@ -233,9 +230,7 @@ object NotificationFactory { private fun NotificationManagerCompat.safelyNotify(context: Context, threadRecipient: Recipient?, notificationId: Int, notification: Notification) { try { notify(notificationId, notification) - if (FeatureFlags.internalUser()) { - Log.i(TAG, "Posted notification: $notification") - } + Log.internal().i(TAG, "Posted notification: $notification") } catch (e: SecurityException) { Log.i(TAG, "Security exception when posting notification, clearing ringtone") if (threadRecipient != null) { diff --git a/core-util/src/main/java/org/signal/core/util/logging/CompoundLogger.java b/core-util/src/main/java/org/signal/core/util/logging/CompoundLogger.java new file mode 100644 index 000000000..24c198fd3 --- /dev/null +++ b/core-util/src/main/java/org/signal/core/util/logging/CompoundLogger.java @@ -0,0 +1,65 @@ +package org.signal.core.util.logging; + +import androidx.annotation.NonNull; + +/** + * A way to treat N loggers as one. Wraps a bunch of other loggers and forwards the method calls to + * all of them. + */ +class CompoundLogger extends Log.Logger { + + private final Log.Logger[] loggers; + + CompoundLogger(@NonNull Log.Logger... loggers) { + this.loggers = loggers; + } + + @Override + public void v(String tag, String message, Throwable t) { + for (Log.Logger logger : loggers) { + logger.v(tag, message, t); + } + } + + @Override + public void d(String tag, String message, Throwable t) { + for (Log.Logger logger : loggers) { + logger.d(tag, message, t); + } + } + + @Override + public void i(String tag, String message, Throwable t) { + for (Log.Logger logger : loggers) { + logger.i(tag, message, t); + } + } + + @Override + public void w(String tag, String message, Throwable t) { + for (Log.Logger logger : loggers) { + logger.w(tag, message, t); + } + } + + @Override + public void e(String tag, String message, Throwable t) { + for (Log.Logger logger : loggers) { + logger.e(tag, message, t); + } + } + + @Override + public void wtf(String tag, String message, Throwable t) { + for (Log.Logger logger : loggers) { + logger.wtf(tag, message, t); + } + } + + @Override + public void blockUntilAllWritesFinished() { + for (Log.Logger logger : loggers) { + logger.blockUntilAllWritesFinished(); + } + } +} diff --git a/core-util/src/main/java/org/signal/core/util/logging/Log.java b/core-util/src/main/java/org/signal/core/util/logging/Log.java index a6b0a62ea..c2b660a99 100644 --- a/core-util/src/main/java/org/signal/core/util/logging/Log.java +++ b/core-util/src/main/java/org/signal/core/util/logging/Log.java @@ -3,15 +3,30 @@ package org.signal.core.util.logging; import android.annotation.SuppressLint; import androidx.annotation.MainThread; +import androidx.annotation.NonNull; + +import java.util.logging.Logger; @SuppressLint("LogNotSignal") public final class Log { - private static Logger[] loggers; + private static final Logger NOOP_LOGGER = new NoopLogger(); + private static InternalCheck internalCheck; + private static Logger logger = new AndroidLogger(); + + /** + * @param internalCheck A checker that will indicate if this is an internal user + * @param loggers A list of loggers that will be given every log statement. + */ @MainThread + public static void initialize(@NonNull InternalCheck internalCheck, Logger... loggers) { + Log.internalCheck = internalCheck; + Log.logger = new CompoundLogger(loggers); + } + public static void initialize(Logger... loggers) { - Log.loggers = loggers; + initialize(() -> false, loggers); } public static void v(String tag, String message) { @@ -63,63 +78,27 @@ public final class Log { } public static void v(String tag, String message, Throwable t) { - if (loggers != null) { - for (Logger logger : loggers) { - logger.v(tag, message, t); - } - } else { - android.util.Log.v(tag, message, t); - } + logger.v(tag, message, t); } public static void d(String tag, String message, Throwable t) { - if (loggers != null) { - for (Logger logger : loggers) { - logger.d(tag, message, t); - } - } else { - android.util.Log.d(tag, message, t); - } + logger.d(tag, message, t); } public static void i(String tag, String message, Throwable t) { - if (loggers != null) { - for (Logger logger : loggers) { - logger.i(tag, message, t); - } - } else { - android.util.Log.i(tag, message, t); - } + logger.i(tag, message, t); } public static void w(String tag, String message, Throwable t) { - if (loggers != null) { - for (Logger logger : loggers) { - logger.w(tag, message, t); - } - } else { - android.util.Log.w(tag, message, t); - } + logger.w(tag, message, t); } public static void e(String tag, String message, Throwable t) { - if (loggers != null) { - for (Logger logger : loggers) { - logger.e(tag, message, t); - } - } else { - android.util.Log.e(tag, message, t); - } + logger.e(tag, message, t); } public static void wtf(String tag, String message, Throwable t) { - if (loggers != null) { - for (Logger logger : loggers) { - logger.wtf(tag, message, t); - } - } else { - android.util.Log.wtf(tag, message, t); - } + logger.wtf(tag, message, t); } public static String tag(Class clazz) { @@ -130,14 +109,24 @@ public final class Log { return simpleName; } - public static void blockUntilAllWritesFinished() { - if (loggers != null) { - for (Logger logger : loggers) { - logger.blockUntilAllWritesFinished(); - } + /** + * Important: This is not something that can be used to log PII. Instead, it's intended use is for + * logs that might be too verbose or otherwise unnecessary for public users. + * + * @return The normal logger if this is an internal user, or a no-op logger if it isn't. + */ + public static Logger internal() { + if (internalCheck.isInternal()) { + return logger; + } else { + return NOOP_LOGGER; } } + public static void blockUntilAllWritesFinished() { + logger.blockUntilAllWritesFinished(); + } + public static abstract class Logger { public abstract void v(String tag, String message, Throwable t); public abstract void d(String tag, String message, Throwable t); @@ -146,5 +135,33 @@ public final class Log { public abstract void e(String tag, String message, Throwable t); public abstract void wtf(String tag, String message, Throwable t); public abstract void blockUntilAllWritesFinished(); + + public void v(String tag, String message) { + v(tag, message, null); + } + + public void d(String tag, String message) { + d(tag, message, null); + } + + public void i(String tag, String message) { + i(tag, message, null); + } + + public void w(String tag, String message) { + w(tag, message, null); + } + + public void e(String tag, String message) { + e(tag, message, null); + } + + public void wtf(String tag, String message) { + wtf(tag, message, null); + } + } + + public interface InternalCheck { + boolean isInternal(); } } diff --git a/core-util/src/main/java/org/signal/core/util/logging/NoopLogger.java b/core-util/src/main/java/org/signal/core/util/logging/NoopLogger.java new file mode 100644 index 000000000..4b4b318ba --- /dev/null +++ b/core-util/src/main/java/org/signal/core/util/logging/NoopLogger.java @@ -0,0 +1,27 @@ +package org.signal.core.util.logging; + +/** + * A logger that does nothing. + */ +class NoopLogger extends Log.Logger { + @Override + public void v(String tag, String message, Throwable t) { } + + @Override + public void d(String tag, String message, Throwable t) { } + + @Override + public void i(String tag, String message, Throwable t) { } + + @Override + public void w(String tag, String message, Throwable t) { } + + @Override + public void e(String tag, String message, Throwable t) { } + + @Override + public void wtf(String tag, String message, Throwable t) { } + + @Override + public void blockUntilAllWritesFinished() { } +}