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

Expose acceptableImageContentTypes #453

Merged

Conversation

lickel
Copy link
Contributor

@lickel lickel commented Jan 27, 2022

Goals ⚽

Expose ImageResponseSerializer.acceptableImageContentTypes for read-only consumption by API consumers.

Implementation Details 🚧

I have had a few instances where I need to express valid content types.
The most recent use case was within a SwiftUI view where I wanted to download an image directly.

Testing Details 🔍

n/a

I have had reasons to access this list directly.
Exposing a readonly getter should be safe for any use cases.
@lickel lickel force-pushed the expose-acceptable-image-content-types branch from 99fc448 to 70cdeb7 Compare January 27, 2022 18:02
@jshier
Copy link
Contributor

jshier commented Jan 27, 2022

Simple enough, but can you clarify why you would ever need to access this value directly? If you need to download an image, I'd suggest wrapping an ImageDownloader into a SwiftUI view so you can take advantage of all of the AlamofireImage features.

@lickel
Copy link
Contributor Author

lickel commented Jan 27, 2022

I am using ImageDownloader. My issue is I want the same URLRequest that I would get from UIImageView.setImage(withURL...)

Those conversions are all in private extension functions with the form:

    private func urlRequest(with url: URL) -> URLRequest {
        var urlRequest = URLRequest(url: url)

        for mimeType in ImageResponseSerializer.acceptableImageContentTypes.sorted() {
            urlRequest.addValue(mimeType, forHTTPHeaderField: "Accept")
        }

        return urlRequest
    }

@lickel
Copy link
Contributor Author

lickel commented Jan 27, 2022

The other alternative would be to expose that func urlRequest(with url: URL) behavior publicly, e.g on ImageDownloader

@jshier
Copy link
Contributor

jshier commented Jan 27, 2022

Makes sense. This is probably an okay change, but I'll think about it more.

@lickel
Copy link
Contributor Author

lickel commented Feb 1, 2022

@jshier have you had a chance to give this more thought?

@jshier
Copy link
Contributor

jshier commented Feb 1, 2022

Makes sense, I just need to do some other updates before I can test and merge.

@lickel
Copy link
Contributor Author

lickel commented Jun 2, 2022

Just following up; do you think you'd be willing to merge this in?

@jshier jshier merged commit 8f4d7a7 into Alamofire:master May 20, 2023
@jshier jshier added this to the 4.3.0 milestone Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants