IDEA Extended

This commit is contained in:
Jakub Zych
2026-02-04 01:06:15 +01:00
parent 7446e886a8
commit d8a4a4555c
11 changed files with 2189 additions and 0 deletions

View File

@@ -0,0 +1,592 @@
# Codebase Concerns
**Analysis Date:** 2026-02-04
## Overview
This document identifies technical debt, architectural concerns, and improvement opportunities in the reference WinterCMS implementation that the Scala rewrite (SummerCMS) should address and avoid replicating.
---
## Tech Debt
### God Classes and Large Modules
**Issue:** Multiple "Manager" classes exceed 1000+ lines, handling too many concerns in single files.
**Files:**
- `modules/backend/widgets/Lists.php` (2056 lines) - UI list rendering and filtering logic combined
- `modules/backend/behaviors/RelationController.php` (1878 lines) - Relationship form handling
- `modules/cms/classes/Controller.php` (1635 lines) - CMS page controller with mixed concerns
- `modules/backend/widgets/Form.php` (1479 lines) - Form widget rendering and logic
- `modules/backend/widgets/MediaManager.php` (1379 lines) - File management UI
- `modules/system/classes/UpdateManager.php` (1147 lines) - Software updates and migrations
- `modules/system/classes/PluginManager.php` (1089 lines) - Plugin lifecycle management
- `modules/backend/widgets/Filter.php` (1129 lines) - List filtering UI
**Impact:** Difficult to test, maintain, and extend. Tight coupling between UI rendering and business logic. Requires careful refactoring to separate concerns.
**Fix approach for Scala rewrite:**
- Enforce smaller, single-responsibility modules
- Separate UI rendering from business logic
- Use composition over inheritance
- Implement clear layer boundaries
### Incomplete Refactoring and TODO Comments
**Issue:** Codebase contains numerous TODO/FIXME/HACK comments indicating incomplete work.
**Found examples:**
- `modules/cms/console/ThemeList.php:53` - TODO: List everything in the marketplace (not just popular)
- `modules/cms/console/CreateTheme.php:156` - TODO: allow support for mix
- `modules/system/classes/UpdateManager.php:937` - @TODO: Refactor when marketplace API finalized
- `modules/system/classes/PluginManager.php:698` - @TODO: Limit this to only disabled flags
- `modules/system/console/CreateModel.php:139` - @TODO: Implement this
- `modules/backend/widgets/MediaManager.php:692` - @TODO: Improve support non-local disks
- `modules/backend/traits/UploadableWidget.php:53` - @TODO: Replace cms.storage system with real disks
- `modules/backend/formwidgets/DataTable.php:110,137` - TODO: provide streaming implementation
- `modules/backend/classes/Controller.php:210` - @TODO: Support detecting module controllers
**Impact:** Features are incomplete or deferred. Maintenance burden increases as workarounds are left in place.
**Fix approach:** SummerCMS should resolve these before shipping: finalize file storage abstraction, complete marketplace integration, implement proper streaming.
### Plugin Dependency Management Complexity
**Issue:** Plugin dependency resolution is complex and fragile, with multiple disabled states.
**Files:** `modules/system/classes/PluginManager.php`
**Current states:**
- `DISABLED_MISSING` - Plugin files missing
- `DISABLED_REPLACED` - Superseded by another plugin
- `DISABLED_REPLACEMENT_FAILED` - Replacement didn't work
- `DISABLED_MISSING_DEPENDENCIES` - Dependencies unavailable
- `DISABLED_REQUEST` - Explicitly disabled
- `DISABLED_BY_USER` - User disabled
- `DISABLED_BY_CONFIG` - Config disabled
**Impact:** Complex error states difficult to diagnose. Plugin resolution order matters. Breaking changes in plugins cascade across installations.
**Fix approach:** SummerCMS should implement:
- Cleaner dependency resolution (single source of truth)
- Better error reporting for dependency conflicts
- Version constraints on plugin dependencies
- Semantic versioning enforcement
---
## Architecture & Design Issues
### Tight Coupling: Global Service Locators
**Issue:** Heavy use of PHP global functions and service locator pattern throughout.
**Examples:**
- Global `App::` calls bypassing constructor injection
- `Cache::`, `Config::`, `Schema::`, `DB::` accessed globally
- `Lang::`, `View::`, `File::` static access
- No consistent dependency injection for cross-cutting concerns
**Files:** `modules/system/classes/UpdateManager.php` (use App, Cache, Config, Schema at top)
**Impact:** Difficult to test. Hidden dependencies. Hard to understand what a class needs. Tight coupling to container.
**Fix approach for Scala rewrite:**
- Eliminate service locator pattern
- Enforce constructor injection
- Use dependency frameworks properly
- Make dependencies explicit
### Security: Debug Mode Default
**Issue:** Debug mode defaults to `true` in production-ready config.
**Files:**
- `config/app.php:23` - `'debug' => env('APP_DEBUG', true)`
- `.env.example` - No APP_DEBUG value (defaults to true)
**Impact:** Detailed stack traces exposed to unauthorized users. Reveals application structure, file paths, and database schema.
**Risk:** Critical security issue if .env file not properly configured on deployment.
**Fix approach:**
- Default to `false` in production-like environments
- Enforce explicit opt-in for debug mode
- Add warnings in logs when debug mode detected in production
### Security: Insufficient CSRF and XSS Protections
**Issue:** CSRF protection is optional and can be disabled.
**Files:** `config/cms.php:406` - `'enableCsrfProtection' => env('ENABLE_CSRF', true)`
**Problem:** While protected by default, the configuration is environment-dependent and could be accidentally disabled.
**Related:** `modules/system/twig/FilterTokenParser.php:23` - Deprecated Twig filter tag still in use.
**Fix approach:**
- Make CSRF protection mandatory, non-optional
- Enforce modern Twig syntax (apply tag instead of filter)
- Add runtime warnings for deprecated patterns
### File Storage Abstraction Missing
**Issue:** Media/file management lacks proper abstraction to non-local disks.
**Files:**
- `modules/backend/widgets/MediaManager.php:692` - @TODO: Improve support non-local disks
- `modules/backend/traits/UploadableWidget.php:53` - @TODO: Replace cms.storage system with real disks
**Impact:** Can't easily use S3, Azure, or other cloud storage. File operations tied to local filesystem.
**Fix approach for Scala rewrite:**
- Abstract file storage behind clear interface
- Support cloud providers from day one
- No assumptions about local filesystem
### Frontend-Backend Communication Constraints
**Issue:** Snowboard framework (modern replacement for AJAX) has limitations.
**Files:** Reference in `CLAUDE.md`
**Constraints:**
- No `data-request-success` attribute support (removed due to eval() safety concerns)
- Must use JavaScript event handlers instead
- Reduces developer convenience for simple cases
**Impact:** More verbose JavaScript required for simple AJAX interactions. Developers forced to use events instead of callbacks.
**Fix approach for Scala rewrite:**
- Replace entire AJAX/frontend framework with modern alternative
- Use standard fetch API with proper security
- Consider GraphQL or REST API with type safety
- Avoid both eval-based callbacks AND the need for workarounds
---
## Performance Bottlenecks
### Inefficient ORM Usage & N+1 Query Problems
**Issue:** Potential for N+1 queries in list rendering and relationship handling.
**Files:**
- `modules/backend/widgets/Lists.php:2056` - Renders many model instances, lazy-loaded relations possible
- `modules/backend/behaviors/RelationController.php:1878` - Handles HasMany/BelongsToMany without query optimization visible
**Impact:** Lists with thousands of records cause severe database load. Pagination alone doesn't fix root issue if relations aren't eagerly loaded.
**Fix approach:**
- Require explicit eager loading declarations
- Add query logging/analysis tools
- Enforce eager load helpers at ORM level
### Asset Compilation Overhead
**Issue:** Multiple asset managers and build systems create complexity.
**Files:**
- `modules/system/classes/asset/PackageManager.php`
- `modules/system/classes/asset/Vite.php`
- `modules/system/classes/asset/BundleManager.php`
- `modules/system/console/asset/mix/` - 5 separate Mix commands
**Problem:** Asset pipeline has multiple entry points and unclear ordering. Vite and Mix both exist.
**Impact:** Slow development builds. Confusion about which build system to use. Multiple cache invalidation points.
**Fix approach:** Single, unified build system in Scala rewrite. Use standard tooling (Vite, esbuild).
### Image Processing Blocking Operations
**Issue:** ImageResizer (935 lines) performs blocking operations on upload/display.
**Files:** `modules/system/classes/ImageResizer.php`
**Problem:** Synchronous image resizing during request cycles. No async/queue-based resizing visible.
**Impact:** Upload requests block on image processing. Slow response times for image-heavy uploads.
**Fix approach:**
- Queue image processing jobs
- Lazy image generation
- Pre-generate common sizes
---
## Fragile Areas
### Plugin System Bootstrap Order Dependencies
**Issue:** Plugin registration and boot order is implicit and error-prone.
**Files:** `modules/system/classes/PluginManager.php:87-94`
**Pattern:**
```php
protected $registered = false;
protected $booted = false;
```
**Problem:**
- Plugin initialization order matters but isn't explicit
- Circular dependencies possible
- No clear contract for what each phase can do
- Tests must carefully manage state
**Impact:** Plugin bugs depend on load order. Hard to reproduce locally.
**Safe modification:**
- Explicitly declare initialization phases
- Validate dependency graph before boot
- Error early if circular dependencies detected
### Form Widget Cascading
**Issue:** Complex form widget system with nested, interdependent behaviors.
**Files:**
- `modules/backend/widgets/Form.php:1479`
- `modules/backend/formwidgets/RelationManager.php`
- `modules/backend/classes/FormField.php:732`
**Problem:**
- Deep inheritance hierarchies
- Widget behavior customization via YAML configuration
- Custom field types require plugin code
- Relationship widgets have special handling mixed in
**Impact:** Adding custom form fields is fragile. Requires deep understanding of widget hierarchy. Easy to break nested forms.
**Safe modification:**
- Add comprehensive form widget tests
- Document widget lifecycle (init, render, process, save)
- Provide clear extension points
### Configuration Cascading and Overrides
**Issue:** Configuration system has multiple levels: hardcoded, env, config files, database.
**Problem:** Unclear precedence. Database settings can override app config, making them fragile.
**Files:** `modules/system/models/LogSetting.php`, `MailSetting.php`, etc.
**Impact:** Configuration state spread across multiple sources. Debugging requires checking all levels.
**Safe modification:** Use explicit configuration layers with clear precedence.
---
## Scaling Limits
### Database: Monolithic Schema Without Partitioning
**Issue:** Single database schema stores everything. No multi-tenancy support visible.
**Impact:**
- Can't shard by tenant
- Large installations get slow
- Backup/restore of entire system required
**Fix approach for Scala rewrite:**
- Support multi-tenancy from day one
- Allow schema partitioning
- Enable sharding strategy
### Memory Usage: Asset Bundling
**Issue:** CombineAssets (900 lines) loads entire asset files into memory for bundling.
**Files:** `modules/system/classes/CombineAssets.php`
**Impact:** Large CSS/JS bundles cause memory spikes during generation.
**Fix approach:** Stream-based asset bundling, not in-memory.
### Plugin System Scalability
**Issue:** Plugin loading uses filesystem iteration, not database registry.
**Files:** `modules/system/classes/PluginManager.php`
**Problem:** Every plugin discovery does `RecursiveDirectoryIterator` scans.
**Impact:** Slow startup with many plugins (100+). No caching strategy visible for plugin list.
**Fix approach:** Cache plugin registry, implement incremental discovery.
---
## Dependencies at Risk
### PHP Version Lock: 8.1 Minimum
**Risk:** PHP 8.1 reaches EOL in November 2023. Platform pins to EOL version.
**File:** `composer.json:32` - `"php": ">=8.1"`
**Impact:**
- Security updates stop after EOL
- Hosting providers drop support
- New PHP features can't be used
**Migration plan:** Move to PHP 8.2+ minimum for SummerCMS. Ensures 5+ years of support.
### Laravel Version: 9.x (EOL)
**Risk:** Laravel 9.x (latest in Winter) reaches EOL August 2024.
**File:** `composer.json:37` - `"laravel/framework": "^9.1"`
**Impact:**
- No new security patches
- Can't use Laravel 11+ features
- Dependency hell for new packages
**Migration plan:** SummerCMS should target Laravel 11+ from start.
### Storm Library Coupling
**Risk:** Heavy reliance on Winter Storm library (custom Laravel wrapper).
**Issue:** `winter/storm ~1.2.0` is single point of failure for Winter compatibility.
**Impact:**
- Updating Laravel requires Storm update first
- Storm updates could break all WinterCMS installations
- Vendor lock-in to Winter ecosystem
**Fix approach for Scala:** Eliminate need for adapters. Use upstream Laravel directly with minimal wrapping.
### Deprecated Twig Features
**Issue:** `modules/system/twig/FilterTokenParser.php:23` still supports deprecated filter tag.
**Impact:** Codebase using old Twig syntax. Upgrade to Twig 3+ blocked.
**Fix:** Migrate all filters to apply tag, remove support for filter tag.
---
## Security Considerations
### Configuration Exposure Risk
**Issue:** Backend authentication with 5-year cookie lifetime.
**File:** `config/cms.php:70` - `'backendForceRemember' => true`
**Risk:**
- XSS vulnerability could steal 5-year session
- Lost/stolen devices have long-term access
- No option to enforce re-authentication for sensitive operations
**Recommendation:**
- Default to false
- Implement role-based session timeout
- Require re-auth for sensitive operations (user creation, payment settings)
### Service Worker Backend Control
**Issue:** Backend service workers disabled by default but capability exists.
**File:** `config/cms.php:475` - `'enableBackendServiceWorkers' => false`
**Risk:** Service workers in backend could be hijacked for XSS. Configuration makes it possible to accidentally enable.
**Recommendation:** Remove this feature entirely. Service workers should only run on frontend with proper scope isolation.
### Path Traversal Protection Conditional
**Issue:** Base directory restriction can be disabled.
**File:** `config/cms.php:453` - `'restrictBaseDir' => env('RESTRICT_BASE_DIR', true)`
**Risk:** If disabled, plugins could load templates from anywhere on filesystem (directory traversal).
**Recommendation:** Make this non-configurable. Always enforce base directory restriction.
### API Keys and Secrets Unencrypted
**Issue:** UpdateManager stores API credentials as plaintext in database.
**Files:** `modules/system/classes/UpdateManager.php:65-72`
**Problem:**
```php
protected $key;
protected $secret;
```
**Risk:** Database breach exposes marketplace credentials.
**Fix approach:**
- Encrypt sensitive config values in database
- Use AWS Secrets Manager / HashiCorp Vault for production
---
## Missing Critical Features
### API Authentication: JWT Without Standards
**Issue:** Custom JWT guard implementation instead of using OAuth2/OpenID Connect.
**Reference in CLAUDE.md:** User plugin implements custom `php-open-source-saver/jwt-auth`
**Problem:**
- Custom auth = custom bugs
- Can't integrate with standard tools
- No SSO capability
- Mobile app authentication requires special handling
**Fix approach for Scala:**
- Use OAuth2 / OpenID Connect standard
- Support multiple auth methods (password, social, SAML)
- Enable federation
### API Documentation
**Issue:** No visible API documentation or schema (GraphQL, OpenAPI).
**Impact:** Third-party integrations difficult. API contracts not documented.
**Fix approach:** Generate OpenAPI/Swagger docs automatically from code.
### Structured Logging
**Issue:** Logging uses Laravel Log facade (text-based), no structured logging.
**Files:** `config/logging.php`
**Problem:** Hard to query logs. No correlation IDs for request tracing.
**Fix approach:** Implement structured JSON logging from day one.
---
## Test Coverage Gaps
### Integration Test Coverage Insufficient
**Issue:** Tests are mostly unit-level. Limited integration testing for critical paths.
**Files:**
- `modules/system/tests/` - 1061 PHP files total in modules
- Test files scattered without clear convention
**Problem:**
- Plugin system integration tests weak
- Form widget interactions not thoroughly tested
- Asset pipeline integration missing tests
- Payment processing integration gaps (expected in payment plugin)
**Priority:** HIGH
**Recommendation:**
- Add comprehensive integration tests for plugin loading
- Test form widget cascading behavior
- Test asset compilation pipeline end-to-end
- Payment system transaction state machine fully tested
### Security Testing Gaps
**Issue:** No visible security test suite.
**Missing:**
- CSRF token validation tests
- XSS injection prevention tests
- Path traversal protection tests
- Authentication bypass scenarios
- SQL injection protection tests
**Priority:** HIGH
**Recommendation:**
- Add OWASP Top 10 security tests
- Implement automated security scanning
- Penetration testing before major releases
### Performance & Load Testing
**Issue:** No load testing or performance benchmarks visible.
**Problem:**
- Can't measure regressions
- Scaling limits unknown
- Database query performance unmonitored
**Priority:** MEDIUM
**Recommendation:**
- Add load testing harness for common operations
- Benchmark plugin loading times
- Profile database queries
---
## Known Broken/Incomplete Areas
### Marketplace Integration Incomplete
**Issue:** Multiple TODO comments indicate marketplace API integration unfinished.
**Files:**
- `modules/cms/console/ThemeList.php` - Lists only popular items
- `modules/system/classes/UpdateManager.php:937` - Awaiting finalized API
**Impact:** Can't install all available plugins/themes. Can't update from marketplace.
### Theme Marketplace
**Issue:** Theme system expects marketplace but API contract undefined.
**Files:** `modules/cms/classes/Theme.php:716`
**Problem:** Theme discovery, installation, updates not fully working.
**Fix approach:** Finalize marketplace API. Build complete plugin/theme discovery and installation.
### Media Library Non-Local Storage
**Issue:** Media manager designed for local filesystem.
**Files:** `modules/backend/widgets/MediaManager.php`
**Problem:** S3, Azure, GCS not properly supported. File operations assume local disk.
**Fix approach:** Implement proper storage abstraction (Laravel Storage under the hood).
---
## Recommendations for SummerCMS Scala Rewrite
### Priority 1 (Must Fix)
1. **Eliminate Service Locator Pattern** - Use dependency injection throughout
2. **Enforce Single Responsibility** - No classes > 500 lines (strict)
3. **Complete File Storage Abstraction** - Full cloud storage support from day one
4. **Modern Authentication** - OAuth2/OpenID Connect, not custom JWT
5. **Security Defaults** - Debug mode off by default, CSRF mandatory
### Priority 2 (Should Fix)
1. **Upgrade PHP/Framework** - Target PHP 8.3+, Laravel 11+
2. **Structured Logging** - JSON logging with correlation IDs
3. **Comprehensive Testing** - >80% coverage, security tests required
4. **Multi-Tenancy Support** - Design architecture for SaaS from start
5. **API Documentation** - OpenAPI/Swagger generated from code
### Priority 3 (Nice to Have)
1. **Async Task Processing** - Built-in for long-running operations
2. **Real-time Collaboration** - WebSocket support for concurrent editing
3. **Performance Monitoring** - Built-in APM integration
4. **Advanced Caching** - Redis, Memcached strategies
5. **Plugin Marketplace** - Centralized discovery and updates
---
*Concerns audit: 2026-02-04*