Generalize update handlers and add the ReadOnlyRepositoryWithFactory trait#76
Generalize update handlers and add the ReadOnlyRepositoryWithFactory trait#76
ReadOnlyRepositoryWithFactory trait#76Conversation
|
Coverage after merging add-memory-cache-for-validators into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull Request Overview
This PR adds the ReadOnlyRepositoryWithFactory trait to enable lazy creation of entities in repositories when they don't exist. The implementation allows repositories to use a factory pattern to create new entities on-demand during read operations.
Key changes:
- Added
ValueFactoryandReadOnlyRepositoryWithFactorytraits with factory-based entity creation - Refactored resource update handlers into a dedicated module structure with composed handling capabilities
- Updated method signatures to use references instead of owned values for better performance
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/base/upsert_repository.rs | Adds new ValueFactory and ReadOnlyRepositoryWithFactory traits |
| src/services/backends/memory.rs | Implements the new factory-based repository trait for in-memory HashMap storage |
| src/services/backends/kubernetes/resource_update_handler.rs | Creates new module structure for resource update handlers |
| src/services/backends/kubernetes/resource_update_handler/logging_update_handler.rs | Moves and refactors logging handler with reference-based method signature |
| src/services/backends/kubernetes/resource_update_handler/composed_update_handler.rs | Adds new composed handler for chaining multiple update handlers |
| src/services/backends/kubernetes/repositories.rs | Updates repository to support generic update handlers and adds factory trait implementation |
| src/services/backends/kubernetes/repositories/schema_repository.rs | Removes duplicate trait implementation |
| src/services/backends/kubernetes/repositories/schema_repository/tests.rs | Updates test to use new generic repository structure |
Comments suppressed due to low confidence (1)
src/services/backends/kubernetes/resource_update_handler/logging_update_handler.rs:1
- Similar issue here -
eventis a reference to aResult, so the match should useevent.as_ref()or destructure the reference properly withmatch event.
use crate::services::backends::kubernetes::resource_update_handler::ResourceUpdateHandler;
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| /// Represents a repository for policies | ||
| pub trait ReadOnlyRepositoryWithFactory<Key, Entity>: Send + Sync { |
There was a problem hiding this comment.
The documentation comment mentions 'policies' but this is a generic trait that can work with any entity type. Update the comment to reflect its generic nature.
| /// Retrieves a policy by id | ||
| async fn get( |
There was a problem hiding this comment.
The documentation comment mentions 'policy by id' but this method works with generic Key and Entity types. Update the comment to be more generic.
src/services/backends/kubernetes/resource_update_handler/logging_update_handler.rs
Outdated
Show resolved
Hide resolved
|
Coverage after merging add-memory-cache-for-validators into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ReadOnlyRepositoryWithFactory traitReadOnlyRepositoryWithFactory trait
…ng_update_handler.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Coverage after merging add-memory-cache-for-validators into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… into add-memory-cache-for-validators
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
src/services/backends/kubernetes/resource_update_handler/composed_update_handler.rs
Outdated
Show resolved
Hide resolved
|
Coverage after merging add-memory-cache-for-validators into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging add-memory-cache-for-validators into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
…sed_update_handler.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… into add-memory-cache-for-validators
|
Coverage after merging add-memory-cache-for-validators into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging add-memory-cache-for-validators into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging add-memory-cache-for-validators into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging add-memory-cache-for-validators into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Coverage after merging add-memory-cache-for-validators into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
This PR adds the ability to customize update handlers for K8S objects and adds the ability to create unbound caches for the JWT validators for the external tokens.
Unit tests to be done later in this PR.
Checklist
latestcommit.