Skip to content

Commit

Permalink
Fix the URL generation code so that it won't attach bogus parameters.
Browse files Browse the repository at this point in the history
Before the fix, for example, [LBFileTests testUpload] generated a path like:
"/containers/container1/files/f1.txt?container=container1&name=f1.txt"
whereas it should have just been "/containers/container1/files/f1.txt".

To fix the issue [SLRESTContract urlForMethod:parameters:] and [SLRESTContract
urlWithPattern:parameters:] now take NSMutableDictionary as the parameters argument
so that the key-value pairs which are consumed when generating the URL can be removed.
  • Loading branch information
hideya committed Jul 8, 2015
1 parent daa1dca commit abe0856
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 15 deletions.
5 changes: 3 additions & 2 deletions SLRemoting/SLRESTAdapter.m
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,14 @@ - (void)invokeStaticMethod:(NSString *)method

NSAssert(self.contract, @"Invalid contract.");

NSMutableDictionary *mutableParams = [NSMutableDictionary dictionaryWithDictionary:parameters];
NSString *verb = [self.contract verbForMethod:method];
NSString *path = [self.contract urlForMethod:method parameters:parameters];
NSString *path = [self.contract urlForMethod:method parameters:mutableParams];
BOOL multipart = [self.contract multipartForMethod:method];

[self requestWithPath:path
verb:verb
parameters:parameters
parameters:mutableParams
multipart:multipart
outputStream:outputStream
success:success
Expand Down
13 changes: 9 additions & 4 deletions SLRemoting/SLRESTContract.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,13 +120,14 @@ extern NSString *SLRESTContractDefaultVerb;
/**
* Resolves a specific method, replacing pattern fragments with the optional
* `parameters` as appropriate.
* Also removes consumed key-value pairs from the mutable `parameters`.
*
* @param method The method to resolve.
* @param parameters Pattern parameters. Can be `nil`.
* @return The complete, resolved URL.
*/
- (NSString *)urlForMethod:(NSString *)method
parameters:(NSDictionary *)parameters;
parameters:(NSMutableDictionary *)parameters;

/**
* Returns the HTTP verb for the given method string.
Expand Down Expand Up @@ -162,15 +163,19 @@ extern NSString *SLRESTContractDefaultVerb;
- (NSString *)patternForMethod:(NSString *)method;

/**
* Returns a rendered URL pattern using the parameters provided. For example,
* `@"/widgets/:id"` + `@{ @"id": "57", @"price": @"42.00" }` begets
* Returns a rendered URL pattern using the parameters provided.
* Also removes consumed key-value pairs from the mutable `parameters`.
* For example,
* `@"/widgets/:id"` + `@{ @"id": @"57", @"price": @"42.00" }` begets
* `@"/widgets/57"`.
* And `parameter` becomes `@{ @"price": @"42.00" }`
*
* @param pattern The pattern to render.
* @param parameters Values to render with.
* This is mutable and consumed key-value pairs get removed.
* @return The rendered URL.
*/
- (NSString *)urlWithPattern:(NSString *)pattern
parameters:(NSDictionary *)parameters;
parameters:(NSMutableDictionary *)parameters;

@end
17 changes: 11 additions & 6 deletions SLRemoting/SLRESTContract.m
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ - (void)addItemsFromContract:(SLRESTContract *)contract {
}

- (NSString *)urlForMethod:(NSString *)method
parameters:(NSDictionary *)parameters {
parameters:(NSMutableDictionary *)parameters {
NSParameterAssert(method);

NSString *pattern = [self patternForMethod:method];
Expand Down Expand Up @@ -173,18 +173,23 @@ - (NSString *)patternForMethod:(NSString *)method {
}

- (NSString *)urlWithPattern:(NSString *)pattern
parameters:(NSDictionary *)parameters {
parameters:(NSMutableDictionary *)parameters {
NSParameterAssert(pattern);

if (!parameters) {
return pattern;
}

NSString __block *url = pattern;
NSString *url = pattern;

[parameters enumerateKeysAndObjectsUsingBlock:^(id key, id obj, BOOL *stop) {
url = [url stringByReplacingOccurrencesOfString:[NSString stringWithFormat:@":%@", key] withString:[NSString stringWithFormat:@"%@", obj]];
}];
for (NSString *key in parameters.allKeys) { // create a copy of allKeys to mutate parameters
NSString *keyPattern = [NSString stringWithFormat:@":%@", key];
if ([url rangeOfString:keyPattern].location == NSNotFound) continue;

NSString *valueStr = [NSString stringWithFormat:@"%@", parameters[key]];
url = [url stringByReplacingOccurrencesOfString:keyPattern withString:valueStr];
[parameters removeObjectForKey:key];
}

return url;
}
Expand Down
16 changes: 13 additions & 3 deletions SLRemotingTests/SLRESTContractTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

@interface SLRESTContractTests() {
SLRESTAdapter *adapter;
SLRESTContract *contract;
SLRepository *TestClass;
}

Expand All @@ -42,6 +41,17 @@ - (void)tearDown {
[super tearDown];
}

- (void)testUrlWithPattern {
SLRESTContract *contract = [SLRESTContract contract];
NSMutableDictionary *parameters = [@{ @"id": @"57", @"price": @"42.00" } mutableCopy];

NSString *url = [contract urlWithPattern:@"/widgets/:id" parameters:parameters];

STAssertEqualObjects(url, @"/widgets/57", @"Invalid URL");
STAssertEqualObjects(parameters, [@{ @"price": @"42.00" } mutableCopy], @"Invalid parameters");
NSLog(@"\n***, %@, %@", url, parameters);
}

- (void)testAddItemsFromContract {
SLRESTContract *parent = [SLRESTContract contract];
SLRESTContract *child = [SLRESTContract contract];
Expand All @@ -51,9 +61,9 @@ - (void)testAddItemsFromContract {
[child addItem:[SLRESTContractItem itemWithPattern:@"/new/route" verb:@"POST"] forMethod:@"new.route"];

[parent addItemsFromContract:child];
STAssertTrue([[parent urlForMethod:@"test.route" parameters:@{}] isEqualToString:@"/test/route"], @"Wrong URL.");
STAssertTrue([[parent urlForMethod:@"test.route" parameters:nil] isEqualToString:@"/test/route"], @"Wrong URL.");
STAssertTrue([[parent verbForMethod:@"test.route"] isEqualToString:@"GET"], @"Wrong verb.");
STAssertTrue([[parent urlForMethod:@"new.route" parameters:@{}] isEqualToString:@"/new/route"], @"Wrong URL.");
STAssertTrue([[parent urlForMethod:@"new.route" parameters:nil] isEqualToString:@"/new/route"], @"Wrong URL.");
STAssertTrue([[parent verbForMethod:@"new.route"] isEqualToString:@"POST"], @"Wrong verb.");
}

Expand Down

0 comments on commit abe0856

Please sign in to comment.