diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/DirectUpload.java b/app/src/main/java/fr/free/nrw/commons/nearby/DirectUpload.java index 5d82b5492..85439145e 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/DirectUpload.java +++ b/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"); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - if (ContextCompat.checkSelfPermission(fragment.getActivity(), READ_EXTERNAL_STORAGE) != PERMISSION_GRANTED) { - 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(); - } + // Only need to handle permissions for Marshmallow and above + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) { + return; } - else { + + Activity parentActivity = fragment.getActivity(); + if (parentActivity == null) { controller.startSingleGalleryPick(); + return; } + + // 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) -> { + parentActivity.requestPermissions(new String[]{READ_EXTERNAL_STORAGE}, PermissionUtils.GALLERY_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[]{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() { - Log.d("deneme7","initiateCameraUpload"); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { - if (ContextCompat.checkSelfPermission(fragment.getActivity(), WRITE_EXTERNAL_STORAGE) != PERMISSION_GRANTED) { - if (fragment.shouldShowRequestPermissionRationale(WRITE_EXTERNAL_STORAGE)) { - new AlertDialog.Builder(fragment.getActivity()) - .setMessage(fragment.getActivity().getString(R.string.write_storage_permission_rationale)) - .setPositiveButton(android.R.string.ok, (dialog, which) -> { - fragment.getActivity().requestPermissions - (new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); - dialog.dismiss(); - }) - .setNegativeButton(android.R.string.cancel, null) - .create() - .show(); - } else { - fragment.getActivity().requestPermissions(new String[]{WRITE_EXTERNAL_STORAGE}, PermissionUtils.CAMERA_PERMISSION_FROM_NEARBY_MAP); - } - } else { - controller.startCameraCapture(); - } - } else { - controller.startCameraCapture(); + // 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; + } + + // 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); } } diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java index e6ac27c90..7faebb70c 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyController.java +++ b/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 places = nearbyPlaces.getFromWikidataQuery(latLangToSearchAround, Locale.getDefault().getLanguage(), returnClosestResult); + List places = nearbyPlaces.radiusExpander(latLangToSearchAround, Locale.getDefault().getLanguage(), returnClosestResult); if (null != places && places.size() > 0) { LatLng[] boundaryCoordinates = {places.get(0).location, // south diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java b/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java index 903a93839..925149093 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/NearbyPlaces.java +++ b/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 - * is made according to returnClosestResult variable. If returnClosestResult true, then our - * number of min results set to 1, and max radius to search is set to 5. If there is no place - * in 5 km radius, empty list will be returned. This method sets radius variable according to - * search type (returnClosestResult or not), but search operation will be handled in overloaded - * method below, which is called from this method. - * @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 + * Expands the radius as needed for the Wikidata query + * @param curLatLng coordinates of search location + * @param lang user's language + * @param returnClosestResult true if only the nearest point is desired + * @return list of places obtained + * @throws IOException if query fails */ - List getFromWikidataQuery(LatLng curLatLng, String lang, boolean returnClosestResult) throws IOException { + List radiusExpander(LatLng curLatLng, String lang, boolean returnClosestResult) throws IOException { + + int minResults; + double maxRadius; + List places = Collections.emptyList(); - /** - * If returnClosestResult is true, then this means that we are trying to get closest point - * to use in cardView above contributions list - */ + // If returnClosestResult is true, then this means that we are trying to get closest point + // to use in cardView in Contributions fragment if (returnClosestResult) { - MIN_RESULTS = 1; // Return closest nearby place - MAX_RADIUS = 5; // Return places only in 5 km area + minResults = 1; // Return closest nearby place + 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; - MAX_RADIUS = 300.0; // in kilometers + minResults = 40; + maxRadius = 300.0; // in kilometers radius = INITIAL_RADIUS; } - // increase the radius gradually to find a satisfactory number of nearby places - while (radius <= MAX_RADIUS) { + // Increase the radius gradually to find a satisfactory number of nearby places + 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) { - radius = MAX_RADIUS; + if (radius > maxRadius) { + radius = maxRadius; } - return places; } /** - * Creates and sends query for nearby wikidata items needs picture. - * Reads results from query and turns them into meaningful place variables. - * Adds them to the list of places. - * @param cur Our reference location to check around - * @param lang Language we will display results in - * @param radius Our query area is a circle, where cur is center and radius is radius - * @return - * @throws IOException + * Runs the Wikidata query to populate the Places around search location + * @param cur coordinates of search location + * @param lang user's language + * @param radius radius for search, as determined by radiusExpander() + * @return list of places obtained + * @throws IOException if query fails */ - private List getFromWikidataQuery(LatLng cur, - String lang, - double radius) - throws IOException { + private List getFromWikidataQuery(LatLng cur, String lang, double radius) throws IOException { List 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() - .appendQueryParameter("query", query).build().toString(); + String url = WIKIDATA_QUERY_URL.buildUpon().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 = - Pattern.compile("Point\\(([^ ]+) ([^ ]+)\\)").matcher(point); + Matcher matcher = Pattern.compile("Point\\(([^ ]+) ([^ ]+)\\)").matcher(point); if (!matcher.find()) { continue; } @@ -192,5 +181,4 @@ public class NearbyPlaces { return places; } - } diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/Place.java b/app/src/main/java/fr/free/nrw/commons/nearby/Place.java index 7635ac06f..d3eca0465 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/Place.java +++ b/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) { diff --git a/app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java b/app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java index f4925af0e..e67136286 100644 --- a/app/src/main/java/fr/free/nrw/commons/nearby/Sitelinks.java +++ b/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 CREATOR = new Creator() { @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;