feat(AppData): Refactor AppData class for declarative env mapping and enhanced static helpers

2025-08-15 13:17:18 +00:00
parent 61fafd2c8f
commit ed969cee47
6 changed files with 628 additions and 138 deletions
--- a/readme.plan.md
+++ b/readme.plan.md
@@ -0,0 +1,225 @@
+# AppData Refactoring Plan
+
+## Overview
+Refactor the AppData class to improve elegance, maintainability, and extensibility while maintaining 100% backward compatibility.
+
+## Current Issues
+- 100+ lines of nested switch statements in processEnvMapping
+- Static helpers recreate Qenv instances on every call
+- Complex boolean conversion logic scattered across multiple places
+- Typo: "ephermal" should be "ephemeral"
+- Difficult to test and extend with new transformations
+
+## Architecture Improvements
+
+### 1. Singleton Qenv Provider
+Create a shared Qenv instance to avoid repeated instantiation:
+
+```typescript
+let sharedQenv: plugins.qenv.Qenv | undefined;
+
+function getQenv(): plugins.qenv.Qenv {
+  if (!sharedQenv) {
+    sharedQenv = new plugins.qenv.Qenv(
+      process.cwd(),
+      plugins.path.join(process.cwd(), '.nogit')
+    );
+  }
+  return sharedQenv;
+}
+```
+
+### 2. Centralized Type Converters
+Extract all conversion logic into pure utility functions:
+
+```typescript
+function toBoolean(value: unknown): boolean {
+  if (typeof value === 'boolean') return value;
+  if (value == null) return false;
+  const s = String(value).toLowerCase();
+  return s === 'true';
+}
+
+function toJson<T>(value: unknown): T | undefined {
+  if (typeof value === 'string') {
+    try {
+      return JSON.parse(value);
+    } catch {
+      return undefined;
+    }
+  }
+  return value as T;
+}
+
+function fromBase64(value: unknown): string {
+  if (value == null) return '';
+  return Buffer.from(String(value), 'base64').toString('utf8');
+}
+
+function toNumber(value: unknown): number | undefined {
+  if (value == null) return undefined;
+  const num = Number(value);
+  return Number.isNaN(num) ? undefined : num;
+}
+
+function toString(value: unknown): string | undefined {
+  if (value == null) return undefined;
+  return String(value);
+}
+```
+
+### 3. Declarative Pipeline Architecture
+
+Replace the giant switch statement with a composable pipeline:
+
+#### Data Structures
+```typescript
+type MappingSpec = {
+  source: 
+    | { type: 'env', key: string }
+    | { type: 'hard', value: string };
+  transforms: Transform[];
+}
+
+type Transform = 'boolean' | 'json' | 'base64' | 'number';
+```
+
+#### Pipeline Functions
+```typescript
+// Parse mapping string into spec
+function parseMappingSpec(input: string): MappingSpec
+
+// Resolve the source value
+async function resolveSource(source: MappingSpec['source']): Promise<unknown>
+
+// Apply transformations
+function applyTransforms(value: unknown, transforms: Transform[]): unknown
+
+// Complete pipeline
+async function processMappingValue(mappingString: string): Promise<unknown>
+```
+
+### 4. Transform Registry
+Enable easy extension with new transforms:
+
+```typescript
+const transformRegistry: Record<string, (v: unknown) => unknown> = {
+  boolean: toBoolean,
+  json: toJson,
+  base64: fromBase64,
+  number: toNumber,
+};
+```
+
+### 5. Simplified processEnvMapping
+Build pure object tree first, then write to kvStore:
+
+```typescript
+async function evaluateMappingValue(mappingValue: any): Promise<any> {
+  if (typeof mappingValue === 'string') {
+    return processMappingValue(mappingValue);
+  }
+  if (mappingValue && typeof mappingValue === 'object') {
+    const out: any = {};
+    for (const [k, v] of Object.entries(mappingValue)) {
+      out[k] = await evaluateMappingValue(v);
+    }
+    return out;
+  }
+  return undefined;
+}
+
+// Main loop becomes:
+for (const key in this.options.envMapping) {
+  const evaluated = await evaluateMappingValue(this.options.envMapping[key]);
+  if (evaluated !== undefined) {
+    await this.kvStore.writeKey(key as keyof T, evaluated);
+  }
+}
+```
+
+## Backward Compatibility
+
+### Supported Prefixes (Maintained)
+- `hard:` - Hardcoded value
+- `hard_boolean:` - Hardcoded boolean
+- `hard_json:` - Hardcoded JSON
+- `hard_base64:` - Hardcoded base64
+- `boolean:` - Environment variable as boolean
+- `json:` - Environment variable as JSON
+- `base64:` - Environment variable as base64
+
+### Supported Suffixes (Maintained)
+- `_JSON` - Auto-parse as JSON
+- `_BASE64` - Auto-decode from base64
+
+### Typo Fix Strategy
+- Add `ephemeral` option to interface
+- Keep reading `ephermal` for backward compatibility
+- Log deprecation warning when old spelling is used
+
+## Implementation Steps
+
+1. **Add utility functions** at the top of the file
+2. **Implement pipeline functions** (parseMappingSpec, resolveSource, applyTransforms)
+3. **Refactor processEnvMapping** to use the pipeline
+4. **Update static helpers** to use shared utilities
+5. **Fix typo** with compatibility shim
+6. **Add error boundaries** for better error reporting
+7. **Test** to ensure backward compatibility
+
+## Benefits
+
+### Code Quality
+- **70% reduction** in processEnvMapping complexity
+- **Better separation** of concerns
+- **Easier testing** - each function is pure and testable
+- **Cleaner error handling** with boundaries
+
+### Performance
+- **Shared Qenv instance** reduces allocations
+- **Optional parallelization** with Promise.all
+- **Fewer repeated operations**
+
+### Maintainability
+- **Extensible** - Easy to add new transforms
+- **Readable** - Clear pipeline flow
+- **Debuggable** - Each step can be logged
+- **Type-safe** - Better TypeScript support
+
+## Testing Strategy
+
+1. **Unit tests** for each utility function
+2. **Integration tests** for the full pipeline
+3. **Backward compatibility tests** for all existing prefixes/suffixes
+4. **Edge case tests** for error conditions
+
+## Future Extensions
+
+With the transform registry, adding new features becomes trivial:
+
+```typescript
+// Add YAML support
+transformRegistry['yaml'] = (v) => YAML.parse(String(v));
+
+// Add integer parsing
+transformRegistry['int'] = (v) => parseInt(String(v), 10);
+
+// Add custom transformers
+transformRegistry['uppercase'] = (v) => String(v).toUpperCase();
+```
+
+## Migration Path
+
+1. Implement new architecture alongside existing code
+2. Gradually migrate internal usage
+3. Mark old patterns as deprecated (with warnings)
+4. Remove deprecated code in next major version
+
+## Success Metrics
+
+- All existing tests pass
+- No breaking changes for users
+- Reduced code complexity (measurable via cyclomatic complexity)
+- Improved test coverage
+- Better performance (fewer allocations, optional parallelization)