From 0aa29f1c3456d49537e8dab710c08e4773a485f4 Mon Sep 17 00:00:00 2001 From: Nick Date: Wed, 25 Oct 2017 11:01:30 -0700 Subject: [PATCH 1/5] Added resolveAssetSource mock from RN --- __tests__/__mocks__/react-native.mock.js | 3 +++ package.json | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 __tests__/__mocks__/react-native.mock.js diff --git a/__tests__/__mocks__/react-native.mock.js b/__tests__/__mocks__/react-native.mock.js new file mode 100644 index 0000000..49cf25c --- /dev/null +++ b/__tests__/__mocks__/react-native.mock.js @@ -0,0 +1,3 @@ +jest.mock('react-native/Libraries/Image/resolveAssetSource', () => { + return () => ({ uri: `asset://test.png` }); +}); diff --git a/package.json b/package.json index 4eedca3..adedbcd 100644 --- a/package.json +++ b/package.json @@ -63,7 +63,8 @@ "javascript/**/*.js" ], "setupFiles": [ - "./__tests__/__mocks__/react-native-mapbox-gl.mock.js" + "./__tests__/__mocks__/react-native-mapbox-gl.mock.js", + "./__tests__/__mocks__/react-native.mock.js" ], "modulePathIgnorePatterns": [ "example", From 071175ba3a31502deb3415a7c8a2b633773b5afe Mon Sep 17 00:00:00 2001 From: Nick Date: Wed, 25 Oct 2017 11:02:06 -0700 Subject: [PATCH 2/5] Added image asset resolving unit test for StyleSheet This unit test will test if the image was required(import, require) by the JS layer that it will resolve it to an asset/network uri --- __tests__/utils/MapboxStyleSheet.test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/__tests__/utils/MapboxStyleSheet.test.js b/__tests__/utils/MapboxStyleSheet.test.js index 1fa3df0..0cf6cf5 100644 --- a/__tests__/utils/MapboxStyleSheet.test.js +++ b/__tests__/utils/MapboxStyleSheet.test.js @@ -24,6 +24,16 @@ describe('MapboxStyleSheet', () => { }); }); + it('should create asset image item for when we require images directly in JS', () => { + verifyStyleSheetsMatch({ fillPattern: 123 }, { + __MAPBOX_STYLESHEET__: true, + fillPattern: { + type: 'constant', + payload: { value: 'asset://test.png', image: true }, + }, + }); + }); + it('should create translate item', () => { verifyStyleSheetsMatch({ fillTranslate: { x: 1, y: 2 } }, { __MAPBOX_STYLESHEET__: true, From 1a88c400bf54e2c1f6d1f4dfa30e358f3f3bf959 Mon Sep 17 00:00:00 2001 From: Nick Date: Wed, 25 Oct 2017 11:04:04 -0700 Subject: [PATCH 3/5] Added helper function to StyleSheet to decide when to resolve images --- javascript/utils/MapboxStyleSheet.js | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/javascript/utils/MapboxStyleSheet.js b/javascript/utils/MapboxStyleSheet.js index 557f264..26c1dd2 100644 --- a/javascript/utils/MapboxStyleSheet.js +++ b/javascript/utils/MapboxStyleSheet.js @@ -77,6 +77,21 @@ class MapStyleFunctionItem extends MapStyleItem { } } +function resolveImage (imageURL) { + let resolved = imageURL; + + if (typeof imageURL === 'number') { // required from JS, local file resolve it's asset filepath + const res = resolveAssetSource(imageURL); + + // we found a local uri + if (res.uri) { + resolved = res.uri; + } + } + + return resolved; +} + function makeStyleValue (prop, value, extras = {}) { let item; @@ -93,8 +108,7 @@ function makeStyleValue (prop, value, extras = {}) { } else if (styleMap[prop] === StyleTypes.Translation) { item = new MapStyleTranslationItem(value.x, value.y, extraData); } else if (styleMap[prop] === StyleTypes.Image) { - const res = resolveAssetSource(value) || {}; - item = new MapStyleConstantItem(res.uri || value, { image: true, ...extraData }); + item = new MapStyleConstantItem(resolveImage(value), { image: true, ...extraData }); } else { item = new MapStyleConstantItem(value, extraData); } From 9e8ef7133912775232588a4589a00b628be77104 Mon Sep 17 00:00:00 2001 From: Nick Date: Wed, 25 Oct 2017 11:04:30 -0700 Subject: [PATCH 4/5] Updates how we add images to map style We were missing the use case when a user wants to use a custom sprite this now checks to make sure that we only add an image to the map style if it has a valid uri/url --- .../rctmgl/components/styles/RCTMGLStyle.java | 14 +++++++++++-- ios/RCTMGL/RCTMGLStyle.m | 21 ++++++++++++++----- scripts/templates/RCTMGLStyle.m.ejs | 13 +++++++++++- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/styles/RCTMGLStyle.java b/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/styles/RCTMGLStyle.java index 01bb41b..b62826e 100644 --- a/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/styles/RCTMGLStyle.java +++ b/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/styles/RCTMGLStyle.java @@ -1,8 +1,10 @@ package com.mapbox.rctmgl.components.styles; +import android.net.Uri; import android.support.annotation.NonNull; import android.support.annotation.StringDef; +import com.facebook.common.util.UriUtil; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.ReadableMapKeySetIterator; import com.mapbox.mapboxsdk.maps.MapboxMap; @@ -57,16 +59,24 @@ public class RCTMGLStyle { } public void addImage(String uriStr) { - if (uriStr == null || isTokenString(uriStr)) { + if (!shouldAddImage(uriStr)) { return; } - Map.Entry[] images = new Map.Entry[]{ new AbstractMap.SimpleEntry(uriStr, uriStr) }; DownloadMapImageTask task = new DownloadMapImageTask(mMap, null); task.execute(images); } + private boolean shouldAddImage(String uriStr) { + return uriStr != null && !isTokenString(uriStr) && isValidURI(uriStr); + } + private boolean isTokenString(String str) { return str.charAt(0) == '{' && str.charAt(str.length() - 1) == '}'; } + + private boolean isValidURI(String str) { + Uri uri = Uri.parse(str); + return UriUtil.isLocalAssetUri(uri) || UriUtil.isNetworkUri(uri); + } } diff --git a/ios/RCTMGL/RCTMGLStyle.m b/ios/RCTMGL/RCTMGLStyle.m index 6bffe47..f19aa4b 100644 --- a/ios/RCTMGL/RCTMGLStyle.m +++ b/ios/RCTMGL/RCTMGLStyle.m @@ -53,7 +53,7 @@ } else if ([prop isEqualToString:@"fillTranslateAnchor"]) { [self setFillTranslateAnchor:layer withReactStyleValue:styleValue]; } else if ([prop isEqualToString:@"fillPattern"]) { - if ([self _isTokenString:styleValue.payload[@"value"]]) { + if (![self _shouldAddImage:styleValue.payload[@"value"]]) { [self setFillPattern:layer withReactStyleValue:styleValue]; } else { [RCTMGLUtils fetchImage:_bridge url:styleValue.payload[@"value"] callback:^(NSError *error, UIImage *image) { @@ -131,7 +131,7 @@ } else if ([prop isEqualToString:@"lineDasharrayTransition"]) { [self setLineDasharrayTransition:layer withReactStyleValue:styleValue]; } else if ([prop isEqualToString:@"linePattern"]) { - if ([self _isTokenString:styleValue.payload[@"value"]]) { + if (![self _shouldAddImage:styleValue.payload[@"value"]]) { [self setLinePattern:layer withReactStyleValue:styleValue]; } else { [RCTMGLUtils fetchImage:_bridge url:styleValue.payload[@"value"] callback:^(NSError *error, UIImage *image) { @@ -185,7 +185,7 @@ } else if ([prop isEqualToString:@"iconTextFitPadding"]) { [self setIconTextFitPadding:layer withReactStyleValue:styleValue]; } else if ([prop isEqualToString:@"iconImage"]) { - if ([self _isTokenString:styleValue.payload[@"value"]]) { + if (![self _shouldAddImage:styleValue.payload[@"value"]]) { [self setIconImage:layer withReactStyleValue:styleValue]; } else { [RCTMGLUtils fetchImage:_bridge url:styleValue.payload[@"value"] callback:^(NSError *error, UIImage *image) { @@ -392,7 +392,7 @@ } else if ([prop isEqualToString:@"fillExtrusionTranslateAnchor"]) { [self setFillExtrusionTranslateAnchor:layer withReactStyleValue:styleValue]; } else if ([prop isEqualToString:@"fillExtrusionPattern"]) { - if ([self _isTokenString:styleValue.payload[@"value"]]) { + if (![self _shouldAddImage:styleValue.payload[@"value"]]) { [self setFillExtrusionPattern:layer withReactStyleValue:styleValue]; } else { [RCTMGLUtils fetchImage:_bridge url:styleValue.payload[@"value"] callback:^(NSError *error, UIImage *image) { @@ -491,7 +491,7 @@ } else if ([prop isEqualToString:@"backgroundColorTransition"]) { [self setBackgroundColorTransition:layer withReactStyleValue:styleValue]; } else if ([prop isEqualToString:@"backgroundPattern"]) { - if ([self _isTokenString:styleValue.payload[@"value"]]) { + if (![self _shouldAddImage:styleValue.payload[@"value"]]) { [self setBackgroundPattern:layer withReactStyleValue:styleValue]; } else { [RCTMGLUtils fetchImage:_bridge url:styleValue.payload[@"value"] callback:^(NSError *error, UIImage *image) { @@ -1847,6 +1847,11 @@ +- (BOOL)_shouldAddImage:(NSString *)str +{ + return str != nil && ![self _isTokenString:str] && [self _isValidURL:str]; +} + - (BOOL)_isTokenString:(NSString *)str { if (str == nil) { @@ -1855,6 +1860,12 @@ return [str hasPrefix:@"{"] && [str hasSuffix:@"}"]; } +- (BOOL)_isValidURL:(NSString *)str +{ + NSURL *url = [NSURL URLWithString:str]; + return [UIApplication.sharedApplication canOpenURL:url]; +} + - (BOOL)_hasReactStyle:(NSDictionary *)reactStyle { return reactStyle != nil && reactStyle.allKeys.count > 0; diff --git a/scripts/templates/RCTMGLStyle.m.ejs b/scripts/templates/RCTMGLStyle.m.ejs index dd69f38..2c7fabc 100644 --- a/scripts/templates/RCTMGLStyle.m.ejs +++ b/scripts/templates/RCTMGLStyle.m.ejs @@ -36,7 +36,7 @@ <% for (let i = 0; i < layer.properties.length; i++) { -%> <%- ifOrElseIf(i) -%> ([prop isEqualToString:@"<%= layer.properties[i].name %>"]) { <%_ if (layer.properties[i].image) { _%> - if ([self _isTokenString:styleValue.payload[@"value"]]) { + if (![self _shouldAddImage:styleValue.payload[@"value"]]) { [self set<%- iosPropMethodName(layer, pascelCase(layer.properties[i].name)) -%>:layer withReactStyleValue:styleValue]; } else { [RCTMGLUtils fetchImage:_bridge url:styleValue.payload[@"value"] callback:^(NSError *error, UIImage *image) { @@ -88,6 +88,11 @@ <% } %> <% } %> +- (BOOL)_shouldAddImage:(NSString *)str +{ + return str != nil && ![self _isTokenString:str] && [self _isValidURL:str]; +} + - (BOOL)_isTokenString:(NSString *)str { if (str == nil) { @@ -96,6 +101,12 @@ return [str hasPrefix:@"{"] && [str hasSuffix:@"}"]; } +- (BOOL)_isValidURL:(NSString *)str +{ + NSURL *url = [NSURL URLWithString:str]; + return [UIApplication.sharedApplication canOpenURL:url]; +} + - (BOOL)_hasReactStyle:(NSDictionary *)reactStyle { return reactStyle != nil && reactStyle.allKeys.count > 0; From ca11ba380bc30279e209af4c845c5f756cfa6300 Mon Sep 17 00:00:00 2001 From: Nick Date: Thu, 26 Oct 2017 16:22:43 -0700 Subject: [PATCH 5/5] Remove isTokenString check now that we validate URIs --- .../mapbox/rctmgl/components/styles/RCTMGLStyle.java | 6 +----- ios/RCTMGL/RCTMGLStyle.m | 10 +--------- scripts/templates/RCTMGLStyle.m.ejs | 10 +--------- 3 files changed, 3 insertions(+), 23 deletions(-) diff --git a/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/styles/RCTMGLStyle.java b/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/styles/RCTMGLStyle.java index b62826e..0abcf5d 100644 --- a/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/styles/RCTMGLStyle.java +++ b/android/rctmgl/src/main/java/com/mapbox/rctmgl/components/styles/RCTMGLStyle.java @@ -68,11 +68,7 @@ public class RCTMGLStyle { } private boolean shouldAddImage(String uriStr) { - return uriStr != null && !isTokenString(uriStr) && isValidURI(uriStr); - } - - private boolean isTokenString(String str) { - return str.charAt(0) == '{' && str.charAt(str.length() - 1) == '}'; + return uriStr != null && isValidURI(uriStr); } private boolean isValidURI(String str) { diff --git a/ios/RCTMGL/RCTMGLStyle.m b/ios/RCTMGL/RCTMGLStyle.m index f19aa4b..c68f938 100644 --- a/ios/RCTMGL/RCTMGLStyle.m +++ b/ios/RCTMGL/RCTMGLStyle.m @@ -1849,15 +1849,7 @@ - (BOOL)_shouldAddImage:(NSString *)str { - return str != nil && ![self _isTokenString:str] && [self _isValidURL:str]; -} - -- (BOOL)_isTokenString:(NSString *)str -{ - if (str == nil) { - return false; - } - return [str hasPrefix:@"{"] && [str hasSuffix:@"}"]; + return str != nil && [self _isValidURL:str]; } - (BOOL)_isValidURL:(NSString *)str diff --git a/scripts/templates/RCTMGLStyle.m.ejs b/scripts/templates/RCTMGLStyle.m.ejs index 2c7fabc..6606ab4 100644 --- a/scripts/templates/RCTMGLStyle.m.ejs +++ b/scripts/templates/RCTMGLStyle.m.ejs @@ -90,15 +90,7 @@ - (BOOL)_shouldAddImage:(NSString *)str { - return str != nil && ![self _isTokenString:str] && [self _isValidURL:str]; -} - -- (BOOL)_isTokenString:(NSString *)str -{ - if (str == nil) { - return false; - } - return [str hasPrefix:@"{"] && [str hasSuffix:@"}"]; + return str != nil && [self _isValidURL:str]; } - (BOOL)_isValidURL:(NSString *)str