diff --git a/.github/workflows/qa.yml b/.github/workflows/qa.yml index 0d3ff49..a8e3c7b 100644 --- a/.github/workflows/qa.yml +++ b/.github/workflows/qa.yml @@ -181,11 +181,11 @@ jobs: id: psalm run: | set +e - vendor/bin/psalm --output-format=json --show-info=false > psalm.json 2>&1 + vendor/bin/psalm --output-format=json --show-info=false > psalm.json EXIT_CODE=$? # Generate SARIF for GitHub Security - vendor/bin/psalm --output-format=sarif --show-info=false > psalm.sarif 2>&1 || true + vendor/bin/psalm --output-format=sarif --show-info=false > psalm.sarif || true ERRORS=$(jq 'length' psalm.json 2>/dev/null || echo "0") echo "errors=${ERRORS}" >> $GITHUB_OUTPUT @@ -200,7 +200,7 @@ jobs: - name: Upload Psalm SARIF if: always() - uses: github/codeql-action/upload-sarif@v3 + uses: github/codeql-action/upload-sarif@v4 with: sarif_file: psalm.sarif category: psalm diff --git a/AUDIT-ARCHITECTURE.md b/AUDIT-ARCHITECTURE.md new file mode 100644 index 0000000..800d890 --- /dev/null +++ b/AUDIT-ARCHITECTURE.md @@ -0,0 +1,63 @@ +# Architecture & Design Patterns Audit + +This document contains the findings of an audit focused on the architecture and design patterns of the Core PHP Framework. The audit covers adherence to SOLID principles, service layer boundaries, event-driven consistency, dependency injection usage, action pattern implementation, and module coupling. + +## SOLID Principle Adherence + +The framework demonstrates a strong adherence to SOLID principles, which contributes to its maintainability and scalability. + +### Single Responsibility Principle (SRP) +- **Action Pattern**: The `Action` trait (`src/Core/Actions/Action.php`) is a standout example of SRP. It encourages the creation of small, focused classes that each encapsulate a single piece of business logic. This keeps controllers and other parts of the application lean and focused on their primary responsibilities. +- **ServiceDiscovery**: The `ServiceDiscovery` class (`src/Core/Service/ServiceDiscovery.php`) has a clear and single responsibility: to discover, manage, and provide access to service definitions. It handles scanning, validation, dependency resolution, and caching, all of which are tightly related to its core purpose. + +### Open/Closed Principle (OCP) +- **Event-Driven Architecture**: The event system, managed by `LifecycleEventProvider`, is a prime example of OCP. New modules can be created to listen for existing events, allowing for the extension of functionality without modifying the core framework. +- **Service Discovery**: The service discovery mechanism is open to extension. New services can be added to the application without any modification to the `ServiceDiscovery` class itself. + +### Liskov Substitution Principle (LSP) +- The framework uses interfaces like `ServiceDefinition` to define contracts for services. The `ServiceDiscovery` class relies on these contracts, and any class that implements the `ServiceDefinition` interface can be substituted without affecting the discovery process. + +### Interface Segregation Principle (ISP) +- The framework makes good use of small, focused interfaces like `ServiceDefinition` and `HealthCheckable`. This allows classes to implement only the functionality they need, avoiding the implementation of unnecessary methods. + +### Dependency Inversion Principle (DIP) +- **Dependency Injection**: The framework makes extensive use of dependency injection, particularly within the `Action` pattern. Actions have their dependencies injected, which means they depend on abstractions (interfaces or other actions) rather than concrete implementations. +- **Service Discovery**: The `ServiceDiscovery` class depends on the `ServiceDefinition` abstraction, not on the concrete service classes themselves. This inversion of control is a key aspect of the framework's design. + +## Service Layer Boundaries + +The framework's service layer is well-defined, with clear boundaries enforced by the `ServiceDiscovery` mechanism. + +- **Explicit Contracts**: Services are required to implement the `ServiceDefinition` interface, which acts as a clear contract for what a service is and how it should behave. This ensures that all services adhere to a common structure and provide the necessary metadata. +- **Dependency Management**: The `ServiceDiscovery` class manages the dependencies between services, ensuring that services are initialized in the correct order and that all required dependencies are met. This centralized dependency management helps to maintain clear boundaries between services and prevents circular dependencies. +- **Loose Coupling**: The service discovery mechanism promotes loose coupling between services. Services do not need to have direct knowledge of each other; they simply declare their dependencies, and the `ServiceDiscovery` class handles the resolution of those dependencies. + +## Event-Driven Consistency + +The event-driven architecture is a core strength of the framework, and it is implemented in a consistent and effective manner. + +- **Centralized Orchestration**: The `LifecycleEventProvider` acts as a central orchestrator for the event system. It is responsible for discovering modules, registering listeners, and firing events at the appropriate points in the application lifecycle. This centralized approach ensures that the event system is consistent and predictable. +- **Lazy Loading**: The event system is used to implement lazy loading of modules, which is a key feature of the framework. Modules are only loaded when an event they are listening for is fired, which helps to improve performance by reducing the amount of code that needs to be loaded and executed on each request. +- **Clear Conventions**: The framework has clear conventions for how events should be defined and used. Events are defined as simple classes, and listeners are defined in the `$listens` array of a module's `Boot.php` file. These conventions make it easy to understand and use the event system. + +## Dependency Injection Usage + +The framework makes effective use of dependency injection, which helps to create loosely coupled and testable code. + +- **Action Pattern**: The `Action` pattern is a prime example of the framework's commitment to dependency injection. Actions have their dependencies injected into their constructors, which makes them easy to test in isolation. The static `run` method on the `Action` trait provides a convenient way to resolve actions from the container, further promoting the use of DI. +- **Container Usage**: The framework leverages Laravel's service container to manage dependencies. This allows for the automatic resolution of dependencies, and it makes it easy to swap out implementations of interfaces. + +## Action Pattern Implementation + +The `Action` pattern is implemented consistently throughout the framework, and it is a key part of the framework's architecture. + +- **Clear Conventions**: The framework has clear conventions for how actions should be implemented. Actions are small, focused classes that have a single `handle` method. This consistency makes it easy to understand and use the action pattern. +- **Testability**: The action pattern promotes the creation of testable code. Actions can be easily tested in isolation, and their dependencies can be mocked. +- **Reusability**: Actions are reusable components that can be called from different parts of the application. This helps to reduce code duplication and improve maintainability. + +## Module Coupling Analysis + +The framework is designed to promote loose coupling between modules, which is a key factor in its scalability and maintainability. + +- **Loose Coupling**: The event-driven architecture and service discovery mechanism are the primary means of communication between modules. This means that modules do not need to have direct knowledge of each other, and they can be developed and maintained independently. +- **`BelongsToWorkspace` Discrepancy**: There is a notable discrepancy in the location of the `BelongsToWorkspace` trait. The `README.md` and some documentation refer to `Core\Mod\Tenant\Concerns\BelongsToWorkspace`, while the actual usage in the codebase (`src/Mod/Trees/Models/TreePlanting.php`) is `Core\Tenant\Concerns\BelongsToWorkspace`. However, the file for this trait could not be located in the repository at all. This suggests a potential issue with the project's structure, vendoring, or documentation. Given the importance of this trait for the multi-tenancy architecture, this is a critical finding that should be addressed.