Skip to content

Commit 2590b95

Browse files
authored
feat!: Move knex-specific loader methods to knexLoader (#407)
# Why This is a long stack, but the goal is to make a place to put postgres-specific loading logic to avoid polluting the core dataloader-based library with logic specific to knex/postgres. And potentially even postgres-specific mutation logic but that's way down the road. The best place for this stuff is in the `entity-database-adapter-knex` library. Therefore, the strategy is (in PR order): 1. Move existing knex/postgres-specific loader methods to a new loader, `knexLoader`. I'm also open to calling this a "Postgres Loader" since the `entity-database-adapter-knex` package already uses postgres-specific queries (and more are added here since this stack eventually produces raw queries) within knex even though technically knex is database agnostic. 1. Move knex-specific logic out of EntityDataManager, move it to its own EntityKnexDataManager. 1. Move these two now-separated pieces into the `entity-database-adapter-knex` package. This requires creating a "plugin" system for database adapters, where when they are included their methods are added to the prototype so that seamless loading continues to work. 1. Add a codemod for these three pieces. 1. Add the first new feature to entity loading, load by tagged template string, which creates a better/safer way to express raw queries for loading entities. Future other things that could be added here (after thorough thought on authorization) are: - Pagination - Create, update, delete by tagged template string - Batch creation - Upsert (maybe) # For Reviewers The most critical things to assess are: - "install" method in #410 - sql tagged template API usability in #414 # How This is part 1. It refactors the loaders by moving `loadManyByFieldEqualityConjunctionAsync`, `loadFirstByFieldEqualityConjunctionAsync`, and `loadManyByRawWhereClauseAsync` to a separate loader. The new pattern for these is: ``` await PostgresTestEntity.knexLoader(vc1).loadManyByRawWhereClauseAsync(...) ``` # Test Plan This has full coverage in new tests.
1 parent bc74562 commit 2590b95

25 files changed

Lines changed: 1201 additions & 662 deletions

packages/entity-database-adapter-knex/src/__integration-tests__/PostgresEntityIntegration-test.ts

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,9 @@ describe('postgres entity integration', () => {
390390
.createAsync(),
391391
);
392392

393-
const results = await PostgresTestEntity.loader(vc1).loadManyByFieldEqualityConjunctionAsync([
393+
const results = await PostgresTestEntity.knexLoader(
394+
vc1,
395+
).loadManyByFieldEqualityConjunctionAsync([
394396
{
395397
fieldName: 'hasACat',
396398
fieldValue: false,
@@ -403,9 +405,11 @@ describe('postgres entity integration', () => {
403405

404406
expect(results).toHaveLength(2);
405407

406-
const results2 = await PostgresTestEntity.loader(vc1).loadManyByFieldEqualityConjunctionAsync(
407-
[{ fieldName: 'hasADog', fieldValues: [true, false] }],
408-
);
408+
const results2 = await PostgresTestEntity.knexLoader(
409+
vc1,
410+
).loadManyByFieldEqualityConjunctionAsync([
411+
{ fieldName: 'hasADog', fieldValues: [true, false] },
412+
]);
409413
expect(results2).toHaveLength(3);
410414
});
411415

@@ -424,19 +428,18 @@ describe('postgres entity integration', () => {
424428
PostgresTestEntity.creatorWithAuthorizationResults(vc1).setField('name', 'c').createAsync(),
425429
);
426430

427-
const results = await PostgresTestEntity.loader(vc1).loadManyByFieldEqualityConjunctionAsync(
428-
[],
429-
{
430-
limit: 2,
431-
offset: 1,
432-
orderBy: [
433-
{
434-
fieldName: 'name',
435-
order: OrderByOrdering.DESCENDING,
436-
},
437-
],
438-
},
439-
);
431+
const results = await PostgresTestEntity.knexLoader(
432+
vc1,
433+
).loadManyByFieldEqualityConjunctionAsync([], {
434+
limit: 2,
435+
offset: 1,
436+
orderBy: [
437+
{
438+
fieldName: 'name',
439+
order: OrderByOrdering.DESCENDING,
440+
},
441+
],
442+
});
440443
expect(results).toHaveLength(2);
441444
expect(results.map((e) => e.getField('name'))).toEqual(['b', 'a']);
442445
});
@@ -468,13 +471,15 @@ describe('postgres entity integration', () => {
468471
.createAsync(),
469472
);
470473

471-
const results = await PostgresTestEntity.loader(vc1).loadManyByFieldEqualityConjunctionAsync([
472-
{ fieldName: 'name', fieldValue: null },
473-
]);
474+
const results = await PostgresTestEntity.knexLoader(
475+
vc1,
476+
).loadManyByFieldEqualityConjunctionAsync([{ fieldName: 'name', fieldValue: null }]);
474477
expect(results).toHaveLength(2);
475478
expect(results[0]!.getField('name')).toBeNull();
476479

477-
const results2 = await PostgresTestEntity.loader(vc1).loadManyByFieldEqualityConjunctionAsync(
480+
const results2 = await PostgresTestEntity.knexLoader(
481+
vc1,
482+
).loadManyByFieldEqualityConjunctionAsync(
478483
[
479484
{ fieldName: 'name', fieldValues: ['a', null] },
480485
{ fieldName: 'hasADog', fieldValue: true },
@@ -504,7 +509,7 @@ describe('postgres entity integration', () => {
504509
.createAsync(),
505510
);
506511

507-
const results = await PostgresTestEntity.loader(vc1).loadManyByRawWhereClauseAsync(
512+
const results = await PostgresTestEntity.knexLoader(vc1).loadManyByRawWhereClauseAsync(
508513
'name = ?',
509514
['hello'],
510515
);
@@ -523,7 +528,7 @@ describe('postgres entity integration', () => {
523528
);
524529

525530
await expect(
526-
PostgresTestEntity.loader(vc1).loadManyByRawWhereClauseAsync('invalid_column = ?', [
531+
PostgresTestEntity.knexLoader(vc1).loadManyByRawWhereClauseAsync('invalid_column = ?', [
527532
'hello',
528533
]),
529534
).rejects.toThrow();
@@ -553,7 +558,7 @@ describe('postgres entity integration', () => {
553558
.createAsync(),
554559
);
555560

556-
const results = await PostgresTestEntity.loader(vc1).loadManyByRawWhereClauseAsync(
561+
const results = await PostgresTestEntity.knexLoader(vc1).loadManyByRawWhereClauseAsync(
557562
'has_a_dog = ?',
558563
[true],
559564
{
@@ -571,7 +576,7 @@ describe('postgres entity integration', () => {
571576
expect(results).toHaveLength(2);
572577
expect(results.map((e) => e.getField('name'))).toEqual(['b', 'c']);
573578

574-
const resultsMultipleOrderBy = await PostgresTestEntity.loader(
579+
const resultsMultipleOrderBy = await PostgresTestEntity.knexLoader(
575580
vc1,
576581
).loadManyByRawWhereClauseAsync('has_a_dog = ?', [true], {
577582
orderBy: [
@@ -589,13 +594,11 @@ describe('postgres entity integration', () => {
589594
expect(resultsMultipleOrderBy).toHaveLength(3);
590595
expect(resultsMultipleOrderBy.map((e) => e.getField('name'))).toEqual(['c', 'b', 'a']);
591596

592-
const resultsOrderByRaw = await PostgresTestEntity.loader(vc1).loadManyByRawWhereClauseAsync(
593-
'has_a_dog = ?',
594-
[true],
595-
{
596-
orderByRaw: 'has_a_dog ASC, name DESC',
597-
},
598-
);
597+
const resultsOrderByRaw = await PostgresTestEntity.knexLoader(
598+
vc1,
599+
).loadManyByRawWhereClauseAsync('has_a_dog = ?', [true], {
600+
orderByRaw: 'has_a_dog ASC, name DESC',
601+
});
599602

600603
expect(resultsOrderByRaw).toHaveLength(3);
601604
expect(resultsOrderByRaw.map((e) => e.getField('name'))).toEqual(['c', 'b', 'a']);

packages/entity-secondary-cache-local-memory/src/__tests__/LocalMemorySecondaryEntityCache-test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ class TestSecondaryLocalMemoryCacheLoader extends EntitySecondaryCacheLoader<
173173
}
174174
return nullthrows(
175175
(
176-
await this.entityLoader.loadManyByFieldEqualityConjunctionAsync([
176+
await this.knexEntityLoader.loadManyByFieldEqualityConjunctionAsync([
177177
{ fieldName: 'id', fieldValue: loadParams.id },
178178
])
179179
)[0],
@@ -198,6 +198,7 @@ describe(LocalMemorySecondaryEntityCache, () => {
198198
createTTLCache<LocalMemoryTestEntityFields>(),
199199
),
200200
LocalMemoryTestEntity.loaderWithAuthorizationResults(viewerContext),
201+
LocalMemoryTestEntity.knexLoaderWithAuthorizationResults(viewerContext),
201202
);
202203

203204
const loadParams = { id: createdEntity.getID() };
@@ -234,6 +235,7 @@ describe(LocalMemorySecondaryEntityCache, () => {
234235
createTTLCache<LocalMemoryTestEntityFields>(),
235236
),
236237
LocalMemoryTestEntity.loaderWithAuthorizationResults(viewerContext),
238+
LocalMemoryTestEntity.knexLoaderWithAuthorizationResults(viewerContext),
237239
);
238240

239241
const loadParams = { id: FAKE_ID };

packages/entity-secondary-cache-redis/src/__integration-tests__/RedisSecondaryEntityCache-integration-test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class TestSecondaryRedisCacheLoader extends EntitySecondaryCacheLoader<
4545
}
4646
return nullthrows(
4747
(
48-
await this.entityLoader.loadManyByFieldEqualityConjunctionAsync([
48+
await this.knexEntityLoader.loadManyByFieldEqualityConjunctionAsync([
4949
{ fieldName: 'id', fieldValue: loadParams.id },
5050
])
5151
)[0],
@@ -98,6 +98,7 @@ describe(RedisSecondaryEntityCache, () => {
9898
(loadParams) => `test-key-${loadParams.id}`,
9999
),
100100
RedisTestEntity.loaderWithAuthorizationResults(viewerContext),
101+
RedisTestEntity.knexLoaderWithAuthorizationResults(viewerContext),
101102
);
102103

103104
const loadParams = { id: createdEntity.getID() };
@@ -137,6 +138,7 @@ describe(RedisSecondaryEntityCache, () => {
137138
(loadParams) => `test-key-${loadParams.id}`,
138139
),
139140
RedisTestEntity.loaderWithAuthorizationResults(viewerContext),
141+
RedisTestEntity.knexLoaderWithAuthorizationResults(viewerContext),
140142
);
141143

142144
const loadParams = { id: FAKE_ID };

packages/entity/src/AuthorizationResultBasedEntityLoader.ts

Lines changed: 2 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,11 @@ import {
88
EntityCompositeFieldValue,
99
EntityConfiguration,
1010
} from './EntityConfiguration';
11-
import {
12-
FieldEqualityCondition,
13-
isSingleValueFieldEqualityCondition,
14-
QuerySelectionModifiers,
15-
QuerySelectionModifiersWithOrderByRaw,
16-
} from './EntityDatabaseAdapter';
1711
import { EntityLoaderUtils } from './EntityLoaderUtils';
1812
import { EntityPrivacyPolicy } from './EntityPrivacyPolicy';
1913
import { EntityQueryContext } from './EntityQueryContext';
2014
import { ReadonlyEntity } from './ReadonlyEntity';
2115
import { ViewerContext } from './ViewerContext';
22-
import { EntityInvalidFieldValueError } from './errors/EntityInvalidFieldValueError';
2316
import { EntityNotFoundError } from './errors/EntityNotFoundError';
2417
import { CompositeFieldHolder, CompositeFieldValueHolder } from './internal/CompositeFieldHolder';
2518
import { CompositeFieldValueMap } from './internal/CompositeFieldValueMap';
@@ -265,87 +258,14 @@ export class AuthorizationResultBasedEntityLoader<
265258
});
266259
}
267260

268-
/**
269-
* Authorization-result-based version of the EnforcingEntityLoader method by the same name.
270-
* @returns the first entity results that matches the query, where result error can be
271-
* UnauthorizedError
272-
*/
273-
async loadFirstByFieldEqualityConjunctionAsync<N extends keyof Pick<TFields, TSelectedFields>>(
274-
fieldEqualityOperands: FieldEqualityCondition<TFields, N>[],
275-
querySelectionModifiers: Omit<QuerySelectionModifiers<TFields>, 'limit'> &
276-
Required<Pick<QuerySelectionModifiers<TFields>, 'orderBy'>>,
277-
): Promise<Result<TEntity> | null> {
278-
const results = await this.loadManyByFieldEqualityConjunctionAsync(fieldEqualityOperands, {
279-
...querySelectionModifiers,
280-
limit: 1,
281-
});
282-
return results[0] ?? null;
283-
}
284-
285-
/**
286-
* Authorization-result-based version of the EnforcingEntityLoader method by the same name.
287-
* @returns array of entity results that match the query, where result error can be UnauthorizedError
288-
*/
289-
async loadManyByFieldEqualityConjunctionAsync<N extends keyof Pick<TFields, TSelectedFields>>(
290-
fieldEqualityOperands: FieldEqualityCondition<TFields, N>[],
291-
querySelectionModifiers: QuerySelectionModifiers<TFields> = {},
292-
): Promise<readonly Result<TEntity>[]> {
293-
for (const fieldEqualityOperand of fieldEqualityOperands) {
294-
const fieldValues = isSingleValueFieldEqualityCondition(fieldEqualityOperand)
295-
? [fieldEqualityOperand.fieldValue]
296-
: fieldEqualityOperand.fieldValues;
297-
this.validateFieldAndValues(fieldEqualityOperand.fieldName, fieldValues);
298-
}
299-
300-
const fieldObjects = await this.dataManager.loadManyByFieldEqualityConjunctionAsync(
301-
this.queryContext,
302-
fieldEqualityOperands,
303-
querySelectionModifiers,
304-
);
305-
return await this.utils.constructAndAuthorizeEntitiesArrayAsync(fieldObjects);
306-
}
307-
308-
/**
309-
* Authorization-result-based version of the EnforcingEntityLoader method by the same name.
310-
* @returns array of entity results that match the query, where result error can be UnauthorizedError
311-
* @throws Error when rawWhereClause or bindings are invalid
312-
*/
313-
async loadManyByRawWhereClauseAsync(
314-
rawWhereClause: string,
315-
bindings: any[] | object,
316-
querySelectionModifiers: QuerySelectionModifiersWithOrderByRaw<TFields> = {},
317-
): Promise<readonly Result<TEntity>[]> {
318-
const fieldObjects = await this.dataManager.loadManyByRawWhereClauseAsync(
319-
this.queryContext,
320-
rawWhereClause,
321-
bindings,
322-
querySelectionModifiers,
323-
);
324-
return await this.utils.constructAndAuthorizeEntitiesArrayAsync(fieldObjects);
325-
}
326-
327-
private validateFieldAndValues<N extends keyof Pick<TFields, TSelectedFields>>(
328-
fieldName: N,
329-
fieldValues: readonly TFields[N][],
330-
): void {
331-
const fieldDefinition = this.entityConfiguration.schema.get(fieldName);
332-
invariant(fieldDefinition, `must have field definition for field = ${String(fieldName)}`);
333-
for (const fieldValue of fieldValues) {
334-
const isInputValid = fieldDefinition.validateInputValue(fieldValue);
335-
if (!isInputValid) {
336-
throw new EntityInvalidFieldValueError(this.entityClass, fieldName, fieldValue);
337-
}
338-
}
339-
}
340-
341261
private validateFieldAndValuesAndConvertToHolders<N extends keyof Pick<TFields, TSelectedFields>>(
342262
fieldName: N,
343263
fieldValues: readonly NonNullable<TFields[N]>[],
344264
): {
345265
loadKey: SingleFieldHolder<TFields, TIDField, N>;
346266
loadValues: readonly SingleFieldValueHolder<TFields, N>[];
347267
} {
348-
this.validateFieldAndValues(fieldName, fieldValues);
268+
this.utils.validateFieldAndValues(fieldName, fieldValues);
349269

350270
return {
351271
loadKey: new SingleFieldHolder<TFields, TIDField, N>(fieldName),
@@ -388,7 +308,7 @@ export class AuthorizationResultBasedEntityLoader<
388308
);
389309
for (const field of compositeField) {
390310
const fieldValue = compositeFieldValueHolder.compositeFieldValue[field];
391-
this.validateFieldAndValues(field, [fieldValue]);
311+
this.utils.validateFieldAndValues(field, [fieldValue]);
392312
}
393313
}
394314

0 commit comments

Comments
 (0)