react-native-google-places icon indicating copy to clipboard operation
react-native-google-places copied to clipboard

Revisiting place photos support

Open thomasw opened this issue 6 years ago • 12 comments

@tolu360, over in https://github.com/tolu360/react-native-google-places/issues/60#issuecomment-317367219 you suggested using the web API to retrieve place photos. Google's looming pricing/quota changes for its places, maps, and routes APIs change the situation significantly. Securing API keys to prevent unauthorized API usage is more important now than ever before. Consumers embedding API keys in their applications for REST API purposes won't be able to easily achieve that.

However, calls coming via your library get that for free (assuming the key is properly restricted to an appropriate package id). Would you reconsider adding wrappers for GeoDataClient.getPlacePhotos, GeoDataClient.getPhoto and GeoDataClient.getScaledPhoto?

For reference, my understanding is that implementations such as the one outlined in https://github.com/tolu360/react-native-google-places/issues/106#issuecomment-371280506 expose keys to theft and abuse. The API call in that implementation can neither be signed as coming from the specific consumer package nor can the key be restricted to an appropriate IP address or HTTP referrer.

thomasw avatar Jun 23 '18 19:06 thomasw

Hi @thomasw I can consider adding support for such wrappers or you may send in a PR if you already worked on it. Cheers.

tolu360 avatar Jun 27 '18 15:06 tolu360

I hacked together preliminary Android support, which you can peruse here: https://github.com/tolu360/react-native-google-places/compare/master...thomasw:add_photo_support?expand=1

It's all more complicated than I anticipated.

Some of the bigger issues we'd have to contend with to support this:

  1. Ideally, every object in the getPlacePhotos() JS call would include a photo URI. However, credit usage happens only on the getPhoto call, as far as I understand it. In terms of API usage, that makes it suboptimal to always call getPhoto for every photo in a places's photo list. We need return metadata about the available photos to consumers and then allow them to optionally select which photo(s) they want to load, allowing them to reduce unnecessary API usage. To facilitate this, getPlacePhotos currently returns the photo metadata objects supplemented by a photoId, which is just the photo's index from the list of photos. The results of getPlacePhotos can be passed to getPlacePhoto to retrieve an Image URI that corresponds with the metadata.
  2. The Places SDK's getPlacePhotos method is non-deterministic. In a single session, my index hack above will likely always work because the Places SDK internally caches responses. However, if someone persists the list and attempts to use the persisted indexes to retrieve photos later, they may end up with mismatched results.
  3. The SDK returns a Bitmap, not a URI. That makes getting the data over to the RN side of things a bit cumbersome. I explored both base64 encoding the data and sending it over to RN as a string and persisting the data to disk and sending over a URI. The base64 encoding approach doesn't seem particularly viable. Consumers would end up storing very large image objects in application state. The file approach, however, requires some sort of cleanup strategy. I attempted to write the images to a cache directory assuming the OS might take care of that for free, but ran into issues with reading those files from RN.

I'm going to step back for a bit and think on alternative interfaces. For example, an interface like the following might be easier to implement and offer a reasonable usability/credit usage compromise because most consumers will likely want to either load just the first image or all of them:

> RNGooglePlaces.getPlaceFirstPhoto(placeID)
{
  attributions: '...'
  uri: 'file://...'
}

> const limit = 10  // Will use up to 10 credits.
> RNGooglePlaces.getPlacePhotos(placeID, limit)
[{
  attributions: '...'
  uri: 'file://...'
},
...
]

thomasw avatar Jun 29 '18 22:06 thomasw

While trying to determine exactly how costly simply loading all of the image data upfront might be, I tried to look further into the specific pricing for place photos via the Android and iOS SDKs. I'm even more confused now than when I started. It's unclear to me whether or not photo retrieval via these SDKs will be subject to billing at all or if they will remain free with the 1,000/150,000 caps.

Notably, https://cloud.google.com/maps-platform/pricing/sheet/ doesn't make a distinction for native Place API usage and, seemingly in contradiction to that, the SKU data for place photos https://developers.google.com/maps/billing/understanding-cost-of-use#places-photo does not mention the Android or iOS SDKs.

thomasw avatar Jun 30 '18 22:06 thomasw

Oh, from https://developers.google.com/maps/billing/understanding-cost-of-use#places-product

Note: Use of the Places SDK for Android and the Places SDK for iOS is not presently charged.

¯\_(ツ)_/¯ That doesn't exactly fill me with confidence moving forward, but it's something, I guess.

thomasw avatar Jun 30 '18 22:06 thomasw

In light of that, I think the following interface seems reasonable:

// Only return the metadata - useful for finding a specific photo that doesn't have
// required attributions or aligns with some consumer sizing requirement without
// incurring the performance penalty of loading all the photos
> RNGooglePlaces.getPlacePhotoMetadata(placeID)
[{
  placeId: '...',
  photoId: '...',
  attributions: '...'
  maxWidth: ...,
  maxHeight: ....
},
...
]
// Same as above, but it saves all the images to disk and returns a uri for each.
> RNGooglePlaces.getPlacePhotos(placeID)
[{
  placeId: '...',
  photoId: '...',
  attributions: '...'
  maxWidth: ...,
  maxHeight: ....,
  uri: 'file://...'
},
...
]
// Using the index hack in my original implementation, this saves a single photo to disk
// and returns the meta and file uri for that photo.
> RNGooglePlaces.getPlacePhoto(placePhotoMeta)
{
  placeId: '...',
  photoId: '...',
  attributions: '...'
  maxWidth: ...,
  maxHeight: ....,
  uri: 'file://...'
}
// Using the index hack in my original implementation, this saves a single photo to disk,
// resizes it using the Places SDK, and returns the meta and file uri for the resized photo.
> RNGooglePlaces.getScaledPlacePhoto(placePhotoMeta, width, height)
{
  placeId: '...',
  photoId: '...',
  attributions: '...'
  maxWidth: ...,
  maxHeight: ....,
  uri: 'file://...'
}

And, in terms of the under the hood implementation, we could:

  1. Write the photo data to a cache file and expose it via a file URI. I'll need to investigate the issues I was having with reading from cache dir on the JS side further.
  2. Since cache data is presumably cleared out periodically, any consumers persisting RNGooglePlaces's responses and reusing them will need to actively determine whether or not the image data still exists on disk when depending on it. This could be achieved by reloading place photo data on failure using an onError handler on an Image component (<Image source={{uri: gplacephoto}} onError={loadPlacePhotos} />). We'd need to document this caveat sufficiently.

I'm really unfamiliar with the iOS side of things, but looking at the iOS Places SDK docs, this approach, at least superficially, seems like it would work there too.

thomasw avatar Jun 30 '18 23:06 thomasw

I'll need to investigate the issues I was having with reading from cache dir on the JS side further.

I've resolved this.

thomasw avatar Jul 01 '18 17:07 thomasw

Hi @thomasw, I see you have put a lot of work into this, nice work by the way! Full disclosure, I have only been able to go through these on the merit of your comments here and just eyeballing the added LOC, but I would make out time, asap, to review your implementations for Android and see if I can knock out similar on iOS while at it.

Cheers

tolu360 avatar Jul 02 '18 09:07 tolu360

@tolu360 No need to rush from my perspective. I don't have any production implementations desperately waiting on place photos support or anything like that. I do appreciate any time you can spare, of course! 👍

I'll modify my working copy to reflect the interface I last proposed when I'm able. If you create a feature branch, I'll open a pull request into it once I've made those changes so we can discuss any specific implementation issues there and then you'll be able to branch from that when starting your iOS implementation.

thomasw avatar Jul 02 '18 17:07 thomasw

Hi Thomas, there you go, you can work with the place-photos branch. Cheers!

tolu360 avatar Jul 03 '18 09:07 tolu360

Would now have to revisit this in the latest beta release, see #198, but I am not asking you to redo all the hard work, should be easy enough for me to leverage the work you have done already.

tolu360 avatar Mar 11 '19 15:03 tolu360

@tolu360 👍 Thanks, dude.

For anyone coming across this ticket at a later date, I quoted the following up above:

Note: Use of the Places SDK for Android and the Places SDK for iOS is not presently charged.

As of January 29, 2019, this is no longer true.

thomasw avatar Mar 11 '19 16:03 thomasw

Looking forward to the added photo support. Thanks for your hard work guys.

levi-beers avatar Mar 28 '19 07:03 levi-beers