Fix log reporting for release builds (#1916)

* Fix log reporting for release builds

* Fix logs for release builds

* wip

* Clean up the branch to exclude unrelated changes

* With java docs

* Uncomment quiz checker

* Check for external storage permissions before sending logs

* With more java docs

* Fix crash while zipping log files

* Do not log token and cookies

* Add instruction to restart app

app/build.gradle

@@ -10,16 +10,24 @@ dependencies {
     implementation 'fr.avianey.com.viewpagerindicator:library:2.4.1.1@aar'
     implementation 'in.yuvi:http.fluent:1.3'
     implementation 'com.github.chrisbanes:PhotoView:2.0.0'
+
     implementation 'ch.acra:acra:4.9.2'
+
     implementation 'org.mediawiki:api:1.3'
     implementation 'commons-codec:commons-codec:1.10'
     implementation 'com.github.pedrovgs:renderers:3.3.3'
     implementation 'com.google.code.gson:gson:2.8.5'
-    implementation 'com.jakewharton.timber:timber:4.6.0'
+    implementation 'com.jakewharton.timber:timber:4.4.0'
     implementation 'info.debatty:java-string-similarity:0.24'
     implementation 'com.borjabravo:readmoretextview:2.1.0'
 
     implementation 'com.android.support.constraint:constraint-layout:1.1.3'
+
+    implementation 'org.slf4j:slf4j-api:1.7.25'
+    api ("com.github.tony19:logback-android-classic:1.1.1-6") {
+        exclude group: 'com.google.android', module: 'android'
+    }
+
     implementation('com.mapbox.mapboxsdk:mapbox-android-sdk:5.5.0@aar') {
         transitive = true
     }

app/prod/release/output.json

@@ -0,0 +1 @@
+[{"outputType":{"type":"APK"},"apkInfo":{"type":"MAIN","splits":[],"versionCode":90,"versionName":"2.8.3","enabled":true,"outputFile":"app-commons-v2.8.3-acra-prod-release.apk","fullName":"prodRelease","baseName":"prod-release"},"path":"app-commons-v2.8.3-acra-prod-release.apk","properties":{}}]

app/proguard-rules.txt

@@ -56,3 +56,17 @@
 -keep class * implements com.google.gson.JsonSerializer
 -keep class * implements com.google.gson.JsonDeserializer
 # --- /Gson ---
+
+
+# --- /logback ---
+
+-keep class ch.qos.** { *; }
+-keep class org.slf4j.** { *; }
+-keepattributes *Annotation*
+
+-dontwarn ch.qos.logback.core.net.*
+
+# --- /acra ---
+-keep class org.acra.** { *; }
+-keepattributes SourceFile,LineNumberTable
+-keepattributes *Annotation*

app/src/main/AndroidManifest.xml

@@ -122,6 +122,8 @@
             android:name=".achievements.AchievementsActivity"
             android:label="@string/Achievements" />
 
+        <activity android:name="com.github.pedrovgs.lynx.LynxActivity"/>
+
         <service android:name=".upload.UploadService" />
         <service
             android:name=".auth.WikiAccountAuthenticatorService"
@@ -157,6 +159,11 @@
                 android:resource="@xml/modifications_sync_adapter" />
         </service>
 
+        <service
+            android:name="org.acra.sender.SenderService"
+            android:exported="false"
+            android:process=":acra" />
+
         <provider
             android:name="android.support.v4.content.FileProvider"
             android:authorities="${applicationId}.provider"

app/src/main/java/fr/free/nrw/commons/CommonsApplication.java

@@ -8,7 +8,9 @@ import android.content.Context;
 import android.content.SharedPreferences;
 import android.database.sqlite.SQLiteDatabase;
 import android.os.Build;
+import android.os.Process;
 import android.support.annotation.NonNull;
+import android.util.Log;
 
 import com.facebook.drawee.backends.pipeline.Fresco;
 import com.facebook.imagepipeline.core.ImagePipelineConfig;
@@ -28,9 +30,13 @@ import javax.inject.Named;
 
 import fr.free.nrw.commons.auth.SessionManager;
 import fr.free.nrw.commons.category.CategoryDao;
+import fr.free.nrw.commons.concurrency.BackgroundPoolExceptionHandler;
+import fr.free.nrw.commons.concurrency.ThreadPoolService;
 import fr.free.nrw.commons.contributions.ContributionDao;
 import fr.free.nrw.commons.data.DBOpenHelper;
 import fr.free.nrw.commons.di.ApplicationlessInjection;
+import fr.free.nrw.commons.logging.FileLoggingTree;
+import fr.free.nrw.commons.logging.LogUtils;
 import fr.free.nrw.commons.modifications.ModifierSequenceDao;
 import fr.free.nrw.commons.upload.FileUtils;
 import fr.free.nrw.commons.utils.ContributionUtils;
@@ -38,7 +44,6 @@ import io.reactivex.android.schedulers.AndroidSchedulers;
 import io.reactivex.schedulers.Schedulers;
 import timber.log.Timber;
 
-// TODO: Use ProGuard to rip out reporting when publishing
 @ReportsCrashes(
         mailTo = "commons-app-android-private@googlegroups.com",
         mode = ReportingInteractionMode.DIALOG,
@@ -48,13 +53,15 @@ import timber.log.Timber;
         resDialogOkToast = R.string.crash_dialog_ok_toast
 )
 public class CommonsApplication extends Application {
-
     @Inject SessionManager sessionManager;
     @Inject DBOpenHelper dbOpenHelper;
 
     @Inject @Named("default_preferences") SharedPreferences defaultPrefs;
     @Inject @Named("application_preferences") SharedPreferences applicationPrefs;
     @Inject @Named("prefs") SharedPreferences otherPrefs;
+    @Inject
+    @Named("isBeta")
+    boolean isBeta;
 
     /**
      * Constants begin
@@ -67,10 +74,6 @@ public class CommonsApplication extends Application {
 
     public static final String FEEDBACK_EMAIL_SUBJECT = "Commons Android App (%s) Feedback";
 
-    public static final String LOGS_PRIVATE_EMAIL = "commons-app-android-private@googlegroups.com";
-
-    public static final String LOGS_PRIVATE_EMAIL_SUBJECT = "Commons Android App (%s) Logs";
-
     public static final String NOTIFICATION_CHANNEL_ID_ALL = "CommonsNotificationAll";
 
     /**
@@ -97,7 +100,7 @@ public class CommonsApplication extends Application {
                 .getCommonsApplicationComponent()
                 .inject(this);
 
-        Timber.plant(new Timber.DebugTree());
+        initTimber();
 
 //        Set DownsampleEnabled to True to downsample the image in case it's heavy
         ImagePipelineConfig config = ImagePipelineConfig.newBuilder(this)
@@ -109,24 +112,62 @@ public class CommonsApplication extends Application {
             Timber.e(e);
             // TODO: Remove when we're able to initialize Fresco in test builds.
         }
-        if (setupLeakCanary() == RefWatcher.DISABLED) {
+
-            return;
-        }
         // Empty temp directory in case some temp files are created and never removed.
         ContributionUtils.emptyTemporaryDirectory();
 
-        if (!BuildConfig.DEBUG) {
+        initAcra();
-            ACRA.init(this);
+        if (BuildConfig.DEBUG) {
-        } else {
             Stetho.initializeWithDefaults(this);
         }
 
         createNotificationChannel(this);
 
+
+        if (setupLeakCanary() == RefWatcher.DISABLED) {
+            return;
+        }
         // Fire progress callbacks for every 3% of uploaded content
         System.setProperty("in.yuvi.http.fluent.PROGRESS_TRIGGER_THRESHOLD", "3.0");
     }
 
+    /**
+     * Plants debug and file logging tree.
+     * Timber lets you plant your own logging trees.
+     *
+     */
+    private void initTimber() {
+        String logFileName = isBeta ? "CommonsBetaAppLogs" : "CommonsAppLogs";
+        String logDirectory = LogUtils.getLogDirectory(isBeta);
+        FileLoggingTree tree = new FileLoggingTree(
+                Log.DEBUG,
+                logFileName,
+                logDirectory,
+                1000,
+                getFileLoggingThreadPool());
+
+        Timber.plant(tree);
+        Timber.plant(new Timber.DebugTree());
+    }
+
+    /**
+     * Remove ACRA's UncaughtExceptionHandler
+     * We do this because ACRA's handler spawns a new process possibly screwing up with a few things
+     */
+    private void initAcra() {
+        Thread.UncaughtExceptionHandler exceptionHandler = Thread.getDefaultUncaughtExceptionHandler();
+        ACRA.init(this);
+        Thread.setDefaultUncaughtExceptionHandler(exceptionHandler);
+    }
+
+    private ThreadPoolService getFileLoggingThreadPool() {
+        return new ThreadPoolService.Builder("file-logging-thread")
+                .setPriority(Process.THREAD_PRIORITY_LOWEST)
+                .setPoolSize(1)
+                .setExceptionHandler(new BackgroundPoolExceptionHandler())
+                .build();
+    }
+
     public static void createNotificationChannel(@NonNull Context context) {
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
             NotificationManager manager = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE);

app/src/main/java/fr/free/nrw/commons/Utils.java

@@ -148,39 +148,6 @@ public class Utils {
         return PreferenceManager.getDefaultSharedPreferences(context).getBoolean("theme", false);
     }
 
-    /**
-     * Will be used to fetch the logs generated by the app ever since the beginning of times....
-     * i.e. since the time the app started.
-     *
-     * @return String containing all the logs since the time the app started
-     */
-    public static String getAppLogs() {
-        final String processId = Integer.toString(android.os.Process.myPid());
-
-        StringBuilder stringBuilder = new StringBuilder();
-
-        try {
-            String[] command = new String[]{"logcat","-d","-v","threadtime"};
-
-            Process process = Runtime.getRuntime().exec(command);
-
-            BufferedReader bufferedReader = new BufferedReader(
-                    new InputStreamReader(process.getInputStream())
-            );
-
-            String line;
-            while ((line = bufferedReader.readLine()) != null) {
-                if (line.contains(processId)) {
-                    stringBuilder.append(line);
-                }
-            }
-        } catch (IOException ioe) {
-            Timber.e("getAppLogs failed", ioe);
-        }
-
-        return stringBuilder.toString();
-    }
-
     public static void rateApp(Context context) {
         final String appPackageName = BuildConfig.class.getPackage().getName();
         try {

app/src/main/java/fr/free/nrw/commons/concurrency/BackgroundPoolExceptionHandler.java

@@ -0,0 +1,23 @@
+package fr.free.nrw.commons.concurrency;
+
+import android.support.annotation.NonNull;
+
+import fr.free.nrw.commons.BuildConfig;
+
+public class BackgroundPoolExceptionHandler implements ExceptionHandler {
+    /**
+     * If an exception occurs on a background thread, this handler will crash for debug builds
+     * but fail silently for release builds.
+     * @param t
+     */
+    @Override
+    public void onException(@NonNull final Throwable t) {
+        //Crash for debug build
+        if (BuildConfig.DEBUG) {
+            Thread thread = new Thread(() -> {
+                throw new RuntimeException(t);
+            });
+            thread.start();
+        }
+    }
+}

app/src/main/java/fr/free/nrw/commons/concurrency/ExceptionAwareThreadPoolExecutor.java

@@ -0,0 +1,39 @@
+package fr.free.nrw.commons.concurrency;
+
+import java.util.concurrent.CancellationException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.ThreadFactory;
+
+class ExceptionAwareThreadPoolExecutor extends ScheduledThreadPoolExecutor {
+
+    private final ExceptionHandler exceptionHandler;
+
+    public ExceptionAwareThreadPoolExecutor(int corePoolSize, ThreadFactory threadFactory,
+                                            ExceptionHandler exceptionHandler) {
+        super(corePoolSize, threadFactory);
+        this.exceptionHandler = exceptionHandler;
+    }
+
+    @Override
+    protected void afterExecute(Runnable r, Throwable t) {
+        super.afterExecute(r, t);
+        if (t == null && r instanceof Future<?>) {
+            try {
+                Future<?> future = (Future<?>) r;
+                if (future.isDone()) future.get();
+            } catch (CancellationException | InterruptedException e) {
+                //ignore
+            } catch (ExecutionException e) {
+                t = e.getCause() != null ? e.getCause() : e;
+            } catch (Exception e) {
+                t = e;
+            }
+        }
+
+        if (t != null) {
+            exceptionHandler.onException(t);
+        }
+    }
+}

app/src/main/java/fr/free/nrw/commons/concurrency/ExceptionHandler.java

@@ -0,0 +1,7 @@
+package fr.free.nrw.commons.concurrency;
+
+import android.support.annotation.NonNull;
+
+public interface ExceptionHandler {
+    void onException(@NonNull Throwable t);
+}

app/src/main/java/fr/free/nrw/commons/concurrency/ThreadPoolService.java

@@ -0,0 +1,124 @@
+package fr.free.nrw.commons.concurrency;
+
+import android.support.annotation.NonNull;
+
+import java.util.concurrent.Callable;
+import java.util.concurrent.Executor;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.ThreadFactory;
+import java.util.concurrent.TimeUnit;
+
+/**
+ * This class is a thread pool which provides some additional features:
+ * - it sets the thread priority to a value lower than foreground priority by default, or you can
+ * supply your own priority
+ * - it gives you a way to handle exceptions thrown in the thread pool
+ */
+
+public class ThreadPoolService implements Executor {
+    private final ScheduledThreadPoolExecutor backgroundPool;
+
+    private ThreadPoolService(final Builder b) {
+        backgroundPool = new ExceptionAwareThreadPoolExecutor(b.poolSize,
+                new ThreadFactory() {
+                    int count = 0;
+                    @Override
+                    public Thread newThread(@NonNull Runnable r) {
+                        count++;
+                        Thread t = new Thread(r, String.format("%s-%s", b.name, count));
+                        //If the priority is specified out of range, we set the thread priority to Thread.MIN_PRIORITY
+                        //It's done prevent IllegalArgumentException and to prevent setting of improper high priority for a less priority task
+                        t.setPriority(b.priority > Thread.MAX_PRIORITY || b.priority < Thread.MIN_PRIORITY ?
+                                Thread.MIN_PRIORITY : b.priority);
+                        return t;
+                    }
+                }, b.exceptionHandler);
+    }
+
+    public <V> ScheduledFuture<V> schedule(Callable<V> callable, long time, TimeUnit timeUnit) {
+        return backgroundPool.schedule(callable, time, timeUnit);
+    }
+
+    public ScheduledFuture<?> schedule(Runnable runnable) {
+        return schedule(runnable, 0, TimeUnit.SECONDS);
+    }
+
+    public ScheduledFuture<?> schedule(Runnable runnable, long time, TimeUnit timeUnit) {
+        return backgroundPool.schedule(runnable, time, timeUnit);
+    }
+
+    public ScheduledFuture<?> scheduleAtFixedRate(final Runnable task, long initialDelay,
+                                                  long period, final TimeUnit timeUnit) {
+        return backgroundPool.scheduleAtFixedRate(task, initialDelay, period, timeUnit);
+    }
+
+    public ScheduledThreadPoolExecutor executor() {
+        return backgroundPool;
+    }
+
+    public void shutdown(){
+        backgroundPool.shutdown();
+    }
+
+    @Override
+    public void execute(Runnable command) {
+        backgroundPool.execute(command);
+    }
+
+    /**
+     * Builder class for {@link ThreadPoolService}
+     */
+    public static class Builder {
+        //Required
+        private final String name;
+
+        //Optional
+        private int poolSize = 1;
+        private int priority = Thread.MIN_PRIORITY;
+        private ExceptionHandler exceptionHandler = null;
+
+        /**
+         * @param name the name of the threads in the service. if there are N threads,
+         *             the thread names will be like name-1, name-2, name-3,...,name-N
+         */
+        public Builder(@NonNull String name) {
+            this.name = name;
+        }
+
+        /**
+         * @param poolSize the number of threads to keep in the pool
+         * @throws IllegalArgumentException if size of pool <=0
+         */
+        public Builder setPoolSize(int poolSize) throws IllegalArgumentException {
+            if (poolSize <= 0) {
+                throw new IllegalArgumentException("Pool size must be grater than 0");
+            }
+            this.poolSize = poolSize;
+            return this;
+        }
+
+        /**
+         * @param priority Priority of the threads in the service. You can supply a constant from
+         *                 {@link java.lang.Thread} or
+         *                 specify your own priority in the range 1(MIN_PRIORITY) to 10(MAX_PRIORITY)
+         *                 By default, the priority is set to {@link java.lang.Thread#MIN_PRIORITY}
+         */
+        public Builder setPriority(int priority) {
+            this.priority = priority;
+            return this;
+        }
+
+        /**
+         * @param handler The handler to use to handle exceptions in the service
+         */
+        public Builder setExceptionHandler(ExceptionHandler handler) {
+            this.exceptionHandler = handler;
+            return this;
+        }
+
+        public ThreadPoolService build() {
+            return new ThreadPoolService(this);
+        }
+    }
+}

app/src/main/java/fr/free/nrw/commons/di/CommonsApplicationModule.java

@@ -150,4 +150,15 @@ public class CommonsApplicationModule {
     public WikidataEditListener provideWikidataEditListener() {
         return new WikidataEditListenerImpl();
     }
+
+    /**
+     * Provides app flavour. Can be used to alter flows in the app
+     * @return
+     */
+    @Named("isBeta")
+    @Provides
+    @Singleton
+    public boolean provideIsBetaVariant() {
+        return BuildConfig.FLAVOR.equals("beta");
+    }
 }

app/src/main/java/fr/free/nrw/commons/explore/recentsearches/RecentSearchesDao.java

@@ -7,7 +7,6 @@ import android.database.sqlite.SQLiteDatabase;
 import android.os.RemoteException;
 import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
-import android.util.Log;
 
 import java.util.ArrayList;
 import java.util.Date;
@@ -17,6 +16,8 @@ import javax.inject.Inject;
 import javax.inject.Named;
 import javax.inject.Provider;
 
+import timber.log.Timber;
+
 /**
  * This class doesn't execute queries in database directly instead it contains the logic behind
  * inserting, deleting, searching data from recent searches database.
@@ -62,12 +63,12 @@ public class RecentSearchesDao {
                 if (recentSearch.getContentUri() == null) {
                     throw new RuntimeException("tried to delete item with no content URI");
                 } else {
-                    Log.d("QUERY_NAME",recentSearch.getContentUri()+"- delete tried");
+                    Timber.d("QUERY_NAME %s - delete tried", recentSearch.getContentUri());
                     db.delete(recentSearch.getContentUri(), null, null);
-                    Log.d("QUERY_NAME",recentSearch.getQuery()+"- query deleted");
+                    Timber.d("QUERY_NAME %s - query deleted", recentSearch.getQuery());
                 }
             } catch (RemoteException e) {
-                Log.d("Exception",e+"- query deleted");
+                Timber.e(e, "query deleted");
                 throw new RuntimeException(e);
             } finally {
                 db.release();

app/src/main/java/fr/free/nrw/commons/logging/CommonsLogSender.java

@@ -0,0 +1,45 @@
+package fr.free.nrw.commons.logging;
+
+import javax.inject.Inject;
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import fr.free.nrw.commons.BuildConfig;
+import fr.free.nrw.commons.auth.SessionManager;
+
+/**
+ * Class responsible for sending logs to developers
+ */
+@Singleton
+public class CommonsLogSender extends LogsSender {
+    private static final String LOGS_PRIVATE_EMAIL = "commons-app-android-private@googlegroups.com";
+    private static final String LOGS_PRIVATE_EMAIL_SUBJECT = "Commons Android App (%s) Logs";
+    private static final String BETA_LOGS_PRIVATE_EMAIL_SUBJECT = "Commons Beta Android App (%s) Logs";
+
+    @Inject
+    public CommonsLogSender(SessionManager sessionManager,
+                            @Named("isBeta") boolean isBeta) {
+        super(sessionManager, isBeta);
+
+        this.logFileName = isBeta ? "CommonsBetaAppLogs.zip" : "CommonsAppLogs.zip";
+        String emailSubjectFormat = isBeta ? BETA_LOGS_PRIVATE_EMAIL_SUBJECT : LOGS_PRIVATE_EMAIL_SUBJECT;
+        String message = String.format(emailSubjectFormat, sessionManager.getUserName());
+        this.emailSubject = message;
+        this.emailBody = message;
+        this.mailTo = LOGS_PRIVATE_EMAIL;
+    }
+
+    /**
+     * Attach any extra meta information about user or device that might help in debugging
+     * @return
+     */
+    @Override
+    protected String getExtraInfo() {
+        StringBuilder builder = new StringBuilder();
+        builder.append("App Version Name: ")
+                .append(BuildConfig.VERSION_NAME)
+                .append("\n");
+
+        return builder.toString();
+    }
+}

app/src/main/java/fr/free/nrw/commons/logging/FileLoggingTree.java

@@ -0,0 +1,144 @@
+package fr.free.nrw.commons.logging;
+
+import android.support.annotation.NonNull;
+import android.util.Log;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Locale;
+import java.util.concurrent.Executor;
+
+import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.encoder.PatternLayoutEncoder;
+import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.rolling.FixedWindowRollingPolicy;
+import ch.qos.logback.core.rolling.RollingFileAppender;
+import ch.qos.logback.core.rolling.SizeBasedTriggeringPolicy;
+import timber.log.Timber;
+
+/**
+ * Extends Timber's debug tree to write logs to a file
+ */
+public class FileLoggingTree extends Timber.DebugTree implements LogLevelSettableTree {
+    private final Logger logger = LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
+    private int logLevel;
+    private final String logFileName;
+    private int fileSize;
+    private FixedWindowRollingPolicy rollingPolicy;
+    private final Executor executor;
+
+    public FileLoggingTree(int logLevel,
+                           String logFileName,
+                           String logDirectory,
+                           int fileSizeInKb,
+                           Executor executor) {
+        this.logLevel = logLevel;
+        this.logFileName = logFileName;
+        this.fileSize = fileSizeInKb;
+        configureLogger(logDirectory);
+        this.executor = executor;
+    }
+
+    /**
+     * Can be overridden to change file's log level
+     * @param logLevel
+     */
+    @Override
+    public void setLogLevel(int logLevel) {
+        this.logLevel = logLevel;
+    }
+
+    /**
+     * Check and log any message
+     * @param priority
+     * @param tag
+     * @param message
+     * @param t
+     */
+    @Override
+    protected void log(final int priority, final String tag, @NonNull final String message, Throwable t) {
+        executor.execute(() -> logMessage(priority, tag, message));
+
+    }
+
+    /**
+     * Log any message based on the priority
+     * @param priority
+     * @param tag
+     * @param message
+     */
+    private void logMessage(int priority, String tag, String message) {
+        String messageWithTag = String.format("[%s] : %s", tag, message);
+        switch (priority) {
+            case Log.VERBOSE:
+                logger.trace(messageWithTag);
+                break;
+            case Log.DEBUG:
+                logger.debug(messageWithTag);
+                break;
+            case Log.INFO:
+                logger.info(messageWithTag);
+                break;
+            case Log.WARN:
+                logger.warn(messageWithTag);
+                break;
+            case Log.ERROR:
+                logger.error(messageWithTag);
+                break;
+            case Log.ASSERT:
+                logger.error(messageWithTag);
+                break;
+        }
+    }
+
+    /**
+     * Checks if a particular log line should be logged in the file or not
+     * @param priority
+     * @return
+     */
+    @Override
+    protected boolean isLoggable(int priority) {
+        return priority >= logLevel;
+    }
+
+    /**
+     * Configures the logger with a file size rolling policy (SizeBasedTriggeringPolicy)
+     * https://github.com/tony19/logback-android/wiki
+     * @param logDir
+     */
+    private void configureLogger(String logDir) {
+        LoggerContext loggerContext = (LoggerContext) LoggerFactory.getILoggerFactory();
+        loggerContext.reset();
+
+        RollingFileAppender<ILoggingEvent> rollingFileAppender = new RollingFileAppender<>();
+        rollingFileAppender.setContext(loggerContext);
+        rollingFileAppender.setFile(logDir + "/" + logFileName + ".0.log");
+
+        rollingPolicy = new FixedWindowRollingPolicy();
+        rollingPolicy.setContext(loggerContext);
+        rollingPolicy.setMinIndex(1);
+        rollingPolicy.setMaxIndex(4);
+        rollingPolicy.setParent(rollingFileAppender);
+        rollingPolicy.setFileNamePattern(logDir + "/" + logFileName + ".%i.log");
+        rollingPolicy.start();
+
+        SizeBasedTriggeringPolicy<ILoggingEvent> triggeringPolicy = new SizeBasedTriggeringPolicy<>();
+        triggeringPolicy.setContext(loggerContext);
+        triggeringPolicy.setMaxFileSize(String.format(Locale.ENGLISH, "%dKB", fileSize));
+        triggeringPolicy.start();
+
+        PatternLayoutEncoder encoder = new PatternLayoutEncoder();
+        encoder.setContext(loggerContext);
+        encoder.setPattern("%-27(%date{ISO8601}) [%-5level] [%thread] %msg%n");
+        encoder.start();
+
+        rollingFileAppender.setEncoder(encoder);
+        rollingFileAppender.setRollingPolicy(rollingPolicy);
+        rollingFileAppender.setTriggeringPolicy(triggeringPolicy);
+        rollingFileAppender.start();
+        ch.qos.logback.classic.Logger logger = (ch.qos.logback.classic.Logger)
+                LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME);
+        logger.addAppender(rollingFileAppender);
+    }
+}

app/src/main/java/fr/free/nrw/commons/logging/LogLevelSettableTree.java

@@ -0,0 +1,8 @@
+package fr.free.nrw.commons.logging;
+
+/**
+ * Can be implemented to set the log level for file tree
+ */
+public interface LogLevelSettableTree {
+    void setLogLevel(int logLevel);
+}

app/src/main/java/fr/free/nrw/commons/logging/LogUtils.java

@@ -0,0 +1,25 @@
+package fr.free.nrw.commons.logging;
+
+import android.content.Context;
+import android.os.Environment;
+
+/**
+ * Returns the log directory
+ */
+public final class LogUtils {
+    private LogUtils() {
+    }
+
+    /**
+     * Returns the directory for saving logs on the device
+     * @param isBeta
+     * @return
+     */
+    public static String getLogDirectory(boolean isBeta) {
+        if(isBeta) {
+            return Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) + "/logs/beta";
+        } else {
+            return Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) + "/logs/prod";
+        }
+    }
+}

app/src/main/java/fr/free/nrw/commons/logging/LogsSender.java

@@ -0,0 +1,183 @@
+package fr.free.nrw.commons.logging;
+
+import android.content.Context;
+import android.content.Intent;
+import android.net.Uri;
+import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+
+import org.acra.collector.CrashReportData;
+import org.acra.sender.ReportSender;
+import org.apache.commons.codec.Charsets;
+
+import java.io.BufferedInputStream;
+import java.io.BufferedOutputStream;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
+
+import fr.free.nrw.commons.auth.SessionManager;
+import timber.log.Timber;
+
+/**
+ * Abstract class that implements Acra's log sender
+ */
+public abstract class LogsSender implements ReportSender {
+
+    String mailTo;
+    String logFileName;
+    String emailSubject;
+    String emailBody;
+
+    private final SessionManager sessionManager;
+    private final boolean isBeta;
+
+    LogsSender(SessionManager sessionManager,
+               boolean isBeta) {
+        this.sessionManager = sessionManager;
+        this.isBeta = isBeta;
+    }
+
+    /**
+     * Overrides send method of ACRA's ReportSender to send logs
+     * @param context
+     * @param report
+     */
+    @Override
+    public void send(@NonNull final Context context, @Nullable CrashReportData report) {
+        sendLogs(context, report);
+    }
+
+    /**
+     * Gets zipped log files and sends it via email. Can be modified to change the send log mechanism
+     * @param context
+     * @param report
+     */
+    private void sendLogs(Context context, CrashReportData report) {
+        final Uri logFileUri = getZippedLogFileUri(context, report);
+        if (logFileUri != null) {
+            sendEmail(context, logFileUri);
+        }
+    }
+
+    /***
+     * Provides any extra information that you want to send. The return value will be
+     * delivered inside the report verbatim
+     *
+     * @return
+     */
+    protected abstract String getExtraInfo();
+
+    /**
+     * Fires an intent to send email with logs
+     * @param context
+     * @param logFileUri
+     */
+    private void sendEmail(Context context, Uri logFileUri) {
+        String subject = emailSubject;
+        String body = emailBody;
+
+        Intent emailIntent = new Intent(Intent.ACTION_SENDTO);
+        emailIntent.setData(Uri.fromParts("mailto", mailTo, null));
+        emailIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
+        emailIntent.putExtra(Intent.EXTRA_SUBJECT, subject);
+        emailIntent.putExtra(Intent.EXTRA_TEXT, body);
+        emailIntent.putExtra(Intent.EXTRA_STREAM, logFileUri);
+        context.startActivity(emailIntent);
+    }
+
+    /**
+     * Returns the URI for the zipped log file
+     * @param context
+     * @param report
+     * @return
+     */
+    private Uri getZippedLogFileUri(Context context, CrashReportData report) {
+        try {
+            StringBuilder builder = new StringBuilder();
+            if (report != null) {
+                attachCrashInfo(report, builder);
+            }
+            attachUserInfo(builder);
+            attachExtraInfo(builder);
+            byte[] metaData = builder.toString().getBytes(Charsets.UTF_8);
+            File zipFile = new File(context.getExternalFilesDir(null), logFileName);
+            writeLogToZipFile(metaData, zipFile);
+            return Uri.fromFile(zipFile);
+        } catch (IOException e) {
+            Timber.w(e, "Error in generating log file");
+        }
+        return null;
+    }
+
+    /**
+     * Checks if there are any pending crash reports and attaches them to the logs
+     * @param report
+     * @param builder
+     */
+    private void attachCrashInfo(CrashReportData report, StringBuilder builder) {
+        if (report == null) {
+            return;
+        }
+        builder.append(report);
+    }
+
+    /**
+     * Attaches username to the the meta_data file
+     * @param builder
+     */
+    private void attachUserInfo(StringBuilder builder) {
+        builder.append("MediaWiki Username = ").append(sessionManager.getUserName()).append("\n");
+    }
+
+    /**
+     * Gets any extra meta information to be attached with the log files
+     * @param builder
+     */
+    private void attachExtraInfo(StringBuilder builder) {
+        String infoToBeAttached = getExtraInfo();
+        builder.append(infoToBeAttached);
+        builder.append("\n");
+    }
+
+    /**
+     * Zips the logs and meta information
+     * @param metaData
+     * @param zipFile
+     * @throws IOException
+     */
+    private void writeLogToZipFile(byte[] metaData, File zipFile) throws IOException {
+        FileOutputStream fos = new FileOutputStream(zipFile);
+        BufferedOutputStream bos = new BufferedOutputStream(fos);
+        ZipOutputStream zos = new ZipOutputStream(bos);
+        File logDir = new File(LogUtils.getLogDirectory(isBeta));
+
+        if (!logDir.exists() || logDir.listFiles().length == 0) {
+            return;
+        }
+        
+        byte[] buffer = new byte[1024];
+        for (File file : logDir.listFiles()) {
+            FileInputStream fis = new FileInputStream(file);
+            BufferedInputStream bis = new BufferedInputStream(fis);
+            zos.putNextEntry(new ZipEntry(file.getName()));
+            int length;
+            while ((length = bis.read(buffer)) > 0) {
+                zos.write(buffer, 0, length);
+            }
+            zos.closeEntry();
+            bis.close();
+        }
+
+        //attach metadata as a separate file
+        zos.putNextEntry(new ZipEntry("meta_data.txt"));
+        zos.write(metaData);
+        zos.closeEntry();
+
+        zos.flush();
+        zos.close();
+    }
+}

app/src/main/java/fr/free/nrw/commons/mwapi/ApacheHttpClientMediaWikiApi.java

@@ -8,7 +8,6 @@ import android.support.annotation.NonNull;
 import android.support.annotation.Nullable;
 import android.support.annotation.VisibleForTesting;
 import android.text.TextUtils;
-import android.util.Log;
 
 import com.google.gson.Gson;
 
@@ -213,8 +212,6 @@ public class ApacheHttpClientMediaWikiApi implements MediaWikiApi {
     @Override
     public void setAuthCookie(String authCookie) {
         api.setAuthCookie(authCookie);
-
-        Timber.d("Mediawiki auth cookie is %s", api.getAuthCookie());
     }
 
     @Override
@@ -894,7 +891,7 @@ public class ApacheHttpClientMediaWikiApi implements MediaWikiApi {
 
         CustomApiResult result = api.upload(filename, file, dataLength, pageContents, editSummary, getCentralAuthToken(), getEditToken(), progressListener::onProgress);
 
-        Log.e("WTF", "Result: " + result.toString());
+        Timber.wtf("Result: " + result.toString());
 
         String resultStatus = result.getString("/api/upload/@result");
 
@@ -981,7 +978,7 @@ public class ApacheHttpClientMediaWikiApi implements MediaWikiApi {
                     new PageTitle(userName).getText());
             HttpUrl.Builder urlBuilder = HttpUrl.parse(url).newBuilder();
             urlBuilder.addQueryParameter("user", userName);
-            Log.i("url", urlBuilder.toString());
+            Timber.i("Url %s", urlBuilder.toString());
             Request request = new Request.Builder()
                     .url(urlBuilder.toString())
                     .build();

app/src/main/java/fr/free/nrw/commons/mwapi/CustomApiResult.java

@@ -38,9 +38,6 @@ public class CustomApiResult {
 
     static CustomApiResult fromRequestBuilder(Http.HttpRequestBuilder builder, HttpClient client) throws IOException {
 
-        Timber.d("API request is %s", builder.toString());
-        Timber.d("API params are %s", client.getParams());
-
         try {
             DocumentBuilder docBuilder = DocumentBuilderFactory.newInstance().newDocumentBuilder();
             Document doc = docBuilder.parse(builder.use(client).charset("utf-8").data("format", "xml").asResponse().getEntity().getContent());

app/src/main/java/fr/free/nrw/commons/mwapi/CustomMwApi.java

@@ -140,8 +140,6 @@ public class CustomMwApi {
     }
 
     public CustomApiResult upload(String filename, InputStream file, long length, String text, String comment, String centralAuthToken, String token, ProgressListener uploadProgressListener) throws IOException {
-        Timber.d("Token being used is %s", token);
-
         Http.HttpRequestBuilder builder = Http.multipart(apiURL)
                 .data("action", "upload")
                 .data("token", token)
@@ -157,8 +155,6 @@ public class CustomMwApi {
             builder.file("file", filename, file);
         }
 
-        Timber.d("Final cookies are %s", client.getCookieStore().getCookies().toString());
-
         return CustomApiResult.fromRequestBuilder(builder, client);
     }
 

app/src/main/java/fr/free/nrw/commons/nearby/PlaceRenderer.java

@@ -7,7 +7,6 @@ import android.support.transition.TransitionManager;
 import android.support.v4.app.Fragment;
 import android.support.v7.app.AlertDialog;
 import android.support.v7.widget.PopupMenu;
-import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.MenuItem;
 import android.view.View;
@@ -91,7 +90,7 @@ public class PlaceRenderer extends Renderer<Place> {
     protected void hookListeners(View view) {
 
         final View.OnClickListener listener = view12 -> {
-            Log.d("Renderer", "clicked");
+            Timber.d("Renderer clicked");
             TransitionManager.beginDelayedTransition(buttonLayout);
 
             if (buttonLayout.isShown()) {

app/src/main/java/fr/free/nrw/commons/quiz/QuizResultActivity.java

@@ -8,6 +8,16 @@ import android.net.Uri;
 import android.os.Bundle;
 import android.support.v7.app.AlertDialog;
 import android.support.v7.app.AppCompatActivity;
+import android.os.Bundle;
+
+import com.dinuscxj.progressbar.CircleProgressBar;
+
+import butterknife.BindView;
+import butterknife.ButterKnife;
+import butterknife.OnClick;
+import fr.free.nrw.commons.R;
+import fr.free.nrw.commons.contributions.ContributionsActivity;
+
 import android.support.v7.widget.Toolbar;
 import android.view.LayoutInflater;
 import android.view.Menu;

app/src/main/java/fr/free/nrw/commons/settings/SettingsFragment.java

@@ -2,13 +2,8 @@ package fr.free.nrw.commons.settings;
 
 import android.Manifest;
 import android.app.AlertDialog;
-import android.content.ActivityNotFoundException;
-import android.content.Context;
-import android.content.Intent;
 import android.content.SharedPreferences;
-import android.content.pm.PackageManager;
 import android.net.Uri;
-import android.os.Build;
 import android.os.Bundle;
 import android.preference.EditTextPreference;
 import android.preference.ListPreference;
@@ -20,23 +15,26 @@ import android.support.v4.content.ContextCompat;
 import android.support.v4.content.FileProvider;
 import android.widget.Toast;
 
-import java.io.File;
+import com.karumi.dexter.Dexter;
+import com.karumi.dexter.listener.PermissionGrantedResponse;
+import com.karumi.dexter.listener.single.BasePermissionListener;
 
 import javax.inject.Inject;
 import javax.inject.Named;
 
-import fr.free.nrw.commons.BuildConfig;
-import fr.free.nrw.commons.CommonsApplication;
 import fr.free.nrw.commons.R;
 import fr.free.nrw.commons.Utils;
 import fr.free.nrw.commons.di.ApplicationlessInjection;
-import fr.free.nrw.commons.upload.FileUtils;
+import fr.free.nrw.commons.logging.CommonsLogSender;
+import fr.free.nrw.commons.utils.PermissionUtils;
+import fr.free.nrw.commons.utils.ViewUtil;
 
 public class SettingsFragment extends PreferenceFragment {
 
     private static final int REQUEST_CODE_WRITE_EXTERNAL_STORAGE = 100;
 
     @Inject @Named("default_preferences") SharedPreferences prefs;
+    @Inject CommonsLogSender commonsLogSender;
 
     @Override
     public void onCreate(Bundle savedInstanceState) {
@@ -99,72 +97,38 @@ public class SettingsFragment extends PreferenceFragment {
 
         Preference betaTesterPreference = findPreference("becomeBetaTester");
         betaTesterPreference.setOnPreferenceClickListener(preference -> {
-                Utils.handleWebUrl(getActivity(),Uri.parse(getResources().getString(R.string.beta_opt_in_link)));
+            Utils.handleWebUrl(getActivity(), Uri.parse(getResources().getString(R.string.beta_opt_in_link)));
-                return true;
+            return true;
         });
         Preference sendLogsPreference = findPreference("sendLogFile");
         sendLogsPreference.setOnPreferenceClickListener(preference -> {
-            //first we need to check if we have the necessary permissions
+            checkPermissionsAndSendLogs();
-            if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-                if (ContextCompat.checkSelfPermission(
-                        getActivity(),
-                        Manifest.permission.WRITE_EXTERNAL_STORAGE)
-                        ==
-                        PackageManager.PERMISSION_GRANTED) {
-                    sendAppLogsViaEmail();
-                } else {
-                    //first get the necessary permission
-                    requestPermissions(new String[]{Manifest.permission.WRITE_EXTERNAL_STORAGE},
-                            REQUEST_CODE_WRITE_EXTERNAL_STORAGE);
-                }
-            } else {
-                sendAppLogsViaEmail();
-            }
-
             return true;
         });
     }
 
-    @Override
+    /**
-    public void onRequestPermissionsResult(int requestCode, @NonNull String[] permissions, @NonNull int[] grantResults) {
+     * First checks for external storage permissions and then sends logs via email
-        super.onRequestPermissionsResult(requestCode, permissions, grantResults);
+     */
-        if (requestCode == REQUEST_CODE_WRITE_EXTERNAL_STORAGE && grantResults.length > 0 && grantResults[0] == PackageManager.PERMISSION_GRANTED) {
+    private void checkPermissionsAndSendLogs() {
-             {
+        if (PermissionUtils.hasPermission(getActivity(), Manifest.permission.WRITE_EXTERNAL_STORAGE)) {
-                sendAppLogsViaEmail();
+            commonsLogSender.send(getActivity(), null);
-            }
+        } else {
+            requestExternalStoragePermissions();
         }
     }
 
-    private void sendAppLogsViaEmail() {
+    /**
-        String appLogs = Utils.getAppLogs();
+     * Requests external storage permissions and shows a toast stating that log collection has started
-        File appLogsFile = FileUtils.createAndGetAppLogsFile(appLogs);
+     */
-
+    private void requestExternalStoragePermissions() {
-        Context applicationContext = getActivity().getApplicationContext();
+        Dexter.withActivity(getActivity())
-        Uri appLogsFilePath = FileProvider.getUriForFile(
+                .withPermission(Manifest.permission.WRITE_EXTERNAL_STORAGE)
-                getActivity(),
+                .withListener(new BasePermissionListener() {
-                applicationContext.getPackageName() + ".provider",
+                    @Override
-                appLogsFile
+                    public void onPermissionGranted(PermissionGrantedResponse response) {
-        );
+                        ViewUtil.showLongToast(getActivity(), getResources().getString(R.string.log_collection_started));
-
+                    }
-        //initialize the emailSelectorIntent
+                }).check();
-        Intent emailSelectorIntent = new Intent(Intent.ACTION_SENDTO);
-        emailSelectorIntent.setData(Uri.parse("mailto:"));
-        //initialize the emailIntent
-        final Intent emailIntent = new Intent(Intent.ACTION_SEND);
-        // Logs must be sent to the PRIVATE email. Please do not modify this without good reason!
-        emailIntent.putExtra(Intent.EXTRA_EMAIL, new String[]{CommonsApplication.LOGS_PRIVATE_EMAIL});
-        emailIntent.putExtra(Intent.EXTRA_SUBJECT, String.format(CommonsApplication.LOGS_PRIVATE_EMAIL_SUBJECT, BuildConfig.VERSION_NAME));
-        emailIntent.addFlags(Intent.FLAG_GRANT_READ_URI_PERMISSION);
-        emailIntent.addFlags(Intent.FLAG_GRANT_WRITE_URI_PERMISSION);
-        emailIntent.setSelector( emailSelectorIntent );
-        //adding the attachment to the intent
-        emailIntent.putExtra(Intent.EXTRA_STREAM, appLogsFilePath);
-
-        try {
-            startActivity(Intent.createChooser(emailIntent, "Send mail.."));
-        } catch (ActivityNotFoundException e) {
-            Toast.makeText(getActivity(), R.string.no_email_client, Toast.LENGTH_SHORT).show();
-        }
     }
-
 }

app/src/main/java/fr/free/nrw/commons/upload/UploadController.java

@@ -15,7 +15,6 @@ import android.os.AsyncTask;
 import android.os.IBinder;
 import android.provider.MediaStore;
 import android.text.TextUtils;
-import android.util.Log;
 
 import java.io.BufferedInputStream;
 import java.io.File;
@@ -167,7 +166,7 @@ public class UploadController {
 
                     //TODO: understand do we really need this code
                     if (contribution.getDataLength() <= 0) {
-                        Log.d("deneme","UploadController/doInBackground, contribution.getLocalUri():"+contribution.getLocalUri());
+                        Timber.d("UploadController/doInBackground, contribution.getLocalUri():" + contribution.getLocalUri());
                         AssetFileDescriptor assetFileDescriptor = contentResolver
                                 .openAssetFileDescriptor(Uri.fromFile(new File(contribution.getLocalUri().getPath())), "r");
                         if (assetFileDescriptor != null) {

app/src/main/java/fr/free/nrw/commons/utils/ImageUtils.java

@@ -8,7 +8,6 @@ import android.graphics.Color;
 import android.graphics.Rect;
 import android.net.Uri;
 import android.support.annotation.Nullable;
-import android.util.Log;
 
 import com.facebook.common.executors.CallerThreadExecutor;
 import com.facebook.common.references.CloseableReference;
@@ -89,7 +88,7 @@ public class ImageUtils {
 
         int allPixelsCount = bitmapWidth * bitmapHeight;
         int[] bitmapPixels = new int[allPixelsCount];
-        Log.e("total", Integer.toString(allPixelsCount));
+        Timber.d("total %s", Integer.toString(allPixelsCount));
 
         bitmap.getPixels(bitmapPixels,0,bitmapWidth,0,0,bitmapWidth,bitmapHeight);
         int numberOfBrightPixels = 0;

app/src/main/java/fr/free/nrw/commons/utils/PermissionUtils.java

@@ -2,8 +2,10 @@ package fr.free.nrw.commons.utils;
 
 import android.app.Activity;
 import android.content.Intent;
+import android.content.pm.PackageManager;
 import android.net.Uri;
 import android.provider.Settings;
+import android.support.v4.content.ContextCompat;
 
 import fr.free.nrw.commons.CommonsApplication;
 
@@ -21,4 +23,16 @@ public class PermissionUtils {
         intent.setData(uri);
         activity.startActivityForResult(intent,CommonsApplication.OPEN_APPLICATION_DETAIL_SETTINGS);
     }
+
+    /**
+     * Checks whether the app already has a particular permission
+     *
+     * @param activity
+     * @param permission permission to be checked
+     * @return
+     */
+    public static boolean hasPermission(Activity activity, String permission) {
+        return ContextCompat.checkSelfPermission(activity, permission) == PackageManager.PERMISSION_GRANTED;
+
+    }
 }

app/src/main/res/values/strings.xml

@@ -240,7 +240,7 @@
   <string name="use_external_storage_summary">Save pictures taken with the in-app camera on your device</string>
   <string name="login_to_your_account">Log in to your account</string>
   <string name="send_log_file">Send log file</string>
-  <string name="send_log_file_description">Send log file to developers via email</string>
+  <string name="send_log_file_description">Send log file to developers via email to help debug problems with the app. Note: logs may potentially contain identifying information</string>
   <string name="no_web_browser">No web browser found to open URL</string>
   <string name="null_url">Error! URL not found</string>
   <string name="nominate_deletion">Nominate for Deletion</string>
@@ -356,4 +356,6 @@
   <string name="notifications_channel_name_all">Commons Notification</string>
   <string name="storage_permission">Storage Permission</string>
   <string name="write_storage_permission_rationale_for_image_share">We need your permission to access the external storage of your device in order to upload images.</string>
+
+  <string name="log_collection_started">Log collection started. Please RESTART the app, perform action that you wish to log, and then tap \'Send Logs\' again</string>
 </resources>