From 10767ad3e28abd5bc8e9979c5519f9c1a98a6590 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Wed, 20 Mar 2024 10:57:51 +0800 Subject: [PATCH] Encode id if it is used in request path (#174) (#177) * feat: encode id if it is used in request path * feat: add to release node --------- (cherry picked from commit 3abf12d2b873811619320ec786cd421c3221acb3) Signed-off-by: SuZhou-Joe Signed-off-by: github-actions[bot] Co-authored-by: github-actions[bot] --- ...boards-assistant.release-notes-2.13.0.0.md | 1 + .../agent_framework_storage_service.test.ts | 121 ++++++++++++++++++ .../agent_framework_storage_service.ts | 16 ++- 3 files changed, 131 insertions(+), 7 deletions(-) diff --git a/release-notes/dashboards-assistant.release-notes-2.13.0.0.md b/release-notes/dashboards-assistant.release-notes-2.13.0.0.md index 33b77f64..e3f996f1 100644 --- a/release-notes/dashboards-assistant.release-notes-2.13.0.0.md +++ b/release-notes/dashboards-assistant.release-notes-2.13.0.0.md @@ -7,6 +7,7 @@ Compatible with OpenSearch and OpenSearch Dashboards Version 2.13.0 - Integrate chatbot with sidecar service. ([#164](https://github.com/opensearch-project/dashboards-assistant/pull/164)) - Update view trace button from suggestions to message action bar. ([#158](https://github.com/opensearch-project/dashboards-assistant/pull/158)) - Fetch root agent id before executing the agent. ([#165](https://github.com/opensearch-project/dashboards-assistant/pull/165)) +- Encode id if it is used in request path. ([#174](https://github.com/opensearch-project/dashboards-assistant/pull/174)) ### Infrastructure diff --git a/server/services/storage/agent_framework_storage_service.test.ts b/server/services/storage/agent_framework_storage_service.test.ts index 74236850..0260f9b7 100644 --- a/server/services/storage/agent_framework_storage_service.test.ts +++ b/server/services/storage/agent_framework_storage_service.test.ts @@ -80,6 +80,24 @@ describe('AgentFrameworkStorageService', () => { `); }); + it('should encode id when calls getConversation with non-standard params in request payload', async () => { + mockedTransport.mockResolvedValue({ + body: { + messages: [], + }, + }); + + await agentFrameworkService.getConversation('../non-standard/id'); + expect(mockedTransport.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "method": "GET", + "path": "/_plugins/_ml/memory/..%2Fnon-standard%2Fid/messages?max_results=1000", + }, + ] + `); + }); + it('getConversations', async () => { mockedTransport.mockImplementation(async (params) => { return { @@ -223,6 +241,21 @@ describe('AgentFrameworkStorageService', () => { expect(agentFrameworkService.deleteConversation('foo')).rejects.toBeDefined(); }); + it('should encode id when calls deleteConversation with non-standard params in request payload', async () => { + mockedTransport.mockResolvedValueOnce({ + statusCode: 200, + }); + await agentFrameworkService.deleteConversation('../non-standard/id'); + expect(mockedTransport.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "method": "DELETE", + "path": "/_plugins/_ml/memory/..%2Fnon-standard%2Fid", + }, + ] + `); + }); + it('updateConversation', async () => { mockedTransport.mockImplementationOnce(async (params) => ({ statusCode: 200, @@ -253,6 +286,24 @@ describe('AgentFrameworkStorageService', () => { expect(agentFrameworkService.updateConversation('foo', 'title')).rejects.toBeDefined(); }); + it('should encode id when calls updateConversation with non-standard params in request payload', async () => { + mockedTransport.mockResolvedValueOnce({ + statusCode: 200, + }); + await agentFrameworkService.updateConversation('../non-standard/id', 'title'); + expect(mockedTransport.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "body": Object { + "name": "title", + }, + "method": "PUT", + "path": "/_plugins/_ml/memory/..%2Fnon-standard%2Fid", + }, + ] + `); + }); + it('getTraces', async () => { mockedTransport.mockImplementationOnce(async (params) => ({ body: { @@ -288,6 +339,32 @@ describe('AgentFrameworkStorageService', () => { ); }); + it('should encode id when calls getTraces with non-standard params in request payload', async () => { + mockedTransport.mockResolvedValueOnce({ + body: { + traces: [ + { + message_id: 'interaction_id', + create_time: 'create_time', + input: 'input', + response: 'response', + origin: 'origin', + trace_number: 1, + }, + ], + }, + }); + await agentFrameworkService.getTraces('../non-standard/id'); + expect(mockedTransport.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "method": "GET", + "path": "/_plugins/_ml/memory/message/..%2Fnon-standard%2Fid/traces", + }, + ] + `); + }); + it('updateInteraction', async () => { mockedTransport.mockImplementationOnce(async (params) => ({ statusCode: 200, @@ -334,6 +411,32 @@ describe('AgentFrameworkStorageService', () => { ).rejects.toMatchInlineSnapshot(`[Error: update interaction failed, reason:"error"]`); }); + it('should encode id when calls updateInteraction with non-standard params in request payload', async () => { + mockedTransport.mockResolvedValueOnce({ + statusCode: 200, + }); + await agentFrameworkService.updateInteraction('../non-standard/id', { + foo: { + bar: 'foo', + }, + }); + expect(mockedTransport.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "body": Object { + "additional_info": Object { + "foo": Object { + "bar": "foo", + }, + }, + }, + "method": "PUT", + "path": "/_plugins/_ml/memory/message/..%2Fnon-standard%2Fid", + }, + ] + `); + }); + it('getInteraction', async () => { mockedTransport.mockImplementation(async (params) => ({ body: { @@ -359,4 +462,22 @@ describe('AgentFrameworkStorageService', () => { `); expect(mockedTransport).toBeCalledTimes(1); }); + + it('should encode id when calls getInteraction with non-standard params in request payload', async () => { + mockedTransport.mockResolvedValueOnce({ + body: { + input: 'input', + response: 'response', + }, + }); + await agentFrameworkService.getInteraction('_id', '../non-standard/id'); + expect(mockedTransport.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + Object { + "method": "GET", + "path": "/_plugins/_ml/memory/message/..%2Fnon-standard%2Fid", + }, + ] + `); + }); }); diff --git a/server/services/storage/agent_framework_storage_service.ts b/server/services/storage/agent_framework_storage_service.ts index 08506073..84cb0a2e 100644 --- a/server/services/storage/agent_framework_storage_service.ts +++ b/server/services/storage/agent_framework_storage_service.ts @@ -35,7 +35,9 @@ export class AgentFrameworkStorageService implements StorageService { const [interactionsResp, conversation] = await Promise.all([ this.client.transport.request({ method: 'GET', - path: `${ML_COMMONS_BASE_API}/memory/${conversationId}/messages?max_results=1000`, + path: `${ML_COMMONS_BASE_API}/memory/${encodeURIComponent( + conversationId + )}/messages?max_results=1000`, }) as TransportRequestPromise< ApiResponse<{ messages: InteractionFromAgentFramework[]; @@ -43,7 +45,7 @@ export class AgentFrameworkStorageService implements StorageService { >, this.client.transport.request({ method: 'GET', - path: `${ML_COMMONS_BASE_API}/memory/${conversationId}`, + path: `${ML_COMMONS_BASE_API}/memory/${encodeURIComponent(conversationId)}`, }) as TransportRequestPromise< ApiResponse<{ conversation_id: string; @@ -147,7 +149,7 @@ export class AgentFrameworkStorageService implements StorageService { try { const response = await this.client.transport.request({ method: 'DELETE', - path: `${ML_COMMONS_BASE_API}/memory/${conversationId}`, + path: `${ML_COMMONS_BASE_API}/memory/${encodeURIComponent(conversationId)}`, }); if (response.statusCode === 200) { return { @@ -172,7 +174,7 @@ export class AgentFrameworkStorageService implements StorageService { try { const response = await this.client.transport.request({ method: 'PUT', - path: `${ML_COMMONS_BASE_API}/memory/${conversationId}`, + path: `${ML_COMMONS_BASE_API}/memory/${encodeURIComponent(conversationId)}`, body: { name: title, }, @@ -197,7 +199,7 @@ export class AgentFrameworkStorageService implements StorageService { try { const response = (await this.client.transport.request({ method: 'GET', - path: `${ML_COMMONS_BASE_API}/memory/message/${interactionId}/traces`, + path: `${ML_COMMONS_BASE_API}/memory/message/${encodeURIComponent(interactionId)}/traces`, })) as ApiResponse<{ traces: Array<{ message_id: string; @@ -229,7 +231,7 @@ export class AgentFrameworkStorageService implements StorageService { try { const response = await this.client.transport.request({ method: 'PUT', - path: `${ML_COMMONS_BASE_API}/memory/message/${interactionId}`, + path: `${ML_COMMONS_BASE_API}/memory/message/${encodeURIComponent(interactionId)}`, body: { additional_info: additionalInfo, }, @@ -276,7 +278,7 @@ export class AgentFrameworkStorageService implements StorageService { } const interactionsResp = (await this.client.transport.request({ method: 'GET', - path: `${ML_COMMONS_BASE_API}/memory/message/${interactionId}`, + path: `${ML_COMMONS_BASE_API}/memory/message/${encodeURIComponent(interactionId)}`, })) as ApiResponse; return formatInteractionFromBackend(interactionsResp.body); }