This is working now 🎉

Gone in favor of the Decodable ETagManager.Response`.

This was the key. To be able to store a type erased Request, the completion handler just deals with the Data itself. The constructor is the only thing that has the Value type information, so it handles the deserialization there so everything else in HTTPClient is type agnostic.

This is one of the last TODOs, when the response fails to deserialize I need to improve how the error is created. Now the CodableError is what's forwarded which isn't ideal.

will this be a part of a different PR or this one?

Working on it in #1437, that's the last piece to finish.

This is officially gone in #1437 .

All this code is now gone 🎉

I understand this code might be hard to follow, it's peak "functional" code. Ultimately it's just converting from Data to Value by calling the HTTPResponseBody decoding method. The nesting is because it's inside of the hierarchy of types of Result and HTTPResponse.

Feedback welcome if there's objections about it.

Honestly, I had a tough time understanding this. I think the mixture of functional methods, generic types, conversion of result types, Result.init with the block passed in directly, deep nesting, and implicit returns was a bit too much for my brain to handle.

Thoughts on making it easier to understand:

• Adding explicit types so that the conversions are easier to follow
• Making the result.init call pass in the block with the name
• using ResponseBody instead of just Value so the type is easier to keep track of

// Parses a `Result<HTTPResponse<Data>>` to `Result<HTTPResponse<Value>>`
func parseResponse<ResponseBody: HTTPResponseBody>() -> HTTPResponse<ResponseBody>.Result {
    return self.flatMap { (response: HTTPResponse<Data>) in
        HTTPResponse<ResponseBody>.Result.init(catching: {
            try response.mapBody { (data: Data) in
                try ResponseBody.create(with: data) // build HTTPResponseBody
            }
        })
    }
}

But even with those changes this is hard to read for me. @taquitos thoughts on this?

Honestly, I had a tough time understanding this. I think the mixture of functional methods, generic types, conversion of result types, Result.init with the block passed in directly, deep nesting, and implicit returns was a bit too much for my brain to handle.

Totally fair.

How about explaining what each line does in English (plus maybe adding all explicit types)?

// Parses a `Result<HTTPResponse<Data>>` to `Result<HTTPResponse<Value>>`
func parseResponse<Value: HTTPResponseBody>() -> HTTPResponse<Value>.Result {
    return self.flatMap { response in          // Convert the `Result` type
        HTTPResponse<Value>.Result {           // Create a new `HTTPResponse<Value>`
            try response.mapBody { data in     // Convert the body of `HTTPResponse<Data>` from `Data` -> `Value`
                try Value.create(with: data)   // Decode `Data` into `Value`
            }
        }
    }
}

Something to comment on this, is that this code is doing just a data transformation, it's not like it has some weird logic that needs to be maintained. There really is only one way to implement this, maybe many ways of writing it, but they would all compile to the same thing.

This is the main change in this PR.

This typealias is 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 underlyingErrors.

The new implementation still allows extracting a plain [String: Any] to maintain compatibility for types that aren't Decodable yet.

This is super simple now. If there's no attribute errors, return customer info. If there are attribute errors, return error.

Composition at its best. Decoding 2 different types to create one 🌮

🤔 is this necessary? I'm wondering if this might make it trickier for customers to use our classes for testing if they're doing inherit+replace

🤔 is this necessary?

It is to be able to use Self in the HTTPResponseBody implementation. I could make that explicitly use CustomerInfo, but.

I'm wondering if this might make it trickier for customers to use our classes for testing if they're doing inherit+replace

None of our classes are open. Since Swift 3, Swift classes are sealed (non-open) by default. So this being final only affects the SDK itself.

is there a PR for this bit?

There's not, I can create a Jira for it.
It's one of the last classes that still needs the conversion. That's why I've made the new system still work with manual serialization.

Working on that in #1496.

👍 is there a PR for a jira for it?

will this be a part of a different PR or this one?

Honestly, I had a tough time understanding this. I think the mixture of functional methods, generic types, conversion of result types, Result.init with the block passed in directly, deep nesting, and implicit returns was a bit too much for my brain to handle.

Thoughts on making it easier to understand:

• Adding explicit types so that the conversions are easier to follow
• Making the result.init call pass in the block with the name
• using ResponseBody instead of just Value so the type is easier to keep track of

// Parses a `Result<HTTPResponse<Data>>` to `Result<HTTPResponse<Value>>`
func parseResponse<ResponseBody: HTTPResponseBody>() -> HTTPResponse<ResponseBody>.Result {
    return self.flatMap { (response: HTTPResponse<Data>) in
        HTTPResponse<ResponseBody>.Result.init(catching: {
            try response.mapBody { (data: Data) in
                try ResponseBody.create(with: data) // build HTTPResponseBody
            }
        })
    }
}

But even with those changes this is hard to read for me. @taquitos thoughts on this?