Swift5: Binary / File type generated incorrectly as URL in responses

[ghost]

After some investigation, the test issue when changing the type binary from URL to Data, is related to the code in ModelUtils line 659.

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/DefaultCodegen.java

Lines 2059 to 2062 in 3894aa4

	} else if (ModelUtils.isFileSchema(schema)) {
	return "file";
	} else if (ModelUtils.isBinarySchema(schema)) {
	return SchemaTypeUtil.BINARY_FORMAT;

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/utils/ModelUtils.java

Lines 643 to 660 in 3894aa4

	public static boolean isBinarySchema(Schema schema) {
	if (schema instanceof BinarySchema) {
	return true;
	}
	if (SchemaTypeUtil.STRING_TYPE.equals(schema.getType())
	&& SchemaTypeUtil.BINARY_FORMAT.equals(schema.getFormat())) { // format: binary
	return true;
	}
	return false;
	}
	public static boolean isFileSchema(Schema schema) {
	if (schema instanceof FileSchema) {
	return true;
	}
	// file type in oas2 mapped to binary in oas3
	return isBinarySchema(schema);
	}

The binary type will be considered as file type.

To add some interesting bits to the discussion
The Kotlin Client generator, considers binary as BiteArray (or Data in Swift) and file as java.io.File (or URL in Swift)

openapi-generator/modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractKotlinCodegen.java

Lines 159 to 165 in 3894aa4

	typeMapping.put("file", "java.io.File");
	typeMapping.put("array", "kotlin.Array");
	typeMapping.put("list", "kotlin.collections.List");
	typeMapping.put("set", "kotlin.collections.Set");
	typeMapping.put("map", "kotlin.collections.Map");
	typeMapping.put("object", "kotlin.Any");
	typeMapping.put("binary", "kotlin.ByteArray");

The Java Client generator, considers binary as byte[], and file as java.io.File.

Since this is a big change on how files/binary is treated in the Swift Generator that is also a breaking change, and doing things differently from the other generators worries me a bit, so I just would like to be sure that this is the right move, but let's continue this discussion on the new PR..

Originally posted by @4brunu in #9206 (comment)

[ghost]

@4brunu I've created an issue to discuss the best solution for this. Then we can create a PR based on this.

That is interesting.

I agree this is a breaking change, but nobody will use URL for binary responses in their app. It simply can't work. The URL object is completely useless currently. In our app we "fixed" it by the type-mapping during generation.

Or did I miss something and there is a working use-case for the current implementation?

CC @aymanbagabas

[4brunu]

@jenswet-el thanks for creating this issue.

I have a binary type in swagger file that I consume, that it's used for file upload, and it's working fine with the URL.
I just pass a URL of the file I want to upload, and it works.

"File": {
    "type": "string",
    "description": "File to upload",
    "format": "binary"
}

[ghost]

@4brunu ahhh, you use it as upload. I am talking about download. There it is useless according to our app devs.

/image:
    get:
      responses:
        '200':
          description: image in JPEG format
          content:
            image/jpeg:
              schema:
                type: string
                format: binary

Generates:

open class func getImage(apiResponseQueue: DispatchQueue = OpenAPIClientAPI.apiResponseQueue, completion: @escaping ((_ data: URL?, _ error: Error?) -> Void)) {
        getImageWithRequestBuilder().execute(apiResponseQueue) { result -> Void in
            switch result {
            case let .success(response):
                completion(response.body, nil)
            case let .failure(error):
                completion(nil, error)
            }
        }
    }

Which maps the binary body into the URL if I am correct?

I don't do Swift (I do the backend), but I got this feedback from our app devs.

[4brunu]

Oh, I see.
Recently the kotlin generator had the same issue that was solved in this PR #9284.
Maybe we need to do something similar.

[aymanbagabas]

I have a binary type in swagger file that I consume, that it's used for file upload, and it's working fine with the URL.
I just pass a URL of the file I want to upload, and it works.

Could you please give us a sample project for this? I never used URLSession nor AlamoFire, but writing back-end Swift in Vapor, a request's body at the end gets converted to Data so like for a upload request I would do this using this fork https://github.com/myhealthily/openapi-generator/tree/swift5-vapor

app.post() { request -> Response in
    let file = Data(base64Encoded: base64)!
    PetAPI.uploadFile(petId: 99, file: file).whenComplete { result in
        switch result {
        case .success(let uploadFile):
            switch uploadFile {
            case .http0(let value, _), .http200(let value, _):
                print(value.code, value.type, value.message)
            }
        case .failure(let error):
            print(error)
        }
    }
    return Response(status: .ok)
}

petstore.zip

[4brunu]

Sure, here you have a sample project that show's the API usage.

openapi-generator/samples/client/petstore/swift5/default/SwaggerClientTests/SwaggerClientTests/PetAPITests.swift

Lines 65 to 89 in 5d94628

	func test3UploadFile() {
	let expectation = self.expectation(description: "testUploadFile")
	let imageName = UUID().uuidString + ".png"
	guard
	let image = UIImage(color: .red, size: CGSize(width: 10, height: 10)),
	let imageURL = FileUtils.saveImage(imageName: imageName, image: image)
	else {
	fatalError()
	}
	PetAPI.uploadFile(petId: 1000, additionalMetadata: "additionalMetadata", file: imageURL) { (_, error) in
	guard error == nil else {
	FileUtils.deleteFile(fileURL: imageURL)
	XCTFail("error uploading file")
	return
	}
	FileUtils.deleteFile(fileURL: imageURL)
	expectation.fulfill()
	}
	self.waitForExpectations(timeout: testTimeout, handler: nil)
	}

And here you have the URLSession implementation for file upload

openapi-generator/samples/client/petstore/swift5/default/PetstoreClient/Classes/OpenAPIs/URLSessionImplementations.swift

Lines 424 to 566 in 5d94628

	private class FormDataEncoding: ParameterEncoding {
	let contentTypeForFormPart: (_ fileURL: URL) -> String?
	init(contentTypeForFormPart: @escaping (_ fileURL: URL) -> String?) {
	self.contentTypeForFormPart = contentTypeForFormPart
	}
	func encode(_ urlRequest: URLRequest, with parameters: [String: Any]?) throws -> URLRequest {
	var urlRequest = urlRequest
	guard let parameters = parameters, !parameters.isEmpty else {
	return urlRequest
	}
	let boundary = "Boundary-\(UUID().uuidString)"
	urlRequest.setValue("multipart/form-data; boundary=\(boundary)", forHTTPHeaderField: "Content-Type")
	for (key, value) in parameters {
	switch value {
	case let fileURL as URL:
	urlRequest = try configureFileUploadRequest(
	urlRequest: urlRequest,
	boundary: boundary,
	name: key,
	fileURL: fileURL
	)
	case let string as String:
	if let data = string.data(using: .utf8) {
	urlRequest = configureDataUploadRequest(
	urlRequest: urlRequest,
	boundary: boundary,
	name: key,
	data: data
	)
	}
	case let number as NSNumber:
	if let data = number.stringValue.data(using: .utf8) {
	urlRequest = configureDataUploadRequest(
	urlRequest: urlRequest,
	boundary: boundary,
	name: key,
	data: data
	)
	}
	default:
	fatalError("Unprocessable value \(value) with key \(key)")
	}
	}
	var body = urlRequest.httpBody.orEmpty
	body.append("\r\n--\(boundary)--\r\n")
	urlRequest.httpBody = body
	return urlRequest
	}
	private func configureFileUploadRequest(urlRequest: URLRequest, boundary: String, name: String, fileURL: URL) throws -> URLRequest {
	var urlRequest = urlRequest
	var body = urlRequest.httpBody.orEmpty
	let fileData = try Data(contentsOf: fileURL)
	let mimetype = contentTypeForFormPart(fileURL) ?? mimeType(for: fileURL)
	let fileName = fileURL.lastPathComponent
	// If we already added something then we need an additional newline.
	if body.count > 0 {
	body.append("\r\n")
	}
	// Value boundary.
	body.append("--\(boundary)\r\n")
	// Value headers.
	body.append("Content-Disposition: form-data; name=\"\(name)\"; filename=\"\(fileName)\"\r\n")
	body.append("Content-Type: \(mimetype)\r\n")
	// Separate headers and body.
	body.append("\r\n")
	// The value data.
	body.append(fileData)
	urlRequest.httpBody = body
	return urlRequest
	}
	private func configureDataUploadRequest(urlRequest: URLRequest, boundary: String, name: String, data: Data) -> URLRequest {
	var urlRequest = urlRequest
	var body = urlRequest.httpBody.orEmpty
	// If we already added something then we need an additional newline.
	if body.count > 0 {
	body.append("\r\n")
	}
	// Value boundary.
	body.append("--\(boundary)\r\n")
	// Value headers.
	body.append("Content-Disposition: form-data; name=\"\(name)\"\r\n")
	// Separate headers and body.
	body.append("\r\n")
	// The value data.
	body.append(data)
	urlRequest.httpBody = body
	return urlRequest
	}
	func mimeType(for url: URL) -> String {
	let pathExtension = url.pathExtension
	if let uti = UTTypeCreatePreferredIdentifierForTag(kUTTagClassFilenameExtension, pathExtension as NSString, nil)?.takeRetainedValue() {
	if let mimetype = UTTypeCopyPreferredTagWithClass(uti, kUTTagClassMIMEType)?.takeRetainedValue() {
	return mimetype as String
	}
	}
	return "application/octet-stream"
	}
	}

[aymanbagabas]

@4brunu URLSessionImplementations.swift translates the URLs into Data when constructing the Request. This doesn't mean that file and binary are equivalent to URL in Swift, instead, here URLSessionImplementations.swift and FileUtils.swift are handling the translation of URLs to Data. At the end, the data is grabbed via Data(contentsOf:) in let fileData = try Data(contentsOf: fileURL) and then used in body.append(fileData) the type here is Data not URL.

IMO, this should be the job of the user to grab the raw data from URLs and not the client library. We could have helpers to assist with that but not to make that the default behavior.

CC @jenswet-el @wing328

openapi-generator/samples/client/petstore/swift5/default/PetstoreClient/Classes/OpenAPIs/URLSessionImplementations.swift

Lines 491 to 524 in 5d94628

	private func configureFileUploadRequest(urlRequest: URLRequest, boundary: String, name: String, fileURL: URL) throws -> URLRequest {
	var urlRequest = urlRequest
	var body = urlRequest.httpBody.orEmpty
	let fileData = try Data(contentsOf: fileURL)
	let mimetype = contentTypeForFormPart(fileURL) ?? mimeType(for: fileURL)
	let fileName = fileURL.lastPathComponent
	// If we already added something then we need an additional newline.
	if body.count > 0 {
	body.append("\r\n")
	}
	// Value boundary.
	body.append("--\(boundary)\r\n")
	// Value headers.
	body.append("Content-Disposition: form-data; name=\"\(name)\"; filename=\"\(fileName)\"\r\n")
	body.append("Content-Type: \(mimetype)\r\n")
	// Separate headers and body.
	body.append("\r\n")
	// The value data.
	body.append(fileData)
	urlRequest.httpBody = body
	return urlRequest
	}

[4brunu]

URL is only kind of a "pointer" to the Data, so to send it to the server, it's necessary to convert the URL to Data.
Maybe this transition can be done with a flag to ensure backward compatible and in a future version, we can remove the old behaviour.
The only thing that is holding this back to me, is that the other generators, also use File instead of ByteArray, just like the Swift Generator does, so I want to have 100% sure that this is the right path.

[4brunu]

I have looked into multiple generators, but the only conclusion that I got is that there is no uniformity between them.

So maybe we can go forward with this change, and create a feature flag to avoid the breaking change.

what do you think of this?

c#

typeMapping.put("binary", "byte[]");
typeMapping.put("ByteArray", "byte[]");
typeMapping.put("file", "System.IO.Stream");

java

typeMapping.put("file", "File");

java play framework

typeMapping.put("file", "InputStream");

Ruby

typeMapping.put("file", "File");
typeMapping.put("binary", "String");
typeMapping.put("ByteArray", "String");

Kotlin

typeMapping.put("ByteArray", "kotlin.ByteArray");
typeMapping.put("file", "java.io.File");
typeMapping.put("binary", "kotlin.ByteArray");

Dart

typeMapping.put("file", "MultipartFile");
typeMapping.put("binary", "MultipartFile");
typeMapping.put("ByteArray", "String");

TypeScript

typeMapping.put("binary", "any");
typeMapping.put("File", "any");
typeMapping.put("file", "any");
typeMapping.put("ByteArray", "string");

[4brunu]

@aymanbagabas do you want to create a PR with this change? Or do you want me to do it?

[aymanbagabas]

@4brunu I've created #9419 and rebased #8515 on top of #9419

[4brunu]

After looking at this issue once more, I found the following code.

openapi-generator/samples/client/petstore/swift5/urlsessionLibrary/Sources/URLSessionImplementations.swift

Lines 196 to 229 in 163d13e

	case is URL.Type:
	do {
	guard error == nil else {
	throw DownloadException.responseFailed
	}
	guard let data = data else {
	throw DownloadException.responseDataMissing
	}
	let fileManager = FileManager.default
	let documentsDirectory = fileManager.urls(for: .documentDirectory, in: .userDomainMask)[0]
	let requestURL = try getURL(from: urlRequest)
	var requestPath = try getPath(from: requestURL)
	if let headerFileName = getFileName(fromContentDisposition: httpResponse.allHeaderFields["Content-Disposition"] as? String) {
	requestPath = requestPath.appending("/\(headerFileName)")
	}
	let filePath = documentsDirectory.appendingPathComponent(requestPath)
	let directoryPath = filePath.deletingLastPathComponent().path
	try fileManager.createDirectory(atPath: directoryPath, withIntermediateDirectories: true, attributes: nil)
	try data.write(to: filePath, options: .atomic)
	completion(.success(Response(response: httpResponse, body: filePath as? T)))
	} catch let requestParserError as DownloadException {
	completion(.failure(ErrorResponse.error(400, data, response, requestParserError)))
	} catch {
	completion(.failure(ErrorResponse.error(400, data, response, error)))
	}

Looking at this, the binary download should have always worked.
The url that is returned as a response, is a url to a local file with the binary content return by the server.
I don't have any endpoint to test this.
@aymanbagabas @jenswet-el Do you have any endpoint where you could test this?
Thanks

[aymanbagabas]

@aymanbagabas @jenswet-el Do you have any endpoint where you could test this?

You can use HTTP Cats API, https://http.cat/500.jpg for instance

[4brunu]

Do they have a swagger/openapi file?

[aymanbagabas]

Do they have a swagger/openapi file?

No, but it's a simple API. Here's one:

openapi: 3.0.0
info:
  title: HTTP Cats
  description: Cats
  version: 1.0.0
servers:
  - url: https://http.cat/
    description: Cats
paths:
  /{code}:
    get:
      summary: Get Cat Image
      parameters:
        - in: path
          name: code
          schema:
            type: integer
          required: true
          description: HTTP Code
      responses:
        '200':
          description: Cat image
          content:
            image/jpeg:
              schema:
                type: string
                format: binary

[4brunu]

Thanks.
After some tests, I confirm that the download of binary response was not working.
I opened the following PR to fix it #9595.

[4brunu]

Now that #9595 got merged, the type binary mapped to URL work with both upload and download.
When type binary is mapped to Data only the download works, the file upload doesn't work (I think looking at the code, but I haven't tested it.)
Yesterday I tried to fix the file upload when mapped to Data, but I faced some challenges, because there are some info that is harder to get from Data than from a URL.
For example:

• What should be the file name and file extension?
• It's harder to extract the mimeType from Data than from a URL. The only way that I found to do this was something like https://stackoverflow.com/a/32765708/976628 or https://github.com/sendyhalim/Swime but that way we would have a limited set of binary type support.

That being said, now that the binary mapped to URL works in all the cases, is the switching to Data mapping worth pursuing?

Also if there is any generator library, for example #9583, that needs the Data type mapping, that generator can always overwrite the type mapping.