From 16aa48151ca981f013181dc072758fbb3493d7dd Mon Sep 17 00:00:00 2001 From: tobrun Date: Mon, 10 Dec 2018 13:23:42 +0100 Subject: [android] - introduce validateState, add illegal state exception to all invocations, add test --- .../main/java/com/mapbox/mapboxsdk/maps/Style.java | 172 ++++++--------------- .../java/com/mapbox/mapboxsdk/maps/StyleTest.kt | 12 ++ 2 files changed, 55 insertions(+), 129 deletions(-) diff --git a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java index 94b5b73378..d1f3da178d 100644 --- a/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java +++ b/platform/android/MapboxGLAndroidSDK/src/main/java/com/mapbox/mapboxsdk/maps/Style.java @@ -5,7 +5,6 @@ import android.support.annotation.IntRange; import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.annotation.StringDef; - import com.mapbox.mapboxsdk.constants.MapboxConstants; import com.mapbox.mapboxsdk.style.layers.Layer; import com.mapbox.mapboxsdk.style.layers.TransitionOptions; @@ -54,11 +53,7 @@ public class Style { */ @NonNull public String getUrl() { - if (!fullyLoaded) { - // we are loading a new style - return ""; - } - + validateState("getUrl"); return nativeMapView.getStyleUrl(); } @@ -69,11 +64,7 @@ public class Style { */ @NonNull public String getJson() { - if (!fullyLoaded) { - // we are loading a new style - return ""; - } - + validateState("getJson"); return nativeMapView.getStyleJson(); } @@ -88,11 +79,7 @@ public class Style { */ @NonNull public List getSources() { - if (!fullyLoaded) { - // we are loading a new style - return new ArrayList<>(); - } - + validateState("getSources"); return nativeMapView.getSources(); } @@ -102,12 +89,7 @@ public class Style { * @param source the source to add */ public void addSource(@NonNull Source source) { - if (!fullyLoaded) { - throw new IllegalStateException( - "Calling addSource on old Style instance, use the more recently loaded Style instead." - ); - } - + validateState("addSource"); sources.put(source.getId(), source); nativeMapView.addSource(source); } @@ -120,11 +102,7 @@ public class Style { */ @Nullable public Source getSource(String id) { - if (!fullyLoaded) { - // we are loading a new style - return null; - } - + validateState("getSource"); Source source = sources.get(id); if (source == null) { source = nativeMapView.getSource(id); @@ -141,11 +119,7 @@ public class Style { */ @Nullable public T getSourceAs(@NonNull String sourceId) { - if (!fullyLoaded) { - // we are loading a new style - return null; - } - + validateState("getSourceAs"); // noinspection unchecked if (sources.containsKey(sourceId)) { return (T) sources.get(sourceId); @@ -160,11 +134,7 @@ public class Style { * @return the source handle or null if the source was not present */ public boolean removeSource(@NonNull String sourceId) { - if (!fullyLoaded) { - // we are loading a new style - return false; - } - + validateState("removeSource"); sources.remove(sourceId); return nativeMapView.removeSource(sourceId); } @@ -176,11 +146,7 @@ public class Style { * @return the source */ public boolean removeSource(@NonNull Source source) { - if (!fullyLoaded) { - // we are loading a new style - return false; - } - + validateState("removeSource"); sources.remove(source.getId()); return nativeMapView.removeSource(source); } @@ -195,12 +161,7 @@ public class Style { * @param layer the layer to add */ public void addLayer(@NonNull Layer layer) { - if (!fullyLoaded) { - throw new IllegalStateException( - "Calling addLayer on old Style instance, use the more recently loaded Style instead." - ); - } - + validateState("addLayer"); layers.put(layer.getId(), layer); nativeMapView.addLayer(layer); } @@ -212,12 +173,7 @@ public class Style { * @param below the layer id to add this layer before */ public void addLayerBelow(@NonNull Layer layer, @NonNull String below) { - if (!fullyLoaded) { - throw new IllegalStateException( - "Calling addLayerBelow on old Style instance, use the more recently loaded Style instead." - ); - } - + validateState("addLayerBelow"); layers.put(layer.getId(), layer); nativeMapView.addLayerBelow(layer, below); } @@ -229,12 +185,7 @@ public class Style { * @param above the layer id to add this layer above */ public void addLayerAbove(@NonNull Layer layer, @NonNull String above) { - if (!fullyLoaded) { - throw new IllegalStateException( - "Calling addLayerAbove on old Style instance, use the more recently loaded Style instead." - ); - } - + validateState("addLayerAbove"); layers.put(layer.getId(), layer); nativeMapView.addLayerAbove(layer, above); } @@ -247,12 +198,7 @@ public class Style { * @param index the index to insert the layer at */ public void addLayerAt(@NonNull Layer layer, @IntRange(from = 0) int index) { - if (!fullyLoaded) { - throw new IllegalStateException( - "Calling addLayerAt on old Style instance, use the more recently loaded Style instead." - ); - } - + validateState("addLayerAbove"); layers.put(layer.getId(), layer); nativeMapView.addLayerAt(layer, index); } @@ -265,11 +211,7 @@ public class Style { */ @Nullable public Layer getLayer(@NonNull String id) { - if (!fullyLoaded) { - // we are loading a new style - return null; - } - + validateState("getLayer"); Layer layer = layers.get(id); if (layer == null) { layer = nativeMapView.getLayer(id); @@ -286,11 +228,7 @@ public class Style { */ @Nullable public T getLayerAs(@NonNull String layerId) { - if (!fullyLoaded) { - // we are loading a new style - return null; - } - + validateState("getLayerAs"); // noinspection unchecked return (T) nativeMapView.getLayer(layerId); } @@ -302,11 +240,7 @@ public class Style { */ @NonNull public List getLayers() { - if (!fullyLoaded) { - // we are loading a new style - return new ArrayList<>(); - } - + validateState("getLayers"); return nativeMapView.getLayers(); } @@ -317,10 +251,7 @@ public class Style { * @return the removed layer or null if not found */ public boolean removeLayer(@NonNull String layerId) { - if (!fullyLoaded) { - // we are loading a new style - return false; - } + validateState("removeLayer"); layers.remove(layerId); return nativeMapView.removeLayer(layerId); } @@ -332,10 +263,7 @@ public class Style { * @return the layer */ public boolean removeLayer(@NonNull Layer layer) { - if (!fullyLoaded) { - // we are loading a new style - return false; - } + validateState("removeLayer"); layers.remove(layer.getId()); return nativeMapView.removeLayer(layer); } @@ -347,6 +275,7 @@ public class Style { * @return the removed layer or null if not found */ public boolean removeLayerAt(@IntRange(from = 0) int index) { + validateState("removeLayerAt"); return nativeMapView.removeLayerAt(index); } @@ -372,12 +301,7 @@ public class Style { * @param sdf the flag indicating image is an SDF or template image */ public void addImage(@NonNull String name, @NonNull Bitmap image, boolean sdf) { - if (!fullyLoaded) { - throw new IllegalStateException( - "Calling addImage on old Style instance, use the more recently loaded Style instead." - ); - } - + validateState("addImage"); nativeMapView.addImage(name, image, sdf); } @@ -385,12 +309,7 @@ public class Style { * Adds an images to be used in the map's style. */ public void addImages(@NonNull HashMap images) { - if (!fullyLoaded) { - throw new IllegalStateException( - "Calling addImages on old Style instance, use the more recently loaded Style instead." - ); - } - + validateState("addImages"); nativeMapView.addImages(images); } @@ -400,11 +319,7 @@ public class Style { * @param name the name of the image to remove */ public void removeImage(@NonNull String name) { - if (!fullyLoaded) { - // we are loading a new style - return; - } - + validateState("removeImage"); nativeMapView.removeImage(name); } @@ -416,11 +331,7 @@ public class Style { */ @Nullable public Bitmap getImage(@NonNull String id) { - if (!fullyLoaded) { - // we are loading a new style - return null; - } - + validateState("getImage"); return nativeMapView.getImage(id); } @@ -437,12 +348,7 @@ public class Style { * @param transitionOptions the transition options */ public void setTransition(@NonNull TransitionOptions transitionOptions) { - if (!fullyLoaded) { - throw new IllegalStateException( - "Calling setTransition on old Style instance, use the more recently loaded Style instead." - ); - } - + validateState("setTransition"); nativeMapView.setTransitionDuration(transitionOptions.getDuration()); nativeMapView.setTransitionDelay(transitionOptions.getDelay()); } @@ -457,11 +363,7 @@ public class Style { */ @NonNull public TransitionOptions getTransition() { - if (!fullyLoaded) { - // we are loading a new style - return new TransitionOptions(0, 0); - } - + validateState("getTransition"); return new TransitionOptions(nativeMapView.getTransitionDuration(), nativeMapView.getTransitionDelay()); } @@ -476,19 +378,20 @@ public class Style { */ @Nullable public Light getLight() { - if (!fullyLoaded) { - // we are loading a new style - return null; - } - + validateState("getLight"); return nativeMapView.getLight(); } + // + // State + // + /** * Called when the underlying map will start loading a new style. This method will clean up this style * by setting the java sources and layers in a detached state and removing them from core. */ void onWillStartLoadingMap() { + fullyLoaded = false; for (Source source : sources.values()) { if (source != null) { source.setDetached(); @@ -511,8 +414,6 @@ public class Style { sources.clear(); layers.clear(); images.clear(); - - fullyLoaded = false; } /** @@ -559,6 +460,19 @@ public class Style { return fullyLoaded; } + /** + * Validates the style state, throw an IllegalArgumentException on invalid state. + * + * @param methodCall the calling method name + */ + private void validateState(String methodCall) { + if (!fullyLoaded) { + throw new IllegalStateException( + String.format("Calling %s when a newer style is loading/has loaded.", methodCall) + ); + } + } + // // Builder // diff --git a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/StyleTest.kt b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/StyleTest.kt index 4b2f3e287e..56223dadef 100644 --- a/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/StyleTest.kt +++ b/platform/android/MapboxGLAndroidSDK/src/test/java/com/mapbox/mapboxsdk/maps/StyleTest.kt @@ -14,6 +14,7 @@ import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner +import java.lang.IllegalStateException @RunWith(RobolectricTestRunner::class) class StyleTest { @@ -304,4 +305,15 @@ class StyleTest { mapboxMap.setStyle(Style.MAPBOX_STREETS) verify(exactly = 1) { callback.onStyleLoaded(any()) } } + + @Test(expected = IllegalStateException::class) + fun testIllegalStateExceptionWithStyleReload() { + val builder = Style.Builder().fromUrl(Style.MAPBOX_STREETS) + mapboxMap.setStyle(builder) + mapboxMap.notifyStyleLoaded() + val style = mapboxMap.style + mapboxMap.setStyle(Style.Builder().fromUrl(Style.DARK)) + style!!.addLayer(mockk()) + } + } \ No newline at end of file -- cgit v1.2.1