Add support for a post-processing hook at the request level

[nbrooke]

Right now, if you want to do any common operation on the final result of a Netable request (say storing something to a global cache), you need to either implement your own finalize function and do it there (which works in some cases but can seem bit strange, and also doesn't have access to the error which might be usefull in some cases), or you need to write a function wrapping the Netable call in some way (which can lead to the sort of excessive abstraction and wrapping describe in #78).

As an an alternative, we could provide another per-request customization point along side decode and finalize that runs after finalize and has an opportunity to use the contents of the incoming request:

protocol Request {
    //...

    func postProcess(result: Result<FinalResource, NetableError>) -> Result<FinalResource, NetableError>
}

public extension Request where FinalResource == RawResource {
    func postProcess(result: Result<FinalResource, NetableError>) -> Result<FinalResource, NetableError> {
        return result
    }
}

A client that wanted to take a global action on a specific type of request, could then implement that to get access to the result without needing a wrapper function:

struct CurrentUserRequest: Request {
    //...

    func postProcess(result: Result<User, NetableError>) -> Result<User, NetableError> {
        if case .success(let user) = result {
            DataManager.shared.currentUser = user
        }
        return result
    }
}    

[amyoulton]

Hey Nigel! I just want to add some clarification that I properly understand the goal and scope of this ticket.

The main goal here is to add in an optional request method that allows people to consistently do some sort of basic action (like updating the user in a general manager) every single time the request is run, but before the result is returned from the request as a whole.

In a whole, it will essentially act as the very final step, receiving the result of the request, doing an action, and then returning the result of the request, ending the functionality the request as a whole. Is that correct?

[nbrooke]

That is correct.

[amyoulton]

I've spend some time exploring ticket #92 and implementing this works perfectly without having access to NetableError. I want confirm my findings and see if my understanding of this is correct, or if there is something I am missing. Currently, the way the request is set up, all of the request specific logic is happening within the startRequestTask. In the implementation I've done without NetableError, we've essentially added an extra step that either does nothing and just returns that finalResource by default, or would allow the client to do some logic like in the ticket. The problem is if the goal is to really have the client have access to that error within the postProcess method, there is nowhere I can see to make a Result type work before the startRequestTask ends. Even moving beyond startRequest task back into the top level request, if startRequest didn't throw an error, we're still inside of that do block. We just are no longer working with the Result type. If we want to have it mimic a Result, we can either have the post process take in a successful finalResource or an error? I just want to get some thoughts before I do anything else here, as I'm not sure whether this concern is founded or if I'm misunderstanding something!

[amyoulton]

Given the natures of the above conversation, I've decided to push the code I wrote up to a draft PR to help the conversation.

[nbrooke]

I think the concern is definitely founded, it looks to me like there is a (likely resolvable, but annoying) mismatch here. That bug does predate the switch to async/await so at that point Result was used in a lot of places that are async throws now, so would be a more natural fit.

I think using Result if we do that does still make sense, you say "we can either have the post process take in a successful finalResource or an error", but "final resource or an error" IS pretty much the definition of the Result<FinalResource> type, so I think using that still makes more sense than like, postProcess taking an optional FinalResource and error.

Doing it with Result seems still possible, but I think requires similar processing in both the success and failure cases. i.e. probably in request, where it calls startRequestTask it needs to call postProcess from BOTH the do and catch blocks and wrap in a result and unwrap after, which is probably going to be an annoying volume of code, something like:

do {
    // ...
    let successfulResult = try away startRequestTask(...)
    let postProcessed = urlRequest.postProcess(.success(sucessfulResult))

    switch postProcessed {
        case .success(let value):
           return value
        case .failure(let error):
           throw error
    }

} catch {
    // ...

    let postProcessed = urlRequest.postProcess(.failure(error))

    switch postProcessed {
        case .success(let value):
           return value
        case .failure(let error):
               throw error
    }

}

Which will WORK, but is pretty gross, and has some weird edge cases (post process gets called twice if post process changes a success into an error, for example). I can't think of a better way to handle that though.