Skip to content

Commit 6e62bd5

Browse files
committed
impl: further work and unit tests
- `KvsBuilder` unit tests suite - Fix for `snapshot_rotate` not handling scenario where only KVS or hash file is provided. - Move parameters to `KvsParameters` and provide access to them with `parameters`. - Make `flush_on_exit` separate from the rest of parameters. - Other params are KVS instance params, this one's for object instance. - Add `max_instances` and `flush_on_exit` to `KvsBuilder` API. - Fix `lib.rs` doc example - created files in CWD. - Use various instance IDs in tests - mitigate issues with common instance pool.
1 parent 9dde677 commit 6e62bd5

File tree

8 files changed

+632
-162
lines changed

8 files changed

+632
-162
lines changed

src/rust/rust_kvs/src/kvs.rs

Lines changed: 59 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,39 @@
99
//
1010
// SPDX-License-Identifier: Apache-2.0
1111

12+
use std::cell::RefCell;
1213
use std::fs;
1314
use std::marker::PhantomData;
1415
use std::path::PathBuf;
1516
use std::sync::{Arc, Mutex};
1617

1718
use crate::error_code::ErrorCode;
18-
use crate::kvs_api::{FlushOnExit, KvsApi, SnapshotId};
19+
use crate::kvs_api::{Defaults, FlushOnExit, InstanceId, KvsApi, KvsLoad, SnapshotId};
1920
use crate::kvs_backend::{KvsBackend, KvsPathResolver};
20-
use crate::kvs_builder::{KvsData, KvsParameters};
21+
use crate::kvs_builder::KvsData;
2122
use crate::kvs_value::{KvsMap, KvsValue};
2223

2324
/// Maximum number of snapshots
2425
///
2526
/// Feature: `FEAT_REQ__KVS__snapshots`
2627
const KVS_MAX_SNAPSHOTS: usize = 3;
2728

29+
/// KVS instance parameters.
30+
#[derive(Clone, PartialEq)]
31+
pub struct KvsParameters {
32+
/// Instance ID.
33+
pub instance_id: InstanceId,
34+
35+
/// Defaults handling mode.
36+
pub defaults: Defaults,
37+
38+
/// KVS load mode.
39+
pub kvs_load: KvsLoad,
40+
41+
/// Working directory.
42+
pub working_dir: PathBuf,
43+
}
44+
2845
/// Key-value-storage data
2946
pub struct GenericKvs<Backend: KvsBackend, PathResolver: KvsPathResolver = Backend> {
3047
/// KVS instance data.
@@ -33,6 +50,9 @@ pub struct GenericKvs<Backend: KvsBackend, PathResolver: KvsPathResolver = Backe
3350
/// KVS instance parameters.
3451
parameters: KvsParameters,
3552

53+
/// Flush on exit flag.
54+
flush_on_exit: RefCell<FlushOnExit>,
55+
3656
/// Marker for `Backend`.
3757
_backend_marker: PhantomData<Backend>,
3858

@@ -41,16 +61,21 @@ pub struct GenericKvs<Backend: KvsBackend, PathResolver: KvsPathResolver = Backe
4161
}
4262

4363
impl<Backend: KvsBackend, PathResolver: KvsPathResolver> GenericKvs<Backend, PathResolver> {
44-
pub(crate) fn new(data: Arc<Mutex<KvsData>>, parameters: KvsParameters) -> Self {
64+
pub(crate) fn new(
65+
data: Arc<Mutex<KvsData>>,
66+
parameters: KvsParameters,
67+
flush_on_exit: FlushOnExit,
68+
) -> Self {
4569
Self {
4670
data,
4771
parameters,
72+
flush_on_exit: RefCell::new(flush_on_exit),
4873
_backend_marker: PhantomData,
4974
_path_resolver_marker: PhantomData,
5075
}
5176
}
5277

53-
pub(crate) fn parameters(&self) -> &KvsParameters {
78+
pub fn parameters(&self) -> &KvsParameters {
5479
&self.parameters
5580
}
5681

@@ -94,18 +119,23 @@ impl<Backend: KvsBackend, PathResolver: KvsPathResolver> GenericKvs<Backend, Pat
94119

95120
println!("rotating: {snap_name_old} -> {snap_name_new}");
96121

97-
let res = fs::rename(hash_path_old, hash_path_new);
98-
if let Err(err) = res {
99-
if err.kind() != std::io::ErrorKind::NotFound {
100-
return Err(err.into());
101-
} else {
102-
continue;
103-
}
104-
}
122+
// Check snapshot and hash files exist.
123+
let snap_old_exists = snap_path_old.exists();
124+
let hash_old_exists = hash_path_old.exists();
105125

106-
let res = fs::rename(snap_path_old, snap_path_new);
107-
if let Err(err) = res {
108-
return Err(err.into());
126+
// If both exist - rename them.
127+
if snap_old_exists && hash_old_exists {
128+
fs::rename(hash_path_old, hash_path_new)?;
129+
fs::rename(snap_path_old, snap_path_new)?;
130+
}
131+
// If neither exist - continue.
132+
else if !snap_old_exists && !hash_old_exists {
133+
continue;
134+
}
135+
// In other case - this is erroneous scenario.
136+
// Either snapshot or hash file got removed.
137+
else {
138+
return Err(ErrorCode::IntegrityCorrupted);
109139
}
110140
}
111141

@@ -121,15 +151,15 @@ impl<Backend: KvsBackend, PathResolver: KvsPathResolver> KvsApi
121151
/// # Return Values
122152
/// * `FlushOnExit`: Current flush on exit behavior.
123153
fn flush_on_exit(&self) -> FlushOnExit {
124-
self.parameters.flush_on_exit.clone()
154+
self.flush_on_exit.borrow().clone()
125155
}
126156

127157
/// Control the flush on exit behavior
128158
///
129159
/// # Parameters
130160
/// * `flush_on_exit`: Flag to control flush-on-exit behavior
131161
fn set_flush_on_exit(&self, flush_on_exit: FlushOnExit) {
132-
self.parameters.flush_on_exit = flush_on_exit;
162+
*self.flush_on_exit.borrow_mut() = flush_on_exit
133163
}
134164

135165
/// Resets a key-value-storage to its initial state
@@ -517,10 +547,10 @@ impl<Backend: KvsBackend, PathResolver: KvsPathResolver> Drop
517547
mod kvs_tests {
518548
use crate::error_code::ErrorCode;
519549
use crate::json_backend::JsonBackend;
520-
use crate::kvs::{GenericKvs, KVS_MAX_SNAPSHOTS};
550+
use crate::kvs::{GenericKvs, KvsParameters, KVS_MAX_SNAPSHOTS};
521551
use crate::kvs_api::{Defaults, FlushOnExit, InstanceId, KvsApi, KvsLoad, SnapshotId};
522552
use crate::kvs_backend::{KvsBackend, KvsPathResolver};
523-
use crate::kvs_builder::{KvsData, KvsParameters};
553+
use crate::kvs_builder::KvsData;
524554
use crate::kvs_value::{KvsMap, KvsValue};
525555
use std::path::PathBuf;
526556
use std::sync::{Arc, Mutex};
@@ -595,10 +625,9 @@ mod kvs_tests {
595625
instance_id,
596626
defaults: Defaults::Optional,
597627
kvs_load: KvsLoad::Optional,
598-
flush_on_exit: FlushOnExit::No,
599628
working_dir,
600629
};
601-
GenericKvs::<B>::new(data, parameters)
630+
GenericKvs::<B>::new(data, parameters, FlushOnExit::No)
602631
}
603632

604633
#[test]
@@ -607,6 +636,15 @@ mod kvs_tests {
607636
get_kvs::<MockBackend>(PathBuf::new(), KvsMap::new(), KvsMap::new());
608637
}
609638

639+
#[test]
640+
fn test_parameters_ok() {
641+
let kvs = get_kvs::<MockBackend>(PathBuf::new(), KvsMap::new(), KvsMap::new());
642+
assert_eq!(kvs.parameters().instance_id, InstanceId(1));
643+
assert_eq!(kvs.parameters().defaults, Defaults::Optional);
644+
assert_eq!(kvs.parameters().kvs_load, KvsLoad::Optional);
645+
assert_eq!(kvs.parameters().working_dir, PathBuf::new());
646+
}
647+
610648
#[test]
611649
fn test_reset() {
612650
let kvs = get_kvs::<MockBackend>(

0 commit comments

Comments
 (0)