[WIP] CF-1196 Fix offerings parsing for Amazon#802
Conversation
…ng for amazon vs play
This reverts commit ad9abedcf006f80353dbbefa6dcc6028d4678ab6.
dc47b7f to
8e3cf2a
Compare
| } | ||
| } | ||
| val planIdentifier = optString("platform_product_plan_identifier").takeIf { it.isNotEmpty() } | ||
| val product = productMatchingMethod.invoke(productsById, productIdentifier, planIdentifier) |
There was a problem hiding this comment.
if we went this route i'd need to think through thread-safety
There was a problem hiding this comment.
Are there potential threading issues because the productMatchingMethod might be accessing some shared cache state? 🤔
There was a problem hiding this comment.
i don't think there are right now, but mayyyybe there could be in the future, so was thinking we should just be defensive? might refactor this anyway...
# Conflicts: # common/src/test/java/com/revenuecat/purchases/common/OfferingsTest.kt # purchases/src/main/kotlin/com/revenuecat/purchases/Purchases.kt
|
|
||
| override val productMatchingMethod: ((Map<String, List<StoreProduct>>, String, String?) -> StoreProduct?) = { | ||
| productsById, productIdentifier: String, _ -> | ||
| productsById[productIdentifier]?.get(0) |
There was a problem hiding this comment.
Do we want firstOrNull() to prevent an index out of bounds?
| productsById[productIdentifier]?.get(0) | |
| productsById[productIdentifier]?.firstOrNull() |
| return@matcher productsById[productIdentifier] | ||
| .takeIf { it?.size == 1 } | ||
| ?.takeIf { it[0].type == ProductType.INAPP } | ||
| ?.get(0) |
There was a problem hiding this comment.
Just for safety? 🤷
| ?.get(0) | |
| ?.firstOrNull() |
| storeProducts?.firstOrNull { storeProduct -> | ||
| storeProduct.subscriptionOptions.firstOrNull { it.isBasePlan }?.id == planIdentifier | ||
| } |
There was a problem hiding this comment.
nit: add a return on this line for explicitness/consistency since there is a return above
| } | ||
| } | ||
| val planIdentifier = optString("platform_product_plan_identifier").takeIf { it.isNotEmpty() } | ||
| val product = productMatchingMethod.invoke(productsById, productIdentifier, planIdentifier) |
There was a problem hiding this comment.
Are there potential threading issues because the productMatchingMethod might be accessing some shared cache state? 🤔
|
spoke with @joshdholtz and we agreed a Factory is the better route... so closing this in lieu of #824 |
Amazon only has a single productId (the
termSku) returned in offerings, while google has both the subId and the basePlanID. So now the way we parse offerings must split between the stores.I see a few routes for this:
this is the easiest, but it breaks our general "don't have store-specific stuff in purchases" rule. see this commit as an example
this feels a bit like over-architecting to me...
I like this route best, as it re-uses code and doesn't create a lot of overhead. There are probably cleaner ways to achieve the same thing... i.e. rather than passing the function through all the methods, just have a billing.createOfferings() method which calls into super() for the repetitive code?
Tests will need to be updated based on the approach chosen..