Skip to content

Commit 60df2c1

Browse files
committed
fix: make /aggregate execute before datasource request hooks
1 parent fbca7c1 commit 60df2c1

4 files changed

Lines changed: 242 additions & 45 deletions

File tree

‎adminforth/modules/restApi.ts‎

Lines changed: 58 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1889,14 +1889,14 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
18891889
server.endpoint({
18901890
method: 'POST',
18911891
path: '/aggregate',
1892-
description: 'Performs aggregation queries (sum, count, avg, min, max, median) on a resource, with optional grouping by field value or date truncation. Requires list and show access to the resource, and only accepts columns the user can read on the show view: backend-only columns and columns hidden from the show view are rejected.',
1892+
description: 'Performs aggregation queries (sum, count, avg, min, max, median) on a resource, with optional grouping by field value or date truncation. Requires list and show access to the resource, and only accepts columns the user can read on the show view: backend-only columns and columns hidden from the show view are rejected. Runs the resource list.beforeDatasourceRequest and list.afterDatasourceResponse hooks.',
18931893
agent: {
18941894
onlyReadsData: true,
18951895
},
18961896
request_schema: aggregateRequestSchema,
18971897
response_schema: aggregateResponseSchema,
1898-
handler: async ({ body, adminUser, headers }) => {
1899-
const { resourceId, aggregations, filters, groupBy } = body;
1898+
handler: async ({ body, adminUser, headers, query, cookies, requestUrl }) => {
1899+
const { resourceId } = body;
19001900
if (!this.adminforth.statuses.dbDiscover) {
19011901
return { error: 'Database discovery not started' };
19021902
}
@@ -1907,7 +1907,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
19071907
if (!resource) {
19081908
return { error: `Resource ${resourceId} not found` };
19091909
}
1910-
const rawFilterError = rejectApiRawFilters(filters);
1910+
const rawFilterError = rejectApiRawFilters(body.filters);
19111911
if (rawFilterError) {
19121912
return rawFilterError;
19131913
}
@@ -1966,7 +1966,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
19661966
return null;
19671967
};
19681968

1969-
for (const [alias, rule] of Object.entries((aggregations || {}) as { [alias: string]: IAggregationRule })) {
1969+
for (const [alias, rule] of Object.entries((body.aggregations || {}) as { [alias: string]: IAggregationRule })) {
19701970
// plain count does not reference any column
19711971
if (!rule?.field) {
19721972
continue;
@@ -1977,7 +1977,7 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
19771977
}
19781978
}
19791979

1980-
const groupByRules = Array.isArray(groupBy) ? groupBy : (groupBy ? [groupBy] : []);
1980+
const groupByRules = Array.isArray(body.groupBy) ? body.groupBy : (body.groupBy ? [body.groupBy] : []);
19811981
for (const groupByRule of groupByRules) {
19821982
if (!groupByRule?.field) {
19831983
continue;
@@ -1988,11 +1988,32 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
19881988
}
19891989
}
19901990

1991-
const backendOnlyFilterError = await rejectBackendOnlyFilters(filters, columnCtx);
1991+
const backendOnlyFilterError = await rejectBackendOnlyFilters(body.filters, columnCtx);
19921992
if (backendOnlyFilterError) {
19931993
return backendOnlyFilterError;
19941994
}
19951995

1996+
// aggregation reads the same set of records as the list, so the list hooks scope it
1997+
for (const hook of listify(resource.hooks?.list?.beforeDatasourceRequest as BeforeDataSourceRequestFunction[])) {
1998+
const filterTools = filtersTools.get(body);
1999+
body.filtersTools = filterTools;
2000+
const resp = await (hook as BeforeDataSourceRequestFunction)({
2001+
resource,
2002+
query: body,
2003+
adminUser,
2004+
filtersTools: filterTools,
2005+
extra: {
2006+
body, query, headers, cookies, requestUrl
2007+
},
2008+
adminforth: this.adminforth,
2009+
});
2010+
const hookRespError = hookResponseError(resp);
2011+
if (hookRespError) {
2012+
return hookRespError;
2013+
}
2014+
}
2015+
const { aggregations, filters, groupBy } = body;
2016+
19962017
// normalize filters same way as get_resource_data
19972018
const normalizedFilters = { operator: AdminForthFilterOperators.AND, subFilters: [] };
19982019
if (filters) {
@@ -2011,25 +2032,45 @@ export default class AdminForthRestAPI implements IAdminForthRestAPI {
20112032
}
20122033
}
20132034

2035+
const userTimeZone = headers['X-TimeZone'];
2036+
const applyUserTimeZone = (groupByRule: any) => groupByRule?.type === 'date_trunc' && userTimeZone
2037+
? { ...groupByRule, timezone: userTimeZone }
2038+
: groupByRule;
2039+
const aggregateGroupBy = Array.isArray(groupBy)
2040+
? groupBy.map(applyUserTimeZone)
2041+
: applyUserTimeZone(groupBy);
2042+
2043+
let data;
20142044
try {
2015-
const userTimeZone = headers['X-TimeZone'];
2016-
const applyUserTimeZone = (groupByRule: any) => groupByRule?.type === 'date_trunc' && userTimeZone
2017-
? { ...groupByRule, timezone: userTimeZone }
2018-
: groupByRule;
2019-
const aggregateGroupBy = Array.isArray(groupBy)
2020-
? groupBy.map(applyUserTimeZone)
2021-
: applyUserTimeZone(groupBy);
2022-
2023-
const data = await this.adminforth.connectors[resource.dataSource].aggregate({
2045+
data = await this.adminforth.connectors[resource.dataSource].aggregate({
20242046
resource,
20252047
filters: normalizedFilters as IAdminForthAndOrFilter,
20262048
aggregations,
20272049
groupBy: aggregateGroupBy,
20282050
});
2029-
return { data };
20302051
} catch (e) {
20312052
return { error: e.message };
20322053
}
2054+
2055+
// hooks get the aggregation rows as response, they can tell this call by query.aggregations
2056+
for (const hook of listify(resource.hooks?.list?.afterDatasourceResponse)) {
2057+
const resp = await hook({
2058+
resource,
2059+
query: body,
2060+
response: data,
2061+
adminUser,
2062+
extra: {
2063+
body, query, headers, cookies, requestUrl
2064+
},
2065+
adminforth: this.adminforth,
2066+
});
2067+
const hookRespError = hookResponseError(resp);
2068+
if (hookRespError) {
2069+
return hookRespError;
2070+
}
2071+
}
2072+
2073+
return { data };
20332074
},
20342075
});
20352076

‎documentation/docs/tutorial/03-Customization/04-hooks.md‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,34 @@ When you enrich records with extra fields for display, declare a matching [virtu
205205
Reusing the same name keeps the enriched value tied to a column the rest of AdminForth knows about. In particular, the [Agent plugin](/docs/tutorial/Plugins/agent/) selects records by column name when answering questions — if the hook writes to a different field than the virtual column, the agent selects the (empty) virtual column and never sees your enriched value.
206206
:::
207207

208+
### Aggregations
209+
210+
The `aggregate` API, which agents and [MCP clients](/docs/tutorial/Plugins/mcp/) use for counts, sums and other statistics, reads the same records as the list, so it runs the same `list.beforeDatasourceRequest` and `list.afterDatasourceResponse` hooks. Filters added in `list.beforeDatasourceRequest`, like the `realtor_id` filter above, limit aggregations too, so a user can't count or sum records the list hides from them.
211+
212+
`list.afterDatasourceResponse` gets aggregation rows in `response` instead of records, for example `[{ group: 'Kyiv', total: 12 }]`. If your hook changes record fields, skip aggregation requests, they are the only ones with `query.aggregations`:
213+
214+
```ts title='./resources/apartments.ts'
215+
{
216+
...
217+
hooks: {
218+
list: {
219+
afterDatasourceResponse: async ({ query, response }: { query: any, response: any }) => {
220+
//diff-add
221+
if (query.aggregations) {
222+
//diff-add
223+
return { ok: true };
224+
//diff-add
225+
}
226+
response.forEach((r: any) => {
227+
r.price = `$${r.price}`;
228+
});
229+
return { ok: true };
230+
},
231+
},
232+
},
233+
}
234+
```
235+
208236
### Dropdown list of foreignResource
209237

210238
By default if there is `foreignResource` like we use for demo on `realtor_id` column, the filter will suggest a

‎tests/application/pnpm-lock.yaml‎

Lines changed: 8 additions & 28 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
import { mkdtempSync, rmSync } from 'node:fs';
2+
import { createRequire } from 'node:module';
3+
import os from 'node:os';
4+
import path from 'node:path';
5+
import { fileURLToPath, pathToFileURL } from 'node:url';
6+
7+
const currentDir = path.dirname(fileURLToPath(import.meta.url));
8+
const applicationDir = path.resolve(currentDir, '../application');
9+
10+
// resolves to the BUILT package (adminforth/dist) the test application runs on;
11+
// rebuild it (`npx tsc` in adminforth/) after editing adminforth sources or the assertions below test stale code
12+
const require = createRequire(import.meta.url);
13+
// jest's resolver does not know the `node:sqlite` builtin, so it is required directly
14+
const { DatabaseSync } = require('node:sqlite');
15+
const adminforthEntry = require.resolve('adminforth', { paths: [applicationDir] });
16+
const { default: AdminForth } = await import(pathToFileURL(adminforthEntry).href);
17+
18+
const fixtureDir = mkdtempSync(path.join(os.tmpdir(), 'af-aggregate-hooks-'));
19+
const dbPath = path.join(fixtureDir, 'db.sqlite');
20+
const db = new DatabaseSync(dbPath);
21+
db.exec(`
22+
CREATE TABLE users (id INTEGER PRIMARY KEY, email TEXT NOT NULL, password_hash TEXT NOT NULL);
23+
CREATE TABLE orders (id INTEGER PRIMARY KEY, owner TEXT NOT NULL, amount INTEGER NOT NULL);
24+
INSERT INTO orders VALUES (1, 'alice', 10);
25+
INSERT INTO orders VALUES (2, 'alice', 20);
26+
INSERT INTO orders VALUES (3, 'bob', 5);
27+
`);
28+
db.close();
29+
30+
process.env.ADMINFORTH_SECRET ??= 'x'.repeat(64);
31+
32+
const afterHookCalls: { query: any, response: any }[] = [];
33+
34+
delete (globalThis as any).adminforth;
35+
const app = new AdminForth({
36+
baseUrl: '',
37+
auth: {
38+
usersResourceId: 'users',
39+
usernameField: 'email',
40+
passwordHashField: 'password_hash',
41+
},
42+
dataSources: [{ id: 'sqlite', url: `sqlite://${dbPath}` }],
43+
resources: [
44+
{
45+
resourceId: 'users',
46+
table: 'users',
47+
dataSource: 'sqlite',
48+
columns: [
49+
{ name: 'id', primaryKey: true },
50+
{ name: 'email' },
51+
{ name: 'password_hash', backendOnly: true },
52+
],
53+
},
54+
{
55+
resourceId: 'orders',
56+
table: 'orders',
57+
dataSource: 'sqlite',
58+
columns: [
59+
{ name: 'id', primaryKey: true },
60+
{ name: 'owner' },
61+
{ name: 'amount' },
62+
],
63+
hooks: {
64+
list: {
65+
beforeDatasourceRequest: async ({ query, adminUser }) => {
66+
if (adminUser.dbUser.role === 'blocked') {
67+
return { ok: false, error: 'Blocked by hook' };
68+
}
69+
if (adminUser.dbUser.role !== 'superadmin') {
70+
query.filtersTools.replaceOrAddTopFilter({ field: 'owner', operator: 'eq', value: adminUser.username });
71+
}
72+
return { ok: true };
73+
},
74+
afterDatasourceResponse: async ({ query, response }) => {
75+
afterHookCalls.push({ query, response });
76+
if (query.aggregations) {
77+
for (const row of response) {
78+
row.currency = 'USD';
79+
}
80+
}
81+
return { ok: true };
82+
},
83+
},
84+
},
85+
},
86+
],
87+
menu: [{ label: 'Orders', resourceId: 'orders' }],
88+
});
89+
await app.discoverDatabases();
90+
91+
const endpoints: any[] = [];
92+
app.restApi.registerEndpoints({ endpoint: (config: any) => endpoints.push(config) });
93+
94+
beforeEach(() => {
95+
afterHookCalls.length = 0;
96+
});
97+
98+
afterAll(() => {
99+
rmSync(fixtureDir, { recursive: true, force: true });
100+
});
101+
102+
function aggregate(body: Record<string, unknown>, username: string, role = 'admin') {
103+
const { handler } = endpoints.find((endpoint) => endpoint.path === '/aggregate');
104+
return handler({
105+
body: { resourceId: 'orders', ...body },
106+
adminUser: { pk: '1', username, dbUser: { role } },
107+
headers: {},
108+
query: {},
109+
cookies: {},
110+
requestUrl: '/aggregate',
111+
abortSignal: new AbortController().signal,
112+
});
113+
}
114+
115+
const totals = { aggregations: { orders: { operation: 'count' }, revenue: { operation: 'sum', field: 'amount' } } };
116+
117+
describe('aggregate list hooks', () => {
118+
it('scopes rows with filters added by list.beforeDatasourceRequest', async () => {
119+
expect((await aggregate(totals, 'alice')).data).toEqual([
120+
{ orders: 2, revenue: 30, currency: 'USD' },
121+
]);
122+
expect((await aggregate(totals, 'root', 'superadmin')).data).toEqual([
123+
{ orders: 3, revenue: 35, currency: 'USD' },
124+
]);
125+
});
126+
127+
it('applies hook filters on top of the request filters and groupBy', async () => {
128+
const result = await aggregate({
129+
...totals,
130+
filters: [{ field: 'amount', operator: 'gt', value: 10 }],
131+
groupBy: { type: 'field', field: 'owner', as: 'owner' },
132+
}, 'alice');
133+
expect(result.data).toEqual([{ owner: 'alice', orders: 1, revenue: 20, currency: 'USD' }]);
134+
});
135+
136+
it('stops when list.beforeDatasourceRequest returns an error', async () => {
137+
expect(await aggregate(totals, 'alice', 'blocked')).toEqual({ error: 'Blocked by hook' });
138+
expect(afterHookCalls).toEqual([]);
139+
});
140+
141+
it('passes the request and the aggregation rows to list.afterDatasourceResponse', async () => {
142+
await aggregate(totals, 'bob');
143+
expect(afterHookCalls).toEqual([{
144+
query: expect.objectContaining({ aggregations: totals.aggregations }),
145+
response: [{ orders: 1, revenue: 5, currency: 'USD' }],
146+
}]);
147+
});
148+
});

0 commit comments

Comments
 (0)