From bb515c8cb95e13b99231ca3cc233af664ca7afc8 Mon Sep 17 00:00:00 2001 From: Ian Calvert Date: Tue, 10 Dec 2024 10:50:46 +0000 Subject: [PATCH 1/5] Allow command execute to take in pojos rather than cards --- .../create-product-requirements-command.ts | 11 +++----- packages/runtime-common/commands.ts | 28 ++++++++++++++++--- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/packages/catalog-realm/AiAppGenerator/create-product-requirements-command.ts b/packages/catalog-realm/AiAppGenerator/create-product-requirements-command.ts index d87ce8340b..5c244dc9b5 100644 --- a/packages/catalog-realm/AiAppGenerator/create-product-requirements-command.ts +++ b/packages/catalog-realm/AiAppGenerator/create-product-requirements-command.ts @@ -59,13 +59,10 @@ export default class CreateProductRequirementsInstance extends Command< let prdCard = new ProductRequirementDocument(); let saveCardCommand = new SaveCardCommand(this.commandContext); - let SaveCardInputType = await saveCardCommand.getInputType(); - await saveCardCommand.execute( - new SaveCardInputType({ - realm: input.realm, - card: prdCard, - }), - ); + await saveCardCommand.execute({ + realm: input.realm, + card: prdCard, + }); // Get patch command, this takes the card and returns a command that can be used to patch the card let patchPRDCommand = new PatchCardCommand(this.commandContext, { cardType: ProductRequirementDocument, diff --git a/packages/runtime-common/commands.ts b/packages/runtime-common/commands.ts index 2a8cdd3e64..f6b6afe474 100644 --- a/packages/runtime-common/commands.ts +++ b/packages/runtime-common/commands.ts @@ -51,7 +51,9 @@ export abstract class Command< CommandConfiguration extends any | undefined = undefined, > { // Is this actually type checking ? - abstract getInputType(): Promise<{ new (args: any): CardInputType }>; // TODO: can we do better than any here? + abstract getInputType(): Promise< + { new (args: any): CardInputType } | undefined + >; // TODO: can we do better than any here? invocations: CommandInvocation[] = []; @@ -66,20 +68,38 @@ export abstract class Command< protected readonly configuration?: CommandConfiguration | undefined, // we'd like this to be required *if* CommandConfiguration is defined, and allow the user to skip it when CommandConfiguration is undefined ) {} - async execute(input: CardInputType): Promise { + async execute( + input: CardInputType extends CardDef | undefined + ? CardInputType | Omit + : never, + ): Promise { // internal bookkeeping // todo: support for this.runTask being defined // runTask would be an ember task, run would just be a normal function + let inputCard: CardInputType; + if (input === undefined) { + inputCard = undefined as CardInputType; + } else if ('isCardDef' in input && input.isCardDef) { + inputCard = input as CardInputType; + } else { + let InputType = await this.getInputType(); + if (!InputType) { + inputCard = undefined as CardInputType; + } else { + inputCard = new InputType(input) as CardInputType; + } + } + let invocation = new CommandInvocation( - input, + inputCard, ); this.invocations.push(invocation); this.nextCompletionDeferred.fulfill(invocation.promise); try { - let result = await this.run(input); + let result = await this.run(inputCard); invocation.fulfill(result); return result; } catch (error) { From c317fa89d5fbb9cc546ffe18b47ec65ca50df714 Mon Sep 17 00:00:00 2001 From: Ian Calvert Date: Wed, 11 Dec 2024 19:58:48 +0000 Subject: [PATCH 2/5] Throw errors when no input type defined and an input provided, better type checking on input types --- packages/runtime-common/commands.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/runtime-common/commands.ts b/packages/runtime-common/commands.ts index f6b6afe474..3ae6ec32bf 100644 --- a/packages/runtime-common/commands.ts +++ b/packages/runtime-common/commands.ts @@ -1,3 +1,4 @@ +import { isCardDef } from './code-ref'; import { Deferred } from './deferred'; import type * as CardAPI from 'https://cardstack.com/base/card-api'; import { CardDef } from 'https://cardstack.com/base/card-api'; @@ -52,7 +53,7 @@ export abstract class Command< > { // Is this actually type checking ? abstract getInputType(): Promise< - { new (args: any): CardInputType } | undefined + { new (args?: Partial): CardInputType } | undefined >; // TODO: can we do better than any here? invocations: CommandInvocation[] = []; @@ -80,17 +81,16 @@ export abstract class Command< let inputCard: CardInputType; if (input === undefined) { inputCard = undefined as CardInputType; - } else if ('isCardDef' in input && input.isCardDef) { + } else if (isCardDef(input.constructor)) { inputCard = input as CardInputType; } else { let InputType = await this.getInputType(); if (!InputType) { - inputCard = undefined as CardInputType; + throw new Error('Input provided but no input type found'); } else { inputCard = new InputType(input) as CardInputType; } } - let invocation = new CommandInvocation( inputCard, ); From f9e14fa88b6af2869b4b50c8d03a95d9aa5eac27 Mon Sep 17 00:00:00 2001 From: Ian Calvert Date: Wed, 11 Dec 2024 20:02:03 +0000 Subject: [PATCH 3/5] Add tests around the commands type setup --- .../commands/commands-calling-test.gts | 192 ++++++++++++++++++ 1 file changed, 192 insertions(+) create mode 100644 packages/host/tests/integration/commands/commands-calling-test.gts diff --git a/packages/host/tests/integration/commands/commands-calling-test.gts b/packages/host/tests/integration/commands/commands-calling-test.gts new file mode 100644 index 0000000000..81180500c6 --- /dev/null +++ b/packages/host/tests/integration/commands/commands-calling-test.gts @@ -0,0 +1,192 @@ +import { getOwner } from '@ember/owner'; +import { RenderingTestContext } from '@ember/test-helpers'; + +import { module, test } from 'qunit'; + +import { Command, CommandContext } from '@cardstack/runtime-common'; + +import type CommandService from '@cardstack/host/services/command-service'; + +import RealmService from '@cardstack/host/services/realm'; + +import { lookupService, testRealmURL, testRealmInfo } from '../../helpers'; +import { + CardDef, + StringField, + contains, + field, + setupBaseRealm, +} from '../../helpers/base-realm'; +import { setupRenderingTest } from '../../helpers/setup'; + +class StubRealmService extends RealmService { + get defaultReadableRealm() { + return { + path: testRealmURL, + info: testRealmInfo, + }; + } +} + +module('Integration | commands | commands-calling', function (hooks) { + setupRenderingTest(hooks); + setupBaseRealm(hooks); + let commandContext: CommandContext; + + hooks.beforeEach(function (this: RenderingTestContext) { + getOwner(this)!.register('service:realm', StubRealmService); + + let commandService = lookupService('command-service'); + commandContext = commandService.commandContext; + }); + + test('can be called with a card as input', async function (assert) { + class CommandInput extends CardDef { + @field inputField1 = contains(StringField); + @field inputField2 = contains(StringField); + } + class CommandOutput extends CardDef { + @field outputField = contains(StringField); + } + + class ExampleCommand extends Command { + inputType = CommandInput; + + async getInputType() { + return CommandInput; + } + + async run(input: CommandInput) { + return new CommandOutput({ + outputField: `Hello ${input.inputField1}${input.inputField2}`, + }); + } + } + let exampleCommand = new ExampleCommand(commandContext); + + const InputType = await exampleCommand.getInputType(); + let input = new InputType({ + inputField1: 'World', + inputField2: '!', + }); + let output = await exampleCommand.execute(input); + assert.strictEqual(output.outputField, 'Hello World!'); + }); + + test('can be called with plain object as input', async function (assert) { + class CommandInput extends CardDef { + @field inputField1 = contains(StringField); + @field inputField2 = contains(StringField); + } + class CommandOutput extends CardDef { + @field outputField = contains(StringField); + } + + class ExampleCommand extends Command { + inputType = CommandInput; + + async getInputType() { + return CommandInput; + } + + async run(input: CommandInput) { + return new CommandOutput({ + outputField: `Hello ${input.inputField1}${input.inputField2}`, + }); + } + } + let exampleCommand = new ExampleCommand(commandContext); + let output = await exampleCommand.execute({ + inputField1: 'World', + inputField2: '!', + }); + assert.strictEqual(output.outputField, 'Hello World!'); + }); + + test('type error when parameters are missing', async function (assert) { + class CommandInput extends CardDef { + @field inputField1 = contains(StringField); + @field inputField2 = contains(StringField); + } + class CommandOutput extends CardDef { + @field outputField = contains(StringField); + } + + class ExampleCommand extends Command { + inputType = CommandInput; + + async getInputType() { + return CommandInput; + } + + async run(input: CommandInput) { + return new CommandOutput({ + outputField: `Hello ${input.inputField1}${input.inputField2}`, + }); + } + } + let exampleCommand = new ExampleCommand(commandContext); + + // @ts-expect-error: missing inputField2 + let output = await exampleCommand.execute({ + inputField1: 'World', + }); + assert.strictEqual(output.outputField, 'Hello Worldundefined'); + }); + + test('CardDef fields are optional', async function (assert) { + class CommandInput extends CardDef { + @field inputField1 = contains(StringField); + @field inputField2 = contains(StringField); + } + class CommandOutput extends CardDef { + @field outputField = contains(StringField); + } + + class ExampleCommand extends Command { + inputType = CommandInput; + + async getInputType() { + return CommandInput; + } + + async run(input: CommandInput) { + return new CommandOutput({ + outputField: `Hello ${input.inputField1}${input.inputField2}`, + }); + } + } + let exampleCommand = new ExampleCommand(commandContext); + + let output = await exampleCommand.execute({ + inputField1: 'World', + inputField2: '!', + title: 'test', + }); + assert.strictEqual(output.outputField, 'Hello World!'); + }); + + test('Commands work without taking input', async function (assert) { + class CommandOutput extends CardDef { + @field outputField = contains(StringField); + } + + class ExampleCommand extends Command { + inputType = undefined; + + async getInputType() { + return undefined; + } + + async run() { + return new CommandOutput({ + outputField: 'Hello', + }); + } + } + let exampleCommand = new ExampleCommand(commandContext); + + let output = await exampleCommand.execute(undefined); + assert.strictEqual(output.outputField, 'Hello'); + }); +}); From 825fabe8b197373b9195a4d27d437a2406d5c821 Mon Sep 17 00:00:00 2001 From: Ian Calvert Date: Wed, 11 Dec 2024 20:07:07 +0000 Subject: [PATCH 4/5] Deal with cases where the input type is undefined --- packages/host/app/services/command-service.ts | 30 +++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/packages/host/app/services/command-service.ts b/packages/host/app/services/command-service.ts index 29963760da..1d1c6faab1 100644 --- a/packages/host/app/services/command-service.ts +++ b/packages/host/app/services/command-service.ts @@ -82,11 +82,17 @@ export default class CommandService extends Service { // Get the input type and validate/construct the payload let InputType = await command.getInputType(); - // Construct a new instance of the input type with the payload - let typedInput = new InputType({ - ...toolCall.arguments.attributes, - ...toolCall.arguments.relationships, - }); + // Construct a new instance of the input type with the + // The input is undefined if the command has no input type + let typedInput; + if (InputType) { + typedInput = new InputType({ + ...toolCall.arguments.attributes, + ...toolCall.arguments.relationships, + }); + } else { + typedInput = undefined; + } let res = await command.execute(typedInput); await this.matrixService.sendReactionEvent( event.room_id!, @@ -131,10 +137,16 @@ export default class CommandService extends Service { // Get the input type and validate/construct the payload let InputType = await commandToRun.getInputType(); // Construct a new instance of the input type with the payload - let typedInput = new InputType({ - ...payload.attributes, - ...payload.relationships, - }); + // The input is undefined if the command has no input type + let typedInput; + if (InputType) { + typedInput = new InputType({ + ...payload.attributes, + ...payload.relationships, + }); + } else { + typedInput = undefined; + } [res] = await all([ await commandToRun.execute(typedInput), await timeout(DELAY_FOR_APPLYING_UI), // leave a beat for the "applying" state of the UI to be shown From c85f62dbb6adf33f35e58def058b4e1f89afab9e Mon Sep 17 00:00:00 2001 From: Ian Calvert Date: Wed, 11 Dec 2024 20:38:46 +0000 Subject: [PATCH 5/5] Make the parameters optional --- .../tests/integration/commands/commands-calling-test.gts | 3 +-- packages/runtime-common/commands.ts | 6 ++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/host/tests/integration/commands/commands-calling-test.gts b/packages/host/tests/integration/commands/commands-calling-test.gts index 81180500c6..066b13486e 100644 --- a/packages/host/tests/integration/commands/commands-calling-test.gts +++ b/packages/host/tests/integration/commands/commands-calling-test.gts @@ -103,7 +103,7 @@ module('Integration | commands | commands-calling', function (hooks) { assert.strictEqual(output.outputField, 'Hello World!'); }); - test('type error when parameters are missing', async function (assert) { + test('can call a command with just some of the fields', async function (assert) { class CommandInput extends CardDef { @field inputField1 = contains(StringField); @field inputField2 = contains(StringField); @@ -127,7 +127,6 @@ module('Integration | commands | commands-calling', function (hooks) { } let exampleCommand = new ExampleCommand(commandContext); - // @ts-expect-error: missing inputField2 let output = await exampleCommand.execute({ inputField1: 'World', }); diff --git a/packages/runtime-common/commands.ts b/packages/runtime-common/commands.ts index 3ae6ec32bf..7762b12965 100644 --- a/packages/runtime-common/commands.ts +++ b/packages/runtime-common/commands.ts @@ -71,7 +71,7 @@ export abstract class Command< async execute( input: CardInputType extends CardDef | undefined - ? CardInputType | Omit + ? CardInputType | Partial> : never, ): Promise { // internal bookkeeping @@ -88,7 +88,9 @@ export abstract class Command< if (!InputType) { throw new Error('Input provided but no input type found'); } else { - inputCard = new InputType(input) as CardInputType; + inputCard = new InputType( + input as Partial, + ) as CardInputType; } } let invocation = new CommandInvocation(