From 00ef8bd22f98335a8bec8705fe0abbe6ead938d2 Mon Sep 17 00:00:00 2001 From: Stephen Hand Date: Wed, 7 Jun 2023 18:57:09 +0100 Subject: [PATCH 01/10] Fix unit tests after nested formatting is removed --- hrm-domain/hrm-service/package.json | 2 +- .../hrm-service/src/case/case-data-access.ts | 5 +- .../src/case/sql/case-search-sql.ts | 16 +- .../hrm-service/src/parameterizedQuery.ts | 146 ++++++++++++++++++ .../unit-tests/parameterizedQuery.test.ts | 98 ++++++++++++ 5 files changed, 257 insertions(+), 10 deletions(-) create mode 100644 hrm-domain/hrm-service/src/parameterizedQuery.ts create mode 100644 hrm-domain/hrm-service/unit-tests/parameterizedQuery.test.ts diff --git a/hrm-domain/hrm-service/package.json b/hrm-domain/hrm-service/package.json index f93bbf8e4..c18b37a53 100644 --- a/hrm-domain/hrm-service/package.json +++ b/hrm-domain/hrm-service/package.json @@ -16,7 +16,7 @@ "test:run:watch": "cross-env AWS_REGION=us-east-1 CI=true TWILIO_ACCOUNT_SID=ACxxx TWILIO_AUTH_TOKEN=xxxxxx RDS_PASSWORD=postgres jest --maxWorkers=1 --forceExit --watch", "test:unit": "jest unit-tests", "test:service": "cross-env POSTGRES_PORT=5433 run-s -c docker:compose:test:up test:service:ci:migrate test:service:run docker:compose:test:down", - "test:service:run": "cross-env AWS_REGION=us-east-1 SSM_ENDPOINT=http://localhost:4566 jest --verbose --maxWorkers=1 --forceExit service-tests", + "test:service:run": "cross-env AWS_REGION=us-east-1 SSM_ENDPOINT=http://localhost:4566 jest --verbose --maxWorkers=1 --forceExit service-tests/case-search.test.ts", "test:service:ci": "run-s test:service:ci:migrate test:service:ci:run", "test:service:ci:migrate": "cross-env CI=true RDS_PASSWORD=postgres node ./db-migrate", "test:service:ci:run": "cross-env AWS_REGION=us-east-1 CI=true TWILIO_ACCOUNT_SID=ACxxx TWILIO_AUTH_TOKEN=xxxxxx RDS_PASSWORD=postgres jest --verbose --maxWorkers=1 --forceExit service-tests", diff --git a/hrm-domain/hrm-service/src/case/case-data-access.ts b/hrm-domain/hrm-service/src/case/case-data-access.ts index 008867279..0335702a3 100644 --- a/hrm-domain/hrm-service/src/case/case-data-access.ts +++ b/hrm-domain/hrm-service/src/case/case-data-access.ts @@ -22,6 +22,7 @@ import { caseSectionUpsertSql, deleteMissingCaseSectionsSql } from './sql/case-s import { DELETE_BY_ID } from './sql/case-delete-sql'; import { selectSingleCaseByIdSql } from './sql/case-get-sql'; import { Contact } from '../contact/contact-data-access'; +import { parameterizedQuery } from '../parameterizedQuery'; export type CaseRecordCommon = { info: any; @@ -159,7 +160,9 @@ export const search = async ( limit: limit, offset: offset, }; - const result: CaseWithCount[] = await connection.any(statement, queryValues); + const result: CaseWithCount[] = await connection.manyOrNone( + parameterizedQuery(statement, queryValues), + ); const totalCount: number = result.length ? result[0].totalCount : 0; return { rows: result, count: totalCount }; }); diff --git a/hrm-domain/hrm-service/src/case/sql/case-search-sql.ts b/hrm-domain/hrm-service/src/case/sql/case-search-sql.ts index d48944ac6..5db417a55 100644 --- a/hrm-domain/hrm-service/src/case/sql/case-search-sql.ts +++ b/hrm-domain/hrm-service/src/case/sql/case-search-sql.ts @@ -95,8 +95,8 @@ const dateFilterCondition = ( if (filter.to || filter.from) { filter.to = filter.to ?? null; filter.from = filter.from ?? null; - return `(($<${filterName}.from> IS NULL OR ${field} >= $<${filterName}.from>::TIMESTAMP WITH TIME ZONE) - AND ($<${filterName}.to> IS NULL OR ${field} <= $<${filterName}.to>::TIMESTAMP WITH TIME ZONE) + return `(($<${filterName}.from>::TIMESTAMP WITH TIME ZONE IS NULL OR ${field} >= $<${filterName}.from>::TIMESTAMP WITH TIME ZONE) + AND ($<${filterName}.to>::TIMESTAMP WITH TIME ZONE IS NULL OR ${field} <= $<${filterName}.to>::TIMESTAMP WITH TIME ZONE) ${existsCondition ? ` AND ${existsCondition}` : ''})`; } return existsCondition; @@ -160,15 +160,15 @@ const nameAndPhoneNumberSearchSql = ( lastNameSources: string[], phoneNumberColumns: string[], ) => - `CASE WHEN $ IS NULL THEN TRUE + `CASE WHEN $::text IS NULL THEN TRUE ELSE ${firstNameSources.map(fns => `${fns} ILIKE $`).join('\n OR ')} END AND - CASE WHEN $ IS NULL THEN TRUE + CASE WHEN $::text IS NULL THEN TRUE ELSE ${lastNameSources.map(lns => `${lns} ILIKE $`).join('\n OR ')} END AND - CASE WHEN $ IS NULL THEN TRUE + CASE WHEN $::text IS NULL THEN TRUE ELSE ( ${phoneNumberColumns .map(pn => `regexp_replace(${pn}, '\\D', '', 'g') ILIKE $`) @@ -178,7 +178,7 @@ const nameAndPhoneNumberSearchSql = ( const SEARCH_WHERE_CLAUSE = `( -- search on childInformation of connectedContacts - ($ IS NULL AND $ IS NULL AND $ IS NULL) OR + ($::text IS NULL AND $::text IS NULL AND $::text IS NULL) OR EXISTS ( SELECT 1 FROM "Contacts" c WHERE c."caseId" = cases.id AND c."accountSid" = cases."accountSid" AND ( @@ -236,7 +236,7 @@ const SEARCH_WHERE_CLAUSE = `( ) ) AND ( - $ IS NULL OR + $::text IS NULL OR EXISTS ( SELECT 1 FROM "Contacts" c WHERE c."caseId" = cases.id AND c."accountSid" = cases."accountSid" AND c.number = $ ) @@ -277,7 +277,7 @@ export const selectCaseSearch = ( `WHERE (info IS NULL OR jsonb_typeof(info) = 'object') AND - $ IS NOT NULL AND cases."accountSid" = $`, + $::text IS NOT NULL AND cases."accountSid" = $`, SEARCH_WHERE_CLAUSE, filterSql(filters), ].filter(sql => sql).join(` diff --git a/hrm-domain/hrm-service/src/parameterizedQuery.ts b/hrm-domain/hrm-service/src/parameterizedQuery.ts new file mode 100644 index 000000000..85b393fa1 --- /dev/null +++ b/hrm-domain/hrm-service/src/parameterizedQuery.ts @@ -0,0 +1,146 @@ +/** + * Copyright (C) 2021-2023 Technology Matters + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://www.gnu.org/licenses/. + */ + +import { ParameterizedQuery } from 'pg-promise'; + +function isJSONValue(token: string, sql: string): boolean { + // If any of the tokens mark it as JSON, treat it as JSON everywhere + // We can deal with weird corner cases where as value is JSON somoe of the time and not others anon. + return sql.indexOf(`$<${token}:json>`) !== -1; +} + +function escapeRegExp(string: string): string { + return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +function flattenParams( + obj: { [key: string]: any }, + parentKey: string, + sql: string, +): { [key: string]: any } { + const flattened: { [key: string]: any } = {}; + + Object.keys(obj).forEach(key => { + const value = obj[key]; + const fullPath = parentKey ? `${parentKey}.${key}` : key; + + if ( + typeof value === 'object' && + !Array.isArray(value) && + value !== null && + !isJSONValue(fullPath, sql) + ) { + const nestedParams = flattenParams(value, fullPath, sql); + Object.keys(nestedParams).forEach(nestedKey => { + flattened[nestedKey] = nestedParams[nestedKey]; + }); + } else { + flattened[fullPath] = value; + } + }); + + return flattened; +} + +export function convertToPostgreSQLQuery( + sql: string, + params: { [key: string]: any }, + valueCount = 0, +): { query: string; values: any[] } { + let query = sql; + const values: any[] = []; + // Replace named parameters with positional parameters + Object.keys(params).forEach(key => { + const paramValue = params[key]; + const tokenRegex = new RegExp(`\\$<${escapeRegExp(key)}(\:[a-z]+)?\\b>`, 'g'); + if ( + typeof paramValue === 'object' && + !Array.isArray(paramValue) && + paramValue !== null && + !isJSONValue(key, sql) + ) { + // Handle nested objects recursively + const nestedParams = flattenParams(paramValue, key, sql); + const nestedResult = convertToPostgreSQLQuery( + query, + nestedParams, + valueCount + values.length, + ); + query = nestedResult.query; + values.push(...nestedResult.values); + } else if (Array.isArray(paramValue)) { + let singleValueAdded = false; + let csvValuesAdded = false; + const positions = []; + const csvValues = []; + query = query.replace(tokenRegex, (match, formatSpecifier) => { + switch (formatSpecifier) { + case ':csv': + if (csvValuesAdded) { + paramValue.forEach((value: any) => { + csvValues.push(value ?? null); + positions.push(`$${values.length + valueCount}`); + }); + console.debug(`Mapping $${positions.join(', ')} to ${key}, values: ${paramValue}`); + csvValuesAdded = true; + } + return positions.join(', '); + case ':json': + if (!singleValueAdded) { + values.push(JSON.stringify(paramValue)); + singleValueAdded = true; + console.debug( + `Mapping $${values.length + valueCount} to ${key}, value: ${JSON.stringify( + paramValue, + )}`, + ); + } + return `$${values.length + valueCount}`; + default: + if (!singleValueAdded) { + values.push(JSON.stringify(paramValue)); + singleValueAdded = true; + console.debug( + `Mapping $${values.length + valueCount} to ${key}, value: ${paramValue}`, + ); + } + return `$${values.length + valueCount}`; + } + }); + } else { + let singleValueAdded = false; + query = query.replace(tokenRegex, () => { + if (!singleValueAdded) { + values.push(paramValue ?? null); + singleValueAdded = true; + console.debug(`Mapping $${values.length + valueCount} to ${key}, value: ${paramValue}`); + } + return `$${values.length + valueCount}`; + }); + } + }); + + return { query, values }; +} + +export function parameterizedQuery( + sql: string, + params: { [key: string]: any }, +): ParameterizedQuery { + const { query, values } = convertToPostgreSQLQuery(sql, params); + + return new ParameterizedQuery({ text: query, values }); +} diff --git a/hrm-domain/hrm-service/unit-tests/parameterizedQuery.test.ts b/hrm-domain/hrm-service/unit-tests/parameterizedQuery.test.ts new file mode 100644 index 000000000..69345aa49 --- /dev/null +++ b/hrm-domain/hrm-service/unit-tests/parameterizedQuery.test.ts @@ -0,0 +1,98 @@ +import { convertToPostgreSQLQuery } from '../src/parameterizedQuery'; + +describe('convertToPostgreSQLQuery', () => { + const testCases = [ + { + description: + 'should convert SQL statement and named parameters to PostgreSQL query with positional parameters', + sql: 'SELECT * FROM users WHERE age >= $ AND country IN ($)', + params: { + minAge: 18, + countries: ['USA', 'Canada', 'UK'], + }, + expectedQuery: 'SELECT * FROM users WHERE age >= $1 AND country IN ($2)', + expectedValues: [18, ['USA', 'Canada', 'UK']], + }, + { + description: 'should flatten nested maps of values', + sql: + 'INSERT INTO users (name, address.line1, address.city, settings) VALUES ($, $, $, $)', + params: { + name: 'John Doe', + address: { + line1: '123 Main St', + city: 'New York', + }, + settings: { + darkMode: true, + theme: 'light', + }, + }, + expectedQuery: + 'INSERT INTO users (name, address.line1, address.city, settings) VALUES ($1, $2, $3, $4)', + expectedValues: ['John Doe', '123 Main St', 'New York', { darkMode: true, theme: 'light' }], + }, + { + description: 'should handle array values as single positional parameters', + sql: 'SELECT * FROM users WHERE id IN ($) AND role = $', + params: { + ids: [1, 2, 3], + role: 'admin', + }, + expectedQuery: 'SELECT * FROM users WHERE id IN ($1) AND role = $2', + expectedValues: [[1, 2, 3], 'admin'], + }, + { + description: + 'should handle array values as comma-separated positional parameters when using :csv suffix', + sql: 'SELECT * FROM users WHERE id IN ($) AND role = $', + params: { + ids: [1, 2, 3], + role: 'admin', + }, + expectedQuery: 'SELECT * FROM users WHERE id IN ($1, $2, $3) AND role = $4', + expectedValues: [1, 2, 3, 'admin'], + }, + { + description: 'should handle nested arrays as single positional parameters', + sql: 'INSERT INTO users (name, hobbies) VALUES ($, $)', + params: { + name: 'John Doe', + hobbies: [['reading', 'coding'], ['gaming']], + }, + expectedQuery: 'INSERT INTO users (name, hobbies) VALUES ($1, $2)', + expectedValues: ['John Doe', [['reading', 'coding'], ['gaming']]], + }, + { + description: + 'nested arrays should not be flattened, but be a comma-separated positional set of array parameters when using :csv suffix', + sql: 'INSERT INTO users (name, hobbies_1, hobbies_2) VALUES ($, $)', + params: { + name: 'John Doe', + hobbies: [['reading', 'coding'], ['gaming']], + }, + expectedQuery: 'INSERT INTO users (name, hobbies_1, hobbies_2) VALUES ($1, $2, $3)', + expectedValues: ['John Doe', ['reading', 'coding'], ['gaming']], + }, + { + description: 'should handle JSONB values specified using :json suffix', + sql: 'INSERT INTO users (name, settings) VALUES ($, $)', + params: { + name: 'John Doe', + settings: { + darkMode: true, + theme: 'light', + }, + }, + expectedQuery: 'INSERT INTO users (name, settings) VALUES ($1, $2)', + expectedValues: ['John Doe', { darkMode: true, theme: 'light' }], + }, + ]; + + test.each(testCases)('$description', ({ sql, params, expectedQuery, expectedValues }) => { + const { query, values } = convertToPostgreSQLQuery(sql, params); + + expect(query).toBe(expectedQuery); + expect(values).toEqual(expectedValues); + }); +}); From 056d11dc8771aff83613a0800189252ca23a505d Mon Sep 17 00:00:00 2001 From: Stephen Hand Date: Wed, 7 Jun 2023 20:23:57 +0100 Subject: [PATCH 02/10] Fix parameterizedQuery & service tests --- .../hrm-service/service-tests/case-search.test.ts | 14 ++++++++++++++ hrm-domain/hrm-service/src/parameterizedQuery.ts | 7 +++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/hrm-domain/hrm-service/service-tests/case-search.test.ts b/hrm-domain/hrm-service/service-tests/case-search.test.ts index 8ba4e60f0..85152bbbd 100644 --- a/hrm-domain/hrm-service/service-tests/case-search.test.ts +++ b/hrm-domain/hrm-service/service-tests/case-search.test.ts @@ -1284,6 +1284,20 @@ describe('/cases route', () => { await caseDb.deleteById(createdCase.id, accountSid); useOpenRules(); }); + + test('Should throw error promptly with malformed input', async () => { + const start = Date.now(); + const response = await request + .post(`${route}/search`) + .query({ limit: 20, offset: 0 }) + .set(headers) + .send({ + filters: { createdAt: { from: '${contactNumber}' } }, + contactNumber: "=')) and (select pg_sleep(5)) is null AND ((1=1) --", + }); + expect(response.ok).toBeFalsy(); + expect(Date.now() - start).toBeLessThan(2000); + }, 10000); }); }); }); diff --git a/hrm-domain/hrm-service/src/parameterizedQuery.ts b/hrm-domain/hrm-service/src/parameterizedQuery.ts index 85b393fa1..fd3692d72 100644 --- a/hrm-domain/hrm-service/src/parameterizedQuery.ts +++ b/hrm-domain/hrm-service/src/parameterizedQuery.ts @@ -83,19 +83,17 @@ export function convertToPostgreSQLQuery( values.push(...nestedResult.values); } else if (Array.isArray(paramValue)) { let singleValueAdded = false; - let csvValuesAdded = false; const positions = []; const csvValues = []; query = query.replace(tokenRegex, (match, formatSpecifier) => { switch (formatSpecifier) { case ':csv': - if (csvValuesAdded) { + if (csvValues.length === 0) { paramValue.forEach((value: any) => { csvValues.push(value ?? null); - positions.push(`$${values.length + valueCount}`); + positions.push(`$${values.length + csvValues.length + valueCount}`); }); console.debug(`Mapping $${positions.join(', ')} to ${key}, values: ${paramValue}`); - csvValuesAdded = true; } return positions.join(', '); case ':json': @@ -120,6 +118,7 @@ export function convertToPostgreSQLQuery( return `$${values.length + valueCount}`; } }); + values.push(...csvValues); } else { let singleValueAdded = false; query = query.replace(tokenRegex, () => { From b55abc9e33751e322fc8cf18b2a1c2711a78b4f4 Mon Sep 17 00:00:00 2001 From: Stephen Hand Date: Wed, 7 Jun 2023 20:34:10 +0100 Subject: [PATCH 03/10] Fix parameterizedQuery & revert package.json --- hrm-domain/hrm-service/package.json | 2 +- hrm-domain/hrm-service/src/parameterizedQuery.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hrm-domain/hrm-service/package.json b/hrm-domain/hrm-service/package.json index c18b37a53..f93bbf8e4 100644 --- a/hrm-domain/hrm-service/package.json +++ b/hrm-domain/hrm-service/package.json @@ -16,7 +16,7 @@ "test:run:watch": "cross-env AWS_REGION=us-east-1 CI=true TWILIO_ACCOUNT_SID=ACxxx TWILIO_AUTH_TOKEN=xxxxxx RDS_PASSWORD=postgres jest --maxWorkers=1 --forceExit --watch", "test:unit": "jest unit-tests", "test:service": "cross-env POSTGRES_PORT=5433 run-s -c docker:compose:test:up test:service:ci:migrate test:service:run docker:compose:test:down", - "test:service:run": "cross-env AWS_REGION=us-east-1 SSM_ENDPOINT=http://localhost:4566 jest --verbose --maxWorkers=1 --forceExit service-tests/case-search.test.ts", + "test:service:run": "cross-env AWS_REGION=us-east-1 SSM_ENDPOINT=http://localhost:4566 jest --verbose --maxWorkers=1 --forceExit service-tests", "test:service:ci": "run-s test:service:ci:migrate test:service:ci:run", "test:service:ci:migrate": "cross-env CI=true RDS_PASSWORD=postgres node ./db-migrate", "test:service:ci:run": "cross-env AWS_REGION=us-east-1 CI=true TWILIO_ACCOUNT_SID=ACxxx TWILIO_AUTH_TOKEN=xxxxxx RDS_PASSWORD=postgres jest --verbose --maxWorkers=1 --forceExit service-tests", diff --git a/hrm-domain/hrm-service/src/parameterizedQuery.ts b/hrm-domain/hrm-service/src/parameterizedQuery.ts index fd3692d72..9b035b65a 100644 --- a/hrm-domain/hrm-service/src/parameterizedQuery.ts +++ b/hrm-domain/hrm-service/src/parameterizedQuery.ts @@ -109,7 +109,7 @@ export function convertToPostgreSQLQuery( return `$${values.length + valueCount}`; default: if (!singleValueAdded) { - values.push(JSON.stringify(paramValue)); + values.push(paramValue); singleValueAdded = true; console.debug( `Mapping $${values.length + valueCount} to ${key}, value: ${paramValue}`, From c80150d3d441098432173376ad08423fa9c8f701 Mon Sep 17 00:00:00 2001 From: Stephen Hand Date: Thu, 8 Jun 2023 07:33:25 +0100 Subject: [PATCH 04/10] Migrate case PUT endpoint to use parameterized query --- .../hrm-service/src/case/case-data-access.ts | 26 ++++++++++++------- .../src/case/sql/case-update-sql.ts | 6 +---- .../hrm-service/src/parameterizedQuery.ts | 4 +-- .../resources-import-producer/index.ts | 2 +- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/hrm-domain/hrm-service/src/case/case-data-access.ts b/hrm-domain/hrm-service/src/case/case-data-access.ts index 0335702a3..d58663c27 100644 --- a/hrm-domain/hrm-service/src/case/case-data-access.ts +++ b/hrm-domain/hrm-service/src/case/case-data-access.ts @@ -179,8 +179,8 @@ export const update = async ( caseRecordUpdates: Partial, accountSid: string, ): Promise => { - const result = await db.tx(async transaction => { - const caseUpdateSqlStatements = [selectSingleCaseByIdSql('Cases')]; + return db.tx(async transaction => { + const caseUpdateSqlStatements = []; const statementValues = { accountSid, caseId: id, @@ -202,16 +202,22 @@ export const update = async ( Object.assign(statementValues, values); caseUpdateSqlStatements.push(sql); } - caseUpdateSqlStatements.push(updateByIdSql(caseRecordUpdates)); - caseUpdateSqlStatements.push(selectSingleCaseByIdSql('Cases')); + const caseUpdateQuery = updateByIdSql(caseRecordUpdates); + // If there are preceding statements, put them in a CTE so we can run a single prepared statement + const fullUpdateQuery = caseUpdateSqlStatements.length + ? `WITH + ${caseUpdateSqlStatements.map((statement, i) => `q${i} AS (${statement})`).join(`, + `)} + ${caseUpdateQuery}` + : caseUpdateQuery; + await transaction.none(parameterizedQuery(fullUpdateQuery, statementValues)); // eslint-disable-next-line @typescript-eslint/no-unused-vars - const output = await transaction.multi( - caseUpdateSqlStatements.join(`; - `), - statementValues, + return transaction.oneOrNone( + parameterizedQuery(selectSingleCaseByIdSql('Cases'), { + accountSid, + caseId: id, + }), ); - return output.pop(); }); - return result.length ? result[0] : void 0; }; diff --git a/hrm-domain/hrm-service/src/case/sql/case-update-sql.ts b/hrm-domain/hrm-service/src/case/sql/case-update-sql.ts index 6ba655c60..f2ac47180 100644 --- a/hrm-domain/hrm-service/src/case/sql/case-update-sql.ts +++ b/hrm-domain/hrm-service/src/case/sql/case-update-sql.ts @@ -15,7 +15,6 @@ */ import { pgp } from '../../connection-pool'; -import { selectSingleCaseByIdSql } from './case-get-sql'; const VALID_CASE_UPDATE_FIELDS = ['info', 'status', 'updatedAt', 'updatedBy']; @@ -27,10 +26,7 @@ const updateCaseColumnSet = new pgp.helpers.ColumnSet( { table: 'Cases' }, ); -export const updateByIdSql = (updatedValues: Record) => `WITH updated AS ( +export const updateByIdSql = (updatedValues: Record) => ` ${pgp.helpers.update(updatedValues, updateCaseColumnSet)} WHERE "Cases"."accountSid" = $ AND "Cases"."id" = $ - RETURNING * - ) - ${selectSingleCaseByIdSql('updated')} `; diff --git a/hrm-domain/hrm-service/src/parameterizedQuery.ts b/hrm-domain/hrm-service/src/parameterizedQuery.ts index 9b035b65a..30126f1fb 100644 --- a/hrm-domain/hrm-service/src/parameterizedQuery.ts +++ b/hrm-domain/hrm-service/src/parameterizedQuery.ts @@ -93,7 +93,7 @@ export function convertToPostgreSQLQuery( csvValues.push(value ?? null); positions.push(`$${values.length + csvValues.length + valueCount}`); }); - console.debug(`Mapping $${positions.join(', ')} to ${key}, values: ${paramValue}`); + console.debug(`Mapping ${positions.join(', ')} to ${key}, values: ${paramValue}`); } return positions.join(', '); case ':json': @@ -131,7 +131,7 @@ export function convertToPostgreSQLQuery( }); } }); - + console.debug('Parameterized query:', query, values); return { query, values }; } diff --git a/resources-domain/resources-import-producer/index.ts b/resources-domain/resources-import-producer/index.ts index 74a308dfe..87f7fdf1a 100644 --- a/resources-domain/resources-import-producer/index.ts +++ b/resources-domain/resources-import-producer/index.ts @@ -65,7 +65,7 @@ export type KhpApiResponse = { const pullUpdates = (externalApiBaseUrl: URL, externalApiKey: string, externalApiAuthorizationHeader: string) => { const configuredPullUpdates = async (from: Date, to: Date, lastObjectId: string = '', limit = updateBatchSize): Promise => { - const response = await fetch(new URL(`api/resources?sort=updatedAt&fromDate=${from.toISOString()}&toDate=${to.toISOString()}&limit=${updateBatchSize}`, externalApiBaseUrl), { + const response = await fetch(new URL(`api/resources?sort=updatedAt&dateType=updatedAt&startDate=${from.toISOString()}&endDate=${to.toISOString()}&limit=${updateBatchSize}`, externalApiBaseUrl), { headers: { 'Authorization': externalApiAuthorizationHeader, 'x-api-key': externalApiKey, From cb7227082bbfcfb1db629fae323c87a98e417a00 Mon Sep 17 00:00:00 2001 From: Stephen Hand Date: Thu, 8 Jun 2023 12:50:46 +0100 Subject: [PATCH 05/10] Added tests for new package. Fixed unit tests. Moved another another case data access method to use PQs --- hrm-domain/hrm-service/package.json | 1 + .../hrm-service/src/case/case-data-access.ts | 7 +- .../src/case/sql/case-search-sql.ts | 11 +-- hrm-domain/hrm-service/src/connection-pool.ts | 12 ++- .../contact-job/contact-job-data-access.ts | 3 +- .../src/contact/contact-data-access.ts | 3 +- .../csam-report/csam-report-data-access.ts | 3 +- .../src/referral/referral-data-access.ts | 4 +- hrm-domain/hrm-service/src/search.ts | 2 +- .../unit-tests/case/case-data-access.test.ts | 90 ++++++++++--------- hrm-domain/hrm-service/unit-tests/mock-db.ts | 13 +-- .../referrals/referrals-data-access.test.ts | 12 +-- package-lock.json | 76 ++++++++++++---- packages/sql/jest.config.js | 31 +++++++ packages/sql/package.json | 30 +++++++ .../src/sql.ts => packages/sql/src/error.ts | 43 +-------- packages/sql/src/index.ts | 26 ++++++ packages/sql/src/ordering.ts | 24 +++++ .../sql}/src/parameterizedQuery.ts | 7 +- .../sql/tests/error.test.ts | 2 +- .../sql/tests}/parameterizedQuery.test.ts | 9 +- packages/sql/tsconfig.json | 7 ++ packages/testing/mock-pgpromise.ts | 6 +- tsconfig.build.service.json | 1 + tsconfig.json | 1 + 25 files changed, 287 insertions(+), 137 deletions(-) create mode 100644 packages/sql/jest.config.js create mode 100644 packages/sql/package.json rename hrm-domain/hrm-service/src/sql.ts => packages/sql/src/error.ts (53%) create mode 100644 packages/sql/src/index.ts create mode 100644 packages/sql/src/ordering.ts rename {hrm-domain/hrm-service => packages/sql}/src/parameterizedQuery.ts (96%) rename hrm-domain/hrm-service/unit-tests/sql.test.ts => packages/sql/tests/error.test.ts (99%) rename {hrm-domain/hrm-service/unit-tests => packages/sql/tests}/parameterizedQuery.test.ts (93%) create mode 100644 packages/sql/tsconfig.json diff --git a/hrm-domain/hrm-service/package.json b/hrm-domain/hrm-service/package.json index f93bbf8e4..41c78a5a6 100644 --- a/hrm-domain/hrm-service/package.json +++ b/hrm-domain/hrm-service/package.json @@ -45,6 +45,7 @@ "@tech-matters/twilio-client": "^1.0.0", "@tech-matters/http": "^1.0.0", "@tech-matters/resources-service": "^1.0.0", + "@tech-matters/sql": "^1.0.0", "@tech-matters/twilio-worker-auth": "^1.0.0", "@tech-matters/types": "^1.0.0", "aws-sdk": "^2.1231.0", diff --git a/hrm-domain/hrm-service/src/case/case-data-access.ts b/hrm-domain/hrm-service/src/case/case-data-access.ts index d58663c27..4c527a168 100644 --- a/hrm-domain/hrm-service/src/case/case-data-access.ts +++ b/hrm-domain/hrm-service/src/case/case-data-access.ts @@ -17,12 +17,13 @@ import { db, pgp } from '../connection-pool'; import { getPaginationElements } from '../search'; import { updateByIdSql } from './sql/case-update-sql'; -import { OrderByColumnType, OrderByDirectionType, selectCaseSearch } from './sql/case-search-sql'; +import { OrderByColumnType, selectCaseSearch } from './sql/case-search-sql'; import { caseSectionUpsertSql, deleteMissingCaseSectionsSql } from './sql/case-sections-sql'; import { DELETE_BY_ID } from './sql/case-delete-sql'; import { selectSingleCaseByIdSql } from './sql/case-get-sql'; import { Contact } from '../contact/contact-data-access'; -import { parameterizedQuery } from '../parameterizedQuery'; +import { parameterizedQuery } from '@tech-matters/sql'; +import { OrderByDirectionType } from '@tech-matters/sql/dist/ordering'; export type CaseRecordCommon = { info: any; @@ -134,7 +135,7 @@ export const getById = async ( return db.task(async connection => { const statement = selectSingleCaseByIdSql('Cases'); const queryValues = { accountSid, caseId }; - return connection.oneOrNone(statement, queryValues); + return connection.oneOrNone(parameterizedQuery(statement, queryValues)); }); }; diff --git a/hrm-domain/hrm-service/src/case/sql/case-search-sql.ts b/hrm-domain/hrm-service/src/case/sql/case-search-sql.ts index 5db417a55..2d324ac61 100644 --- a/hrm-domain/hrm-service/src/case/sql/case-search-sql.ts +++ b/hrm-domain/hrm-service/src/case/sql/case-search-sql.ts @@ -19,15 +19,8 @@ import { SELECT_CASE_SECTIONS } from './case-sections-sql'; import { CaseListFilters, DateExistsCondition, DateFilter } from '../case-data-access'; import { leftJoinCsamReportsOnFK } from '../../csam-report/sql/csam-report-get-sql'; import { leftJoinReferralsOnFK } from '../../referral/sql/referral-get-sql'; - -export const OrderByDirection = { - ascendingNullsLast: 'ASC NULLS LAST', - descendingNullsLast: 'DESC NULLS LAST', - ascending: 'ASC', - descending: 'DESC', -} as const; - -export type OrderByDirectionType = typeof OrderByDirection[keyof typeof OrderByDirection]; +import { OrderByDirection } from '@tech-matters/sql'; +import { OrderByDirectionType } from '@tech-matters/sql/dist/ordering'; export const OrderByColumn = { ID: 'id', diff --git a/hrm-domain/hrm-service/src/connection-pool.ts b/hrm-domain/hrm-service/src/connection-pool.ts index ff05857df..a43e6e22e 100644 --- a/hrm-domain/hrm-service/src/connection-pool.ts +++ b/hrm-domain/hrm-service/src/connection-pool.ts @@ -14,7 +14,7 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -import pgPromise from 'pg-promise'; +import pgPromise, { ITask } from 'pg-promise'; import config from './config/db'; export const pgp = pgPromise({}); @@ -24,3 +24,13 @@ export const db = pgp( config.host }:${config.port}/${encodeURIComponent(config.database)}?&application_name=hrm-service`, ); + +export const txIfNotInOne = async ( + task: ITask | undefined, + work: (y: ITask) => Promise, +): Promise => { + if (task) { + return task.txIf(work); + } + return db.tx(work); +}; diff --git a/hrm-domain/hrm-service/src/contact-job/contact-job-data-access.ts b/hrm-domain/hrm-service/src/contact-job/contact-job-data-access.ts index 2008e7a53..853597fea 100644 --- a/hrm-domain/hrm-service/src/contact-job/contact-job-data-access.ts +++ b/hrm-domain/hrm-service/src/contact-job/contact-job-data-access.ts @@ -14,7 +14,7 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { db, pgp } from '../connection-pool'; +import { db, pgp, txIfNotInOne } from '../connection-pool'; // eslint-disable-next-line prettier/prettier import type { Contact } from '../contact/contact-data-access'; import { @@ -29,7 +29,6 @@ import { UPDATE_JOB_CLEANUP_ACTIVE_SQL, UPDATE_JOB_CLEANUP_PENDING_SQL, } from './sql/contact-job-sql'; -import { txIfNotInOne } from '../sql'; import { ContactJobType } from '@tech-matters/types'; diff --git a/hrm-domain/hrm-service/src/contact/contact-data-access.ts b/hrm-domain/hrm-service/src/contact/contact-data-access.ts index 2708e44b2..fd7c7355b 100644 --- a/hrm-domain/hrm-service/src/contact/contact-data-access.ts +++ b/hrm-domain/hrm-service/src/contact/contact-data-access.ts @@ -13,7 +13,7 @@ * You should have received a copy of the GNU Affero General Public License * along with this program. If not, see https://www.gnu.org/licenses/. */ - +import { txIfNotInOne } from '../connection-pool'; import { db } from '../connection-pool'; import { UPDATE_CASEID_BY_ID, @@ -31,7 +31,6 @@ import { } from './contact-json'; // eslint-disable-next-line prettier/prettier import type { ITask } from 'pg-promise'; -import { txIfNotInOne } from '../sql'; type ExistingContactRecord = { id: number; diff --git a/hrm-domain/hrm-service/src/csam-report/csam-report-data-access.ts b/hrm-domain/hrm-service/src/csam-report/csam-report-data-access.ts index 5c03cc664..eec317cf8 100644 --- a/hrm-domain/hrm-service/src/csam-report/csam-report-data-access.ts +++ b/hrm-domain/hrm-service/src/csam-report/csam-report-data-access.ts @@ -14,7 +14,7 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { db } from '../connection-pool'; +import { db, txIfNotInOne } from '../connection-pool'; import { insertCSAMReportSql } from './sql/csam-report-insert-sql'; import { selectSingleCsamReportByIdSql, @@ -24,7 +24,6 @@ import { updateContactIdByCsamReportIdsSql, updateAcknowledgedByCsamReportIdSql, } from './sql/csam-report-update-sql'; -import { txIfNotInOne } from '../sql'; export type NewCSAMReport = { reportType: 'counsellor-generated' | 'self-generated'; diff --git a/hrm-domain/hrm-service/src/referral/referral-data-access.ts b/hrm-domain/hrm-service/src/referral/referral-data-access.ts index b975897e8..106e14454 100644 --- a/hrm-domain/hrm-service/src/referral/referral-data-access.ts +++ b/hrm-domain/hrm-service/src/referral/referral-data-access.ts @@ -20,8 +20,8 @@ import { DatabaseForeignKeyViolationError, DatabaseUniqueConstraintViolationError, inferPostgresError, - txIfNotInOne, -} from '../sql'; +} from '@tech-matters/sql'; +import { txIfNotInOne } from '../connection-pool'; // Working around the lack of a 'cause' property in the Error class for ES2020 - can be removed when we upgrade to ES2022 export class DuplicateReferralError extends Error { diff --git a/hrm-domain/hrm-service/src/search.ts b/hrm-domain/hrm-service/src/search.ts index 6c7490dc0..cdc937370 100644 --- a/hrm-domain/hrm-service/src/search.ts +++ b/hrm-domain/hrm-service/src/search.ts @@ -14,7 +14,7 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { OrderByDirection } from './sql'; +import { OrderByDirection } from '@tech-matters/sql'; export type PaginationQuery = { limit?: string; diff --git a/hrm-domain/hrm-service/unit-tests/case/case-data-access.test.ts b/hrm-domain/hrm-service/unit-tests/case/case-data-access.test.ts index 35e3f81ec..d5944da5c 100644 --- a/hrm-domain/hrm-service/unit-tests/case/case-data-access.test.ts +++ b/hrm-domain/hrm-service/unit-tests/case/case-data-access.test.ts @@ -21,7 +21,12 @@ import * as caseDb from '../../src/case/case-data-access'; import each from 'jest-each'; import { db } from '../../src/connection-pool'; import { OrderByColumn } from '../../src/case/sql/case-search-sql'; -import { expectValuesInSql, getSqlStatement } from '@tech-matters/testing'; +import { + expectValuesInSql, + getParameterizedSqlStatement, + getSqlStatement, +} from '@tech-matters/testing'; +import { ParameterizedQuery } from 'pg-promise'; const accountSid = 'account-sid'; let conn: pgPromise.ITask; @@ -46,10 +51,11 @@ describe('getById', () => { const result = await caseDb.getById(caseId, accountSid); - expect(oneOrNoneSpy).toHaveBeenCalledWith( - expect.stringContaining('Cases'), - expect.objectContaining({ accountSid, caseId }), - ); + expect(oneOrNoneSpy).toHaveBeenCalledWith(expect.any(ParameterizedQuery)); + const px = oneOrNoneSpy.mock.calls[0][0] as ParameterizedQuery; + expect(px.text).toContain('Cases'); + + expect(px.values).toStrictEqual([accountSid, caseId]); expect(result).toStrictEqual(caseFromDB); }); @@ -59,10 +65,11 @@ describe('getById', () => { const result = await caseDb.getById(caseId, accountSid); - expect(oneOrNoneSpy).toHaveBeenCalledWith( - expect.stringContaining('CSAMReports'), - expect.objectContaining({ accountSid, caseId }), - ); + expect(oneOrNoneSpy).toHaveBeenCalledWith(expect.any(ParameterizedQuery)); + const px = oneOrNoneSpy.mock.calls[0][0] as ParameterizedQuery; + expect(px.text).toContain('CSAMReports'); + + expect(px.values).toStrictEqual([accountSid, caseId]); expect(result).not.toBeDefined(); }); }); @@ -217,13 +224,15 @@ describe('search', () => { { ...createMockCaseRecord({ id: 2 }), totalCount: 1337 }, { ...createMockCaseRecord({ id: 1 }), totalCount: 1337 }, ]; - const anySpy = jest.spyOn(conn, 'any').mockResolvedValue(dbResult); + const manyOrNoneSpy = jest.spyOn(conn, 'manyOrNone').mockResolvedValue(dbResult); const result = await caseDb.search(listConfig, accountSid, {}, filters); - expect(anySpy).toHaveBeenCalledWith( - expect.stringContaining('Cases'), - expect.objectContaining({ ...expectedDbParameters, accountSid }), - ); - const sql = getSqlStatement(anySpy); + expect(manyOrNoneSpy).toHaveBeenCalledWith(expect.any(ParameterizedQuery)); + const pq = manyOrNoneSpy.mock.calls[0][0] as ParameterizedQuery; + expect(pq.text).toContain('Cases'); + Object.values({ ...expectedDbParameters, accountSid }).forEach(expected => { + expect(pq.values).toContainEqual(expected); + }); + const sql = getParameterizedSqlStatement(manyOrNoneSpy); expectedInSql.forEach(expected => { expect(sql).toContain(expected); }); @@ -292,19 +301,22 @@ describe('search', () => { dbResult, expectedResult = dbResult, }) => { - const anySpy = jest.spyOn(conn, 'any').mockResolvedValue(dbResult); + const manyOrNoneSpy = jest.spyOn(conn, 'manyOrNone').mockResolvedValue(dbResult); const result = await caseDb.search(listConfig, accountSid, {}, filters); - expect(anySpy).toHaveBeenCalledWith( - expect.stringContaining('Cases'), - expect.objectContaining({ - contactNumber: null, - firstName: null, - lastName: null, - ...expectedDbParameters, - accountSid, - }), - ); - const statementExecuted = getSqlStatement(anySpy); + expect(manyOrNoneSpy).toHaveBeenCalledWith(expect.any(ParameterizedQuery)); + const pq = manyOrNoneSpy.mock.calls[0][0] as ParameterizedQuery; + + expect(pq.text).toContain('Cases'); + Object.values({ + contactNumber: null, + firstName: null, + lastName: null, + ...expectedDbParameters, + accountSid, + }).forEach(expected => { + expect(pq.values).toContainEqual(expected); + }); + const statementExecuted = getParameterizedSqlStatement(manyOrNoneSpy); expect(statementExecuted).toContain('Contacts'); expect(statementExecuted).toContain('CSAMReports'); expect(result.count).toEqual(1337); @@ -331,22 +343,20 @@ describe('update', () => { test('runs update SQL against cases table with provided ID.', async () => { const caseUpdateResult = createMockCaseRecord(caseUpdate); - const multiSpy = jest.spyOn(tx, 'multi').mockResolvedValue([ - [{ ...createMockCaseRecord({}), id: caseId }], - [2], //Simulate outputs from caseSection queries - [{ ...caseUpdateResult, id: caseId }], - ]); - + const oneOrNoneSpy = jest + .spyOn(tx, 'oneOrNone') + .mockResolvedValue({ ...caseUpdateResult, id: caseId }); + const noneSpy = jest.spyOn(tx, 'none'); const result = await caseDb.update(caseId, caseUpdate, accountSid); - const updateSql = getSqlStatement(multiSpy); + const updateSql = getParameterizedSqlStatement(noneSpy); + const selectSql = getParameterizedSqlStatement(oneOrNoneSpy); expect(updateSql).toContain('Cases'); - expect(updateSql).toContain('Contacts'); - expect(updateSql).toContain('CSAMReports'); + expect(selectSql).toContain('Cases'); + expect(selectSql).toContain('Contacts'); + expect(selectSql).toContain('CSAMReports'); expectValuesInSql(updateSql, { info: caseUpdate.info, status: caseUpdate.status }); - expect(multiSpy).toHaveBeenCalledWith( - expect.any(String), - expect.objectContaining({ accountSid, caseId }), - ); + expect(noneSpy).toHaveBeenCalledWith(expect.any(ParameterizedQuery)); + expect(oneOrNoneSpy).toHaveBeenCalledWith(expect.any(ParameterizedQuery)); expect(result).toStrictEqual({ ...caseUpdateResult, id: caseId }); }); }); diff --git a/hrm-domain/hrm-service/unit-tests/mock-db.ts b/hrm-domain/hrm-service/unit-tests/mock-db.ts index 1735d03e8..61e21b273 100644 --- a/hrm-domain/hrm-service/unit-tests/mock-db.ts +++ b/hrm-domain/hrm-service/unit-tests/mock-db.ts @@ -18,11 +18,14 @@ import * as pgPromise from 'pg-promise'; // eslint-disable-next-line import/no-extraneous-dependencies import * as pgMocking from '@tech-matters/testing'; import { db } from '../src/connection-pool'; - -jest.mock('../src/connection-pool', () => ({ - db: pgMocking.createMockConnection(), - pgp: jest.requireActual('../src/connection-pool').pgp, -})); +jest.mock('../src/connection-pool', () => { + const mockDb = pgMocking.createMockConnection(); + return { + db: mockDb, + pgp: jest.requireActual('../src/connection-pool').pgp, + txIfNotInOne: jest.fn().mockImplementation((tx, work) => (tx ?? mockDb).tx(work)), + }; +}); export const mockConnection = pgMocking.createMockConnection; diff --git a/hrm-domain/hrm-service/unit-tests/referrals/referrals-data-access.test.ts b/hrm-domain/hrm-service/unit-tests/referrals/referrals-data-access.test.ts index 2e87918d0..d3a606596 100644 --- a/hrm-domain/hrm-service/unit-tests/referrals/referrals-data-access.test.ts +++ b/hrm-domain/hrm-service/unit-tests/referrals/referrals-data-access.test.ts @@ -16,18 +16,18 @@ import * as pgPromise from 'pg-promise'; import { subHours } from 'date-fns'; +import { + DatabaseForeignKeyViolationError, + DatabaseUniqueConstraintViolationError, + DatabaseError, +} from '@tech-matters/sql'; +import { getSqlStatement } from '@tech-matters/testing'; import { mockConnection, mockTransaction } from '../mock-db'; import * as referralDb from '../../src/referral/referral-data-access'; import { DuplicateReferralError, OrphanedReferralError, } from '../../src/referral/referral-data-access'; -import { - DatabaseForeignKeyViolationError, - DatabaseUniqueConstraintViolationError, - DatabaseError, -} from '../../src/sql'; -import { getSqlStatement } from '@tech-matters/testing'; let conn: pgPromise.ITask; diff --git a/package-lock.json b/package-lock.json index 6ccf78711..aefb75d8d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -120,6 +120,7 @@ "dependencies": { "@tech-matters/http": "^1.0.0", "@tech-matters/resources-service": "^1.0.0", + "@tech-matters/sql": "^1.0.0", "@tech-matters/ssm-cache": "^1.0.0", "@tech-matters/twilio-client": "^1.0.0", "@tech-matters/twilio-worker-auth": "^1.0.0", @@ -4992,10 +4993,6 @@ "resolved": "hrm-domain/hrm-service", "link": true }, - "node_modules/@tech-matters/hrm-ssm-cache": { - "resolved": "packages/ssm-cache", - "link": true - }, "node_modules/@tech-matters/http": { "resolved": "packages/http", "link": true @@ -5024,6 +5021,10 @@ "resolved": "packages/sns-client", "link": true }, + "node_modules/@tech-matters/sql": { + "resolved": "packages/sql", + "link": true + }, "node_modules/@tech-matters/ssm-cache": { "resolved": "packages/ssm-cache", "link": true @@ -13520,6 +13521,34 @@ "aws-sdk": "^2.1225.0" } }, + "packages/sql": { + "version": "1.0.0", + "license": "AGPL", + "dependencies": { + "cors": "^2.8.5", + "express": "^4.17.1", + "http-errors": "^2.0.0", + "morgan": "^1.10.0" + }, + "devDependencies": { + "@types/morgan": "^1.9.4" + } + }, + "packages/sql/node_modules/http-errors": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-2.0.0.tgz", + "integrity": "sha512-FtwrG/euBzaEjYeRqOgly7G0qviiXoJWnvEH2Z1plBdXgbyjv34pHTSb9zoeHMyDy33+DWy5Wt9Wo+TURtOYSQ==", + "dependencies": { + "depd": "2.0.0", + "inherits": "2.0.4", + "setprototypeof": "1.2.0", + "statuses": "2.0.1", + "toidentifier": "1.0.1" + }, + "engines": { + "node": ">= 0.8" + } + }, "packages/ssm-cache": { "name": "@tech-matters/ssm-cache", "version": "1.0.0", @@ -13600,7 +13629,7 @@ "version": "0.0.1", "license": "AGPL", "dependencies": { - "@tech-matters/hrm-ssm-cache": "^1.0.0", + "@tech-matters/ssm-cache": "^1.0.0", "aws-sdk": "^2.1225.0", "date-fns": "^2.30.0", "lodash": "^4.17.21" @@ -16733,6 +16762,7 @@ "requires": { "@tech-matters/http": "^1.0.0", "@tech-matters/resources-service": "^1.0.0", + "@tech-matters/sql": "^1.0.0", "@tech-matters/ssm-cache": "^1.0.0", "@tech-matters/testing": "^1.0.0", "@tech-matters/twilio-client": "^1.0.0", @@ -16787,14 +16817,6 @@ } } }, - "@tech-matters/hrm-ssm-cache": { - "version": "file:packages/ssm-cache", - "requires": { - "aws-sdk": "^2.1225.0", - "date-fns": "^2.29.3", - "typescript": "^4.8.4" - } - }, "@tech-matters/http": { "version": "file:packages/http", "requires": { @@ -16823,7 +16845,7 @@ "@tech-matters/resources-import-producer": { "version": "file:resources-domain/resources-import-producer", "requires": { - "@tech-matters/hrm-ssm-cache": "^1.0.0", + "@tech-matters/ssm-cache": "^1.0.0", "@tech-matters/types": "^1.0.0", "@types/aws-lambda": "^8.10.108", "@types/lodash": "^4.14.194", @@ -16890,7 +16912,7 @@ "http-errors": "^1.7.3", "jest-each": "^29.2.1", "pg-promise": "^10.15.4", - "pg-query-stream": "*", + "pg-query-stream": "^4.5.0", "sequelize": "^6.28.0", "sequelize-cli": "^6.5.2", "sqslite": "^2.1.1", @@ -16913,6 +16935,30 @@ "aws-sdk": "^2.1225.0" } }, + "@tech-matters/sql": { + "version": "file:packages/sql", + "requires": { + "@types/morgan": "^1.9.4", + "cors": "^2.8.5", + "express": "^4.17.1", + "http-errors": "^2.0.0", + "morgan": "^1.10.0" + }, + "dependencies": { + "http-errors": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-2.0.0.tgz", + "integrity": "sha512-FtwrG/euBzaEjYeRqOgly7G0qviiXoJWnvEH2Z1plBdXgbyjv34pHTSb9zoeHMyDy33+DWy5Wt9Wo+TURtOYSQ==", + "requires": { + "depd": "2.0.0", + "inherits": "2.0.4", + "setprototypeof": "1.2.0", + "statuses": "2.0.1", + "toidentifier": "1.0.1" + } + } + } + }, "@tech-matters/ssm-cache": { "version": "file:packages/ssm-cache", "requires": { diff --git a/packages/sql/jest.config.js b/packages/sql/jest.config.js new file mode 100644 index 000000000..9a48d1257 --- /dev/null +++ b/packages/sql/jest.config.js @@ -0,0 +1,31 @@ +/** + * Copyright (C) 2021-2023 Technology Matters + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://www.gnu.org/licenses/. + */ + +module.exports = config => { + return ( + config || { + preset: 'ts-jest', + rootDir: './', + maxWorkers: 1, + globals: { + 'ts-jest': { + // to give support to const enum. Not working, conflicting with module resolution + useExperimentalLanguageServer: true, + }, + }, + } + ); +}; diff --git a/packages/sql/package.json b/packages/sql/package.json new file mode 100644 index 000000000..d3bd21837 --- /dev/null +++ b/packages/sql/package.json @@ -0,0 +1,30 @@ +{ + "name": "@tech-matters/sql", + "version": "1.0.0", + "description": "SQL lib to help Aselo service conform to common practices when using pg-promise", + "author": "", + "license": "AGPL", + "main": "dist/index.js", + "dependencies": { + "cors": "^2.8.5", + "express": "^4.17.1", + "http-errors": "^2.0.0", + "morgan": "^1.10.0", + "pg-promise": "^10.11.1" + }, + "scripts": { + "test:unit": "jest tests/unit" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/techmatters/hrm.git" + }, + "keywords": [], + "bugs": { + "url": "https://github.com/techmatters/hrm/issues" + }, + "homepage": "https://github.com/techmatters/hrm#readme", + "devDependencies": { + "@types/morgan": "^1.9.4" + } +} diff --git a/hrm-domain/hrm-service/src/sql.ts b/packages/sql/src/error.ts similarity index 53% rename from hrm-domain/hrm-service/src/sql.ts rename to packages/sql/src/error.ts index 3d9d75789..62b3df619 100644 --- a/hrm-domain/hrm-service/src/sql.ts +++ b/packages/sql/src/error.ts @@ -1,28 +1,3 @@ -/** - * Copyright (C) 2021-2023 Technology Matters - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as published - * by the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see https://www.gnu.org/licenses/. - */ -import { ITask } from 'pg-promise'; -import { db } from './connection-pool'; - -export const OrderByDirection = { - ascendingNullsLast: 'ASC NULLS LAST', - descendingNullsLast: 'DESC NULLS LAST', - ascending: 'ASC', - descending: 'DESC', -} as const; - export class DatabaseError extends Error { cause: Error; @@ -39,7 +14,7 @@ export class DatabaseConstraintViolationError extends DatabaseError { constraint: string; - constructor(error, table, constraint) { + constructor(error: Error, table: string, constraint: string) { super(error); this.name = 'DatabaseConstraintViolationError'; Object.setPrototypeOf(this, DatabaseConstraintViolationError.prototype); @@ -49,15 +24,15 @@ export class DatabaseConstraintViolationError extends DatabaseError { } export class DatabaseForeignKeyViolationError extends DatabaseConstraintViolationError { - constructor(message, table, constraint) { - super(message, table, constraint); + constructor(error: string | Error, table: string, constraint: string) { + super(typeof error === 'string' ? Error(error) : error, table, constraint); this.name = 'DatabaseForeignKeyViolationError'; Object.setPrototypeOf(this, DatabaseForeignKeyViolationError.prototype); } } export class DatabaseUniqueConstraintViolationError extends DatabaseConstraintViolationError { - constructor(error, table, constraint) { + constructor(error: Error, table: string, constraint: string) { super(error, table, constraint); this.name = 'DatabaseUniqueConstraintViolationError'; Object.setPrototypeOf(this, DatabaseUniqueConstraintViolationError.prototype); @@ -79,13 +54,3 @@ export const inferPostgresError = (rawError: Error): DatabaseError => { return new DatabaseError(rawError); } }; - -export const txIfNotInOne = async ( - task: ITask | undefined, - work: (y: ITask) => Promise, -): Promise => { - if (task) { - return task.txIf(work); - } - return db.tx(work); -}; diff --git a/packages/sql/src/index.ts b/packages/sql/src/index.ts new file mode 100644 index 000000000..a5042ff6b --- /dev/null +++ b/packages/sql/src/index.ts @@ -0,0 +1,26 @@ +/** + * Copyright (C) 2021-2023 Technology Matters + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://www.gnu.org/licenses/. + */ + +export { parameterizedQuery } from './parameterizedQuery'; +export { + DatabaseUniqueConstraintViolationError, + DatabaseForeignKeyViolationError, + DatabaseError, + DatabaseConstraintViolationError, + inferPostgresError, +} from './error'; + +export { OrderByDirection } from './ordering'; diff --git a/packages/sql/src/ordering.ts b/packages/sql/src/ordering.ts new file mode 100644 index 000000000..105cc44cd --- /dev/null +++ b/packages/sql/src/ordering.ts @@ -0,0 +1,24 @@ +/** + * Copyright (C) 2021-2023 Technology Matters + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://www.gnu.org/licenses/. + */ + +export const OrderByDirection = { + ascendingNullsLast: 'ASC NULLS LAST', + descendingNullsLast: 'DESC NULLS LAST', + ascending: 'ASC', + descending: 'DESC', +} as const; + +export type OrderByDirectionType = typeof OrderByDirection[keyof typeof OrderByDirection]; diff --git a/hrm-domain/hrm-service/src/parameterizedQuery.ts b/packages/sql/src/parameterizedQuery.ts similarity index 96% rename from hrm-domain/hrm-service/src/parameterizedQuery.ts rename to packages/sql/src/parameterizedQuery.ts index 30126f1fb..1e7e481fb 100644 --- a/hrm-domain/hrm-service/src/parameterizedQuery.ts +++ b/packages/sql/src/parameterizedQuery.ts @@ -14,7 +14,8 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { ParameterizedQuery } from 'pg-promise'; +// eslint-disable-next-line prettier/prettier +import type { ParameterizedQuery } from 'pg-promise'; function isJSONValue(token: string, sql: string): boolean { // If any of the tokens mark it as JSON, treat it as JSON everywhere @@ -83,8 +84,8 @@ export function convertToPostgreSQLQuery( values.push(...nestedResult.values); } else if (Array.isArray(paramValue)) { let singleValueAdded = false; - const positions = []; - const csvValues = []; + const positions: string[] = []; + const csvValues: string[] = []; query = query.replace(tokenRegex, (match, formatSpecifier) => { switch (formatSpecifier) { case ':csv': diff --git a/hrm-domain/hrm-service/unit-tests/sql.test.ts b/packages/sql/tests/error.test.ts similarity index 99% rename from hrm-domain/hrm-service/unit-tests/sql.test.ts rename to packages/sql/tests/error.test.ts index 34128b6e4..74084883a 100644 --- a/hrm-domain/hrm-service/unit-tests/sql.test.ts +++ b/packages/sql/tests/error.test.ts @@ -19,7 +19,7 @@ import { DatabaseUniqueConstraintViolationError, DatabaseError, inferPostgresError, -} from '../src/sql'; +} from '../src'; describe('inferPostgresError', () => { test('Error with code property set to 23503 - creates DatabaseKeyViolationError, wrapping original and copying table and constraint properties', () => { diff --git a/hrm-domain/hrm-service/unit-tests/parameterizedQuery.test.ts b/packages/sql/tests/parameterizedQuery.test.ts similarity index 93% rename from hrm-domain/hrm-service/unit-tests/parameterizedQuery.test.ts rename to packages/sql/tests/parameterizedQuery.test.ts index 69345aa49..f488b67e1 100644 --- a/hrm-domain/hrm-service/unit-tests/parameterizedQuery.test.ts +++ b/packages/sql/tests/parameterizedQuery.test.ts @@ -1,6 +1,6 @@ -import { convertToPostgreSQLQuery } from '../src/parameterizedQuery'; +import { parameterizedQuery } from '../dist'; -describe('convertToPostgreSQLQuery', () => { +describe('parameterizedQuery', () => { const testCases = [ { description: @@ -90,9 +90,8 @@ describe('convertToPostgreSQLQuery', () => { ]; test.each(testCases)('$description', ({ sql, params, expectedQuery, expectedValues }) => { - const { query, values } = convertToPostgreSQLQuery(sql, params); - - expect(query).toBe(expectedQuery); + const { text, values } = parameterizedQuery(sql, params); + expect(text).toBe(expectedQuery); expect(values).toEqual(expectedValues); }); }); diff --git a/packages/sql/tsconfig.json b/packages/sql/tsconfig.json new file mode 100644 index 000000000..2cab8eff0 --- /dev/null +++ b/packages/sql/tsconfig.json @@ -0,0 +1,7 @@ +{ + "extends": "../tsconfig.packages-base.json", + "include": ["src/**/*.ts"], + "compilerOptions": { + "outDir": "./dist" + } +} diff --git a/packages/testing/mock-pgpromise.ts b/packages/testing/mock-pgpromise.ts index 36990e8d2..c7d30552c 100644 --- a/packages/testing/mock-pgpromise.ts +++ b/packages/testing/mock-pgpromise.ts @@ -15,7 +15,7 @@ */ import * as pgPromise from 'pg-promise'; -import { IDatabase, QueryParam } from 'pg-promise'; +import { IDatabase, ParameterizedQuery, QueryParam } from 'pg-promise'; export function createMockConnection(): pgPromise.ITask { return { @@ -94,6 +94,10 @@ export const getSqlStatement = (mockQueryMethod: PgQuerySpy, callIndex = -1): st return mockQueryMethod.mock.calls[callIndex < 0 ? mockQueryMethod.mock.calls.length + callIndex : callIndex][0].toString(); }; +export const getParameterizedSqlStatement = (mockQueryMethod: PgQuerySpy, callIndex = -1): string => { + expect(mockQueryMethod).toHaveBeenCalled(); + return (mockQueryMethod.mock.calls[callIndex < 0 ? mockQueryMethod.mock.calls.length + callIndex : callIndex][0] as ParameterizedQuery).text.toString(); +}; export const getSqlStatementFromNone = (mockQueryMethod: jest.SpyInstance, callIndex = -1): string => { expect(mockQueryMethod).toHaveBeenCalled(); diff --git a/tsconfig.build.service.json b/tsconfig.build.service.json index 62d6a1f54..10ef72ae7 100644 --- a/tsconfig.build.service.json +++ b/tsconfig.build.service.json @@ -7,6 +7,7 @@ { "path": "packages/sns-client" }, { "path": "packages/ssm-cache" }, { "path": "packages/elasticsearch-client" }, + { "path": "packages/sql" }, { "path": "packages/twilio-client" }, { "path": "packages/types" }, { "path": "resources-domain/resources-service" }, diff --git a/tsconfig.json b/tsconfig.json index 9340ea8a8..cc299c2b8 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -13,6 +13,7 @@ { "path": "packages/sns-client" }, { "path": "packages/twilio-client" }, { "path": "packages/twilio-worker-auth" }, + { "path": "packages/sql" }, { "path": "hrm-domain/contact-complete" }, { "path": "hrm-domain/contact-retrieve-recording-url" }, { "path": "hrm-domain/contact-retrieve-transcript" }, From edd747ceb388ff08adef9d2ed90f3ae92c06c505 Mon Sep 17 00:00:00 2001 From: Stephen Hand Date: Thu, 8 Jun 2023 13:15:51 +0100 Subject: [PATCH 06/10] Migrate remaining case queries to using PQs --- .../hrm-service/src/case/case-data-access.ts | 12 ++++++----- .../unit-tests/case/case-data-access.test.ts | 21 +++++++++---------- packages/sql/src/parameterizedQuery.ts | 5 ++--- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/hrm-domain/hrm-service/src/case/case-data-access.ts b/hrm-domain/hrm-service/src/case/case-data-access.ts index 4c527a168..245e05d43 100644 --- a/hrm-domain/hrm-service/src/case/case-data-access.ts +++ b/hrm-domain/hrm-service/src/case/case-data-access.ts @@ -24,6 +24,7 @@ import { selectSingleCaseByIdSql } from './sql/case-get-sql'; import { Contact } from '../contact/contact-data-access'; import { parameterizedQuery } from '@tech-matters/sql'; import { OrderByDirectionType } from '@tech-matters/sql/dist/ordering'; +import { ParameterizedQuery } from 'pg-promise'; export type CaseRecordCommon = { info: any; @@ -116,11 +117,12 @@ export const create = async ( let inserted: CaseRecord = await transaction.one(statement); if ((caseSections ?? []).length) { const allSections = caseSections.map(s => ({ ...s, caseId: inserted.id, accountSid })); - const sectionStatement = `${caseSectionUpsertSql(allSections)};${selectSingleCaseByIdSql( - 'Cases', - )}`; + const queryValues = { accountSid, caseId: inserted.id }; - inserted = await transaction.one(sectionStatement, queryValues); + await transaction.none(caseSectionUpsertSql(allSections)); + inserted = await transaction.one( + parameterizedQuery(selectSingleCaseByIdSql('Cases'), queryValues), + ); } return inserted; @@ -172,7 +174,7 @@ export const search = async ( }; export const deleteById = async (id, accountSid) => { - return db.oneOrNone(DELETE_BY_ID, [accountSid, id]); + return db.oneOrNone(new ParameterizedQuery({ text: DELETE_BY_ID, values: [accountSid, id] })); }; export const update = async ( diff --git a/hrm-domain/hrm-service/unit-tests/case/case-data-access.test.ts b/hrm-domain/hrm-service/unit-tests/case/case-data-access.test.ts index d5944da5c..1df2bfe1a 100644 --- a/hrm-domain/hrm-service/unit-tests/case/case-data-access.test.ts +++ b/hrm-domain/hrm-service/unit-tests/case/case-data-access.test.ts @@ -102,7 +102,7 @@ describe('createCase', () => { twilioWorkerId: 'twilio-worker-id', }); const oneSpy = jest.spyOn(tx, 'one').mockResolvedValue({ ...caseFromDB, id: 1337 }); - + const noneSpy = jest.spyOn(tx, 'none'); const result = await caseDb.create(caseFromDB, accountSid); const insertSql = getSqlStatement(oneSpy, -2); const { caseSections, ...caseWithoutSections } = caseFromDB; @@ -112,7 +112,7 @@ describe('createCase', () => { createdAt: expect.anything(), updatedAt: expect.anything(), }); - const insertSectionSql = getSqlStatement(oneSpy, -1); + const insertSectionSql = getSqlStatement(noneSpy); expectValuesInSql(insertSectionSql, { ...caseSections[0], caseId: 1337, @@ -368,11 +368,10 @@ describe('delete', () => { const oneOrNoneSpy = jest.spyOn(db, 'oneOrNone').mockResolvedValue(caseFromDB); const result = await caseDb.deleteById(caseId, accountSid); - - expect(oneOrNoneSpy).toHaveBeenCalledWith(expect.stringContaining('Cases'), [ - accountSid, - caseId, - ]); + expect(oneOrNoneSpy).toHaveBeenCalledWith(expect.any(ParameterizedQuery)); + const pq = oneOrNoneSpy.mock.calls[0][0] as ParameterizedQuery; + expect(pq.text).toContain('Cases'); + expect(pq.values).toStrictEqual([accountSid, caseId]); expect(result).toStrictEqual(caseFromDB); }); test('returns nothing if nothing at the specified ID exists to delete', async () => { @@ -381,10 +380,10 @@ describe('delete', () => { const result = await caseDb.deleteById(caseId, accountSid); - expect(oneOrNoneSpy).toHaveBeenCalledWith(expect.stringContaining('Cases'), [ - accountSid, - caseId, - ]); + expect(oneOrNoneSpy).toHaveBeenCalledWith(expect.any(ParameterizedQuery)); + const pq = oneOrNoneSpy.mock.calls[0][0] as ParameterizedQuery; + expect(pq.text).toContain('Cases'); + expect(pq.values).toStrictEqual([accountSid, caseId]); expect(result).not.toBeDefined(); }); }); diff --git a/packages/sql/src/parameterizedQuery.ts b/packages/sql/src/parameterizedQuery.ts index 1e7e481fb..de591c98e 100644 --- a/packages/sql/src/parameterizedQuery.ts +++ b/packages/sql/src/parameterizedQuery.ts @@ -14,8 +14,7 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -// eslint-disable-next-line prettier/prettier -import type { ParameterizedQuery } from 'pg-promise'; +import { ParameterizedQuery } from 'pg-promise'; function isJSONValue(token: string, sql: string): boolean { // If any of the tokens mark it as JSON, treat it as JSON everywhere @@ -132,7 +131,7 @@ export function convertToPostgreSQLQuery( }); } }); - console.debug('Parameterized query:', query, values); + // console.debug('Parameterized query:', query, values); return { query, values }; } From cc9953b5a38dd9b80c936cfd56d1b6148e67c4d2 Mon Sep 17 00:00:00 2001 From: Stephen Hand Date: Fri, 16 Jun 2023 09:16:57 +0100 Subject: [PATCH 07/10] Fix merge --- .../hrm-service/src/case/case-data-access.ts | 3 +-- package-lock.json | 7 +++++-- packages/sql/src/error.ts | 16 ++++++++++++++++ packages/sql/src/index.ts | 2 +- packages/sql/tests/parameterizedQuery.test.ts | 16 ++++++++++++++++ 5 files changed, 39 insertions(+), 5 deletions(-) diff --git a/hrm-domain/hrm-service/src/case/case-data-access.ts b/hrm-domain/hrm-service/src/case/case-data-access.ts index 478322a45..2747af492 100644 --- a/hrm-domain/hrm-service/src/case/case-data-access.ts +++ b/hrm-domain/hrm-service/src/case/case-data-access.ts @@ -22,8 +22,7 @@ import { caseSectionUpsertSql, deleteMissingCaseSectionsSql } from './sql/case-s import { DELETE_BY_ID } from './sql/case-delete-sql'; import { selectSingleCaseByIdSql } from './sql/case-get-sql'; import { Contact } from '../contact/contact-data-access'; -import { parameterizedQuery } from '@tech-matters/sql'; -import { OrderByDirectionType } from '@tech-matters/sql/dist/ordering'; +import { parameterizedQuery, OrderByDirectionType } from '@tech-matters/sql'; import { ParameterizedQuery } from 'pg-promise'; export type CaseRecordCommon = { diff --git a/package-lock.json b/package-lock.json index 19ac8e37d..1f906bca1 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13533,13 +13533,15 @@ } }, "packages/sql": { + "name": "@tech-matters/sql", "version": "1.0.0", "license": "AGPL", "dependencies": { "cors": "^2.8.5", "express": "^4.17.1", "http-errors": "^2.0.0", - "morgan": "^1.10.0" + "morgan": "^1.10.0", + "pg-promise": "^10.11.1" }, "devDependencies": { "@types/morgan": "^1.9.4" @@ -16998,7 +17000,8 @@ "cors": "^2.8.5", "express": "^4.17.1", "http-errors": "^2.0.0", - "morgan": "^1.10.0" + "morgan": "^1.10.0", + "pg-promise": "^10.11.1" }, "dependencies": { "http-errors": { diff --git a/packages/sql/src/error.ts b/packages/sql/src/error.ts index 62b3df619..61d3c8d1d 100644 --- a/packages/sql/src/error.ts +++ b/packages/sql/src/error.ts @@ -1,3 +1,19 @@ +/** + * Copyright (C) 2021-2023 Technology Matters + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://www.gnu.org/licenses/. + */ + export class DatabaseError extends Error { cause: Error; diff --git a/packages/sql/src/index.ts b/packages/sql/src/index.ts index a5042ff6b..b2e5218a6 100644 --- a/packages/sql/src/index.ts +++ b/packages/sql/src/index.ts @@ -23,4 +23,4 @@ export { inferPostgresError, } from './error'; -export { OrderByDirection } from './ordering'; +export { OrderByDirection, OrderByDirectionType } from './ordering'; diff --git a/packages/sql/tests/parameterizedQuery.test.ts b/packages/sql/tests/parameterizedQuery.test.ts index f488b67e1..6929999eb 100644 --- a/packages/sql/tests/parameterizedQuery.test.ts +++ b/packages/sql/tests/parameterizedQuery.test.ts @@ -1,3 +1,19 @@ +/** + * Copyright (C) 2021-2023 Technology Matters + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published + * by the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see https://www.gnu.org/licenses/. + */ + import { parameterizedQuery } from '../dist'; describe('parameterizedQuery', () => { From 69f7d5aaa165d5b94801018b1c88e9de4dedf2e9 Mon Sep 17 00:00:00 2001 From: Stephen Hand Date: Fri, 16 Jun 2023 09:21:57 +0100 Subject: [PATCH 08/10] Fix import path --- packages/sql/tests/parameterizedQuery.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/sql/tests/parameterizedQuery.test.ts b/packages/sql/tests/parameterizedQuery.test.ts index 6929999eb..5d87a11e5 100644 --- a/packages/sql/tests/parameterizedQuery.test.ts +++ b/packages/sql/tests/parameterizedQuery.test.ts @@ -14,7 +14,7 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { parameterizedQuery } from '../dist'; +import { parameterizedQuery } from '../src'; describe('parameterizedQuery', () => { const testCases = [ From 9f929cc54637a0c3eb044ce7c6eb9b51235b1d4f Mon Sep 17 00:00:00 2001 From: Stephen Hand Date: Fri, 16 Jun 2023 09:37:04 +0100 Subject: [PATCH 09/10] Fix test merging issue --- .../tests/unit/handler.test.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/resources-domain/resources-import-producer/tests/unit/handler.test.ts b/resources-domain/resources-import-producer/tests/unit/handler.test.ts index 8acdebc06..e2e54af61 100644 --- a/resources-domain/resources-import-producer/tests/unit/handler.test.ts +++ b/resources-domain/resources-import-producer/tests/unit/handler.test.ts @@ -125,8 +125,8 @@ const testCases: HandlerTestCase[] = [ externalApiResponse: { data: [], totalResults: 0 }, expectedExternalApiCallParameters: [ { - fromDate: new Date(0).toISOString(), - toDate: testNow.toISOString(), + startDate: new Date(0).toISOString(), + endDate: testNow.toISOString(), limit: '1000', }, ], @@ -140,8 +140,8 @@ const testCases: HandlerTestCase[] = [ ], totalResults:2 }, expectedExternalApiCallParameters: [ { - fromDate:new Date(0).toISOString(), - toDate:testNow.toISOString(), + startDate:new Date(0).toISOString(), + endDate:testNow.toISOString(), limit: '1000', }, ], @@ -158,8 +158,8 @@ const testCases: HandlerTestCase[] = [ ], totalResults:2 }, expectedExternalApiCallParameters: [ { - fromDate:addMilliseconds(subHours(testNow, 1), 1).toISOString(), - toDate:testNow.toISOString(), + startDate:addMilliseconds(subHours(testNow, 1), 1).toISOString(), + endDate:testNow.toISOString(), limit: '1000', }, ], From c52b853d5b4b468c369d234575f8cefb76876ba9 Mon Sep 17 00:00:00 2001 From: Stephen Hand Date: Fri, 16 Jun 2023 09:59:36 +0100 Subject: [PATCH 10/10] Fix sql package tests --- packages/sql/tests/{ => unit}/error.test.ts | 2 +- packages/sql/tests/{ => unit}/parameterizedQuery.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename packages/sql/tests/{ => unit}/error.test.ts (99%) rename packages/sql/tests/{ => unit}/parameterizedQuery.test.ts (98%) diff --git a/packages/sql/tests/error.test.ts b/packages/sql/tests/unit/error.test.ts similarity index 99% rename from packages/sql/tests/error.test.ts rename to packages/sql/tests/unit/error.test.ts index 74084883a..9c5896a1b 100644 --- a/packages/sql/tests/error.test.ts +++ b/packages/sql/tests/unit/error.test.ts @@ -19,7 +19,7 @@ import { DatabaseUniqueConstraintViolationError, DatabaseError, inferPostgresError, -} from '../src'; +} from '../../src'; describe('inferPostgresError', () => { test('Error with code property set to 23503 - creates DatabaseKeyViolationError, wrapping original and copying table and constraint properties', () => { diff --git a/packages/sql/tests/parameterizedQuery.test.ts b/packages/sql/tests/unit/parameterizedQuery.test.ts similarity index 98% rename from packages/sql/tests/parameterizedQuery.test.ts rename to packages/sql/tests/unit/parameterizedQuery.test.ts index 5d87a11e5..77e70bcc6 100644 --- a/packages/sql/tests/parameterizedQuery.test.ts +++ b/packages/sql/tests/unit/parameterizedQuery.test.ts @@ -14,7 +14,7 @@ * along with this program. If not, see https://www.gnu.org/licenses/. */ -import { parameterizedQuery } from '../src'; +import { parameterizedQuery } from '../../src'; describe('parameterizedQuery', () => { const testCases = [