### Motivation:
Initially I wanted to make `SubscriberAttribute.asDictionary` work using `Codable`. However, this isn't the right refactor because we need the actual raw types to be encoded inside `UserDefaults`.
At least this refactor provides several improvements:
### Changes:
- `SubscriberAttribute` now has value semantics
- ⚠️ `SubscriberAttribute.isSynced` used to be mutable, with reference semantics, which was dangerous
- `SubscriberAttribute: Equatable` implementation is now automatic
- Cleaned up `SubscriberAttribute.asDictionary` implementation, extracting keys into an `enum`
- Moved `Dictionary` -> `SubscriberAttribute` creation to the type itself
- ⚠️ `SubscriberAttribute.init(dictionary:)` returns an `Optional` instead of using force-unwrap, avoiding potential crashes
- Added more serialization / deserialization tests
- Removed `SubscriberAttributesMarshaller` and moved method into `SubscriberAttribute` extension
- Moved `SubscriberAttributeDict` into `SubscriberAttribute.Dictionary`
### Changes:
- Using `Dictionary.init(minimumCapacity:)` to allocate the necessary size initially instead of growing when adding data to it
- Defined `uppercase` in terms of `lowercase`
- Removed magic `13` and `26` numbers
Fixes#1685.
### Description
Swift's `lazy` and `static` property initialization is always thread-safe. However, `key` was only created as an empty dictionary, and the evaluation of `ROT13.string`, which was the one initializing that data, wasn't isolated.
By creating `ROT13.key` inline, this ensures that the initialization is only done once in a thread-safe way.
See also #1658.
This updates the behavior of `CustomerInfoResponse` decoding so that having invalid content inside of one of the structures fails more loudly instead of getting ignored.
For that, `@DefaultValue` no longer ignores errors, and instead `@IgnoreDecodeErrors` must be used.
For some of the values in `CustomerInfoResponse` this makes sense (like `Store`), but not for things like subscriptions where we would be losing data.
See #1651.
Since that regression fix, we're no longer using it because it's not possible to make it decode dictionary keys without changing their case.
Therefore we can't ensure data isn't changed in the process.
Both `return` statements where the same, the `else` was just providing a default value to `customerInfoIfSynced`.
Because `??` doesn't support an `async` function, we couldn't use that. By using this new operator we can simplify the implementation to a single `return` statement.
The method doesn't actually throw, so calling `throw decoder.throwValueNotFoundError()` was confusing. Now the code reads `throw decoder.valueNotFoundError()`.
This allows having properties in an `Encodable` type that won't be encoded.
This will be used to storing the `rawData` when decoding our types (see #1496).
Turns out `Data(string.utf8)` is a much simpler way of converting `String` to `Data` that doesn't lead to an `Optional`.
UTF-8 can represent all valid Unicode code points, so this was always safe, but now we know at compile time.
The reason the old variant returned an `Optional` is because it accepts other encodings like `.ascii` which might not be able to represent everything in the `String`.
This simplifies several parts of the code that were dealing with an error that was impossible.
The new test illustrates the behavior change that adding `@LossyDictionary` to a `[String: Decodable]` property had. Keys were being converted to camel case, which is inconsistent from the default behavior that `container.decode([String: Value].self, forKey: key)` would have.
The reason for this is some implementation detail on Foundation that ignores the `keyDecodingStrategy` when decoding "raw" dictionaries versus property names. To deal with this, we decode the strings first as "raw", and then match them to the corresponding converted key.
This was necessary to make sure that keys [like these](https://github.com/RevenueCat/purchases-ios/blob/4.3.0/Tests/UnitTests/Purchasing/CustomerInfoTests.swift#L58) aren't incorrectly converted to camel case, which is required for #1496.
Inspired by https://github.com/marksands/BetterCodable/blob/master/Sources/BetterCodable but supporting nested collections and with simplified code.
These property wrappers allows decoding `Array`s and `Dictionary`s and ignore elements that fail to decode.
For example, this will ignore any elements that aren't numbers, instead of failing to decode altogether.
```swift
struct Data: Decodable {
@LossyArray var list: [Int]
@LossyDictionary var map: [String: Int]
}
```
This does require that the values are the right type, but these wrappers can be composed with `@DefaultDecodable.EmptyArray` and `@DefaultDecodable.EmptyDictionary` introduced in #1537 to make it produce an empty array in case of any other type error:
```swift
struct Data: Decodable {
@DefaultDecodable.EmptyArray @LossyArray var list: [Int]
@DefaultDecodable.EmptyDictionary @LossyDictionary var map: [String: Int]
}
```
Because of limitations of the property wrappers in Swift, an extra type `LossyArrayDictionary` allows lossy decoding of types `[String: [Decodable]`.
This will be used to vastly simplify #1496.
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.
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
}
```
The main difference is that `default` has `KeyEncodingStrategy.convertToSnakeCase`. Probably everything was still working because the types that use this explicitly define the keys.
Either way this makes more sense long term.
## Changes:
- Improved output of decoding errors:
#### Before:
```
[Purchases] - ERROR: 😿‼️ Couldn't decode data from json. Error:
The operation couldn’t be completed. (RevenueCat.CodableError error 0.))
```
#### After:
```
[Purchases] - ERROR: 😿‼️ Couldn't decode data from json. Error:
Found unexpected value 'tienda' for type 'Store'
```
- `CodableStrings.decodingError` relied on `Error.localizedDescription`, so `CodableError` now conforms to `LocalizedError` to provide that as well
- Added incorrect `value` to the error to find out why it was invalid
- Removed extra `)` at the end of the error string
- Cleaned up `switch`es to use a single `let` syntax
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: https://github.com/RevenueCat/purchases-ios/pull/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
Follow up to #1429. For #695.
This was duplicated in all operations.
`HTTPClient` now only forwards `Result.success` if the request actually succeeded,
leaving each `NetworkOperation` to just parse the body (which will become part of `HTTPClient` too).
Other changes:
- Simplified logic for when to retry requests in `HTTPClient` by using a new `Result.asOptionalResult`
- Removed several response handlers that don't need to do anything anymore
- Removed several tests that were basically duplicated:
- The `finishable` checks are already tested as part of `ErrorResponse`.
- All the checks for the error details are also part of `ErrorResponse`.
- No longer need to have a test for each type of failed response since that's part of `HTTPClient` tests now.
Follow up to #1429. The `decode` methods no longer mutate the underlying `JSONDecoder`, and instead the codebase uses a shared `defaultJSONDecoder` with the correct configuration.
Follow up to #1427.
The new way to create errors is like this:
```swift
ErrorResponse
.from(response)
.asBackendError(with: statusCode)
```
## Other changes:
- Removed `UserInfoAttributeParser`
- Simplified `JSONDecoder.decode` implementation by simply relying on `defaultJsonDecoder`
- Added support for "wrapped" error responses. This was previously handled by `UserInfoAttributeParser`.
- Removed all duplicated code parsing error responses
- The new `ErrorResponse` introduced in #1427 correctly decodes the format sent by the backend. Note that some tests had the wrong format, and `UserInfoAttributeParser` was parsing that wrong as well.
An upcoming PR will actually move these calls to `ErrorResponse` to `HTTPClient`.
For #695.
The biggest change is that this code at the beginning of `HTTPClient.handle` is now gone:
```swift
var statusCode: HTTPStatusCode = .networkConnectTimeoutError
var jsonObject: [String: Any]?
var httpResponse: HTTPResponse? = HTTPResponse(statusCode: statusCode, jsonObject: jsonObject)
var receivedJSONError: Error?
```
It was hard to trace which paths modified these default values. The new implementation has a mostly linear path (part of it abstracted in smaller methods now) and doesn’t have mutable values.
This is the new `handle` implementation:
```swift
func handle(urlResponse: URLResponse?,
request: Request,
urlRequest: URLRequest,
data: Data?,
error networkError: Error?) {
let (response, shouldRetry) = self.parse(
urlResponse: urlResponse,
request: request,
urlRequest: urlRequest,
data: data,
error: networkError
)
if shouldRetry {
Logger.debug(Strings.network.retrying_request(httpMethod: request.method.httpMethod,
path: request.path))
self.state.modify {
$0.queuedRequests.insert(request.retriedRequest(), at: 0)
}
} else {
request.completionHandler?(response)
}
self.beginNextRequest()
}
```
## All Changes:
- `HTTPClient.perform` no longer has a bunch of mutable variables with forking paths that make it hard to follow. Most of the response handling is now in a new `parse` method. This will simplify the task of continuing to refactor response deserialization.
- `HTTPClient`’s completion abstracts response into `HTTPResponse`
- `UserInfoAttributeParser` is now an `enum`
- `ETagManager` no longer takes an `Error`. The only thing it did with it is to return the default response.
- `HTTPResponse.jsonObject` is no longer `Optional`
- Moved `defaultJsonEncoder` / `defaultJsonDecoder` to `JSONDecoder+Extensions`. They used by `HTTPClient` and others soon.
I noticed that the existing implementation was creating many copies of the string as it grows, which isn't very efficient.
The new implementation allocates the required size initially and writes to the same buffer inside the loop.
NachoSoto