Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementing protocol EmptyResponse so that an empty response value can be returned when an empty http status code is encountered #2664

Merged
merged 6 commits into from Mar 7, 2019

Conversation

Projects
None yet
3 participants
@jvannoord
Copy link
Contributor

jvannoord commented Dec 12, 2018

Goals ⚽️

An empty result should not need to be derived from the Empty type.

Implementation Details 🚧

It is inconvenient (and restrictive) to require that all empty response value derived form Empty. Using a protocol allows any response value type to declare its own empty value.

The EmptyResponse protocol essential does what the Empty struct did; and the Empty struct has been extended to implement EmptyResponse. No implementations of Empty should be adversely affected.

This PR supersedes #2662, which was originally based the alamofire5 branch and had a number of requested changes.

Testing Details 🔍

Added tests to test for objects that implement EmptyResponse and objects that incorrectly implement EmptyResponse. Bool is extended in order to test a built-in type; Int is not extended and tested for failure.

James Van Noord added some commits Dec 7, 2018

James Van Noord
Implemented protocol EmptyResponse so that an empty response value ca…
…n be returned without requiring that T be dervied from Empty

(cherry picked from commit 7048265)
James Van Noord
adding tests for DecodableResponseSerializer that test empty http sta…
…tus codes against types that do and do not conform to EmptyResponse
@jshier
Copy link
Contributor

jshier left a comment

Would it work to have the protocol property return Self?

Show resolved Hide resolved Source/ResponseSerialization.swift Outdated
@jshier
Copy link
Contributor

jshier left a comment

I think the static function has made things better, but there still seems to be a bit of cleanup left. Looking forward to this feature!

Show resolved Hide resolved Source/ResponseSerialization.swift Outdated
Show resolved Hide resolved Source/ResponseSerialization.swift Outdated
Show resolved Hide resolved Source/ResponseSerialization.swift Outdated
Show resolved Hide resolved Tests/ResponseSerializationTests.swift Outdated
Show resolved Hide resolved Source/ResponseSerialization.swift Outdated

@jshier jshier added this to the 5.X milestone Mar 6, 2019

@jshier

jshier approved these changes Mar 7, 2019

Copy link
Contributor

jshier left a comment

👍 Thanks again, this should be in beta 3!

@jshier jshier merged commit 8006676 into Alamofire:master Mar 7, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jshier jshier modified the milestones: 5.X, 5.0.0-beta.3 Mar 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.