From f022c6bfd737ce7e1e88e1dd665d975cf7092bc8 Mon Sep 17 00:00:00 2001 From: Sam Lanning Date: Thu, 2 Dec 2021 15:30:12 +0000 Subject: [PATCH 01/10] =?UTF-8?q?=E2=9C=A8=20Introduce=20new=20custom=20wh?= =?UTF-8?q?ere=20condition=20construction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Given a knex query builder, and one of our own custom conditions, return the prepared condition to be used directly by knex. There are a few reasons we do this rather than simply using knex query builder directly: * the query builder does not offer good type-safety (for example whereIn does not require the type of the values to match the type of the column) * we find ourselves needing to use query builders often for very simple conditions that could be abstracted + simplified However, we retain the ability to use a query builder when needed. --- src/db/util/conditions.ts | 178 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 src/db/util/conditions.ts diff --git a/src/db/util/conditions.ts b/src/db/util/conditions.ts new file mode 100644 index 00000000..bc40308d --- /dev/null +++ b/src/db/util/conditions.ts @@ -0,0 +1,178 @@ +/* eslint-disable @typescript-eslint/no-namespace */ +// we use namespaces here to neatly organize and scope the many type and type guard definitions + +import Knex = require('knex'); + +/** + * Symbols to use for where condition construction + */ +namespace ConditionSymbols { + export const BUILDER = Symbol('builder'); + export const AND = Symbol('and'); + export const OR = Symbol('or'); + + /** + * Symbols to use for where condition construction + */ + export const Cond = { + BUILDER: BUILDER, + AND: AND, + OR: OR, + } as const; +} + +export const Cond = ConditionSymbols.Cond; + +/** + * Symbols to use when constructing conditions for a single property + */ +namespace PropertySymbols { + export const IN = Symbol('in'); + export const NOT_IN = Symbol('not in'); + + /** + * Symbols to use when constructing conditions for a single property + */ + export const Op = { + IN: IN, + NOT_IN: NOT_IN, + } as const; +} + +export const Op = PropertySymbols.Op; + +namespace PropertyConditions { + export type EqualityCondition = T; + export type InCondition = { + [Op.IN]: T[]; + }; + export type NotInCondition = { + [Op.NOT_IN]: T[]; + }; + /** + * A condition that must hold over a single property whose type is T + */ + export type Condition = + | EqualityCondition + | InCondition + | NotInCondition; + + export const isEqualityCondition = ( + condition: Condition + ): condition is EqualityCondition => + typeof condition !== 'object' || condition === null; + + export const isInCondition = ( + condition: Condition + ): condition is InCondition => + Object.prototype.hasOwnProperty.call(condition, Op.IN); + + export const isNotInCondition = ( + condition: Condition + ): condition is NotInCondition => + Object.prototype.hasOwnProperty.call(condition, Op.NOT_IN); +} + +namespace OverallConditions { + // Overall Condition Definitions + + /** + * A collection of conditions across different properties that must all + * hold true at the same time + */ + export type PropertyConjunctionCondition = { + [P in keyof InstanceData]?: PropertyConditions.Condition; + }; + + export type QueryBuilderCondition = { + [Cond.BUILDER]: Knex.QueryCallback; + }; + + export type ConjunctionCondition = { + [Cond.AND]: Array>; + }; + + export type DisjunctionCondition = { + [Cond.OR]: Array>; + }; + + /** + * The root type for a custom condition that can be passed to `prepareCondition` + * to construct a knex condition / query in a type-safe manner + */ + export type Condition = + | PropertyConjunctionCondition + | QueryBuilderCondition + | ConjunctionCondition + | DisjunctionCondition; + + export const isQueryBuilderCondition = ( + condition: Condition + ): condition is QueryBuilderCondition => + Object.prototype.hasOwnProperty.call(condition, Cond.BUILDER); + + export const isConjunctionCondition = ( + condition: Condition + ): condition is ConjunctionCondition => + Object.prototype.hasOwnProperty.call(condition, Cond.AND); + + export const isDisjunctionCondition = ( + condition: Condition + ): condition is DisjunctionCondition => + Object.prototype.hasOwnProperty.call(condition, Cond.OR); +} + +/** + * The root type for a custom condition that can be passed to `prepareCondition` + * to construct a knex condition / query in a type-safe manner + */ +export type Condition = OverallConditions.Condition; + +/** + * Given a knex query builder, and one of our own custom conditions, + * return the prepared condition to be used directly by knex. + * + * There are a few reasons we do this rather than simply using knex query + * builder directly: + * + * * the query builder does not offer good type-safety (for example whereIn + * does not require the type of the values to match the type of the column) + * * we find ourselves needing to use query builders often for very simple + * conditions that could be abstracted + simplified + * + * However, we retain the ability to use a query builder when needed. + */ +export const prepareCondition = + ( + condition: Condition + ): Knex.QueryCallback => + (builder) => { + if (OverallConditions.isQueryBuilderCondition(condition)) { + builder.where(condition[Cond.BUILDER]); + } else if (OverallConditions.isConjunctionCondition(condition)) { + for (const c of condition[Cond.AND]) { + builder.andWhere(prepareCondition(c)); + } + } else if (OverallConditions.isDisjunctionCondition(condition)) { + for (const c of condition[Cond.OR]) { + builder.orWhere(prepareCondition(c)); + } + } else { + // Combine all property conditions in a single + const propertyConditions = Object.entries(condition) as [ + keyof InstanceData, + PropertyConditions.Condition + ][]; + for (const [property, propertyCondition] of propertyConditions) { + if (PropertyConditions.isEqualityCondition(propertyCondition)) { + builder.where(property, propertyCondition); + } else if (PropertyConditions.isInCondition(propertyCondition)) { + builder.whereIn(property, propertyCondition[Op.IN]); + } else if (PropertyConditions.isNotInCondition(propertyCondition)) { + builder.whereNotIn(property, propertyCondition[Op.NOT_IN]); + } else { + throw new Error(`Unexpected condition type: ${propertyCondition}`); + } + } + } + }; From 9f1f46915b9f52b8b6d002a703c9fcc1c8f5deb7 Mon Sep 17 00:00:00 2001 From: Sam Lanning Date: Thu, 2 Dec 2021 15:37:31 +0000 Subject: [PATCH 02/10] Use new prepareCondition functionality in raw query --- src/db/util/raw-model.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/db/util/raw-model.ts b/src/db/util/raw-model.ts index 295b9679..f3ec36b9 100644 --- a/src/db/util/raw-model.ts +++ b/src/db/util/raw-model.ts @@ -3,6 +3,7 @@ * Use of `any` in this module is generally deliberate to help with generics */ import Knex = require('knex'); +import { Condition, prepareCondition } from './conditions'; import { FieldDefinition, @@ -25,9 +26,7 @@ export type CreateManyFn = ( } ) => Promise>>; -export type WhereCond = - | Knex.QueryCallback, InstanceDataOf> - | Partial>; +export type WhereCond = Condition>; export type OrderByCond = { column: keyof InstanceDataOf; @@ -133,7 +132,7 @@ export const defineRawModel = trx, } = {}) => { const builder = trx ? tbl().transacting(trx) : tbl(); - const query = builder.where(where || {}).select('*'); + const query = builder.where(prepareCondition(where || {})).select('*'); if (limit !== undefined && limit > 0) { query.limit(limit); @@ -168,7 +167,7 @@ export const defineRawModel = const update: UpdateFn = async ({ values, where, trx }) => { const builder = trx ? tbl().transacting(trx) : tbl(); const res = await builder - .where(where) + .where(prepareCondition(where || {})) .update(values as any) .returning('*'); return (res as unknown[]).map(validateAndFilter); @@ -176,7 +175,7 @@ export const defineRawModel = const destroy: DestroyFn = async ({ where, trx }) => { const builder = trx ? tbl().transacting(trx) : tbl(); - const count = await builder.where(where).delete(); + const count = await builder.where(prepareCondition(where || {})).delete(); return count; }; From 5cac6dc2e0a994ce4e6340cd76aa370238ac5ed0 Mon Sep 17 00:00:00 2001 From: Sam Lanning Date: Thu, 2 Dec 2021 15:41:04 +0000 Subject: [PATCH 03/10] =?UTF-8?q?=E2=9C=A8=20Allow=20iterables=20to=20be?= =?UTF-8?q?=20used=20instead=20of=20arrays=20for=20whereIn?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/db/util/conditions.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/db/util/conditions.ts b/src/db/util/conditions.ts index bc40308d..bfe5373a 100644 --- a/src/db/util/conditions.ts +++ b/src/db/util/conditions.ts @@ -44,10 +44,10 @@ export const Op = PropertySymbols.Op; namespace PropertyConditions { export type EqualityCondition = T; export type InCondition = { - [Op.IN]: T[]; + [Op.IN]: Iterable; }; export type NotInCondition = { - [Op.NOT_IN]: T[]; + [Op.NOT_IN]: Iterable; }; /** * A condition that must hold over a single property whose type is T @@ -167,9 +167,9 @@ export const prepareCondition = if (PropertyConditions.isEqualityCondition(propertyCondition)) { builder.where(property, propertyCondition); } else if (PropertyConditions.isInCondition(propertyCondition)) { - builder.whereIn(property, propertyCondition[Op.IN]); + builder.whereIn(property, [...propertyCondition[Op.IN]]); } else if (PropertyConditions.isNotInCondition(propertyCondition)) { - builder.whereNotIn(property, propertyCondition[Op.NOT_IN]); + builder.whereNotIn(property, [...propertyCondition[Op.NOT_IN]]); } else { throw new Error(`Unexpected condition type: ${propertyCondition}`); } From 57ccf3b7a0943fda6a213ae8d4c1283d734dd3a0 Mon Sep 17 00:00:00 2001 From: Sam Lanning Date: Thu, 2 Dec 2021 15:59:29 +0000 Subject: [PATCH 04/10] =?UTF-8?q?=F0=9F=8E=A8=20Use=20new=20where=20condit?= =?UTF-8?q?ions=20across=20library?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/auth/index.ts | 39 ++++++++++++++----------- src/db/util/id-model.ts | 7 ++++- src/db/util/versioned-model.ts | 17 +++++++---- src/lib/data/attachments.ts | 30 ++++++++++++-------- src/lib/data/governingEntities.ts | 14 ++++----- src/lib/data/planEntities.ts | 23 +++++++++------ src/lib/data/projects.ts | 47 ++++++++++++++++++------------- 7 files changed, 108 insertions(+), 69 deletions(-) diff --git a/src/auth/index.ts b/src/auth/index.ts index f7f4f134..5e851db9 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -27,6 +27,7 @@ import { filterValidRoleStrings, RolesGrant, } from './roles'; +import { Op } from '../db/util/conditions'; const randomBytes = promisify(crypto.randomBytes); @@ -68,11 +69,11 @@ const activateInvitesForEmail = async ( const targets = new Map(); for (const target of await models.authTarget.find({ - where: (builder) => - builder.whereIn( - 'id', - invites.map((i) => i.target) - ), + where: { + id: { + [Op.IN]: invites.map((i) => i.target), + }, + }, })) { targets.set(target.id, target); } @@ -284,26 +285,30 @@ export const getRoleGrantsForUsers = async ({ // Get the grantees const grantees = await models.authGrantee.find({ - where: (builder) => - builder.where('type', 'user').andWhere('granteeId', 'in', users), + where: { + type: 'user', + granteeId: { + [Op.IN]: users, + }, + }, }); if (grantees.length === 0) { return new Map(); } const granteesById = organizeObjectsByUniqueProperty(grantees, 'id'); const grants = await models.authGrant.find({ - where: (builder) => - builder.whereIn( - 'grantee', - grantees.map((g) => g.id) - ), + where: { + grantee: { + [Op.IN]: grantees.map((g) => g.id), + }, + }, }); const targets = await models.authTarget.find({ - where: (builder) => - builder.whereIn( - 'id', - grants.map((g) => g.target) - ), + where: { + id: { + [Op.IN]: grants.map((g) => g.target), + }, + }, }); const targetsById = organizeObjectsByUniqueProperty(targets, 'id'); diff --git a/src/db/util/id-model.ts b/src/db/util/id-model.ts index 37c0b735..bbc1bdc7 100644 --- a/src/db/util/id-model.ts +++ b/src/db/util/id-model.ts @@ -8,6 +8,7 @@ import { InstanceDataOf, } from './model-definition'; import { defineSequelizeModel, FieldsWithSequelize } from './sequelize-model'; +import { Op } from './conditions'; /** * Given a definition of fields, and the name of an ID prop, @@ -97,7 +98,11 @@ export const defineIDModel = const getAll = async (ids: Iterable): Promise> => { const items = await model.find({ - where: (builder) => builder.whereIn<'id'>('id', [...(ids as any)]), + where: { + id: { + [Op.IN]: ids, + }, + }, }); const grouped = new Map(); for (const item of items) { diff --git a/src/db/util/versioned-model.ts b/src/db/util/versioned-model.ts index d37b195a..a7f27055 100644 --- a/src/db/util/versioned-model.ts +++ b/src/db/util/versioned-model.ts @@ -10,6 +10,7 @@ import { ParticipantId, PARTICIPANT_ID } from '../models/participant'; import { FieldDefinition, UserDataOf } from './model-definition'; import { defineIDModel } from './id-model'; import { InstanceDataOfModel, ModelInternals, WhereCond } from './raw-model'; +import { Op } from './conditions'; type LookupColumnDefinition = Pick; @@ -247,10 +248,12 @@ export const defineVersionedModel = const roots = await rootModel.find(args as any); const ids = new Set(roots.map((r) => r.id)); const versionsList = await versionModel.find({ - where: (builder) => - builder - .whereIn<'root'>('root', [...ids]) - .andWhere<'isLatest'>('isLatest', true), + where: { + root: { + [Op.IN]: ids, + }, + isLatest: true, + }, }); const versions = new Map(versionsList.map((v) => [v.root, v])); @@ -296,7 +299,11 @@ export const defineVersionedModel = }, getAll: async (ids) => { const items = await result.findAll({ - where: (builder) => builder.whereIn<'id'>('id', [...(ids as any)]), + where: { + id: { + [Op.IN]: ids as Iterable, + }, + }, }); const grouped = new Map>(); for (const item of items) { diff --git a/src/lib/data/attachments.ts b/src/lib/data/attachments.ts index 0671ecf2..3171b312 100644 --- a/src/lib/data/attachments.ts +++ b/src/lib/data/attachments.ts @@ -11,6 +11,7 @@ import { ATTACHMENT_VERSION_VALUE } from '../../db/models/json/attachment'; import { PlanId } from '../../db/models/plan'; import { PlanEntityId } from '../../db/models/planEntity'; import { Database } from '../../db/type'; +import { Op } from '../../db/util/conditions'; import { InstanceDataOfModel } from '../../db/util/raw-model'; import { AnnotatedMap, @@ -124,27 +125,34 @@ export const getAllAttachments = async ({ database.attachmentPrototype, (t) => t.find({ - where: (builder) => - builder.whereIn('type', types).andWhere('planId', planId), + where: { + planId, + type: { + [Op.IN]: types, + }, + }, }), 'id' ); const attachments = await database.attachment.find({ - where: (builder) => - builder.whereIn('type', types).andWhere('planId', planId), + where: { + planId, + type: { + [Op.IN]: types, + }, + }, }); const attachmentVersionsByAttachmentId = await findAndOrganizeObjectsByUniqueProperty( database.attachmentVersion, (t) => t.find({ - where: (builder) => - builder - .whereIn( - 'attachmentId', - attachments.map((pa) => pa.id) - ) - .andWhere('latestVersion', true), + where: { + latestVersion: true, + attachmentId: { + [Op.IN]: attachments.map((pa) => pa.id), + }, + }, }), 'attachmentId' ); diff --git a/src/lib/data/governingEntities.ts b/src/lib/data/governingEntities.ts index 891ddbe9..6172a417 100644 --- a/src/lib/data/governingEntities.ts +++ b/src/lib/data/governingEntities.ts @@ -3,6 +3,7 @@ import { EntityPrototypeId } from '../../db/models/entityPrototype'; import { GoverningEntityId } from '../../db/models/governingEntity'; import { PlanId } from '../../db/models/plan'; import { Database } from '../../db/type'; +import { Op } from '../../db/util/conditions'; import { InstanceDataOfModel } from '../../db/util/raw-model'; import { annotatedMap, AnnotatedMap, getRequiredData } from '../../util'; @@ -50,13 +51,12 @@ export const getAllGoverningEntitiesForPlan = async ({ database.governingEntityVersion, (t) => t.find({ - where: (builder) => - builder - .whereIn( - 'governingEntityId', - ges.map((ge) => ge.id) - ) - .andWhere('latestVersion', true), + where: { + latestVersion: true, + governingEntityId: { + [Op.IN]: ges.map((ge) => ge.id), + }, + }, }), 'governingEntityId' ); diff --git a/src/lib/data/planEntities.ts b/src/lib/data/planEntities.ts index b5b9dcd3..bd299f32 100644 --- a/src/lib/data/planEntities.ts +++ b/src/lib/data/planEntities.ts @@ -4,6 +4,7 @@ import { GoverningEntityId } from '../../db/models/governingEntity'; import { PlanId } from '../../db/models/plan'; import { PlanEntityId } from '../../db/models/planEntity'; import { Database } from '../../db/type'; +import { Op } from '../../db/util/conditions'; import { InstanceDataOfModel } from '../../db/util/raw-model'; import { isDefined, @@ -72,10 +73,12 @@ export const getAndValidateAllPlanEntities = async ({ database.planEntityVersion, (t) => t.find({ - where: (builder) => - builder - .whereIn('planEntityId', [...planEntityIDs]) - .andWhere('latestVersion', true), + where: { + latestVersion: true, + planEntityId: { + [Op.IN]: planEntityIDs, + }, + }, }), 'planEntityId' ); @@ -84,11 +87,13 @@ export const getAndValidateAllPlanEntities = async ({ database.entitiesAssociation, (t) => t.find({ - where: (builder) => - builder - .whereIn('childId', [...planEntityIDs]) - .andWhere('parentType', 'governingEntity') - .andWhere('childType', 'planEntity'), + where: { + childType: 'planEntity', + parentType: 'governingEntity', + childId: { + [Op.IN]: planEntityIDs, + }, + }, }), 'childId' ); diff --git a/src/lib/data/projects.ts b/src/lib/data/projects.ts index 411a84ad..0a4842cb 100644 --- a/src/lib/data/projects.ts +++ b/src/lib/data/projects.ts @@ -6,6 +6,7 @@ import { ProjectId } from '../../db/models/project'; import { Database } from '../../db/type'; import { InstanceOfModel } from '../../db/util/types'; import { getRequiredData, groupObjectsByProperty, isDefined } from '../../util'; +import { Op } from '../../db/util/conditions'; export interface ProjectData { project: InstanceOfModel; @@ -37,31 +38,33 @@ export async function getAllProjectsForPlan({ database.projectVersion, (t) => t.find({ - where: (builder) => - builder.whereIn('id', [ - ...new Set(pvps.map((pvp) => pvp.projectVersionId)), - ]), + where: { + id: { + [Op.IN]: new Set(pvps.map((pvp) => pvp.projectVersionId)), + }, + }, }), 'id' ); const projectVersions = [...pvsById.values()]; const projects = await database.project.find({ - where: (builder) => - builder.whereIn('id', [ - ...new Set(projectVersions.map((pv) => pv.projectId)), - ]), + where: { + id: { + [Op.IN]: new Set(projectVersions.map((pv) => pv.projectId)), + }, + }, }); const workflowStatusById = await findAndOrganizeObjectsByUniqueProperty( database.workflowStatusOption, (t) => t.find({ - where: (builder) => - builder.whereIn( - 'id', - [...new Set(pvps.map((pvp) => pvp.workflowStatusOptionId))].filter( - isDefined - ) - ), + where: { + id: { + [Op.IN]: [ + ...new Set(pvps.map((pvp) => pvp.workflowStatusOptionId)), + ].filter(isDefined), + }, + }, }), 'id' ); @@ -117,8 +120,11 @@ export async function getOrganizationIDsForProjects({ const groupedPVOs = groupObjectsByProperty( await database.projectVersionOrganization.find({ - where: (builder) => - builder.whereIn('projectVersionId', projectVersionIds), + where: { + projectVersionId: { + [Op.IN]: projectVersionIds, + }, + }, }), 'projectVersionId' ); @@ -148,8 +154,11 @@ export async function getGoverningEntityIDsForProjects({ const groupedPVGEs = groupObjectsByProperty( await database.projectVersionGoverningEntity.find({ - where: (builder) => - builder.whereIn('projectVersionId', projectVersionIds), + where: { + projectVersionId: { + [Op.IN]: projectVersionIds, + }, + }, }), 'projectVersionId' ); From d2c41453e13ba752913a90428d9bbaac4490ff08 Mon Sep 17 00:00:00 2001 From: Sam Lanning Date: Thu, 2 Dec 2021 16:00:02 +0000 Subject: [PATCH 05/10] =?UTF-8?q?=F0=9F=9A=A8=20Ignore=20use=20of=20any=20?= =?UTF-8?q?in=20versioned-model.ts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/db/util/versioned-model.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/db/util/versioned-model.ts b/src/db/util/versioned-model.ts index a7f27055..c974b887 100644 --- a/src/db/util/versioned-model.ts +++ b/src/db/util/versioned-model.ts @@ -1,3 +1,7 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +/** + * Use of `any` in this module is generally deliberate to help with generics + */ import Knex = require('knex'); import merge = require('lodash/merge'); import * as t from 'io-ts'; From d69245ee6325a9d3191ef5a787d0048844ba8518 Mon Sep 17 00:00:00 2001 From: Sam Lanning Date: Thu, 2 Dec 2021 16:10:58 +0000 Subject: [PATCH 06/10] =?UTF-8?q?=E2=9C=A8=20Expose=20`Op`=20and=20`Cond`?= =?UTF-8?q?=20under=20a=20`util`=20prop=20on=20`Database`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/db/fetching.ts | 5 +++-- src/db/index.ts | 11 +++++++++++ src/db/util/conditions.ts | 4 ++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/db/fetching.ts b/src/db/fetching.ts index 49ed6748..b22e2f18 100644 --- a/src/db/fetching.ts +++ b/src/db/fetching.ts @@ -1,3 +1,4 @@ +import { UTILS } from '.'; import { AnnotatedMap, organizeObjectsByUniqueValue } from '../util'; import { Database } from './type'; import { InstanceDataOfModel, Model } from './util/raw-model'; @@ -24,7 +25,7 @@ type InstanceOf = T extends Model * database, otherwise an error will be thrown */ export const findAndOrganizeObjectsByUniqueProperty = < - Table extends Database[keyof Database], + Table extends Database[keyof Omit], P extends keyof InstanceOf >( table: Table, @@ -47,7 +48,7 @@ export const findAndOrganizeObjectsByUniqueProperty = < * and organize them into an AnnotatedMap by a unique value. */ export const findAndOrganizeObjectsByUniqueValue = async < - Table extends Database[keyof Database], + Table extends Database[keyof Omit], V >( table: Table, diff --git a/src/db/index.ts b/src/db/index.ts index 4b7fe392..b845ac3c 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -95,8 +95,19 @@ import unitType from './models/unitType'; import usageYear from './models/usageYear'; import workflowStatusOption from './models/workflowStatusOption'; import workflowStatusOptionStep from './models/workflowStatusOptionStep'; +import { Op, Cond } from './util/conditions'; + +/** + * Utilities that are frequently used, and should also be included as part of + * the model root for easy access. + */ +export const UTILS = { + Op, + Cond, +}; export default (conn: Knex) => ({ + ...UTILS, attachment: attachment(conn), attachmentPrototype: attachmentPrototype(conn), attachmentVersion: attachmentVersion(conn), diff --git a/src/db/util/conditions.ts b/src/db/util/conditions.ts index bfe5373a..72e4864f 100644 --- a/src/db/util/conditions.ts +++ b/src/db/util/conditions.ts @@ -6,7 +6,7 @@ import Knex = require('knex'); /** * Symbols to use for where condition construction */ -namespace ConditionSymbols { +export namespace ConditionSymbols { export const BUILDER = Symbol('builder'); export const AND = Symbol('and'); export const OR = Symbol('or'); @@ -26,7 +26,7 @@ export const Cond = ConditionSymbols.Cond; /** * Symbols to use when constructing conditions for a single property */ -namespace PropertySymbols { +export namespace PropertySymbols { export const IN = Symbol('in'); export const NOT_IN = Symbol('not in'); From 5edc3a12a7dc31688c8384f580c780ecfa00a7a6 Mon Sep 17 00:00:00 2001 From: Sam Lanning Date: Thu, 2 Dec 2021 16:35:47 +0000 Subject: [PATCH 07/10] =?UTF-8?q?=F0=9F=8E=A8=20Reorganize=20initializatio?= =?UTF-8?q?n=20of=20database=20root?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some of our unit tests iterate through every table (e.g. to check the schema for each table). Combining tables along with util properties made this more complicated, so we also need to provide a way to easily get all tables. Beyond this, we can tidy up the types a bit more. --- src/db/fetching.ts | 32 +++++++++++++------------------- src/db/index.ts | 26 ++++++++++++++++++++++++-- src/db/type.ts | 4 +--- 3 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/db/fetching.ts b/src/db/fetching.ts index b22e2f18..556db406 100644 --- a/src/db/fetching.ts +++ b/src/db/fetching.ts @@ -1,6 +1,5 @@ -import { UTILS } from '.'; +import { Table } from '.'; import { AnnotatedMap, organizeObjectsByUniqueValue } from '../util'; -import { Database } from './type'; import { InstanceDataOfModel, Model } from './util/raw-model'; import { InstanceOfVersionedModel, @@ -11,7 +10,7 @@ import { * Generic type that can be used to obtain the type of an instance of either a * standard table, or a versioned table. */ -type InstanceOf = T extends Model +type InstanceOf = T extends Model ? InstanceDataOfModel : T extends VersionedModel ? InstanceOfVersionedModel @@ -25,21 +24,19 @@ type InstanceOf = T extends Model * database, otherwise an error will be thrown */ export const findAndOrganizeObjectsByUniqueProperty = < - Table extends Database[keyof Omit], - P extends keyof InstanceOf
+ T extends Table, + P extends keyof InstanceOf >( - table: Table, - fetch: (tbl: Table) => Promise>>, + table: T, + fetch: (tbl: T) => Promise>>, property: P -): Promise< - AnnotatedMap[P], null>, InstanceOf
> -> => { +): Promise[P], null>, InstanceOf>> => { return findAndOrganizeObjectsByUniqueValue(table, fetch, (item) => { const val = item[property]; if (val === null) { throw new Error(`Unexpected null value in ${property} for ${item}`); } - return val as Exclude[P], null>; + return val as Exclude[P], null>; }); }; @@ -47,14 +44,11 @@ export const findAndOrganizeObjectsByUniqueProperty = < * Fetch a number of items of a particular type from the database, * and organize them into an AnnotatedMap by a unique value. */ -export const findAndOrganizeObjectsByUniqueValue = async < - Table extends Database[keyof Omit], - V ->( - table: Table, - fetch: (tbl: Table) => Promise>>, - getValue: (i: InstanceOf
) => Exclude -): Promise, InstanceOf
>> => { +export const findAndOrganizeObjectsByUniqueValue = async ( + table: T, + fetch: (tbl: T) => Promise>>, + getValue: (i: InstanceOf) => Exclude +): Promise, InstanceOf>> => { const values = await fetch(table); const tableName = table._internals.type === 'single-table' diff --git a/src/db/index.ts b/src/db/index.ts index b845ac3c..0a3c632a 100644 --- a/src/db/index.ts +++ b/src/db/index.ts @@ -106,8 +106,7 @@ export const UTILS = { Cond, }; -export default (conn: Knex) => ({ - ...UTILS, +const initializeTables = (conn: Knex) => ({ attachment: attachment(conn), attachmentPrototype: attachmentPrototype(conn), attachmentVersion: attachmentVersion(conn), @@ -205,3 +204,26 @@ export default (conn: Knex) => ({ workflowStatusOption: workflowStatusOption(conn), workflowStatusOptionStep: workflowStatusOptionStep(conn), }); + +export type Tables = ReturnType; + +export type Table = Tables[keyof Tables]; + +const initializeRoot = (conn: Knex) => { + const _tables = initializeTables(conn); + return { + ...UTILS, + ..._tables, + /** + * Expose the tables grouped together under one object under the root to + * allow for easier iteration through each of them. + * + * (this is used in unit tests) + */ + _tables, + }; +}; + +export type Database = ReturnType; + +export default initializeRoot; diff --git a/src/db/type.ts b/src/db/type.ts index 5100597e..60435dae 100644 --- a/src/db/type.ts +++ b/src/db/type.ts @@ -1,3 +1 @@ -import type db from '.'; - -export type Database = ReturnType; +export type { Database } from '.'; From 49d61f0ce849d4a1ab1ccbcc225e930c1399bcfd Mon Sep 17 00:00:00 2001 From: Sam Lanning Date: Thu, 2 Dec 2021 18:30:02 +0000 Subject: [PATCH 08/10] =?UTF-8?q?=E2=9C=A8=20Add=20additional=20operators?= =?UTF-8?q?=20to=20where=20conditions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/db/util/conditions.ts | 129 +++++++++++++++++++++++++++++++++++++- 1 file changed, 126 insertions(+), 3 deletions(-) diff --git a/src/db/util/conditions.ts b/src/db/util/conditions.ts index 72e4864f..e6223c43 100644 --- a/src/db/util/conditions.ts +++ b/src/db/util/conditions.ts @@ -29,6 +29,14 @@ export const Cond = ConditionSymbols.Cond; export namespace PropertySymbols { export const IN = Symbol('in'); export const NOT_IN = Symbol('not in'); + export const IS_NULL = Symbol('is null'); + export const BETWEEN = Symbol('between'); + export const LIKE = Symbol('like'); + export const ILIKE = Symbol('ilike'); + export const LT = Symbol('less than'); + export const LTE = Symbol('less than or equal to'); + export const GT = Symbol('greater than'); + export const GTE = Symbol('greater than or equal to'); /** * Symbols to use when constructing conditions for a single property @@ -36,6 +44,14 @@ export namespace PropertySymbols { export const Op = { IN: IN, NOT_IN: NOT_IN, + IS_NULL: IS_NULL, + BETWEEN: BETWEEN, + LIKE: LIKE, + ILIKE: ILIKE, + LT: LT, + LTE: LTE, + GT: GT, + GTE: GTE, } as const; } @@ -43,19 +59,57 @@ export const Op = PropertySymbols.Op; namespace PropertyConditions { export type EqualityCondition = T; + export type InCondition = { [Op.IN]: Iterable; }; export type NotInCondition = { [Op.NOT_IN]: Iterable; }; + + export type IsNullCondition = { + [Op.IS_NULL]: boolean; + }; + + export type BetweenCondition = { + [Op.BETWEEN]: [T, T]; + }; + + // Simple binary operator conditions + + export type LikeCondition = { + [Op.LIKE]: T & string; + }; + export type ILikeCondition = { + [Op.ILIKE]: T & string; + }; + export type LtCondition = { + [Op.LT]: T; + }; + export type LteCondition = { + [Op.LTE]: T; + }; + export type GtCondition = { + [Op.GT]: T; + }; + export type GteCondition = { + [Op.GTE]: T; + }; /** * A condition that must hold over a single property whose type is T */ export type Condition = | EqualityCondition | InCondition - | NotInCondition; + | NotInCondition + | IsNullCondition + | BetweenCondition + | LikeCondition + | ILikeCondition + | LtCondition + | LteCondition + | GtCondition + | GteCondition; export const isEqualityCondition = ( condition: Condition @@ -71,6 +125,46 @@ namespace PropertyConditions { condition: Condition ): condition is NotInCondition => Object.prototype.hasOwnProperty.call(condition, Op.NOT_IN); + + export const isNullCondition = ( + condition: Condition + ): condition is IsNullCondition => + Object.prototype.hasOwnProperty.call(condition, Op.IS_NULL); + + export const isBetweenCondition = ( + condition: Condition + ): condition is BetweenCondition => + Object.prototype.hasOwnProperty.call(condition, Op.BETWEEN); + + export const isLikeCondition = ( + condition: Condition + ): condition is LikeCondition => + Object.prototype.hasOwnProperty.call(condition, Op.LIKE); + + export const isILikeCondition = ( + condition: Condition + ): condition is ILikeCondition => + Object.prototype.hasOwnProperty.call(condition, Op.ILIKE); + + export const isLtCondition = ( + condition: Condition + ): condition is LtCondition => + Object.prototype.hasOwnProperty.call(condition, Op.LT); + + export const isLteCondition = ( + condition: Condition + ): condition is LteCondition => + Object.prototype.hasOwnProperty.call(condition, Op.LTE); + + export const isGtCondition = ( + condition: Condition + ): condition is GtCondition => + Object.prototype.hasOwnProperty.call(condition, Op.GT); + + export const isGteCondition = ( + condition: Condition + ): condition is GteCondition => + Object.prototype.hasOwnProperty.call(condition, Op.GTE); } namespace OverallConditions { @@ -81,7 +175,9 @@ namespace OverallConditions { * hold true at the same time */ export type PropertyConjunctionCondition = { - [P in keyof InstanceData]?: PropertyConditions.Condition; + [P in keyof InstanceData & string]?: PropertyConditions.Condition< + InstanceData[P] + >; }; export type QueryBuilderCondition = { @@ -170,8 +266,35 @@ export const prepareCondition = builder.whereIn(property, [...propertyCondition[Op.IN]]); } else if (PropertyConditions.isNotInCondition(propertyCondition)) { builder.whereNotIn(property, [...propertyCondition[Op.NOT_IN]]); + } else if (PropertyConditions.isNullCondition(propertyCondition)) { + if (propertyCondition[Op.IS_NULL]) { + builder.whereNull(property); + } else { + builder.whereNotNull(property); + } + } else if (PropertyConditions.isBetweenCondition(propertyCondition)) { + builder.whereBetween(property, [ + propertyCondition[Op.BETWEEN][0], + propertyCondition[Op.BETWEEN][1], + ]); + } else if (PropertyConditions.isLikeCondition(propertyCondition)) { + builder.where(property as string, 'like', propertyCondition[Op.LIKE]); + } else if (PropertyConditions.isILikeCondition(propertyCondition)) { + builder.where( + property as string, + 'ilike', + propertyCondition[Op.ILIKE] + ); + } else if (PropertyConditions.isLtCondition(propertyCondition)) { + builder.where(property, '<', propertyCondition[Op.LT] as any); + } else if (PropertyConditions.isLteCondition(propertyCondition)) { + builder.where(property, '<=', propertyCondition[Op.LTE] as any); + } else if (PropertyConditions.isGtCondition(propertyCondition)) { + builder.where(property, '>', propertyCondition[Op.GT] as any); + } else if (PropertyConditions.isGteCondition(propertyCondition)) { + builder.where(property, '>=', propertyCondition[Op.GTE] as any); } else { - throw new Error(`Unexpected condition type: ${propertyCondition}`); + throw new Error(`Unexpected condition: ${propertyCondition}`); } } } From 7decaf6edbfd7fddad62377430932ab74f1c6a01 Mon Sep 17 00:00:00 2001 From: Sam Lanning Date: Thu, 2 Dec 2021 18:30:26 +0000 Subject: [PATCH 09/10] =?UTF-8?q?=E2=9C=A8=20Handle=20undefined=20property?= =?UTF-8?q?=20values=20in=20where=20conditions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/db/util/conditions.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/db/util/conditions.ts b/src/db/util/conditions.ts index e6223c43..e6111ce5 100644 --- a/src/db/util/conditions.ts +++ b/src/db/util/conditions.ts @@ -260,7 +260,9 @@ export const prepareCondition = PropertyConditions.Condition ][]; for (const [property, propertyCondition] of propertyConditions) { - if (PropertyConditions.isEqualityCondition(propertyCondition)) { + if (propertyCondition === undefined) { + throw new Error(`Unexpected undefined value for ${property}`); + } else if (PropertyConditions.isEqualityCondition(propertyCondition)) { builder.where(property, propertyCondition); } else if (PropertyConditions.isInCondition(propertyCondition)) { builder.whereIn(property, [...propertyCondition[Op.IN]]); From deb26b6de0f0ec53b1adde36105ec340ba6928b2 Mon Sep 17 00:00:00 2001 From: Sam Lanning Date: Thu, 2 Dec 2021 18:31:00 +0000 Subject: [PATCH 10/10] =?UTF-8?q?=F0=9F=94=96=20Bump=20to=20v1.0.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6c447c4c..8542c77b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@unocha/hpc-api-core", - "version": "0.10.0", + "version": "1.0.0", "description": "Core libraries supporting HPC.Tools API Backend", "license": "Apache-2.0", "private": false,