diff --git a/CHANGELOG.md b/CHANGELOG.md index c810cc7..ce7078f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [3.5.0] - 2026-03-12 + +### Added + +- **Optional `tableFactory` parameter in SchemaManager constructor** (SOSO-451) + - New optional 3rd parameter: `tableFactory?: DynamicTableFactoryInterface` + - When provided, the injected factory is used instead of creating one internally + - Enables custom policies (e.g., `WriteConversionPolicy`, `UnknownFieldPolicy`) without unsafe property overrides + - When not provided, behavior is identical to v3.4.0 (fully backward compatible) + - Resolves OCP violation workaround in downstream MCP servers + ## [3.1.0] - 2026-02-21 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index cd17df8..e5d61c8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -124,9 +124,10 @@ npx appsheet inspect --help # After npm install (uses bin entry) **SchemaManager** (`src/utils/SchemaManager.ts`) - Central management class using factory injection (v3.0.0) -- **v3.0.0 Constructor**: `new SchemaManager(clientFactory, schema)` +- **v3.0.0 Constructor**: `new SchemaManager(clientFactory, schema, tableFactory?)` - `clientFactory`: AppSheetClientFactoryInterface (use AppSheetClientFactory or MockAppSheetClientFactory) - `schema`: SchemaConfig from SchemaLoader + - `tableFactory` (optional): DynamicTableFactoryInterface — when provided, used instead of creating one internally. Enables custom policies (e.g., WriteConversionPolicy). - **`table(connection, tableName, runAsUserEmail)`**: Creates DynamicTable instances on-the-fly - `runAsUserEmail` is required in v3.0.0 (not optional) - Each call creates a new client instance (lightweight operation) @@ -367,6 +368,10 @@ await table.add([{ discount: 1.5 }]); const prodFactory = new AppSheetClientFactory(); const prodDb = new SchemaManager(prodFactory, schema); +// Production with custom policies: Inject pre-configured DynamicTableFactory +const tableFactory = new DynamicTableFactory(prodFactory, schema, undefined, writePolicy); +const prodDbWithPolicy = new SchemaManager(prodFactory, schema, tableFactory); + // Testing: Use MockAppSheetClientFactory const testFactory = new MockAppSheetClientFactory(mockData); const testDb = new SchemaManager(testFactory, schema); @@ -378,6 +383,7 @@ const testDb = new SchemaManager(testFactory, schema); - No need to mock axios or network calls - Test data can be pre-seeded via MockDataProvider - Same code paths for production and test environments +- Custom DynamicTableFactory injection for policies (e.g., WriteConversionPolicy) ### Error Handling diff --git a/docs/SOSO-451/FEATURE_CONCEPT.md b/docs/SOSO-451/FEATURE_CONCEPT.md new file mode 100644 index 0000000..8c6ecd5 --- /dev/null +++ b/docs/SOSO-451/FEATURE_CONCEPT.md @@ -0,0 +1,256 @@ +# SchemaManager: Optionaler tableFactory-Parameter + +## Status: Konzept + +| Feld | Wert | +| ---------- | ----------------------------------------------------------- | +| JIRA | SOSO-451 | +| GitHub | #21 (Feature Request) | +| Version | v3.5.0 (Minor — neuer optionaler Parameter) | +| Abhaengig | Keine | +| Betrifft | SchemaManager (`src/utils/SchemaManager.ts`) | +| Prioritaet | Mittel — Workaround: unsicherer Property-Override existiert | + +--- + +## Problemanalyse + +### Ausgangslage + +Der `SchemaManager`-Konstruktor (Zeile 64-78 in `src/utils/SchemaManager.ts`) erstellt +seine `DynamicTableFactory` intern mit nur 2 Parametern: + +```typescript +constructor( + clientFactory: AppSheetClientFactoryInterface, + private readonly schema: SchemaConfig +) { + // ... + this.tableFactory = new DynamicTableFactory(clientFactory, schema); +} +``` + +### Konsequenz + +Consumer koennen keine vorkonfigurierte `DynamicTableFactory` mit Custom Policies +uebergeben. Im `service_portfolio_mcp`-Projekt wird eine Factory mit +`LocaleWriteConversionPolicy` fuer korrekte Datums-/Zahlenformatierung benoetigt. + +Aktueller Workaround (OCP-Verletzung): + +```typescript +(schemaManager as unknown as { tableFactory: DynamicTableFactory }).tableFactory = tableFactory; +``` + +Dieser Workaround: + +- Bricht bei internem Refactoring (private Property-Name aendert sich) +- Umgeht TypeScript-Sicherheit (`unknown`-Cast) +- Ist fuer Consumer nicht dokumentiert und nicht auffindbar + +--- + +## Loesung: Optionaler 3. Konstruktor-Parameter + +### Kernidee + +Der `SchemaManager`-Konstruktor erhaelt einen optionalen 3. Parameter +`tableFactory?: DynamicTableFactoryInterface`. Wenn uebergeben, wird die +injizierte Factory verwendet. Wenn nicht, wird wie bisher intern eine +`DynamicTableFactory` erstellt. + +### Vorher (v3.4.0) + +```typescript +const clientFactory = new AppSheetClientFactory(); +const schema = SchemaLoader.fromYaml('./schema.yaml'); + +// Standard — keine Custom Policies moeglich +const db = new SchemaManager(clientFactory, schema); +``` + +### Nachher (v3.5.0) + +```typescript +const clientFactory = new AppSheetClientFactory(); +const schema = SchemaLoader.fromYaml('./schema.yaml'); + +// Option A: Ohne Factory (unveraendertes Verhalten) +const db = new SchemaManager(clientFactory, schema); + +// Option B: Mit Custom Factory (z.B. fuer Locale-Konvertierung) +const tableFactory = new DynamicTableFactory( + clientFactory, + schema, + undefined, // unknownFieldPolicy + new LocaleWriteConversionPolicy() // writeConversionPolicy +); +const dbWithLocale = new SchemaManager(clientFactory, schema, tableFactory); +``` + +--- + +## Betroffene Dateien + +### `src/utils/SchemaManager.ts` + +**Konstruktor-Signatur erweitern:** + +```typescript +constructor( + clientFactory: AppSheetClientFactoryInterface, + private readonly schema: SchemaConfig, + tableFactory?: DynamicTableFactoryInterface // NEU: optional +) { + // Validate schema + const validation = SchemaLoader.validate(schema); + if (!validation.valid) { + throw new ValidationError( + `Invalid schema: ${validation.errors.join(', ')}`, + validation.errors + ); + } + + // Use injected factory or create default + this.tableFactory = tableFactory ?? new DynamicTableFactory(clientFactory, schema); +} +``` + +**Aenderung:** 1 Zeile Signatur, 1 Zeile Body (Fallback mit Nullish Coalescing). + +### TSDoc aktualisieren + +````typescript +/** + * Creates a new SchemaManager. + * + * @param clientFactory - Factory to create AppSheetClient instances + * @param schema - Schema configuration containing connection and table definitions + * @param tableFactory - Optional pre-configured DynamicTableFactory. + * When provided, this factory is used instead of creating a new one internally. + * Use this to inject factories with custom policies (e.g., WriteConversionPolicy). + * @throws {ValidationError} If the schema is invalid + * + * @example + * ```typescript + * // Without custom factory (default behavior) + * const db = new SchemaManager(clientFactory, schema); + * + * // With custom factory (e.g., for locale-aware write conversion) + * const tableFactory = new DynamicTableFactory(clientFactory, schema, undefined, writePolicy); + * const db = new SchemaManager(clientFactory, schema, tableFactory); + * ``` + */ +```` + +--- + +## Abwaertskompatibilitaet + +| Aspekt | Bewertung | Begruendung | +| --------------------- | ------------- | ------------------------------------------------------ | +| Bestehende Aufrufe | Kompatibel | Parameter ist optional, Default-Verhalten unveraendert | +| API-Kontrakt | Kompatibel | Additive Aenderung (neuer optionaler Parameter) | +| Semver-Einstufung | Minor (3.5.0) | Feature-Addition ohne Breaking Change | +| TypeScript-Interfaces | Kompatibel | Keine Interface-Aenderung noetig | + +**Kein Consumer muss Code aendern.** Bestehende Aufrufe mit 2 Parametern +funktionieren identisch. + +--- + +## Test-Strategie + +### Unit-Tests: Konstruktor-Pfade + +```typescript +describe('SchemaManager constructor', () => { + describe('without tableFactory (default behavior)', () => { + it('should create internal DynamicTableFactory', () => { + const db = new SchemaManager(clientFactory, validSchema); + // Verify table() works (implicitly tests internal factory creation) + const table = db.table('default', 'users', 'user@example.com'); + expect(table).toBeInstanceOf(DynamicTable); + }); + }); + + describe('with injected tableFactory', () => { + it('should use the provided factory instead of creating a new one', () => { + const mockTableFactory: DynamicTableFactoryInterface = { + create: jest.fn().mockReturnValue(mockDynamicTable), + }; + + const db = new SchemaManager(clientFactory, validSchema, mockTableFactory); + db.table('default', 'users', 'user@example.com'); + + expect(mockTableFactory.create).toHaveBeenCalledWith('default', 'users', 'user@example.com'); + }); + + it('should not create a DynamicTableFactory when one is provided', () => { + const spy = jest.spyOn(DynamicTableFactory.prototype, 'create'); + const customFactory: DynamicTableFactoryInterface = { + create: jest.fn().mockReturnValue(mockDynamicTable), + }; + + new SchemaManager(clientFactory, validSchema, customFactory); + + // Verify the internal factory's create() is never called + expect(spy).not.toHaveBeenCalled(); + spy.mockRestore(); + }); + }); + + describe('schema validation still applies', () => { + it('should throw ValidationError for invalid schema even with custom factory', () => { + const customFactory: DynamicTableFactoryInterface = { + create: jest.fn(), + }; + + expect(() => { + new SchemaManager(clientFactory, invalidSchema, customFactory); + }).toThrow(ValidationError); + }); + }); +}); +``` + +### Integration-Test: Custom Policy durchreichen + +```typescript +describe('SchemaManager with custom WriteConversionPolicy', () => { + it('should use locale-aware conversion when custom factory is provided', async () => { + const writePolicy = new LocaleWriteConversionPolicy('de-DE'); + const tableFactory = new DynamicTableFactory(mockClientFactory, schema, undefined, writePolicy); + + const db = new SchemaManager(mockClientFactory, schema, tableFactory); + const table = db.table('default', 'worklogs', 'user@example.com'); + + // Verify that write operations use the locale policy + await table.add([{ date: '2026-03-12' }]); + // Assert that the date was converted to de-DE format before API call + }); +}); +``` + +--- + +## Implementierungsplan + +| Phase | Aufwand | Beschreibung | +| --------------- | --------- | ------------------------------------------------- | +| 1. Konstruktor | 0.25h | Optionalen Parameter + Fallback-Logik hinzufuegen | +| 2. TSDoc | 0.25h | Dokumentation aktualisieren | +| 3. Unit-Tests | 0.5h | Tests fuer beide Pfade + Validation | +| 4. Version Bump | 0.1h | `package.json` auf 3.5.0 | +| 5. AGENTS.md | 0.25h | Dokumentation in AGENTS.md aktualisieren | +| **Gesamt** | **~1.5h** | | + +--- + +## Risikobewertung + +| Risiko | Einstufung | Mitigation | +| ------------------------------- | ----------- | ------------------------------------------------ | +| Breaking Change | Kein Risiko | Neuer optionaler Parameter, Default unveraendert | +| Schema-Validation wird umgangen | Kein Risiko | Validation laeuft vor Factory-Zuweisung | +| Inkonsistente Factory/Schema | Niedrig | Consumer-Verantwortung, dokumentiert in TSDoc | diff --git a/package-lock.json b/package-lock.json index 37b43b1..d1141f4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@techdivision/appsheet", - "version": "3.4.0", + "version": "3.5.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@techdivision/appsheet", - "version": "3.4.0", + "version": "3.5.0", "license": "MIT", "dependencies": { "@types/uuid": "^10.0.0", diff --git a/package.json b/package.json index 64bb6b9..c353ffb 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@techdivision/appsheet", - "version": "3.4.0", + "version": "3.5.0", "description": "Generic TypeScript library for AppSheet CRUD operations", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/src/utils/SchemaManager.ts b/src/utils/SchemaManager.ts index b6151ad..4161a9f 100644 --- a/src/utils/SchemaManager.ts +++ b/src/utils/SchemaManager.ts @@ -52,18 +52,27 @@ export class SchemaManager { * * @param clientFactory - Factory to create AppSheetClient instances * @param schema - Schema configuration containing connection and table definitions + * @param tableFactory - Optional pre-configured DynamicTableFactory. + * When provided, this factory is used instead of creating a new one internally. + * Use this to inject factories with custom policies (e.g., WriteConversionPolicy). * @throws {ValidationError} If the schema is invalid * * @example * ```typescript + * // Without custom factory (default behavior) * const factory = new AppSheetClientFactory(); * const schema = SchemaLoader.fromYaml('./schema.yaml'); * const db = new SchemaManager(factory, schema); + * + * // With custom factory (e.g., for locale-aware write conversion) + * const tableFactory = new DynamicTableFactory(factory, schema, undefined, writePolicy); + * const db = new SchemaManager(factory, schema, tableFactory); * ``` */ constructor( clientFactory: AppSheetClientFactoryInterface, - private readonly schema: SchemaConfig + private readonly schema: SchemaConfig, + tableFactory?: DynamicTableFactoryInterface ) { // Validate schema const validation = SchemaLoader.validate(schema); @@ -74,8 +83,8 @@ export class SchemaManager { ); } - // Create table factory using injected client factory - this.tableFactory = new DynamicTableFactory(clientFactory, schema); + // Use injected factory or create default + this.tableFactory = tableFactory ?? new DynamicTableFactory(clientFactory, schema); } /** diff --git a/tests/utils/SchemaManager.test.ts b/tests/utils/SchemaManager.test.ts index 25671b4..97fa79d 100644 --- a/tests/utils/SchemaManager.test.ts +++ b/tests/utils/SchemaManager.test.ts @@ -11,8 +11,8 @@ */ import { SchemaManager } from '../../src/utils/SchemaManager'; -import { SchemaConfig, MockDataProvider } from '../../src/types'; -import { DynamicTable, MockAppSheetClientFactory } from '../../src/client'; +import { SchemaConfig, MockDataProvider, DynamicTableFactoryInterface } from '../../src/types'; +import { DynamicTable, DynamicTableFactory, MockAppSheetClientFactory } from '../../src/client'; describe('SchemaManager v3.0.0', () => { /** @@ -92,9 +92,7 @@ describe('SchemaManager v3.0.0', () => { }, }; - expect(() => new SchemaManager(factory, invalidSchema)).toThrow( - /Invalid schema/ - ); + expect(() => new SchemaManager(factory, invalidSchema)).toThrow(/Invalid schema/); }); it('should work with MockAppSheetClientFactory', () => { @@ -103,6 +101,60 @@ describe('SchemaManager v3.0.0', () => { expect(manager.getConnections()).toEqual(['test-conn', 'hr-conn']); }); + + it('should accept optional tableFactory parameter', () => { + const clientFactory = new MockAppSheetClientFactory(); + const tableFactory = new DynamicTableFactory(clientFactory, baseSchema); + + const manager = new SchemaManager(clientFactory, baseSchema, tableFactory); + + expect(manager).toBeInstanceOf(SchemaManager); + }); + + it('should use injected tableFactory when provided', () => { + const clientFactory = new MockAppSheetClientFactory(); + const mockTable = {} as DynamicTable; + const customFactory: DynamicTableFactoryInterface = { + create: jest.fn().mockReturnValue(mockTable), + }; + + const manager = new SchemaManager(clientFactory, baseSchema, customFactory); + const result = manager.table('test-conn', 'users', 'user@example.com'); + + expect(customFactory.create).toHaveBeenCalledWith('test-conn', 'users', 'user@example.com'); + expect(result).toBe(mockTable); + }); + + it('should create internal DynamicTableFactory when tableFactory is not provided', () => { + const clientFactory = new MockAppSheetClientFactory(); + + const manager = new SchemaManager(clientFactory, baseSchema); + const table = manager.table('test-conn', 'users', 'user@example.com'); + + // Internal factory creates real DynamicTable instances + expect(table).toBeInstanceOf(DynamicTable); + expect(table.getTableName()).toBe('extract_user'); + }); + + it('should validate schema even when custom tableFactory is provided', () => { + const clientFactory = new MockAppSheetClientFactory(); + const customFactory: DynamicTableFactoryInterface = { + create: jest.fn(), + }; + const invalidSchema: SchemaConfig = { + connections: { + invalid: { + appId: '', + applicationAccessKey: 'key', + tables: {}, + }, + }, + }; + + expect(() => new SchemaManager(clientFactory, invalidSchema, customFactory)).toThrow( + /Invalid schema/ + ); + }); }); describe('table()', () => { @@ -273,7 +325,9 @@ describe('SchemaManager v3.0.0', () => { const table = manager.table('test-conn', 'users', 'test@example.com'); // Add - const added = await table.add([{ id: '1', email: 'alice@example.com', name: 'Alice', status: 'Active' }]); + const added = await table.add([ + { id: '1', email: 'alice@example.com', name: 'Alice', status: 'Active' }, + ]); expect(added).toHaveLength(1); // Find @@ -292,7 +346,9 @@ describe('SchemaManager v3.0.0', () => { await table.add([{ id: '1', email: 'alice@example.com', status: 'Active' }]); // Update - const updated = await table.update([{ id: '1', email: 'alice.new@example.com', status: 'Active' }]); + const updated = await table.update([ + { id: '1', email: 'alice.new@example.com', status: 'Active' }, + ]); expect(updated[0].email).toBe('alice.new@example.com'); // Delete