Skip to content

Commit

Permalink
Fix agama config show before a product is selected (#2066)
Browse files Browse the repository at this point in the history
## Problem

- #2057 
- https://trello.com/c/I7rdVd7D

## Solution

~Return an empty JSON object (Ruby Hash) instead of null/nil~

Adapt the Rust client to deal with the `null` and omit the section.

## Testing

- Tested manually: 
  - `./testing-using-container.sh` and in its shell, `agama config show`
  - Web UI still works normally
- Added a unit test

## Debugging

[`git bisect`](https://git-scm.com/docs/git-bisect) saved me

## Screenshots

No
  • Loading branch information
mvidner authored Feb 25, 2025
2 parents fc6984a + e5cd13d commit 073e016
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 11 deletions.
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());
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

0 comments on commit 073e016

Please sign in to comment.