From 13e2c7b449f0f8014bf94bdfc49cfd1ca7479060 Mon Sep 17 00:00:00 2001 From: Ola Okelola <10857143+lolopinto@users.noreply.github.com> Date: Wed, 6 Sep 2023 21:27:45 -0700 Subject: [PATCH] misc context | cache fixes (#1641) --- ts/src/action/operations.ts | 6 +- .../action/transformed_orchestrator.test.ts | 158 ++++++++++-------- ts/src/core/base.ts | 24 ++- ts/src/core/clause.ts | 4 +- ts/src/core/context.ts | 39 +++-- ts/src/core/ent_custom_data.test.ts | 2 - ts/src/core/ent_data.test.ts | 1 - .../core/loaders/assoc_count_loader.test.ts | 57 ++++--- ts/src/core/loaders/assoc_edge_loader.test.ts | 109 ++++++------ ts/src/core/loaders/assoc_edge_loader.ts | 2 - ts/src/core/loaders/object_loader.test.ts | 93 +++++------ ts/src/core/loaders/query_loader.test.ts | 13 -- ts/src/core/loaders/raw_count_loader.test.ts | 22 +-- ts/src/core/query_impl.ts | 15 +- 14 files changed, 289 insertions(+), 256 deletions(-) diff --git a/ts/src/action/operations.ts b/ts/src/action/operations.ts index 3878f496d..f1e85fbe3 100644 --- a/ts/src/action/operations.ts +++ b/ts/src/action/operations.ts @@ -516,7 +516,7 @@ export class EdgeOperation implements DataOperation { ): Promise { const params = this.getDeleteRowParams(edgeData, edge, context); if (params.op === SQLStatementOperation.Delete) { - return deleteRows(q, params.options, params.clause); + return deleteRows(q, { ...params.options, context }, params.clause); } else { if (params.op !== SQLStatementOperation.Update) { throw new Error(`invalid operation ${params.op}`); @@ -526,6 +526,7 @@ export class EdgeOperation implements DataOperation { whereClause: params.clause, fields: params.updateData!, fieldsToLog: params.updateData!, + context, }); } } @@ -538,7 +539,7 @@ export class EdgeOperation implements DataOperation { ): void { const params = this.getDeleteRowParams(edgeData, edge, context); if (params.op === SQLStatementOperation.Delete) { - return deleteRowsSync(q, params.options, params.clause); + return deleteRowsSync(q, { ...params.options, context }, params.clause); } else { if (params.op !== SQLStatementOperation.Update) { throw new Error(`invalid operation ${params.op}`); @@ -547,6 +548,7 @@ export class EdgeOperation implements DataOperation { tableName: params.options.tableName, whereClause: params.clause, fields: params.updateData!, + context, }); } } diff --git a/ts/src/action/transformed_orchestrator.test.ts b/ts/src/action/transformed_orchestrator.test.ts index cb8064819..7a4c4416f 100644 --- a/ts/src/action/transformed_orchestrator.test.ts +++ b/ts/src/action/transformed_orchestrator.test.ts @@ -1,7 +1,7 @@ import { advanceTo } from "jest-date-mock"; import { WriteOperation } from "../action"; import { EntChangeset } from "../action/orchestrator"; -import { Data, Ent, Viewer } from "../core/base"; +import { Context, Data, Ent, Viewer } from "../core/base"; import { LoggedOutViewer } from "../core/viewer"; import { StringType, TimestampType } from "../schema/field"; import { @@ -20,7 +20,7 @@ import { createRowForTest } from "../testutils/write"; import * as clause from "../core/clause"; import { snakeCase } from "snake-case"; import DB, { Dialect } from "../core/db"; -import { ObjectLoader } from "../core/loaders"; +import { ObjectLoaderFactory } from "../core/loaders"; import { TestContext } from "../testutils/context/test_context"; import { assoc_edge_config_table, @@ -33,7 +33,6 @@ import { import { convertDate } from "../core/convert"; import { FieldMap } from "../schema"; import { loadRawEdgeCountX } from "../core/ent"; -import { RawQueryOperation } from "./operations"; const edges = ["edge", "inverseEdge", "symmetricEdge"]; async function createEdges() { @@ -269,38 +268,59 @@ describe("sqlite", () => { commonTests(); }); +const usersLoaderFactory = new ObjectLoaderFactory({ + tableName: "users", + fields: ["id", "first_name", "last_name", "deleted_at"], + key: "id", + clause: clause.Eq("deleted_at", null), +}); + +const accountsLoaderFactory = new ObjectLoaderFactory({ + tableName: "accounts", + fields: ["id", "first_name", "last_name", "deleted_at"], + key: "id", + clause: clause.Eq("deleted_at", null), +}); + +const contactsLoaderFactory = new ObjectLoaderFactory({ + tableName: "contacts", + fields: ["id", "first_name", "last_name", "deleted_at"], + key: "id", + clause: clause.Eq("deleted_at", null), +}); + +const usersLoaderFactoryNoClause = new ObjectLoaderFactory({ + tableName: "users", + fields: ["id", "first_name", "last_name", "deleted_at"], + key: "id", +}); + +const accountsLoaderFactoryNoClause = new ObjectLoaderFactory({ + tableName: "accounts", + fields: ["id", "first_name", "last_name", "deleted_at"], + key: "id", +}); + +const contactsLoaderFactoryNoClause = new ObjectLoaderFactory({ + tableName: "contacts", + fields: ["id", "first_name", "last_name", "deleted_at"], + key: "id", +}); + const getNewLoader = (context: boolean = true) => { - return new ObjectLoader( - { - tableName: "users", - fields: ["id", "first_name", "last_name", "deleted_at"], - key: "id", - clause: clause.Eq("deleted_at", null), - }, + return usersLoaderFactory.createLoader( context ? new TestContext() : undefined, ); }; const getAccountNewLoader = (context: boolean = true) => { - return new ObjectLoader( - { - tableName: "accounts", - fields: ["id", "first_name", "last_name", "deleted_at"], - key: "id", - clause: clause.Eq("deleted_at", null), - }, + return accountsLoaderFactory.createLoader( context ? new TestContext() : undefined, ); }; const getContactNewLoader = (context: boolean = true) => { - return new ObjectLoader( - { - tableName: "contacts", - fields: ["id", "first_name", "last_name", "deleted_at"], - key: "id", - clause: clause.Eq("deleted_at", null), - }, + return contactsLoaderFactory.createLoader( context ? new TestContext() : undefined, ); }; @@ -308,34 +328,19 @@ const getContactNewLoader = (context: boolean = true) => { // deleted_at field but no custom_clause // behavior when we're ignoring deleted_at. exception... const getNewLoaderNoCustomClause = (context: boolean = true) => { - return new ObjectLoader( - { - tableName: "users", - fields: ["id", "first_name", "last_name", "deleted_at"], - key: "id", - }, + return usersLoaderFactoryNoClause.createLoader( context ? new TestContext() : undefined, ); }; const getContactNewLoaderNoCustomClause = (context: boolean = true) => { - return new ObjectLoader( - { - tableName: "contacts", - fields: ["id", "first_name", "last_name", "deleted_at"], - key: "id", - }, + return contactsLoaderFactoryNoClause.createLoader( context ? new TestContext() : undefined, ); }; const getAccountNewLoaderNoCustomClause = (context: boolean = true) => { - return new ObjectLoader( - { - tableName: "accounts", - fields: ["id", "first_name", "last_name", "deleted_at"], - key: "id", - }, + return accountsLoaderFactoryNoClause.createLoader( context ? new TestContext() : undefined, ); }; @@ -353,15 +358,19 @@ function transformDeletedAt(row: Data | null) { function getInsertUserAction( map: Map, - viewer: Viewer = new LoggedOutViewer(), + context: Context | undefined, ) { + const viewer = new LoggedOutViewer(context); + return new SimpleAction(viewer, UserSchema, map, WriteOperation.Insert, null); } function getInsertAccountAction( map: Map, - viewer: Viewer = new LoggedOutViewer(), + context: Context | undefined, ) { + const viewer = new LoggedOutViewer(context); + return new SimpleAction( viewer, AccountSchema, @@ -373,8 +382,10 @@ function getInsertAccountAction( function getInsertContactAction( map: Map, - viewer: Viewer = new LoggedOutViewer(), + context: Context | undefined, ) { + const viewer = new LoggedOutViewer(context); + return new SimpleAction( viewer, ContactSchema, @@ -386,14 +397,15 @@ function getInsertContactAction( function commonTests() { test("delete -> update", async () => { + const loader = getNewLoader(); const action = getInsertUserAction( new Map([ ["FirstName", "Jon"], ["LastName", "Snow"], ]), + loader.context, ); const user = await action.saveX(); - const loader = getNewLoader(); const row = await loader.load(user.id); expect(row).toEqual({ @@ -406,7 +418,7 @@ function commonTests() { const d = new Date(); advanceTo(d); const action2 = new SimpleAction( - new LoggedOutViewer(), + new LoggedOutViewer(loader.context), UserSchema, new Map(), WriteOperation.Delete, @@ -415,7 +427,6 @@ function commonTests() { await action2.save(); - loader.clearAll(); const row2 = await loader.load(user.id); expect(row2).toBeNull(); @@ -431,11 +442,13 @@ function commonTests() { }); test("delete -> update with extra writes", async () => { + const loader = getAccountNewLoader(); const action = getInsertAccountAction( new Map([ ["FirstName", "Jon"], ["LastName", "Snow"], ]), + loader.context, ); const account1 = await action.saveX(); @@ -444,6 +457,7 @@ function commonTests() { ["FirstName", "Jon"], ["LastName", "Snow"], ]), + loader.context, ); const account2 = await action2.saveX(); @@ -452,6 +466,7 @@ function commonTests() { ["FirstName", "Jon"], ["LastName", "Snow"], ]), + loader.context, ); action3.builder.orchestrator.addOutboundEdge( account1.id, @@ -471,7 +486,6 @@ function commonTests() { ); const account3 = await action3.saveX(); - const loader = getAccountNewLoader(); const row = await loader.load(account3.id); expect(row).toEqual({ @@ -504,7 +518,7 @@ function commonTests() { expect(inverseEdgeCt).toBe(1); const action4 = new SimpleAction( - new LoggedOutViewer(), + account1.viewer, AccountSchema, new Map(), WriteOperation.Delete, @@ -513,7 +527,6 @@ function commonTests() { await action4.save(); - loader.clearAll(); const row2 = await loader.load(account3.id); expect(row2).toBeNull(); @@ -548,14 +561,15 @@ function commonTests() { }); test("really delete", async () => { + const loader = getNewLoader(); const action = getInsertUserAction( new Map([ ["FirstName", "Jon"], ["LastName", "Snow"], ]), + loader.context, ); const user = await action.saveX(); - const loader = getNewLoader(); const row = await loader.load(user.id); expect(row).toEqual({ @@ -566,7 +580,7 @@ function commonTests() { }); const action2 = new SimpleAction( - new LoggedOutViewer(), + action.viewer, UserSchema, new Map(), WriteOperation.Delete, @@ -576,7 +590,6 @@ function commonTests() { await action2.save(); - loader.clearAll(); const row2 = await loader.load(user.id); expect(row2).toBeNull(); @@ -599,17 +612,19 @@ function commonTests() { }; await verifyRows(0); + + const loader = getNewLoader(); + const action = getInsertUserAction( new Map([ ["FirstName", "Jon"], ["LastName", "Snow"], ]), + loader.context, ); const user = await action.saveX(); await verifyRows(1); - const loader = getNewLoader(); - const row = await loader.load(user.id); expect(row).toEqual({ id: user.id, @@ -641,6 +656,7 @@ function commonTests() { ["FirstName", "Aegon"], ["LastName", "Targaryen"], ]), + loader.context, ); // @ts-ignore action2.transformWrite = tranformJonToAegon; @@ -656,6 +672,7 @@ function commonTests() { ["FirstName", "Sansa"], ["LastName", "Stark"], ]), + loader.context, ); // @ts-ignore action3.transformWrite = tranformJonToAegon; @@ -669,14 +686,15 @@ function commonTests() { }); test("delete -> update. snake_case", async () => { + const loader = getContactNewLoader(); const action = getInsertContactAction( new Map([ ["first_name", "Jon"], ["last_name", "Snow"], ]), + loader.context, ); const contact = await action.saveX(); - const loader = getContactNewLoader(); const row = await loader.load(contact.id); expect(row).toEqual({ @@ -689,7 +707,7 @@ function commonTests() { const d = new Date(); advanceTo(d); const action2 = new SimpleAction( - new LoggedOutViewer(), + action.viewer, ContactSchema, new Map(), WriteOperation.Delete, @@ -698,7 +716,6 @@ function commonTests() { await action2.save(); - loader.clearAll(); const row2 = await loader.load(contact.id); expect(row2).toBeNull(); @@ -714,14 +731,15 @@ function commonTests() { }); test("really delete. snake_case", async () => { + const loader = getContactNewLoader(); const action = getInsertContactAction( new Map([ ["first_name", "Jon"], ["last_name", "Snow"], ]), + loader.context, ); const contact = await action.saveX(); - const loader = getContactNewLoader(); const row = await loader.load(contact.id); expect(row).toEqual({ @@ -732,7 +750,7 @@ function commonTests() { }); const action2 = new SimpleAction( - new LoggedOutViewer(), + action.viewer, ContactSchema, new Map(), WriteOperation.Delete, @@ -742,7 +760,6 @@ function commonTests() { await action2.save(); - loader.clearAll(); const row2 = await loader.load(contact.id); expect(row2).toBeNull(); @@ -763,17 +780,18 @@ function commonTests() { expect(parseInt(res.rows[0].count)).toBe(ct); }; await verifyRows(0); + const loader = getContactNewLoader(); + const action = getInsertContactAction( new Map([ ["first_name", "Jon"], ["last_name", "Snow"], ]), + loader.context, ); const contact = await action.saveX(); await verifyRows(1); - const loader = getContactNewLoader(); - const row = await loader.load(contact.id); expect(row).toEqual({ id: contact.id, @@ -805,6 +823,7 @@ function commonTests() { ["first_name", "Aegon"], ["last_name", "Targaryen"], ]), + loader.context, ); // @ts-ignore action2.transformWrite = tranformJonToAegon; @@ -820,6 +839,7 @@ function commonTests() { ["first_name", "Sansa"], ["last_name", "Stark"], ]), + loader.context, ); // @ts-ignore action3.transformWrite = tranformJonToAegon; @@ -843,17 +863,17 @@ function commonTests() { expect(parseInt(res.rows[0].count)).toBe(ct); }; await verifyRows(0); + const loader = getContactNewLoader(); const action = getInsertContactAction( new Map([ ["first_name", "Jon"], ["last_name", "Snow"], ]), + loader.context, ); const contact = await action.saveX(); await verifyRows(1); - const loader = getContactNewLoader(); - const row = await loader.load(contact.id); expect(row).toEqual({ id: contact.id, @@ -884,6 +904,7 @@ function commonTests() { ["first_name", "Aegon"], ["last_name", "Targaryen"], ]), + loader.context, ); // @ts-ignore action2.transformWrite = tranformJonToAegon; @@ -904,17 +925,17 @@ function commonTests() { expect(parseInt(res.rows[0].count)).toBe(ct); }; await verifyRows(0); + const loader = getContactNewLoader(); const action = getInsertContactAction( new Map([ ["first_name", "Jon"], ["last_name", "Snow"], ]), + loader.context, ); const contact = await action.saveX(); await verifyRows(1); - const loader = getContactNewLoader(); - const row = await loader.load(contact.id); expect(row).toEqual({ id: contact.id, @@ -934,6 +955,7 @@ function commonTests() { ["first_name", "Aegon"], ["last_name", "Targaryen"], ]), + loader.context, ); // @ts-ignore action2.transformWrite = tranformJonToAegon; diff --git a/ts/src/core/base.ts b/ts/src/core/base.ts index 7bdab2037..b3f13b511 100644 --- a/ts/src/core/base.ts +++ b/ts/src/core/base.ts @@ -68,26 +68,24 @@ export interface PrimableLoader extends Loader { primeAll?(d: V): void; } +export type QueryOptions = Required< + Pick +> & + Pick; + interface cache { getLoader(name: string, create: () => Loader): Loader; getLoaderWithLoadMany( name: string, create: () => LoaderWithLoadMany, ): LoaderWithLoadMany; - getCachedRows(options: queryOptions): Data[] | null; - getCachedRow(options: queryOptions): Data | null; - primeCache(options: queryOptions, rows: Data[]): void; - primeCache(options: queryOptions, rows: Data): void; + getCachedRows(options: QueryOptions): Data[] | null; + getCachedRow(options: QueryOptions): Data | null; + primeCache(options: QueryOptions, rows: Data[]): void; + primeCache(options: QueryOptions, rows: Data): void; clearCache(): void; } -interface queryOptions { - fields: string[]; - tableName: string; - clause: clause.Clause; - orderby?: OrderBy; -} - export interface Context { getViewer(): TViewer; // optional per (request)contet @@ -177,7 +175,7 @@ export interface QueryableDataOptions interface JoinOptions { tableName: string; alias?: string; - clause: clause.Clause; + clause: clause.Clause; } export interface QueryDataOptions { @@ -187,7 +185,7 @@ export interface QueryDataOptions { groupby?: K; limit?: number; disableTransformations?: boolean; - join?:JoinOptions; + join?: JoinOptions; } // For loading data from database diff --git a/ts/src/core/clause.ts b/ts/src/core/clause.ts index 61323bc67..e6e874648 100644 --- a/ts/src/core/clause.ts +++ b/ts/src/core/clause.ts @@ -106,7 +106,7 @@ class simpleClause implements Clause { } // NB: we're not using alias in this class in clause method -// if we end up with a subclass that does, we need to handle it +// if we end up with a subclass that does, we need to handle it class queryClause implements Clause { constructor( protected dependentQueryOptions: QueryableDataOptions, // private value: any, // private op: string, // private handleNull?: Clause, @@ -1222,4 +1222,4 @@ export function Expression( expression: string, ): Clause { return new simpleExpression(expression); -} \ No newline at end of file +} diff --git a/ts/src/core/context.ts b/ts/src/core/context.ts index 9814ea10d..21b50ea26 100644 --- a/ts/src/core/context.ts +++ b/ts/src/core/context.ts @@ -1,10 +1,9 @@ -import { Viewer, Data, Loader, LoaderWithLoadMany } from "./base"; +import { Viewer, Data, Loader, LoaderWithLoadMany, QueryOptions } from "./base"; import { IncomingMessage, ServerResponse } from "http"; -import * as clause from "./clause"; import { log } from "./logger"; import { Context } from "./base"; -import { OrderBy, getOrderByPhrase } from "./query_impl"; +import { getJoinPhrase, getOrderByPhrase } from "./query_impl"; // RequestBasedContext e.g. from an HTTP request with a server/response conponent export interface RequestContext @@ -20,6 +19,10 @@ export class ContextCache { // we should eventually combine the two but better for typing to be separate for now loaderWithLoadMany: Map> = new Map(); + // keep track of discarded loaders in case someone ends up holding onto a reference + // so that we can call clearAll() on them + discardedLoaders: Loader[] = []; + getLoader(name: string, create: () => Loader): Loader { let l = this.loaders.get(name); if (l) { @@ -52,7 +55,7 @@ export class ContextCache { // tableName is ignored bcos already indexed on that // maybe we just want to store sql queries??? - private getkey(options: queryOptions): string { + private getkey(options: QueryOptions): string { let parts: string[] = [ options.fields.join(","), options.clause.instanceKey(), @@ -60,10 +63,13 @@ export class ContextCache { if (options.orderby) { parts.push(getOrderByPhrase(options.orderby)); } + if (options.join) { + parts.push(getJoinPhrase(options.join)); + } return parts.join(","); } - getCachedRows(options: queryOptions): Data[] | null { + getCachedRows(options: QueryOptions): Data[] | null { let m = this.listMap.get(options.tableName); if (!m) { return null; @@ -79,7 +85,7 @@ export class ContextCache { return rows || null; } - getCachedRow(options: queryOptions): Data | null { + getCachedRow(options: QueryOptions): Data | null { let m = this.itemMap.get(options.tableName); if (!m) { return null; @@ -95,9 +101,9 @@ export class ContextCache { return row || null; } - primeCache(options: queryOptions, rows: Data[]): void; - primeCache(options: queryOptions, rows: Data): void; - primeCache(options: queryOptions, rows: Data[] | Data): void { + primeCache(options: QueryOptions, rows: Data[]): void; + primeCache(options: QueryOptions, rows: Data): void; + primeCache(options: QueryOptions, rows: Data[] | Data): void { if (Array.isArray(rows)) { let m = this.listMap.get(options.tableName) || new Map(); m.set(this.getkey(options), rows); @@ -110,15 +116,15 @@ export class ContextCache { } clearCache(): void { + this.discardedLoaders.forEach((l) => l.clearAll()); + for (const [_key, loader] of this.loaders) { - // may not need this since we're clearing the loaders themselves... - // but may have some benefits by explicitily doing so? loader.clearAll(); + this.discardedLoaders.push(loader); } for (const [_key, loader] of this.loaderWithLoadMany) { - // may not need this since we're clearing the loaders themselves... - // but may have some benefits by explicitily doing so? loader.clearAll(); + this.discardedLoaders.push(loader); } this.loaders.clear(); this.loaderWithLoadMany.clear(); @@ -126,10 +132,3 @@ export class ContextCache { this.listMap.clear(); } } - -interface queryOptions { - fields: string[]; - tableName: string; - clause: clause.Clause; - orderby?: OrderBy; -} diff --git a/ts/src/core/ent_custom_data.test.ts b/ts/src/core/ent_custom_data.test.ts index 304694ca5..fe30353d9 100644 --- a/ts/src/core/ent_custom_data.test.ts +++ b/ts/src/core/ent_custom_data.test.ts @@ -9,7 +9,6 @@ import { Allow, Skip, LoadCustomEntOptions, - QueryableDataOptions, } from "./base"; import { LoggedOutViewer, IDViewer } from "./viewer"; import { AlwaysDenyRule } from "./privacy"; @@ -257,7 +256,6 @@ beforeEach(() => { }); afterEach(() => { - ctx.cache?.clearCache(); clearLogLevels(); }); diff --git a/ts/src/core/ent_data.test.ts b/ts/src/core/ent_data.test.ts index 8d6b06286..de09acb94 100644 --- a/ts/src/core/ent_data.test.ts +++ b/ts/src/core/ent_data.test.ts @@ -228,7 +228,6 @@ beforeEach(() => { }); afterEach(() => { - ctx.cache?.clearCache(); clearLogLevels(); }); diff --git a/ts/src/core/loaders/assoc_count_loader.test.ts b/ts/src/core/loaders/assoc_count_loader.test.ts index e6dc95a65..72dce4149 100644 --- a/ts/src/core/loaders/assoc_count_loader.test.ts +++ b/ts/src/core/loaders/assoc_count_loader.test.ts @@ -30,15 +30,21 @@ import { setupTempDB, tempDBTables, } from "../../testutils/fake_data/test_helpers"; -import { AssocEdgeCountLoader } from "./assoc_count_loader"; +import { + AssocEdgeCountLoader, + AssocEdgeCountLoaderFactory, +} from "./assoc_count_loader"; import { testEdgeGlobalSchema } from "../../testutils/test_edge_global_schema"; import { SimpleAction } from "../../testutils/builder"; const ml = new MockLogs(); +const assocEdgeLoaderFactory = new AssocEdgeCountLoaderFactory( + EdgeType.UserToContacts, +); + const getNewLoader = (context: boolean = true) => { - return new AssocEdgeCountLoader( - EdgeType.UserToContacts, + return assocEdgeLoaderFactory.createLoader( context ? new TestContext() : undefined, ); }; @@ -47,10 +53,9 @@ const getConfigurableLoader = ( context: boolean = true, opts: EdgeQueryableDataOptionsConfigureLoader, ) => { - return new AssocEdgeCountLoader( - EdgeType.UserToContacts, - context ? new TestContext() : undefined, + return assocEdgeLoaderFactory.createConfigurableLoader( opts, + context ? new TestContext() : undefined, ); }; @@ -262,11 +267,11 @@ async function verifySingleIDHit( verifyPostFirstQuery: (id: ID) => void, verifyPostSecondQuery: (id: ID) => void, ) { - const [user, contacts] = await createAllContacts(); + const loader = loaderFn(); + const [user, contacts] = await createAllContacts({ ctx: loader.context }); // clear post creation ml.clear(); - const loader = loaderFn(); const count = await loader.load(user.id); expect(count).toBe(contacts.length); @@ -337,9 +342,14 @@ async function testMultiQueryDataAvail( const m = new Map(); const ids: ID[] = []; + const loader = loaderFn(); + await Promise.all( [1, 2, 3, 4, 5].map(async (count, idx) => { - const [user, contacts] = await createAllContacts({ slice: count }); + const [user, contacts] = await createAllContacts({ + slice: count, + ctx: loader.context, + }); m.set(user.id, contacts); ids[idx] = user.id; @@ -348,8 +358,6 @@ async function testMultiQueryDataAvail( ml.clear(); - const loader = loaderFn(); - const counts = await Promise.all(ids.map(async (id) => loader.load(id))); for (let i = 0; i < ids.length; i++) { expect( @@ -383,9 +391,15 @@ async function testWithDeleteMultiQueryDataAvail( const ids: ID[] = []; const users: FakeUser[] = []; + // pass context to createAllContacts + const loader = loaderFn(); + await Promise.all( [1, 2, 3, 4, 5].map(async (count, idx) => { - const [user, contacts] = await createAllContacts({ slice: count }); + const [user, contacts] = await createAllContacts({ + slice: count, + ctx: loader.context, + }); m.set(user.id, contacts); ids[idx] = user.id; @@ -395,8 +409,6 @@ async function testWithDeleteMultiQueryDataAvail( ml.clear(); - const loader = loaderFn(); - const counts = await Promise.all(ids.map(async (id) => loader.load(id))); for (let i = 0; i < ids.length; i++) { expect( @@ -424,7 +436,6 @@ async function testWithDeleteMultiQueryDataAvail( await action.saveX(); // clear the logs ml.clear(); - loader.clearAll(); // re-load const counts2 = await Promise.all(ids.map(async (id) => loader.load(id))); @@ -452,9 +463,14 @@ async function testWithDeleteMultiQueryDataLoadDeleted( const ids: ID[] = []; const users: FakeUser[] = []; + const loader = loaderFn({}); + await Promise.all( [1, 2, 3, 4, 5].map(async (count, idx) => { - const [user, contacts] = await createAllContacts({ slice: count }); + const [user, contacts] = await createAllContacts({ + slice: count, + ctx: loader.context, + }); m.set(user.id, contacts); ids[idx] = user.id; @@ -464,8 +480,6 @@ async function testWithDeleteMultiQueryDataLoadDeleted( ml.clear(); - const loader = loaderFn({}); - const counts = await Promise.all(ids.map(async (id) => loader.load(id))); for (let i = 0; i < ids.length; i++) { expect( @@ -493,7 +507,6 @@ async function testWithDeleteMultiQueryDataLoadDeleted( await action.saveX(); // clear the logs ml.clear(); - loader.clearAll(); const loader2 = loaderFn({ disableTransformations: true, @@ -516,12 +529,11 @@ async function testWithDeleteSingleQueryDataLoadDeleted( verifyPostFirstQuery: (ids: ID[]) => void, verifyPostSecondQuery: (ids: ID[], disableTransformations?: boolean) => void, ) { - const [user, contacts] = await createAllContacts(); + const loader = loaderFn({}); + const [user, contacts] = await createAllContacts({ ctx: loader.context }); ml.clear(); - const loader = loaderFn({}); - const counts = await loader.load(user.id); expect(counts).toBe(contacts.length); @@ -543,7 +555,6 @@ async function testWithDeleteSingleQueryDataLoadDeleted( await action.saveX(); // clear the logs ml.clear(); - loader.clearAll(); const loader2 = loaderFn({ disableTransformations: true, diff --git a/ts/src/core/loaders/assoc_edge_loader.test.ts b/ts/src/core/loaders/assoc_edge_loader.test.ts index c42b5832d..4dad5affa 100644 --- a/ts/src/core/loaders/assoc_edge_loader.test.ts +++ b/ts/src/core/loaders/assoc_edge_loader.test.ts @@ -12,7 +12,13 @@ import { } from "../global_schema"; import * as clause from "../clause"; -import { EdgeQueryableDataOptions, ID, Loader, WriteOperation } from "../base"; +import { + Context, + EdgeQueryableDataOptions, + ID, + Loader, + WriteOperation, +} from "../base"; import { setupSqlite, TempDB } from "../../testutils/db/temp_db"; import { FakeUser, @@ -41,31 +47,43 @@ const ml = new MockLogs(); let ctx: TestContext; +const userToContactsLoader = new AssocEdgeLoaderFactory( + EdgeType.UserToContacts, + AssocEdge, +); + +const userToContactsCustomLoader = new AssocEdgeLoaderFactory( + EdgeType.UserToContacts, + CustomEdge, +); + +const userToFollowingCustomLoader = new AssocEdgeLoaderFactory( + EdgeType.UserToFollowing, + CustomEdge, +); + const getNewContactsLoader = (context: boolean = true) => { - return new AssocEdgeLoaderFactory( - EdgeType.UserToContacts, - AssocEdge, - ).createLoader(context ? ctx : undefined); + return userToContactsLoader.createLoader(context ? ctx : undefined); }; const getConfigurableContactsLoader = ( context: boolean, options: EdgeQueryableDataOptions, ) => { - return new AssocEdgeLoaderFactory( - EdgeType.UserToContacts, - CustomEdge, - ).createConfigurableLoader(options, context ? ctx : undefined); + return userToContactsCustomLoader.createConfigurableLoader( + options, + context ? ctx : undefined, + ); }; const getConfigurableFollowingLoader = ( context: boolean, options: EdgeQueryableDataOptions, ) => { - return new AssocEdgeLoaderFactory( - EdgeType.UserToFollowing, - CustomEdge, - ).createConfigurableLoader(options, context ? ctx : undefined); + return userToFollowingCustomLoader.createConfigurableLoader( + options, + context ? ctx : undefined, + ); }; describe("postgres", () => { @@ -452,7 +470,6 @@ function commonTests() { async function verifyTwoWayEdges( loaderFn: (opts: EdgeQueryableDataOptions) => AssocLoader, ) { - // create loader first so we can pass context to createTestUser const loader = loaderFn({}); const user = await createTestUser({}, loader.context); @@ -506,19 +523,16 @@ function commonTests() { i++; } await action.saveX(); - user.viewer.context?.cache?.clearCache(); - loader.clearAll(); - // TODO why isn't this done automatically... const edges2 = await loader.load(user.id); const twoWay2 = await loader.loadTwoWay(user.id); - expect(twowayIds.sort()).toEqual(twoWay2.map((e) => e.id2).sort()); - // deleted some things here which shouldn't show up here expect(edges2.length).toBe(8); expect(twoWay2.length).toBe(3); + expect(twowayIds.sort()).toEqual(twoWay2.map((e) => e.id2).sort()); + const hasGlobal = __hasGlobalSchema(); const loader2 = loaderFn({ @@ -633,10 +647,6 @@ function globalTests() { } await action.saveX(); ml.clear(); - loader.clearAll(); - user.viewer.context?.cache?.clearCache(); - // do edge writes not call mutateRow??? - // why isn't this done automatically... const loader2 = loaderFn({ disableTransformations: true, @@ -668,14 +678,17 @@ interface createdData { users: FakeUser[]; } -async function createData(): Promise { +async function createData(context?: Context): Promise { const m = new Map(); const ids: ID[] = []; const users: FakeUser[] = []; await Promise.all( [1, 2, 3, 4, 5].map(async (count, idx) => { - let [user, contacts] = await createAllContacts({ slice: count }); + let [user, contacts] = await createAllContacts({ + slice: count, + ctx: context, + }); m.set(user.id, contacts.reverse()); ids[idx] = user.id; @@ -692,19 +705,22 @@ async function testMultiQueryDataAvail( slice?: number, data?: createdData, ) { + // TODO this needs to be done prior to the JS event loop + // need to make this work for scenarios where the loader is created in same loop + + // TODOOO + const loader = loaderFn({ + limit: slice, + }); + if (!data) { - data = await createData(); + data = await createData(loader.context); } let { m, ids, users } = data; // clear post creation ml.clear(); - // TODO this needs to be done prior to the JS event loop - // need to make this work for scenarios where the loader is created in same loop - const loader = loaderFn({ - limit: slice, - }); const edges = await Promise.all(ids.map(async (id) => loader.load(id))); ml.verifyNoErrors(); @@ -731,19 +747,20 @@ async function testWithDeleteMultiQueryDataAvail( slice?: number, data?: createdData, ) { + // TODO this needs to be done prior to the JS event loop + // need to make this work for scenarios where the loader is created in same loop + const loader = loaderFn({ + limit: slice, + }); + if (!data) { - data = await createData(); + data = await createData(loader.context); } let { m, ids, users } = data; // clear post creation ml.clear(); - // TODO this needs to be done prior to the JS event loop - // need to make this work for scenarios where the loader is created in same loop - const loader = loaderFn({ - limit: slice, - }); const edges = await Promise.all(ids.map(async (id) => loader.load(id))); ml.verifyNoErrors(); @@ -769,8 +786,6 @@ async function testWithDeleteMultiQueryDataAvail( await action.saveX(); ml.clear(); - // clear loader - loader.clearAll(); // reload data const edges2 = await Promise.all(ids.map(async (id) => loader.load(id))); @@ -792,15 +807,15 @@ async function testWithDeleteMultiQueryDataLoadDeleted( disableTransformations?: boolean, ) => void, ) { - const data = await createData(); + // TODO this needs to be done prior to the JS event loop + // need to make this work for scenarios where the loader is created in same loop + const loader = loaderFn({}); + const data = await createData(loader.context); let { m, ids, users } = data; // clear post creation ml.clear(); - // TODO this needs to be done prior to the JS event loop - // need to make this work for scenarios where the loader is created in same loop - const loader = loaderFn({}); const edges = await Promise.all(ids.map(async (id) => loader.load(id))); ml.verifyNoErrors(); @@ -828,9 +843,6 @@ async function testWithDeleteMultiQueryDataLoadDeleted( await action.saveX(); ml.clear(); - // clear loader - loader.clearAll(); - const loader2 = loaderFn({ disableTransformations: true, }); @@ -859,9 +871,9 @@ async function testWithDeleteSingleQueryDataLoadDeleted( disableTransformations?: boolean, ) => void, ) { - const [user, contacts] = await createAllContacts(); - ml.clear(); const loader = loaderFn({}); + const [user, contacts] = await createAllContacts({ ctx: loader.context }); + ml.clear(); const edges = await loader.load(user.id); verifyUserToContactEdges(user, edges, contacts.reverse()); verifyPostFirstQuery([user.id]); @@ -884,7 +896,6 @@ async function testWithDeleteSingleQueryDataLoadDeleted( } await action.saveX(); ml.clear(); - loader.clearAll(); const loader2 = getConfigurableContactsLoader(true, { disableTransformations: true, diff --git a/ts/src/core/loaders/assoc_edge_loader.ts b/ts/src/core/loaders/assoc_edge_loader.ts index 3570b7d83..84f680b28 100644 --- a/ts/src/core/loaders/assoc_edge_loader.ts +++ b/ts/src/core/loaders/assoc_edge_loader.ts @@ -24,8 +24,6 @@ import { logEnabled } from "../logger"; import { CacheMap, getCustomLoader } from "./loader"; import memoizee from "memoizee"; -// any loader created here or in places like this doesn't get context cache cleared... -// so any manual createLoader needs to be changed and added to ContextCache so that ContextCache.clearAll() works function createLoader( options: EdgeQueryableDataOptions, edgeType: string, diff --git a/ts/src/core/loaders/object_loader.test.ts b/ts/src/core/loaders/object_loader.test.ts index 45aeeb22d..38781ff84 100644 --- a/ts/src/core/loaders/object_loader.test.ts +++ b/ts/src/core/loaders/object_loader.test.ts @@ -35,12 +35,38 @@ interface LoaderRow { first_name: string; } -const getNewLoader = (context: boolean | TestContext = true) => { - return new ObjectLoaderFactory({ +const usersLoaderFactory = new ObjectLoaderFactory({ + tableName: "users", + fields: ["id", "first_name"], + key: "id", +}); + +const usersLoaderFactoryDeleted = + new ObjectLoaderFactory({ tableName: "users", - fields: ["id", "first_name"], + fields: ["id", "first_name", "deleted_at"], + key: "id", + clause: clause.Eq("deleted_at", null), + }); + +const usersLoaderFactoryDeletedNoClause = + new ObjectLoaderFactory({ + tableName: "users", + fields: ["id", "first_name", "deleted_at"], + key: "id", + }); + +const usersLoaderFactoryDeletedFunc = + new ObjectLoaderFactory({ + tableName: "users", + fields: ["id", "first_name", "deleted_at"], key: "id", - }).createLoader( + clause: () => clause.Eq("deleted_at", null), + instanceKey: "users:transformedReadClause", + }); + +const getNewLoader = (context: boolean | TestContext = true) => { + return usersLoaderFactory.createLoader( context ? typeof context === "boolean" ? new TestContext() @@ -50,11 +76,7 @@ const getNewLoader = (context: boolean | TestContext = true) => { }; const getNewCountLoader = (context: boolean | TestContext = true) => { - return new ObjectLoaderFactory({ - tableName: "users", - fields: ["id", "first_name"], - key: "id", - }).createCountLoader( + return usersLoaderFactory.createCountLoader( context ? typeof context === "boolean" ? new TestContext() @@ -70,12 +92,7 @@ interface LoaderRowWithCustomClause extends LoaderRow { const getNewLoaderWithCustomClause = ( context: boolean | TestContext = true, ) => { - return new ObjectLoaderFactory({ - tableName: "users", - fields: ["id", "first_name", "deleted_at"], - key: "id", - clause: clause.Eq("deleted_at", null), - }).createLoader( + return usersLoaderFactoryDeleted.createLoader( context ? typeof context === "boolean" ? new TestContext() @@ -87,12 +104,7 @@ const getNewLoaderWithCustomClause = ( const getNewCountLoaderWithCustomClause = ( context: boolean | TestContext = true, ) => { - return new ObjectLoaderFactory({ - tableName: "users", - fields: ["id", "first_name", "deleted_at"], - key: "id", - clause: clause.Eq("deleted_at", null), - }).createCountLoader( + return usersLoaderFactoryDeleted.createCountLoader( context ? typeof context === "boolean" ? new TestContext() @@ -104,13 +116,7 @@ const getNewCountLoaderWithCustomClause = ( const getNewLoaderWithCustomClauseFunc = ( context: boolean | TestContext = true, ) => { - return new ObjectLoaderFactory({ - tableName: "users", - fields: ["id", "first_name", "deleted_at"], - key: "id", - clause: () => clause.Eq("deleted_at", null), - instanceKey: "users:transformedReadClause", - }).createLoader( + return usersLoaderFactoryDeletedFunc.createLoader( context ? typeof context === "boolean" ? new TestContext() @@ -122,13 +128,7 @@ const getNewLoaderWithCustomClauseFunc = ( const getNewCountLoaderWithCustomClauseFunc = ( context: boolean | TestContext = true, ) => { - return new ObjectLoaderFactory({ - tableName: "users", - fields: ["id", "first_name", "deleted_at"], - key: "id", - clause: () => clause.Eq("deleted_at", null), - instanceKey: "users:transformedReadClause", - }).createCountLoader( + return usersLoaderFactoryDeletedFunc.createCountLoader( context ? typeof context === "boolean" ? new TestContext() @@ -140,12 +140,7 @@ const getNewCountLoaderWithCustomClauseFunc = ( // deleted_at field but no custom_clause // behavior when we're ignoring deleted_at. exception... const getNewLoaderWithDeletedAtField = (context: boolean = true) => { - return new ObjectLoader( - { - tableName: "users", - fields: ["id", "first_name", "deleted_at"], - key: "id", - }, + return usersLoaderFactoryDeletedNoClause.createLoader( context ? new TestContext() : undefined, ); }; @@ -773,9 +768,9 @@ function commonTests() { fields: { deleted_at: new Date(), }, + context: ctx, }); - ctx.cache.clearCache(); const rowPostDelete = await loader.load(1); expect(rowPostDelete).toEqual({ id: 1, first_name: "Jon" }); @@ -946,20 +941,20 @@ function commonTests() { ml.clear(); const ctx = new TestContext(); - const newUserLoader = new ObjectLoaderFactory({ + const newUserLoaderFactory = new ObjectLoaderFactory({ ...FakeUser.loaderOptions(), key: "id", }); - const newUserPhoneLoader = new ObjectLoaderFactory({ + const newUserPhoneLoaderFactory = new ObjectLoaderFactory({ ...FakeUser.loaderOptions(), key: "phone_number", }); - const newUserEmailAddressLoader = new ObjectLoaderFactory({ + const newUserEmailAddressLoaderFactory = new ObjectLoaderFactory({ ...FakeUser.loaderOptions(), key: "email_address", }); - await newUserLoader.createLoader(ctx).load(user.id); + await newUserLoaderFactory.createLoader(ctx).load(user.id); expect(ml.logs.length).toBe(1); @@ -974,7 +969,7 @@ function commonTests() { ml.clear(); - await newUserPhoneLoader.createLoader(ctx).load(user.phoneNumber!); + await newUserPhoneLoaderFactory.createLoader(ctx).load(user.phoneNumber!); expect(ml.logs.length).toBe(1); const phoneQuery = buildQuery({ @@ -990,7 +985,9 @@ function commonTests() { ml.clear(); - await newUserEmailAddressLoader.createLoader(ctx).load(user.emailAddress); + await newUserEmailAddressLoaderFactory + .createLoader(ctx) + .load(user.emailAddress); expect(ml.logs.length).toEqual(1); const emailQuery = buildQuery({ diff --git a/ts/src/core/loaders/query_loader.test.ts b/ts/src/core/loaders/query_loader.test.ts index a6b08d2ab..4f13f2e49 100644 --- a/ts/src/core/loaders/query_loader.test.ts +++ b/ts/src/core/loaders/query_loader.test.ts @@ -87,11 +87,6 @@ describe("postgres", () => { tdb = await setupTempDB(); }); - beforeEach(() => { - // reset context for each test - ctx?.cache.clearCache(); - }); - afterEach(() => { ml.clear(); }); @@ -111,14 +106,6 @@ describe("sqlite", () => { ml.mock(); }); - beforeEach(async () => { - // reset context for each test - ctx?.cache.clearCache(); - - // create once - // await createEdges(); - }); - afterEach(() => { ml.clear(); }); diff --git a/ts/src/core/loaders/raw_count_loader.test.ts b/ts/src/core/loaders/raw_count_loader.test.ts index 2b319b554..c23cdeddf 100644 --- a/ts/src/core/loaders/raw_count_loader.test.ts +++ b/ts/src/core/loaders/raw_count_loader.test.ts @@ -22,12 +22,13 @@ import { clear } from "jest-date-mock"; const ml = new MockLogs(); +const fakeContactsLoader = new RawCountLoaderFactory({ + tableName: "fake_contacts", + groupCol: "user_id", +}); + const getNewLoader = (context: boolean = true) => { - return new RawCountLoader( - { - tableName: "fake_contacts", - groupCol: "user_id", - }, + return fakeContactsLoader.createLoader( context ? new TestContext() : undefined, ); }; @@ -248,11 +249,12 @@ function commonTests() { const INTERVAL = 24 * 60 * 60 * 1000; const getNonGroupableLoader = (id: ID, context: boolean = true) => { - return new RawCountLoader( - { - tableName: "fake_events", - clause: getCompleteClause(id), - }, + // unclear if there's benefit of using factory here but we're consistent with API access patterns this way + const fakeEventsLoaderFactory = new RawCountLoaderFactory({ + tableName: "fake_events", + clause: getCompleteClause(id), + }); + return fakeEventsLoaderFactory.createLoader( context ? new TestContext() : undefined, ); }; diff --git a/ts/src/core/query_impl.ts b/ts/src/core/query_impl.ts index 07f0bdb3d..b2057c4a3 100644 --- a/ts/src/core/query_impl.ts +++ b/ts/src/core/query_impl.ts @@ -34,6 +34,16 @@ export function reverseOrderBy(orderby: OrderBy): OrderBy { }); } +export function getJoinPhrase( + join: NonNullable, + clauseIdx = 1, +): string { + const joinTable = join.alias + ? `${join.tableName} ${join.alias}` + : join.tableName; + return `${joinTable} ON ${join.clause.clause(clauseIdx)}`; +} + export function buildQuery(options: QueryableDataOptions): string { const fields = options.alias ? options.fields.map((f) => `${options.alias}.${f}`).join(", ") @@ -48,11 +58,10 @@ export function buildQuery(options: QueryableDataOptions): string { let whereStart = 1; if (options.join) { - const joinTable = options.join.alias ? `${options.join.tableName} ${options.join.alias}` : options.join.tableName; - parts.push(`JOIN ${joinTable} ON ${options.join.clause.clause(1)}`); + parts.push(`JOIN ${getJoinPhrase(options.join, 1)}`); whereStart += options.join.clause.values().length; } - + parts.push(`WHERE ${options.clause.clause(whereStart, options.alias)}`); if (options.groupby) { parts.push(`GROUP BY ${options.groupby}`);