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

Changes made to support AFNetworking 2.0 #55

Closed
wants to merge 6 commits into from

Conversation

mlwelles
Copy link

@mlwelles mlwelles commented Oct 8, 2013

Based on pull request #50 that's still pending from @nilsou. Found it needed a few more tweaks to get working properly.

@Zhukn1
Copy link

Zhukn1 commented Oct 17, 2013

Please accept this one.

@djmadcat
Copy link

This is like a game - support AFNetworking 2.0=))) Makes me laugh=)))))
Make your own repo and use it, because "nobody cares" (no time to fix). When maintainers update the library, you just fix your code and dependencies.

@Zhukn1
Copy link

Zhukn1 commented Oct 17, 2013

Of course I already did that, just pointing out to the maintainers that this pull request if worth accepting.

@djmadcat
Copy link

I think, that this library has one "fatal lack" - it can not be applied to two entities (AFHTTPSessionManager and AFHTTPRequestOperationManager) at once. It must be rewrited twice(!). Sooo... We must abstract from "request level" and make it as independent class.

@MattNewberry
Copy link

@mlwelles Can you update the podspec to require AFNetworking 2.0? It's linking against 1.x now

@lmcd
Copy link

lmcd commented Oct 30, 2013

Can we please accept one of these

@ynechaev
Copy link

bump

@dennisreimann
Copy link

I'm currently using the result of this pull request in a rather complex app and it seems good to go.

I had to change some minor things to make it work for my case, but I think they could be used in general, so here's something you might want to add/change:

  • Make - (void)setAuthorizationHeaderWithToken:(NSString *)token a public method
  • Correct the documentation for - (void)setAuthorizationHeaderWithCredential:(AFOAuthCredential *)credential which seems to have been c&p'ed
  • I might be wrong here, but imho the requestSerializer should not be altered in - (void)setAuthorizationHeaderWithToken:(NSString *)token ofType:(NSString *)type. The AFHTTPRequestSerializer is the default anyways and changing it here seems to be an unintended side effect.

@jstart
Copy link

jstart commented Dec 3, 2013

👍

@christianselig
Copy link

How do I use the custom podspec file to incorporate this pull into my project?

@jstart
Copy link

jstart commented Dec 3, 2013

pod 'AFOAuth2Client', :git => 'https://github.com/mlwelles/AFOAuth2Client'

@conradev
Copy link
Contributor

conradev commented Dec 4, 2013

This makes AFOAuth2Client compile against AFNetworking 2.0, but I feel like it requires additional changes to make it support AFNetworking 2.0 better.

For example: setting the bearer token as a header on the OAuth client itself is old behavior, and should not be used anymore. Instead, the OAuth client should be used to obtain a credential that is then used in a standard client. This is pretty easy to implement in AFNetworking 2.0, by writing an AFOAuth2RequestSerializer class that has a credential property and conforms to AFURLRequestSerialization:

- (NSURLRequest *)requestBySerializingRequest:(NSURLRequest *)request
                               withParameters:(NSDictionary *)parameters
                                        error:(NSError *__autoreleasing *)error {
    request = [super requestBySerializingRequest:request withParameters:parameters error:error];
    if (self.credential) {
        NSMutableURLRequest *mutableRequest = [request mutableCopy];
        if ([self.credential.tokenType compare:@"Bearer" options:NSCaseInsensitiveSearch] == NSOrderedSame) {
            [mutableRequest setValue:[@"Bearer " stringByAppendingString:self.credential.accessToken] forHTTPHeaderField:@"Authorization"];
        }
        request = [mutableRequest copy];
    }
    return request;
}

I do, however, agree that AFHTTPRequestOperationManager should be used because it has backwards compatibility with iOS 6, and NSURLConnection is not going anywhere.

@ollieatkinson
Copy link

Would be good to see this merged in so I dont have to point at a different repo

@mattt
Copy link
Contributor

mattt commented Nov 24, 2014

See #79 (comment).

@mattt mattt closed this Nov 24, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet