Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
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: 8 additions & 4 deletions Apollo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@
9FF90A711DDDEB420034C3B6 /* ReadFieldValueTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FF90A6B1DDDEB420034C3B6 /* ReadFieldValueTests.swift */; };
9FF90A731DDDEB420034C3B6 /* ParseQueryResponseTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FF90A6C1DDDEB420034C3B6 /* ParseQueryResponseTests.swift */; };
C304EBD722DDC8E600748F72 /* a.txt in Resources */ = {isa = PBXBuildFile; fileRef = C304EBD322DDC7B200748F72 /* a.txt */; };
C338DF1722DD9DE9006AF33E /* MultipartFormDataTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C338DF1622DD9DE9006AF33E /* MultipartFormDataTests.swift */; };
C3279FC72345234D00224790 /* TestCustomRequestCreator.swift in Sources */ = {isa = PBXBuildFile; fileRef = C3279FC52345233000224790 /* TestCustomRequestCreator.swift */; };
C338DF1722DD9DE9006AF33E /* RequestCreatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C338DF1622DD9DE9006AF33E /* RequestCreatorTests.swift */; };
C35D43C222DDD4AC00BCBABE /* b.txt in Resources */ = {isa = PBXBuildFile; fileRef = C35D43BE22DDD3C100BCBABE /* b.txt */; };
C35D43C322DDD4AF00BCBABE /* c.txt in Resources */ = {isa = PBXBuildFile; fileRef = C35D43BF22DDD3C100BCBABE /* c.txt */; };
C35D43C422DDD4D100BCBABE /* b.txt in Resources */ = {isa = PBXBuildFile; fileRef = C35D43BE22DDD3C100BCBABE /* b.txt */; };
Expand Down Expand Up @@ -381,7 +382,8 @@
9FF90A6B1DDDEB420034C3B6 /* ReadFieldValueTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ReadFieldValueTests.swift; sourceTree = "<group>"; };
9FF90A6C1DDDEB420034C3B6 /* ParseQueryResponseTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ParseQueryResponseTests.swift; sourceTree = "<group>"; };
C304EBD322DDC7B200748F72 /* a.txt */ = {isa = PBXFileReference; lastKnownFileType = text; path = a.txt; sourceTree = "<group>"; };
C338DF1622DD9DE9006AF33E /* MultipartFormDataTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MultipartFormDataTests.swift; sourceTree = "<group>"; };
C3279FC52345233000224790 /* TestCustomRequestCreator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestCustomRequestCreator.swift; sourceTree = "<group>"; };
C338DF1622DD9DE9006AF33E /* RequestCreatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RequestCreatorTests.swift; sourceTree = "<group>"; };
C35D43BE22DDD3C100BCBABE /* b.txt */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = b.txt; sourceTree = "<group>"; };
C35D43BF22DDD3C100BCBABE /* c.txt */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = c.txt; sourceTree = "<group>"; };
C377CCA822D798BD00572E03 /* GraphQLFile.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GraphQLFile.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -664,7 +666,7 @@
9BF1A94C22CA54F9005292C2 /* HTTPTransportTests.swift */,
9FF90A6A1DDDEB420034C3B6 /* InputValueEncodingTests.swift */,
E86D8E03214B32DA0028EFE1 /* JSONTests.swift */,
C338DF1622DD9DE9006AF33E /* MultipartFormDataTests.swift */,
C338DF1622DD9DE9006AF33E /* RequestCreatorTests.swift */,
9F91CF8E1F6C0DB2008DD0BE /* MutatingResultsTests.swift */,
9F295E301E27534800A24949 /* NormalizeQueryResults.swift */,
9FF90A6C1DDDEB420034C3B6 /* ParseQueryResponseTests.swift */,
Expand All @@ -675,6 +677,7 @@
C304EBD322DDC7B200748F72 /* a.txt */,
C35D43BE22DDD3C100BCBABE /* b.txt */,
C35D43BF22DDD3C100BCBABE /* c.txt */,
C3279FC52345233000224790 /* TestCustomRequestCreator.swift */,
);
path = ApolloTests;
sourceTree = "<group>";
Expand Down Expand Up @@ -1251,13 +1254,14 @@
9F91CF8F1F6C0DB2008DD0BE /* MutatingResultsTests.swift in Sources */,
9F19D8461EED8D3B00C57247 /* ResultOrPromiseTests.swift in Sources */,
9F533AB31E6C4A4200CBE097 /* BatchedLoadTests.swift in Sources */,
C3279FC72345234D00224790 /* TestCustomRequestCreator.swift in Sources */,
9B95EDC022CAA0B000702BB2 /* GETTransformerTests.swift in Sources */,
9FF90A6F1DDDEB420034C3B6 /* InputValueEncodingTests.swift in Sources */,
9FE1C6E71E634C8D00C02284 /* PromiseTests.swift in Sources */,
9FADC8541E6B86D900C677E6 /* DataLoaderTests.swift in Sources */,
E86D8E05214B32FD0028EFE1 /* JSONTests.swift in Sources */,
9F8622FA1EC2117C00C38162 /* FragmentConstructionAndConversionTests.swift in Sources */,
C338DF1722DD9DE9006AF33E /* MultipartFormDataTests.swift in Sources */,
C338DF1722DD9DE9006AF33E /* RequestCreatorTests.swift in Sources */,
F16D083C21EF6F7300C458B8 /* QueryFromJSONBuildingTests.swift in Sources */,
9FF90A711DDDEB420034C3B6 /* ReadFieldValueTests.swift in Sources */,
9F295E311E27534800A24949 /* NormalizeQueryResults.swift in Sources */,
Expand Down
3 changes: 2 additions & 1 deletion Sources/Apollo/HTTPNetworkTransport.swift
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,8 @@ public class HTTPNetworkTransport {
for: operation,
files: files,
sendOperationIdentifiers: self.sendOperationIdentifiers,
serializationFormat: self.serializationFormat)
serializationFormat: self.serializationFormat,
manualBoundary: nil)

request.setValue("multipart/form-data; boundary=\(formData.boundary)", forHTTPHeaderField: "Content-Type")
request.httpBody = try formData.encode()
Expand Down
7 changes: 6 additions & 1 deletion Sources/Apollo/MultipartFormData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ public class MultipartFormData {
self.init(boundary: "apollo-ios.boundary.\(UUID().uuidString)")
}

func appendPart(string: String, name: String) throws {
/// Appends the passed-in string as a part of the body.
///
/// - Parameters:
/// - string: The string to append
/// - name: The name of the part to pass along to the server
public func appendPart(string: String, name: String) throws {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@designatednerd this was only internal. The TestCustomRequestCreator used it and I know I use in my project too.
https://github.com/apollographql/apollo-ios/pull/804/files#diff-b1b6d9ae6149908bf9768bbc4bc4b3b0R50

self.appendPart(data: try self.encode(string: string),
name: name,
contentType: nil)
Expand Down
4 changes: 2 additions & 2 deletions Sources/Apollo/RequestCreator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ extension RequestCreator {
/// - operation: The operation to use
/// - sendOperationIdentifiers: Whether or not to send operation identifiers. Defaults to false.
/// - Returns: The created `GraphQLMap`
public func requestBody<Operation: GraphQLOperation>(for operation: Operation, sendOperationIdentifiers: Bool = false) -> GraphQLMap {
public func requestBody<Operation: GraphQLOperation>(for operation: Operation, sendOperationIdentifiers: Bool) -> GraphQLMap {
var body: GraphQLMap = [
"variables": operation.variables,
"operationName": operation.operationName,
Expand Down Expand Up @@ -66,7 +66,7 @@ extension RequestCreator {
files: [GraphQLFile],
sendOperationIdentifiers: Bool,
serializationFormat: JSONSerializationFormat.Type,
manualBoundary: String? = nil) throws -> MultipartFormData {
manualBoundary: String?) throws -> MultipartFormData {
let formData: MultipartFormData

if let boundary = manualBoundary {
Expand Down
6 changes: 3 additions & 3 deletions Tests/ApolloTests/GETTransformerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class GETTransformerTests: XCTestCase {

func testEncodingQueryWithSingleParameter() {
let operation = HeroNameQuery(episode: .empire)
let body = requestCreator.requestBody(for: operation)
let body = requestCreator.requestBody(for: operation, sendOperationIdentifiers: false)

let transformer = GraphQLGETTransformer(body: body, url: self.url)

Expand All @@ -27,7 +27,7 @@ class GETTransformerTests: XCTestCase {

func testEncodingQueryWithMoreThanOneParameterIncludingNonHashableValue() {
let operation = HeroNameTypeSpecificConditionalInclusionQuery(episode: .jedi, includeName: true)
let body = requestCreator.requestBody(for: operation)
let body = requestCreator.requestBody(for: operation, sendOperationIdentifiers: false)

let transformer = GraphQLGETTransformer(body: body, url: self.url)

Expand Down Expand Up @@ -90,7 +90,7 @@ class GETTransformerTests: XCTestCase {

func testEncodingQueryWithNullDefaultParameter() {
let operation = HeroNameQuery()
let body = requestCreator.requestBody(for: operation)
let body = requestCreator.requestBody(for: operation, sendOperationIdentifiers: false)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably worth adding one test to this file where sendOperationIdentifers is true just to make sure that's actually including the operation identifier

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Well, the default implementation here was actually false.
Maybe that is for another PR?
I can open an issue to you can track it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just not to add something that is not related to this PR and RequestCreator? 😁

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah I'll add it in a follow-up PR - you don't need to open an issue for it, I'll leave this comment open


let transformer = GraphQLGETTransformer(body: body, url: self.url)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ import XCTest
@testable import Apollo
import StarWarsAPI

class MultipartFormDataTests: XCTestCase {
private let requestCreator = ApolloRequestCreator()
class RequestCreatorTests: XCTestCase {
private let customRequestCreator = TestCustomRequestCreator()
private let apolloRequestCreator = ApolloRequestCreator()

private func checkString(_ string: String,
includes expectedString: String,
Expand Down Expand Up @@ -158,15 +159,15 @@ Charlie file content.
XCTAssertEqual(stringToCompare, expectedString)
}

func testSingleFileWithRequestCreator() throws {
func testSingleFileWithApolloRequestCreator() throws {
let alphaFileUrl = self.fileURLForFile(named: "a", extension: "txt")

let alphaFile = GraphQLFile(fieldName: "upload",
originalName: "a.txt",
mimeType: "text/plain",
fileURL: alphaFileUrl)

let data = try requestCreator.requestMultipartFormData(
let data = try apolloRequestCreator.requestMultipartFormData(
for: HeroNameQuery(),
files: [alphaFile!],
sendOperationIdentifiers: false,
Expand Down Expand Up @@ -213,8 +214,8 @@ Alpha file content.
self.checkString(stringToCompare, includes: expectedEndString)
}
}
func testMultipleFilesWithRequestCreator() throws {

func testMultipleFilesWithApolloRequestCreator() throws {
let alphaFileURL = self.fileURLForFile(named: "a", extension: "txt")
let alphaFile = GraphQLFile(fieldName: "uploads",
originalName: "a.txt",
Expand All @@ -228,7 +229,7 @@ Alpha file content.
fileURL: betaFileURL)!


let data = try requestCreator.requestMultipartFormData(
let data = try apolloRequestCreator.requestMultipartFormData(
for: HeroNameQuery(),
files: [alphaFile, betaFile],
sendOperationIdentifiers: false,
Expand Down Expand Up @@ -283,4 +284,82 @@ Bravo file content.
self.checkString(stringToCompare, includes: endString)
}
}

func testRequestBodyWithApolloRequestCreator() {
let query = HeroNameQuery()
let req = apolloRequestCreator.requestBody(for: query, sendOperationIdentifiers: false)

XCTAssertEqual(query.queryDocument, req["query"] as? String)
}

// MARK: - Custom request creator tests

func testSingleFileWithCustomRequestCreator() throws {
let alphaFileUrl = self.fileURLForFile(named: "a", extension: "txt")

let alphaFile = GraphQLFile(fieldName: "upload",
originalName: "a.txt",
mimeType: "text/plain",
fileURL: alphaFileUrl)

let data = try customRequestCreator.requestMultipartFormData(
for: HeroNameQuery(),
files: [alphaFile!],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably worth guard let ing this on creation, since a force-unwrap failing in a test causes the whole test suite to crash rather than failing the single test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is just copy/paste from other code made by you 🙈
Should I remove all force unwrap in this class?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤦‍♀️Nah I'll fix it in a follow-up PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sowwy 🙈

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Haha, I'm the moron who did it first, no worries

sendOperationIdentifiers: false,
serializationFormat: JSONSerializationFormat.self,
manualBoundary: "TEST.BOUNDARY"
)

let stringToCompare = try self.string(from: data)

print(stringToCompare)

if #available(iOS 11, macOS 13, tvOS 11, watchOS 4, *) {
let expectedString = """
--TEST.BOUNDARY
Content-Disposition: form-data; name="test_variables"

{"episode":null}
--TEST.BOUNDARY
Content-Disposition: form-data; name="test_query"

query HeroName($episode: Episode) { hero(episode: $episode) { __typename name } }
--TEST.BOUNDARY
Content-Disposition: form-data; name="test_operationName"

HeroName
--TEST.BOUNDARY
Content-Disposition: form-data; name="upload"; filename="a.txt"
Content-Type: text/plain

Alpha file content.

--TEST.BOUNDARY--
"""
XCTAssertEqual(stringToCompare, expectedString)
} else {
// Operation parameters may be in weird order, so let's at least check that the files and single parameter got encoded properly.
let expectedEndString = """
--TEST.BOUNDARY
Content-Disposition: form-data; name="map"

{"0":["variables.upload"]}
--TEST.BOUNDARY
Content-Disposition: form-data; name="0"; filename="a.txt"
Content-Type: text/plain

Alpha file content.

--TEST.BOUNDARY--
"""
self.checkString(stringToCompare, includes: expectedEndString)
}
}

func testRequestBodyWithCustomRequestCreator() {
let query = HeroNameQuery()
let req = customRequestCreator.requestBody(for: query, sendOperationIdentifiers: false)

XCTAssertEqual(query.queryDocument, req["test_query"] as? String)
}
}
62 changes: 62 additions & 0 deletions Tests/ApolloTests/TestCustomRequestCreator.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
//
// TestCustomRequestCreator.swift
// Apollo
//
// Created by Kim de Vos on 02/10/2019.
// Copyright © 2019 Apollo GraphQL. All rights reserved.
//

import Apollo

struct TestCustomRequestCreator: RequestCreator {
public func requestBody<Operation: GraphQLOperation>(for operation: Operation, sendOperationIdentifiers: Bool) -> GraphQLMap {
var body: GraphQLMap = [
"test_variables": operation.variables,
"test_operationName": operation.operationName,
]

if sendOperationIdentifiers {
guard let operationIdentifier = operation.operationIdentifier else {
preconditionFailure("To send operation identifiers, Apollo types must be generated with operationIdentifiers")
}

body["test_id"] = operationIdentifier
} else {
body["test_query"] = operation.queryDocument
}

return body
}

public func requestMultipartFormData<Operation: GraphQLOperation>(for operation: Operation,
files: [GraphQLFile],
sendOperationIdentifiers: Bool,
serializationFormat: JSONSerializationFormat.Type,
manualBoundary: String?) throws -> MultipartFormData {
let formData: MultipartFormData

if let boundary = manualBoundary {
formData = MultipartFormData(boundary: boundary)
} else {
formData = MultipartFormData()
}

let fields = requestBody(for: operation, sendOperationIdentifiers: false)
for (name, data) in fields {
if let data = data as? GraphQLMap {
let data = try serializationFormat.serialize(value: data)
formData.appendPart(data: data, name: name)
} else if let data = data as? String {
try formData.appendPart(string: data, name: name)
} else {
try formData.appendPart(string: data.debugDescription, name: name)
}
}

files.forEach {
formData.appendPart(inputStream: $0.inputStream, contentLength: $0.contentLength, name: $0.fieldName, contentType: $0.mimeType, filename: $0.originalName)
}

return formData
}
}