Skip to content

Refactor offerings code out of Purchases#1027

Merged
tonidero merged 10 commits into
mainfrom
toniricodiez/sdk-3094-persist-offerings-cache-android
May 29, 2023
Merged

Refactor offerings code out of Purchases#1027
tonidero merged 10 commits into
mainfrom
toniricodiez/sdk-3094-persist-offerings-cache-android

Conversation

@tonidero

@tonidero tonidero commented May 26, 2023

Copy link
Copy Markdown
Contributor

Description

This is a first PR in preparation of persisting offerings in cache (SDK-3094). There are no behavior changes in this PR. This just extracts most of the code related to offerings from Purchases to the new classes OfferingsManager and OfferingsFactory.

Main changes

  • Created OfferingsManager that will be the entry point for all offerings-related logic
  • Created OfferingsFactory that allows to create the Offerings object from a JSON response. It will query the store to get the necessary products and create the final object.
  • Moved GoogleOfferingParser from the feature/google module to the common module. This is to be able to use the actual code in the unit tests more easily.

@tonidero tonidero marked this pull request as ready for review May 26, 2023 08:56
@tonidero tonidero requested a review from a team May 26, 2023 08:56
@codecov

codecov Bot commented May 26, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1027 (3c5e8b1) into main (7cb1fc3) will decrease coverage by 0.08%.
The diff coverage is 90.07%.

@@            Coverage Diff             @@
##             main    #1027      +/-   ##
==========================================
- Coverage   85.60%   85.52%   -0.08%     
==========================================
  Files         170      172       +2     
  Lines        6042     6066      +24     
  Branches      842      845       +3     
==========================================
+ Hits         5172     5188      +16     
- Misses        540      545       +5     
- Partials      330      333       +3     
Impacted Files Coverage Δ
...evenuecat/purchases/common/GoogleOfferingParser.kt 63.63% <ø> (ø)
.../com/revenuecat/purchases/OfferingParserFactory.kt 50.00% <ø> (ø)
...ava/com/revenuecat/purchases/utils/productStubs.kt 81.33% <ø> (ø)
...cat/purchases/common/offerings/OfferingsManager.kt 83.63% <83.63%> (ø)
...cat/purchases/common/offerings/OfferingsFactory.kt 93.44% <93.44%> (ø)
.../main/kotlin/com/revenuecat/purchases/Purchases.kt 81.11% <100.00%> (-1.77%) ⬇️
...otlin/com/revenuecat/purchases/PurchasesFactory.kt 83.59% <100.00%> (+0.52%) ⬆️

@@ -1,7 +1,6 @@
package com.revenuecat.purchases.google
package com.revenuecat.purchases.common

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this class is for Google and ideally it should live in the feature/google module, that made it significantly harder to keep the new classes in the common module. I could have moved them to purchases, but I kinda want to keep as much code as possible in common.

Without this, I would have had to mock the offerings parser in the common module tests, and it felt those test should use the real implementation (as they were doing until now)


// region Private Methods

private fun fetchAndCacheOfferings(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It felt good removing all this from Purchases

// region offerings

@Test
fun `fetch offerings on foregrounded if it's stale`() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of that code in iOS is no longer in Purchases, but we have a lot of tests since this is the facade so it's useful to test from top to bottom.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we definitely need to improve these tests situation. We already have a first step with #1011 (Thanks @vegaro!).

As for keeping tests at the Purchases level, I think that makes sense to test the main flows and main things we should be careful of. I guess that would be pretty close to integration tests, since at that point we probably only want to mock shared preferences/backend... I think that would be a separate test suite since they might be slightly harder to write, but we can discuss it further.

Also, thanks for sharing those! It can definitely serve as inspiration

@NachoSoto NachoSoto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great refactor 👏🏻

// region offerings

@Test
fun `fetch offerings on foregrounded if it's stale`() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of that code in iOS is no longer in Purchases, but we have a lot of tests since this is the facade so it's useful to test from top to bottom.

import org.json.JSONException
import org.json.JSONObject

class OfferingsFactory(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍🏻 From 1 class to 3.

@tonidero tonidero merged commit 50ae3dd into main May 29, 2023
@tonidero tonidero deleted the toniricodiez/sdk-3094-persist-offerings-cache-android branch May 29, 2023 08:22
tonidero added a commit that referenced this pull request Jun 1, 2023
**This is an automatic release.**

### New Features
* Offline entitlements support (#1030) via Toni Rico (@tonidero)
### Bugfixes
* Fix billing connection error when querying purchases early in the
process lifetime (#1032) via Toni Rico (@tonidero)
### Performance Improvements
* Perform product entitlement mapping request after more critical
requests (#1017) via Toni Rico (@tonidero)
### Dependency Updates
* Bump fastlane from 2.212.2 to 2.213.0 (#1024) via dependabot[bot]
(@dependabot[bot])
### Other Changes
* Get offerings response from disk cache if available (#1029) via Toni
Rico (@tonidero)
* Improve offline entitlements logs to mention limitations of this mode
(#1039) via Toni Rico (@tonidero)
* Improve error message when backend returns internal error code (#1038)
via Toni Rico (@tonidero)
* PurchaseTester: Add new UI to configure internal proxy behavior
(#1016) via Toni Rico (@tonidero)
* Updated readme to include links to migration guides (#1021) via Marcos
Castany (@mcastany)
* Store offerings response in SharedPreferences (#1028) via Toni Rico
(@tonidero)
* Refactor offerings code out of Purchases (#1027) via Toni Rico
(@tonidero)

---------

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants