Skip to content

Commit

Permalink
Bug 1867606 - Make message param logging api non nullable
Browse files Browse the repository at this point in the history
  • Loading branch information
rahulsainani authored and mergify[bot] committed Dec 19, 2023
1 parent 3298b76 commit 1640dc0
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ object Log {
priority: Priority = Priority.DEBUG,
tag: String? = null,
throwable: Throwable? = null,
message: String? = null,
message: String,
) {
if (priority.value >= logLevel.value) {
synchronized(sinks) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,28 @@ class Logger(
/**
* Send a DEBUG log message.
*/
fun debug(message: String? = null, throwable: Throwable? = null) {
fun debug(message: String, throwable: Throwable? = null) {
Log.log(Log.Priority.DEBUG, tag = tag, message = message, throwable = throwable)
}

/**
* Send a INFO log message.
*/
fun info(message: String? = null, throwable: Throwable? = null) {
fun info(message: String, throwable: Throwable? = null) {
Log.log(Log.Priority.INFO, tag = tag, message = message, throwable = throwable)
}

/**
* Send a WARN log message.
*/
fun warn(message: String? = null, throwable: Throwable? = null) {
fun warn(message: String, throwable: Throwable? = null) {
Log.log(Log.Priority.WARN, tag = tag, message = message, throwable = throwable)
}

/**
* Send a ERROR log message.
*/
fun error(message: String? = null, throwable: Throwable? = null) {
fun error(message: String, throwable: Throwable? = null) {
Log.log(Log.Priority.ERROR, tag = tag, message = message, throwable = throwable)
}

Expand Down Expand Up @@ -71,22 +71,22 @@ class Logger(
/**
* Send a DEBUG log message.
*/
fun debug(message: String? = null, throwable: Throwable? = null) = DEFAULT.debug(message, throwable)
fun debug(message: String, throwable: Throwable? = null) = DEFAULT.debug(message, throwable)

/**
* Send a INFO log message.
*/
fun info(message: String? = null, throwable: Throwable? = null) = DEFAULT.info(message, throwable)
fun info(message: String, throwable: Throwable? = null) = DEFAULT.info(message, throwable)

/**
* Send a WARN log message.
*/
fun warn(message: String? = null, throwable: Throwable? = null) = DEFAULT.warn(message, throwable)
fun warn(message: String, throwable: Throwable? = null) = DEFAULT.warn(message, throwable)

/**
* Send a ERROR log message.
*/
fun error(message: String? = null, throwable: Throwable? = null) = DEFAULT.error(message, throwable)
fun error(message: String, throwable: Throwable? = null) = DEFAULT.error(message, throwable)

/**
* Measure the time it takes to execute the provided block and print a log message before and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,21 @@ class AndroidLogSink(
/**
* Low-level logging call.
*/
override fun log(priority: Log.Priority, tag: String?, throwable: Throwable?, message: String?) {
override fun log(priority: Log.Priority, tag: String?, throwable: Throwable?, message: String) {
val logTag = tag(tag)

val logMessage: String = if (message != null && throwable != null) {
val logMessage: String = if (throwable != null) {
"$message\n${throwable.getStacktraceAsString()}"
} else {
message ?: (throwable?.getStacktraceAsString() ?: "(empty)")
message
}

android.util.Log.println(priority.value, logTag, logMessage)
}

private fun tag(candidate: String?): String {
val tag = candidate ?: defaultTag
if (Build.VERSION.SDK_INT < android.os.Build.VERSION_CODES.N && tag.length > MAX_TAG_LENGTH) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.N && tag.length > MAX_TAG_LENGTH) {
return tag.substring(0, MAX_TAG_LENGTH)
}
return tag
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,23 @@ package mozilla.components.support.base.log.sink

import mozilla.components.support.base.log.Log

/**
* Common interface for log sinks.
*/
interface LogSink {

/**
* Low-level logging call that should be implemented based on the Sink's capabilities.
*
* @param priority The [Log.Priority] of the log message, defaults to [Log.Priority.DEBUG].
* @param tag The tag of the log message.
* @param throwable The optional [Throwable] that should be logged.
* @param message The message that should be logged.
*/
fun log(
priority: Log.Priority = Log.Priority.DEBUG,
tag: String? = null,
throwable: Throwable? = null,
message: String? = null,
message: String,
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -175,38 +175,6 @@ class AndroidLogSinkTest {
)
}

@Test
fun `Sink will print stacktrace of throwable`() {
val sink = AndroidLogSink()

sink.log(throwable = MockThrowable())

val logs = ShadowLog.getLogs()
assertEquals(1, logs.size)
assertEquals(
"java.lang.RuntimeException: This is broken\n\tat A\n\tat B\n\tat C",
logs.last().msg,
)
}

@Test
fun `Sink will print empty default message if no message and no throwable is provided`() {
val sink = AndroidLogSink()

sink.log()
sink.log(tag = "Tag")

val logs = ShadowLog.getLogs()

assertEquals(2, logs.size)

assertEquals("(empty)", logs[0].msg)
assertEquals("App", logs[0].tag)

assertEquals("(empty)", logs[1].msg)
assertEquals("Tag", logs[1].tag)
}

private class MockThrowable : Throwable("Kaput") {
override fun printStackTrace(writer: PrintWriter) {
writer.write("java.lang.RuntimeException: This is broken\n\tat A\n\tat B\n\tat C")
Expand Down
3 changes: 3 additions & 0 deletions docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ permalink: /changelog/
* Added `countBookmarksInTrees` to more efficiently determine how many bookmarks exist under part or parts of a bookmarks tree, which
is taken advantage of by `DesktopFolders`.

* **support-base**
* Make `message` param non optional for the Logging APIs. [Bug 1867606](https://bugzilla.mozilla.org/show_bug.cgi?id=1867606)

# 121.0
* [Commits](https://github.com/mozilla-mobile/firefox-android/compare/releases_v120..releases_v121)
* [Dependencies](https://github.com/mozilla-mobile/firefox-android/blob/releases_v121/android-components/plugins/dependencies/src/main/java/DependenciesPlugin.kt)
Expand Down
2 changes: 1 addition & 1 deletion fenix/app/src/main/java/org/mozilla/fenix/FenixLogSink.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class FenixLogSink(
priority: Log.Priority,
tag: String?,
throwable: Throwable?,
message: String?,
message: String,
) {
if (priority == Log.Priority.DEBUG && !logsDebug) {
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class FenixLogSinkTest {
"test",
message = "test",
)
verify(exactly = 0) { androidLogSink.log(any(), any(), any()) }
verify(exactly = 0) { androidLogSink.log(any(), any(), any(), any()) }
}

@Test
Expand Down

0 comments on commit 1640dc0

Please sign in to comment.