Updates based on review

app/src/main/java/org/openobservatory/ooniprobe/activity/MainActivity.java

@@ -23,6 +23,7 @@
 import androidx.fragment.app.Fragment;
 
 import com.google.android.material.snackbar.Snackbar;
+import com.google.common.base.Optional;
 
 import org.openobservatory.engine.OONIRunDescriptor;
 import org.openobservatory.ooniprobe.R;
@@ -34,9 +35,9 @@
 import org.openobservatory.ooniprobe.fragment.DashboardFragment;
 import org.openobservatory.ooniprobe.fragment.PreferenceGlobalFragment;
 import org.openobservatory.ooniprobe.fragment.ResultListFragment;
-import org.openobservatory.ooniprobe.fragment.dynamic_progress.DynamicProgressFragment;
-import org.openobservatory.ooniprobe.fragment.dynamic_progress.OnActionListener;
-import org.openobservatory.ooniprobe.fragment.dynamic_progress.ProgressType;
+import org.openobservatory.ooniprobe.fragment.dynamicprogressbar.OONIRunDynamicProgressBar;
+import org.openobservatory.ooniprobe.fragment.dynamicprogressbar.OnActionListener;
+import org.openobservatory.ooniprobe.fragment.dynamicprogressbar.ProgressType;
 
 import java.io.Serializable;
 
@@ -193,6 +194,7 @@ protected void onNewIntent(Intent intent) {
         /**
          * Check if we are starting the activity from a link [Intent.ACTION_VIEW].
          * This is invoked when a v2 link is opened.
+         * @see {@link org.openobservatory.ooniprobe.activity.OoniRunActivity#newIntent}. for v1 links.
          */
         if (Intent.ACTION_VIEW.equals(intent.getAction())) {
             Uri uri = intent.getData();
@@ -201,10 +203,15 @@ protected void onNewIntent(Intent intent) {
                 return;
             }
 
-            long runId = getRunId(uri);
+            Optional<Long> possibleRunId = getRunId(uri);
 
-            // If the intent contains a link, but the `link_id` is zero, or the link is not supported, do nothing.
-            if (runId == 0) {
+            // If the intent contains a link, but it is not a supported link or has a non-numerical `link_id`.
+            if (!possibleRunId.isPresent()) {
+                return;
+            }
+
+            // If the intent contains a link, but the `link_id` is zero.
+            if (possibleRunId.get() == 0) {
                 return;
             }
 
@@ -214,7 +221,7 @@ protected void onNewIntent(Intent intent) {
 
             executor.executeTask(() -> {
                 try {
-                    return descriptorManager.fetchDescriptorFromRunId(runId, this);
+                    return descriptorManager.fetchDescriptorFromRunId(possibleRunId.get(), this);
                 } catch (Exception exception) {
                     exception.printStackTrace();
                     ThirdPartyServices.logException(exception);
@@ -284,7 +291,7 @@ private void displayAddLinkProgressFragment(TaskExecutor executor) {
         getSupportFragmentManager().beginTransaction()
                 .add(
                         R.id.dynamic_progress_fragment,
-                        DynamicProgressFragment.newInstance(ProgressType.ADD_LINK, new OnActionListener() {
+                        OONIRunDynamicProgressBar.newInstance(ProgressType.ADD_LINK, new OnActionListener() {
                             @Override
                             public void onActionButtonCLicked() {
                                 executor.cancelTask();
@@ -296,52 +303,61 @@ public void onCloseButtonClicked() {
                                 removeProgressFragment();
                             }
                         }),
-                        DynamicProgressFragment.getTAG()
+                        OONIRunDynamicProgressBar.getTAG()
                 ).commit();
     }
 
     /**
-     * Get the run id from the intent.
-     * The run id can be in two different formats.
+     * Extracts the run id from the provided Uri.
+     * The run id can be in two different formats:
+     * <p>
+     * 1. ooni://runv2/#link_id
+     * 2. https://run.test.ooni.org/v2/#link_id
      * <p>
-     * 1. ooni://runv2/<link_id>
-     * 2. https://run.test.ooni.org/v2/<link_id>
      * The run id is the `link_id` in the link.
-     * If the intent contains a link, but the `link_id` is not a number, return zero.
-     * If the intent contains a link, but it is not a supported link, return zero.
+     * If the Uri contains a link, but the `link_id` is not a number, an empty Optional is returned.
+     * If the Uri contains a link, but it is not a supported link, an empty Optional is returned.
+     * <p>
      *
-     * @param uri The intent data.
-     * @return The run id if the intent contains a link with a valid `link_id`.
+     * @param uri The Uri data.
+     * @return An Optional containing the run id if the Uri contains a link with a valid `link_id`, or an empty Optional otherwise.
      */
-    private long getRunId(Uri uri) {
+    private Optional<Long> getRunId(Uri uri) {
         String host = uri.getHost();
-        long runId = 0;
 
         try {
             if ("runv2".equals(host)) {
                 /**
                  * The run id is the first segment of the path.
                  * Launched when `Open Link in OONI Probe` is clicked.
-                 * e.g. ooni://runv2/<link_id>
+                 * e.g. ooni://runv2/#link_id
                  */
-                runId = Long.parseLong(uri.getPathSegments().get(0));
+                return Optional.of(
+                        Long.parseLong(
+                                uri.getPathSegments().get(0)
+                        )
+                );
             } else if ("run.test.ooni.org".equals(host)) {
                 /**
                  * The run id is the second segment of the path.
                  * Launched when the system recognizes this app can open this link
                  * and launches the app when a link is clicked.
-                 * e.g. https://run.test.ooni.org/v2/<link_id>
+                 * e.g. https://run.test.ooni.org/v2/#link_id
                  */
-                runId = Long.parseLong(uri.getPathSegments().get(1));
+                return Optional.of(
+                        Long.parseLong(
+                                uri.getPathSegments().get(1)
+                        )
+                );
             } else {
-                // If the intent contains a link, but it is not a supported link, return zero.
-                return 0;
+                // If the intent contains a link, but it is not a supported link.
+                return Optional.absent();
             }
         } catch (Exception e) {
             // If the intent contains a link, but the `link_id` is not a number.
             e.printStackTrace();
+            return Optional.absent();
         }
-        return runId;
     }
 
     /**
@@ -350,7 +366,7 @@ private long getRunId(Uri uri) {
      * This method is called when the task is completed.
      */
     private void removeProgressFragment() {
-        Fragment fragment = getSupportFragmentManager().findFragmentByTag(DynamicProgressFragment.getTAG());
+        Fragment fragment = getSupportFragmentManager().findFragmentByTag(OONIRunDynamicProgressBar.getTAG());
         if (fragment != null && fragment.isAdded()) {
             getSupportFragmentManager().beginTransaction().remove(fragment).commit();
         }

app/src/main/java/org/openobservatory/ooniprobe/activity/add_descriptor/AddDescriptorActivity.kt

@@ -82,8 +82,8 @@ class AddDescriptorActivity : AbstractActivity() {
         @JvmStatic
         @BindingAdapter(value = ["resource"])
         fun setImageViewResource(imageView: ImageView, iconName: String?) {
-            /*TODO(aanorbel): Update to parse the icon name and set the correct icon.
-            *  Remember to ignore icons generated when generated doing this.*/
+            /* TODO(aanorbel): Update to parse the icon name and set the correct icon.
+            * Remember to ignore icons generated when generated doing this.*/
             imageView.setImageResource(R.drawable.ooni_empty_state)
         }
 

app/src/main/java/org/openobservatory/ooniprobe/activity/add_descriptor/AddDescriptorViewModel.kt

@@ -44,18 +44,18 @@ class AddDescriptorViewModel constructor(
      */
     fun getName(): String {
         return descriptor.value?.let { descriptor ->
-            descriptor.nameIntl[LocaleUtils.sLocale.language] ?: descriptor.name
+            descriptor.nameIntl[LocaleUtils.getLocale().language] ?: descriptor.name
         } ?: ""
     }
 
     /**
-     * This method is used to get the name of the descriptor.
+     * This method is used to get the description of the descriptor.
      * Used by the UI during data binding.
      * @return the name of the descriptor.
      */
     fun getDescription(): String {
         return descriptor.value?.let { descriptor ->
-            descriptor.descriptionIntl[LocaleUtils.sLocale.language] ?: descriptor.description
+            descriptor.descriptionIntl[LocaleUtils.getLocale().language] ?: descriptor.description
         } ?: ""
     }
 
@@ -66,7 +66,7 @@ class AddDescriptorViewModel constructor(
      */
     fun getShortDescription(): String {
         return descriptor.value?.let { descriptor ->
-            descriptor.shortDescriptionIntl[LocaleUtils.sLocale.language]
+            descriptor.shortDescriptionIntl[LocaleUtils.getLocale().language]
                 ?: descriptor.shortDescription
         } ?: ""
     }

app/src/main/java/org/openobservatory/ooniprobe/activity/add_descriptor/adapter/AddDescriptorExpandableListAdapter.kt

@@ -116,7 +116,6 @@ class AddDescriptorExpandableListAdapter(
         groupCheckBox.setOnClickListener {
             if (groupItem.selected) {
                 groupItem.selected = false
-                //viewModel.disableTest(groupItem.name)
                 notifyDataSetChanged()
                 if (isNotSelectedAnyGroupItem()) {
                     viewModel.setSelectedAllBtnStatus(STATE_UNCHECKED)
@@ -125,7 +124,6 @@ class AddDescriptorExpandableListAdapter(
                 }
             } else {
                 groupItem.selected = true
-                //viewModel.enableTest(groupItem.name)
                 notifyDataSetChanged()
 
                 if (isSelectedAllItems()) {

app/src/main/java/org/openobservatory/ooniprobe/common/LocaleUtils.java

@@ -9,7 +9,7 @@
 
 public class LocaleUtils {
 
-    public static Locale sLocale;
+    private static Locale sLocale;
 
     public static void setLocale(Locale locale) {
         sLocale = locale;
@@ -36,4 +36,8 @@ public static void updateConfig(Application app, Configuration configuration) {
             res.updateConfiguration(config, res.getDisplayMetrics());
         }
     }
+
+    public static Locale getLocale() {
+        return sLocale;
+    }
 }

app/src/main/java/org/openobservatory/ooniprobe/common/TaskExecutor.kt

@@ -16,7 +16,7 @@ abstract class ProgressTask<P, R> {
 }
 
 /**
- * Callable is an alias for the java.util.concurrent.Callable interface.
+ * Task is an alias for the java.util.concurrent.Callable interface.
  * @param R The type of the result.
  */
 typealias Task<R> = Callable<R>

app/src/main/java/org/openobservatory/ooniprobe/common/TestDescriptorManager.kt

@@ -52,7 +52,7 @@ class TestDescriptorManager @Inject constructor(private val context: Context) {
     }
 
     fun addDescriptor(descriptor: OONIRunDescriptor, automatedUpdates: Boolean): Boolean {
-        // persist to database
+        // TODO(aanorbel): persist to database
         return false
     }
 }

app/src/main/java/org/openobservatory/ooniprobe/fragment/dynamic_progress/DynamicProgressFragment.kt → app/src/main/java/org/openobservatory/ooniprobe/fragment/dynamicprogressbar/OONIRunDynamicProgressBar.kt

@@ -1,21 +1,20 @@
-package org.openobservatory.ooniprobe.fragment.dynamic_progress
+package org.openobservatory.ooniprobe.fragment.dynamicprogressbar
 
 import android.os.Bundle
 import androidx.fragment.app.Fragment
 import android.view.LayoutInflater
 import android.view.View
 import android.view.ViewGroup
 import org.openobservatory.ooniprobe.R
-import org.openobservatory.ooniprobe.databinding.FragmentDynamicProgressBinding
-
+import org.openobservatory.ooniprobe.databinding.RunDynamicProgressBarBinding
 
 /**
  * A [Fragment] subclass that displays a dynamic progress bar and handles user actions.
  * The progress bar can be in one of the following states: [ProgressType.ADD_LINK], [ProgressType.UPDATE_LINK], [ProgressType.REVIEW_LINK].
  * The user actions are handled through the OnActionListener interface.
- * Use the [DynamicProgressFragment.newInstance] factory method to create an instance of this fragment.
+ * Use the [OONIRunDynamicProgressBar.newInstance] factory method to create an instance of this fragment.
  */
-class DynamicProgressFragment : Fragment() {
+class OONIRunDynamicProgressBar : Fragment() {
     private var progressType: ProgressType? = null
     private var onActionListener: OnActionListener? = null
 
@@ -31,7 +30,7 @@ class DynamicProgressFragment : Fragment() {
         inflater: LayoutInflater, container: ViewGroup?,
         savedInstanceState: Bundle?
     ): View {
-        val binding = FragmentDynamicProgressBinding.inflate(inflater, container, false)
+        val binding = RunDynamicProgressBarBinding.inflate(inflater, container, false)
         binding.actionButton.setOnClickListener {
             onActionListener?.onActionButtonCLicked()
         }
@@ -71,7 +70,7 @@ class DynamicProgressFragment : Fragment() {
         private const val PROGRESS_TYPE = "PROGRESS_TYPE"
 
         @JvmStatic
-        var TAG: String = DynamicProgressFragment::class.java.name
+        var TAG: String = OONIRunDynamicProgressBar::class.java.name
 
         /**
          * Use this factory method to create a new instance of
@@ -85,8 +84,8 @@ class DynamicProgressFragment : Fragment() {
         fun newInstance(
             progressType: ProgressType,
             onActionListener: OnActionListener?
-        ): DynamicProgressFragment {
-            return DynamicProgressFragment().apply {
+        ): OONIRunDynamicProgressBar {
+            return OONIRunDynamicProgressBar().apply {
                 arguments = Bundle().apply {
                     putString(PROGRESS_TYPE, progressType.name)
                 }

app/src/main/res/layout/fragment_dynamic_progress.xml → app/src/main/res/layout/run_dynamic_progress_bar.xml

@@ -4,7 +4,7 @@
     android:layout_width="match_parent"
     android:layout_height="match_parent"
     android:background="@color/progress_background"
-    tools:context=".fragment.dynamic_progress.DynamicProgressFragment">
+    tools:context=".fragment.dynamicprogressbar.OONIRunDynamicProgressBar">
 
     <LinearLayout
         android:layout_width="match_parent"

engine/src/main/java/org/openobservatory/engine/OONIRunFetchResponse.kt

@@ -5,39 +5,84 @@ import java.io.Serializable
 import java.util.Date
 import java.util.HashMap
 
+/**
+ * This class represents the response from a fetch request to the OONI API.
+ *
+ * @property archived Whether the descriptor is archived.
+ * @property creationTime The creation time of the descriptor.
+ * @property translationCreationTime The translation creation time of the descriptor.
+ * @property descriptor The descriptor.
+ */
 data class OONIRunFetchResponse(
     @JvmField
     val archived: Boolean,
+
     @JvmField
     @SerializedName("descriptor_creation_time")
     val creationTime: Date,
+
     @JvmField
     @SerializedName("translation_creation_time")
     val translationCreationTime: Date,
+
     @JvmField
     val descriptor: OONIRunDescriptor
 ) : Serializable
 
+/**
+ * Data class representing the OONI Run Descriptor.
+ *
+ * @property author The author of the nettest.
+ * @property description The description of the nettest in English.
+ * @property descriptionIntl The description of the nettest in other languages. The key is the language code and the value is the description in that language.
+ * @property icon The URL of the icon representing the nettest.
+ * @property color The color associated with the nettest in hexadecimal format.
+ * @property animation The URL of the animation representing the nettest.
+ * @property name The name of the nettest in English.
+ * @property nameIntl The name of the nettest in other languages. The key is the language code and the value is the name in that language.
+ * @property shortDescription The short description of the nettest in English.
+ * @property shortDescriptionIntl The short description of the nettest in other languages. The key is the language code and the value is the short description in that language.
+ * @property nettests A list of nettests associated with the run descriptor.
+ *
+ * @see [https://github.com/ooni/spec/blob/master/backends/bk-005-ooni-run-v2.md]
+ */
 data class OONIRunDescriptor(
     val author: String,
+
     val description: String,
+
     @SerializedName("description_intl")
     val descriptionIntl: HashMap<String, String>,
+
     val icon: String,
+
     val color: String,
+
     val animation: String,
+
     var name: String,
+
     @SerializedName("name_intl")
     val nameIntl: HashMap<String, String>,
+
     @SerializedName("short_description")
     val shortDescription: String,
+
     @SerializedName("short_description_intl")
     val shortDescriptionIntl: HashMap<String, String>,
+
     var nettests: List<OONIRunNettest>
 ) : Serializable
 
+/**
+ * Class representing a single OONI Run Nettest.
+ *
+ * @property name The name of the nettest.
+ * @property inputs The inputs of the nettest.
+ */
 open class OONIRunNettest(
     @SerializedName("test_name")
     open var name: String,
+
     open var inputs: List<String>?
 ) : Serializable