Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions rust/agama-lib/src/storage/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl<'a> StorageClient<'a> {
pub async fn set_config(&self, settings: StorageSettings) -> Result<u32, ServiceError> {
Ok(self
.storage_proxy
.set_config(serde_json::to_string(&settings).unwrap().as_str())
.set_config(serde_json::to_string(&settings)?.as_str())
.await?)
}

Expand All @@ -158,9 +158,9 @@ impl<'a> StorageClient<'a> {
}

/// Get the storage config according to the JSON schema
pub async fn get_config(&self) -> Result<StorageSettings, ServiceError> {
pub async fn get_config(&self) -> Result<Option<StorageSettings>, ServiceError> {
let serialized_settings = self.storage_proxy.get_config().await?;
let settings = serde_json::from_str(serialized_settings.as_str()).unwrap();
let settings = serde_json::from_str(serialized_settings.as_str())?;
Ok(settings)
}

Expand Down Expand Up @@ -230,7 +230,7 @@ impl<'a> StorageClient<'a> {
&'b self,
object: &'b DBusObject,
name: &str,
) -> Option<&HashMap<String, OwnedValue>> {
) -> Option<&'b HashMap<String, OwnedValue>> {
let interface: OwnedInterfaceName = InterfaceName::from_str_unchecked(name).into();
let interfaces = &object.1;
interfaces.get(&interface)
Expand Down
2 changes: 1 addition & 1 deletion rust/agama-lib/src/storage/http_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ impl StorageHTTPClient {
Self { client: base }
}

pub async fn get_config(&self) -> Result<StorageSettings, ServiceError> {
pub async fn get_config(&self) -> Result<Option<StorageSettings>, ServiceError> {
self.client.get("/storage/config").await
}

Expand Down
28 changes: 26 additions & 2 deletions rust/agama-lib/src/storage/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl StorageStore {
})
}

pub async fn load(&self) -> Result<StorageSettings, ServiceError> {
pub async fn load(&self) -> Result<Option<StorageSettings>, ServiceError> {
self.storage_client.get_config().await
}

Expand Down Expand Up @@ -80,7 +80,9 @@ mod test {
let url = server.url("/api");

let store = storage_store(url);
let settings = store.load().await?;
let opt_settings = store.load().await?;
assert!(opt_settings.is_some());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this condition is not met, the assert! call will crash the program. Is that what we want? However, it is just the CLI, so I guess we can live with that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ENOCOFFEE? This is the test module not the CLI :)

Copy link
Copy Markdown
Contributor

@imobachgs imobachgs Feb 25, 2025

Choose a reason for hiding this comment

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

And if crashing is expected anyway, I would say that using expect is more Rust-like.

let settings = op_settings.expect("...")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, let's grab another coffee :-)

let settings = opt_settings.unwrap();

// main assertion
assert_eq!(settings.storage.unwrap().get(), r#"{ "some": "stuff" }"#);
Expand All @@ -91,6 +93,28 @@ mod test {
Ok(())
}

#[test]
async fn test_getting_storage_null() -> Result<(), Box<dyn Error>> {
let server = MockServer::start();
let storage_mock = server.mock(|when, then| {
when.method(GET).path("/api/storage/config");
then.status(200)
.header("content-type", "application/json")
.body("null");
});
let url = server.url("/api");

let store = storage_store(url);
let opt_settings = store.load().await?;

// main assertion
assert!(opt_settings.is_none());

// Ensure the specified mock was called exactly one time (or fail with a detailed error description).
storage_mock.assert();
Ok(())
}

#[test]
async fn test_setting_storage_ok() -> Result<(), Box<dyn Error>> {
let server = MockServer::start();
Expand Down
7 changes: 4 additions & 3 deletions rust/agama-lib/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,10 @@ impl Store {
..Default::default()
};

let storage_settings = self.storage.load().await?;
settings.storage = storage_settings.storage;
settings.storage_autoyast = storage_settings.storage_autoyast;
if let Some(storage_settings) = self.storage.load().await? {
settings.storage = storage_settings.storage;
settings.storage_autoyast = storage_settings.storage_autoyast;
}

// TODO: use try_join here
Ok(settings)
Expand Down
4 changes: 3 additions & 1 deletion rust/agama-server/src/storage/web.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ pub async fn storage_service(dbus: zbus::Connection) -> Result<Router, ServiceEr
(status = 400, description = "The D-Bus service could not perform the action")
)
)]
async fn get_config(State(state): State<StorageState<'_>>) -> Result<Json<StorageSettings>, Error> {
async fn get_config(
State(state): State<StorageState<'_>>,
) -> Result<Json<Option<StorageSettings>>, Error> {
// StorageSettings is just a wrapper over serde_json::value::RawValue
let settings = state.client.get_config().await.map_err(Error::Service)?;
Ok(Json(settings))
Expand Down