Switch to Timber for logging (#617)

* Switch to Timber for logging Signed-off-by: Harsh Shandilya <msfjarvis@gmail.com> * Update changelog Signed-off-by: Harsh Shandilya <msfjarvis@gmail.com> * Fix rebase error Signed-off-by: Harsh Shandilya <msfjarvis@gmail.com>
2020-01-26 11:35:10 +05:30 · 2020-01-26 11:35:10 +05:30 · 2c78470642
commit 2c78470642
parent 14f1fa6538
13 changed files with 70 additions and 44 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -3,6 +3,9 @@ All notable changes to this project will be documented in this file.

 ## Unreleased

+### Changed
+- Logging is now enabled in release builds
+
 ### Fixed
 - OpenKeychain errors cause app crash

--- a/app/build.gradle
+++ b/app/build.gradle
@ -97,6 +97,7 @@ dependencies {
    implementation deps.third_party.jsch
    implementation deps.third_party.openpgp_ktx
    implementation deps.third_party.ssh_auth
+    implementation deps.third_party.timber

    // Testing-only dependencies
    androidTestImplementation deps.testing.junit
--- a/app/src/main/AndroidManifest.xml
+++ b/app/src/main/AndroidManifest.xml
@ -16,6 +16,7 @@
    <uses-permission android:name="android.permission.USE_FINGERPRINT" />

    <application
+        android:name=".Application"
        android:allowBackup="false"
        android:icon="@mipmap/ic_launcher"
        android:label="@string/app_name"
--- a/app/src/main/java/com/zeapo/pwdstore/Application.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/Application.kt
@ -0,0 +1,16 @@
+/*
+ * Copyright © 2014-2019 The Android Password Store Authors. All Rights Reserved.
+ * SPDX-License-Identifier: GPL-3.0-only
+ */
+package com.zeapo.pwdstore
+
+import timber.log.Timber
+
+@Suppress("Unused")
+class Application : android.app.Application() {
+
+    override fun onCreate() {
+        super.onCreate()
+        Timber.plant(Timber.DebugTree())
+    }
+}
--- a/app/src/main/java/com/zeapo/pwdstore/PasswordStore.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/PasswordStore.kt
@ -17,7 +17,6 @@ import android.graphics.drawable.Icon
 import android.os.Build
 import android.os.Bundle
 import android.text.TextUtils
-import android.util.Log
 import android.view.KeyEvent
 import android.view.Menu
 import android.view.MenuItem
@ -56,6 +55,7 @@ import org.apache.commons.io.FilenameUtils
 import org.eclipse.jgit.api.Git
 import org.eclipse.jgit.api.errors.GitAPIException
 import org.eclipse.jgit.revwalk.RevCommit
+import timber.log.Timber

 class PasswordStore : AppCompatActivity() {

@ -279,7 +279,7 @@ class PasswordStore : AppCompatActivity() {
        } catch (e: Exception) {
            e.printStackTrace()
            if (!localDir.delete()) {
-                Log.d(TAG, "Failed to delete local repository")
+                Timber.tag(TAG).d("Failed to delete local repository")
            }
            return
        }
@ -326,7 +326,7 @@ class PasswordStore : AppCompatActivity() {
        val fragmentManager = supportFragmentManager
        val fragmentTransaction = fragmentManager.beginTransaction()
        if (localDir != null && settings.getBoolean("repository_initialized", false)) {
-            Log.d(TAG, "Check, dir: " + localDir.absolutePath)
+            Timber.tag(TAG).d("Check, dir: ${localDir.absolutePath}")
            // do not push the fragment if we already have it
            if (fragmentManager.findFragmentByTag("PasswordsList") == null ||
                    settings.getBoolean("repo_changed", false)) {
@ -375,7 +375,7 @@ class PasswordStore : AppCompatActivity() {
        val repoPath = getRepositoryDirectory(this)
        val repository = getRepository(repoPath)
        if (repository == null) {
-            Log.d(TAG, "getLastChangedTimestamp: No git repository")
+            Timber.tag(TAG).d("getLastChangedTimestamp: No git repository")
            return File(fullPath).lastModified()
        }
        val git = Git(repository)
@ -384,11 +384,11 @@ class PasswordStore : AppCompatActivity() {
        iterator = try {
            git.log().addPath(relativePath).call().iterator()
        } catch (e: GitAPIException) {
-            Log.e(TAG, "getLastChangedTimestamp: GITAPIException", e)
+            Timber.tag(TAG).e(e, "getLastChangedTimestamp: GITAPIException")
            return -1
        }
        if (!iterator.hasNext()) {
-            Log.w(TAG, "getLastChangedTimestamp: No commits for file: $relativePath")
+            Timber.tag(TAG).w("getLastChangedTimestamp: No commits for file: $relativePath")
            return -1
        }
        return iterator.next().commitTime.toLong() * 1000
@ -455,7 +455,7 @@ class PasswordStore : AppCompatActivity() {
            return
        }
        val currentDir = currentDir
-        Log.i(TAG, "Adding file to : " + currentDir!!.absolutePath)
+        Timber.tag(TAG).i("Adding file to : ${currentDir!!.absolutePath}")
        val intent = Intent(this, PgpActivity::class.java)
        intent.putExtra("FILE_PATH", currentDir.absolutePath)
        intent.putExtra("REPO_PATH", getRepositoryDirectory(applicationContext).absolutePath)
@ -519,7 +519,7 @@ class PasswordStore : AppCompatActivity() {
    private fun commitChange(message: String) {
        object : GitOperation(getRepositoryDirectory(activity), activity) {
            override fun execute() {
-                Log.d(TAG, "Committing with message $message")
+                Timber.tag(TAG).d("Committing with message $message")
                val git = Git(repository)
                val tasks = GitAsyncTask(activity, false, true, this)
                tasks.execute(git.add().addFilepattern("."), git.commit().setAll(true).setMessage(message))
@ -589,13 +589,16 @@ class PasswordStore : AppCompatActivity() {
                    startActivityForResult(intent, GitActivity.REQUEST_CLONE)
                }
                REQUEST_CODE_SELECT_FOLDER -> {
-                    Log.d(TAG, "Moving passwords to " + data!!.getStringExtra("SELECTED_FOLDER_PATH"))
-                    Log.d(TAG, TextUtils.join(", ", requireNotNull(data.getStringArrayListExtra("Files"))))
+                    Timber.tag(TAG)
+                        .d("Moving passwords to ${data!!.getStringExtra("SELECTED_FOLDER_PATH")}")
+                    Timber.tag(TAG).d(
+                        TextUtils.join(", ", requireNotNull(data.getStringArrayListExtra("Files")))
+                    )

                    val target = File(requireNotNull(data.getStringExtra("SELECTED_FOLDER_PATH")))
                    val repositoryPath = getRepositoryDirectory(applicationContext).absolutePath
                    if (!target.isDirectory) {
-                        Log.e(TAG, "Tried moving passwords to a non-existing folder.")
+                        Timber.tag(TAG).e("Tried moving passwords to a non-existing folder.")
                        return
                    }

@ -603,7 +606,7 @@ class PasswordStore : AppCompatActivity() {
                    for (fileString in requireNotNull(data.getStringArrayListExtra("Files"))) {
                        val source = File(fileString)
                        if (!source.exists()) {
-                            Log.e(TAG, "Tried moving something that appears non-existent.")
+                            Timber.tag(TAG).e("Tried moving something that appears non-existent.")
                            continue
                        }
                        val destinationFile = File(target.absolutePath + "/" + source.name)
@ -611,7 +614,7 @@ class PasswordStore : AppCompatActivity() {
                        val sourceLongName = getLongName(requireNotNull(source.parent), repositoryPath, basename)
                        val destinationLongName = getLongName(target.absolutePath, repositoryPath, basename)
                        if (destinationFile.exists()) {
-                            Log.e(TAG, "Trying to move a file that already exists.")
+                            Timber.tag(TAG).e("Trying to move a file that already exists.")
                            // TODO: Add option to cancel overwrite. Will be easier once this is an async task.
                            MaterialAlertDialogBuilder(this)
                                    .setTitle(resources.getString(R.string.password_exists_title))
@ -625,7 +628,7 @@ class PasswordStore : AppCompatActivity() {
                        }
                        if (!source.renameTo(destinationFile)) {
                            // TODO this should show a warning to the user
-                            Log.e(TAG, "Something went wrong while moving.")
+                            Timber.tag(TAG).e("Something went wrong while moving.")
                        } else {
                            commitChange(this.resources
                                    .getString(
--- a/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/UserPreference.kt
@ -15,7 +15,6 @@ import android.os.Bundle
 import android.os.Environment
 import android.provider.DocumentsContract
 import android.provider.Settings
-import android.util.Log
 import android.view.MenuItem
 import android.view.accessibility.AccessibilityManager
 import android.widget.Toast
@ -47,6 +46,7 @@ import java.util.HashSet
 import java.util.TimeZone
 import me.msfjarvis.openpgpktx.util.OpenPgpUtils
 import org.apache.commons.io.FileUtils
+import timber.log.Timber

 typealias ClickListener = Preference.OnPreferenceClickListener
 typealias ChangeListener = Preference.OnPreferenceChangeListener
@ -477,13 +477,13 @@ class UserPreference : AppCompatActivity() {
                SELECT_GIT_DIRECTORY -> {
                    val uri = data.data

-                    Log.d(TAG, "Selected repository URI is $uri")
+                    Timber.tag(TAG).d("Selected repository URI is $uri")
                    // TODO: This is fragile. Workaround until PasswordItem is backed by DocumentFile
                    val docId = DocumentsContract.getTreeDocumentId(uri)
                    val split = docId.split(":".toRegex()).dropLastWhile { it.isEmpty() }.toTypedArray()
                    val repoPath = "${Environment.getExternalStorageDirectory()}/${split[1]}"

-                    Log.d(TAG, "Selected repository path is $repoPath")
+                    Timber.tag(TAG).d("Selected repository path is $repoPath")

                    if (Environment.getExternalStorageDirectory().path == repoPath) {
                        MaterialAlertDialogBuilder(this)
@ -532,7 +532,7 @@ class UserPreference : AppCompatActivity() {
        val repositoryDirectory = requireNotNull(PasswordRepository.getRepositoryDirectory(applicationContext))
        val sourcePassDir = DocumentFile.fromFile(repositoryDirectory)

-        Log.d(TAG, "Copying ${repositoryDirectory.path} to $targetDirectory")
+        Timber.tag(TAG).d("Copying ${repositoryDirectory.path} to $targetDirectory")

        val dateString = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
            LocalDateTime
--- a/app/src/main/java/com/zeapo/pwdstore/autofill/AutofillActivity.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/autofill/AutofillActivity.kt
@ -10,13 +10,13 @@ import android.content.Intent
 import android.content.IntentSender
 import android.content.SharedPreferences
 import android.os.Bundle
-import android.util.Log
 import androidx.appcompat.app.AppCompatActivity
 import com.zeapo.pwdstore.PasswordStore
 import com.zeapo.pwdstore.utils.splitLines
 import java.util.ArrayList
 import java.util.Arrays
 import org.eclipse.jgit.util.StringUtils
+import timber.log.Timber

 // blank activity started by service for calling startIntentSenderForResult
 class AutofillActivity : AppCompatActivity() {
@ -30,7 +30,7 @@ class AutofillActivity : AppCompatActivity() {
                val pi = extras.getParcelable<PendingIntent>("pending_intent") ?: return
                startIntentSenderForResult(pi.intentSender, REQUEST_CODE_DECRYPT_AND_VERIFY, null, 0, 0, 0)
            } catch (e: IntentSender.SendIntentException) {
-                Log.e(AutofillService.Constants.TAG, "SendIntentException", e)
+                Timber.tag(AutofillService.Constants.TAG).e(e, "SendIntentException")
            }
        } else if (extras != null && extras.containsKey("pick")) {
            val intent = Intent(applicationContext, PasswordStore::class.java)
--- a/app/src/main/java/com/zeapo/pwdstore/autofill/AutofillService.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/autofill/AutofillService.kt
@ -17,7 +17,6 @@ import android.net.Uri
 import android.os.Build
 import android.os.Bundle
 import android.provider.Settings
-import android.util.Log
 import android.view.ViewGroup.LayoutParams.WRAP_CONTENT
 import android.view.WindowManager
 import android.view.accessibility.AccessibilityEvent
@ -45,6 +44,7 @@ import me.msfjarvis.openpgpktx.util.OpenPgpServiceConnection
 import org.apache.commons.io.FileUtils
 import org.openintents.openpgp.IOpenPgpService2
 import org.openintents.openpgp.OpenPgpError
+import timber.log.Timber

 class AutofillService : AccessibilityService() {
    private var serviceConnection: OpenPgpServiceConnection? = null
@ -518,11 +518,11 @@ class AutofillService : AccessibilityService() {
                        lastPasswordMaxDate = System.currentTimeMillis() + ttl * 1000L
                    }
                } catch (e: UnsupportedEncodingException) {
-                    Log.e(Constants.TAG, "UnsupportedEncodingException", e)
+                    Timber.tag(Constants.TAG).e(e, "UnsupportedEncodingException")
                }
            }
            OpenPgpApi.RESULT_CODE_USER_INTERACTION_REQUIRED -> {
-                Log.i("PgpHandler", "RESULT_CODE_USER_INTERACTION_REQUIRED")
+                Timber.tag("PgpHandler").i("RESULT_CODE_USER_INTERACTION_REQUIRED")
                val pi = result.getParcelableExtra<PendingIntent>(OpenPgpApi.RESULT_INTENT)
                // need to start a blank activity to call startIntentSenderForResult
                val intent = Intent(this@AutofillService, AutofillActivity::class.java)
@ -536,8 +536,8 @@ class AutofillService : AccessibilityService() {
                    Toast.makeText(this@AutofillService,
                            "Error from OpenKeyChain : " + error.message,
                            Toast.LENGTH_LONG).show()
-                    Log.e(Constants.TAG, "onError getErrorId:" + error.errorId)
-                    Log.e(Constants.TAG, "onError getMessage:" + error.message)
+                    Timber.tag(Constants.TAG).e("onError getErrorId: ${error.errorId}")
+                    Timber.tag(Constants.TAG).e("onError getMessage: ${error.message}")
                }
            }
        }
--- a/app/src/main/java/com/zeapo/pwdstore/crypto/PgpActivity.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/crypto/PgpActivity.kt
@ -19,7 +19,6 @@ import android.os.Handler
 import android.text.TextUtils
 import android.text.format.DateUtils
 import android.text.method.PasswordTransformationMethod
-import android.util.Log
 import android.view.LayoutInflater
 import android.view.Menu
 import android.view.MenuItem
@ -67,6 +66,7 @@ import org.apache.commons.io.FileUtils
 import org.apache.commons.io.FilenameUtils
 import org.openintents.openpgp.IOpenPgpService2
 import org.openintents.openpgp.OpenPgpError
+import timber.log.Timber

 class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound {
    private val clipboard: ClipboardManager by lazy {
@ -103,6 +103,7 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        window.setFlags(WindowManager.LayoutParams.FLAG_SECURE, WindowManager.LayoutParams.FLAG_SECURE)
+        Timber.tag(TAG)

        // some persistence
        val providerPackageName = settings.getString("openpgp_provider_list", "")
@ -202,7 +203,7 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound {
     * @param requestCode The code we'd like to use to identify the behaviour
     */
    private fun handleUserInteractionRequest(result: Intent, requestCode: Int) {
-        Log.i(TAG, "RESULT_CODE_USER_INTERACTION_REQUIRED")
+        Timber.i("RESULT_CODE_USER_INTERACTION_REQUIRED")

        val pi: PendingIntent? = result.getParcelableExtra(RESULT_INTENT)
        try {
@ -211,7 +212,7 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound {
                    null, 0, 0, 0
            )
        } catch (e: IntentSender.SendIntentException) {
-            Log.e(TAG, "SendIntentException", e)
+            Timber.e(e, "SendIntentException")
        }
    }

@ -230,8 +231,8 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound {
        val error: OpenPgpError? = result.getParcelableExtra(RESULT_ERROR)
        if (error != null) {
            showSnackbar("Error from OpenKeyChain : " + error.message)
-            Log.e(TAG, "onError getErrorId:" + error.message)
-            Log.e(TAG, "onError getMessage:" + error.message)
+            Timber.e("onError getErrorId: ${error.errorId}")
+            Timber.e("onError getMessage: ${error.message}")
        }
    }

@ -408,7 +409,7 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound {
                                    copyPasswordToClipBoard()
                                }
                            } catch (e: Exception) {
-                                Log.e(TAG, "An Exception occurred", e)
+                                Timber.e(e, "An Exception occurred")
                            }
                        }
                        RESULT_CODE_USER_INTERACTION_REQUIRED -> handleUserInteractionRequest(result, REQUEST_DECRYPT)
@ -482,7 +483,7 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound {
                                setResult(RESULT_OK, returnIntent)
                                finish()
                            } catch (e: Exception) {
-                                Log.e(TAG, "An Exception occurred", e)
+                                Timber.e(e, "An Exception occurred")
                            }
                        }
                        RESULT_CODE_ERROR -> handleError(result)
@ -584,7 +585,7 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound {
                                setResult(RESULT_OK)
                                finish()
                            } catch (e: Exception) {
-                                Log.e(TAG, "An Exception occurred", e)
+                                Timber.e(e, "An Exception occurred")
                            }
                        }
                        RESULT_CODE_USER_INTERACTION_REQUIRED -> handleUserInteractionRequest(result, REQUEST_KEY_ID)
@ -806,7 +807,7 @@ class PgpActivity : AppCompatActivity(), OpenPgpServiceConnection.OnBound {
            checkAndIncrementHotp()

            // No need to validate clear_after_copy. It was validated in copyPasswordToClipBoard()
-            Log.d("DELAY_SHOW", "Clearing the clipboard")
+            Timber.d("Clearing the clipboard")
            val clip = ClipData.newPlainText("pgp_handler_result_pm", "")
            clipboard.setPrimaryClip(clip)
            if (settings.getBoolean("clear_clipboard_20x", false)) {
--- a/app/src/main/java/com/zeapo/pwdstore/git/GitActivity.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/git/GitActivity.kt
@ -10,7 +10,6 @@ import android.content.SharedPreferences
 import android.os.Bundle
 import android.text.Editable
 import android.text.TextWatcher
-import android.util.Log
 import android.view.Menu
 import android.view.MenuItem
 import android.view.View
@ -32,6 +31,7 @@ import java.io.IOException
 import java.util.regex.Pattern
 import org.apache.commons.io.FileUtils
 import org.eclipse.jgit.lib.Constants
+import timber.log.Timber

 open class GitActivity : AppCompatActivity() {
    private lateinit var context: Context
@ -596,7 +596,7 @@ open class GitActivity : AppCompatActivity() {
                SshApiSessionFactory.POST_SIGNATURE -> return

                else -> {
-                    Log.e(TAG, "Operation not recognized : $operation")
+                    Timber.tag(TAG).e("Operation not recognized : $operation")
                    setResult(AppCompatActivity.RESULT_CANCELED)
                    finish()
                    return
--- a/app/src/main/java/com/zeapo/pwdstore/utils/Otp.java
+++ b/app/src/main/java/com/zeapo/pwdstore/utils/Otp.java
@ -4,7 +4,6 @@
 */
 package com.zeapo.pwdstore.utils;

-import android.util.Log;
 import java.math.BigInteger;
 import java.nio.ByteBuffer;
 import java.security.InvalidKeyException;
@ -13,6 +12,7 @@ import java.util.Arrays;
 import javax.crypto.Mac;
 import javax.crypto.spec.SecretKeySpec;
 import org.apache.commons.codec.binary.Base32;
+import timber.log.Timber;

 public class Otp {

@ -34,10 +34,10 @@ public class Otp {
            mac = Mac.getInstance(ALGORITHM);
            mac.init(signingKey);
        } catch (NoSuchAlgorithmException e) {
-            Log.e("TOTP", ALGORITHM + " unavailable - should never happen", e);
+            Timber.tag("TOTP").e(e, "%s unavailable - should never happen", ALGORITHM);
            return null;
        } catch (InvalidKeyException e) {
-            Log.e("TOTP", "Key is malformed", e);
+            Timber.tag("TOTP").e(e, "Key is malformed");
            return null;
        }

--- a/app/src/main/java/com/zeapo/pwdstore/utils/auth/Authenticator.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/utils/auth/Authenticator.kt
@ -5,11 +5,11 @@
 package com.zeapo.pwdstore.utils.auth

 import android.os.Handler
-import android.util.Log
 import androidx.biometric.BiometricManager
 import androidx.biometric.BiometricPrompt
 import androidx.fragment.app.FragmentActivity
 import com.zeapo.pwdstore.R
+import timber.log.Timber

 internal class Authenticator(
    private val fragmentActivity: FragmentActivity,
@ -22,19 +22,19 @@ internal class Authenticator(

        override fun onAuthenticationError(errorCode: Int, errString: CharSequence) {
            super.onAuthenticationError(errorCode, errString)
-            Log.d(TAG, "Error: $errorCode: $errString")
+            Timber.tag(TAG).d("Error: $errorCode: $errString")
            callback(AuthenticationResult.UnrecoverableError(errorCode, errString))
        }

        override fun onAuthenticationFailed() {
            super.onAuthenticationFailed()
-            Log.d(TAG, "Failed")
+            Timber.tag(TAG).d("Failed")
            callback(AuthenticationResult.Failure)
        }

        override fun onAuthenticationSucceeded(result: BiometricPrompt.AuthenticationResult) {
            super.onAuthenticationSucceeded(result)
-            Log.d(TAG, "Success")
+            Timber.tag(TAG).d("Success")
            callback(AuthenticationResult.Success(result.cryptoObject))
        }
    }
--- a/dependencies.gradle
+++ b/dependencies.gradle
@ -41,7 +41,8 @@ ext.deps = [
        jsch: 'com.jcraft:jsch:0.1.55',
        jgit: 'org.eclipse.jgit:org.eclipse.jgit:3.7.1.201504261725-r',
        openpgp_ktx: 'com.github.android-password-store:openpgp-ktx:1.2.0',
-        ssh_auth: 'org.sufficientlysecure:sshauthentication-api:1.0'
+        ssh_auth: 'org.sufficientlysecure:sshauthentication-api:1.0',
+        timber: "com.jakewharton.timber:timber:4.7.1",
    ],

    testing: [