Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/engine-read-context-options.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"@objectstack/objectql": minor
---

engine: accept execution context via the trailing `options` argument on the read
methods (`find` / `findOne` / `count` / `aggregate`), aligning them with the
write methods (`insert` / `update`).

Previously reads took context only inside the query (`query.context`) while
writes took it in a trailing `options.context`. The same `{ context }` object was
therefore correct as the 3rd argument to `insert` but **silently dropped** as the
3rd argument to `find` — a recurring footgun where an intended `isSystem` bypass
just vanished (e.g. control-plane reads returning empty once org-scoping hooks
were added). Now "execution context goes in the trailing `options` argument" is a
single rule across reads and writes. `query.context` remains fully supported; when
both are supplied, `options.context` wins.
47 changes: 47 additions & 0 deletions packages/objectql/src/engine.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,53 @@ describe('ObjectQL Engine', () => {
});
});

describe('execution context via the trailing options arg (read methods)', () => {
// Regression: reads took context inside the query while writes took it in
// a trailing options arg — so `find(obj, q, { context })` silently dropped
// the context (e.g. an intended isSystem bypass), a real source of
// "empty result once scoping was added" bugs. Reads now ALSO accept the
// trailing options.context, aligning with insert/update. `tenantId` is a
// convenient observable: buildDriverOptions forwards it to the driver.
beforeEach(async () => {
engine.registerDriver(mockDriver, true);
await engine.init();
vi.mocked(SchemaRegistry.getObject).mockReturnValue({ name: 'task', fields: {} });
});

const lastFindOpts = () => (mockDriver.find as any).mock.calls.at(-1)?.[2];

it('find: context from the trailing options arg reaches the driver', async () => {
await engine.find('task', { filters: [] }, { context: { tenantId: 't-opts' } as any });
expect(lastFindOpts()).toMatchObject({ tenantId: 't-opts' });
});

it('find: context inside the query still works (legacy form)', async () => {
await engine.find('task', { filters: [], context: { tenantId: 't-query' } as any });
expect(lastFindOpts()).toMatchObject({ tenantId: 't-query' });
});

it('find: when both are given, the trailing options.context wins', async () => {
await engine.find(
'task',
{ context: { tenantId: 't-query' } as any },
{ context: { tenantId: 't-opts' } as any },
);
expect(lastFindOpts()).toMatchObject({ tenantId: 't-opts' });
});

it('findOne accepts context via the trailing options arg', async () => {
(mockDriver.findOne as any).mockResolvedValue({ id: '1' });
await engine.findOne('task', { filters: [] }, { context: { tenantId: 't-fo' } as any });
expect((mockDriver.findOne as any).mock.calls.at(-1)?.[2]).toMatchObject({ tenantId: 't-fo' });
});

it('count accepts context via the trailing options arg', async () => {
(mockDriver.count as any).mockResolvedValue(0);
await engine.count('task', {}, { context: { tenantId: 't-cnt' } as any });
expect((mockDriver.count as any).mock.calls.at(-1)?.[2]).toMatchObject({ tenantId: 't-cnt' });
});
});

describe('Update hooks — previous-record snapshot', () => {
beforeEach(async () => {
engine.registerDriver(mockDriver, true);
Expand Down
44 changes: 36 additions & 8 deletions packages/objectql/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,34 @@ export interface OperationContext {
result?: any;
}

/**
* Trailing options for the READ methods (find / findOne / count / aggregate).
*
* Historically the read methods took their execution context INSIDE the query
* (`query.context`), while the WRITE methods (insert / update) took it in a
* trailing `options.context`. That split was a footgun: the same `{ context }`
* object is correct as the 3rd arg to `insert` but was SILENTLY DROPPED as the
* 3rd arg to `find` — a class of bugs where an intended `isSystem` bypass just
* vanished (e.g. control-plane reads coming back empty once org-scoping hooks
* were added). We now ALSO accept `context` via this trailing options arg on the
* read methods, so "execution context goes in the trailing options argument" is
* one rule across reads and writes. `query.context` remains supported; when both
* are given, `options.context` wins (it is the explicit channel).
*/
export interface EngineReadOptions {
context?: ExecutionContext;
}

/** Merge read-path execution context from the query and the trailing options. */
function mergeReadContext(
fromQuery?: ExecutionContext,
fromOptions?: ExecutionContext,
): ExecutionContext | undefined {
if (fromOptions == null) return fromQuery;
if (fromQuery == null) return fromOptions;
return { ...fromQuery, ...fromOptions };
}

/**
* Engine Middleware (Onion model)
*/
Expand Down Expand Up @@ -1766,7 +1794,7 @@ export class ObjectQL implements IDataEngine {
// Data Access Methods (IDataEngine Interface)
// ============================================

async find(object: string, query?: EngineQueryOptions): Promise<any[]> {
async find(object: string, query?: EngineQueryOptions, options?: EngineReadOptions): Promise<any[]> {
object = this.resolveObjectName(object);
this.logger.debug('Find operation starting', { object, query });
const driver = this.getDriver(object);
Expand Down Expand Up @@ -1818,7 +1846,7 @@ export class ObjectQL implements IDataEngine {
operation: 'find',
ast,
options: query,
context: query?.context,
context: mergeReadContext(query?.context, options?.context),
};

await this.executeWithMiddleware(opCtx, async () => {
Expand Down Expand Up @@ -1864,7 +1892,7 @@ export class ObjectQL implements IDataEngine {
return opCtx.result as any[];
}

async findOne(objectName: string, query?: EngineQueryOptions): Promise<any> {
async findOne(objectName: string, query?: EngineQueryOptions, options?: EngineReadOptions): Promise<any> {
objectName = this.resolveObjectName(objectName);
this.logger.debug('FindOne operation', { objectName });
const driver = this.getDriver(objectName);
Expand Down Expand Up @@ -1896,7 +1924,7 @@ export class ObjectQL implements IDataEngine {
operation: 'findOne',
ast,
options: query,
context: query?.context,
context: mergeReadContext(query?.context, options?.context),
};

await this.executeWithMiddleware(opCtx, async () => {
Expand Down Expand Up @@ -2341,15 +2369,15 @@ export class ObjectQL implements IDataEngine {
return opCtx.result;
}

async count(object: string, query?: EngineCountOptions): Promise<number> {
async count(object: string, query?: EngineCountOptions, options?: EngineReadOptions): Promise<number> {
object = this.resolveObjectName(object);
const driver = this.getDriver(object);

const opCtx: OperationContext = {
object,
operation: 'count',
options: query,
context: query?.context,
context: mergeReadContext(query?.context, options?.context),
};

await this.executeWithMiddleware(opCtx, async () => {
Expand All @@ -2366,7 +2394,7 @@ export class ObjectQL implements IDataEngine {
return opCtx.result as number;
}

async aggregate(object: string, query: EngineAggregateOptions): Promise<any[]> {
async aggregate(object: string, query: EngineAggregateOptions, options?: EngineReadOptions): Promise<any[]> {
object = this.resolveObjectName(object);
const driver = this.getDriver(object);
this.logger.debug(`Aggregate on ${object} using ${driver.name}`, query);
Expand All @@ -2375,7 +2403,7 @@ export class ObjectQL implements IDataEngine {
object,
operation: 'aggregate',
options: query,
context: query?.context,
context: mergeReadContext(query?.context, options?.context),
};

await this.executeWithMiddleware(opCtx, async () => {
Expand Down
Loading