Externalize parsing of access token response #79
Conversation
| @@ -544,6 +662,8 @@ class HeimdallrSpec: QuickSpec { | |||
| }, withStubResponse: { request in | |||
| return OHHTTPStubsResponse(data: NSData(contentsOfFile: self.bundle.pathForResource("request-valid", ofType: "json")!)!, statusCode: 200, headers: [ "Content-Type": "application/json" ]) | |||
| }) | |||
|
|
|||
| accessTokenParser.reset() | |||
There was a problem hiding this comment.
Do we need the reset here? If I comment this line out, the test still succeeds.
There was a problem hiding this comment.
@bckr: the test succeeds because the first request (that returns an expired access token) already triggered the flag. The reset is there to ensure that the parser is called a second time with the second request. If this is too fragile I can change the flag for a counter.
There was a problem hiding this comment.
Yeah, I think the current way doesn't really verify that it is only called once for the initial request. I think a counter would work here. We could also think about introducing Dobby here for recording the invocations. What do you think @hffmnn?
Update: If you want to have a look at Dobby for simple invocation recording, feel free to add it as a dependency and try it out. I would also be fine with a counter 🙂
There was a problem hiding this comment.
Great! I'll update the PR ASAP!
|
@poislagarde Thank you very much for the quick pull request 🎉 At a first glance I would say that this seams like a good approach of externalising the access token parsing. I've made a few notes on code that might need a bit of fine tuning. Maybe one of the other team members want to have a look at this as well. |
|
Hey @bckr, I answered to your notes. Let me know if you want me to make those changes or not. Thanks! |
|
Sorry for the delay. I pushed and squashed the changes we talked about. Let me know if you need me to change something else. |
|
@poislagarde Nice work! I think we can merge this 🚀 |
Closes #76