Skip to content
This repository was archived by the owner on Dec 12, 2019. It is now read-only.

Add ability to use auth header as part of file name #38

Open
designatednerd wants to merge 7 commits intovokal:masterfrom
designatednerd:master
Open

Add ability to use auth header as part of file name #38
designatednerd wants to merge 7 commits intovokal:masterfrom
designatednerd:master

Conversation

@designatednerd
Copy link
Contributor

Allows verification of auth header if the consumer so desires.

if (authHeader) {
NSString *encodedHeader = [authHeader stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding];
NSString *authString = [NSString stringWithFormat:@"&authorization=%@", encodedHeader];
[resourceName appendString:authString];
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vokal-isaac Pretty sure there's more I should be doing here in terms of sha256'ing this to work better with the bEncoding stuff, but I'm not 100% clear on what I need to do here to make it work. Any direction would be most appreciated.

@designatednerd
Copy link
Contributor Author

Build errored, but tests are passing locally, fwiw. I believe if someone kicks the build it'll pass.

@vokal-isaac
Copy link
Contributor

Since this affects the names of the mock data files, the change should be documented in MockDataNaming.md. I think the way you're doing it, a request that had an authorization parameter in the query string could end up with the same resource name as one that didn't, but had the auth header. My instinct is that if headers are to affect resource names, it should be a more generic/flexible set of headers and it'd need to be a separate part of the resource name to avoid collision with the query string or request body.

@designatednerd
Copy link
Contributor Author

Ah, good catch on the mock naming file. Once we hammer this out I'll update that as well.

I'd like to restrict which headers actually contribute to the filename, so that not every single header winds up as part of the filename for files where we want this. To narrow that down, how about changing the BOOL to an array of Strings which are HTTP header fields? Then, we can use the field names as keys and values as...values.

Trying to think of a good way to split out the headers from the query params and the JSON keys/values, and coming up a little blank. Any suggestions?

@vokal-isaac
Copy link
Contributor

The current naming is something like

(method)|(path)
(method)|(path)?(query)
(method)|(path)|(body)
(method)|(path)?(query)|(body)

I'm not sure there's a way to add a (headers) section to that without breaking compatibility. My offhand inclination is something like

(method)|(path)|(headers)
(method)|(path)?(query)|(headers)
(method)|(path)|(headers)|(body)
(method)|(path)?(query)|(headers)|(body)

(Having the (headers) section always present avoids ambiguity about whether one extra section is headers or body.)

@designatednerd
Copy link
Contributor Author

@vokal-isaac Take a look at my proposed change here - This adds an additional pipe-separated section specifically for any headers if headers are expected. If you think it'd be useful from a readability I could also add some kind of indicator of whether a given section is the headers section (maybe |headers-%@) - I'd add one for the body but that would break compatibility with existing files.

{
[self verifyRequestWithURLString:@"http://example.com/DoesntExist.html"
additionalHeaders:nil
bodyData:nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to have a convenience wrapper method like

- (void)verifyRequestWithURLString:(NSString *)urlString
                        completion:(void (^)(NSData *data, NSHTTPURLResponse *response, NSError *error))completion
{
     [self verifyRequestWithURLString:urlString
                    additionalHeaders:nil
                             bodyData:nil
                            completion:completion];
}

to avoid adding these two lines in a bunch of places?

@vokal-isaac
Copy link
Contributor

Still need to update MockDataNaming.md.

@designatednerd
Copy link
Contributor Author

One day I'll come back to this, I swear. It's literally been on my todo list for months.

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.

2 participants