Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Further storage refactoring #1306

Merged

Conversation

Miles-Garnsey
Copy link
Contributor

@Miles-Garnsey Miles-Garnsey commented Jun 8, 2023

This PR continues refactoring the storage classes. It breaks the MemoryStorage class out into delegated classes, turning MemoryStorage into MemoryStorageFacade. CassandraStorage is likewise renamed CassandraStorageFacade since most of its methods are now also delegates (there are some residual ones that we will deal with in the next round of refactoring).

The DAOs are now lined up on the domain model from core, with each sitting in its own package containing:

  1. An interface.
  2. A Cassandra DAO class implementing the interface.
  3. A memory based DAO implementing the interface

IStorage has been substantially broken down and now extends a variety of smaller interfaces again adhering to the domain model from the core package.

To preserve the existing semantics for callers, I have retained MemoryStorageFacade and CassandraStorageFacade, which we will attempt to now completely eliminate in the next round of refactoring. Several methods have been eliminated from these classes where they were only required internally within the DAO.

We also fix a bug where the implementation of deleteRepairRun differed between the memory and Cassandra implementations.

Still to do:

  1. This PR does not touch anything from IDistributedStorage, that needs to be straightened out as some methods listed there have made their way into CassRepairSegmentDao.
  2. Eliminate the use of CassandraStorageFacade and MemoryStorageFacade, and replace IStorage uses with the smaller interfaces which map to the domain model. This will mostly affect the classes in resources.

@github-actions
Copy link

github-actions bot commented Jun 8, 2023

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

@Miles-Garnsey Miles-Garnsey force-pushed the refactor/repairrun-storage branch from ca97fd5 to 46abfab Compare June 8, 2023 07:58
@Miles-Garnsey Miles-Garnsey marked this pull request as ready for review June 9, 2023 03:57
adejanovski
adejanovski previously approved these changes Jun 14, 2023
Copy link
Contributor

@adejanovski adejanovski left a comment

Choose a reason for hiding this comment

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

Looks good 👍
I've left a few non blocking comments. Check them and let me know what you think.

@Miles-Garnsey Miles-Garnsey merged commit b620b74 into thelastpickle:master Jun 19, 2023
adejanovski pushed a commit that referenced this pull request Jun 22, 2023
* Further storage layer refactoring. Move memory storage and Cassandra storage implementations, and interfaces for DAOs into their own packages.
* Make CassandraStorage and MemoryStorage facades which delegate their methods to the new DAOs.
* Split IStorage interface out into several smaller interfaces.
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