forked from trent_larson/crowd-funder-for-time-pwa
chore: possible upgrade
This commit is contained in:
Matthew Raymer
510
doc/active-identity-upgrade-plan.md
Normal file
510
doc/active-identity-upgrade-plan.md
Normal file
@@ -0,0 +1,510 @@
|
||||
# Active Identity Migration & Upgrade (Merged)
|
||||
|
||||
## Part 1: Database Migration Impact Analysis
|
||||
|
||||
# Database Migration Impact Analysis
|
||||
|
||||
## Current State Analysis
|
||||
|
||||
### Existing Migration Structure
|
||||
The current migration system has:
|
||||
- **Migration 003**: `003_active_identity_and_seed_backup` - Creates `active_identity` table with basic structure
|
||||
- **Foreign Key**: `ON DELETE SET NULL` constraint
|
||||
- **Basic Indexing**: Single unique index on `id`
|
||||
- **Bootstrapping**: Auto-selects first account if `activeDid` is null/empty
|
||||
|
||||
### Current Schema (Migration 003)
|
||||
```sql
|
||||
CREATE TABLE IF NOT EXISTS active_identity (
|
||||
id INTEGER PRIMARY KEY CHECK (id = 1),
|
||||
activeDid TEXT DEFAULT NULL,
|
||||
lastUpdated TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
FOREIGN KEY (activeDid) REFERENCES accounts(did) ON DELETE SET NULL
|
||||
);
|
||||
```
|
||||
|
||||
## Proposed Changes Impact
|
||||
|
||||
### 1. Foreign Key Constraint Change
|
||||
**Current**: `ON DELETE SET NULL`
|
||||
**Proposed**: `ON DELETE RESTRICT`
|
||||
|
||||
**Impact**:
|
||||
- **Data Safety**: Prevents accidental deletion of active account
|
||||
- **Migration Rewrite**: Update existing migration 003
|
||||
|
||||
### 2. Automatic Timestamp Updates
|
||||
**Current**: Manual `lastUpdated` updates in code
|
||||
**Proposed**: Database trigger for automatic updates
|
||||
|
||||
**Impact**:
|
||||
- **Code Simplification**: Remove manual timestamp updates from `PlatformServiceMixin`
|
||||
- **Consistency**: Ensures `lastUpdated` is always current
|
||||
|
||||
### 3. Enhanced Indexing
|
||||
**Current**: Single unique index on `id`
|
||||
**Proposed**: Additional index on `accounts(dateCreated, did)`
|
||||
|
||||
**Impact**:
|
||||
- **Performance Improvement**: Faster account selection queries
|
||||
- **Minimal Risk**: Additive change only
|
||||
|
||||
## Implementation Strategy
|
||||
|
||||
### Rewrite Migration 003
|
||||
Since the `active_identity` table is new and we're in active development, we can simply rewrite the existing migration:
|
||||
|
||||
```sql
|
||||
{
|
||||
name: "003_active_identity_and_seed_backup",
|
||||
sql: `
|
||||
-- Enable foreign key constraints for data integrity
|
||||
PRAGMA foreign_keys = ON;
|
||||
|
||||
-- Add UNIQUE constraint to accounts.did for foreign key support
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_accounts_did_unique ON accounts(did);
|
||||
|
||||
-- Create active_identity table with enhanced constraints
|
||||
CREATE TABLE IF NOT EXISTS active_identity (
|
||||
id INTEGER PRIMARY KEY CHECK (id = 1),
|
||||
activeDid TEXT REFERENCES accounts(did) ON DELETE RESTRICT,
|
||||
lastUpdated TEXT NOT NULL DEFAULT (datetime('now'))
|
||||
);
|
||||
|
||||
-- Add performance indexes
|
||||
CREATE UNIQUE INDEX IF NOT EXISTS idx_active_identity_single_record ON active_identity(id);
|
||||
CREATE INDEX IF NOT EXISTS idx_accounts_pick ON accounts(dateCreated, did);
|
||||
|
||||
-- Seed singleton row (only if not already exists)
|
||||
INSERT INTO active_identity (id, activeDid, lastUpdated)
|
||||
SELECT 1, NULL, datetime('now')
|
||||
WHERE NOT EXISTS (SELECT 1 FROM active_identity WHERE id = 1);
|
||||
|
||||
-- Add hasBackedUpSeed field to settings
|
||||
ALTER TABLE settings ADD COLUMN hasBackedUpSeed BOOLEAN DEFAULT FALSE;
|
||||
|
||||
-- Create automatic timestamp trigger
|
||||
CREATE TRIGGER IF NOT EXISTS trg_active_identity_touch
|
||||
AFTER UPDATE ON active_identity
|
||||
FOR EACH ROW
|
||||
BEGIN
|
||||
UPDATE active_identity
|
||||
SET lastUpdated = datetime('now')
|
||||
WHERE id = 1;
|
||||
END;
|
||||
`
|
||||
}
|
||||
```
|
||||
|
||||
|
||||
## Migration Service Updates Required
|
||||
|
||||
### Enhanced Validation Logic
|
||||
**File**: `src/services/migrationService.ts`
|
||||
|
||||
The existing validation for migration 003 needs to be enhanced to check for:
|
||||
- **Trigger existence**: `trg_active_identity_touch`
|
||||
- **Performance index**: `idx_accounts_pick`
|
||||
- **Foreign key constraint**: `ON DELETE RESTRICT`
|
||||
|
||||
### Enhanced Schema Detection
|
||||
**File**: `src/services/migrationService.ts`
|
||||
|
||||
The existing schema detection for migration 003 needs to be enhanced to verify:
|
||||
- **Table structure**: Enhanced constraints
|
||||
- **Trigger presence**: Automatic timestamp updates
|
||||
- **Index presence**: Performance optimization
|
||||
|
||||
## Risk Assessment
|
||||
|
||||
### Low Risk Changes
|
||||
- **Performance Index**: Additive only, no data changes
|
||||
- **Trigger Creation**: Additive only, improves consistency
|
||||
- **Migration Rewrite**: Clean implementation, no additional migrations
|
||||
|
||||
### Medium Risk Changes
|
||||
- **Foreign Key Change**: `ON DELETE RESTRICT` is more restrictive
|
||||
- **Validation Updates**: Need to test enhanced validation logic
|
||||
|
||||
### Mitigation Strategies
|
||||
1. **Comprehensive Testing**: Test migration on various database states
|
||||
2. **Development Phase**: Since we're in active development, risk is minimal
|
||||
3. **Clean Implementation**: Single migration with all enhancements
|
||||
4. **Validation Coverage**: Enhanced validation ensures correctness
|
||||
|
||||
## Implementation Timeline
|
||||
|
||||
### Phase 1: Migration Rewrite
|
||||
- [ ] Update migration 003 with enhanced constraints
|
||||
- [ ] Add enhanced validation logic
|
||||
- [ ] Add enhanced schema detection logic
|
||||
- [ ] Test migration on clean database
|
||||
|
||||
### Phase 2: Testing
|
||||
- [ ] Test migration on existing databases
|
||||
- [ ] Validate foreign key constraints work correctly
|
||||
- [ ] Test trigger functionality
|
||||
- [ ] Test performance improvements
|
||||
|
||||
### Phase 3: Deployment
|
||||
- [ ] Deploy enhanced migration to development
|
||||
- [ ] Monitor migration success rates
|
||||
- [ ] Deploy to production
|
||||
- [ ] Monitor for any issues
|
||||
|
||||
## Code Changes Required
|
||||
|
||||
### Files to Modify
|
||||
1. **`src/db-sql/migration.ts`** - Update migration 003 with enhanced constraints
|
||||
2. **`src/services/migrationService.ts`** - Add enhanced validation and detection logic
|
||||
3. **`src/utils/PlatformServiceMixin.ts`** - Remove manual timestamp updates
|
||||
|
||||
### Estimated Impact
|
||||
- **Migration File**: ~20 lines modified (enhanced constraints)
|
||||
- **Migration Service**: ~50 lines added (enhanced validation)
|
||||
- **PlatformServiceMixin**: ~20 lines removed (manual timestamps)
|
||||
- **Total**: ~50 lines changed
|
||||
|
||||
## Conclusion
|
||||
|
||||
The proposed database changes are **feasible and beneficial** with a much simpler implementation:
|
||||
|
||||
**Benefits**:
|
||||
- Improved data integrity with `ON DELETE RESTRICT`
|
||||
- Automatic timestamp consistency
|
||||
- Better query performance
|
||||
- Cleaner code (no manual timestamp updates)
|
||||
- **Simpler implementation** - just rewrite migration 003
|
||||
|
||||
**Risks**:
|
||||
- More restrictive foreign key constraints
|
||||
- Enhanced validation logic needs testing
|
||||
|
||||
**Recommendation**: Proceed with **rewriting migration 003** as it provides the cleanest path forward with minimal complexity and maximum benefit.
|
||||
|
||||
|
||||
---
|
||||
|
||||
## Part 2: Active Identity System Upgrade Plan
|
||||
|
||||
# Active Identity System Upgrade Plan
|
||||
|
||||
## Overview
|
||||
|
||||
This document outlines a comprehensive upgrade to the `$getActiveIdentity` system
|
||||
to address current ambiguities, race conditions, and improve overall robustness.
|
||||
|
||||
## Current Issues Identified
|
||||
|
||||
### 1. **Ambiguous Return States**
|
||||
|
||||
- Current: Returns `{ activeDid: string }` where empty string conflates multiple states
|
||||
- Problem: Cannot distinguish between "no accounts", "migration gap", "corruption cleared"
|
||||
- Impact: UI cannot provide appropriate user guidance
|
||||
|
||||
### 2. **Race Conditions (TOCTOU)**
|
||||
|
||||
- Current: Read → Write → Re-read across multiple statements
|
||||
- Problem: No transactional guard, potential for inconsistent state
|
||||
- Impact: Rare but possible data corruption scenarios
|
||||
|
||||
### 3. **No Single-Row Enforcement**
|
||||
|
||||
- Current: App logic assumes `id=1` singleton, but DB doesn't enforce
|
||||
- Problem: Multiple rows could theoretically exist
|
||||
- Impact: Unpredictable behavior
|
||||
|
||||
### 4. **Deletion Hazards**
|
||||
|
||||
- Current: No protection against deleting active account
|
||||
- Problem: App "discovers" corruption after the fact
|
||||
- Impact: Poor user experience, potential data loss
|
||||
|
||||
### 5. **Inconsistent Updates**
|
||||
|
||||
- Current: `lastUpdated` only set on some code paths
|
||||
- Problem: Audit trail incomplete
|
||||
- Impact: Debugging difficulties
|
||||
|
||||
## Proposed Solution Architecture
|
||||
|
||||
### Database Schema Changes
|
||||
|
||||
```sql
|
||||
-- Enhanced active_identity table with constraints
|
||||
CREATE TABLE IF NOT EXISTS active_identity (
|
||||
id INTEGER PRIMARY KEY CHECK (id = 1),
|
||||
activeDid TEXT REFERENCES accounts(did) ON DELETE RESTRICT,
|
||||
lastUpdated TEXT NOT NULL DEFAULT (datetime('now'))
|
||||
);
|
||||
|
||||
-- Automatic timestamp updates
|
||||
CREATE TRIGGER IF NOT EXISTS trg_active_identity_touch
|
||||
AFTER UPDATE ON active_identity
|
||||
FOR EACH ROW
|
||||
BEGIN
|
||||
UPDATE active_identity
|
||||
SET lastUpdated = datetime('now')
|
||||
WHERE id = 1;
|
||||
END;
|
||||
|
||||
-- Deterministic account selection index
|
||||
CREATE INDEX IF NOT EXISTS idx_accounts_pick
|
||||
ON accounts(dateCreated, did);
|
||||
|
||||
-- Seed singleton row
|
||||
INSERT INTO active_identity (id, activeDid)
|
||||
SELECT 1, NULL
|
||||
WHERE NOT EXISTS (SELECT 1 FROM active_identity WHERE id = 1);
|
||||
```
|
||||
|
||||
### Type System Enhancement
|
||||
|
||||
```typescript
|
||||
type ActiveIdentity =
|
||||
| { state: 'ok'; activeDid: string }
|
||||
| { state: 'unset'; activeDid: '' } // row exists, but no active set yet
|
||||
| { state: 'auto-selected'; activeDid: string } // we selected & saved first account
|
||||
| { state: 'empty-accounts'; activeDid: '' } // there are no accounts to select
|
||||
| { state: 'cleared-corrupt'; activeDid: '' } // DB had a non-existent DID; cleared
|
||||
| { state: 'error'; activeDid: '' };
|
||||
```
|
||||
|
||||
## Migration Strategy
|
||||
|
||||
### Phase 1: Database Schema Migration
|
||||
|
||||
**Files to modify:**
|
||||
|
||||
- `src/db-sql/migration.ts` - Add new migration
|
||||
- `src/services/migrationService.ts` - Update validation logic
|
||||
|
||||
**Migration Steps:**
|
||||
|
||||
1. Add foreign key constraint to `active_identity.activeDid`
|
||||
2. Add `ON DELETE RESTRICT` constraint
|
||||
3. Create timestamp trigger
|
||||
4. Add deterministic index
|
||||
5. Ensure singleton row exists
|
||||
|
||||
**Risk Assessment:** LOW
|
||||
- Additive changes only
|
||||
- Existing data preserved
|
||||
- Rollback possible
|
||||
|
||||
### Phase 2: Core Method Refactor
|
||||
|
||||
**Files to modify:**
|
||||
|
||||
- `src/utils/PlatformServiceMixin.ts` - Replace `$getActiveIdentity`
|
||||
- `src/interfaces/` - Add new type definitions
|
||||
|
||||
**Changes:**
|
||||
|
||||
1. Implement transactional `$getActiveIdentity` with explicit states
|
||||
2. Add `$setActiveIdentity` method
|
||||
3. Add `$deleteAccountSafely` method
|
||||
4. Update return type to `ActiveIdentity`
|
||||
|
||||
**Risk Assessment:** MEDIUM
|
||||
|
||||
- Breaking change to return type
|
||||
- Requires coordinated updates across codebase
|
||||
- Extensive testing needed
|
||||
|
||||
### Phase 3: Consumer Updates
|
||||
|
||||
**Files to modify:**
|
||||
|
||||
- All Vue components using `$getActiveIdentity`
|
||||
- Router guards
|
||||
- Service layer components
|
||||
|
||||
**Changes:**
|
||||
|
||||
1. Update all callers to handle new return type
|
||||
2. Implement state-specific UI logic
|
||||
3. Add appropriate user messaging
|
||||
|
||||
**Risk Assessment:** HIGH
|
||||
|
||||
- Wide impact across codebase
|
||||
- UI/UX changes required
|
||||
- Extensive testing needed
|
||||
|
||||
## Implementation Phases
|
||||
|
||||
### Phase 1: Database Foundation
|
||||
|
||||
- [ ] Create migration for schema changes
|
||||
- [ ] Test migration on development databases
|
||||
- [ ] Validate foreign key constraints work correctly
|
||||
- [ ] Test rollback scenarios
|
||||
|
||||
### Phase 2: Core Implementation
|
||||
|
||||
- [ ] Implement new `$getActiveIdentity` method
|
||||
- [ ] Add `$setActiveIdentity` method
|
||||
- [ ] Add `$deleteAccountSafely` method
|
||||
- [ ] Create comprehensive unit tests
|
||||
- [ ] Test transactional behavior
|
||||
|
||||
### Phase 3: Consumer Updates
|
||||
|
||||
- [ ] Audit all current usage of `$getActiveIdentity`
|
||||
- [ ] Create migration guide for components
|
||||
- [ ] Update critical path components first
|
||||
- [ ] Implement state-specific UI logic
|
||||
|
||||
### Phase 4: Testing & Rollout
|
||||
|
||||
- [ ] End-to-end testing
|
||||
- [ ] Performance testing
|
||||
- [ ] User acceptance testing
|
||||
- [ ] Gradual rollout with feature flags
|
||||
|
||||
## Backward Compatibility Strategy
|
||||
|
||||
### Gradual Migration Approach
|
||||
|
||||
The gradual migration strategy allows us to implement the new active identity system without breaking existing functionality. This approach minimizes risk and provides a clear path for incremental updates.
|
||||
|
||||
#### Phase 1: Dual Method Implementation
|
||||
|
||||
- **Keep existing method**: Leave current `$getActiveIdentity` unchanged (no renaming)
|
||||
- **Add new method**: Implement `$getActiveIdentityV2` with new return type and logic
|
||||
- **Feature flag control**: Use `FEATURE_FLAG_ACTIVE_IDENTITY_V2` to control which method is used
|
||||
- **Default to existing**: Start with existing method enabled to ensure no breaking changes
|
||||
|
||||
#### Phase 2: Component Migration
|
||||
|
||||
- **Audit usage**: Identify all components using `$getActiveIdentity`
|
||||
- **Create migration guide**: Document the changes needed for each component type
|
||||
- **Update incrementally**: Migrate components to use `$getActiveIdentityV2` one
|
||||
by one, starting with non-critical paths
|
||||
- **Test each migration**: Validate each component works with the new method
|
||||
before proceeding
|
||||
|
||||
#### Phase 3: Feature Flag Rollout
|
||||
|
||||
- **Enable for migrated components**: Switch feature flag for components that have been updated
|
||||
- **Monitor performance**: Track any performance or behavior changes
|
||||
- **Gradual enablement**: Enable the new method for more components as they're migrated
|
||||
- **Fallback capability**: Keep legacy method available for rollback if issues arise
|
||||
|
||||
#### Phase 4: Legacy Cleanup
|
||||
|
||||
- **Remove feature flag**: Once all components are migrated, remove the feature flag
|
||||
- **Replace existing method**: Replace the implementation of `$getActiveIdentity` with the V2 logic
|
||||
- **Remove V2 method**: Delete `$getActiveIdentityV2` and related code
|
||||
- **Update documentation**: Remove references to the V2 method
|
||||
- **Final validation**: Ensure no remaining references to V2 method
|
||||
|
||||
#### Benefits of This Approach
|
||||
|
||||
- **Zero downtime**: No breaking changes during migration
|
||||
- **Incremental testing**: Each component can be tested individually
|
||||
- **Easy rollback**: Can revert individual components or entire system if needed
|
||||
- **Clear progress tracking**: Easy to see which components still need migration
|
||||
- **Reduced risk**: Issues are isolated to individual components rather than system-wide
|
||||
|
||||
## Risk Mitigation
|
||||
|
||||
### Database Risks
|
||||
|
||||
- **Foreign Key Violations**: Test with existing data
|
||||
- **Migration Failures**: Comprehensive rollback plan
|
||||
- **Performance Impact**: Monitor query performance
|
||||
|
||||
### Code Risks
|
||||
|
||||
- **Breaking Changes**: Extensive testing, gradual rollout
|
||||
- **Race Conditions**: Comprehensive unit tests
|
||||
- **State Management**: Clear documentation, examples
|
||||
|
||||
### User Experience Risks
|
||||
|
||||
- **Confusing States**: Clear UI messaging
|
||||
- **Data Loss**: Backup strategies, validation
|
||||
- **Performance**: Monitor response times
|
||||
|
||||
## Success Metrics
|
||||
|
||||
### Technical Metrics
|
||||
|
||||
- Zero race condition incidents
|
||||
- 100% test coverage for new methods
|
||||
- <50ms average response time
|
||||
- Zero data corruption incidents
|
||||
|
||||
### User Experience Metrics
|
||||
|
||||
- Clear state-specific messaging
|
||||
- Reduced user confusion
|
||||
- Improved onboarding flow
|
||||
- Faster account switching
|
||||
|
||||
## Rollback Plan
|
||||
|
||||
### Database Rollback
|
||||
|
||||
1. Remove foreign key constraints
|
||||
2. Remove triggers
|
||||
3. Remove indexes
|
||||
4. Restore previous migration state
|
||||
|
||||
### Code Rollback
|
||||
|
||||
1. Revert to previous `$getActiveIdentity` implementation
|
||||
2. Remove new type definitions
|
||||
3. Restore previous component logic
|
||||
4. Remove new methods
|
||||
|
||||
## Testing Strategy
|
||||
|
||||
### Unit Tests
|
||||
|
||||
- Test all state transitions
|
||||
- Test transactional behavior
|
||||
- Test error conditions
|
||||
- Test edge cases
|
||||
|
||||
### Integration Tests
|
||||
|
||||
- Test with real database
|
||||
- Test migration scenarios
|
||||
- Test concurrent access
|
||||
- Test foreign key constraints
|
||||
|
||||
### End-to-End Tests
|
||||
|
||||
- Test complete user flows
|
||||
- Test account creation/deletion
|
||||
- Test active identity switching
|
||||
- Test error recovery
|
||||
|
||||
## Documentation Requirements
|
||||
|
||||
### Developer Documentation
|
||||
|
||||
- Migration guide for components
|
||||
- API documentation for new methods
|
||||
- Type definitions and examples
|
||||
- Best practices guide
|
||||
|
||||
### User Documentation
|
||||
|
||||
- State-specific UI messaging
|
||||
- Error message guidelines
|
||||
- Onboarding flow updates
|
||||
- Troubleshooting guide
|
||||
|
||||
## Conclusion
|
||||
|
||||
This upgrade addresses critical architectural issues while providing a clear path
|
||||
forward. The phased approach minimizes risk while ensuring thorough testing and
|
||||
validation at each step.
|
||||
|
||||
**Recommendation:** Proceed with Phase 1 (Database Schema) immediately, as it
|
||||
provides immediate benefits with minimal risk. The gradual migration approach allows
|
||||
for incremental updates without breaking existing functionality.
|
||||
Reference in New Issue
Block a user