Skip to content

Commit d84f380

Browse files
committed
refactor: remove duplicate state persistence methods from commands
Implemented Proposal #1 - Remove Duplicate State Persistence Methods Changes: - Removed 6 duplicate persist_*_state() helper methods (3 from each command) - Commands now call repository.save() directly with ? operator - Added RepositoryError to command error enums (StatePersistence variant) - Added ErrorKind::StatePersistence for error classification - Updated Traceable implementations for new error variants - Repository already provides proper error context via with_context() Rationale: - Persistence errors should fail commands (fail-fast, not silent) - Repository is already injected, no need for wrapper methods - Single Responsibility: repository handles I/O, commands propagate errors - YAGNI: Don't abstract simple one-line dependency calls - Better observability: errors preserved with full context Impact: - -42 lines of duplicate code removed - +6 lines for error variants - All 100 tests passing - All linters passing
1 parent 658af31 commit d84f380

File tree

3 files changed

+30
-89
lines changed

3 files changed

+30
-89
lines changed

src/application/commands/configure.rs

Lines changed: 12 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ use crate::domain::environment::repository::EnvironmentRepository;
77
use crate::domain::environment::state::{
88
BaseFailureContext, ConfigureFailureContext, ConfigureStep,
99
};
10-
use crate::domain::environment::{
11-
ConfigureFailed, Configured, Configuring, Environment, Provisioned, TraceId,
12-
};
10+
use crate::domain::environment::{Configured, Configuring, Environment, Provisioned, TraceId};
1311
use crate::infrastructure::external_tools::ansible::adapter::AnsibleClient;
1412
use crate::infrastructure::trace::ConfigureTraceWriter;
1513
use crate::shared::command::CommandError;
@@ -21,6 +19,9 @@ use crate::shared::SystemClock;
2119
pub enum ConfigureCommandError {
2220
#[error("Command execution failed: {0}")]
2321
Command(#[from] CommandError),
22+
23+
#[error("Failed to persist environment state: {0}")]
24+
StatePersistence(#[from] crate::domain::environment::repository::RepositoryError),
2425
}
2526

2627
impl crate::shared::Traceable for ConfigureCommandError {
@@ -29,18 +30,23 @@ impl crate::shared::Traceable for ConfigureCommandError {
2930
Self::Command(e) => {
3031
format!("ConfigureCommandError: Command execution failed - {e}")
3132
}
33+
Self::StatePersistence(e) => {
34+
format!("ConfigureCommandError: Failed to persist environment state - {e}")
35+
}
3236
}
3337
}
3438

3539
fn trace_source(&self) -> Option<&dyn crate::shared::Traceable> {
3640
match self {
3741
Self::Command(e) => Some(e),
42+
Self::StatePersistence(_) => None,
3843
}
3944
}
4045

4146
fn error_kind(&self) -> crate::shared::ErrorKind {
4247
match self {
4348
Self::Command(_) => crate::shared::ErrorKind::CommandExecution,
49+
Self::StatePersistence(_) => crate::shared::ErrorKind::StatePersistence,
4450
}
4551
}
4652
}
@@ -126,13 +132,13 @@ impl ConfigureCommand {
126132
let environment = environment.start_configuring();
127133

128134
// Persist intermediate state
129-
self.persist_configuring_state(&environment);
135+
self.repository.save(&environment.clone().into_any())?;
130136

131137
// Execute configuration steps with explicit step tracking
132138
match self.execute_configuration_with_tracking(&environment) {
133139
Ok(configured_env) => {
134140
// Persist final state
135-
self.persist_configured_state(&configured_env);
141+
self.repository.save(&configured_env.clone().into_any())?;
136142

137143
info!(
138144
command = "configure",
@@ -150,7 +156,7 @@ impl ConfigureCommand {
150156
let failed = environment.configure_failed(context);
151157

152158
// Persist error state
153-
self.persist_configure_failed_state(&failed);
159+
self.repository.save(&failed.clone().into_any())?;
154160

155161
Err(e)
156162
}
@@ -190,44 +196,6 @@ impl ConfigureCommand {
190196
}
191197

192198
/// Persist configuring state
193-
fn persist_configuring_state(&self, environment: &Environment<Configuring>) {
194-
let any_state = environment.clone().into_any();
195-
196-
if let Err(e) = self.repository.save(&any_state) {
197-
warn!(
198-
environment = %environment.name(),
199-
error = %e,
200-
"Failed to persist configuring state. Command execution continues."
201-
);
202-
}
203-
}
204-
205-
/// Persist configured state
206-
fn persist_configured_state(&self, environment: &Environment<Configured>) {
207-
let any_state = environment.clone().into_any();
208-
209-
if let Err(e) = self.repository.save(&any_state) {
210-
warn!(
211-
environment = %environment.name(),
212-
error = %e,
213-
"Failed to persist configured state. Command execution continues."
214-
);
215-
}
216-
}
217-
218-
/// Persist configure failed state
219-
fn persist_configure_failed_state(&self, environment: &Environment<ConfigureFailed>) {
220-
let any_state = environment.clone().into_any();
221-
222-
if let Err(e) = self.repository.save(&any_state) {
223-
warn!(
224-
environment = %environment.name(),
225-
error = %e,
226-
"Failed to persist configure failed state. Command execution continues."
227-
);
228-
}
229-
}
230-
231199
/// Build failure context for a configuration error and generate trace file
232200
///
233201
/// This helper method builds structured error context including the failed step,

src/application/commands/provision.rs

Lines changed: 12 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,7 @@ use crate::domain::environment::repository::EnvironmentRepository;
2727
use crate::domain::environment::state::{
2828
BaseFailureContext, ProvisionFailureContext, ProvisionStep,
2929
};
30-
use crate::domain::environment::{
31-
Created, Environment, ProvisionFailed, Provisioned, Provisioning, TraceId,
32-
};
30+
use crate::domain::environment::{Created, Environment, Provisioned, Provisioning, TraceId};
3331
#[allow(unused_imports)]
3432
use crate::domain::{InstanceName, ProfileName};
3533
use crate::infrastructure::external_tools::ansible::adapter::AnsibleClient;
@@ -58,6 +56,9 @@ pub enum ProvisionCommandError {
5856

5957
#[error("SSH connectivity failed: {0}")]
6058
SshConnectivity(#[from] SshError),
59+
60+
#[error("Failed to persist environment state: {0}")]
61+
StatePersistence(#[from] crate::domain::environment::repository::RepositoryError),
6162
}
6263

6364
impl crate::shared::Traceable for ProvisionCommandError {
@@ -78,6 +79,9 @@ impl crate::shared::Traceable for ProvisionCommandError {
7879
Self::SshConnectivity(e) => {
7980
format!("ProvisionCommandError: SSH connectivity failed - {e}")
8081
}
82+
Self::StatePersistence(e) => {
83+
format!("ProvisionCommandError: Failed to persist environment state - {e}")
84+
}
8185
}
8286
}
8387

@@ -88,6 +92,7 @@ impl crate::shared::Traceable for ProvisionCommandError {
8892
Self::OpenTofu(e) => Some(e),
8993
Self::Command(e) => Some(e),
9094
Self::SshConnectivity(e) => Some(e),
95+
Self::StatePersistence(_) => None,
9196
}
9297
}
9398

@@ -99,6 +104,7 @@ impl crate::shared::Traceable for ProvisionCommandError {
99104
Self::OpenTofu(_) => crate::shared::ErrorKind::InfrastructureOperation,
100105
Self::SshConnectivity(_) => crate::shared::ErrorKind::NetworkConnectivity,
101106
Self::Command(_) => crate::shared::ErrorKind::CommandExecution,
107+
Self::StatePersistence(_) => crate::shared::ErrorKind::StatePersistence,
102108
}
103109
}
104110
}
@@ -206,7 +212,7 @@ impl ProvisionCommand {
206212
let environment = environment.start_provisioning();
207213

208214
// Persist intermediate state
209-
self.persist_provisioning_state(&environment);
215+
self.repository.save(&environment.clone().into_any())?;
210216

211217
// Execute provisioning steps with explicit step tracking
212218
// This allows us to know exactly which step failed if an error occurs
@@ -216,7 +222,7 @@ impl ProvisionCommand {
216222
let provisioned = provisioned.with_instance_ip(instance_ip);
217223

218224
// Persist final state
219-
self.persist_provisioned_state(&provisioned);
225+
self.repository.save(&provisioned.clone().into_any())?;
220226

221227
info!(
222228
command = "provision",
@@ -235,7 +241,7 @@ impl ProvisionCommand {
235241
let failed = environment.provision_failed(context);
236242

237243
// Persist error state
238-
self.persist_provision_failed_state(&failed);
244+
self.repository.save(&failed.clone().into_any())?;
239245

240246
Err(e)
241247
}
@@ -294,45 +300,6 @@ impl ProvisionCommand {
294300
Ok((provisioned, instance_ip))
295301
}
296302

297-
/// Persist provisioning state
298-
fn persist_provisioning_state(&self, environment: &Environment<Provisioning>) {
299-
let any_state = environment.clone().into_any();
300-
301-
if let Err(e) = self.repository.save(&any_state) {
302-
warn!(
303-
environment = %environment.name(),
304-
error = %e,
305-
"Failed to persist provisioning state. Command execution continues."
306-
);
307-
}
308-
}
309-
310-
/// Persist provisioned state
311-
fn persist_provisioned_state(&self, environment: &Environment<Provisioned>) {
312-
let any_state = environment.clone().into_any();
313-
314-
if let Err(e) = self.repository.save(&any_state) {
315-
warn!(
316-
environment = %environment.name(),
317-
error = %e,
318-
"Failed to persist provisioned state. Command execution continues."
319-
);
320-
}
321-
}
322-
323-
/// Persist provision failed state
324-
fn persist_provision_failed_state(&self, environment: &Environment<ProvisionFailed>) {
325-
let any_state = environment.clone().into_any();
326-
327-
if let Err(e) = self.repository.save(&any_state) {
328-
warn!(
329-
environment = %environment.name(),
330-
error = %e,
331-
"Failed to persist provision failed state. Command execution continues."
332-
);
333-
}
334-
}
335-
336303
// Private helper methods - organized from higher to lower level of abstraction
337304

338305
/// Render `OpenTofu` templates

src/shared/error/kind.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,12 @@ pub enum ErrorKind {
7171
/// Examples: Invalid YAML/TOML, missing required fields,
7272
/// configuration value out of range
7373
Configuration,
74+
75+
/// State persistence operation failed
76+
///
77+
/// Examples: Failed to save environment state, repository errors,
78+
/// serialization/deserialization failures, storage access issues
79+
StatePersistence,
7480
}
7581

7682
#[cfg(test)]

0 commit comments

Comments
 (0)