Skip to content

Commit

Permalink
feat: function configuration for trx
Browse files Browse the repository at this point in the history
  • Loading branch information
tomfrew committed Sep 27, 2024
1 parent 2679658 commit ab8389b
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 130 deletions.
6 changes: 3 additions & 3 deletions node/codegen.go
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,7 @@ func writeFunctionWrapperType(w *codegen.Writer, schema *proto.Schema, model *pr
// we use the 'declare' keyword to indicate to the typescript compiler that the function
// has already been declared in the underlying vanilla javascript and therefore we are just
// decorating existing js code with types.
w.Writef("export declare function %s", casing.ToCamel(action.Name))
w.Writef("export declare const %s: runtime.FuncWithConfig<{", casing.ToCamel(action.Name))

if action.IsArbitraryFunction() {
inputType := action.InputMessageName
Expand All @@ -1197,14 +1197,14 @@ func writeFunctionWrapperType(w *codegen.Writer, schema *proto.Schema, model *pr
w.Write(toCustomFunctionReturnType(model, action, false))
w.Write("): ")
w.Write(toCustomFunctionReturnType(model, action, false))
w.Writeln(";")
w.Writeln("}>;")
return
}

hooksType := fmt.Sprintf("%sHooks", casing.ToCamel(action.Name))

// TODO: void return type here is wrong. It should be the type of the function e.g. (ctx, inputs) => ReturnType
w.Writef("(hooks?: %s): void\n", hooksType)
w.Writef("(hooks?: %s): void}>\n", hooksType)

w.Writef("export type %s = ", hooksType)

Expand Down
184 changes: 76 additions & 108 deletions packages/functions-runtime/src/auditing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,23 +65,19 @@ test("auditing - capturing identity id in transaction", async () => {

const identityId = request.meta.identity.id;

const row = await withDatabase(
db,
PROTO_ACTION_TYPES.CREATE, // CREATE will ensure a transaction is opened
async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "James",
});
const row = await withDatabase(db, true, async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "James",
});
});

expect(await identityIdFromConfigParam(transaction)).toEqual(identityId);
expect(await identityIdFromConfigParam(db)).toBeNull();
expect(await identityIdFromConfigParam(transaction)).toEqual(identityId);
expect(await identityIdFromConfigParam(db)).toBeNull();

return row;
}
);
return row;
});

expect(row.name).toEqual("James");
expect(await identityIdFromConfigParam(db)).toBeNull();
Expand All @@ -101,23 +97,19 @@ test("auditing - capturing tracing in transaction", async () => {
request.meta.tracing.traceparent
).traceId;

const row = await withDatabase(
db,
PROTO_ACTION_TYPES.CREATE, // CREATE will ensure a transaction is opened
async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "Jim",
});
const row = await withDatabase(db, true, async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "Jim",
});
});

expect(await traceIdFromConfigParam(transaction)).toEqual(traceId);
expect(await traceIdFromConfigParam(db)).toBeNull();
expect(await traceIdFromConfigParam(transaction)).toEqual(traceId);
expect(await traceIdFromConfigParam(db)).toBeNull();

return row;
}
);
return row;
});

expect(row.name).toEqual("Jim");
expect(await traceIdFromConfigParam(db)).toBeNull();
Expand All @@ -131,23 +123,19 @@ test("auditing - capturing identity id without transaction", async () => {
},
};

const row = await withDatabase(
db,
PROTO_ACTION_TYPES.GET, // GET will _not_ open a transaction
async ({ sDb }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "James",
});
const row = await withDatabase(db, false, async ({ sDb }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "James",
});
});

expect(await identityIdFromConfigParam(sDb)).toBeNull();
expect(await identityIdFromConfigParam(db)).toBeNull();
expect(await identityIdFromConfigParam(sDb)).toBeNull();
expect(await identityIdFromConfigParam(db)).toBeNull();

return row;
}
);
return row;
});

expect(row.name).toEqual("James");
expect(await identityIdFromConfigParam(db)).toBeNull();
Expand All @@ -163,23 +151,19 @@ test("auditing - capturing tracing without transaction", async () => {
},
};

const row = await withDatabase(
db,
PROTO_ACTION_TYPES.GET, // GET will _not_ open a transaction
async ({ sDb }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "Jim",
});
const row = await withDatabase(db, false, async ({ sDb }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "Jim",
});
});

expect(await traceIdFromConfigParam(sDb)).toBeNull();
expect(await traceIdFromConfigParam(db)).toBeNull();
expect(await traceIdFromConfigParam(sDb)).toBeNull();
expect(await traceIdFromConfigParam(db)).toBeNull();

return row;
}
);
return row;
});

expect(row.name).toEqual("Jim");
expect(await traceIdFromConfigParam(db)).toBeNull();
Expand Down Expand Up @@ -225,23 +209,19 @@ test("auditing - ModelAPI.create", async () => {
request.meta.tracing.traceparent
).traceId;

const row = await withDatabase(
db,
PROTO_ACTION_TYPES.CREATE,
async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "Jake",
});
const row = await withDatabase(db, true, async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "Jake",
});
});

expect(await identityIdFromConfigParam(transaction)).toEqual(identityId);
expect(await traceIdFromConfigParam(transaction)).toEqual(traceId);
expect(await identityIdFromConfigParam(transaction)).toEqual(identityId);
expect(await traceIdFromConfigParam(transaction)).toEqual(traceId);

return row;
}
);
return row;
});

expect(row.name).toEqual("Jake");
});
Expand All @@ -266,20 +246,16 @@ test("auditing - ModelAPI.update", async () => {
name: "Jake",
});

const row = await withDatabase(
db,
PROTO_ACTION_TYPES.CREATE,
async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.update({ id: created.id }, { name: "Jim" });
});
const row = await withDatabase(db, true, async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.update({ id: created.id }, { name: "Jim" });
});

expect(await identityIdFromConfigParam(transaction)).toEqual(identityId);
expect(await traceIdFromConfigParam(transaction)).toEqual(traceId);
expect(await identityIdFromConfigParam(transaction)).toEqual(identityId);
expect(await traceIdFromConfigParam(transaction)).toEqual(traceId);

return row;
}
);
return row;
});

expect(row.name).toEqual("Jim");
});
Expand All @@ -304,20 +280,16 @@ test("auditing - ModelAPI.delete", async () => {
name: "Jake",
});

const row = await withDatabase(
db,
PROTO_ACTION_TYPES.CREATE,
async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.delete({ id: created.id });
});
const row = await withDatabase(db, true, async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.delete({ id: created.id });
});

expect(await identityIdFromConfigParam(transaction)).toEqual(identityId);
expect(await traceIdFromConfigParam(transaction)).toEqual(traceId);
expect(await identityIdFromConfigParam(transaction)).toEqual(identityId);
expect(await traceIdFromConfigParam(transaction)).toEqual(traceId);

return row;
}
);
return row;
});

expect(row).toEqual(created.id);
});
Expand All @@ -337,23 +309,19 @@ test("auditing - identity id and trace id fields dropped from result", async ()
request.meta.tracing.traceparent
).traceId;

const row = await withDatabase(
db,
PROTO_ACTION_TYPES.CREATE,
async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "Jake",
});
const row = await withDatabase(db, true, async ({ transaction }) => {
const row = withAuditContext(request, async () => {
return await personAPI.create({
id: KSUID.randomSync().string,
name: "Jake",
});
});

expect(await identityIdFromConfigParam(transaction)).toEqual(identityId);
expect(await traceIdFromConfigParam(transaction)).toEqual(traceId);
expect(await identityIdFromConfigParam(transaction)).toEqual(identityId);
expect(await traceIdFromConfigParam(transaction)).toEqual(traceId);

return row;
}
);
return row;
});

expect(row.name).toEqual("Jake");
expect(row.keelIdentityId).toBeUndefined();
Expand Down
13 changes: 1 addition & 12 deletions packages/functions-runtime/src/database.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,7 @@ const ws = require("ws");
// the user's custom function is wrapped in a transaction so we can rollback
// the transaction if something goes wrong.
// withDatabase shouldn't be exposed in the public api of the sdk
async function withDatabase(db, actionType, cb) {
let requiresTransaction = true;

switch (actionType) {
case PROTO_ACTION_TYPES.SUBSCRIBER:
case PROTO_ACTION_TYPES.JOB:
case PROTO_ACTION_TYPES.GET:
case PROTO_ACTION_TYPES.LIST:
requiresTransaction = false;
break;
}

async function withDatabase(db, requiresTransaction, cb) {
// db.transaction() provides a kysely instance bound to a transaction.
if (requiresTransaction) {
return db.transaction().execute(async (transaction) => {
Expand Down
12 changes: 11 additions & 1 deletion packages/functions-runtime/src/handleRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,18 @@ async function handleRequest(request, config) {
const customFunction = functions[request.method];
const actionType = actionTypes[request.method];

const functionConfig = customFunction?.config ?? {};

const result = await tryExecuteFunction(
{ request, ctx, permitted, db, permissionFns, actionType },
{
request,
ctx,
permitted,
db,
permissionFns,
actionType,
functionConfig,
},
async () => {
// parse request params to convert objects into rich field types (e.g. InlineFile)
const inputs = parseInputs(request.params);
Expand Down
12 changes: 12 additions & 0 deletions packages/functions-runtime/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,3 +227,15 @@ export type Errors = {
*/
Unknown: typeof UnknownError;
};

export type FunctionConfig = {
/**
* All DB calls within the function will be executed within a transaction.
* The transaction is rolled back if the function throws an error.
*/
autoTransaction?: boolean;
};

export type FuncWithConfig<T> = T & {
config: FunctionConfig;
};
17 changes: 15 additions & 2 deletions packages/functions-runtime/src/tryExecuteFunction.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,24 @@ const { PROTO_ACTION_TYPES } = require("./consts");
// tryExecuteFunction will create a new database transaction around a function call
// and handle any permissions checks. If a permission check fails, then an Error will be thrown and the catch block will be hit.
function tryExecuteFunction(
{ request, db, permitted, permissionFns, actionType, ctx },
{ request, db, permitted, permissionFns, actionType, ctx, functionConfig },
cb
) {
return withPermissions(permitted, async ({ getPermissionState }) => {
return withDatabase(db, actionType, async ({ transaction }) => {
let requiresTransaction = true;
switch (actionType) {
case PROTO_ACTION_TYPES.GET:
case PROTO_ACTION_TYPES.LIST:
case PROTO_ACTION_TYPES.READ:
requiresTransaction = false;
break;
}

if (functionConfig?.autoTransaction !== undefined) {
requiresTransaction = functionConfig.autoTransaction;
}

return withDatabase(db, requiresTransaction, async ({ transaction }) => {
const fnResult = await withAuditContext(request, async () => {
return cb();
});
Expand Down
4 changes: 2 additions & 2 deletions packages/functions-runtime/src/tryExecuteJob.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ const { PermissionError } = require("./errors");

// tryExecuteJob will create a new database transaction around a function call
// and handle any permissions checks. If a permission check fails, then an Error will be thrown and the catch block will be hit.
function tryExecuteJob({ db, permitted, actionType, request }, cb) {
function tryExecuteJob({ db, permitted, request }, cb) {
return withPermissions(permitted, async ({ getPermissionState }) => {
return withDatabase(db, actionType, async () => {
return withDatabase(db, false, async () => {
await withAuditContext(request, async () => {
return cb();
});
Expand Down
4 changes: 2 additions & 2 deletions packages/functions-runtime/src/tryExecuteSubscriber.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ const { withDatabase } = require("./database");
const { withAuditContext } = require("./auditing");

// tryExecuteSubscriber will create a new database connection and execute the function call.
function tryExecuteSubscriber({ request, db, actionType }, cb) {
return withDatabase(db, actionType, async () => {
function tryExecuteSubscriber({ request, db }, cb) {
return withDatabase(db, false, async () => {
await withAuditContext(request, async () => {
return cb();
});
Expand Down

0 comments on commit ab8389b

Please sign in to comment.