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

Upgrade stuff + get rid of any + lints #223

Merged
merged 6 commits into from
Sep 2, 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
2 changes: 0 additions & 2 deletions .eslintignore

This file was deleted.

55 changes: 0 additions & 55 deletions .eslintrc

This file was deleted.

2 changes: 1 addition & 1 deletion .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
- name: Setup Node
uses: actions/setup-node@v3
with:
node-version: 18.x
node-version-file: '.nvmrc'
cache: pnpm

- name: pnpm install
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/pr-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
- name: Setup Node
uses: actions/setup-node@v3
with:
node-version: 18.x
node-version-file: '.nvmrc'
cache: pnpm

- name: Install
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ jobs:
if: steps.check.outputs.changed == 'true'
uses: actions/setup-node@v3
with:
node-version: 18.x
node-version-file: '.nvmrc'
registry-url: https://registry.npmjs.org/

- name: Install
Expand Down
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ ca.key
.brackets.json
.npmrc
npm-debug.log
.nvmrc
yarn-error.log
coverage/
dim-api-types/*.js
Expand Down
1 change: 1 addition & 0 deletions .nvmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
20
6 changes: 5 additions & 1 deletion api/db/apps-queries.test.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { DatabaseError } from 'pg';
import { v4 as uuid } from 'uuid';
import { ApiApp } from '../shapes/app.js';
import { getAllApps, getAppById, insertApp } from './apps-queries.js';
Expand Down Expand Up @@ -32,7 +33,10 @@ it('cannot create a new app with the same name as an existing one', async () =>
try {
await insertApp(client, app);
} catch (e) {
expect(e.code).toBe('23505');
if (!(e instanceof DatabaseError)) {
fail('should have thrown a DatabaseError');
}
expect((e).code).toBe('23505');
}
});
});
Expand Down
54 changes: 21 additions & 33 deletions api/db/apps-queries.ts
Original file line number Diff line number Diff line change
@@ -1,54 +1,42 @@
import { ClientBase, QueryResult } from 'pg';
import { ApiApp } from '../shapes/app.js';
import { camelize } from '../utils.js';
import { camelize, KeysToSnakeCase, TypesForKeys } from '../utils.js';

/**
* Get all registered apps.
*/
export async function getAllApps(client: ClientBase): Promise<ApiApp[]> {
try {
const results = await client.query<ApiApp>({
name: 'get_all_apps',
text: 'SELECT * FROM apps',
});
return results.rows.map((row) => camelize<ApiApp>(row));
} catch (e) {
throw new Error(e.name + ': ' + e.message);
}
const results = await client.query<KeysToSnakeCase<ApiApp>>({
name: 'get_all_apps',
text: 'SELECT * FROM apps',
});
return results.rows.map((row) => camelize(row));
}

/**
* Get an app by its ID.
*/
export async function getAppById(client: ClientBase, id: string): Promise<ApiApp | null> {
try {
const results = await client.query<ApiApp>({
name: 'get_apps',
text: 'SELECT * FROM apps where id = $1',
values: [id],
});
if (results.rows.length > 0) {
return camelize<ApiApp>(results.rows[0]);
} else {
return null;
}
} catch (e) {
throw new Error(e.name + ': ' + e.message);
const results = await client.query<KeysToSnakeCase<ApiApp>>({
name: 'get_apps',
text: 'SELECT * FROM apps where id = $1',
values: [id],
});
if (results.rows.length > 0) {
return camelize(results.rows[0]);
} else {
return null;
}
}

/**
* Insert a new app into the list of registered apps.
*/
export async function insertApp(client: ClientBase, app: ApiApp): Promise<QueryResult<any>> {
try {
return client.query({
name: 'insert_app',
text: `insert into apps (id, bungie_api_key, dim_api_key, origin)
export async function insertApp(client: ClientBase, app: ApiApp): Promise<QueryResult> {
return client.query<any, TypesForKeys<ApiApp, ['id', 'bungieApiKey', 'dimApiKey', 'origin']>>({
name: 'insert_app',
text: `insert into apps (id, bungie_api_key, dim_api_key, origin)
values ($1, $2, $3, $4)`,
values: [app.id, app.bungieApiKey, app.dimApiKey, app.origin],
});
} catch (e) {
throw new Error(e.name + ': ' + e.message);
}
values: [app.id, app.bungieApiKey, app.dimApiKey, app.origin],
});
}
23 changes: 17 additions & 6 deletions api/db/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ beforeEach(async () => {
)`);
});

interface TransactionTestRow {
id: number;
test: string;
}

afterAll(async () => {
try {
await pool.query(`DROP TABLE transaction_test`);
Expand All @@ -28,10 +33,10 @@ describe('transaction', () => {
});
fail('should have thrown an error');
} catch (e) {
expect(e.message).toBe('oops');
expect((e as Error).message).toBe('oops');
}

const result = await pool.query('select * from transaction_test');
const result = await pool.query<TransactionTestRow>('select * from transaction_test');
expect(result.rows.length).toBe(1);
expect(result.rows[0].id).toBe(1);
});
Expand All @@ -41,7 +46,7 @@ describe('transaction', () => {
await client.query("insert into transaction_test (id, test) values (3, 'testing commits')");
});

const result = await pool.query('select * from transaction_test');
const result = await pool.query<TransactionTestRow>('select * from transaction_test');
expect(result.rows.length).toBe(1);
expect(result.rows[0].test).toBe('testing commits');
});
Expand All @@ -61,7 +66,9 @@ describe('readTransaction', () => {

// Now request that info from our original client.
// should be read-committed, so we shouldn't see that update
const result = await client.query('select * from transaction_test where id = 1');
const result = await client.query<TransactionTestRow>(
'select * from transaction_test where id = 1',
);
expect(result.rows[0].test).toBe('testing');

// Commit the update
Expand All @@ -74,12 +81,16 @@ describe('readTransaction', () => {
}

// once that other transaction commits, we'll see its update
const result = await client.query('select * from transaction_test where id = 1');
const result = await client.query<TransactionTestRow>(
'select * from transaction_test where id = 1',
);
expect(result.rows[0].test).toBe('updated');
});

// outside, we should still see the transactional update
const result = await pool.query('select * from transaction_test where id = 1');
const result = await pool.query<TransactionTestRow>(
'select * from transaction_test where id = 1',
);
expect(result.rows[0].test).toBe('updated');
});
});
6 changes: 3 additions & 3 deletions api/db/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pool.on('acquire', () => {
});
pool.on('error', (e: Error) => {
metrics.increment('db.pool.error.count');
metrics.increment('db.pool.error.' + e.name + '.count');
metrics.increment(`db.pool.error.${ e.name }.count`);
});
pool.on('remove', () => {
metrics.increment('db.pool.remove.count');
Expand Down Expand Up @@ -66,10 +66,10 @@ export async function readTransaction<T>(fn: (client: ClientBase) => Promise<T>)
const client = await pool.connect();
try {
// We used to wrap multiple reads in a transaction but I'm not sure it matters all that much.
//await client.query('BEGIN');
// await client.query('BEGIN');
return await fn(client);
} finally {
//await client.query('ROLLBACK');
// await client.query('ROLLBACK');
client.release();
}
}
Loading
Loading