Javadocs and minor refactoring for Nearby package (#2188)

* Add Traceur for getting meaningful RxJava stack traces (#1832)

* Hotfix for overwrite issue in 2.8.0  (#1838)

* This solution is an hotfix for overrite issue came back on 2.8.0 version. What I did is checking the extension, and if it is null, adding .jpg suffix. Because commons files always have suffixes, and we should compare file names after adding suffixes. Othervise overrides are possible.

* Check if file title includes an extension already, by checking if is there any dot in it.

* Fix logic error

* Add uncovered tests

* Remove unecessary line breaks

* Make Javadocs more explicit

* Versioning and changelog for v2.8.2 (#1842)

* Versioning for v2.8.2

* Changelog for v2.8.2

* Delete unused MaterialShowcase class

* Add Javadocs and fix lint errors for DirectUpload.java

* Fix whitespace and add docs

* Replace fragment.getActivity() with the parentActivity var

* Rename unnecessarily-overloaded method getFromWikidataQuery(), add Javadocs

* Javadocs and whitespaces for NearbyPlaces.java

* Use local vars where possible instead of class fields. Non-constants should not be in all caps

* Missed one unnecessary class field

* Remove unnecessary whitespaces that don't improve readability

* Add class summary

* Optimize imports

* Fix access modifiers in Place.java

* Clearer Javadocs

* Add Javadocs to Place.java

* Remove residual conflict

* Fix lint issues in Sitelinks

* Javadocs for Sitelinks.java

* DirectUpload: Replace nested conditionals with guard clauses

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

@@ -1,20 +1,22 @@
 package fr.free.nrw.commons.nearby;
 
+import android.app.Activity;
 import android.os.Build;
 import android.support.v4.app.Fragment;
 import android.support.v4.content.ContextCompat;
 import android.support.v7.app.AlertDialog;
-import android.util.Log;
 
 import fr.free.nrw.commons.R;
 import fr.free.nrw.commons.contributions.ContributionController;
 import fr.free.nrw.commons.utils.PermissionUtils;
-import timber.log.Timber;
 
 import static android.Manifest.permission.READ_EXTERNAL_STORAGE;
 import static android.Manifest.permission.WRITE_EXTERNAL_STORAGE;
 import static android.content.pm.PackageManager.PERMISSION_GRANTED;
 
+/**
+ * Initiates the uploads made from a Nearby Place, in both the list and map views.
+ */
 class DirectUpload {
 
     private ContributionController controller;
@@ -25,59 +27,85 @@ class DirectUpload {
         this.controller = controller;
     }
 
-    // These permission requests will be handled by the Fragments.
+    /**
-    // Do not use requestCode 1 as it will conflict with NearbyFragment's requestCodes
+     * Initiates the upload if user selects the Gallery FAB.
+     * The permission requests will be handled by the Fragments.
+     * Do not use requestCode 1 as it will conflict with NearbyFragment's requestCodes.
+     */
     void initiateGalleryUpload() {
-        Log.d("deneme7","initiateGalleryUpload");
+        // Only need to handle permissions for Marshmallow and above
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
-            if (ContextCompat.checkSelfPermission(fragment.getActivity(), READ_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
+            return;
-                if (fragment.shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) {
-                    new AlertDialog.Builder(fragment.getActivity())
-                            .setMessage(fragment.getActivity().getString(R.string.read_storage_permission_rationale))
-                            .setPositiveButton(android.R.string.ok, (dialog, which) -> {
-                                Timber.d("Requesting permissions for read external storage");
-                                fragment.getActivity().requestPermissions
-                                        (new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP);
-                                dialog.dismiss();
-                            })
-                            .setNegativeButton(android.R.string.cancel, null)
-                            .create()
-                            .show();
-                } else {
-                    fragment.getActivity().requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP);
-                }
-            } else {
-                controller.startSingleGalleryPick();
-            }
-        }
-        else {
-            controller.startSingleGalleryPick();
-        }
         }
 
-    void initiateCameraUpload() {
+        Activity parentActivity = fragment.getActivity();
-        Log.d("deneme7","initiateCameraUpload");
+        if (parentActivity == null) {
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
+            controller.startSingleGalleryPick();
-            if (ContextCompat.checkSelfPermission(fragment.getActivity(), WRITE_EXTERNAL_STORAGE) != PERMISSION_GRANTED) {
+            return;
-                if (fragment.shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE)) {
+        }
-                    new AlertDialog.Builder(fragment.getActivity())
+
-                            .setMessage(fragment.getActivity().getString(R.string.write_storage_permission_rationale))
+        // If we have permission, go ahead
+        if (ContextCompat.checkSelfPermission(parentActivity, READ_EXTERNAL_STORAGE) == PERMISSION_GRANTED) {
+            controller.startSingleGalleryPick();
+            return;
+        }
+
+        // If we don't have permission, and we need to show the rationale, show the rationale
+        if (fragment.shouldShowRequestPermissionRationale(READ_EXTERNAL_STORAGE)) {
+            new AlertDialog.Builder(parentActivity)
+                    .setMessage(parentActivity.getString(R.string.read_storage_permission_rationale))
                     .setPositiveButton(android.R.string.ok, (dialog, which) -> {
-                                fragment.getActivity().requestPermissions
+                        parentActivity.requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP);
-                                        (new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP);
                         dialog.dismiss();
                     })
                     .setNegativeButton(android.R.string.cancel, null)
                     .create()
                     .show();
-                } else {
+            return;
-                    fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP);
         }
-            } else {
+
+        // If we don't have permission, and we don't need to show rationale just request permission
+        parentActivity.requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_PERMISSION_FROM_NEARBY_MAP);
+    }
+
+    /**
+     * Initiates the upload if user selects the Camera FAB.
+     * The permission requests will be handled by the Fragments.
+     * Do not use requestCode 1 as it will conflict with NearbyFragment's requestCodes.
+     */
+    void initiateCameraUpload() {
+        // Only need to handle permissions for Marshmallow and above
+        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
+            return;
+        }
+
+        Activity parentActivity = fragment.getActivity();
+        if (parentActivity == null) {
             controller.startCameraCapture();
+            return;
         }
-        } else {
+
+        // If we have permission, go ahead
+        if (ContextCompat.checkSelfPermission(parentActivity, WRITE_EXTERNAL_STORAGE) == PERMISSION_GRANTED) {
             controller.startCameraCapture();
+            return;
         }
+
+        // If we don't have permission, and we need to show the rationale, show the rationale
+        if (fragment.shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE)) {
+            new AlertDialog.Builder(parentActivity)
+                    .setMessage(parentActivity.getString(R.string.write_storage_permission_rationale))
+                    .setPositiveButton(android.R.string.ok, (dialog, which) -> {
+                        parentActivity.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP);
+                        dialog.dismiss();
+                    })
+                    .setNegativeButton(android.R.string.cancel, null)
+                    .create()
+                    .show();
+            return;
+        }
+
+        // If we don't have permission, and we don't need to show rationale just request permission
+        parentActivity.requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP);
     }
 }

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

@@ -60,7 +60,7 @@ public class NearbyController {
             Timber.d("Loading attractions neari, but curLatLng is null");
             return null;
         }
-        List<Place> places = nearbyPlaces.getFromWikidataQuery(latLangToSearchAround, Locale.getDefault().getLanguage(), returnClosestResult);
+        List<Place> places = nearbyPlaces.radiusExpander(latLangToSearchAround, Locale.getDefault().getLanguage(), returnClosestResult);
 
         if (null != places && places.size() > 0) {
             LatLng[] boundaryCoordinates = {places.get(0).location,   // south

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

@@ -21,11 +21,12 @@ import fr.free.nrw.commons.location.LatLng;
 import fr.free.nrw.commons.upload.FileUtils;
 import timber.log.Timber;
 
+/**
+ * Handles the Wikidata query to obtain Places around search location
+ */
 public class NearbyPlaces {
 
-    private static int MIN_RESULTS = 40;
     private static final double INITIAL_RADIUS = 1.0; // in kilometers
-    private static double MAX_RADIUS = 300.0; // in kilometers
     private static final double RADIUS_MULTIPLIER = 1.618;
     private static final Uri WIKIDATA_QUERY_URL = Uri.parse("https://query.wikidata.org/sparql");
     private static final Uri WIKIDATA_QUERY_UI_URL = Uri.parse("https://query.wikidata.org/");
@@ -46,38 +47,34 @@ public class NearbyPlaces {
     }
 
     /**
-     * Returns list of nearby places around curLatLng or closest result near curLatLng. This decision
+     * Expands the radius as needed for the Wikidata query
-     * is made according to returnClosestResult variable. If returnClosestResult true, then our
+     * @param curLatLng coordinates of search location
-     * number of min results set to 1, and max radius to search is set to 5. If there is no place
+     * @param lang user's language
-     * in 5 km radius, empty list will be returned. This method sets radius variable according to
+     * @param returnClosestResult true if only the nearest point is desired
-     * search type (returnClosestResult or not), but search operation will be handled in overloaded
+     * @return list of places obtained
-     * method below, which is called from this method.
+     * @throws IOException if query fails
-     * @param curLatLng Our reference location
-     * @param lang Language we will display results in
-     * @param returnClosestResult If true, return only first result found, else found satisfactory
-     *                            number of results
-     * @return List of nearby places around, or closest nearby place
-     * @throws IOException
      */
-    List<Place> getFromWikidataQuery(LatLng curLatLng, String lang, boolean returnClosestResult) throws IOException {
+    List<Place> radiusExpander(LatLng curLatLng, String lang, boolean returnClosestResult) throws IOException {
+
+        int minResults;
+        double maxRadius;
+
         List<Place> places = Collections.emptyList();
 
-        /**
+        // If returnClosestResult is true, then this means that we are trying to get closest point
-         * If returnClosestResult is true, then this means that we are trying to get closest point
+        // to use in cardView in Contributions fragment
-         * to use in cardView above contributions list
-         */
         if (returnClosestResult) {
-            MIN_RESULTS = 1; // Return closest nearby place
+            minResults = 1; // Return closest nearby place
-            MAX_RADIUS = 5;  // Return places only in 5 km area
+            maxRadius = 5;  // Return places only in 5 km area
             radius = INITIAL_RADIUS; // refresh radius again, otherwise increased radius is grater than MAX_RADIUS, thus returns null
         } else {
-            MIN_RESULTS = 40;
+            minResults = 40;
-            MAX_RADIUS = 300.0; // in kilometers
+            maxRadius = 300.0; // in kilometers
             radius = INITIAL_RADIUS;
         }
 
-            // increase the radius gradually to find a satisfactory number of nearby places
+            // Increase the radius gradually to find a satisfactory number of nearby places
-            while (radius <= MAX_RADIUS) {
+            while (radius <= maxRadius) {
                 try {
                     places = getFromWikidataQuery(curLatLng, lang, radius);
                 } catch (InterruptedIOException e) {
@@ -85,34 +82,28 @@ public class NearbyPlaces {
                     return places;
                 }
                 Timber.d("%d results at radius: %f", places.size(), radius);
-                if (places.size() >= MIN_RESULTS) {
+                if (places.size() >= minResults) {
                     break;
                 } else {
                     radius *= RADIUS_MULTIPLIER;
                 }
             }
         // make sure we will be able to send at least one request next time
-        if (radius > MAX_RADIUS) {
+        if (radius > maxRadius) {
-            radius = MAX_RADIUS;
+            radius = maxRadius;
         }
-
         return places;
     }
 
     /**
-     * Creates and sends query for nearby wikidata items needs picture.
+     * Runs the Wikidata query to populate the Places around search location
-     * Reads results from query and turns them into meaningful place variables.
+     * @param cur coordinates of search location
-     * Adds them to the list of places.
+     * @param lang user's language
-     * @param cur Our reference location to check around
+     * @param radius radius for search, as determined by radiusExpander()
-     * @param lang Language we will display results in
+     * @return list of places obtained
-     * @param radius Our query area is a circle, where cur is center and radius is radius
+     * @throws IOException if query fails
-     * @return
-     * @throws IOException
      */
-    private List<Place> getFromWikidataQuery(LatLng cur,
+    private List<Place> getFromWikidataQuery(LatLng cur, String lang, double radius) throws IOException {
-                                             String lang,
-                                             double radius)
-            throws IOException {
         List<Place> places = new ArrayList<>();
 
         String query = wikidataQuery
@@ -125,8 +116,7 @@ public class NearbyPlaces {
 
         // format as a URL
         Timber.d(WIKIDATA_QUERY_UI_URL.buildUpon().fragment(query).build().toString());
-        String url = WIKIDATA_QUERY_URL.buildUpon()
+        String url = WIKIDATA_QUERY_URL.buildUpon().appendQueryParameter("query", query).build().toString();
-                .appendQueryParameter("query", query).build().toString();
         URLConnection conn = new URL(url).openConnection();
         conn.setRequestProperty("Accept", "text/tab-separated-values");
         BufferedReader in = new BufferedReader(new InputStreamReader(conn.getInputStream()));
@@ -162,8 +152,7 @@ public class NearbyPlaces {
 
             double latitude;
             double longitude;
-            Matcher matcher =
+            Matcher matcher = Pattern.compile("Point\\(([^ ]+) ([^ ]+)\\)").matcher(point);
-                    Pattern.compile("Point\\(([^ ]+) ([^ ]+)\\)").matcher(point);
             if (!matcher.find()) {
                 continue;
             }
@@ -192,5 +181,4 @@ public class NearbyPlaces {
 
         return places;
     }
-
 }

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

@@ -12,6 +12,9 @@ import fr.free.nrw.commons.R;
 import fr.free.nrw.commons.location.LatLng;
 import timber.log.Timber;
 
+/**
+ * A single geolocated Wikidata item
+ */
 public class Place {
 
     public final String name;
@@ -22,7 +25,7 @@ public class Place {
     private final String category;
 
     public Bitmap image;
-    public Bitmap secondaryImage;
+    private Bitmap secondaryImage;
     public String distance;
     public final Sitelinks siteLinks;
 
@@ -38,20 +41,44 @@ public class Place {
         this.siteLinks = siteLinks;
     }
 
+    /**
+     * Gets the name of the place
+     * @return name
+     */
     public String getName() { return name; }
 
+    /** Gets the label of the place
+     * e.g. "building", "city", etc
+     * @return label
+     */
     public Label getLabel() {
         return label;
     }
 
+    /**
+     * Gets the long description of the place
+     * @return long description
+     */
     public String getLongDescription() { return longDescription; }
 
+    /**
+     * Gets the Commons category of the place
+     * @return Commons category
+     */
     public String getCategory() {return category; }
 
+    /**
+     * Sets the distance of the place from the user's location
+     * @param distance distance of place from user's location
+     */
     public void setDistance(String distance) {
         this.distance = distance;
     }
 
+    /**
+     * Gets the secondary image url for bookmarks
+     * @return secondary image url
+     */
     public Uri getSecondaryImageUrl() { return this.secondaryImageUrl; }
 
     /**
@@ -59,7 +86,7 @@ public class Place {
      * @return returns the entity id if wikidata link exists
      */
     @Nullable
-    public String getWikiDataEntityId() {
+    String getWikiDataEntityId() {
         if (!hasWikidataLink()) {
             Timber.d("Wikidata entity ID is null for place with sitelink %s", siteLinks.toString());
             return null;
@@ -70,18 +97,35 @@ public class Place {
         return wikiDataLink.replace("http://www.wikidata.org/entity/", "");
     }
 
-    public boolean hasWikipediaLink() {
+    /**
+     * Checks if the Wikidata item has a Wikipedia page associated with it
+     * @return true if there is a Wikipedia link
+     */
+    boolean hasWikipediaLink() {
         return !(siteLinks == null || Uri.EMPTY.equals(siteLinks.getWikipediaLink()));
     }
 
-    public boolean hasWikidataLink() {
+    /**
+     * Checks if the Wikidata item has a Wikidata page associated with it
+     * @return true if there is a Wikidata link
+     */
+    boolean hasWikidataLink() {
         return !(siteLinks == null || Uri.EMPTY.equals(siteLinks.getWikidataLink()));
     }
 
-    public boolean hasCommonsLink() {
+    /**
+     * Checks if the Wikidata item has a Commons page associated with it
+     * @return true if there is a Commons link
+     */
+    boolean hasCommonsLink() {
         return !(siteLinks == null || Uri.EMPTY.equals(siteLinks.getCommonsLink()));
     }
 
+    /**
+     * Check if we already have the exact same Place
+     * @param o Place being tested
+     * @return true if name and location of Place is exactly the same
+     */
     @Override
     public boolean equals(Object o) {
         if (o instanceof Place) {

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

@@ -5,13 +5,16 @@ import android.os.Parcel;
 import android.os.Parcelable;
 import android.support.annotation.Nullable;
 
+/**
+ * Handles the links to Wikipedia, Commons, and Wikidata that are displayed for a Place
+ */
 public class Sitelinks implements Parcelable {
     private final String wikipediaLink;
     private final String commonsLink;
     private final String wikidataLink;
 
 
-    protected Sitelinks(Parcel in) {
+    private Sitelinks(Parcel in) {
         wikipediaLink = in.readString();
         commonsLink = in.readString();
         wikidataLink = in.readString();
@@ -29,6 +32,9 @@ public class Sitelinks implements Parcelable {
         return 0;
     }
 
+    /**
+     * Creates sitelinks from the parcel
+     */
     public static final Creator<Sitelinks> CREATOR = new Creator<Sitelinks>() {
         @Override
         public Sitelinks createFromParcel(Parcel in) {
@@ -41,18 +47,35 @@ public class Sitelinks implements Parcelable {
         }
     };
 
+    /**
+     * Gets the Wikipedia link for a Place
+     * @return Wikipedia link
+     */
     public Uri getWikipediaLink() {
         return sanitiseString(wikipediaLink);
     }
 
+    /**
+     * Gets the Commons link for a Place
+     * @return Commons link
+     */
     public Uri getCommonsLink() {
         return sanitiseString(commonsLink);
     }
 
+    /**
+     * Gets the Wikidata link for a Place
+     * @return Wikidata link
+     */
     public Uri getWikidataLink() {
         return sanitiseString(wikidataLink);
     }
 
+    /**
+     * Sanitises and parses the links before using them
+     * @param stringUrl unsanitised link
+     * @return sanitised and parsed link
+     */
     private static Uri sanitiseString(String stringUrl) {
         String sanitisedStringUrl = stringUrl.replaceAll("[<>\n\r]", "").trim();
         return Uri.parse(sanitisedStringUrl);
@@ -73,6 +96,9 @@ public class Sitelinks implements Parcelable {
         this.commonsLink = builder.commonsLink;
     }
 
+    /**
+     * Builds a list of Sitelinks for a Place
+     */
     public static class Builder {
         private String wikidataLink;
         private String commonsLink;