Move methods from ReportUtils to their corresponding collectors, clea…

…nup, comments

src/main/java/org/acra/ACRAConstants.java

@@ -159,4 +159,6 @@ private ACRAConstants(){}
     public static final String DATE_TIME_FORMAT_STRING = "yyyy-MM-dd'T'HH:mm:ss.SSSZZZZZ";
 
     public static final String DEFAULT_CERTIFICATE_TYPE = "X.509";
+
+    public static final String NOT_AVAILABLE = "N/A";
 }

src/main/java/org/acra/collector/Collector.java

@@ -24,9 +24,12 @@
 import java.util.Set;
 
 /**
- * Created on 11.08.2016.
+ * Represents a collector.
+ * Maintains information on which fields can be collected by this collector.
+ * Validates constraints in which a field should (not) be collected.
  *
  * @author F43nd1r
+ * @since 4.9.1
  */
 abstract class Collector {
     private final ReportField[] reportFields;

src/main/java/org/acra/collector/CustomDataCollector.java

@@ -25,11 +25,12 @@
 import java.util.Map;
 
 /**
- * Created on 12.08.2016.
+ * Collects custom data supplied by the user
  *
  * @author F43nd1r
+ * @since 4.9.1
  */
-class CustomDataCollector extends Collector {
+final class CustomDataCollector extends Collector {
     private final Map<String, String> customParameters;
 
     CustomDataCollector(Map<String, String> customParameters){
@@ -44,8 +45,7 @@ String collect(ReportField reportField, ReportBuilder reportBuilder) {
 
 
     /**
-     * Generates the string which is posted in the single custom data field in
-     * the GoogleDocs Form.
+     * Generates the string which is posted in the single custom data field
      *
      * @return A string with a 'key = value' pair on each line.
      */

src/main/java/org/acra/collector/DeviceFeaturesCollector.java

@@ -19,6 +19,7 @@
 import android.content.pm.FeatureInfo;
 import android.content.pm.PackageManager;
 import android.support.annotation.NonNull;
+
 import org.acra.ACRA;
 import org.acra.ReportField;
 import org.acra.builder.ReportBuilder;
@@ -27,29 +28,34 @@
 
 /**
  * Features declared as available on the device.
- * 
+ *
  * @author Kevin Gaudin
- * 
  */
-final class DeviceFeaturesCollector extends Collector{
+final class DeviceFeaturesCollector extends Collector {
     private final Context context;
 
-    DeviceFeaturesCollector(Context context){
+    DeviceFeaturesCollector(Context context) {
         super(ReportField.DEVICE_FEATURES);
         this.context = context;
     }
 
+    /**
+     * collects device features
+     *
+     * @param reportField   the ReportField to collect
+     * @param reportBuilder the current reportBuilder
+     * @return String of all device feature names
+     */
     @NonNull
     @Override
     String collect(ReportField reportField, ReportBuilder reportBuilder) {
-
         final StringBuilder result = new StringBuilder();
         try {
             final PackageManager pm = context.getPackageManager();
             final FeatureInfo[] features = pm.getSystemAvailableFeatures();
             for (final FeatureInfo feature : features) {
                 final String featureName = feature.name;
-                if(featureName != null) {
+                if (featureName != null) {
                     result.append(featureName);
                 } else {
                     result.append("glEsVersion = ").append(feature.getGlEsVersion());

src/main/java/org/acra/collector/DeviceIdCollector.java

@@ -16,24 +16,30 @@
 package org.acra.collector;
 
 import android.Manifest;
+import android.annotation.SuppressLint;
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.support.annotation.NonNull;
+import android.support.annotation.Nullable;
+import android.telephony.TelephonyManager;
 
 import org.acra.ACRA;
+import org.acra.ACRAConstants;
 import org.acra.ReportField;
 import org.acra.builder.ReportBuilder;
 import org.acra.util.PackageManagerWrapper;
-import org.acra.util.ReportUtils;
 
 import java.util.Set;
 
+import static org.acra.ACRA.LOG_TAG;
+
 /**
- * Created on 12.08.2016.
+ * Collects the device ID
  *
  * @author F43nd1r
+ * @since 4.9.1
  */
-class DeviceIdCollector extends Collector {
+final class DeviceIdCollector extends Collector {
     private final Context context;
     private final PackageManagerWrapper pm;
     private final SharedPreferences prefs;
@@ -54,7 +60,24 @@ boolean shouldCollect(Set<ReportField> crashReportFields, ReportField collect, R
     @NonNull
     @Override
     String collect(ReportField reportField, ReportBuilder reportBuilder) {
-        String result = ReportUtils.getDeviceId(context);
-        return result != null ? result : "N/A";
+        String result = getDeviceId();
+        return result != null ? result : ACRAConstants.NOT_AVAILABLE;
+    }
+
+    /**
+     * Returns the DeviceId according to the TelephonyManager.
+     *
+     * @return Returns the DeviceId according to the TelephonyManager or null if there is no TelephonyManager.
+     */
+    @SuppressLint("HardwareIds")
+    @Nullable
+    private String getDeviceId() {
+        try {
+            final TelephonyManager tm = (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE);
+            return tm.getDeviceId();
+        } catch (RuntimeException e) {
+            ACRA.log.w(LOG_TAG, "Couldn't retrieve DeviceId for : " + context.getPackageName(), e);
+            return null;
+        }
     }
 }

src/main/java/org/acra/collector/DisplayManagerCollector.java

@@ -32,15 +32,18 @@
 
 import java.lang.reflect.Field;
 
+/**
+ * Collects information about the connected display(s)
+ */
 final class DisplayManagerCollector extends Collector {
     private final Context context;
+    private final SparseArray<String> flagNames = new SparseArray<String>();
 
     DisplayManagerCollector(Context context) {
         super(ReportField.DISPLAY);
         this.context = context;
     }
 
-    private static final SparseArray<String> mFlagsNames = new SparseArray<String>();
 
     @NonNull
     @Override
@@ -69,7 +72,7 @@ String collect(ReportField reportField, ReportBuilder reportBuilder) {
     }
 
     @NonNull
-    private static Object collectDisplayData(@NonNull Display display) {
+    private Object collectDisplayData(@NonNull Display display) {
         final DisplayMetrics metrics = new DisplayMetrics();
         display.getMetrics(metrics);
 
@@ -165,18 +168,18 @@ private static String collectCurrentSizeRange(@NonNull Display display) {
     }
 
     @NonNull
-    private static String collectFlags(@NonNull Display display) {
+    private String collectFlags(@NonNull Display display) {
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1) {
             final int flags = display.getFlags();
             for (Field field : display.getClass().getFields()) {
                 if (field.getName().startsWith("FLAG_")) {
                     try {
-                        mFlagsNames.put(field.getInt(null), field.getName());
+                        flagNames.put(field.getInt(null), field.getName());
                     } catch (IllegalAccessException ignored) {
                     }
                 }
             }
-            return display.getDisplayId() + ".flags=" + activeFlags(mFlagsNames, flags) + '\n';
+            return display.getDisplayId() + ".flags=" + activeFlags(flags) + '\n';
         }
         return "";
     }
@@ -222,25 +225,23 @@ private static String collectMetrics(@NonNull String prefix, @NonNull DisplayMet
      * applying a bitmask. That method returns the concatenation of active
      * values.
      *
-     * @param valueNames The array containing the different values and names for this
-     *                   field. Must contain mask values too.
      * @param bitfield   The bitfield to inspect.
      * @return The names of the different values contained in the bitfield,
      * separated by '+'.
      */
     @NonNull
-    private static String activeFlags(@NonNull SparseArray<String> valueNames, int bitfield) {
+    private String activeFlags(int bitfield) {
         final StringBuilder result = new StringBuilder();
 
         // Look for masks, apply it an retrieve the masked value
-        for (int i = 0; i < valueNames.size(); i++) {
-            final int maskValue = valueNames.keyAt(i);
+        for (int i = 0; i < flagNames.size(); i++) {
+            final int maskValue = flagNames.keyAt(i);
             final int value = bitfield & maskValue;
             if (value > 0) {
                 if (result.length() > 0) {
                     result.append('+');
                 }
-                result.append(valueNames.get(value));
+                result.append(flagNames.get(value));
             }
         }
         return result.toString();

src/main/java/org/acra/collector/DropBoxCollector.java

@@ -22,6 +22,7 @@
 import android.support.annotation.NonNull;
 
 import org.acra.ACRA;
+import org.acra.ACRAConstants;
 import org.acra.ReportField;
 import org.acra.builder.ReportBuilder;
 import org.acra.config.ACRAConfiguration;
@@ -64,7 +65,6 @@ final class DropBoxCollector extends Collector {
             "SYSTEM_RECOVERY_LOG", "SYSTEM_BOOT", "SYSTEM_LAST_KMSG", "APANIC_CONSOLE", "APANIC_THREADS",
             "SYSTEM_RESTART", "SYSTEM_TOMBSTONE", "data_app_strictmode"};
 
-    private static final String NO_RESULT = "N/A";
     private final SimpleDateFormat dateFormat = new SimpleDateFormat("yyyyMMdd'T'HHmmss", Locale.getDefault()); //iCal format (used to prevent logic changes). Why use this bad readable format?
 
     /**
@@ -125,7 +125,7 @@ String collect(ReportField reportField, ReportBuilder reportBuilder) {
             if (ACRA.DEV_LOGGING) ACRA.log.d(LOG_TAG, "DropBoxManager not available.");
         }
 
-        return NO_RESULT;
+        return ACRAConstants.NOT_AVAILABLE;
     }
 
     @Override

src/main/java/org/acra/collector/LogCatCollector.java

@@ -45,7 +45,11 @@
  *
  * @author Kevin Gaudin
  */
-class LogCatCollector extends Collector {
+final class LogCatCollector extends Collector {
+    /**
+     * Default number of latest lines kept from the logcat output.
+     */
+    private static final int DEFAULT_TAIL_COUNT = 100;
 
     private final ACRAConfiguration config;
     private final PackageManagerWrapper pm;
@@ -56,11 +60,6 @@ class LogCatCollector extends Collector {
         this.pm = pm;
     }
 
-    /**
-     * Default number of latest lines kept from the logcat output.
-     */
-    private static final int DEFAULT_TAIL_COUNT = 100;
-
     /**
      * Executes the logcat command with arguments taken from
      * {@link ReportsCrashes#logcatArguments()}

src/main/java/org/acra/collector/LogFileCollector.java

@@ -24,6 +24,7 @@
 
 import org.acra.ACRA;
 import org.acra.file.Directory;
+import org.acra.ACRAConstants;
 import org.acra.ReportField;
 import org.acra.builder.ReportBuilder;
 import org.acra.config.ACRAConfiguration;
@@ -36,11 +37,10 @@
 /**
  * Collects the N last lines of a text stream. Use this collector if your
  * application handles its own logging system.
- * 
+ *
  * @author Kevin Gaudin
- * 
  */
-class LogFileCollector extends Collector{
+final class LogFileCollector extends Collector {
     private final Context context;
     private final ACRAConfiguration config;
 
@@ -61,14 +61,21 @@ class LogFileCollector extends Collector{
     @Override
     String collect(ReportField reportField, ReportBuilder reportBuilder) {
         try {
-            return IOUtils.streamToString(getStream(context, config.applicationLogFileDir(), config.applicationLogFile()), config.applicationLogFileLines());
+            return IOUtils.streamToString(getStream(config.applicationLogFileDir(), config.applicationLogFile()), config.applicationLogFileLines());
         } catch (IOException e) {
-            return "Application log not available";
+            return ACRAConstants.NOT_AVAILABLE;
         }
     }
 
+    /**
+     * guess the application log file location and open it
+     *
+     * @param directory the base directory for the file path
+     * @param fileName the name of the file
+     * @return a stream to the file or an empty stream if the file was not found
+     */
     @NonNull
-    private static InputStream getStream(@NonNull Context context, @NonNull Directory directory, @NonNull String fileName) {
+    private InputStream getStream(@NonNull Directory directory, @NonNull String fileName) {
         if (directory == Directory.FILES_LEGACY) {
             directory = fileName.startsWith("/") ? Directory.ROOT : Directory.FILES;
         }