Skip to content

Commit

Permalink
Return 'null' on status 404 (#57)
Browse files Browse the repository at this point in the history 
* Add a null type to IRestResponse.result since it might be null

* Improve initialization of default result object

- Use const qualifiers
- Initialize 'result' attribute with 'null', so the code matches the documentation
- Use strict identity check for 'null' in test

* Adjust rest tests

* Rename variable in RestClient._processResponse()
  • Loading branch information
@andrew8er @bryanmacfarlane
andrew8er authored and bryanmacfarlane committed Jan 31, 2018
1 parent 56c385c commit d0bff29
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 56 deletions.
23 changes: 13 additions & 10 deletions lib/RestClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import util = require("./Util");

export interface IRestResponse<T> {
statusCode: number,
result: T
result: T | null
}

export interface IRequestOptions {
Expand Down Expand Up @@ -174,13 +174,16 @@ export class RestClient {

private async _processResponse<T>(res: httpm.HttpClientResponse, options: IRequestOptions): Promise<IRestResponse<T>> {
return new Promise<IRestResponse<T>>(async (resolve, reject) => {
let rres: IRestResponse<T> = <IRestResponse<T>>{};
let statusCode: number = res.message.statusCode;
rres.statusCode = statusCode;
const statusCode: number = res.message.statusCode;

const response: IRestResponse<T> = {
statusCode: statusCode,
result: null,
};

// not found leads to null obj returned
if (statusCode == httpm.HttpCodes.NotFound) {
resolve(rres);
resolve(response);
}

let obj: any;
Expand All @@ -191,10 +194,10 @@ export class RestClient {
if (contents && contents.length > 0) {
obj = JSON.parse(contents);
if (options && options.responseProcessor) {
rres.result = options.responseProcessor(obj);
response.result = options.responseProcessor(obj);
}
else {
rres.result = obj;
response.result = obj;
}
}
}
Expand All @@ -217,13 +220,13 @@ export class RestClient {

// attach statusCode and body obj (if available) to the error object
err['statusCode'] = statusCode;
if (rres.result) {
err['result'] = rres.result;
if (response.result) {
err['result'] = response.result;
}

reject(err);
} else {
resolve(rres);
resolve(response);
}
});
}
Expand Down
92 changes: 46 additions & 46 deletions test/resttests.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe('Rest Tests', function () {

it('constructs', () => {
this.timeout(1000);

let rest: restm.RestClient = new restm.RestClient('typed-test-client-tests');
assert(rest, 'rest client should not be null');
})
Expand All @@ -38,77 +38,77 @@ describe('Rest Tests', function () {

let restRes: restm.IRestResponse<HttpBinData> = await _rest.get<HttpBinData>('https://httpbin.org/get');
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/get');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/get');
});

it('gets a resource with baseUrl', async() => {
let restRes: restm.IRestResponse<HttpBinData> = await _restBin.get<HttpBinData>('get');
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/get');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/get');
});

it('creates a resource', async() => {
let res: any = { name: 'foo' };
let restRes: restm.IRestResponse<HttpBinData> = await _rest.create<HttpBinData>('https://httpbin.org/post', res);
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/post');
assert(restRes.result.json.name === 'foo');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/post');
assert(restRes.result && restRes.result.json.name === 'foo');
});

it('creates a resource with a baseUrl', async() => {
let res: any = { name: 'foo' };
let restRes: restm.IRestResponse<HttpBinData> = await _restBin.create<HttpBinData>('post', res);
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/post');
assert(restRes.result.json.name === 'foo');
});
assert(restRes.result && restRes.result.url === 'https://httpbin.org/post');
assert(restRes.result && restRes.result.json.name === 'foo');
});

it('replaces a resource', async() => {
this.timeout(3000);

let res: any = { name: 'foo' };
let restRes: restm.IRestResponse<HttpBinData> = await _rest.replace<HttpBinData>('https://httpbin.org/put', res);
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/put');
assert(restRes.result.json.name === 'foo');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/put');
assert(restRes.result && restRes.result.json.name === 'foo');
});

it('replaces a resource with a baseUrl', async() => {
let res: any = { name: 'foo' };
let restRes: restm.IRestResponse<HttpBinData> = await _restBin.replace<HttpBinData>('put', res);
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/put');
assert(restRes.result.json.name === 'foo');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/put');
assert(restRes.result && restRes.result.json.name === 'foo');
});

it('updates a resource', async() => {
let res: any = { name: 'foo' };
let restRes: restm.IRestResponse<HttpBinData> = await _rest.update<HttpBinData>('https://httpbin.org/patch', res);
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/patch');
assert(restRes.result.json.name === 'foo');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/patch');
assert(restRes.result && restRes.result.json.name === 'foo');
});

it('updates a resource with a baseUrl', async() => {
let res: any = { name: 'foo' };
let restRes: restm.IRestResponse<HttpBinData> = await _restBin.update<HttpBinData>('patch', res);
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/patch');
assert(restRes.result.json.name === 'foo');
});
assert(restRes.result && restRes.result.url === 'https://httpbin.org/patch');
assert(restRes.result && restRes.result.json.name === 'foo');
});

it('deletes a resource', async() => {
let restRes: restm.IRestResponse<HttpBinData> = await _rest.del<HttpBinData>('https://httpbin.org/delete');
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/delete');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/delete');
});

it('deletes a resource with a baseUrl', async() => {
let restRes: restm.IRestResponse<HttpBinData> = await _restBin.del<HttpBinData>('delete');
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/delete');
});
assert(restRes.result && restRes.result.url === 'https://httpbin.org/delete');
});

it('does an options request', async() => {
let restRes: restm.IRestResponse<HttpBinData> = await _rest.options<HttpBinData>('https://httpbin.org');
assert(restRes.statusCode == 200, "statusCode should be 200");
Expand All @@ -117,7 +117,7 @@ describe('Rest Tests', function () {
it('does an options request with baseUrl', async() => {
let restRes: restm.IRestResponse<HttpBinData> = await _restBin.options<HttpBinData>('');
assert(restRes.statusCode == 200, "statusCode should be 200");
});
});

//----------------------------------------------
// Get Error Cases
Expand All @@ -132,9 +132,9 @@ describe('Rest Tests', function () {

try {
let restRes: restm.IRestResponse<HttpBinData> = await _rest.get<HttpBinData>('https://httpbin.org/status/404');

assert(restRes.statusCode == 404, "statusCode should be 404");
assert(restRes.result == null, "object should be null");
assert(restRes.result === null, "object should be null");
}
catch(err) {
assert(false, "should not throw");
Expand All @@ -145,7 +145,7 @@ describe('Rest Tests', function () {
// Unauthorized (401)
// should throw and attach statusCode to the Error object
// err.message is message proerty of resourceful error object or if not supplied, a generic error message
//
//
it('gets and handles unauthorized (401)', async() => {
try {
let restRes: restm.IRestResponse<HttpBinData> = await _rest.get<HttpBinData>('https://httpbin.org/status/401');
Expand All @@ -155,13 +155,13 @@ describe('Rest Tests', function () {
assert(err['statusCode'] == 401, "statusCode should be 401");
assert(err.message && err.message.length > 0, "should have error message");
}
});
});

//
// Internal Server Error
// should throw and attach statusCode to the Error object
// err.message is message proerty of resourceful error object or if not supplied, a generic error message
//
//
it('gets and handles a server error (500)', async() => {
try {
let restRes: restm.IRestResponse<HttpBinData> = await _rest.get<HttpBinData>('https://httpbin.org/status/500');
Expand All @@ -187,7 +187,7 @@ describe('Rest Tests', function () {

// Assert
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/anything/anythingextra');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/anything/anythingextra');
});

it('maintains the path from the base url with no slashes', async() => {
Expand All @@ -199,7 +199,7 @@ describe('Rest Tests', function () {

// Assert
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/anything/anythingextra');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/anything/anythingextra');
});

it('maintains the path from the base url with double slashes', async() => {
Expand All @@ -211,7 +211,7 @@ describe('Rest Tests', function () {

// Assert
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/anything/anythingextra');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/anything/anythingextra');
});

it('maintains the path from the base url with multiple parts', async() => {
Expand All @@ -223,7 +223,7 @@ describe('Rest Tests', function () {

// Assert
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/anything/extrapart/anythingextra');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/anything/extrapart/anythingextra');
});

it('maintains the path from the base url where request has multiple parts', async() => {
Expand All @@ -235,7 +235,7 @@ describe('Rest Tests', function () {

// Assert
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/anything/anythingextra/moreparts');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/anything/anythingextra/moreparts');
});

it('maintains the path from the base url where both have multiple parts', async() => {
Expand All @@ -247,7 +247,7 @@ describe('Rest Tests', function () {

// Assert
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/anything/multiple/anythingextra/moreparts');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/anything/multiple/anythingextra/moreparts');
});

it('maintains the path from the base url where request has query parameters', async() => {
Expand All @@ -260,9 +260,9 @@ describe('Rest Tests', function () {

// Assert
assert(restRes.statusCode == 200, "statusCode should be 200");
assert(restRes.result.url === 'https://httpbin.org/anything/multiple/anythingextra/moreparts?foo=bar&baz=top');
assert(restRes.result.args.foo === 'bar');
assert(restRes.result.args.baz === 'top');
assert(restRes.result && restRes.result.url === 'https://httpbin.org/anything/multiple/anythingextra/moreparts?foo=bar&baz=top');
assert(restRes.result && restRes.result.args.foo === 'bar');
assert(restRes.result && restRes.result.args.baz === 'top');
});

//
Expand All @@ -277,11 +277,11 @@ describe('Rest Tests', function () {
let res: string = util.getUrl('', 'http://httpbin.org');
assert(res === 'http://httpbin.org', "should be http://httpbin.org");
});

it('resolves a null resource with baseUrl', async() => {
let res: string = util.getUrl(null, 'http://httpbin.org');
assert(res === 'http://httpbin.org', "should be http://httpbin.org");
});
});

it('resolves a full resource and no baseUrl', async() => {
let res: string = util.getUrl('http://httpbin.org/get?x=y&a=b');
Expand All @@ -296,7 +296,7 @@ describe('Rest Tests', function () {
it('resolves a relative path resource with host baseUrl', async() => {
let res: string = util.getUrl('get/foo', 'http://httpbin.org');
assert(res === 'http://httpbin.org/get/foo', `should be http://httpbin.org/get/foo but is ${res}`);
});
});

it('resolves a rooted path resource with pathed baseUrl', async() => {
let res: string = util.getUrl('/get/foo', 'http://httpbin.org/bar');
Expand Down

0 comments on commit d0bff29

Please sign in to comment.