Skip to content

Conversation

@DarkPhoenix42
Copy link
Contributor

No description provided.

@DarkPhoenix42 DarkPhoenix42 changed the title Feat/caching feat(cache-manager): Implement CacheManager class with LRU caching and file operations Apr 26, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a new CacheManager class to implement LRU caching for file operations, along with a comprehensive suite of unit tests.

  • Adds unit tests in tests/unittests/server/cache_manager_test.cpp to cover read, write, append, cache invalidation, clear cache, LRU eviction, and concurrent reads.
  • Implements a CacheManager class in src/server/cache_manager.cpp using file system operations and LRU tracking.
  • Updates supporting files: renames a helper in src/common/file_operations.cpp and removes the unused mutex from ConnectionManager in src/client/connection_manager.cpp and include/client/connection_manager.hpp.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/unittests/server/cache_manager_test.cpp New tests covering all aspects of the CacheManager functionality
src/server/cache_manager.cpp Implementation of CacheManager with LRU caching, file read/write, append
src/common/file_operations.cpp Renamed helper function for improved consistency
src/client/connection_manager.cpp Removed redundant mutex locking from disconnect method
include/server/cache_manager.hpp Declaration and documentation of the CacheManager interface
include/client/connection_manager.hpp Removed the m_socket_mutex field to align with ConnectionManager update
Files not reviewed (1)
  • tests/unittests/server/CMakeLists.txt: Language not supported
Comments suppressed due to low confidence (1)

src/client/connection_manager.cpp:171

  • The removal of the mutex lock in disconnect() may affect thread safety when accessing m_server_socket. Please verify that appropriate synchronization is maintained.
    {

@DarkPhoenix42 DarkPhoenix42 force-pushed the feat/caching branch 2 times, most recently from 966a93a to 3ef7e0a Compare April 26, 2025 17:15
Copy link
Contributor

@v1bh475u v1bh475u left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Believing the conversion from std::vector<uint8_t> to std::string is done to remove the current hassle about handling the data properly, I approve your changes.

Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
… .hpp file

Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
…d refactor file operations

Signed-off-by: Praneeth Sarode <praneethsarode@gmail.com>
@DarkPhoenix42 DarkPhoenix42 merged commit 757fc00 into std-fenris:main Apr 27, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants