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.

Sign up for GitHub

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

Jump to bottom

fix duplicated Content-Type values #247

Merged
merged 3 commits into from
Jun 22, 2018
Merged

fix duplicated Content-Type values #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a9bcda7
fix duplicated Content-Type values
Apr 16, 2018
b541915
Additional specs for content-type override order
pcantrell Jun 22, 2018
adddd50
More API docs for content-type override order
pcantrell Jun 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions Source/Siesta/Request/RequestCreation.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ public extension Resource

/**
Convenience method to initiate a request with a body containing arbitrary data.
- Parameter method: The HTTP method of the request.
- Parameter data: The body of the request.
- Parameter contentType: The value for the request’s `Content-Type` header. The priority order is as follows:
- any content-type set in `Configuration.mutateRequests(...)` overrides
- any content-type set in `requestMutation`, which overrides
- this parameter, which overrides
- any content-type set with `Configuration.headers`.
- Parameter requestMutation: Allows you to override details fo the HTTP request before it is sent.
See `request(_:requestMutation:)`.
*/
public func request(
_ method: RequestMethod,
Expand All @@ -25,9 +34,8 @@ public extension Resource
{
underlyingRequest in

underlyingRequest.addValue(contentType, forHTTPHeaderField: "Content-Type")
underlyingRequest.setValue(contentType, forHTTPHeaderField: "Content-Type")
Copy link

Choose a reason for hiding this comment

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

It took me a while digging into Siesta 1.4 to understand what happened to my failing request.
I arrived here @pcantrell and I think this will break quite some code around.

At some point in my app I have a Resource R1 I need to load using a Resource R2.
R1.load(using: R2.withParam("someParam", value).request(.post, text: "SomeText") )
Doing this the request contentType param has its default "text/plain" value

The change-set here is going to overwrite completely the contenType stored by my Service Resource Configuration.
I don't think this is a wanted behaviour. Isn't it?

Copy link

Choose a reason for hiding this comment

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

I guess this can be a viable solution. I Can't produce a fully tested PR due to my limited time. Sorry about it.

if underlyingRequest.value(forHTTPHeaderField: "Content-Type")?.contains(contentType) == false {
    underlyingRequest.addValue(contentType, forHTTPHeaderField: "Content-Type")
}

underlyingRequest.httpBody = data

requestMutation(&underlyingRequest)
}
}
Expand Down
31 changes: 31 additions & 0 deletions Tests/Functional/RequestSpec.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,37 @@ class RequestSpec: ResourceSpecBase
}
}
}

it("overrides any Content-Type set in configuration headers")
{
service().configure { $0.headers["Content-Type"] = "frotzle/ooglatz" }
_ = stubRequest(resource, "POST")
.withHeader("Content-Type", "application/json")
awaitNewData(resource().request(.post, json: ["foo": "bar"]))
}

it("lets ad hoc request mutation override the Content-Type")
{
_ = stubRequest(resource, "POST")
.withHeader("Content-Type", "person/json")
let req = resource().request(.post, json: ["foo": "bar"])
{ $0.setValue("person/json", forHTTPHeaderField: "Content-Type") }
awaitNewData(req)
}

it("lets configured mutators override the Content-Type")
{
service().configure
{
$0.mutateRequests
{ $0.setValue("argonaut/json", forHTTPHeaderField: "Content-Type") } // This one wins, even though...
}

_ = stubRequest(resource, "POST").withHeader("Content-Type", "argonaut/json")
let req = resource().request(.post, json: ["foo": "bar"]) // ...request(json:) sets it to "application/json"...
{ $0.setValue("person/json", forHTTPHeaderField: "Content-Type") } // ...and ad hoc mutation overrides that.
awaitNewData(req)
}
}

describe("chained()")
Expand Down