Skip to content

Simplified EntitlementInfo decoding using new @DefaultValue property wrappers#1540

Merged
NachoSoto merged 1 commit into
mainfrom
default-value-decoder-using-it
May 10, 2022
Merged

Simplified EntitlementInfo decoding using new @DefaultValue property wrappers#1540
NachoSoto merged 1 commit into
mainfrom
default-value-decoder-using-it

Conversation

@NachoSoto

Copy link
Copy Markdown
Contributor

Follow up to #1537.
EntitlementInfo.ProductData and related types will be moved out as part of #1496, to become part of the CustomerInfo response. But this is an intermediate change to simplify the decoding of this part of the data.

@NachoSoto NachoSoto requested a review from a team April 26, 2022 17:35
Comment on lines 91 to 100

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.

This is replaced by the new method that provides the default value statically.

Comment on lines 26 to 35

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.

Goodbye custom code 👋🏻

@NachoSoto NachoSoto force-pushed the default-value-decoder branch from 918f6cf to ff8f417 Compare April 26, 2022 18:11
@NachoSoto NachoSoto force-pushed the default-value-decoder-using-it branch from 500f997 to bf067e6 Compare April 26, 2022 18:12
@NachoSoto NachoSoto force-pushed the default-value-decoder branch from ff8f417 to 3a9ec92 Compare April 26, 2022 23:36
@NachoSoto NachoSoto force-pushed the default-value-decoder-using-it branch from bf067e6 to e4647a3 Compare April 26, 2022 23:37
@NachoSoto NachoSoto force-pushed the default-value-decoder branch from 3a9ec92 to f7a3452 Compare April 26, 2022 23:56
@NachoSoto NachoSoto force-pushed the default-value-decoder-using-it branch from e4647a3 to c4efd86 Compare April 26, 2022 23:57
@NachoSoto NachoSoto force-pushed the default-value-decoder branch from 77ef317 to a9b4a53 Compare April 28, 2022 21:25
@NachoSoto NachoSoto force-pushed the default-value-decoder-using-it branch from c4efd86 to b507a95 Compare April 28, 2022 21:25
@NachoSoto NachoSoto force-pushed the default-value-decoder branch 2 times, most recently from 4d23d75 to e0b26ce Compare May 10, 2022 15:50

@joshdholtz joshdholtz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm learning so much about Swift 🤯 I love this

Base automatically changed from default-value-decoder to main May 10, 2022 17:26
NachoSoto added a commit that referenced this pull request May 10, 2022
A future PR will make use of these: #1540.
The purpose of this is to be able to define `Decodable` types with default values without needing to implement the whole `init(from decoder: Decoder)` from scratch. If any one property has a default value, that's the only way to provide it without these.

For example, `EntitlementInfo.ProductData` implements this:
```swift
init(from decoder: Decoder) throws {
    let container = try decoder.container(keyedBy: CodingKeys.self)

    self.isSandbox = try container.decodeIfPresent(Bool.self, forKey: .isSandbox) ?? false
    self.originalPurchaseDate = try container.decodeIfPresent(Date.self, forKey: .originalPurchaseDate)
    self.expiresDate = try container.decodeIfPresent(Date.self, forKey: .expiresDate)
    self.unsubscribeDetectedAt = try container.decodeIfPresent(Date.self, forKey: .unsubscribeDetectedAt)
    self.billingIssuesDetectedAt = try container.decodeIfPresent(Date.self, forKey: .billingIssuesDetectedAt)
    self.periodType = container.decode(PeriodType.self, forKey: .periodType, defaultValue: .normal)
    self.store = container.decode(Store.self, forKey: .store, defaultValue: .unknownStore)
    self.ownershipType = container.decode(PurchaseOwnershipType.self,
                                          forKey: .ownershipType,
                                          defaultValue: .purchased)
}
```

With these new types, that whole implementation is unnecessary and can be data-driven instead, which is more maintanable and less error-prone:
```swift
struct ProductData: Decodable {

    @DefaultValue<PeriodType> var periodType: PeriodType
    var originalPurchaseDate: Date?
    var expiresDate: Date?
    @DefaultValue<Store> var store: Store
    @DefaultDecodable.False var isSandbox: Bool
    var unsubscribeDetectedAt: Date?
    var billingIssuesDetectedAt: Date?
    @DefaultValue<PurchaseOwnershipType> var ownershipType: PurchaseOwnershipType

}
```
@NachoSoto NachoSoto force-pushed the default-value-decoder-using-it branch from b507a95 to 4551c0d Compare May 10, 2022 17:27
@NachoSoto NachoSoto merged commit 1d7d54a into main May 10, 2022
@NachoSoto NachoSoto deleted the default-value-decoder-using-it branch May 10, 2022 17:29
NachoSoto added a commit that referenced this pull request May 24, 2022
### Depends on:
- #1537
- #1540
- #1541
- #1543
- #1546
- #1547
- #1550
- #1551,
- #1565

### Changes

- Removed all custom deserialization code thanks to #1537, #1541, and #1543.
- The format of the customer info response is now very concise and only in one place (`CustomerInfoResponse`) instead of being passed through dictionaries and magic strings everywhere.
- `CustomerInfo` was combining the dictionaries inside of `"subscriptions"` and `"non_subscriptions"`, despite them having different formats. This is now made explicit through type conversions between the two, (see `CustomerInfoResponse.allTransactionsByProductId`).
- Removed all custom `CustomerInfo` errors since deserialization is automatic, and the underlying `Error` information will be provided thanks to `ErrorUtils.logDecodingError`.
- `CustomerInfo` deserialization always wraps errors into `ErrorCode.customerInfoError`, which is now also covered in a new test.
- Added a new snapshot test that covers `CustomerInfo` serialization by storing the result in a JSON file.

### Other changes:
- `CustomerInfo` equality is now automatic through the `CustomerInfoResponse` `Equatable` conformance.
- Simplified `schemaVersion` handling
- All tests still use `CustomerInfo(data: [String: Any])`, but that method is only visible for tests now, and it uses the underlying `Decodable` implementation (see `CustomerInfo+TestExtensions.swift`).
- Added extra tests for `CustomerInfoResponse` that use a fixture `CustomerInfo.json`, which is easier to maintain than having JSON in code.
- `RawDataContainer` implementation is changed in #1565.

### TODO:
- [x] Fix all tests
- [x] Update `RawValueContainer`
- [x] Consider recovering `CustomerInfoError`
- [x] Look at all previously logged errors
- [x] Finish `CustomerInfoResponseTests`
- [x] Handle all TODOs
- [x] Make `RawDataContainer` actually store all the original data: #1565
@joshdholtz joshdholtz mentioned this pull request May 26, 2022
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