Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added Express request handler #7

Merged
merged 6 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
"sql-formatter": "^15.0.2"
},
"devDependencies": {
"@types/express": "^4.17.21",
"@types/node": "^20.10.5",
"@types/pg": "^8.10.9",
"@vitest/coverage-v8": "^1.2.2",
Expand Down
6 changes: 6 additions & 0 deletions packages/backend/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
export { collectLast } from './util/generators/collectLast';
export {
type ExpressSynthqlHandler,
type ExpressSynthqlHandlerRequest,
type ExpressSynthqlHandlerResponse,
createExpressSynthqlHandler,
} from './util/handlers/createExpressSynthqlHandler';

export type * from './types/QueryPlan';
export * from './QueryEngine';
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { describe, test } from 'vitest';
import { query } from '@synthql/queries';

import { DB } from '../../tests/db';
const from = query<DB>().from;

describe('createExpressSynthqlHandler', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This test isn't really testing anything useful @jimezesinachi

A few of the important invariants you should be testing:

  1. The response is always 1..n lines of JSON
  2. when returnLastOnly the status code can vary depending on the response kind
  3. There's really no need to mock: spin up an actual server using http.createServer
  4. Tests for error cases i.e. when the query is malformed
  5. Add also tests using the react client. Notice that there is already a http.createServer in /react, instead of the custom handler code refactor to use the createExpressSynthqlHandler

test(`Query execution is successful`, async () => {
const q = from('actor')
fhur marked this conversation as resolved.
Show resolved Hide resolved
.columns('actor_id', 'first_name', 'last_name')
.groupingId('actor_id')
.where({ actor_id: { in: [1] } })
.one();
});

test(`Query execution is successful with returnLastOnly passed`, async () => {
const q = from('actor')
.columns('actor_id', 'first_name', 'last_name')
.groupingId('actor_id')
.where({ actor_id: { in: [1] } })
.maybe();
});
});
63 changes: 63 additions & 0 deletions packages/backend/src/util/handlers/createExpressSynthqlHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { QueryEngine, collectLast } from '../..';
import type { Request, Response } from 'express';

export type ExpressSynthqlHandlerRequest = Pick<Request, 'body' | 'headers'>;

export type ExpressSynthqlHandlerResponse = Pick<
Response,
'statusCode' | 'write' | 'setHeader' | 'end'
>;

export type ExpressSynthqlHandler = (
req: ExpressSynthqlHandlerRequest,
res: ExpressSynthqlHandlerResponse,
) => void;

export function createExpressSynthqlHandler<T>(
queryEngine: QueryEngine<T>,
): ExpressSynthqlHandler {
return async (req, res) => {
try {
const headers = req.headers;
const query = await JSON.parse(req.body);
const returnLastOnly = headers['x-return-last-only'] === 'true';

if (returnLastOnly) {
try {
const result = await collectLast(
queryEngine.execute(query, {
returnLastOnly,
}),
);

res.statusCode = 200;
res.setHeader('Content-Type', 'application/json');
res.write(JSON.stringify(result));
res.end();
} catch (error) {
res.statusCode = 500;
res.setHeader('Content-Type', 'application/json');
res.write(JSON.stringify({ error: String(error) }));
res.end();
}
} else {
res.statusCode = 200;
res.setHeader('Content-Type', 'application/x-ndjson');

for await (const intermediateResult of queryEngine.execute(
query,
)) {
res.write(JSON.stringify(intermediateResult));
res.write('\n');
}

res.end();
}
} catch (error) {
res.statusCode = 400;
res.setHeader('Content-Type', 'application/json');
res.write(JSON.stringify({ error: 'Invalid JSON body' }));
res.end();
}
};
}
2 changes: 1 addition & 1 deletion packages/docs/static/reference/assets/navigation.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/docs/static/reference/assets/search.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

46 changes: 46 additions & 0 deletions packages/react/src/test/appendBody.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { IncomingMessage, ServerResponse } from 'http';
import {
ExpressSynthqlHandler,
ExpressSynthqlHandlerRequest,
ExpressSynthqlHandlerResponse,
} from '@synthql/backend';

function readBody(req: IncomingMessage): Promise<string> {
return new Promise((resolve, reject) => {
let body = '';

req.on('data', (chunk) => {
body += chunk;
});

req.on('end', () => {
resolve(body);
});

req.on('error', (e) => {
reject(e);
});
});
}

export type IncomingMessageWithBody = IncomingMessage &
ExpressSynthqlHandlerRequest;

export type ServerResponseWithEnd = ServerResponse &
ExpressSynthqlHandlerResponse;

export function appendBody(expressHandler: ExpressSynthqlHandler) {
return async (req: IncomingMessage, res: ServerResponse) => {
const body = await readBody(req);

const newReq = {
...req,
headers: req.headers,
body: body,
} as IncomingMessageWithBody;
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't do as Type, this is generally unsafe behaviour. Instead do

 const newReq: Type = { ... }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be ideal, but the goal here is actually type casting. We don't have all the values for the type, so using a type annotation in this case, causes a type mismatch error.


const newRes = res as ServerResponseWithEnd;

expressHandler(newReq, newRes);
};
}
41 changes: 8 additions & 33 deletions packages/react/src/test/createPagilaServer.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { QueryEngine } from '@synthql/backend';
import { DB } from './db';
import { QueryEngine, createExpressSynthqlHandler } from '@synthql/backend';
import http from 'http';

import { DB } from './db';
import { appendBody } from './appendBody';

export interface PagilaServer {
url: string;
server: http.Server;
Expand All @@ -13,42 +15,15 @@ export function createPagilaServer({
queryEngine: QueryEngine<DB>;
}): Promise<PagilaServer> {
return new Promise((resolve, reject) => {
const server = http.createServer((req, res) => {
let body = '';

req.on('data', (chunk) => {
body += chunk;
});

req.on('end', async () => {
const headers = req.headers;
const handler = createExpressSynthqlHandler<DB>(queryEngine);

const json = JSON.parse(body);
const nodeHandler = appendBody(handler);

res.writeHead(200, { 'Content-Type': 'application/json' });

try {
for await (const intermediateResult of queryEngine.execute(
json,
{
returnLastOnly:
headers['x-return-last-only'] === 'true',
},
)) {
res.write(JSON.stringify(intermediateResult) + '\n');
}

res.end();
} catch (error) {
res.write(JSON.stringify({ error: String(error) }) + '\n');

res.end();
}
});
});
const server = http.createServer(nodeHandler);

server.listen(() => {
const address = server.address();

if (typeof address === 'string' || address === null) {
reject('Failed to get server address: ' + address);
} else {
Expand Down
4 changes: 2 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2833,9 +2833,9 @@
"@types/range-parser" "*"
"@types/send" "*"

"@types/express@*", "@types/express@^4.17.13":
"@types/express@*", "@types/express@^4.17.13", "@types/express@^4.17.21":
version "4.17.21"
resolved "https://registry.npmjs.org/@types/express/-/express-4.17.21.tgz"
resolved "https://registry.yarnpkg.com/@types/express/-/express-4.17.21.tgz#c26d4a151e60efe0084b23dc3369ebc631ed192d"
integrity sha512-ejlPM315qwLpaQlQDTjPdsUFSc6ZsP4AN6AlWnogPjQ7CVi7PYF3YVz+CY3jE2pwYf7E/7HlDAN0rV2GxTG0HQ==
dependencies:
"@types/body-parser" "*"
Expand Down
Loading