Skip to content

chore: Refactoring (WIP) - #79

Open
rromanowski-figure wants to merge 10 commits into
mainfrom
chore/refactor-depulicate-code
Open

chore: Refactoring (WIP)#79
rromanowski-figure wants to merge 10 commits into
mainfrom
chore/refactor-depulicate-code

Conversation

@rromanowski-figure

@rromanowski-figure rromanowski-figure commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Context

Refactor effort to mainly address duplicate code and better organization

Changes

  • Define nested config objects
  • New domain-to-proto converters
  • Standardize env parsing
  • Improve module organization
  • Small renames for readability

@rromanowski-figure rromanowski-figure changed the title Chore/refactor depulicate code chore: Refactoring (WIP) Jul 31, 2026
Comment thread src/config/env_var.rs

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Functions extracted from Config::from_env to increase readability, improve error messages (by providing actual parse error, when applicable) and provide defaults in native type

Comment thread src/config/mod.rs
}

#[derive(Debug)]
pub struct DbConfig {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

same settings, now logically grouped

Comment thread src/config/mod.rs
}

#[derive(Clone, Debug)]
pub struct MiddlewareConfig {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

same settings, now logically grouped

Comment thread src/domain/types.rs
fn from(error: OsError) -> Self {
let code = match error {
OsError::AddrParseError(_) => tonic::Code::Internal,
OsError::ProstDecodeError(_) => tonic::Code::Internal,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

new error to handle with improper sqlx error removed from domain error chaining

Comment thread src/object/mod.rs
}

impl Object {
pub fn storage_path(&self) -> StoragePath {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

new extracted function

Comment thread src/public_key/cache.rs
impl Cache {
/// populate initial cache
pub async fn new(pool: Arc<PgPool>) -> Result<Arc<Mutex<Cache>>, OsError> {
pub async fn new(keys: Vec<PublicKey>) -> Result<Arc<Mutex<Cache>>, OsError> {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

remove coupling to db - pass in the initial keys when initialized

Comment thread src/datastore.rs
#[allow(
dead_code,
reason = "https://github.com/provenance-io/object-store/issues/47"
)]

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

these were removed since no dead code was reported, but I believe these were an anachronism and this was "fixed" previously

Comment thread src/db.rs
/// 3. Migrates database
pub async fn connect_and_migrate(config: &Config) -> Result<Arc<PgPool>, Error> {
let schema = config.db_schema.clone();
pub async fn connect_and_migrate(config: &DbConfig) -> Result<Arc<PgPool>, Error> {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

tighten scope of config needed

Comment thread src/lib.rs
Comment on lines +61 to +62
let initial_keys = datastore::get_all_public_keys(&db_pool).await?;
Cache::new(initial_keys).await?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

pass in keys instead of reference to db

Comment thread tests/common/config.rs
StorageType,
};

pub fn test_replication_config(

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Replaced ReplicationConfig::new from source and renamed for tests

@rromanowski-figure
rromanowski-figure marked this pull request as ready for review August 1, 2026 14:13
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.

1 participant