From db5dbbfdfd25c23a02277a533e4754d49fb27da4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Sch=C3=B6nberg?= Date: Wed, 24 May 2017 22:15:38 +0200 Subject: [PATCH 1/3] Fix crash of nearby list when the places list is null Currently the NearbyController crashes when it is called with a places list that is null. This is caused by the NearbyMapFragment not handling this state correctly. This commit adds tests for an empty list and null, which reproduce the problem. An early return in the NearbyController can make the tests pass, but might not be the best solution for this issue. --- .../nrw/commons/NearbyControllerTest.java | 57 +++++++++++++++++++ .../nrw/commons/nearby/NearbyController.java | 14 +++-- 2 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java diff --git a/app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java b/app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java new file mode 100644 index 000000000..e26c42265 --- /dev/null +++ b/app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java @@ -0,0 +1,57 @@ +package fr.free.nrw.commons; + +import android.content.Context; +import android.support.test.InstrumentationRegistry; +import android.support.test.runner.AndroidJUnit4; + +import fr.free.nrw.commons.location.LatLng; +import fr.free.nrw.commons.nearby.NearbyBaseMarker; +import fr.free.nrw.commons.nearby.NearbyController; +import fr.free.nrw.commons.nearby.Place; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.util.ArrayList; +import java.util.List; + +import static org.hamcrest.CoreMatchers.is; + +@RunWith(AndroidJUnit4.class) +public class NearbyControllerTest { + private Context instrumentationContext; + + @Before + public void setup() { + instrumentationContext = InstrumentationRegistry.getContext(); + } + + @Test public void testNullAttractions() { + LatLng location = new LatLng(0, 0); + + List options = + NearbyController.loadAttractionsFromLocationToBaseMarkerOptions( + location, + null, + instrumentationContext + ); + + Assert.assertThat(options.size(), is(0)); + } + + @Test public void testEmptyList() { + LatLng location = new LatLng(0, 0); + List emptyList = new ArrayList<>(); + + List options = + NearbyController.loadAttractionsFromLocationToBaseMarkerOptions( + location, + emptyList, + instrumentationContext + ); + + Assert.assertThat(options.size(), is(0)); + } +} 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 1c0699e17..dc6efe754 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 @@ -84,14 +84,20 @@ public class NearbyController { *Loads attractions from location for map view, we need to return BaseMarkerOption data type. * @param curLatLng users current location * @param placeList list of nearby places in Place data type - * @return BaseMarkerOprions list that holds nearby places + * @return BaseMarkerOptions list that holds nearby places */ public static List loadAttractionsFromLocationToBaseMarkerOptions( LatLng curLatLng, List placeList, Context context) { - List baseMarkerOptionses = new ArrayList<>(); + List baseMarkerOptions = new ArrayList<>(); + + if (placeList == null) { + return baseMarkerOptions; + } + placeList = placeList.subList(0, Math.min(placeList.size(), MAX_RESULTS)); + for (Place place: placeList) { String distance = formatDistanceBetween(curLatLng, place.location); place.setDistance(distance); @@ -108,8 +114,8 @@ public class NearbyController { nearbyBaseMarker.place(place); nearbyBaseMarker.icon(icon); - baseMarkerOptionses.add(nearbyBaseMarker); + baseMarkerOptions.add(nearbyBaseMarker); } - return baseMarkerOptionses; + return baseMarkerOptions; } } From 5574fa8b7c1e7b2b5de904696fd0ceb672cca9ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Sch=C3=B6nberg?= Date: Fri, 26 May 2017 22:45:12 +0200 Subject: [PATCH 2/3] Android studio import ordering --- .../java/fr/free/nrw/commons/NearbyControllerTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java b/app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java index e26c42265..585b5709f 100644 --- a/app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java +++ b/app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java @@ -4,11 +4,6 @@ import android.content.Context; import android.support.test.InstrumentationRegistry; import android.support.test.runner.AndroidJUnit4; -import fr.free.nrw.commons.location.LatLng; -import fr.free.nrw.commons.nearby.NearbyBaseMarker; -import fr.free.nrw.commons.nearby.NearbyController; -import fr.free.nrw.commons.nearby.Place; - import org.junit.Assert; import org.junit.Before; import org.junit.Test; @@ -17,6 +12,11 @@ import org.junit.runner.RunWith; import java.util.ArrayList; import java.util.List; +import fr.free.nrw.commons.location.LatLng; +import fr.free.nrw.commons.nearby.NearbyBaseMarker; +import fr.free.nrw.commons.nearby.NearbyController; +import fr.free.nrw.commons.nearby.Place; + import static org.hamcrest.CoreMatchers.is; @RunWith(AndroidJUnit4.class) From bc61f8d2ec74e9a6b471765a30b0b254c80654ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Sch=C3=B6nberg?= Date: Fri, 26 May 2017 23:00:54 +0200 Subject: [PATCH 3/3] Code style --- .../java/fr/free/nrw/commons/NearbyControllerTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java b/app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java index 585b5709f..1eb73b3d1 100644 --- a/app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java +++ b/app/src/androidTest/java/fr/free/nrw/commons/NearbyControllerTest.java @@ -32,11 +32,11 @@ public class NearbyControllerTest { LatLng location = new LatLng(0, 0); List options = - NearbyController.loadAttractionsFromLocationToBaseMarkerOptions( - location, - null, - instrumentationContext - ); + NearbyController.loadAttractionsFromLocationToBaseMarkerOptions( + location, + null, + instrumentationContext + ); Assert.assertThat(options.size(), is(0)); }