225 lines
6.0 KiB
Markdown
225 lines
6.0 KiB
Markdown
|
# 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)
|