Added isEnabled getters for OHHTTPStubs. #159
Conversation
Reflects current implementation
| @@ -149,9 +149,11 @@ +(void)removeAllStubs | |||
|
|
|||
| #pragma mark > Disabling & Re-Enabling stubs | |||
|
|
|||
| static BOOL currentEnabledState = NO; | |||
There was a problem hiding this comment.
global variables are not thread safe, so this is not an acceptable solution. You could instead add an atomic property on the shared instance for example.
There was a problem hiding this comment.
Thanks for the suggestion!
|
What's the CP issue/error message you're encountering? |
|
This is a thread about the issue. Supposedly fixed in the next beta. Definitely present in 0.39 CocoaPods/CocoaPods#4478 |
|
Oh I see. Never had that issue before, it passed when running OHHTTPStubs tests on CI before which runs the checks for each platform, so will take a look later 👍 (PS: a bit busy this week so I might not be able to test this PR until this weekend) |
|
No rush on this pull request. Just saw an opportunity to help and wanted to contribute. |
|
👍 |
|
There's not a ton of great documentation on @synchronized, seems it's slightly out of favor. I tried to follow your pattern in this implementation. Also open to using a NSLock or a mutex, if you'd prefer. Just figured I'd try out your original suggestion first. |
| -(BOOL)isEnabled | ||
| { | ||
| BOOL enabled = NO; | ||
| @synchronized(self) |
There was a problem hiding this comment.
Using self instead of _enabledStateNumber because that might change and cause multiple locks to be enabled for this property
|
Yeah, OHHTTPStubs starts having history, that code was written a while back when |
| @@ -44,6 +44,7 @@ @interface OHHTTPStubsProtocol : NSURLProtocol @end | |||
| @interface OHHTTPStubs() | |||
| + (instancetype)sharedInstance; | |||
| @property(atomic, copy) NSMutableArray* stubDescriptors; | |||
| @property(atomic, copy) NSNumber* enabledStateNumber; | |||
There was a problem hiding this comment.
Why not using a BOOL, as it will be initialized to YES at init time so you don't really need the YES/NO/nil tri-state anyway?
There was a problem hiding this comment.
Yes, that seems obvious now. I originally wrote it intending to do @synchronized(_enabledState) when I didn't fully understand the @synchronized() directive. Anyways, updated with another commit.
| { | ||
| NSMutableArray * urlProtocolClasses = [NSMutableArray arrayWithArray:sessionConfig.protocolClasses]; | ||
| Class protoCls = OHHTTPStubsProtocol.class; | ||
| if ([urlProtocolClasses containsObject:protoCls]) |
There was a problem hiding this comment.
Why not directly return [urlProtocolClasses containsObject:protoCls];?
|
Fixed! |
|
Awesome, thanks again for the PR 👌 |
Added isEnabled getters for OHHTTPStubs.
Reflects current implementation. This branch does not update the version or run
pod updatefor the examples. There is a minor bug in Cocoapods for building projects for different platforms. Not sure how you build your projects so I can add that to the branch when I know what to do. This is in response to #139