Skip to content
This repository has been archived by the owner on Jan 17, 2023. It is now read-only.

Added in "image/jpg" acceptable content type for Image Requests #199

Closed
wants to merge 1 commit into from
Closed

Added in "image/jpg" acceptable content type for Image Requests #199

wants to merge 1 commit into from

Conversation

kcharwood
Copy link
Contributor

I ran into an API that was returning images as "image/jpg", which I
noticed was not included as a default content type for the image
request. This change fixes that.

I ran into an API that was returning images as "image/jpg", which I
noticed was not included as a default content type for the image
request.  This change fixes that.
@mattt
Copy link
Contributor

mattt commented Feb 6, 2012

Bah, that's not the right Mime type at all :(
What API is this, out of curiosity?

This actually brings up a weakness in the framework that I've come across on a few recent occasions—the amount of effort needed to workaround the built-in strictness to content-types. There was someone on Stack Overflow recently that had an API returning JSON as an incorrect mime type, and I was frustrated by how complicated the answer my answer was for that.

And it's not just incorrect mime types—Amazon and GitHub, for example, serve specifally-vendor-prefix-types, which would result in an error with AFNetworking. Expanding the acceptable content types is a good band-aid, but it's not a great long-term solution, I'm afraid...

Do you (or does anyone else) have any strong opinions on the matter? Any ideas on how this should be addressed, if at all?

@kcharwood
Copy link
Contributor Author

That's a great point. I didn't even double check to see if that was valid Mime type - just assumed it.

It's API in development right now for a client, so I'll actually just get them to change it on their end.

However, it does present a problem since I was using the UIImageView category to load images for a TableView, and as far as I know the only way I could fix this issue here is by changing AFImageRequestOperation directly and adding it there. (which I prefer to not do).

@mattt
Copy link
Contributor

mattt commented Feb 14, 2012

What do you think about using class methods to define acceptable content types and path extensions? e.g.:

[AFImageRequestOperation setAcceptableContentTypes:[NSSet setWithObjects:@"image/jpg", ...]];

I worry about the possibility of declarations overriding each other like this, so maybe only support adding of acceptableContentTypes?

[AFImageRequestOperation addAcceptableContentType:@"image/jpg"];

@kcharwood
Copy link
Contributor Author

I think I like the second approach better than the first. I'm having a hardtime why people wouldn't want the default set.

@nickpack
Copy link

nickpack commented Mar 2, 2012

+1 for the second option, as long as I can pass an array of types in rather than one by one ;)

@kcharwood
Copy link
Contributor Author

To piggy back on my comment in #228, this issue might just be related to AFImageRequestOperation and UIImageView+AFNetworking. Since users are free to subclass and register/use any of the AFHTTPRequestOperations, it should be trivial to add additional mime types to those by using a subclass. This allows the main framework to only support valid mime types out of the box, and simple subclassing for anyone who needs something custom.

However, there is no patch through to the UIImageView+AFNetworking category, so the user is forced to deal with AFImageRequestOperation. Perhaps the answer is to create the ability for a user to register a class with a category, and use that instead. For example, we could have [UIImageView registerImageRequestOperationClass:[KHImageRequestOperation class]]. This would allow me to subclass AFImageRequestOperation, and force the category to use that instead.

I may take some time this weekend and play around with this idea.

@kcharwood
Copy link
Contributor Author

Just noticed that @mattt has an experimental branch going looking to solve this problem. @mattt, if you feel good about that implementation, I won't bother going down this route.

@mattt
Copy link
Contributor

mattt commented Mar 9, 2012

@kcharwood Yeah, I'm actually quite happy with the approach described in #228. Thanks again for your thoughts on this—I'm excited to have this in the next release of AFNetworking.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants