Skip to content

Commit a9d09f4

Browse files
wschurmanwschurman
authored
fix: Use TSelectedFields for knex loader order by method (#424)
# Why Noticed while creating an upcoming pagination PR that loaders should use `TSelectedFields` for `orderBy` clauses. This was an accidental omission when the selected fields concept was added. Succinctly, one should only be able to order entities by their fields rather than by all their underlying table's fields. # How Create new orderBy clause type that lives at the loader level. It is fully compatible (subset or equal to) the database orderBy clause, so calling the data manager methods directly with the arguments is by design. # Test Plan `yarn tsc`
1 parent a9980b2 commit a9d09f4

5 files changed

Lines changed: 142 additions & 58 deletions

File tree

‎packages/entity-database-adapter-knex/src/AuthorizationResultBasedKnexEntityLoader.ts‎

Lines changed: 75 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,70 @@ import { Result } from '@expo/results';
1111
import {
1212
FieldEqualityCondition,
1313
isSingleValueFieldEqualityCondition,
14-
QuerySelectionModifiers,
15-
QuerySelectionModifiersWithOrderByFragment,
16-
QuerySelectionModifiersWithOrderByRaw,
14+
OrderByOrdering,
1715
} from './BasePostgresEntityDatabaseAdapter';
1816
import { BaseSQLQueryBuilder } from './BaseSQLQueryBuilder';
1917
import { SQLFragment } from './SQLOperator';
2018
import { EntityKnexDataManager } from './internal/EntityKnexDataManager';
2119

20+
export interface EntityLoaderOrderByClause<
21+
TFields extends Record<string, any>,
22+
TSelectedFields extends keyof TFields,
23+
> {
24+
/**
25+
* The field name to order by.
26+
*/
27+
fieldName: TSelectedFields;
28+
29+
/**
30+
* The OrderByOrdering to order by.
31+
*/
32+
order: OrderByOrdering;
33+
}
34+
35+
/**
36+
* SQL modifiers that only affect the selection but not the projection.
37+
*/
38+
export interface EntityLoaderQuerySelectionModifiers<
39+
TFields extends Record<string, any>,
40+
TSelectedFields extends keyof TFields,
41+
> {
42+
/**
43+
* Order the entities by specified columns and orders.
44+
*/
45+
orderBy?: readonly EntityLoaderOrderByClause<TFields, TSelectedFields>[];
46+
47+
/**
48+
* Skip the specified number of entities queried before returning.
49+
*/
50+
offset?: number;
51+
52+
/**
53+
* Limit the number of entities returned.
54+
*/
55+
limit?: number;
56+
}
57+
58+
export interface EntityLoaderQuerySelectionModifiersWithOrderByRaw<
59+
TFields extends Record<string, any>,
60+
TSelectedFields extends keyof TFields,
61+
> extends EntityLoaderQuerySelectionModifiers<TFields, TSelectedFields> {
62+
/**
63+
* Order the entities by a raw SQL `ORDER BY` clause.
64+
*/
65+
orderByRaw?: string;
66+
}
67+
68+
export interface EntityLoaderQuerySelectionModifiersWithOrderByFragment<
69+
TFields extends Record<string, any>,
70+
TSelectedFields extends keyof TFields,
71+
> extends EntityLoaderQuerySelectionModifiers<TFields, TSelectedFields> {
72+
/**
73+
* Order the entities by a SQL fragment `ORDER BY` clause.
74+
*/
75+
orderByFragment?: SQLFragment;
76+
}
77+
2278
/**
2379
* Authorization-result-based knex entity loader for non-data-loader-based load methods.
2480
* All loads through this loader are results (or null for some loader methods), where an
@@ -60,8 +116,11 @@ export class AuthorizationResultBasedKnexEntityLoader<
60116
*/
61117
async loadFirstByFieldEqualityConjunctionAsync<N extends keyof Pick<TFields, TSelectedFields>>(
62118
fieldEqualityOperands: FieldEqualityCondition<TFields, N>[],
63-
querySelectionModifiers: Omit<QuerySelectionModifiers<TFields>, 'limit'> &
64-
Required<Pick<QuerySelectionModifiers<TFields>, 'orderBy'>>,
119+
querySelectionModifiers: Omit<
120+
EntityLoaderQuerySelectionModifiers<TFields, TSelectedFields>,
121+
'limit'
122+
> &
123+
Required<Pick<EntityLoaderQuerySelectionModifiers<TFields, TSelectedFields>, 'orderBy'>>,
65124
): Promise<Result<TEntity> | null> {
66125
const results = await this.loadManyByFieldEqualityConjunctionAsync(fieldEqualityOperands, {
67126
...querySelectionModifiers,
@@ -76,7 +135,7 @@ export class AuthorizationResultBasedKnexEntityLoader<
76135
*/
77136
async loadManyByFieldEqualityConjunctionAsync<N extends keyof Pick<TFields, TSelectedFields>>(
78137
fieldEqualityOperands: FieldEqualityCondition<TFields, N>[],
79-
querySelectionModifiers: QuerySelectionModifiers<TFields> = {},
138+
querySelectionModifiers: EntityLoaderQuerySelectionModifiers<TFields, TSelectedFields> = {},
80139
): Promise<readonly Result<TEntity>[]> {
81140
for (const fieldEqualityOperand of fieldEqualityOperands) {
82141
const fieldValues = isSingleValueFieldEqualityCondition(fieldEqualityOperand)
@@ -101,7 +160,10 @@ export class AuthorizationResultBasedKnexEntityLoader<
101160
async loadManyByRawWhereClauseAsync(
102161
rawWhereClause: string,
103162
bindings: any[] | object,
104-
querySelectionModifiers: QuerySelectionModifiersWithOrderByRaw<TFields> = {},
163+
querySelectionModifiers: EntityLoaderQuerySelectionModifiersWithOrderByRaw<
164+
TFields,
165+
TSelectedFields
166+
> = {},
105167
): Promise<readonly Result<TEntity>[]> {
106168
const fieldObjects = await this.knexDataManager.loadManyByRawWhereClauseAsync(
107169
this.queryContext,
@@ -118,7 +180,10 @@ export class AuthorizationResultBasedKnexEntityLoader<
118180
*/
119181
loadManyBySQL(
120182
fragment: SQLFragment,
121-
modifiers: QuerySelectionModifiersWithOrderByFragment<TFields> = {},
183+
modifiers: EntityLoaderQuerySelectionModifiersWithOrderByFragment<
184+
TFields,
185+
TSelectedFields
186+
> = {},
122187
): AuthorizationResultBasedSQLQueryBuilder<
123188
TFields,
124189
TIDField,
@@ -153,7 +218,7 @@ export class AuthorizationResultBasedSQLQueryBuilder<
153218
TSelectedFields
154219
>,
155220
TSelectedFields extends keyof TFields,
156-
> extends BaseSQLQueryBuilder<TFields, Result<TEntity>> {
221+
> extends BaseSQLQueryBuilder<TFields, TSelectedFields, Result<TEntity>> {
157222
constructor(
158223
private readonly knexDataManager: EntityKnexDataManager<TFields, TIDField>,
159224
private readonly constructionUtils: EntityConstructionUtils<
@@ -166,7 +231,7 @@ export class AuthorizationResultBasedSQLQueryBuilder<
166231
>,
167232
private readonly queryContext: EntityQueryContext,
168233
sqlFragment: SQLFragment,
169-
modifiers: QuerySelectionModifiersWithOrderByFragment<TFields>,
234+
modifiers: EntityLoaderQuerySelectionModifiersWithOrderByFragment<TFields, TSelectedFields>,
170235
) {
171236
super(sqlFragment, modifiers);
172237
}

‎packages/entity-database-adapter-knex/src/BasePostgresEntityDatabaseAdapter.ts‎

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -75,24 +75,26 @@ export enum OrderByOrdering {
7575
DESCENDING = 'desc',
7676
}
7777

78+
export interface PostgresOrderByClause<TFields extends Record<string, any>> {
79+
/**
80+
* The field name to order by.
81+
*/
82+
fieldName: keyof TFields;
83+
84+
/**
85+
* The OrderByOrdering to order by.
86+
*/
87+
order: OrderByOrdering;
88+
}
89+
7890
/**
7991
* SQL modifiers that only affect the selection but not the projection.
8092
*/
81-
export interface QuerySelectionModifiers<TFields extends Record<string, any>> {
93+
export interface PostgresQuerySelectionModifiers<TFields extends Record<string, any>> {
8294
/**
8395
* Order the entities by specified columns and orders.
8496
*/
85-
orderBy?: {
86-
/**
87-
* The field name to order by.
88-
*/
89-
fieldName: keyof TFields;
90-
91-
/**
92-
* The OrderByOrdering to order by.
93-
*/
94-
order: OrderByOrdering;
95-
}[];
97+
orderBy?: readonly PostgresOrderByClause<TFields>[];
9698

9799
/**
98100
* Skip the specified number of entities queried before returning.
@@ -105,18 +107,18 @@ export interface QuerySelectionModifiers<TFields extends Record<string, any>> {
105107
limit?: number;
106108
}
107109

108-
export interface QuerySelectionModifiersWithOrderByRaw<
110+
export interface PostgresQuerySelectionModifiersWithOrderByRaw<
109111
TFields extends Record<string, any>,
110-
> extends QuerySelectionModifiers<TFields> {
112+
> extends PostgresQuerySelectionModifiers<TFields> {
111113
/**
112114
* Order the entities by a raw SQL `ORDER BY` clause.
113115
*/
114116
orderByRaw?: string;
115117
}
116118

117-
export interface QuerySelectionModifiersWithOrderByFragment<
119+
export interface PostgresQuerySelectionModifiersWithOrderByFragment<
118120
TFields extends Record<string, any>,
119-
> extends QuerySelectionModifiers<TFields> {
121+
> extends PostgresQuerySelectionModifiers<TFields> {
120122
/**
121123
* Order the entities by a SQL fragment `ORDER BY` clause.
122124
*/
@@ -159,7 +161,7 @@ export abstract class BasePostgresEntityDatabaseAdapter<
159161
async fetchManyByFieldEqualityConjunctionAsync<N extends keyof TFields>(
160162
queryContext: EntityQueryContext,
161163
fieldEqualityOperands: FieldEqualityCondition<TFields, N>[],
162-
querySelectionModifiers: QuerySelectionModifiers<TFields>,
164+
querySelectionModifiers: PostgresQuerySelectionModifiers<TFields>,
163165
): Promise<readonly Readonly<TFields>[]> {
164166
const tableFieldSingleValueOperands: TableFieldSingleValueEqualityCondition[] = [];
165167
const tableFieldMultipleValueOperands: TableFieldMultiValueEqualityCondition[] = [];
@@ -211,7 +213,7 @@ export abstract class BasePostgresEntityDatabaseAdapter<
211213
queryContext: EntityQueryContext,
212214
rawWhereClause: string,
213215
bindings: any[] | object,
214-
querySelectionModifiers: QuerySelectionModifiersWithOrderByRaw<TFields>,
216+
querySelectionModifiers: PostgresQuerySelectionModifiersWithOrderByRaw<TFields>,
215217
): Promise<readonly Readonly<TFields>[]> {
216218
const results = await this.fetchManyByRawWhereClauseInternalAsync(
217219
queryContext.getQueryInterface(),
@@ -245,7 +247,7 @@ export abstract class BasePostgresEntityDatabaseAdapter<
245247
async fetchManyBySQLFragmentAsync(
246248
queryContext: EntityQueryContext,
247249
sqlFragment: SQLFragment,
248-
querySelectionModifiers: QuerySelectionModifiersWithOrderByFragment<TFields>,
250+
querySelectionModifiers: PostgresQuerySelectionModifiersWithOrderByFragment<TFields>,
249251
): Promise<readonly Readonly<TFields>[]> {
250252
const results = await this.fetchManyBySQLFragmentInternalAsync(
251253
queryContext.getQueryInterface(),
@@ -267,7 +269,7 @@ export abstract class BasePostgresEntityDatabaseAdapter<
267269
): Promise<object[]>;
268270

269271
private convertToTableQueryModifiersWithOrderByRaw(
270-
querySelectionModifiers: QuerySelectionModifiersWithOrderByRaw<TFields>,
272+
querySelectionModifiers: PostgresQuerySelectionModifiersWithOrderByRaw<TFields>,
271273
): TableQuerySelectionModifiersWithOrderByRaw {
272274
return {
273275
...this.convertToTableQueryModifiers(querySelectionModifiers),
@@ -276,7 +278,7 @@ export abstract class BasePostgresEntityDatabaseAdapter<
276278
}
277279

278280
private convertToTableQueryModifiersWithOrderByFragment(
279-
querySelectionModifiers: QuerySelectionModifiersWithOrderByFragment<TFields>,
281+
querySelectionModifiers: PostgresQuerySelectionModifiersWithOrderByFragment<TFields>,
280282
): TableQuerySelectionModifiersWithOrderByFragment {
281283
return {
282284
...this.convertToTableQueryModifiers(querySelectionModifiers),
@@ -285,7 +287,7 @@ export abstract class BasePostgresEntityDatabaseAdapter<
285287
}
286288

287289
private convertToTableQueryModifiers(
288-
querySelectionModifiers: QuerySelectionModifiers<TFields>,
290+
querySelectionModifiers: PostgresQuerySelectionModifiers<TFields>,
289291
): TableQuerySelectionModifiers {
290292
const orderBy = querySelectionModifiers.orderBy;
291293
return {

‎packages/entity-database-adapter-knex/src/BaseSQLQueryBuilder.ts‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,26 @@
11
import {
2-
OrderByOrdering,
3-
QuerySelectionModifiersWithOrderByFragment,
4-
} from './BasePostgresEntityDatabaseAdapter';
2+
EntityLoaderOrderByClause,
3+
EntityLoaderQuerySelectionModifiersWithOrderByFragment,
4+
} from './AuthorizationResultBasedKnexEntityLoader';
5+
import { OrderByOrdering } from './BasePostgresEntityDatabaseAdapter';
56
import { SQLFragment } from './SQLOperator';
67

78
/**
89
* Base SQL query builder that provides common functionality for building SQL queries.
910
*/
10-
export abstract class BaseSQLQueryBuilder<TFields extends Record<string, any>, TResultType> {
11+
export abstract class BaseSQLQueryBuilder<
12+
TFields extends Record<string, any>,
13+
TSelectedFields extends keyof TFields,
14+
TResultType,
15+
> {
1116
private executed = false;
1217

1318
constructor(
1419
private readonly sqlFragment: SQLFragment,
1520
private readonly modifiers: {
1621
limit?: number;
1722
offset?: number;
18-
orderBy?: { fieldName: keyof TFields; order: OrderByOrdering }[];
23+
orderBy?: readonly EntityLoaderOrderByClause<TFields, TSelectedFields>[];
1924
orderByFragment?: SQLFragment;
2025
},
2126
) {}
@@ -39,7 +44,7 @@ export abstract class BaseSQLQueryBuilder<TFields extends Record<string, any>, T
3944
/**
4045
* Order by a field. Can be called multiple times to add multiple order bys.
4146
*/
42-
orderBy(fieldName: keyof TFields, order: OrderByOrdering = OrderByOrdering.ASCENDING): this {
47+
orderBy(fieldName: TSelectedFields, order: OrderByOrdering = OrderByOrdering.ASCENDING): this {
4348
this.modifiers.orderBy = [...(this.modifiers.orderBy ?? []), { fieldName, order }];
4449
return this;
4550
}
@@ -71,7 +76,10 @@ export abstract class BaseSQLQueryBuilder<TFields extends Record<string, any>, T
7176
/**
7277
* Get the current modifiers as QuerySelectionModifiersWithOrderByFragment<TFields>
7378
*/
74-
protected getModifiers(): QuerySelectionModifiersWithOrderByFragment<TFields> {
79+
protected getModifiers(): EntityLoaderQuerySelectionModifiersWithOrderByFragment<
80+
TFields,
81+
TSelectedFields
82+
> {
7583
return this.modifiers;
7684
}
7785

‎packages/entity-database-adapter-knex/src/EnforcingKnexEntityLoader.ts‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
import { EntityPrivacyPolicy, ReadonlyEntity, ViewerContext } from '@expo/entity';
22

3-
import { AuthorizationResultBasedKnexEntityLoader } from './AuthorizationResultBasedKnexEntityLoader';
43
import {
5-
FieldEqualityCondition,
6-
QuerySelectionModifiers,
7-
QuerySelectionModifiersWithOrderByFragment,
8-
QuerySelectionModifiersWithOrderByRaw,
9-
} from './BasePostgresEntityDatabaseAdapter';
4+
AuthorizationResultBasedKnexEntityLoader,
5+
EntityLoaderQuerySelectionModifiers,
6+
EntityLoaderQuerySelectionModifiersWithOrderByFragment,
7+
EntityLoaderQuerySelectionModifiersWithOrderByRaw,
8+
} from './AuthorizationResultBasedKnexEntityLoader';
9+
import { FieldEqualityCondition } from './BasePostgresEntityDatabaseAdapter';
1010
import { BaseSQLQueryBuilder } from './BaseSQLQueryBuilder';
1111
import { SQLFragment } from './SQLOperator';
1212

@@ -52,8 +52,11 @@ export class EnforcingKnexEntityLoader<
5252
*/
5353
async loadFirstByFieldEqualityConjunctionAsync<N extends keyof Pick<TFields, TSelectedFields>>(
5454
fieldEqualityOperands: FieldEqualityCondition<TFields, N>[],
55-
querySelectionModifiers: Omit<QuerySelectionModifiers<TFields>, 'limit'> &
56-
Required<Pick<QuerySelectionModifiers<TFields>, 'orderBy'>>,
55+
querySelectionModifiers: Omit<
56+
EntityLoaderQuerySelectionModifiers<TFields, TSelectedFields>,
57+
'limit'
58+
> &
59+
Required<Pick<EntityLoaderQuerySelectionModifiers<TFields, TSelectedFields>, 'orderBy'>>,
5760
): Promise<TEntity | null> {
5861
const entityResult = await this.knexEntityLoader.loadFirstByFieldEqualityConjunctionAsync(
5962
fieldEqualityOperands,
@@ -74,7 +77,7 @@ export class EnforcingKnexEntityLoader<
7477
*/
7578
async loadManyByFieldEqualityConjunctionAsync<N extends keyof Pick<TFields, TSelectedFields>>(
7679
fieldEqualityOperands: FieldEqualityCondition<TFields, N>[],
77-
querySelectionModifiers: QuerySelectionModifiers<TFields> = {},
80+
querySelectionModifiers: EntityLoaderQuerySelectionModifiers<TFields, TSelectedFields> = {},
7881
): Promise<readonly TEntity[]> {
7982
const entityResults = await this.knexEntityLoader.loadManyByFieldEqualityConjunctionAsync(
8083
fieldEqualityOperands,
@@ -116,7 +119,10 @@ export class EnforcingKnexEntityLoader<
116119
async loadManyByRawWhereClauseAsync(
117120
rawWhereClause: string,
118121
bindings: any[] | object,
119-
querySelectionModifiers: QuerySelectionModifiersWithOrderByRaw<TFields> = {},
122+
querySelectionModifiers: EntityLoaderQuerySelectionModifiersWithOrderByRaw<
123+
TFields,
124+
TSelectedFields
125+
> = {},
120126
): Promise<readonly TEntity[]> {
121127
const entityResults = await this.knexEntityLoader.loadManyByRawWhereClauseAsync(
122128
rawWhereClause,
@@ -147,7 +153,10 @@ export class EnforcingKnexEntityLoader<
147153
*/
148154
loadManyBySQL(
149155
fragment: SQLFragment,
150-
modifiers: QuerySelectionModifiersWithOrderByFragment<TFields> = {},
156+
modifiers: EntityLoaderQuerySelectionModifiersWithOrderByFragment<
157+
TFields,
158+
TSelectedFields
159+
> = {},
151160
): EnforcingSQLQueryBuilder<
152161
TFields,
153162
TIDField,
@@ -177,7 +186,7 @@ export class EnforcingSQLQueryBuilder<
177186
TSelectedFields
178187
>,
179188
TSelectedFields extends keyof TFields,
180-
> extends BaseSQLQueryBuilder<TFields, TEntity> {
189+
> extends BaseSQLQueryBuilder<TFields, TSelectedFields, TEntity> {
181190
constructor(
182191
private readonly knexEntityLoader: AuthorizationResultBasedKnexEntityLoader<
183192
TFields,
@@ -188,7 +197,7 @@ export class EnforcingSQLQueryBuilder<
188197
TSelectedFields
189198
>,
190199
sqlFragment: SQLFragment,
191-
modifiers: QuerySelectionModifiersWithOrderByFragment<TFields>,
200+
modifiers: EntityLoaderQuerySelectionModifiersWithOrderByFragment<TFields, TSelectedFields>,
192201
) {
193202
super(sqlFragment, modifiers);
194203
}

0 commit comments

Comments
 (0)