Skip to content

Commit 778aa14

Browse files
authored
Merge pull request #22 from jacksonkasi1/dev
fix(engine): add join column support for ?select= parameter
2 parents 7be361d + d9b6d0e commit 778aa14

File tree

7 files changed

+290
-73
lines changed

7 files changed

+290
-73
lines changed

packages/engine/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@tablecraft/engine",
3-
"version": "0.1.4",
3+
"version": "0.1.5",
44
"description": "The backend query engine for TableCraft",
55
"type": "module",
66
"main": "dist/index.js",

packages/engine/src/core/fieldSelector.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Column, SQL } from 'drizzle-orm';
22
import { TableConfig } from '../types/table';
3+
import { collectSelectableJoinFields } from '../utils/joinUtils';
34

45
/**
56
* Filters the selection to only include requested fields.
@@ -17,11 +18,17 @@ export class FieldSelector {
1718
): Record<string, SQL | Column> {
1819
if (!requestedFields?.length) return selection;
1920

20-
// Build a whitelist of allowed (non-hidden) fields
21+
// Build a whitelist of allowed (non-hidden) fields — base columns first
2122
const allowed = new Set(
2223
config.columns.filter((c) => !c.hidden).map((c) => c.name)
2324
);
2425

26+
// Also allow selectable fields from joined tables so that
27+
// ?select=joinColumn correctly includes them in the SQL selection.
28+
if (config.joins?.length) {
29+
collectSelectableJoinFields(config.joins, allowed);
30+
}
31+
2532
const filtered: Record<string, SQL | Column> = {};
2633

2734
for (const field of requestedFields) {

packages/engine/src/core/filterBuilder.ts

Lines changed: 3 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { TableConfig, JoinConfig, ColumnConfig } from '../types/table';
99
import { FilterParam } from '../types/engine';
1010
import { applyOperator } from '../utils/operators';
1111
import { isDatePreset, buildDatePresetCondition } from './datePresets';
12+
import { collectFilterableJoinFields, isJoinColumnInJoins } from '../utils/joinUtils';
1213

1314
// Internal: resolved column reference plus the config entry it came from
1415
interface ResolvedColumn {
@@ -19,7 +20,7 @@ interface ResolvedColumn {
1920
}
2021

2122
export class FilterBuilder {
22-
constructor(private schema: Record<string, unknown>) {}
23+
constructor(private schema: Record<string, unknown>) { }
2324

2425
/**
2526
* Builds dynamic WHERE conditions from user-provided filter params.
@@ -118,7 +119,7 @@ export class FilterBuilder {
118119
for (const filter of config.filters) {
119120
if (filter.type !== 'static' || filter.value === undefined) continue;
120121

121-
const isJoinField = isJoinColumn(config, filter.field);
122+
const isJoinField = isJoinColumnInJoins(config.joins ?? [], filter.field);
122123
const resolved = this.resolveColumn(config, baseColumns, filter.field, isJoinField);
123124

124125
if (!resolved) {
@@ -218,45 +219,3 @@ export class FilterBuilder {
218219
}
219220
}
220221

221-
// ---------------------------------------------------------------------------
222-
// Module-level helpers (pure, no `this` dependency)
223-
// ---------------------------------------------------------------------------
224-
225-
/**
226-
* Recursively populates `out` with filterable field names from all join configs.
227-
*/
228-
function collectFilterableJoinFields(joins: JoinConfig[], out: Set<string>): void {
229-
for (const join of joins) {
230-
if (join.columns) {
231-
for (const col of join.columns) {
232-
if (col.filterable !== false) {
233-
out.add(col.name);
234-
}
235-
}
236-
}
237-
if (join.joins) {
238-
collectFilterableJoinFields(join.joins, out);
239-
}
240-
}
241-
}
242-
243-
/**
244-
* Returns true if `fieldName` is defined in any join config (recursively).
245-
* Used to prevent base-table columns from shadowing join columns.
246-
*/
247-
function isJoinColumn(config: TableConfig, fieldName: string): boolean {
248-
if (!config.joins) return false;
249-
return isJoinColumnInJoins(config.joins, fieldName);
250-
}
251-
252-
function isJoinColumnInJoins(joins: JoinConfig[], fieldName: string): boolean {
253-
for (const join of joins) {
254-
if (join.columns?.some(c => c.name === fieldName)) {
255-
return true;
256-
}
257-
if (join.joins && isJoinColumnInJoins(join.joins, fieldName)) {
258-
return true;
259-
}
260-
}
261-
return false;
262-
}

packages/engine/src/core/inputValidator.ts

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { TableConfig, ColumnConfig } from '../types/table';
22
import { EngineParams, FilterParam } from '../types/engine';
33
import { ValidationError, FieldError } from '../errors';
4+
import { collectSortableJoinFields, collectSelectableJoinFields } from '../utils/joinUtils';
45

56
/**
67
* Validates request params against the table config BEFORE hitting the database.
@@ -15,15 +16,17 @@ export function validateInput(params: EngineParams, config: TableConfig): void {
1516
function validateSelectFields(params: EngineParams, config: TableConfig): void {
1617
if (!params.select?.length) return;
1718

18-
const validNames = new Set(config.columns.map((c) => c.name));
19+
// Build valid set from base columns (non-hidden only)
20+
const validNames = new Set(config.columns.filter((c) => !c.hidden).map((c) => c.name));
21+
22+
// Also allow selectable fields from joined tables
23+
if (config.joins?.length) {
24+
collectSelectableJoinFields(config.joins, validNames);
25+
}
1926

2027
for (const field of params.select) {
2128
if (!validNames.has(field)) {
22-
throw new FieldError(field, 'does not exist. Available: ' + [...validNames].join(', '));
23-
}
24-
const col = config.columns.find((c) => c.name === field);
25-
if (col?.hidden) {
26-
throw new FieldError(field, 'is not accessible');
29+
throw new FieldError(field, 'does not exist or is not accessible. Available: ' + [...validNames].join(', '));
2730
}
2831
}
2932
}
@@ -137,7 +140,9 @@ function validateSortFields(params: EngineParams, config: TableConfig): void {
137140

138141
// Also collect sortable fields from joins (same pattern as FilterBuilder
139142
// uses collectFilterableJoinFields for filterable fields)
140-
collectSortableJoinFields(config.joins, sortable);
143+
if (config.joins?.length) {
144+
collectSortableJoinFields(config.joins, sortable);
145+
}
141146

142147
for (const s of params.sort) {
143148
if (!sortable.has(s.field)) {
@@ -146,26 +151,7 @@ function validateSortFields(params: EngineParams, config: TableConfig): void {
146151
}
147152
}
148153

149-
/** Recursively collects sortable column names from join configs. */
150-
function collectSortableJoinFields(
151-
joins: TableConfig['joins'],
152-
sortable: Set<string>
153-
): void {
154-
if (!joins?.length) return;
155-
for (const join of joins) {
156-
if (join.columns) {
157-
for (const col of join.columns) {
158-
if (col.sortable !== false) {
159-
sortable.add(col.name);
160-
}
161-
}
162-
}
163-
// Recurse into nested joins
164-
if (join.joins) {
165-
collectSortableJoinFields(join.joins, sortable);
166-
}
167-
}
168-
}
154+
169155

170156
function isValidUUID(str: string): boolean {
171157
return /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(str);

packages/engine/src/utils/joinUtils.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,41 @@ export function collectSortableJoinFields(joins: JoinConfig[], out: Set<string>)
1616
}
1717
}
1818

19+
/** Recursively populates `out` with filterable field names from all join configs. */
20+
export function collectFilterableJoinFields(joins: JoinConfig[], out: Set<string>): void {
21+
for (const join of joins) {
22+
if (join.columns) {
23+
for (const col of join.columns) {
24+
if (col.filterable !== false) {
25+
out.add(col.name);
26+
}
27+
}
28+
}
29+
if (join.joins) {
30+
collectFilterableJoinFields(join.joins, out);
31+
}
32+
}
33+
}
34+
35+
/**
36+
* Recursively populates `out` with selectable (non-hidden) field names from all join configs.
37+
* Used by FieldSelector and validateSelectFields so that ?select=joinColumn works correctly.
38+
*/
39+
export function collectSelectableJoinFields(joins: JoinConfig[], out: Set<string>): void {
40+
for (const join of joins) {
41+
if (join.columns) {
42+
for (const col of join.columns) {
43+
if (!col.hidden) {
44+
out.add(col.name);
45+
}
46+
}
47+
}
48+
if (join.joins) {
49+
collectSelectableJoinFields(join.joins, out);
50+
}
51+
}
52+
}
53+
1954
/**
2055
* Returns true if `fieldName` is defined in any join config (recursively).
2156
*/
Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { FieldSelector } from '../../src/core/fieldSelector';
3+
import { TableConfig } from '../../src/types/table';
4+
import type { Column, SQL } from 'drizzle-orm';
5+
6+
// ── Minimal column stubs ──
7+
// FieldSelector only checks presence in `selection` object keys —
8+
// the actual Drizzle Column value doesn't matter for this unit test.
9+
function makeCol(name: string): SQL {
10+
return { name } as unknown as SQL;
11+
}
12+
13+
const selector = new FieldSelector();
14+
15+
// ── Base-only config ──
16+
const baseConfig: TableConfig = {
17+
name: 'users',
18+
base: 'users',
19+
columns: [
20+
{ name: 'id', type: 'uuid', hidden: false, sortable: true, filterable: true },
21+
{ name: 'name', type: 'string', hidden: false, sortable: true, filterable: true },
22+
{ name: 'secret', type: 'string', hidden: true, sortable: false, filterable: false },
23+
],
24+
};
25+
26+
// ── Config with a join ──
27+
const joinConfig: TableConfig = {
28+
name: 'orders',
29+
base: 'orders',
30+
columns: [
31+
{ name: 'id', type: 'number', hidden: false, sortable: true, filterable: true },
32+
{ name: 'status', type: 'string', hidden: false, sortable: true, filterable: true },
33+
],
34+
joins: [{
35+
table: 'users',
36+
type: 'left',
37+
on: 'orders.user_id = users.id',
38+
columns: [
39+
{ name: 'email', type: 'string', hidden: false, sortable: true, filterable: true },
40+
{ name: 'role', type: 'string', hidden: false, sortable: false, filterable: true },
41+
{ name: 'internalNote', type: 'string', hidden: true, sortable: false, filterable: false },
42+
],
43+
}],
44+
};
45+
46+
// ── Full selection objects (simulates what queryBuilder.buildSelect returns) ──
47+
const baseSelection = {
48+
id: makeCol('id'),
49+
name: makeCol('name'),
50+
secret: makeCol('secret'),
51+
};
52+
53+
const joinSelection = {
54+
id: makeCol('id'),
55+
status: makeCol('status'),
56+
email: makeCol('email'),
57+
role: makeCol('role'),
58+
internalNote: makeCol('internalNote'),
59+
};
60+
61+
// ═══════════════════════════════════════════════════════════
62+
// applyFieldSelection — base table only
63+
// ═══════════════════════════════════════════════════════════
64+
describe('FieldSelector.applyFieldSelection — base columns', () => {
65+
it('returns full selection when no fields requested', () => {
66+
const result = selector.applyFieldSelection(baseSelection, undefined, baseConfig);
67+
expect(Object.keys(result)).toEqual(['id', 'name', 'secret']);
68+
});
69+
70+
it('narrows selection to requested base columns', () => {
71+
const result = selector.applyFieldSelection(baseSelection, ['name'], baseConfig);
72+
// PK guard always injects 'id'; exact membership must be correct
73+
expect(Object.keys(result).sort()).toEqual(['id', 'name'].sort());
74+
});
75+
76+
it('excludes hidden base columns even when explicitly requested', () => {
77+
const result = selector.applyFieldSelection(baseSelection, ['secret'], baseConfig);
78+
// 'secret' is hidden — filtered is empty after the loop, but the PK guard
79+
// injects 'id', so the result contains only the primary key.
80+
expect(Object.keys(result)).toEqual(['id']);
81+
});
82+
});
83+
84+
// ═══════════════════════════════════════════════════════════
85+
// applyFieldSelection — join columns (the previously-broken path)
86+
// ═══════════════════════════════════════════════════════════
87+
describe('FieldSelector.applyFieldSelection — join columns (bug fix)', () => {
88+
it('includes a valid join column in the narrowed selection', () => {
89+
const result = selector.applyFieldSelection(joinSelection, ['id', 'email'], joinConfig);
90+
// Exact: only the requested fields plus PK (already included in request)
91+
expect(Object.keys(result).sort()).toEqual(['email', 'id'].sort());
92+
});
93+
94+
it('includes base and join columns together', () => {
95+
const result = selector.applyFieldSelection(joinSelection, ['status', 'email', 'role'], joinConfig);
96+
// Exact: requested fields + PK guard injects 'id'
97+
expect(Object.keys(result).sort()).toEqual(['email', 'id', 'role', 'status'].sort());
98+
});
99+
100+
it('excludes a hidden join column even if requested', () => {
101+
const result = selector.applyFieldSelection(joinSelection, ['internalNote'], joinConfig);
102+
// 'internalNote' is hidden — filtered is empty after the loop, but the PK guard
103+
// injects 'id', so the result contains only the primary key.
104+
expect(Object.keys(result)).toEqual(['id']);
105+
});
106+
107+
it('includes join columns from a nested join', () => {
108+
const nestedConfig: TableConfig = {
109+
name: 'orders',
110+
base: 'orders',
111+
columns: [
112+
{ name: 'id', type: 'number', hidden: false, sortable: true, filterable: true },
113+
],
114+
joins: [{
115+
table: 'users',
116+
type: 'left',
117+
on: 'orders.user_id = users.id',
118+
columns: [],
119+
joins: [{
120+
table: 'profiles',
121+
type: 'left',
122+
on: 'users.id = profiles.user_id',
123+
columns: [
124+
{ name: 'avatarUrl', type: 'string', hidden: false, sortable: false, filterable: false },
125+
],
126+
}],
127+
}],
128+
};
129+
130+
const selection = {
131+
id: makeCol('id'),
132+
avatarUrl: makeCol('avatarUrl'),
133+
};
134+
135+
const result = selector.applyFieldSelection(selection, ['id', 'avatarUrl'], nestedConfig);
136+
// Exact: both requested fields (no PK injection since 'id' already in request)
137+
expect(Object.keys(result).sort()).toEqual(['avatarUrl', 'id'].sort());
138+
});
139+
});
140+
141+
// ═══════════════════════════════════════════════════════════
142+
// filterResponseFields — post-fetch field filtering (unchanged by this fix)
143+
// ═══════════════════════════════════════════════════════════
144+
describe('FieldSelector.filterResponseFields', () => {
145+
const rows = [
146+
{ id: 1, name: 'Alice', email: 'alice@example.com', secret: 'x' },
147+
{ id: 2, name: 'Bob', email: 'bob@example.com', secret: 'y' },
148+
];
149+
150+
it('returns all rows unchanged when no fields requested', () => {
151+
const result = selector.filterResponseFields(rows, undefined);
152+
expect(result).toEqual(rows);
153+
});
154+
155+
it('strips fields not in requested set', () => {
156+
const result = selector.filterResponseFields(rows, ['id', 'email']);
157+
expect(result[0]).toEqual({ id: 1, email: 'alice@example.com' });
158+
expect(result[1]).toEqual({ id: 2, email: 'bob@example.com' });
159+
});
160+
161+
it('silently ignores requested fields not present in row', () => {
162+
const result = selector.filterResponseFields(rows, ['id', 'nonexistent']);
163+
expect(result[0]).toEqual({ id: 1 });
164+
});
165+
});

0 commit comments

Comments
 (0)