diff --git a/src/client/catalogs.ts b/src/client/catalogs.ts index 6bf275e..85a0ca6 100644 --- a/src/client/catalogs.ts +++ b/src/client/catalogs.ts @@ -1,3 +1,6 @@ +import type { AxiosInstance } from "axios"; + +import type { CatalogDocuments } from "../types/catalogs.js"; import { BulkDeleteCatalogItemsParams, CatalogFieldMappingsResponse, @@ -16,9 +19,10 @@ import { GetCatalogsResponse, GetCatalogsResponseSchema, PartialUpdateCatalogItemParams, + PartialUpdateCatalogItemsParams, ReplaceCatalogItemParams, + ReplaceCatalogItemsParams, UpdateCatalogFieldMappingsParams, - UpdateCatalogItemParams, } from "../types/catalogs.js"; import { IterableSuccessResponse, @@ -28,6 +32,19 @@ import type { Constructor } from "./base.js"; import type { BaseIterableClient } from "./base.js"; import { validateResponse } from "./base.js"; +async function bulkUploadCatalogItems( + client: AxiosInstance, + catalogName: string, + documents: CatalogDocuments, + replaceUploadedFieldsOnly: boolean +): Promise { + const response = await client.post( + `/api/catalogs/${encodeURIComponent(catalogName)}/items`, + { documents, replaceUploadedFieldsOnly } + ); + return validateResponse(response, IterableSuccessResponseSchema); +} + /** * Catalogs operations mixin */ @@ -42,31 +59,32 @@ export function Catalogs>(Base: T) { return validateResponse(response, IterableSuccessResponseSchema); } - async updateCatalogItems( - options: UpdateCatalogItemParams + /** + * Bulk partial update catalog items + */ + async partialUpdateCatalogItems( + params: PartialUpdateCatalogItemsParams ): Promise { - // Convert items array to documents object (map of id to values) - const documents: Record = {}; - options.items.forEach((item) => { - documents[item.id] = { - name: item.name, - description: item.description, - price: item.price, - categories: item.categories, - imageUrl: item.imageUrl, - url: item.url, - ...item.dataFields, - }; - }); + return bulkUploadCatalogItems( + this.client, + params.catalogName, + params.documents, + true + ); + } - const response = await this.client.post( - `/api/catalogs/${encodeURIComponent(options.catalogName)}/items`, - { - documents, - replaceUploadedFieldsOnly: false, - } + /** + * Bulk replace catalog items + */ + async replaceCatalogItems( + params: ReplaceCatalogItemsParams + ): Promise { + return bulkUploadCatalogItems( + this.client, + params.catalogName, + params.documents, + false ); - return validateResponse(response, IterableSuccessResponseSchema); } async getCatalogItem( diff --git a/src/types/catalogs.ts b/src/types/catalogs.ts index ebe9caf..76935dd 100644 --- a/src/types/catalogs.ts +++ b/src/types/catalogs.ts @@ -2,45 +2,84 @@ import { z } from "zod"; import { UnixTimestampSchema } from "./common.js"; -/** - * Catalog management schemas and types - */ - -export const CatalogItemSchema = z.object({ - id: z.string(), - name: z.string(), - description: z.string().optional(), - price: z.number().optional(), - categories: z.array(z.string()).optional(), - imageUrl: z.string().optional(), - url: z.string().optional(), - dataFields: z.record(z.string(), z.any()).optional(), -}); - -export type CatalogItem = z.infer; -export type UpdateCatalogItemParams = z.infer< - typeof UpdateCatalogItemParamsSchema ->; -export type GetCatalogItemParams = z.infer; -export type DeleteCatalogItemParams = z.infer< - typeof DeleteCatalogItemParamsSchema ->; +const CATALOG_ITEM_ID_REGEX = /^[a-zA-Z0-9-]{1,255}$/; +const MAX_CATALOG_BULK_DOCUMENTS = 1000; + +export const CatalogItemIdSchema = z + .string() + .regex( + CATALOG_ITEM_ID_REGEX, + "Invalid catalog item ID (alphanumeric and dashes only, max 255 chars)" + ); -export const UpdateCatalogItemParamsSchema = z.object({ - catalogName: z.string().describe("Name of the catalog"), - items: z.array(CatalogItemSchema).describe("Catalog items to update"), -}); +function validateCatalogItemFieldNames( + fields: Record, + ctx: z.RefinementCtx +): void { + for (const fieldName of Object.keys(fields)) { + if (fieldName.includes(".")) { + ctx.addIssue({ + code: "custom", + message: `Field name "${fieldName}" must not contain periods`, + }); + } + } +} + +export const CatalogDocumentFieldsSchema = z + .record(z.string(), z.unknown()) + .superRefine(validateCatalogItemFieldNames) + .describe("Catalog item field values"); + +export type CatalogDocumentFields = z.infer; + +function validateCatalogDocuments( + documents: Record, + ctx: z.RefinementCtx +): void { + const itemIds = Object.keys(documents); + + if (itemIds.length === 0) { + ctx.addIssue({ + code: "custom", + message: "documents must contain at least one catalog item", + }); + return; + } + + if (itemIds.length > MAX_CATALOG_BULK_DOCUMENTS) { + ctx.addIssue({ + code: "custom", + message: `documents may contain at most ${MAX_CATALOG_BULK_DOCUMENTS} items`, + }); + } + + for (const itemId of itemIds) { + if (!CATALOG_ITEM_ID_REGEX.test(itemId)) { + ctx.addIssue({ + code: "custom", + message: `Invalid catalog item ID "${itemId}" (alphanumeric and dashes only, max 255 chars)`, + }); + } + } +} export const GetCatalogItemParamsSchema = z.object({ catalogName: z.string().describe("Name of the catalog"), - itemId: z.string().describe("ID of the catalog item to retrieve"), + itemId: CatalogItemIdSchema.describe("ID of the catalog item to retrieve"), }); +export type GetCatalogItemParams = z.infer; + export const DeleteCatalogItemParamsSchema = z.object({ catalogName: z.string().describe("Name of the catalog"), - itemId: z.string().describe("ID of the catalog item to delete"), + itemId: CatalogItemIdSchema.describe("ID of the catalog item to delete"), }); +export type DeleteCatalogItemParams = z.infer< + typeof DeleteCatalogItemParamsSchema +>; + export const CatalogNameSchema = z.object({ name: z.string(), }); @@ -193,10 +232,39 @@ export type UpdateCatalogFieldMappingsParams = z.infer< typeof UpdateCatalogFieldMappingsParamsSchema >; +export const CatalogDocumentsSchema = z + .record(z.string(), CatalogDocumentFieldsSchema) + .superRefine(validateCatalogDocuments) + .describe("Map of catalog item ID to field values"); + +export type CatalogDocuments = z.infer; + +const BulkCatalogItemsParamsSchema = z.object({ + catalogName: z.string().describe("Name of the catalog"), + documents: CatalogDocumentsSchema, +}); + +// Bulk partial update catalog items +export const PartialUpdateCatalogItemsParamsSchema = + BulkCatalogItemsParamsSchema; + +export type PartialUpdateCatalogItemsParams = z.infer< + typeof PartialUpdateCatalogItemsParamsSchema +>; + +// Bulk replace catalog items +export const ReplaceCatalogItemsParamsSchema = BulkCatalogItemsParamsSchema; + +export type ReplaceCatalogItemsParams = z.infer< + typeof ReplaceCatalogItemsParamsSchema +>; + // Bulk delete catalog items export const BulkDeleteCatalogItemsParamsSchema = z.object({ catalogName: z.string().describe("Name of the catalog"), - itemIds: z.array(z.string()).describe("Array of item IDs to delete"), + itemIds: z + .array(CatalogItemIdSchema) + .describe("Array of item IDs to delete"), }); export type BulkDeleteCatalogItemsParams = z.infer< @@ -206,8 +274,8 @@ export type BulkDeleteCatalogItemsParams = z.infer< // Partial update catalog item (PATCH) export const PartialUpdateCatalogItemParamsSchema = z.object({ catalogName: z.string().describe("Name of the catalog"), - itemId: z.string().describe("ID of the catalog item"), - update: z.record(z.string(), z.any()).describe("Fields to update"), + itemId: CatalogItemIdSchema.describe("ID of the catalog item"), + update: CatalogDocumentFieldsSchema.describe("Fields to update"), }); export type PartialUpdateCatalogItemParams = z.infer< @@ -217,8 +285,8 @@ export type PartialUpdateCatalogItemParams = z.infer< // Replace catalog item (PUT) export const ReplaceCatalogItemParamsSchema = z.object({ catalogName: z.string().describe("Name of the catalog"), - itemId: z.string().describe("ID of the catalog item"), - value: z.record(z.string(), z.any()).describe("New value for the item"), + itemId: CatalogItemIdSchema.describe("ID of the catalog item"), + value: CatalogDocumentFieldsSchema.describe("New value for the item"), }); export type ReplaceCatalogItemParams = z.infer< diff --git a/tests/integration/catalogs.test.ts b/tests/integration/catalogs.test.ts index e2cb923..05c5b10 100644 --- a/tests/integration/catalogs.test.ts +++ b/tests/integration/catalogs.test.ts @@ -6,6 +6,7 @@ import { createTestIdentifiers, uniqueId, waitForCatalogItems, + waitForCatalogItemValue, withTimeout, } from "../utils/test-helpers"; @@ -46,19 +47,16 @@ describe("Catalog Management Integration Tests", () => { // Add some items to the catalog with various field types await withTimeout( - client.updateCatalogItems({ + client.partialUpdateCatalogItems({ catalogName, - items: [ - { - id: uniqueId("test-item"), + documents: { + [uniqueId("test-item")]: { name: "Test Item", price: 10.99, - dataFields: { - category: "test", - inStock: true, - }, + category: "test", + inStock: true, }, - ], + }, }) ); @@ -83,26 +81,20 @@ describe("Catalog Management Integration Tests", () => { const itemId2 = uniqueId("catalog-items-test-2"); await withTimeout( - client.updateCatalogItems({ + client.partialUpdateCatalogItems({ catalogName, - items: [ - { - id: itemId1, + documents: { + [itemId1]: { name: "Catalog Items Test Item 1", price: 5.99, - dataFields: { - category: "test", - }, + category: "test", }, - { - id: itemId2, + [itemId2]: { name: "Catalog Items Test Item 2", price: 7.99, - dataFields: { - category: "test", - }, + category: "test", }, - ], + }, }) ); @@ -134,20 +126,18 @@ describe("Catalog Management Integration Tests", () => { const itemId2 = uniqueId("pagination-test-2"); await withTimeout( - client.updateCatalogItems({ + client.partialUpdateCatalogItems({ catalogName, - items: [ - { - id: itemId1, + documents: { + [itemId1]: { name: "Pagination Test Item 1", price: 5.99, }, - { - id: itemId2, + [itemId2]: { name: "Pagination Test Item 2", price: 7.99, }, - ], + }, }) ); @@ -216,18 +206,15 @@ describe("Catalog Management Integration Tests", () => { // Add items with some fields first await withTimeout( - client.updateCatalogItems({ + client.partialUpdateCatalogItems({ catalogName, - items: [ - { - id: uniqueId("test-item"), + documents: { + [uniqueId("test-item")]: { name: "Test Item", - dataFields: { - price: 19.99, - quantity: 10, - }, + price: 19.99, + quantity: 10, }, - ], + }, }) ); @@ -266,13 +253,13 @@ describe("Catalog Management Integration Tests", () => { // Create items await withTimeout( - client.updateCatalogItems({ + client.partialUpdateCatalogItems({ catalogName, - items: [ - { id: itemId1, name: "Item 1", price: 10 }, - { id: itemId2, name: "Item 2", price: 20 }, - { id: itemId3, name: "Item 3", price: 30 }, - ], + documents: { + [itemId1]: { name: "Item 1", price: 10 }, + [itemId2]: { name: "Item 2", price: 20 }, + [itemId3]: { name: "Item 3", price: 30 }, + }, }) ); @@ -302,20 +289,17 @@ describe("Catalog Management Integration Tests", () => { // Create an item with multiple fields await withTimeout( - client.updateCatalogItems({ + client.partialUpdateCatalogItems({ catalogName, - items: [ - { - id: itemId, + documents: { + [itemId]: { name: "Original Item", price: 100, - dataFields: { - description: "Original description", - category: "electronics", - inStock: true, - }, + description: "Original description", + category: "electronics", + inStock: true, }, - ], + }, }) ); @@ -351,20 +335,17 @@ describe("Catalog Management Integration Tests", () => { // Create an item with multiple fields await withTimeout( - client.updateCatalogItems({ + client.partialUpdateCatalogItems({ catalogName, - items: [ - { - id: itemId, + documents: { + [itemId]: { name: "Original Item", price: 100, - dataFields: { - description: "Original description", - category: "electronics", - inStock: true, - }, + description: "Original description", + category: "electronics", + inStock: true, }, - ], + }, }) ); @@ -390,4 +371,91 @@ describe("Catalog Management Integration Tests", () => { // Note: We're testing the request format, not verifying the actual replacement // due to eventual consistency }, 120000); + + it("merges fields with partialUpdateCatalogItems", async () => { + const catalogName = uniqueId("test-catalog-merge"); + await withTimeout(client.createCatalog({ catalogName })); + + const itemId = uniqueId("merge-item"); + + await withTimeout( + client.replaceCatalogItems({ + catalogName, + documents: { + [itemId]: { + name: "Original Item", + price: 100, + keepField: true, + }, + }, + }) + ); + + await waitForCatalogItems(client, catalogName, [itemId]); + + await withTimeout( + client.partialUpdateCatalogItems({ + catalogName, + documents: { + [itemId]: { price: 50 }, + }, + }) + ); + + const item = await waitForCatalogItemValue( + client, + catalogName, + itemId, + (value) => + value.name === "Original Item" && + value.price === 50 && + value.keepField === true + ); + + expect(item.value).toMatchObject({ + name: "Original Item", + price: 50, + keepField: true, + }); + }, 180000); + + it("replaces items with replaceCatalogItems", async () => { + const catalogName = uniqueId("test-catalog-overwrite"); + await withTimeout(client.createCatalog({ catalogName })); + + const itemId = uniqueId("overwrite-item"); + + await withTimeout( + client.replaceCatalogItems({ + catalogName, + documents: { + [itemId]: { + name: "Original Item", + price: 100, + keepField: true, + }, + }, + }) + ); + + await waitForCatalogItems(client, catalogName, [itemId]); + + await withTimeout( + client.replaceCatalogItems({ + catalogName, + documents: { + [itemId]: { price: 50 }, + }, + }) + ); + + const item = await waitForCatalogItemValue( + client, + catalogName, + itemId, + (value) => value.price === 50 && Object.keys(value).length === 1 + ); + + expect(item.value).toEqual({ price: 50 }); + }, 180000); }); diff --git a/tests/unit/catalogs.test.ts b/tests/unit/catalogs.test.ts index 265f94d..f5ded8e 100644 --- a/tests/unit/catalogs.test.ts +++ b/tests/unit/catalogs.test.ts @@ -13,7 +13,12 @@ import { isIterableApiError, IterableApiError, } from "../../src/errors.js"; -import { UpdateCatalogItemParamsSchema } from "../../src/types/catalogs.js"; +import { + PartialUpdateCatalogItemParamsSchema, + PartialUpdateCatalogItemsParamsSchema, + ReplaceCatalogItemParamsSchema, + ReplaceCatalogItemsParamsSchema, +} from "../../src/types/catalogs.js"; import { createMockClient } from "../utils/test-helpers"; describe("Catalog Operations", () => { @@ -64,24 +69,80 @@ describe("Catalog Operations", () => { }); }); - describe("updateCatalogItems", () => { - it("should encode catalog name with special characters", async () => { - const mockResponse = { - data: { - msg: "Items updated", - code: "Success", - }, - }; - mockAxiosInstance.post.mockResolvedValue(mockResponse); + describe("bulk catalog items", () => { + it("encodes catalog name with special characters", async () => { + mockAxiosInstance.post.mockResolvedValue({ + data: { msg: "Items updated", code: "Success" }, + }); - await client.updateCatalogItems({ + await client.partialUpdateCatalogItems({ catalogName: "my catalog", - items: [{ id: "item1", name: "Test" }], + documents: { + item1: { name: "Test" }, + }, }); expect(mockAxiosInstance.post).toHaveBeenCalledWith( "/api/catalogs/my%20catalog/items", - expect.any(Object) + { + documents: { + item1: { name: "Test" }, + }, + replaceUploadedFieldsOnly: true, + } + ); + }); + + it("passes through arbitrary document fields without transformation", async () => { + mockAxiosInstance.post.mockResolvedValue({ + data: { msg: "Items updated", code: "Success" }, + }); + + const documents = { + "restaurant-123": { + name: "Good Thai", + cuisine: "thai", + averagePrice: 25, + location: { + lat: 37.78, + lon: -122.42, + }, + }, + }; + + await client.partialUpdateCatalogItems({ + catalogName: "restaurants", + documents, + }); + + expect(mockAxiosInstance.post).toHaveBeenCalledWith( + "/api/catalogs/restaurants/items", + { + documents, + replaceUploadedFieldsOnly: true, + } + ); + }); + + it.each([ + ["partialUpdateCatalogItems", true], + ["replaceCatalogItems", false], + ] as const)("sets replaceUploadedFieldsOnly via %s", async (method, flag) => { + mockAxiosInstance.post.mockResolvedValue({ + data: { msg: "Items updated", code: "Success" }, + }); + + await client[method]({ + catalogName: "products", + documents: { item1: { price: 10 } }, + }); + + expect(mockAxiosInstance.post).toHaveBeenCalledWith( + "/api/catalogs/products/items", + { + documents: { item1: { price: 10 } }, + replaceUploadedFieldsOnly: flag, + } ); }); }); @@ -395,28 +456,121 @@ describe("Catalog Operations", () => { }); describe("Schema Validation", () => { - it("should validate catalog parameters", () => { - // Valid catalog update + it("validates bulk catalog item parameters", () => { + const validParams = { + catalogName: "products", + documents: { + item1: { + name: "Test Product", + price: 29.99, + description: "A test product", + categories: ["electronics"], + }, + }, + }; + + expect(() => + PartialUpdateCatalogItemsParamsSchema.parse(validParams) + ).not.toThrow(); + expect(() => + ReplaceCatalogItemsParamsSchema.parse(validParams) + ).not.toThrow(); + + expect(() => + PartialUpdateCatalogItemsParamsSchema.parse({ + catalogName: "products", + documents: { + item1: "not-an-object", + }, + }) + ).toThrow(); + + expect(() => + PartialUpdateCatalogItemsParamsSchema.parse({ + catalogName: "products", + }) + ).toThrow(); + + expect(() => + PartialUpdateCatalogItemsParamsSchema.parse({ + catalogName: "products", + documents: {}, + }) + ).toThrow(); + + expect(() => + PartialUpdateCatalogItemsParamsSchema.parse({ + catalogName: "products", + documents: { + "invalid.id": { price: 29.99 }, + }, + }) + ).toThrow(); + + expect(() => + PartialUpdateCatalogItemsParamsSchema.parse({ + catalogName: "products", + documents: { + item1: { "bad.field": 29.99 }, + }, + }) + ).toThrow(); + expect(() => - UpdateCatalogItemParamsSchema.parse({ + PartialUpdateCatalogItemsParamsSchema.parse({ catalogName: "products", - items: [ - { - id: "item1", + documents: { + item1: { name: "Test Product", - price: 29.99, - description: "A test product", - categories: ["electronics"], + categories: ["electronics", "sale"], }, - ], + }, }) ).not.toThrow(); - // Invalid - missing required item fields expect(() => - UpdateCatalogItemParamsSchema.parse({ + PartialUpdateCatalogItemsParamsSchema.parse({ catalogName: "products", - items: [{ price: 29.99 }], // missing id and name + documents: { + item1: { + name: "Kelly's Cafe", + bestItem: { dish: "Tibs", price: 12 }, + location: { lat: 37.77, lon: -122.43 }, + }, + }, + }) + ).not.toThrow(); + }); + + it("validates single catalog item parameters", () => { + const validParams = { + catalogName: "products", + itemId: "item-123", + update: { price: 29.99 }, + }; + + expect(() => + PartialUpdateCatalogItemParamsSchema.parse(validParams) + ).not.toThrow(); + expect(() => + ReplaceCatalogItemParamsSchema.parse({ + catalogName: "products", + itemId: "item-123", + value: { name: "Test", price: 29.99 }, + }) + ).not.toThrow(); + + expect(() => + PartialUpdateCatalogItemParamsSchema.parse({ + ...validParams, + itemId: "invalid id", + }) + ).toThrow(); + + expect(() => + PartialUpdateCatalogItemParamsSchema.parse({ + ...validParams, + update: { "bad.field": 29.99 }, }) ).toThrow(); }); diff --git a/tests/utils/test-helpers.ts b/tests/utils/test-helpers.ts index 99a12d4..31fbcf8 100644 --- a/tests/utils/test-helpers.ts +++ b/tests/utils/test-helpers.ts @@ -4,7 +4,10 @@ import { randomUUID } from "crypto"; import { IterableClient } from "../../src/client"; import { config } from "../../src/config.js"; import { logger } from "../../src/logger.js"; -import type { GetCatalogItemsResponse } from "../../src/types/catalogs.js"; +import type { + CatalogItemWithProperties, + GetCatalogItemsResponse, +} from "../../src/types/catalogs.js"; import type { UserEvent, UserResponse } from "../../src/types/users.js"; // Constants for unit tests @@ -413,3 +416,32 @@ export async function waitForCatalogItems( } ); } + +export async function waitForCatalogItemValue( + client: IterableClient, + catalogName: string, + itemId: string, + predicate: (value: CatalogItemWithProperties["value"]) => boolean +): Promise { + return retryWithBackoff( + async () => { + const response = await client.getCatalogItems({ catalogName }); + const item = response.catalogItemsWithProperties.find( + (catalogItem) => catalogItem.itemId === itemId + ); + if (!item) { + throw new Error(`Catalog item ${itemId} not found yet`); + } + if (!predicate(item.value)) { + throw new Error( + `Catalog item ${itemId} value did not match expected state yet` + ); + } + return item; + }, + { + description: `Catalog item ${itemId} value to match expected state in ${catalogName}`, + timeoutMs: 90000, + } + ); +}