Skip to content
New issue

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

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

Already on GitHub? # to your account

Feature - Request Image Content Types #33

Merged
merged 1 commit into from
Oct 14, 2015
Merged
Show file tree
Hide file tree
Changes from all 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
36 changes: 22 additions & 14 deletions Source/Request+AlamofireImage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,27 @@ import Cocoa
#endif

extension Request {
static var acceptableImageContentTypes: Set<String> = [
"image/tiff",
"image/jpeg",
"image/gif",
"image/png",
"image/ico",
"image/x-icon",
"image/bmp",
"image/x-bmp",
"image/x-xbitmap",
"image/x-win-bitmap"
]

/**
Adds the content types specified to the list of acceptable images content types for validation.

- parameter contentTypes: The additional content types.
*/
public class func addAcceptableImageContentTypes(contentTypes: Set<String>) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to have a remove?

Copy link
Contributor

Choose a reason for hiding this comment

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

Only scenario I think of for having a remove is a user who wants an invalid content type error for some reason. I can't think of a valid use, though more and more I'm coming to realize I have yet to experience the full range of terrible backend APIs. 👀

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question @kcharwood. I think @jshier hit the only use case I can think of for requiring a removal, and I don't think it's a very realistic case. I could certainly add the API for completeness, but I think I'd prefer to leave it out now, and add it when we have a use case actually come up where it makes sense to add it.

Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to me, just wanted to pose the question.

Request.acceptableImageContentTypes.unionInPlace(contentTypes)
}

// MARK: - iOS and watchOS

Expand Down Expand Up @@ -195,20 +216,7 @@ extension Request {
return true
}

let acceptableContentTypes: Set<String> = [
"image/tiff",
"image/jpeg",
"image/gif",
"image/png",
"image/ico",
"image/x-icon",
"image/bmp",
"image/x-bmp",
"image/x-xbitmap",
"image/x-win-bitmap"
]

if let mimeType = response?.MIMEType where acceptableContentTypes.contains(mimeType) {
if let mimeType = response?.MIMEType where Request.acceptableImageContentTypes.contains(mimeType) {
return true
}

Expand Down
31 changes: 30 additions & 1 deletion Tests/RequestTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,40 @@
// THE SOFTWARE.

import Alamofire
import AlamofireImage
@testable import AlamofireImage
import Foundation
import XCTest

class RequestTestCase: BaseTestCase {
var acceptableImageContentTypes: Set<String>!

// MARK: - Setup and Teardown

override func setUp() {
super.setUp()
acceptableImageContentTypes = Request.acceptableImageContentTypes
}

override func tearDown() {
super.tearDown()
Request.acceptableImageContentTypes = acceptableImageContentTypes
}

// MARK: - Image Content Type Tests

func testThatAddingAcceptableImageContentTypesInsertsThemIntoTheGlobalList() {
// Given
let contentTypes: Set<String> = ["image/jpg", "binary/octet-stream"]

// When
let beforeCount = Request.acceptableImageContentTypes.count
Request.addAcceptableImageContentTypes(contentTypes)
let afterCount = Request.acceptableImageContentTypes.count

// Then
XCTAssertEqual(beforeCount, 10, "before count should be 10")
XCTAssertEqual(afterCount, 12, "after count should be 12")
}

// MARK: - Image Serialization Tests

Expand Down