`OfferingsManager`/`OfferingsFactory`: using `OfferingsResponse` #1435

NachoSoto · 2022-03-31T19:01:12Z

Depends on #1437.
This takes full advantage of Decodable when decoding and creating Offerings.

taquitos

Awesome. 🥅 ⚽ 🐐 🎉

NachoSoto · 2022-04-01T21:02:44Z

Sources/Networking/Operations/GetOfferingsOperation.swift

@@ -46,7 +46,7 @@ private extension GetOfferingsOperation {

        let request = HTTPRequest(method: .get, path: .getOfferings(appUserID: appUserID))

-        httpClient.perform(request, authHeaders: self.authHeaders) { (response: HTTPResponse<[String: Any]>.Result) in
+        httpClient.perform(request, authHeaders: self.authHeaders) { (response: HTTPResponse<OfferingsResponse>.Result) in


This is all it takes to get HTTPClient to deserialize the response 🤓

This will become the response type used everywhere by `OfferingsManager` in #1435.

NachoSoto · 2022-04-11T22:30:18Z

This is finished now 🎉

NachoSoto · 2022-04-11T22:30:50Z

Sources/Purchasing/OfferingsFactory.swift

@@ -17,66 +17,66 @@ import StoreKit

 class OfferingsFactory {


No more magic strings in this file 😄

NachoSoto · 2022-04-11T22:31:22Z

Sources/Purchasing/OfferingsManager.swift

@@ -37,7 +37,7 @@ class OfferingsManager {
        self.productsManager = productsManager
    }

-    func offerings(appUserID: String, completion: ((Offerings?, Error?) -> Void)?) {
+    func offerings(appUserID: String, completion: ((Result<Offerings, Error>) -> Void)?) {


Finished using Result around OfferingsManager too.

NachoSoto · 2022-04-11T22:31:36Z

Sources/Purchasing/OfferingsManager.swift

    }

-    func extractProductIdentifiers(fromOfferingsData offeringsData: [String: Any]) -> Set<String> {
-        // Fixme: parse Data directly instead of converting from Data to Dictionary back to Data


NachoSoto · 2022-04-11T22:32:12Z

Tests/UnitTests/Networking/Backend/BackendGetOfferingsTests.swift

-        expect(offeringA["identifier"] as? String) == "offering_a"
-        expect(offeringA["description"] as? String) == "This is the base offering"
-        expect(packageA["identifier"]) == "$rc_monthly"
-        expect(packageA["platform_product_identifier"]) == "monthly_freetrial"
-        expect(packageB["identifier"]) == "$rc_annual"
-        expect(packageB["platform_product_identifier"]) == "annual_freetrial"
-        expect(result?.value?["current_offering_id"] as? String) == "offering_a"


No more magic strings

🎉🎉🎉

NachoSoto · 2022-04-11T22:32:45Z

Tests/UnitTests/Purchasing/OfferingsTests.swift

@@ -202,11 +199,15 @@ class OfferingsTests: XCTestCase {
    }

    func testOfferingsIsNilIfNoOfferingCanBeCreated() throws {


This is kind of an end-to-end test including deserialization too.

…rkError` and `BackendError` (#1425) Closes #695 and finishes [CF-195]. Depends on ~~#1431, #1432, #1433, #1437~~. ## Goals: - [x] Handle all requests / response / deserialization errors in `HTTPClient`. Clients of `HTTPClient` will only have to handle the "happy" path: #1431 - [x] `HTTPClient` shouldn't return `Result.success` with failed responses, forcing clients to verify the response is actually a failure: #1431 - [x] Abstract error response deserialization / error creation: #1427 - [x] Abstract attribute error parsing: #1427 - [x] Move response deserialization to `HTTPClient` based on a `ResponseType: Decodable` type, so the completion block will simply return a `Result<HTTPResponse<ResponseType>, Error>` - [x] `ETagManager` should store response `Data` instead of a deserialized `[String: Any]` - [x] Improved error types in `HTTPClient` to fix tests: #1437 - [x] Dealt with non-backwards compatible `ETagManager: #1438 ## Changes: - Replaced `HTTPResponse`'s body from `[String: Any]` to a generic `HTTPResponseBody`. - Created `HTTPResponseBody` to abstract `Decodable` and provide some default implementations for types like `Data,` `[String: Any]` (for backwards compatibility to types that aren't `Decodable` yet), and `Decodable` itself. - New `HTTPResponse.Result` typealias (`Result<HTTPResponse<HTTPResponseBody>, Error>`) used everywhere. This will allow replacing `Error` with a more specific `Error` so we can forward known typed errors, and make sure that we don't end up with the wrong error type, or with a very complex error hierarchy and the details buried in `underlyingError`. - Each layer (only a few for now) has its own error type: `NetworkError`, `BackendError`, `OfferingsManager.Error`. - `HTTPClient` for example has to produce `NetworkError`, operations produce `BackendError` - The parent `BackendError` can have specific errors like `.missingAppUserID`, but also be simply a child error `case networkError(NetworkError)` - All of these conform to a `ErrorCodeConvertible`, so there is a single point of code that converts from simple and readable errors (like `BackendError.emptySubscriberAttributes`, `.unexpectedBackendResponse(.loginResponseDecoding)`) into errors with all the context using `ErrorUtils` - Converted `DNSError` into `NetworkError.dnsError`. Its functionality remains unchanged. - Removed `Backend.RCSuccessfullySyncedKey` and `ErrorDetails.finishableKey` in favor of tested properties on `NetworkError` ### Leftovers There's a few things I've decided to not finish for now: - [ ] `CustomerInfo` still does't conform to `Decodable` (manual deserialization is still supported by the current system): #1496 - `SubscriberAttributeDict` could also be `Codable` - [x] Replace `OfferingsFactory` with `Decodable`: #1435 [CF-195]: https://revenuecats.atlassian.net/browse/CF-195?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

NachoSoto · 2022-04-14T12:40:44Z

This is still waiting for review 🤓

This takes full advantage of `Decodable` when decoding and creating `Offerings`.

aboedo · 2022-04-22T18:01:50Z

Tests/UnitTests/Networking/Backend/BackendGetOfferingsTests.swift

-        expect(offeringA["identifier"] as? String) == "offering_a"
-        expect(offeringA["description"] as? String) == "This is the base offering"
-        expect(packageA["identifier"]) == "$rc_monthly"
-        expect(packageA["platform_product_identifier"]) == "monthly_freetrial"
-        expect(packageB["identifier"]) == "$rc_annual"
-        expect(packageB["platform_product_identifier"]) == "annual_freetrial"
-        expect(result?.value?["current_offering_id"] as? String) == "offering_a"


🎉🎉🎉

aboedo · 2022-04-22T18:03:28Z

Sources/Networking/Responses/OfferingsResponse.swift

+struct OfferingsResponse {
+
+    struct Offering {
+
+        struct Package {
+
+            let identifier: String
+            let platformProductIdentifier: String
+
+        }
+
+        let identifier: String
+        let description: String
+        let packages: [Package]
+
+    }
+
+    let currentOfferingId: String?
+    let offerings: [Offering]
+
+}


NachoSoto mentioned this pull request Mar 31, 2022

HTTPClient now handles response deserialization + Introduced NetworkError and BackendError #1425

Merged

10 tasks

NachoSoto force-pushed the wip-decodable-3-rebased branch 2 times, most recently from 63cbf52 to 4e0dfa8 Compare March 31, 2022 19:11

NachoSoto force-pushed the offerings-response branch from 8ec75cd to c6c0680 Compare March 31, 2022 19:12

NachoSoto mentioned this pull request Mar 31, 2022

OfferingsManager: replaced manual decoding with Decodable #1433

Merged

taquitos approved these changes Mar 31, 2022

View reviewed changes

NachoSoto force-pushed the wip-decodable-3-rebased branch from 4e0dfa8 to c53e94b Compare April 1, 2022 00:31

NachoSoto force-pushed the offerings-response branch from c6c0680 to 957cb7b Compare April 1, 2022 00:31

NachoSoto force-pushed the wip-decodable-3-rebased branch from c53e94b to 7bfbd17 Compare April 1, 2022 20:52

NachoSoto force-pushed the offerings-response branch from 957cb7b to 8d75181 Compare April 1, 2022 21:01

NachoSoto commented Apr 1, 2022

View reviewed changes

NachoSoto force-pushed the wip-decodable-3-rebased branch 2 times, most recently from 0e34e67 to 87a2c90 Compare April 5, 2022 18:57

NachoSoto added a commit that referenced this pull request Apr 5, 2022

OfferingsManager: replaced manual decoding with Decodable (#1433)

cdf3ecd

This will become the response type used everywhere by `OfferingsManager` in #1435.

NachoSoto force-pushed the offerings-response branch from 8d75181 to 743c458 Compare April 5, 2022 19:10

NachoSoto force-pushed the wip-decodable-3-rebased branch from 87a2c90 to 27a3ea3 Compare April 6, 2022 21:35

NachoSoto force-pushed the offerings-response branch from 743c458 to 8430670 Compare April 6, 2022 21:36

NachoSoto force-pushed the wip-decodable-3-rebased branch from a6e6d43 to 826abb8 Compare April 8, 2022 01:23

NachoSoto force-pushed the offerings-response branch from 8430670 to 921d8a8 Compare April 8, 2022 18:16

NachoSoto force-pushed the wip-decodable-3-rebased branch from 4a6ba22 to 7445957 Compare April 8, 2022 21:36

NachoSoto force-pushed the offerings-response branch from 921d8a8 to da91c0c Compare April 8, 2022 21:40

NachoSoto force-pushed the wip-decodable-3-rebased branch from 7445957 to 8e0c0be Compare April 8, 2022 22:49

NachoSoto force-pushed the offerings-response branch 4 times, most recently from ad774e8 to 7456ec6 Compare April 11, 2022 22:29

NachoSoto changed the title ~~[WIP] Using OfferingsResponse in OfferingsManager~~ OfferingsManager/OfferingsFactory: using OfferingsResponse Apr 11, 2022

NachoSoto changed the base branch from wip-decodable-3-rebased to http-error April 11, 2022 22:30

NachoSoto requested review from taquitos and a team April 11, 2022 22:30

NachoSoto commented Apr 11, 2022

View reviewed changes

NachoSoto added the sharpen the saw label Apr 12, 2022

NachoSoto force-pushed the http-error branch from 11d5c7f to e570b81 Compare April 12, 2022 19:46

NachoSoto force-pushed the offerings-response branch from 7456ec6 to 62eaa0e Compare April 12, 2022 19:46

Base automatically changed from http-error to wip-decodable-3-rebased April 12, 2022 20:58

NachoSoto force-pushed the offerings-response branch from 62eaa0e to 88eb5be Compare April 12, 2022 21:06

Base automatically changed from wip-decodable-3-rebased to main April 12, 2022 22:11

NachoSoto force-pushed the offerings-response branch 3 times, most recently from 97fb1d0 to 80e56be Compare April 14, 2022 12:37

NachoSoto force-pushed the offerings-response branch from 80e56be to 1af39a9 Compare April 20, 2022 20:26

OfferingsManager/OfferingsFactory: using OfferingsResponse

7aafc8f

This takes full advantage of `Decodable` when decoding and creating `Offerings`.

NachoSoto force-pushed the offerings-response branch from 1af39a9 to 7aafc8f Compare April 21, 2022 17:07

aboedo approved these changes Apr 22, 2022

View reviewed changes

NachoSoto merged commit 7530a67 into main Apr 22, 2022

NachoSoto deleted the offerings-response branch April 22, 2022 18:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`OfferingsManager`/`OfferingsFactory`: using `OfferingsResponse` #1435

`OfferingsManager`/`OfferingsFactory`: using `OfferingsResponse` #1435

NachoSoto commented Mar 31, 2022 •

edited

Loading

taquitos left a comment

NachoSoto Apr 1, 2022

NachoSoto commented Apr 11, 2022

NachoSoto Apr 11, 2022

NachoSoto Apr 11, 2022

NachoSoto Apr 11, 2022

NachoSoto Apr 11, 2022

aboedo Apr 22, 2022

NachoSoto Apr 11, 2022

NachoSoto commented Apr 14, 2022

aboedo Apr 22, 2022

aboedo Apr 22, 2022

		@@ -202,11 +199,15 @@ class OfferingsTests: XCTestCase {
		}

		func testOfferingsIsNilIfNoOfferingCanBeCreated() throws {

OfferingsManager/OfferingsFactory: using OfferingsResponse #1435

OfferingsManager/OfferingsFactory: using OfferingsResponse #1435

Conversation

NachoSoto commented Mar 31, 2022 • edited Loading

taquitos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NachoSoto commented Apr 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NachoSoto commented Apr 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

`OfferingsManager`/`OfferingsFactory`: using `OfferingsResponse` #1435

`OfferingsManager`/`OfferingsFactory`: using `OfferingsResponse` #1435

NachoSoto commented Mar 31, 2022 •

edited

Loading