refactor(daemon)!: verticalize architecture#164
refactor(daemon)!: verticalize architecture#164LordTermor wants to merge 1 commit intoanydistro:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the daemon architecture from horizontal layers to a vertical slice approach, modernizing the codebase with contemporary C++ features and replacing key infrastructure components. The changes move away from LMDB persistence to SQL via sqlgen, introduce a new error handling library (cpperr), and migrate from CMake to xmake for the main build while keeping CMake for libraries.
Key Changes:
- Introduced a new
percpptencylibrary implementing DDD-like patterns with repositories, aggregates, and domain events - Added AsyncAPI specification for WebSocket real-time updates
- Removed legacy horizontal layer code including LMDB persistence, utilities, presentation controllers, and test infrastructure
- Enhanced Docker Compose configuration with user-specific settings and development mode support
Reviewed changes
Copilot reviewed 247 out of 357 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vertical/api/asyncapi.yaml | WebSocket API specification for package synchronization events |
| percpptency/* | Complete DDD-inspired persistence library with repositories, aggregates, unit of work patterns, and SQLite adapter |
| docker-compose.yml | Enhanced with user/UID/GID mapping and additional volume mounts |
| docker-compose.dev.yml | New development-specific Docker Compose configuration |
| daemon/* (removed) | Legacy horizontal architecture removed including utilities, presentation, persistence layers, tests, and Swagger documentation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| // Extract events first if this is an aggregate root | ||
| std::vector<std::unique_ptr<DomainEvent>> events; | ||
| if constexpr (std::is_base_of_v<AggregateRoot<TEntity>, TEntity>) { | ||
| events = entity.take_uncommitted_events(); |
There was a problem hiding this comment.
Type mismatch: take_uncommitted_events() returns std::vector<DomainEvent> (by value), but the code assigns it to std::vector<std::unique_ptr<DomainEvent>>. The same issue occurs on line 183 in the update method.
| events = entity.take_uncommitted_events(); | |
| auto raw_events = entity.take_uncommitted_events(); | |
| events.reserve(raw_events.size()); | |
| for (auto& event : raw_events) { | |
| events.push_back(std::make_unique<DomainEvent>(std::move(event))); | |
| } |
| virtual ResultCount count() = 0; | ||
| virtual ResultVoid create(TEntity& entity) = 0; | ||
| virtual ResultVoid update(TEntity& entity) = 0; | ||
| virtual ResultBool delete_by_id(TId id) = 0; |
There was a problem hiding this comment.
The return type ResultBool is inconsistent with the other methods. Most CRUD delete operations return ResultVoid (indicating success/failure) rather than a boolean. Consider changing to ResultVoid for consistency unless the boolean has specific semantic meaning.
| virtual ResultBool delete_by_id(TId id) = 0; | |
| virtual ResultVoid delete_by_id(TId id) = 0; |
| ResultVoid delete_by_id(std::string id) override { | ||
| auto it = std::remove_if(storage.begin(), storage.end(), | ||
| [&](auto const& e) { return e.m_data.id == id; }); | ||
| if (it != storage.end()) { | ||
| storage.erase(it, storage.end()); | ||
| return {}; |
There was a problem hiding this comment.
Return type mismatch: The interface declares delete_by_id to return ResultBool, but the mock implementation returns ResultVoid. This violates the interface contract.
| ResultVoid delete_by_id(std::string id) override { | |
| auto it = std::remove_if(storage.begin(), storage.end(), | |
| [&](auto const& e) { return e.m_data.id == id; }); | |
| if (it != storage.end()) { | |
| storage.erase(it, storage.end()); | |
| return {}; | |
| ResultBool delete_by_id(std::string id) override { | |
| auto it = std::remove_if(storage.begin(), storage.end(), | |
| [&](auto const& e) { return e.m_data.id == id; }); | |
| if (it != storage.end()) { | |
| storage.erase(it, storage.end()); | |
| return true; |
WIP - vertical slice instead of horizontal layers - not strict DDD, more DDD-like KISS YAGNI approach - reworked persistency layer (sql via sqlgen instead of lmdb) - reworked errors (new cpperr library) - more extensive usage of modern C++ ranges - xmake instead of cmake (keeping cmake for libs) - new API with more common backend features like pagination
c9bb752 to
f74e1f1
Compare
WIP