Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove common module #1106

Merged
merged 5 commits into from
Jul 5, 2023
Merged

Remove common module #1106

merged 5 commits into from
Jul 5, 2023

Conversation

vegaro
Copy link
Contributor

@vegaro vegaro commented Jun 28, 2023 •
edited
Loading

Before:
image

After:
image

@vegaro vegaro force-pushed the cesar/sdk-3209-remove-common-module branch from 0e20d03 to 36fea5a Compare June 29, 2023 18:44
@vegaro vegaro changed the base branch from main to cesar/sdk-3211-move-featureamazon-files-into-common-module June 30, 2023 06:59
@codecov
Copy link

codecov bot commented Jun 30, 2023 •
edited
Loading

Codecov Report

Merging #1106 (9fe4745) into main (6f336b1) will decrease coverage by 0.35%.
The diff coverage is 80.72%.

@@            Coverage Diff             @@
##             main    #1106      +/-   ##
==========================================
- Coverage   85.16%   84.81%   -0.35%     
==========================================
  Files         186      180       -6     
  Lines        6551     6164     -387     
  Branches      929      915      -14     
==========================================
- Hits         5579     5228     -351     
+ Misses        601      573      -28     
+ Partials      371      363       -8
Impacted Files Coverage Δ
...main/java/com/revenuecat/purchases/CustomerInfo.kt 91.30% <ø> (ø)
...n/java/com/revenuecat/purchases/EntitlementInfo.kt 66.32% <ø> (ø)
...om/revenuecat/purchases/models/StoreTransaction.kt 100.00% <ø> (ø)
...revenuecat/purchases/models/SubscriptionOptions.kt 62.85% <ø> (ø)
...a/com/revenuecat/purchases/common/IntExtensions.kt 100.00% <ø> (ø)
...a/com/revenuecat/purchases/utils/Iso8601Utils.java 62.28% <ø> (ø)
...m/revenuecat/purchases/utils/JSONObjectParceler.kt 100.00% <ø> (ø)
.../revenuecat/purchases/AttributionFetcherFactory.kt 9.09% <ø> (ø)
.../kotlin/com/revenuecat/purchases/BillingFactory.kt 72.22% <ø> (ø)
.../com/revenuecat/purchases/OfferingParserFactory.kt 50.00% <ø> (ø)
... and 102 more

... and 8 files with indirect coverage changes

Base automatically changed from cesar/sdk-3211-move-featureamazon-files-into-common-module to main June 30, 2023 07:38
@vegaro vegaro force-pushed the cesar/sdk-3209-remove-common-module branch 2 times, most recently from eed6873 to 085ae00 Compare June 30, 2023 08:05
@vegaro vegaro force-pushed the cesar/sdk-3209-remove-common-module branch from 085ae00 to d3dae47 Compare June 30, 2023 08:06
vegaro
vegaro commented Jun 30, 2023
View reviewed changes
purchases/src/main/kotlin/com/revenuecat/purchases/common/HTTPClient.kt
@@ -36,7 +36,7 @@ import java.net.URLConnection
import java.util.Date
import kotlin.time.Duration

class HTTPClient(
internal class HTTPClient(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

making these internal is breaking the compilation because AmazonBackend is public and it exposes these (which are internal). Not sure if it's better to break semver and make AmazonBackend internal or expose these. cc: @tonidero

Copy link
Contributor

Choose a reason for hiding this comment

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

Huh... I would have expected that since AmazonBackend which is public in the feature/amazon module, has an internal class in the constructor BackendHelper (note that the amazon module is not exposing the common module), Kotlin should have given an error, that would force us to make the constructor of that class internal at least...

Not sure why that's not the case... currently, if I try to create an AmazonBackend in the API tests, I can't really do it...

So I guess they would be able to create one if someone also included the common module. And since we are making these internal, that would indeed be a breaking change... However, honestly, no-one should be creating these classes directly, so I'm kinda leaning towards making them internal

@vegaro vegaro marked this pull request as ready for review June 30, 2023 12:20
vegaro
vegaro commented Jun 30, 2023
View reviewed changes
settings.gradle
@@ -8,8 +8,6 @@ pluginManagement {

include ':feature:amazon'
include ':integration-tests'
include ':test-utils'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was easier to remove the test-utils now in this PR and merge it in purchases so I removed it

tonidero
tonidero reviewed Jul 5, 2023
View reviewed changes
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

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

Looks great! left a few questions

purchases/src/defaults/kotlin/com/revenuecat/purchases/common/DispatcherConstants.kt Outdated Show resolved Hide resolved
public/src/main/java/com/revenuecat/purchases/models/SubscriptionOptions.kt Show resolved Hide resolved
purchases/src/main/java/com/revenuecat/purchases/common/IntExtensions.kt Outdated Show resolved Hide resolved
purchases/src/main/java/com/revenuecat/purchases/common/verification/Signature.kt Outdated Show resolved Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/OfferingParserFactory.kt Outdated Show resolved Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/common/diagnostics/DiagnosticsEntry.kt Outdated Show resolved Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/common/logUtils.kt Show resolved Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/common/networking/ETagManager.kt Show resolved Hide resolved
purchases/src/main/kotlin/com/revenuecat/purchases/common/utils.kt Outdated Show resolved Hide resolved
purchases/src/test/java/com/revenuecat/purchases/PurchasesCoroutinesTest.kt Show resolved Hide resolved
tonidero
tonidero approved these changes Jul 5, 2023
View reviewed changes
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

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

Nice job! Almost there!

@vegaro vegaro merged commit ef34062 into main Jul 5, 2023
@vegaro vegaro deleted the cesar/sdk-3209-remove-common-module branch July 5, 2023 15:00
@tonidero tonidero mentioned this pull request Jul 6, 2023
Merged
tonidero added a commit that referenced this pull request Jul 6, 2023
@tonidero
Fix integration tests and change to a different project (#1123)
982d3f8 
### Description
This fixes a couple of things:
- Fixes integration tests after the changes in #1106. There were some
compiler issues
- Changes the project used by production integration tests and
production backend integration tests to make it more stable (We were
using a shared project before 😬 )
@tonidero tonidero mentioned this pull request Jul 10, 2023
Merged
tonidero added a commit that referenced this pull request Jul 10, 2023
@tonidero
Fix release path after module refactor (#1129)
fc9e67b 
### Description
We forgot to update the paths to update versions in #1106. This fixes it
so we can make a new release
@RCGitBot RCGitBot mentioned this pull request Jul 10, 2023
Merged
tonidero added a commit that referenced this pull request Jul 12, 2023
@RCGitBot @tonidero
[AUTOMATIC] Release/6.6.0 (#1130)
5acb6ee 
**This is an automatic release.**

### New Features
* `Trusted Entitlements`: made API stable (#1105) via NachoSoto
(@NachoSoto)

This new feature prevents MitM attacks between the SDK and the
RevenueCat server.
With verification enabled, the SDK ensures that the response created by
the server was not modified by a third-party, and the entitlements
received are exactly what was sent.
This is 100% opt-in. `EntitlementInfos` have a new `VerificationResult`
property, which will indicate the validity of the responses when this
feature is enabled.

```kotlin
fun configureRevenueCat() {
    val configuration = PurchasesConfiguration.Builder(context, apiKey)
        .entitlementVerificationMode(EntitlementVerificationMode.INFORMATIONAL)
        .build()
    Purchases.configure(configuration)
}
```
### Experimental features
* Add await offerings (#1096) via Cesar de la Vega (@vegaro)
### Bugfixes
* Fix issue updating customer info on app open (#1128) via Toni Rico
(@tonidero)
### Dependency Updates
* Bump fastlane-plugin-revenuecat_internal from `13773d2` to `b2108fb`
(#1095) via dependabot[bot] (@dependabot[bot])
### Other Changes
* [PurchaseTester] Add option to purchase an arbitrary product id
(#1099) via Mark Villacampa (@MarkVillacampa)
* Fix release path after module refactor (#1129) via Toni Rico
(@tonidero)
* Fix load shedder integration tests (#1125) via Toni Rico (@tonidero)
* Trusted entitlements: New trusted entitlements signature format
(#1117) via Toni Rico (@tonidero)
* Fix integration tests and change to a different project (#1123) via
Toni Rico (@tonidero)
* Move files into src/main/kotlin (#1122) via Cesar de la Vega (@vegaro)
* Remove public module (#1113) via Cesar de la Vega (@vegaro)
* Remove common module (#1106) via Cesar de la Vega (@vegaro)
* Fix flaky integration tests: Wait for coroutines to finish before
continuing (#1120) via Toni Rico (@tonidero)
* Move amazon module into purchases (#1112) via Cesar de la Vega
(@vegaro)
* Trusted entitlements: Add IntermediateSignatureHelper to handle
intermediate signature verification process (#1110) via Toni Rico
(@tonidero)
* Trusted entitlements: Add Signature type to process new signature
response format (#1109) via Toni Rico (@tonidero)
* [EXTERNAL] Add `awaitCustomerInfo` / coroutines tests to
`TrustedEntitlementsInformationalModeIntegrationTest` (#1077) via
@pablo-guardiola (#1107) via Toni Rico (@tonidero)
* Remove feature:google module (#1104) via Cesar de la Vega (@vegaro)
* Remove identity module (#1103) via Cesar de la Vega (@vegaro)
* Remove subscriber attributes module (#1102) via Cesar de la Vega
(@vegaro)
* Delete utils module (#1098) via Cesar de la Vega (@vegaro)
* Remove strings module (#1097) via Cesar de la Vega (@vegaro)
* Update CHANGELOG.md to include external contribution (#1100) via Cesar
de la Vega (@vegaro)
* [EXTERNAL] Add missing `fetchPolicy` parameter to `awaitCustomerInfo`
API (#1086) via @pablo-guardiola (#1090) via Toni Rico (@tonidero)

---------

Co-authored-by: revenuecat-ops <[email protected]>
Co-authored-by: Toni Rico <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tonidero tonidero tonidero approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@vegaro @tonidero