Skip to content

Commit

Permalink
fix: fix bug where request.query is not merged into request.data
Browse files Browse the repository at this point in the history
this fixes bug where request.query is not merged into request.data and simplifies how routeResponses
are handled

BREAKING CHANGE: this update breaks how routeResponses are sent back to the client
  • Loading branch information
teclone committed Jan 10, 2025
1 parent 010f072 commit 29f45d9
Show file tree
Hide file tree
Showing 7 changed files with 50 additions and 89 deletions.
10 changes: 5 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,26 +149,26 @@ If `https.enabled` is false, the server will listen for http only requests.

It comes with an inbuilt **request body parser**, that supports all forms of http request data such as **urlencoded query strings**, **application/json data**, **application/x-www-form-urlencoded data** and **multipart/form-data**.

Parsed fields and files are made available on the request object via the `query`, `body`, `data` and `files` properties. Uploaded files are stored in a tmp folder, **tmp/uploads**.
Parsed fields and files are made available on the request object via the `data` properties. `request.data` contains parsed query string and parsed body. Fields from the query string are overwritten by fields in the response body.

The `data` property is a combination of all fields in the `query` and `body` properties, with values in the `body` property winning the battle in case of conflicting field keys.
`request.query` contains the parsed query parameters.

Multi-value fields are supported as well. They are recognised if the field name ends with the bracket notation `[]`. Note that the brackets are stripped out during the parsing. It uses the same principle like in [PHP](http://php.net/manual/en/tutorial.forms.php).
Multi-value fields are supported. They are recognised if the field name ends with the bracket notation `[]`. Note that the brackets are stripped out during the parsing.

```typescript
const { Server } = require('@teclone/r-server'); // import rserver
const server = new Server(); // create server instance

server.put('users/{userId}/profile-picture', (req, res) => {
const picture = req.files.picture;
const picture = req.data.picture;

return res.json({
status: 'success',
message: 'got your file',
fileSize: picture.size,
mimeType: picture.type,
filename: picture.name,
fileTempLocation: picture.path,
bufferData: picture.data,
});
});

Expand Down
77 changes: 25 additions & 52 deletions src/modules/Response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,6 @@ import { ErrorCallback, RouteResponse } from '../@types';
import { handleError } from './Utils';
import { ServerRequest } from './Request';

export interface AfterResponseCallbacksOpt<Data, Errors> {
/**
* handler callback after a successful response has been sent to the client
*/
onSuccess?: (response: RouteResponse<Data, Errors>) => void;

/**
* callback is called after an error response is sent back to the client
*/
onError?: (response: RouteResponse<Data, Errors>) => void;
}

export type ServerResponse<
T extends typeof Http2ServerResponse | typeof Http1ServerResponse =
| typeof Http2ServerResponse
Expand Down Expand Up @@ -92,10 +80,15 @@ export type ServerResponse<
removeHeaders(this: ServerResponse<T>, ...names: string[]): ServerResponse<T>;

/**
* sets response status code
* @param code - the response code
* sets response http status code and message if given
* @param code - the status code
* @param message - the http status message to given sent
*/
status(this: ServerResponse<T>, code: number): ServerResponse<T>;
status(
this: ServerResponse<T>,
code: number,
message?: string
): ServerResponse<T>;

/**
* sends json response back to the client.
Expand Down Expand Up @@ -128,17 +121,15 @@ export type ServerResponse<
*/
jsonError<Data, Errors>(
this: ServerResponse<T>,
response?: RouteResponse<Data, Errors>,
options?: AfterResponseCallbacksOpt<Data, Errors>
response?: RouteResponse<Data, Errors>
): Promise<boolean>;

/**
* sends json success data back to the client
*/
jsonSuccess<Data, Errors>(
this: ServerResponse<T>,
response?: RouteResponse<Data, Errors>,
options?: AfterResponseCallbacksOpt<Data, Errors>
response?: RouteResponse<Data, Errors>
): Promise<boolean>;

/**
Expand All @@ -158,9 +149,7 @@ export type ServerResponse<
/**
* the response promise
*/
responsePromise: Promise<RouteResponse<Data, Errors>>,

options?: AfterResponseCallbacksOpt<Data, Errors>
responsePromise: Promise<RouteResponse<Data, Errors>>
): Promise<boolean>;
};

Expand Down Expand Up @@ -247,8 +236,11 @@ const createResponseClass = <
return this;
};

ResponseClass.prototype.status = function (code) {
ResponseClass.prototype.status = function (code, message) {
this.statusCode = code;
if (message) {
this.statusMessage = message;
}
return this;
};

Expand All @@ -266,23 +258,15 @@ const createResponseClass = <
return this.setHeader('Content-Type', 'application/json').end(resolvedData);
};

ResponseClass.prototype.jsonError = function (response, opts) {
ResponseClass.prototype.jsonError = function (response) {
const { headers, message, errors = null, statusCode } = response || {};

return this.status(statusCode || 400)
return this.status(statusCode || 400, message)
.setHeaders(headers)
.json({
message: message || 'Request failed',
errors,
})
.finally(() => {
if (opts?.onError) {
opts.onError(response);
}
});
.json(errors as any);
};

ResponseClass.prototype.jsonSuccess = function (response, opts) {
ResponseClass.prototype.jsonSuccess = function (response) {
const {
statusCode = 200,
headers,
Expand All @@ -292,20 +276,12 @@ const createResponseClass = <
} = response || {};

if (statusCode >= 300 || errors) {
return this.jsonError(response, opts);
return this.jsonError(response);
}

return this.status(statusCode)
return this.status(statusCode, message)
.setHeaders(headers)
.json({
message: message || 'Request successful',
data,
})
.finally(() => {
if (opts?.onSuccess) {
opts.onSuccess(response);
}
});
.json(data as any);
};

ResponseClass.prototype.wait = function (time: number) {
Expand All @@ -316,20 +292,17 @@ const createResponseClass = <
});
};

ResponseClass.prototype.processRouteResponse = function (
responsePromise,
opts
) {
ResponseClass.prototype.processRouteResponse = function (responsePromise) {
return responsePromise
.then((response) => {
return this.jsonSuccess(response, opts);
return this.jsonSuccess(response);
})

.catch((err) => {
if (err instanceof Error) {
return handleError(err, this);
}
return this.jsonError(err, opts);
return this.jsonError(err);
});
};

Expand Down
4 changes: 4 additions & 0 deletions src/modules/Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,10 @@ export class Server<
...request.query,
...data,
};
} else {
request.data = {
...request.query,
};
}
}

Expand Down
4 changes: 4 additions & 0 deletions src/modules/Utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ import { isString } from '@teclone/utils';
import { ServerResponse } from './Response';
import { brotliDecompress, unzip } from 'node:zlib';

export const noop = () => {
// do nothing
};

export const handleError = (
err: Error | string,
response: ServerResponse,
Expand Down
36 changes: 8 additions & 28 deletions tests/modules/Response.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ describe('Response', function () {
});
});

describe(`#jsonSuccess(statusCode: number = 200, message: string = 'success', data: object = {}): Promise<boolean>`, function () {
describe(`#jsonSuccess(statusCode: number = 200, message: string = 'ok', data: object = {}): Promise<boolean>`, function () {
it(`should send success json data back to the client`, function () {
server.get('/', (req, res) => {
return res.jsonSuccess({
Expand All @@ -138,22 +138,19 @@ describe('Response', function () {

return server.listen().then(() => {
return sendRequest({ uri: httpHost, json: true }).then((res) => {
expect(res.body).toHaveProperty(
'message',
'user created successfully'
);
expect(res.body).toHaveProperty('user', { id: 1 });
});
});
});

it(`should default the statusCode to 200, message string to 'Request successful' and data object to empty object`, function () {
it(`should default the statusCode to 200, and body to empty string`, function () {
server.get('/', (req, res) => {
return res.jsonSuccess();
});

return server.listen().then(() => {
return sendRequest({ uri: httpHost, json: true }).then((res) => {
expect(res.body).toHaveProperty('message', 'Request successful');
expect(res.body).toEqual('');
});
});
});
Expand All @@ -164,37 +161,20 @@ describe('Response', function () {
server.get('/', (req, res) => {
return res.jsonError({
statusCode: 403,
message: 'permission denied',
});
});

return server.listen().then(() => {
return expect(
sendRequest({ uri: httpHost, json: true })
).rejects.toMatchObject({
statusCode: 403,
response: {
body: {
message: 'permission denied',
},
errors: {
error: 'permission denied',
},
});
});
});

it(`should default the statusCode to 400, message string to 'Request failed' and data to null`, function () {
server.get('/', (req, res) => {
return res.jsonError();
});

return server.listen().then(() => {
return expect(
sendRequest({ uri: httpHost, json: true })
).rejects.toMatchObject({
response: {
statusCode: 403,
body: {
message: 'Request failed',
errors: null,
error: 'permission denied',
},
},
});
Expand Down
6 changes: 3 additions & 3 deletions tests/modules/Server.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ describe(`Server`, function () {
};

server.post('/process-data', (req, res) => {
return res.json(req.body);
return res.json(req.data);
});

return server.listen().then(async () => {
Expand Down Expand Up @@ -462,7 +462,7 @@ describe(`Server`, function () {
password: 'passwd_243',
};
server.post('/check-data', (req, res) => {
return res.json(req.body);
return res.json(req.data);
});

return server.listen().then(() => {
Expand Down Expand Up @@ -546,7 +546,7 @@ describe(`Server`, function () {
describe('mounted routing', function () {
it(`should start the routing engine on the mounted routers if request path did not map
to a public file, and did not get resolved by the main router, and run the matching registered all method route if any`, function () {
const router = new Router(false);
const router = new Router({ inheritMiddlewares: false });

router.any('/{id}', function (req, res) {
return res.end();
Expand Down
2 changes: 1 addition & 1 deletion tests/modules/Wrapper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ describe('Wrapper', function () {
let wrapper: Wrapper, router: Router;

beforeEach(function () {
router = new Router(false);
router = new Router({ inheritMiddlewares: false });
wrapper = new Wrapper(router, url);
});

Expand Down

0 comments on commit 29f45d9

Please sign in to comment.