[android] - harden offline region creation, add include method, expose world bounds, update tests (#8517)

7 files changed, 252 insertions, 32 deletions

diff --git a/platform/android/CHANGELOG.md b/platform/android/CHANGELOG.md
index d8338347c2..ec0a5f1586 100644
--- a/platform/android/CHANGELOG.md
+++ b/platform/android/CHANGELOG.md
@@ -2,6 +2,16 @@
 
 Mapbox welcomes participation and contributions from everyone.  If you'd like to do so please see the [`Contributing Guide`](https://github.com/mapbox/mapbox-gl-native/blob/master/CONTRIBUTING.md) first to get started.
 
+## 5.1.0 - TBA
+
+5.1.0 builds further on 5.0.1 and adds:
+
+* LatLngBounds includes with another bounds [#8517](https://github.com/mapbox/mapbox-gl-native/pull/8517)
+* LatLngBounds includes takes in account LatLng on the edges (cfr. core) [#8517](https://github.com/mapbox/mapbox-gl-native/pull/8517)
+* LatLngBounds facility getters/setters for LatLnbg on the edges of the bounds [#8517](https://github.com/mapbox/mapbox-gl-native/pull/8517)
+* Expose world bounds in LatLngBounds [#8517](https://github.com/mapbox/mapbox-gl-native/pull/8517)
+* OfflineRegion are validated if the bounds is found in the world bounds, else onError will be invoked [#8517](https://github.com/mapbox/mapbox-gl-native/pull/8517)
+
 ## 5.0.0 - March 17th, 2017
 
 5.0.0 final release contains:
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java
index 3b92f0f0f5..4a4e2a30aa 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/geometry/LatLngBounds.java
@@ -5,12 +5,16 @@ import android.os.Parcelable;
 import android.support.annotation.NonNull;
 
 import com.mapbox.mapboxsdk.exceptions.InvalidLatLngBoundsException;
+import com.mapbox.services.android.telemetry.constants.GeoConstants;
 
 import java.util.ArrayList;
 import java.util.List;
 
 /**
  * A geographical area representing a latitude/longitude aligned rectangle.
+ * <p>
+ * This class does not wrap values to the world bounds.
+ * </p>
  */
 public class LatLngBounds implements Parcelable {
 
@@ -37,6 +41,18 @@ public class LatLngBounds implements Parcelable {
   }
 
   /**
+   * Returns the world bounds.
+   *
+   * @return the bounds representing the world
+   */
+  public static LatLngBounds world() {
+    return new LatLngBounds.Builder()
+      .include(new LatLng(GeoConstants.MAX_LATITUDE, GeoConstants.MAX_LONGITUDE))
+      .include(new LatLng(GeoConstants.MIN_LATITUDE, GeoConstants.MIN_LONGITUDE))
+      .build();
+  }
+
+  /**
    * Calculates the centerpoint of this LatLngBounds by simple interpolation and returns
    * it as a point. This is a non-geodesic calculation which is not the geographic center.
    *
@@ -47,23 +63,79 @@ public class LatLngBounds implements Parcelable {
       (this.mLonEast + this.mLonWest) / 2);
   }
 
+  /**
+   * Get the north latitude value of this bounds.
+   *
+   * @return double latitude value for north
+   */
   public double getLatNorth() {
     return this.mLatNorth;
   }
 
+  /**
+   * Get the south latitude value of this bounds.
+   *
+   * @return double latitude value for south
+   */
   public double getLatSouth() {
     return this.mLatSouth;
   }
 
+  /**
+   * Get the east longitude value of this bounds.
+   *
+   * @return double longitude value for east
+   */
   public double getLonEast() {
     return this.mLonEast;
   }
 
+  /**
+   * Get the west longitude value of this bounds.
+   *
+   * @return double longitude value for west
+   */
   public double getLonWest() {
     return this.mLonWest;
   }
 
   /**
+   * Get the latitude-longitude pair of the south west corner of this bounds.
+   *
+   * @return LatLng of the south west corner
+   */
+  public LatLng getSouthWest() {
+    return new LatLng(mLatSouth, mLonWest);
+  }
+
+  /**
+   * Get the latitude-longitude paur if the north east corner of this bounds.
+   *
+   * @return LatLng of the north east corner
+   */
+  public LatLng getNorthEast() {
+    return new LatLng(mLatNorth, mLonEast);
+  }
+
+  /**
+   * Get the latitude-longitude pair of the south east corner of this bounds.
+   *
+   * @return LatLng of the south east corner
+   */
+  public LatLng getSouthEast() {
+    return new LatLng(mLatSouth, mLonEast);
+  }
+
+  /**
+   * Get the latitude-longitude pair of the north west corner of this bounds.
+   *
+   * @return LatLng of the north west corner
+   */
+  public LatLng getNorthWest() {
+    return new LatLng(mLatNorth, mLonWest);
+  }
+
+  /**
    * Get the area spanned by this LatLngBounds
    *
    * @return LatLngSpan area
@@ -133,8 +205,27 @@ public class LatLngBounds implements Parcelable {
     return new LatLngBounds(maxLat, maxLon, minLat, minLon);
   }
 
+  /**
+   * Return an array of LatLng objects resembling this bounds.
+   *
+   * @return an array of 2 LatLng objects.
+   */
   public LatLng[] toLatLngs() {
-    return new LatLng[] {new LatLng(mLatNorth, mLonEast), new LatLng(mLatSouth, mLonWest)};
+    return new LatLng[] {getNorthEast(), getSouthWest()};
+  }
+
+  /**
+   * Constructs a LatLngBounds from current bounds with an additional latitude-longitude pair.
+   *
+   * @param latLng the latitude lognitude pair to include in the bounds.
+   * @return the newly constructed bounds
+   */
+  public LatLngBounds include(LatLng latLng) {
+    return new LatLngBounds.Builder()
+      .include(getNorthEast())
+      .include(getSouthWest())
+      .include(latLng)
+      .build();
   }
 
   /**
@@ -159,19 +250,28 @@ public class LatLngBounds implements Parcelable {
   }
 
   /**
-   * Determines whether this LatLngBounds contains a point and the point
-   * does not touch its boundary.
+   * Determines whether this LatLngBounds contains a point.
    *
    * @param latLng the point which may be contained
-   * @return true, if the point is contained within the box.
+   * @return true, if the point is contained within the bounds
    */
   public boolean contains(final ILatLng latLng) {
     final double latitude = latLng.getLatitude();
     final double longitude = latLng.getLongitude();
-    return ((latitude < this.mLatNorth)
-      && (latitude > this.mLatSouth))
-      && ((longitude < this.mLonEast)
-      && (longitude > this.mLonWest));
+    return ((latitude <= this.mLatNorth)
+      && (latitude >= this.mLatSouth))
+      && ((longitude <= this.mLonEast)
+      && (longitude >= this.mLonWest));
+  }
+
+  /**
+   * Determines whether this LatLngBounds contains another bounds.
+   *
+   * @param other the bounds which may be contained
+   * @return true, if the bounds is contained within the bounds
+   */
+  public boolean contains(final LatLngBounds other) {
+    return contains(other.getNorthEast()) && contains(other.getSouthWest());
   }
 
   /**
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineManager.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineManager.java
index 8bf19c4065..225278b17d 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineManager.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineManager.java
@@ -5,6 +5,8 @@ import android.os.Handler;
 import android.os.Looper;
 import android.support.annotation.NonNull;
 
+import com.mapbox.mapboxsdk.R;
+import com.mapbox.mapboxsdk.geometry.LatLngBounds;
 import com.mapbox.mapboxsdk.net.ConnectivityReceiver;
 import com.mapbox.mapboxsdk.storage.FileSource;
 
@@ -26,7 +28,6 @@ public class OfflineManager {
     System.loadLibrary("mapbox-gl");
   }
 
-
   // Native peer pointer
   private long nativePtr;
 
@@ -139,7 +140,8 @@ public class OfflineManager {
    *
    * @param callback the callback to be invoked
    */
-  public void listOfflineRegions(@NonNull final ListOfflineRegionsCallback callback) {
+  public void listOfflineRegions(@NonNull
+                                 final ListOfflineRegionsCallback callback) {
     listOfflineRegions(fileSource, new ListOfflineRegionsCallback() {
 
       @Override
@@ -180,10 +182,15 @@ public class OfflineManager {
    * @param metadata   the metadata in bytes
    * @param callback   the callback to be invoked
    */
-  public void createOfflineRegion(
-    @NonNull OfflineRegionDefinition definition,
-    @NonNull byte[] metadata,
-    @NonNull final CreateOfflineRegionCallback callback) {
+  public void createOfflineRegion(@NonNull OfflineRegionDefinition definition, @NonNull byte[] metadata,
+                                  final CreateOfflineRegionCallback callback) {
+    if (!isValidOfflineRegionDefinition(definition)) {
+      callback.onError(
+        String.format(context.getString(R.string.mapbox_offline_error_region_definition_invalid),
+          definition.getBounds())
+      );
+      return;
+    }
 
     ConnectivityReceiver.instance(context).activate();
     createOfflineRegion(fileSource, definition, metadata, new CreateOfflineRegionCallback() {
@@ -212,6 +219,16 @@ public class OfflineManager {
     });
   }
 
+  /**
+   * Validates if the offline region definition bounds is valid for an offline region download.
+   *
+   * @param definition the offline region definition
+   * @return true if the region fits the world bounds.
+   */
+  private boolean isValidOfflineRegionDefinition(OfflineRegionDefinition definition) {
+    return LatLngBounds.world().contains(definition.getBounds());
+  }
+
   /*
   * Changing or bypassing this limit without permission from Mapbox is prohibited
   * by the Mapbox Terms of Service.
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineRegionDefinition.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineRegionDefinition.java
index a21ff0a443..18d662a286 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineRegionDefinition.java
+++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/offline/OfflineRegionDefinition.java
@@ -1,9 +1,14 @@
 package com.mapbox.mapboxsdk.offline;
 
+import com.mapbox.mapboxsdk.geometry.LatLngBounds;
+
 /**
  * This is the interface that all Offline Region definitions have to implement.
  * <p>
  * For the present, a tile pyramid is the only type of offline region.
  */
 public interface OfflineRegionDefinition {
+
+  LatLngBounds getBounds();
+
 }
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/res-public/values/public.xml b/platform/android/MapboxGLAndroidSDK/src/main/res-public/values/public.xml
index 7283aced5f..b544a257a5 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/res-public/values/public.xml
+++ b/platform/android/MapboxGLAndroidSDK/src/main/res-public/values/public.xml
@@ -89,6 +89,9 @@
     <public name="mapbox_style_satellite" type="string" />
     <public name="mapbox_style_satellite_streets" type="string" />
 
+    <!-- Exposed error messages -->
+    <public name="mapbox_offline_error_region_definition_invalid" type="string" />
+
     <!-- Exposed drawables -->
     <public name="mapbox_logo_icon" type="drawable" />
     <public name="mapbox_compass_icon" type="drawable" />
diff --git a/platform/android/MapboxGLAndroidSDK/src/main/res/values/strings.xml b/platform/android/MapboxGLAndroidSDK/src/main/res/values/strings.xml
index 0d8e9bdc49..d4a9b2f58b 100644
--- a/platform/android/MapboxGLAndroidSDK/src/main/res/values/strings.xml
+++ b/platform/android/MapboxGLAndroidSDK/src/main/res/values/strings.xml
@@ -13,6 +13,7 @@
     <string name="mapbox_infoWindowTitle">Title</string>
     <string name="mapbox_infoWindowDescription">Description</string>
     <string name="mapbox_infoWindowAddress">Address</string>
+    <string name="mapbox_offline_error_region_definition_invalid">Provided OfflineRegionDefinition doesn\'t fit the world bounds: %s</string>
 
     <!-- these are public -->
     <!-- Using one of these constants means your map style will always use the latest version and
diff --git a/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java b/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java
index 60573f4b74..4630f9f8b6 100644
--- a/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java
+++ b/platform/android/MapboxGLAndroidSDKTestApp/src/test/java/com/mapbox/mapboxsdk/geometry/LatLngBoundsTest.java
@@ -4,6 +4,7 @@ import android.os.Parcelable;
 
 import com.mapbox.mapboxsdk.exceptions.InvalidLatLngBoundsException;
 import com.mapbox.mapboxsdk.utils.MockParcel;
+import com.mapbox.services.android.telemetry.constants.GeoConstants;
 
 import org.junit.Before;
 import org.junit.Test;
@@ -11,10 +12,10 @@ import org.junit.Test;
 import java.util.ArrayList;
 import java.util.List;
 
+import static junit.framework.Assert.assertNotNull;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 
@@ -42,39 +43,39 @@ public class LatLngBoundsTest {
   }
 
   @Test(expected = InvalidLatLngBoundsException.class)
-  public void testNoLatLngs() {
+  public void noLatLngs() {
     new LatLngBounds.Builder().build();
   }
 
   @Test(expected = InvalidLatLngBoundsException.class)
-  public void testOneLatLngs() {
+  public void oneLatLngs() {
     new LatLngBounds.Builder().include(LAT_LNG_NULL_ISLAND).build();
   }
 
   @Test
-  public void testLatitiudeSpan() {
+  public void latitiudeSpan() {
     assertEquals("Span should be the same", 2, latLngBounds.getLatitudeSpan(), DELTA);
   }
 
   @Test
-  public void testLongitudeSpan() {
+  public void longitudeSpan() {
     assertEquals("Span should be the same", 2, latLngBounds.getLongitudeSpan(), DELTA);
   }
 
   @Test
-  public void testCoordinateSpan() {
+  public void coordinateSpan() {
     LatLngSpan latLngSpan = latLngBounds.getSpan();
     assertEquals("LatLngSpan should be the same", new LatLngSpan(2, 2), latLngSpan);
   }
 
   @Test
-  public void testCenter() {
+  public void center() {
     LatLng center = latLngBounds.getCenter();
     assertEquals("Center should match", new LatLng(1, 1), center);
   }
 
   @Test
-  public void testEmptySpan() {
+  public void emptySpan() {
     latLngBounds = new LatLngBounds.Builder()
       .include(LAT_LNG_NOT_NULL_ISLAND)
       .include(LAT_LNG_NOT_NULL_ISLAND)
@@ -83,7 +84,7 @@ public class LatLngBoundsTest {
   }
 
   @Test
-  public void testNotEmptySpan() {
+  public void notEmptySpan() {
     latLngBounds = new LatLngBounds.Builder()
       .include(LAT_LNG_NOT_NULL_ISLAND)
       .include(LAT_LNG_NULL_ISLAND)
@@ -92,7 +93,7 @@ public class LatLngBoundsTest {
   }
 
   @Test
-  public void testToLatLngs() {
+  public void toLatLngs() {
     latLngBounds = new LatLngBounds.Builder()
       .include(LAT_LNG_NOT_NULL_ISLAND)
       .include(LAT_LNG_NULL_ISLAND)
@@ -104,12 +105,12 @@ public class LatLngBoundsTest {
   }
 
   @Test
-  public void testIncluding() {
+  public void include() {
     assertTrue("LatLng should be included", latLngBounds.contains(new LatLng(1, 1)));
   }
 
   @Test
-  public void testIncludes() {
+  public void includes() {
     List<LatLng> points = new ArrayList<>();
     points.add(LAT_LNG_NULL_ISLAND);
     points.add(LAT_LNG_NOT_NULL_ISLAND);
@@ -127,17 +128,40 @@ public class LatLngBoundsTest {
   }
 
   @Test
-  public void testNoIncluding() {
+  public void containsNot() {
     assertFalse("LatLng should not be included", latLngBounds.contains(new LatLng(3, 1)));
   }
 
   @Test
+  public void containsBoundsInWorld() {
+    assertTrue("LatLngBounds should be contained in the world", LatLngBounds.world().contains(latLngBounds));
+  }
+
+  @Test
+  public void containsNotBiggerBoundsInWorld() {
+    LatLngBounds biggerWorldBounds = new LatLngBounds.Builder()
+      .include(new LatLng(GeoConstants.MAX_LATITUDE + 10, GeoConstants.MIN_LONGITUDE - 10))
+      .include(new LatLng(GeoConstants.MIN_LATITUDE - 10, GeoConstants.MAX_LONGITUDE + 10))
+      .build();
+    assertFalse("Bounds should not be contained ", LatLngBounds.world().contains(biggerWorldBounds));
+  }
+
+  @Test
+  public void containsNotBoundsInWorld() {
+    LatLngBounds outSideWorldBounds = new LatLngBounds.Builder()
+      .include(new LatLng(GeoConstants.MAX_LATITUDE + 10, GeoConstants.MAX_LONGITUDE + 10))
+      .include(new LatLng(GeoConstants.MAX_LATITUDE + 20, GeoConstants.MAX_LONGITUDE + 20))
+      .build();
+    assertFalse("Bounds should not be contained ", LatLngBounds.world().contains(outSideWorldBounds));
+  }
+
+  @Test
   public void testHashCode() {
     assertEquals(2147483647, latLngBounds.hashCode(), -1946419200);
   }
 
   @Test
-  public void testEquality() {
+  public void equality() {
     LatLngBounds latLngBounds = new LatLngBounds.Builder()
       .include(LAT_LNG_NULL_ISLAND)
       .include(LAT_LNG_NOT_NULL_ISLAND)
@@ -152,7 +176,7 @@ public class LatLngBoundsTest {
   }
 
   @Test
-  public void testIntersect() {
+  public void intersect() {
     LatLngBounds latLngBounds = new LatLngBounds.Builder()
       .include(new LatLng(1, 1))
       .include(LAT_LNG_NULL_ISLAND)
@@ -162,7 +186,7 @@ public class LatLngBoundsTest {
   }
 
   @Test
-  public void testNoIntersect() {
+  public void intersectNot() {
     LatLngBounds latLngBounds = new LatLngBounds.Builder()
       .include(new LatLng(10, 10))
       .include(new LatLng(9, 8))
@@ -171,7 +195,7 @@ public class LatLngBoundsTest {
   }
 
   @Test
-  public void testInnerUnion() {
+  public void innerUnion() {
     LatLngBounds latLngBounds = new LatLngBounds.Builder()
       .include(new LatLng(1, 1))
       .include(LAT_LNG_NULL_ISLAND)
@@ -180,7 +204,7 @@ public class LatLngBoundsTest {
   }
 
   @Test
-  public void testOuterUnion() {
+  public void outerUnion() {
     LatLngBounds latLngBounds = new LatLngBounds.Builder()
       .include(new LatLng(10, 10))
       .include(new LatLng(9, 8))
@@ -194,6 +218,66 @@ public class LatLngBoundsTest {
   }
 
   @Test
+  public void northWest() {
+    double minLat = 5;
+    double minLon = 6;
+    double maxLat = 20;
+    double maxLon = 21;
+
+    LatLngBounds latLngBounds = new LatLngBounds.Builder()
+      .include(new LatLng(minLat, minLon))
+      .include(new LatLng(maxLat, maxLon))
+      .build();
+
+    assertEquals("NorthWest should match", latLngBounds.getNorthWest(), new LatLng(maxLat, minLon));
+  }
+
+  @Test
+  public void southWest() {
+    double minLat = 5;
+    double minLon = 6;
+    double maxLat = 20;
+    double maxLon = 21;
+
+    LatLngBounds latLngBounds = new LatLngBounds.Builder()
+      .include(new LatLng(minLat, minLon))
+      .include(new LatLng(maxLat, maxLon))
+      .build();
+
+    assertEquals("SouthWest should match", latLngBounds.getSouthWest(), new LatLng(minLat, minLon));
+  }
+
+  @Test
+  public void northEast() {
+    double minLat = 5;
+    double minLon = 6;
+    double maxLat = 20;
+    double maxLon = 21;
+
+    LatLngBounds latLngBounds = new LatLngBounds.Builder()
+      .include(new LatLng(minLat, minLon))
+      .include(new LatLng(maxLat, maxLon))
+      .build();
+
+    assertEquals("NorthEast should match", latLngBounds.getNorthEast(), new LatLng(maxLat, maxLon));
+  }
+
+  @Test
+  public void southEast() {
+    double minLat = 5;
+    double minLon = 6;
+    double maxLat = 20;
+    double maxLon = 21;
+
+    LatLngBounds latLngBounds = new LatLngBounds.Builder()
+      .include(new LatLng(minLat, minLon))
+      .include(new LatLng(maxLat, maxLon))
+      .build();
+
+    assertEquals("SouthEast should match", latLngBounds.getSouthEast(), new LatLng(minLat, maxLon));
+  }
+
+  @Test
   public void testParcelable() {
     LatLngBounds latLngBounds = new LatLngBounds.Builder()
       .include(new LatLng(10, 10))