From a8e12a1a284d620e6dade531381745a17e7db2c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 30 Jul 2025 14:01:05 +0100 Subject: [PATCH 01/38] feat(rust): add a client ID to the JWT --- rust/Cargo.lock | 7 +++++-- rust/agama-lib/Cargo.toml | 1 + rust/agama-lib/src/auth.rs | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 2 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index bd02cd3525..39686dd44e 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -88,6 +88,7 @@ dependencies = [ "tokio-tungstenite 0.26.2", "url", "utoipa", + "uuid", "zbus", ] @@ -4694,12 +4695,14 @@ dependencies = [ [[package]] name = "uuid" -version = "1.16.0" +version = "1.17.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "458f7a779bf54acc9f347480ac654f68407d3aab21269a6e3c9f922acd9e2da9" +checksum = "3cf4199d1e5d15ddd86a694e4d0dffa9c323ce759fea589f00fef9d81cc1931d" dependencies = [ "getrandom 0.3.2", + "js-sys", "serde", + "wasm-bindgen", ] [[package]] diff --git a/rust/agama-lib/Cargo.toml b/rust/agama-lib/Cargo.toml index 76219dde95..86717c2b40 100644 --- a/rust/agama-lib/Cargo.toml +++ b/rust/agama-lib/Cargo.toml @@ -45,6 +45,7 @@ fluent-uri = { version = "0.3.2", features = ["serde"] } tokio-tungstenite = { version = "0.26.2", features = ["native-tls"] } tokio-native-tls = "0.3.1" percent-encoding = "2.3.1" +uuid = { version = "1.17.0", features = ["serde", "v4"] } [dev-dependencies] httpmock = "0.7.0" diff --git a/rust/agama-lib/src/auth.rs b/rust/agama-lib/src/auth.rs index 6cdab442e7..54c9f89348 100644 --- a/rust/agama-lib/src/auth.rs +++ b/rust/agama-lib/src/auth.rs @@ -56,6 +56,7 @@ use chrono::{Duration, Utc}; use jsonwebtoken::{DecodingKey, EncodingKey, Header, Validation}; use serde::{Deserialize, Serialize}; use thiserror::Error; +use uuid::Uuid; #[derive(Error, Debug)] #[error("Invalid authentication token: {0}")] @@ -195,8 +196,10 @@ impl Display for AuthToken { #[derive(Debug, Serialize, Deserialize)] pub struct TokenClaims { pub exp: i64, + pub client_id: ClientId, } +// FIXME: replace with TokenClaims::new, as it does not exist a "default" token. impl Default for TokenClaims { fn default() -> Self { let mut exp = Utc::now(); @@ -207,10 +210,21 @@ impl Default for TokenClaims { Self { exp: exp.timestamp(), + client_id: ClientId::new(), } } } +/// Identifies a client. +#[derive(Debug, Serialize, Deserialize)] +pub struct ClientId(Uuid); + +impl ClientId { + pub fn new() -> Self { + ClientId(Uuid::new_v4()) + } +} + #[cfg(test)] mod tests { use tempfile::tempdir; From 0a50b38065b7f76bf5027da752d7caee6aee63ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 30 Jul 2025 14:06:39 +0100 Subject: [PATCH 02/38] feat(rust): add the client ID to each request --- rust/agama-server/src/web/service.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/rust/agama-server/src/web/service.rs b/rust/agama-server/src/web/service.rs index 6e7160a925..a6fe10d0eb 100644 --- a/rust/agama-server/src/web/service.rs +++ b/rust/agama-server/src/web/service.rs @@ -22,6 +22,7 @@ use super::http::{login, login_from_query, logout, session}; use super::{config::ServiceConfig, state::ServiceState, EventsSender}; use agama_lib::auth::TokenClaims; use axum::http::HeaderValue; +use axum::middleware::Next; use axum::{ body::Body, extract::Request, @@ -31,6 +32,7 @@ use axum::{ Router, }; use hyper::header::CACHE_CONTROL; +use std::sync::Arc; use std::time::Duration; use std::{ convert::Infallible, @@ -107,8 +109,9 @@ impl MainServiceBuilder { let api_router = self .api_router - .route_layer(middleware::from_extractor_with_state::( + .route_layer(middleware::from_fn_with_state( state.clone(), + auth_middleware, )) .route("/ping", get(super::http::ping)) .route("/auth", post(login).get(session).delete(logout)); @@ -149,3 +152,13 @@ impl MainServiceBuilder { .with_state(state) } } + +// Authentication middleware. +// +// 1. Extracts the claims of the authentication token. +// 2. Adds the client ID as a extension to the request. +async fn auth_middleware(claims: TokenClaims, mut request: Request, next: Next) -> Response { + request.extensions_mut().insert(Arc::new(claims.client_id)); + let response = next.run(request).await; + response +} From 018478c457799bfb642ae4ac658f45e76deb256d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 30 Jul 2025 14:06:57 +0100 Subject: [PATCH 03/38] chore: log the client ID * Just for demonstration purposes. This commit should be reverted. --- rust/agama-server/src/storage/web.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rust/agama-server/src/storage/web.rs b/rust/agama-server/src/storage/web.rs index e6effdbb1c..7b5fa8e996 100644 --- a/rust/agama-server/src/storage/web.rs +++ b/rust/agama-server/src/storage/web.rs @@ -25,7 +25,10 @@ //! * `storage_service` which returns the Axum service. //! * `storage_stream` which offers an stream that emits the storage events coming from D-Bus. +use std::sync::Arc; + use agama_lib::{ + auth::ClientId, error::ServiceError, storage::{ model::{Action, Device, DeviceSid, ProposalSettings, ProposalSettingsPatch, Volume}, @@ -37,7 +40,7 @@ use anyhow::Context; use axum::{ extract::{Query, State}, routing::{get, post, put}, - Json, Router, + Extension, Json, Router, }; use iscsi::storage_iscsi_service; use serde::{Deserialize, Serialize}; @@ -218,7 +221,9 @@ async fn set_config( )] async fn get_config_model( State(state): State>, + Extension(client_id): Extension>, ) -> Result>, Error> { + tracing::debug!("{client_id:?}"); let config_model = state .client .get_config_model() From 959859801f58529865f1ae6d222b29b341d39334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 30 Jul 2025 16:11:56 +0100 Subject: [PATCH 04/38] feat(rust): emit a Connected event --- rust/agama-lib/src/auth.rs | 2 +- rust/agama-lib/src/http/event.rs | 6 ++++++ rust/agama-server/src/web/ws.rs | 17 +++++++++++++++-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/rust/agama-lib/src/auth.rs b/rust/agama-lib/src/auth.rs index 54c9f89348..49ceeff3ec 100644 --- a/rust/agama-lib/src/auth.rs +++ b/rust/agama-lib/src/auth.rs @@ -216,7 +216,7 @@ impl Default for TokenClaims { } /// Identifies a client. -#[derive(Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct ClientId(Uuid); impl ClientId { diff --git a/rust/agama-lib/src/http/event.rs b/rust/agama-lib/src/http/event.rs index 9fdabb8b1c..85f3a1749e 100644 --- a/rust/agama-lib/src/http/event.rs +++ b/rust/agama-lib/src/http/event.rs @@ -19,6 +19,7 @@ // find current contact information at www.suse.com. use crate::{ + auth::ClientId, jobs::Job, localization::model::LocaleConfig, manager::InstallationPhase, @@ -42,6 +43,11 @@ use crate::issue::Issue; #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(tag = "type")] pub enum Event { + ClientConnected { + // FIXME: use #[serde(rename_all = "camelCase")] + #[serde(rename = "clientId")] + client_id: ClientId, + }, L10nConfigChanged(LocaleConfig), LocaleChanged { locale: String, diff --git a/rust/agama-server/src/web/ws.rs b/rust/agama-server/src/web/ws.rs index 3f8a3a2c52..7bba41ac2c 100644 --- a/rust/agama-server/src/web/ws.rs +++ b/rust/agama-server/src/web/ws.rs @@ -20,24 +20,37 @@ //! Implements the websocket handling. +use std::sync::Arc; + use super::{state::ServiceState, EventsSender}; +use agama_lib::{auth::ClientId, http::Event}; use axum::{ extract::{ ws::{Message, WebSocket}, State, WebSocketUpgrade, }, response::IntoResponse, + Extension, }; pub async fn ws_handler( State(state): State, + Extension(client_id): Extension>, ws: WebSocketUpgrade, ) -> impl IntoResponse { - ws.on_upgrade(move |socket| handle_socket(socket, state.events)) + ws.on_upgrade(move |socket| handle_socket(socket, state.events, client_id)) } -async fn handle_socket(mut socket: WebSocket, events: EventsSender) { +async fn handle_socket(mut socket: WebSocket, events: EventsSender, client_id: Arc) { let mut rx = events.subscribe(); + + let conn_event = Event::ClientConnected { + client_id: client_id.as_ref().clone(), + }; + if let Ok(json) = serde_json::to_string(&conn_event) { + _ = socket.send(Message::Text(json)).await; + } + while let Ok(msg) = rx.recv().await { match serde_json::to_string(&msg) { Ok(json) => { From eb0ff35d4759173f3b32669e145dba09518adf6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Wed, 30 Jul 2025 17:47:38 +0100 Subject: [PATCH 05/38] feat(web): add infrastructure for displaying out-of-sync alert Introduce a reusable component that listens for change events on a given scope and displays a toast alert when the change comes from a different client. This helps notify users when the interface may be out of sync due to external changes, without forcing a reload or disrupting their workflow. Its usage is straightforward and consists of mounting one instance per watched scope in the desired pages. In future iterations, the component may support accepting multiple scopes if a real need for that arises. --- web/src/assets/styles/index.scss | 6 +- .../components/core/AlertOutOfSync.test.tsx | 136 ++++++++++++++++++ web/src/components/core/AlertOutOfSync.tsx | 115 +++++++++++++++ 3 files changed, 256 insertions(+), 1 deletion(-) create mode 100644 web/src/components/core/AlertOutOfSync.test.tsx create mode 100644 web/src/components/core/AlertOutOfSync.tsx diff --git a/web/src/assets/styles/index.scss b/web/src/assets/styles/index.scss index 8c1dc3f55e..29723f3686 100644 --- a/web/src/assets/styles/index.scss +++ b/web/src/assets/styles/index.scss @@ -339,7 +339,7 @@ strong { // --pf-v6-c-nav--PaddingBlockStart: 0; // } -.pf-v6-c-alert { +:not(.pf-v6-c-alert-group__item) > .pf-v6-c-alert { --pf-v6-c-alert--m-info__title--Color: var(--agm-t--color--waterhole); --pf-v6-c-alert__icon--FontSize: var(--pf-t--global--font--size--md); --pf-v6-c-content--MarginBlockEnd: var(--pf-t--global--spacer--xs); @@ -377,6 +377,10 @@ strong { } } +.pf-v6-c-alert-group.pf-m-toast { + padding: var(--pf-t--global--spacer--xl); +} + .pf-v6-c-alert__title { font-size: var(--pf-t--global--font--size--md); } diff --git a/web/src/components/core/AlertOutOfSync.test.tsx b/web/src/components/core/AlertOutOfSync.test.tsx new file mode 100644 index 0000000000..47687c4607 --- /dev/null +++ b/web/src/components/core/AlertOutOfSync.test.tsx @@ -0,0 +1,136 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React, { act } from "react"; +import { screen } from "@testing-library/dom"; +import { installerRender, plainRender } from "~/test-utils"; +import AlertOutOfSync from "./AlertOutOfSync"; + +const mockOnEvent = jest.fn(); + +const mockClient = { + id: "current-client", + isConnected: jest.fn().mockResolvedValue(true), + isRecoverable: jest.fn(), + onConnect: jest.fn(), + onClose: jest.fn(), + onError: jest.fn(), + onEvent: mockOnEvent, +}; + +let consoleErrorSpy: jest.SpyInstance; + +jest.mock("~/context/installer", () => ({ + ...jest.requireActual("~/context/installer"), + useInstallerClient: () => mockClient, +})); + +describe("AlertOutOfSync", () => { + beforeAll(() => { + consoleErrorSpy = jest.spyOn(console, "error"); + consoleErrorSpy.mockImplementation(); + }); + + it("renders nothing if scope is missing", () => { + // @ts-expect-error: scope is required prop + const { container } = plainRender(); + expect(container).toBeEmptyDOMElement(); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining("must receive a value for `scope`"), + ); + }); + + it("renders nothing if scope empty", () => { + const { container } = plainRender(); + expect(container).toBeEmptyDOMElement(); + expect(console.error).toHaveBeenCalledWith( + expect.stringContaining("must receive a value for `scope`"), + ); + }); + + it("shows alert on matching changes event from a different client for subscribed scope", () => { + let eventCallback; + mockClient.onEvent.mockImplementation((cb) => { + eventCallback = cb; + return () => {}; + }); + installerRender(); + + // Should not render the alert initially + expect(screen.queryByText("Info alert:")).toBeNull(); + + // Simulate a change event for a different scope + act(() => { + eventCallback({ type: "NotWatchedChanged", clientId: "other-client" }); + }); + + expect(screen.queryByText("Info alert:")).toBeNull(); + + // Simulate a change event for the subscribed scope, from current client + act(() => { + eventCallback({ type: "WatchedChanged", clientId: "current-client" }); + }); + + expect(screen.queryByText("Info alert:")).toBeNull(); + + // Simulate a change event for the subscribed scope, from different client + act(() => { + eventCallback({ type: "WatchedChanged", clientId: "other-client" }); + }); + + screen.getByText("Info alert:"); + screen.getByText("Configuration out of sync"); + screen.getByText(/issues or data loss/); + screen.getByRole("link", { name: "Reload now" }); + }); + + it("dismisses automatically the alert on matching changes event from current client for subscribed scope", () => { + let eventCallback; + mockClient.onEvent.mockImplementation((cb) => { + eventCallback = cb; + return () => {}; + }); + installerRender(); + + // Should not render the alert initially + expect(screen.queryByText("Info alert:")).toBeNull(); + + // Simulate a change event for the subscribed scope, from different client + act(() => { + eventCallback({ type: "WatchedChanged", clientId: "other-client" }); + }); + + screen.getByText("Info alert:"); + screen.getByText("Configuration out of sync"); + screen.getByText(/issues or data loss/); + screen.getByRole("link", { name: "Reload now" }); + + // Simulate a change event for the subscribed scope, from current client + act(() => { + eventCallback({ type: "WatchedChanged", clientId: "current-client" }); + }); + + expect(screen.queryByText("Info alert:")).toBeNull(); + }); + + it.todo("refresh page when clicking on `Reload now`"); +}); diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx new file mode 100644 index 0000000000..392954e3d0 --- /dev/null +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -0,0 +1,115 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React, { useEffect, useState } from "react"; +import { + Alert, + AlertActionCloseButton, + AlertGroup, + AlertProps, + Content, +} from "@patternfly/react-core"; +import { Link } from "react-router-dom"; +import Text from "./Text"; +import { useInstallerClient } from "~/context/installer"; +import { isEmpty } from "radashi"; +import { _ } from "~/i18n"; + +type AlertOutOfSyncProps = Partial & { + /** + * The scope to listen for change events on (e.g., `SoftwareProposal`, + * `L10nConfig`). + */ + scope: string; +}; + +/** + * Reactive alert shown when the configuration for a given scope has been + * changed externally. + * + * It warns that the interface may be out of sync and recommends reloading + * before continuing to avoid issues and data loss. Reloading is intentionally + * left up to the user rather than forced automatically, to prevent confusion + * caused by unexpected refreshes. + * + * It works by listening for "Changed" events on the specified scope: + * + * - Displays a toast alert if the event originates from a different client + * (based on client ID). + * - Automatically dismisses the alert if a subsequent event originates from + * the current client. + * + * @example + * ```tsx + * + * ``` + */ +export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncProps) { + const client = useInstallerClient(); + const [active, setActive] = useState(false); + const missingScope = isEmpty(scope); + + useEffect(() => { + if (missingScope) return; + + return client.onEvent((event) => { + event.type === `${scope}Changed` && setActive(event.clientId !== client.id); + }); + }); + + if (missingScope) { + console.error("AlertOutOfSync must receive a value for `scope` prop"); + return; + } + + const title = _("Configuration out of sync"); + + return ( + + {active && ( + setActive(false)} + /> + } + {...alertProps} + key={`${scope}-out-of-sync`} + > + + {_( + "The configuration has been updated externally. To avoid issues or data loss, \ +please reload the page to ensure you're working with the latest data.", + )} + + + {_("Reload now")} + + + )} + + ); +} From 10c69bc14e341b04e221f88c3d3eef8e28eb1e1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 30 Jul 2025 23:11:26 +0100 Subject: [PATCH 06/38] feat(rust): add the client ID to events --- rust/agama-lib/src/auth.rs | 2 +- rust/agama-lib/src/http.rs | 2 +- rust/agama-lib/src/http/event.rs | 101 ++++++++++++++++-- rust/agama-lib/src/monitor.rs | 12 ++- rust/agama-server/src/l10n/web.rs | 11 +- rust/agama-server/src/manager/web.rs | 4 +- rust/agama-server/src/network/web.rs | 5 +- rust/agama-server/src/questions/web.rs | 5 +- rust/agama-server/src/software/web.rs | 15 +-- rust/agama-server/src/storage/web.rs | 5 +- .../src/storage/web/dasd/stream.rs | 15 +-- rust/agama-server/src/storage/web/iscsi.rs | 3 +- .../src/storage/web/iscsi/stream.rs | 7 +- .../src/storage/web/zfcp/stream.rs | 21 ++-- rust/agama-server/src/users/web.rs | 7 +- rust/agama-server/src/web/common.rs | 6 +- rust/agama-server/src/web/common/issues.rs | 6 +- rust/agama-server/src/web/common/jobs.rs | 9 +- rust/agama-server/src/web/common/progress.rs | 5 +- rust/agama-server/src/web/ws.rs | 6 +- 20 files changed, 174 insertions(+), 73 deletions(-) diff --git a/rust/agama-lib/src/auth.rs b/rust/agama-lib/src/auth.rs index 49ceeff3ec..b3f403edaa 100644 --- a/rust/agama-lib/src/auth.rs +++ b/rust/agama-lib/src/auth.rs @@ -216,7 +216,7 @@ impl Default for TokenClaims { } /// Identifies a client. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct ClientId(Uuid); impl ClientId { diff --git a/rust/agama-lib/src/http.rs b/rust/agama-lib/src/http.rs index b8d31ccbd5..b4ea8cbb2b 100644 --- a/rust/agama-lib/src/http.rs +++ b/rust/agama-lib/src/http.rs @@ -22,7 +22,7 @@ mod base_http_client; pub use base_http_client::{BaseHTTPClient, BaseHTTPClientError}; mod event; -pub use event::Event; +pub use event::{Event, EventPayload}; mod websocket; pub use websocket::{WebSocketClient, WebSocketError}; diff --git a/rust/agama-lib/src/http/event.rs b/rust/agama-lib/src/http/event.rs index 85f3a1749e..8693641139 100644 --- a/rust/agama-lib/src/http/event.rs +++ b/rust/agama-lib/src/http/event.rs @@ -40,14 +40,47 @@ use std::collections::HashMap; use crate::issue::Issue; +/// Agama event. +/// +/// It represents an event that occurs in Agama. +#[derive(Clone, Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct Event { + /// The identifier of the client which caused the event. + #[serde(skip_serializing_if = "Option::is_none")] + pub client_id: Option, + /// Event payload. + #[serde(flatten)] + pub payload: EventPayload, +} + +impl Event { + /// Creates a new event. + /// + /// * `payload`: event payload. + pub fn new(payload: EventPayload) -> Self { + Event { + client_id: None, + payload, + } + } + + /// Creates a new event with a client ID. + /// + /// * `payload`: event payload. + /// * `client_id`: client ID. + pub fn new_with_client_id(payload: EventPayload, client_id: &ClientId) -> Self { + Event { + client_id: Some(client_id.clone()), + payload, + } + } +} + #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(tag = "type")] -pub enum Event { - ClientConnected { - // FIXME: use #[serde(rename_all = "camelCase")] - #[serde(rename = "clientId")] - client_id: ClientId, - }, +pub enum EventPayload { + ClientConnected, L10nConfigChanged(LocaleConfig), LocaleChanged { locale: String, @@ -150,3 +183,59 @@ pub enum Event { device: ZFCPController, }, } + +#[macro_export] +macro_rules! event { + ($variant:ident) => { + agama_lib::http::Event::new(agama_lib::http::EventPayload::$variant) + }; + ($variant:ident, $client:expr) => { + agama_lib::http::Event::new_with_client_id( + agama_lib::http::EventPayload::$variant, + $client, + ) + }; + ($variant:ident $inner:tt, $client:expr) => { + agama_lib::http::Event::new_with_client_id( + agama_lib::http::EventPayload::$variant $inner, + $client + ) + }; + ($variant:ident $inner:tt) => { + agama_lib::http::Event::new(agama_lib::http::EventPayload::$variant $inner) + }; +} + +#[cfg(test)] +mod test { + use crate as agama_lib; + use crate::{auth::ClientId, http::EventPayload}; + + #[test] + fn test_event_macro() { + let subject = event!(ClientConnected); + assert!(matches!(subject.payload, EventPayload::ClientConnected)); + assert!(subject.client_id.is_none()); + + let subject = event!(LocaleChanged { + locale: "es_ES".to_string() + }); + assert!(matches!( + subject.payload, + EventPayload::LocaleChanged { locale: _ } + )); + + let client_id = ClientId::new(); + let subject = event!(ClientConnected, &client_id); + assert_eq!(subject.client_id, Some(client_id)); + + let client_id = ClientId::new(); + let subject = event!( + LocaleChanged { + locale: "es_ES".to_string() + }, + &client_id + ); + assert_eq!(subject.client_id, Some(client_id)); + } +} diff --git a/rust/agama-lib/src/monitor.rs b/rust/agama-lib/src/monitor.rs index c56ecd7075..8a2d5d4369 100644 --- a/rust/agama-lib/src/monitor.rs +++ b/rust/agama-lib/src/monitor.rs @@ -55,7 +55,9 @@ use std::collections::HashMap; use tokio::sync::{broadcast, mpsc, oneshot}; use crate::{ - http::{BaseHTTPClient, BaseHTTPClientError, Event, WebSocketClient, WebSocketError}, + http::{ + BaseHTTPClient, BaseHTTPClientError, Event, EventPayload, WebSocketClient, WebSocketError, + }, manager::{InstallationPhase, InstallerStatus}, progress::Progress, }; @@ -223,16 +225,16 @@ impl Monitor { /// /// * `event`: Agama event. fn handle_event(&mut self, event: Event) { - match event { - Event::ProgressChanged { path, progress } => { + match event.payload { + EventPayload::ProgressChanged { path, progress } => { self.status.update_progress(path, progress); } - Event::ServiceStatusChanged { service, status } => { + EventPayload::ServiceStatusChanged { service, status } => { if service.as_str() == MANAGER_PROGRESS_OBJECT_PATH { self.status.set_is_busy(status == 1); } } - Event::InstallationPhaseChanged { phase } => { + EventPayload::InstallationPhaseChanged { phase } => { self.status.set_phase(phase); } _ => {} diff --git a/rust/agama-server/src/l10n/web.rs b/rust/agama-server/src/l10n/web.rs index a4e138e611..73f63b69ed 100644 --- a/rust/agama-server/src/l10n/web.rs +++ b/rust/agama-server/src/l10n/web.rs @@ -26,7 +26,9 @@ use super::{ }; use crate::{error::Error, web::EventsSender}; use agama_lib::{ - error::ServiceError, http::Event, localization::model::LocaleConfig, localization::LocaleProxy, + error::ServiceError, + event, + localization::{model::LocaleConfig, LocaleProxy}, proxies::LocaleMixinProxy as ManagerLocaleProxy, }; use agama_locale_data::LocaleId; @@ -160,10 +162,9 @@ async fn set_config( let locale_string = locale.to_string(); state.manager_proxy.set_locale(&locale_string).await?; changes.ui_locale = Some(locale_string); - - _ = state.events.send(Event::LocaleChanged { + _ = state.events.send(event!(LocaleChanged { locale: locale.to_string(), - }); + })); } if let Some(ui_keymap) = &value.ui_keymap { @@ -174,7 +175,7 @@ async fn set_config( if let Err(e) = update_dbus(&state.proxy, &changes).await { tracing::warn!("Could not synchronize settings in the localization D-Bus service: {e}"); } - _ = state.events.send(Event::L10nConfigChanged(changes)); + _ = state.events.send(event!(L10nConfigChanged(changes))); Ok(StatusCode::NO_CONTENT) } diff --git a/rust/agama-server/src/manager/web.rs b/rust/agama-server/src/manager/web.rs index ad37553a91..201352876b 100644 --- a/rust/agama-server/src/manager/web.rs +++ b/rust/agama-server/src/manager/web.rs @@ -27,7 +27,7 @@ use agama_lib::{ error::ServiceError, - logs, + event, logs, manager::{FinishMethod, InstallationPhase, InstallerStatus, ManagerClient}, proxies::Manager1Proxy, }; @@ -71,7 +71,7 @@ pub async fn manager_stream( .then(|change| async move { if let Ok(phase) = change.get().await { match InstallationPhase::try_from(phase) { - Ok(phase) => Some(Event::InstallationPhaseChanged { phase }), + Ok(phase) => Some(event!(InstallationPhaseChanged { phase })), Err(error) => { tracing::warn!("Ignoring the installation phase change. Error: {}", error); None diff --git a/rust/agama-server/src/network/web.rs b/rust/agama-server/src/network/web.rs index e6742c9b8c..37303ecd22 100644 --- a/rust/agama-server/src/network/web.rs +++ b/rust/agama-server/src/network/web.rs @@ -21,7 +21,6 @@ //! This module implements the web API for the network module. use crate::{error::Error, web::EventsSender}; -use agama_lib::http::Event; use anyhow::Context; use axum::{ extract::{Path, State}, @@ -34,6 +33,7 @@ use uuid::Uuid; use agama_lib::{ error::ServiceError, + event, network::{ error::NetworkStateError, model::{AccessPoint, Connection, Device, GeneralState}, @@ -100,7 +100,8 @@ pub async fn network_service( loop { match changes.recv().await { Ok(message) => { - if let Err(e) = events.send(Event::NetworkChange { change: message }) { + let change = event!(NetworkChange { change: message }); + if let Err(e) = events.send(change) { eprintln!("Could not send the event: {}", e); } } diff --git a/rust/agama-server/src/questions/web.rs b/rust/agama-server/src/questions/web.rs index 59e37f406c..777f155e0e 100644 --- a/rust/agama-server/src/questions/web.rs +++ b/rust/agama-server/src/questions/web.rs @@ -28,6 +28,7 @@ use crate::error::Error; use agama_lib::{ error::ServiceError, + event, http::Event, proxies::questions::{GenericQuestionProxy, QuestionWithPasswordProxy, QuestionsProxy}, questions::model::{Answer, GenericQuestion, PasswordAnswer, Question, QuestionWithPassword}, @@ -274,11 +275,11 @@ pub async fn questions_stream( let add_stream = proxy .receive_interfaces_added() .await? - .then(|_| async move { Event::QuestionsChanged }); + .then(|_| async move { event!(QuestionsChanged) }); let remove_stream = proxy .receive_interfaces_removed() .await? - .then(|_| async move { Event::QuestionsChanged }); + .then(|_| async move { event!(QuestionsChanged) }); let stream = StreamExt::merge(add_stream, remove_stream); Ok(Box::pin(stream)) } diff --git a/rust/agama-server/src/software/web.rs b/rust/agama-server/src/software/web.rs index 9c8eb0258c..03273e45e3 100644 --- a/rust/agama-server/src/software/web.rs +++ b/rust/agama-server/src/software/web.rs @@ -38,7 +38,8 @@ use crate::{ use agama_lib::{ error::ServiceError, - http::Event, + event, + http::{Event, EventPayload}, product::{proxies::RegistrationProxy, Product, ProductClient}, software::{ model::{ @@ -116,7 +117,7 @@ async fn product_changed_stream( .await .then(|change| async move { if let Ok(id) = change.get().await { - return Some(Event::ProductChanged { id }); + return Some(event!(ProductChanged { id })); } None }) @@ -143,7 +144,7 @@ async fn patterns_changed_stream( } None }) - .filter_map(|e| e.map(|patterns| Event::SoftwareProposalChanged { patterns })); + .filter_map(|e| e.map(|patterns| event!(SoftwareProposalChanged { patterns }))); Ok(stream) } @@ -165,7 +166,7 @@ async fn conflicts_changed_stream( } None }) - .filter_map(|e| e.map(|conflicts| Event::ConflictsChanged { conflicts })); + .filter_map(|e| e.map(|conflicts| event!(ConflictsChanged { conflicts }))); Ok(stream) } @@ -179,7 +180,7 @@ async fn registration_email_changed_stream( .then(|change| async move { if let Ok(_id) = change.get().await { // TODO: add to stream also proxy and return whole cached registration info - return Some(Event::RegistrationChanged); + return Some(event!(RegistrationChanged)); } None }) @@ -196,7 +197,7 @@ async fn registration_code_changed_stream( .await .then(|change| async move { if let Ok(_id) = change.get().await { - return Some(Event::RegistrationChanged); + return Some(event!(RegistrationChanged)); } None }) @@ -228,7 +229,7 @@ pub async fn receive_events( client: ProductClient<'_>, ) { while let Ok(event) = events.recv().await { - if let Event::LocaleChanged { locale: _ } = event { + if let EventPayload::LocaleChanged { locale: _ } = event.payload { let mut cached_products = products.write().await; if let Ok(products) = client.products().await { *cached_products = products; diff --git a/rust/agama-server/src/storage/web.rs b/rust/agama-server/src/storage/web.rs index 7b5fa8e996..b08807d04f 100644 --- a/rust/agama-server/src/storage/web.rs +++ b/rust/agama-server/src/storage/web.rs @@ -30,6 +30,8 @@ use std::sync::Arc; use agama_lib::{ auth::ClientId, error::ServiceError, + event, + http::Event, storage::{ model::{Action, Device, DeviceSid, ProposalSettings, ProposalSettingsPatch, Volume}, proxies::Storage1Proxy, @@ -63,7 +65,6 @@ use crate::{ ProgressClient, ProgressRouterBuilder, }, }; -use agama_lib::http::Event; pub async fn storage_streams(dbus: zbus::Connection) -> Result { let mut result: EventStreams = vec![( @@ -87,7 +88,7 @@ async fn devices_dirty_stream(dbus: zbus::Connection) -> Result { let device = Self::update_device(cache, path, values)?; - Ok(Event::DASDDeviceAdded { + Ok(event!(DASDDeviceAdded { device: device.clone(), - }) + })) } DBusObjectChange::Changed(path, updated) => { let device = Self::update_device(cache, path, updated)?; - Ok(Event::DASDDeviceChanged { + Ok(event!(DASDDeviceChanged { device: device.clone(), - }) + })) } DBusObjectChange::Removed(path) => { let device = Self::remove_device(cache, path)?; - Ok(Event::DASDDeviceRemoved { device }) + Ok(event!(DASDDeviceRemoved { device })) } } } @@ -251,10 +252,10 @@ impl DASDFormatJobStream { ); } - Some(Event::DASDFormatJobChanged { + Some(event!(DASDFormatJobChanged { job_id: path.to_string(), summary: format_summary, - }) + })) } } diff --git a/rust/agama-server/src/storage/web/iscsi.rs b/rust/agama-server/src/storage/web/iscsi.rs index d9837f37fb..216d383c71 100644 --- a/rust/agama-server/src/storage/web/iscsi.rs +++ b/rust/agama-server/src/storage/web/iscsi.rs @@ -31,6 +31,7 @@ use crate::{ }; use agama_lib::{ error::ServiceError, + event, http::Event, storage::{ client::iscsi::{ISCSIAuth, ISCSIInitiator, ISCSINode, LoginResult}, @@ -104,7 +105,7 @@ fn handle_initiator_change(change: PropertiesChanged) -> Result, S let changes = to_owned_hash(args.changed_properties())?; let name = get_optional_property(&changes, "InitiatorName")?; let ibft = get_optional_property(&changes, "IBFT")?; - Ok(Some(Event::ISCSIInitiatorChanged { ibft, name })) + Ok(Some(event!(ISCSIInitiatorChanged { ibft, name }))) } #[derive(Clone)] diff --git a/rust/agama-server/src/storage/web/iscsi/stream.rs b/rust/agama-server/src/storage/web/iscsi/stream.rs index 3de2e94a7a..6749a5a4da 100644 --- a/rust/agama-server/src/storage/web/iscsi/stream.rs +++ b/rust/agama-server/src/storage/web/iscsi/stream.rs @@ -22,6 +22,7 @@ use std::{collections::HashMap, task::Poll}; use agama_lib::{ error::ServiceError, + event, http::Event, storage::{ISCSIClient, ISCSINode}, }; @@ -131,15 +132,15 @@ impl ISCSINodeStream { match change { DBusObjectChange::Added(path, values) => { let node = Self::update_node(cache, path, values)?; - Ok(Event::ISCSINodeAdded { node: node.clone() }) + Ok(event!(ISCSINodeAdded { node: node.clone() })) } DBusObjectChange::Changed(path, updated) => { let node = Self::update_node(cache, path, updated)?; - Ok(Event::ISCSINodeChanged { node: node.clone() }) + Ok(event!(ISCSINodeChanged { node: node.clone() })) } DBusObjectChange::Removed(path) => { let node = Self::remove_node(cache, path)?; - Ok(Event::ISCSINodeRemoved { node }) + Ok(event!(ISCSINodeRemoved { node })) } } } diff --git a/rust/agama-server/src/storage/web/zfcp/stream.rs b/rust/agama-server/src/storage/web/zfcp/stream.rs index f58be37d73..99a46669b9 100644 --- a/rust/agama-server/src/storage/web/zfcp/stream.rs +++ b/rust/agama-server/src/storage/web/zfcp/stream.rs @@ -24,6 +24,7 @@ use std::{collections::HashMap, task::Poll}; use agama_lib::{ error::ServiceError, + event, http::Event, storage::{ client::zfcp::ZFCPClient, @@ -127,19 +128,19 @@ impl ZFCPDiskStream { match change { DBusObjectChange::Added(path, values) => { let device = Self::update_device(cache, path, values)?; - Ok(Event::ZFCPDiskAdded { + Ok(event!(ZFCPDiskAdded { device: device.clone(), - }) + })) } DBusObjectChange::Changed(path, updated) => { let device = Self::update_device(cache, path, updated)?; - Ok(Event::ZFCPDiskChanged { + Ok(event!(ZFCPDiskChanged { device: device.clone(), - }) + })) } DBusObjectChange::Removed(path) => { let device = Self::remove_device(cache, path)?; - Ok(Event::ZFCPDiskRemoved { device }) + Ok(event!(ZFCPDiskRemoved { device })) } } } @@ -260,19 +261,19 @@ impl ZFCPControllerStream { match change { DBusObjectChange::Added(path, values) => { let device = Self::update_device(cache, path, values)?; - Ok(Event::ZFCPControllerAdded { + Ok(event!(ZFCPControllerAdded { device: device.clone(), - }) + })) } DBusObjectChange::Changed(path, updated) => { let device = Self::update_device(cache, path, updated)?; - Ok(Event::ZFCPControllerChanged { + Ok(event!(ZFCPControllerChanged { device: device.clone(), - }) + })) } DBusObjectChange::Removed(path) => { let device = Self::remove_device(cache, path)?; - Ok(Event::ZFCPControllerRemoved { device }) + Ok(event!(ZFCPControllerRemoved { device })) } } } diff --git a/rust/agama-server/src/users/web.rs b/rust/agama-server/src/users/web.rs index 293f540a19..30cc24d3b7 100644 --- a/rust/agama-server/src/users/web.rs +++ b/rust/agama-server/src/users/web.rs @@ -31,6 +31,7 @@ use crate::{ }; use agama_lib::{ error::ServiceError, + event, http::Event, users::{model::RootPatchSettings, proxies::Users1Proxy, FirstUser, RootUser, UsersClient}, }; @@ -54,7 +55,7 @@ struct UsersState<'a> { /// Returns streams that emits users related events coming from D-Bus. /// -/// It emits the Event::RootPasswordChange, Event::RootSSHKeyChanged and Event::FirstUserChanged events. +/// It emits the RootPasswordChange, RootSSHKeyChanged and FirstUserChanged events. /// /// * `connection`: D-Bus connection to listen for events. pub async fn users_streams(dbus: zbus::Connection) -> Result { @@ -89,7 +90,7 @@ async fn first_user_changed_stream( password: user.2, hashed_password: user.3, }; - return Some(Event::FirstUserChanged(user_struct)); + return Some(event!(FirstUserChanged(user_struct))); } None }) @@ -107,7 +108,7 @@ async fn root_user_changed_stream( .then(|change| async move { if let Ok(user) = change.get().await { if let Ok(root) = RootUser::from_dbus(user) { - return Some(Event::RootUserChanged(root)); + return Some(event!(RootUserChanged(root))); } } None diff --git a/rust/agama-server/src/web/common.rs b/rust/agama-server/src/web/common.rs index 2a28120dde..0820cd884f 100644 --- a/rust/agama-server/src/web/common.rs +++ b/rust/agama-server/src/web/common.rs @@ -22,7 +22,7 @@ use std::pin::Pin; -use agama_lib::{error::ServiceError, proxies::ServiceStatusProxy}; +use agama_lib::{error::ServiceError, event, proxies::ServiceStatusProxy}; use axum::{extract::State, routing::get, Json, Router}; use serde::Serialize; use tokio_stream::{Stream, StreamExt}; @@ -115,10 +115,10 @@ pub async fn service_status_stream( .await .then(move |change| async move { if let Ok(status) = change.get().await { - Some(Event::ServiceStatusChanged { + Some(event!(ServiceStatusChanged { service: destination.to_string(), status, - }) + })) } else { None } diff --git a/rust/agama-server/src/web/common/issues.rs b/rust/agama-server/src/web/common/issues.rs index 5e9887805f..a9cf8c10ac 100644 --- a/rust/agama-server/src/web/common/issues.rs +++ b/rust/agama-server/src/web/common/issues.rs @@ -36,7 +36,7 @@ //! At this point, it only handles the issues that are exposed through D-Bus. use crate::web::EventsSender; -use agama_lib::{http::Event, issue::Issue}; +use agama_lib::{event, http::Event, issue::Issue}; use agama_utils::dbus::build_properties_changed_stream; use axum::{extract::State, routing::get, Json, Router}; use std::collections::HashMap; @@ -172,10 +172,10 @@ impl IssuesService { self.cache.insert(path.to_string(), issues.clone()); - let event = Event::IssuesChanged { + let event = event!(IssuesChanged { path: path.to_string(), issues, - }; + }); self.events.send(event)?; Ok(()) } diff --git a/rust/agama-server/src/web/common/jobs.rs b/rust/agama-server/src/web/common/jobs.rs index 9e13333029..0f67483deb 100644 --- a/rust/agama-server/src/web/common/jobs.rs +++ b/rust/agama-server/src/web/common/jobs.rs @@ -22,6 +22,8 @@ use std::{collections::HashMap, pin::Pin, task::Poll}; use agama_lib::{ error::ServiceError, + event, + http::Event, jobs::{client::JobsClient, Job}, }; use agama_utils::{dbus::get_optional_property, property_from_dbus}; @@ -35,7 +37,6 @@ use zbus::zvariant::{ObjectPath, OwnedObjectPath, OwnedValue}; use crate::{ dbus::{DBusObjectChange, DBusObjectChangesStream, ObjectsCache}, error::Error, - web::Event, }; /// Builds a router for the jobs objects. @@ -162,15 +163,15 @@ impl JobsStream { match change { DBusObjectChange::Added(path, values) => { let job = Self::update_job(cache, path, values)?; - Ok(Event::JobAdded { job: job.clone() }) + Ok(event!(JobAdded { job: job.clone() })) } DBusObjectChange::Changed(path, updated) => { let job = Self::update_job(cache, path, updated)?; - Ok(Event::JobChanged { job: job.clone() }) + Ok(event!(JobChanged { job: job.clone() })) } DBusObjectChange::Removed(path) => { let job = Self::remove_job(cache, path)?; - Ok(Event::JobRemoved { job }) + Ok(event!(JobRemoved { job })) } } } diff --git a/rust/agama-server/src/web/common/progress.rs b/rust/agama-server/src/web/common/progress.rs index 7b123c2456..5a4056cea5 100644 --- a/rust/agama-server/src/web/common/progress.rs +++ b/rust/agama-server/src/web/common/progress.rs @@ -37,6 +37,7 @@ use crate::web::{event::log_event, EventsSender}; use agama_lib::{ + event, http::Event, progress::{Progress, ProgressSequence}, proxies::{ProgressChanged, ProgressProxy}, @@ -170,10 +171,10 @@ impl ProgressService { }; self.cache.insert(path.to_string(), sequence.clone()); - let event = Event::ProgressChanged { + let event = event!(ProgressChanged { path: path.to_string(), progress, - }; + }); log_event(&event); self.events.send(event)?; Ok(()) diff --git a/rust/agama-server/src/web/ws.rs b/rust/agama-server/src/web/ws.rs index 7bba41ac2c..c42bc0caff 100644 --- a/rust/agama-server/src/web/ws.rs +++ b/rust/agama-server/src/web/ws.rs @@ -23,7 +23,7 @@ use std::sync::Arc; use super::{state::ServiceState, EventsSender}; -use agama_lib::{auth::ClientId, http::Event}; +use agama_lib::auth::ClientId; use axum::{ extract::{ ws::{Message, WebSocket}, @@ -44,9 +44,7 @@ pub async fn ws_handler( async fn handle_socket(mut socket: WebSocket, events: EventsSender, client_id: Arc) { let mut rx = events.subscribe(); - let conn_event = Event::ClientConnected { - client_id: client_id.as_ref().clone(), - }; + let conn_event = agama_lib::event!(ClientConnected, client_id.as_ref()); if let Ok(json) = serde_json::to_string(&conn_event) { _ = socket.send(Message::Text(json)).await; } From 199cef901f007d6a2719d08f3f9a73e81563a440 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Wed, 30 Jul 2025 23:35:38 +0100 Subject: [PATCH 07/38] fix(rust): add clientId to the L10nConfigChanged event --- rust/agama-server/src/l10n/web.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rust/agama-server/src/l10n/web.rs b/rust/agama-server/src/l10n/web.rs index 73f63b69ed..ed87be2c22 100644 --- a/rust/agama-server/src/l10n/web.rs +++ b/rust/agama-server/src/l10n/web.rs @@ -26,6 +26,7 @@ use super::{ }; use crate::{error::Error, web::EventsSender}; use agama_lib::{ + auth::ClientId, error::ServiceError, event, localization::{model::LocaleConfig, LocaleProxy}, @@ -37,7 +38,7 @@ use axum::{ http::StatusCode, response::IntoResponse, routing::{get, patch}, - Json, Router, + Extension, Json, Router, }; use std::sync::Arc; use tokio::sync::RwLock; @@ -132,6 +133,7 @@ async fn keymaps(State(state): State>) -> Json> { )] async fn set_config( State(state): State>, + Extension(client_id): Extension>, Json(value): Json, ) -> Result { let mut data = state.locale.write().await; @@ -175,7 +177,9 @@ async fn set_config( if let Err(e) = update_dbus(&state.proxy, &changes).await { tracing::warn!("Could not synchronize settings in the localization D-Bus service: {e}"); } - _ = state.events.send(event!(L10nConfigChanged(changes))); + _ = state + .events + .send(event!(L10nConfigChanged(changes), client_id.as_ref())); Ok(StatusCode::NO_CONTENT) } From bf2faa2aaa8fdea0f31b4af4e64a85b7eeae1624 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 08:38:25 +0100 Subject: [PATCH 08/38] fix(web): simplify out of sync alert text By making sentence shorter and not using "Please". --- web/src/components/core/AlertOutOfSync.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx index 392954e3d0..31962ff87f 100644 --- a/web/src/components/core/AlertOutOfSync.tsx +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -101,8 +101,8 @@ export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncP > {_( - "The configuration has been updated externally. To avoid issues or data loss, \ -please reload the page to ensure you're working with the latest data.", + "The configuration has been updated externally. \ +Reload the page to get the latest data and avoid issues or data loss.", )} From 847ad24ec044f16ef02de85e9f7355f7a452f05e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 10:04:29 +0100 Subject: [PATCH 09/38] feat(web): make client aware of its ID Listen for the event that provides the ID assigned by the server and store it internally. This allows the client to use its ID when needed, for example, to determine when to show an out-of-sync alert triggered by events from other clients. --- web/src/client/index.ts | 2 ++ web/src/context/installer.tsx | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/web/src/client/index.ts b/web/src/client/index.ts index 81c33e12db..c881f3d887 100644 --- a/web/src/client/index.ts +++ b/web/src/client/index.ts @@ -26,6 +26,8 @@ type VoidFn = () => void; type BooleanFn = () => boolean; export type InstallerClient = { + /** Unique client identifier. */ + id?: string; /** Whether the client is connected. */ isConnected: BooleanFn; /** Whether the client is recoverable after disconnecting. */ diff --git a/web/src/context/installer.tsx b/web/src/context/installer.tsx index 4a2f8c650f..799419ef79 100644 --- a/web/src/context/installer.tsx +++ b/web/src/context/installer.tsx @@ -53,6 +53,13 @@ function InstallerClientProvider({ children, client = null }: InstallerClientPro useEffect(() => { const connectClient = async () => { const client = await createDefaultClient(); + + client.onEvent((event) => { + if (event.type === "ClientConnected") { + client.id = event.clientId; + } + }); + setValue(client); }; From e147a34aa72ff19e24e7e13d81451a63cc1e6828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 31 Jul 2025 10:15:23 +0100 Subject: [PATCH 10/38] refactor(rust): move event! macro tests to doctests --- rust/agama-lib/src/http/event.rs | 78 ++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 34 deletions(-) diff --git a/rust/agama-lib/src/http/event.rs b/rust/agama-lib/src/http/event.rs index 8693641139..1072a4424e 100644 --- a/rust/agama-lib/src/http/event.rs +++ b/rust/agama-lib/src/http/event.rs @@ -184,6 +184,50 @@ pub enum EventPayload { }, } +/// Makes it easier to create an event, reducing the boilerplate. +/// +/// # Event without additional data +/// +/// ``` +/// # use agama_lib::{event, http::EventPayload}; +/// let my_event = event!(ClientConnected); +/// assert!(matches!(my_event.payload, EventPayload::ClientConnected)); +/// assert!(my_event.client_id.is_none()); +/// ``` +/// +/// # Event with some additional data +/// +/// ``` +/// # use agama_lib::{event, http::EventPayload}; +/// let my_event = event!(LocaleChanged { locale: "es_ES".to_string() }); +/// assert!(matches!( +/// my_event.payload, +/// EventPayload::LocaleChanged { locale: _ } +/// )); +/// ``` +/// +/// # Adding the client ID +/// +/// ``` +/// # use agama_lib::{auth::ClientId, event, http::EventPayload}; +/// let client_id = ClientId::new(); +/// let my_event = event!(ClientConnected, &client_id); +/// assert!(matches!(my_event.payload, EventPayload::ClientConnected)); +/// assert!(my_event.client_id.is_some()); +/// ``` +/// +/// # Add the client ID to a complex event +/// +/// ``` +/// # use agama_lib::{auth::ClientId, event, http::EventPayload}; +/// let client_id = ClientId::new(); +/// let my_event = event!(LocaleChanged { locale: "es_ES".to_string() }, &client_id); +/// assert!(matches!( +/// my_event.payload, +/// EventPayload::LocaleChanged { locale: _ } +/// )); +/// assert!(my_event.client_id.is_some()); +/// ``` #[macro_export] macro_rules! event { ($variant:ident) => { @@ -205,37 +249,3 @@ macro_rules! event { agama_lib::http::Event::new(agama_lib::http::EventPayload::$variant $inner) }; } - -#[cfg(test)] -mod test { - use crate as agama_lib; - use crate::{auth::ClientId, http::EventPayload}; - - #[test] - fn test_event_macro() { - let subject = event!(ClientConnected); - assert!(matches!(subject.payload, EventPayload::ClientConnected)); - assert!(subject.client_id.is_none()); - - let subject = event!(LocaleChanged { - locale: "es_ES".to_string() - }); - assert!(matches!( - subject.payload, - EventPayload::LocaleChanged { locale: _ } - )); - - let client_id = ClientId::new(); - let subject = event!(ClientConnected, &client_id); - assert_eq!(subject.client_id, Some(client_id)); - - let client_id = ClientId::new(); - let subject = event!( - LocaleChanged { - locale: "es_ES".to_string() - }, - &client_id - ); - assert_eq!(subject.client_id, Some(client_id)); - } -} From eec9d8ad2ff85de8e2cef64dd0335eb7115a23b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 10:09:22 +0100 Subject: [PATCH 11/38] feat(web): enable use of `replace` prop in core/Link Allow using the `replace` prop in core/Link to support replacing the current history entry during navigation. This enables using core/Link instead of react-router-dom/Link to maintain consistent look and feel across the app, as core/Link is built on top of PF/Button. --- web/src/components/core/AlertOutOfSync.tsx | 4 ++-- web/src/components/core/Link.tsx | 26 +++++++++++++++++++--- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx index 31962ff87f..ea9c62b321 100644 --- a/web/src/components/core/AlertOutOfSync.tsx +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -28,7 +28,7 @@ import { AlertProps, Content, } from "@patternfly/react-core"; -import { Link } from "react-router-dom"; +import Link from "./Link"; import Text from "./Text"; import { useInstallerClient } from "~/context/installer"; import { isEmpty } from "radashi"; @@ -105,7 +105,7 @@ export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncP Reload the page to get the latest data and avoid issues or data loss.", )} - + {_("Reload now")} diff --git a/web/src/components/core/Link.tsx b/web/src/components/core/Link.tsx index c5816fcfa1..d05401db7b 100644 --- a/web/src/components/core/Link.tsx +++ b/web/src/components/core/Link.tsx @@ -22,11 +22,13 @@ import React from "react"; import { Button, ButtonProps } from "@patternfly/react-core"; -import { To, useHref } from "react-router-dom"; +import { To, useHref, useLinkClickHandler } from "react-router-dom"; export type LinkProps = Omit & { /** The target route */ to: string | To; + /** Whether the link should replace the current entry in the browser history */ + replace?: boolean; /** Whether use PF/Button primary variant */ isPrimary?: boolean; }; @@ -37,11 +39,29 @@ export type LinkProps = Omit & { * @note when isPrimary not given or false and props does not contain a variant prop, * it will default to "secondary" variant */ -export default function Link({ to, isPrimary, variant, children, ...props }: LinkProps) { +export default function Link({ + to, + replace = false, + isPrimary, + variant, + children, + onClick, + ...props +}: LinkProps) { const href = useHref(to); const linkVariant = isPrimary ? "primary" : variant || "secondary"; + const handleClick = useLinkClickHandler(to, { replace }); return ( - ); From 062686956985365a77ed3dce7a624a6350fb8b4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 10:16:57 +0100 Subject: [PATCH 12/38] doc(web): add entry to changes file --- web/package/agama-web-ui.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index 5418693509..98eaa8b0f8 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Thu Jul 31 09:13:09 UTC 2025 - David Diaz + +- Allow displaying out-of-sync alerts by listening to changes + events (gh#agama-project/agama#2630). + ------------------------------------------------------------------- Mon Jul 21 15:07:40 UTC 2025 - Imobach Gonzalez Sosa From 88839db660345923d43ddc118db88aa13300d1bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 10:27:57 +0100 Subject: [PATCH 13/38] fix(web): add missing mock To avoid real execution of react-router-dom/useLinkClickHandler, now used at core/Link --- web/src/test-utils.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web/src/test-utils.tsx b/web/src/test-utils.tsx index c74603f33f..bd4fb144e3 100644 --- a/web/src/test-utils.tsx +++ b/web/src/test-utils.tsx @@ -96,6 +96,11 @@ jest.mock("react-router-dom", () => ({ Navigate: ({ to: route }) => <>Navigating to {route}, Outlet: () => <>Outlet Content, useRevalidator: () => mockUseRevalidator, + useLinkClickHandler: + ({ to }) => + () => { + to; + }, })); const Providers = ({ children, withL10n }) => { From 84743d83024b55aea9cb908c831c4aa6fd5aace0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 11:06:27 +0100 Subject: [PATCH 14/38] fix(web): force a reload via JavaScript By using the locationReload util instead of just "navigating to the same place", since this does not work for storage. --- .../components/core/AlertOutOfSync.test.tsx | 30 +++++++++++++++++-- web/src/components/core/AlertOutOfSync.tsx | 10 +++---- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/web/src/components/core/AlertOutOfSync.test.tsx b/web/src/components/core/AlertOutOfSync.test.tsx index 47687c4607..1eaf6166db 100644 --- a/web/src/components/core/AlertOutOfSync.test.tsx +++ b/web/src/components/core/AlertOutOfSync.test.tsx @@ -26,6 +26,7 @@ import { installerRender, plainRender } from "~/test-utils"; import AlertOutOfSync from "./AlertOutOfSync"; const mockOnEvent = jest.fn(); +const mockReload = jest.fn(); const mockClient = { id: "current-client", @@ -43,6 +44,10 @@ jest.mock("~/context/installer", () => ({ ...jest.requireActual("~/context/installer"), useInstallerClient: () => mockClient, })); +jest.mock("~/utils", () => ({ + ...jest.requireActual("~/utils"), + locationReload: () => mockReload(), +})); describe("AlertOutOfSync", () => { beforeAll(() => { @@ -100,7 +105,7 @@ describe("AlertOutOfSync", () => { screen.getByText("Info alert:"); screen.getByText("Configuration out of sync"); screen.getByText(/issues or data loss/); - screen.getByRole("link", { name: "Reload now" }); + screen.getByRole("button", { name: "Reload now" }); }); it("dismisses automatically the alert on matching changes event from current client for subscribed scope", () => { @@ -122,7 +127,7 @@ describe("AlertOutOfSync", () => { screen.getByText("Info alert:"); screen.getByText("Configuration out of sync"); screen.getByText(/issues or data loss/); - screen.getByRole("link", { name: "Reload now" }); + screen.getByRole("button", { name: "Reload now" }); // Simulate a change event for the subscribed scope, from current client act(() => { @@ -132,5 +137,24 @@ describe("AlertOutOfSync", () => { expect(screen.queryByText("Info alert:")).toBeNull(); }); - it.todo("refresh page when clicking on `Reload now`"); + it("trigger a location relaod when clicking on `Reload now`", async () => { + let eventCallback; + mockClient.onEvent.mockImplementation((cb) => { + eventCallback = cb; + return () => {}; + }); + const { user } = installerRender(); + + // Should not render the alert initially + expect(screen.queryByText("Info alert:")).toBeNull(); + + // Simulate a change event for the subscribed scope, from different client + act(() => { + eventCallback({ type: "WatchedChanged", clientId: "other-client" }); + }); + + const reloadButton = screen.getByRole("button", { name: "Reload now" }); + await user.click(reloadButton); + expect(mockReload).toHaveBeenCalled(); + }); }); diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx index ea9c62b321..773bed87c2 100644 --- a/web/src/components/core/AlertOutOfSync.tsx +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -26,13 +26,13 @@ import { AlertActionCloseButton, AlertGroup, AlertProps, + Button, Content, } from "@patternfly/react-core"; -import Link from "./Link"; -import Text from "./Text"; import { useInstallerClient } from "~/context/installer"; import { isEmpty } from "radashi"; import { _ } from "~/i18n"; +import { locationReload } from "~/utils"; type AlertOutOfSyncProps = Partial & { /** @@ -105,9 +105,9 @@ export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncP Reload the page to get the latest data and avoid issues or data loss.", )} - - {_("Reload now")} - + )} From 9e72bb54388226004fe30741028b151b3f20baa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 11:42:36 +0100 Subject: [PATCH 15/38] fix(web): minor adjustments in test file --- web/src/components/core/AlertOutOfSync.test.tsx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/web/src/components/core/AlertOutOfSync.test.tsx b/web/src/components/core/AlertOutOfSync.test.tsx index 1eaf6166db..9e2f76b1aa 100644 --- a/web/src/components/core/AlertOutOfSync.test.tsx +++ b/web/src/components/core/AlertOutOfSync.test.tsx @@ -78,6 +78,7 @@ describe("AlertOutOfSync", () => { eventCallback = cb; return () => {}; }); + installerRender(); // Should not render the alert initially @@ -114,10 +115,8 @@ describe("AlertOutOfSync", () => { eventCallback = cb; return () => {}; }); - installerRender(); - // Should not render the alert initially - expect(screen.queryByText("Info alert:")).toBeNull(); + installerRender(); // Simulate a change event for the subscribed scope, from different client act(() => { @@ -137,16 +136,14 @@ describe("AlertOutOfSync", () => { expect(screen.queryByText("Info alert:")).toBeNull(); }); - it("trigger a location relaod when clicking on `Reload now`", async () => { + it("triggers a location relaod when clicking on `Reload now`", async () => { let eventCallback; mockClient.onEvent.mockImplementation((cb) => { eventCallback = cb; return () => {}; }); - const { user } = installerRender(); - // Should not render the alert initially - expect(screen.queryByText("Info alert:")).toBeNull(); + const { user } = installerRender(); // Simulate a change event for the subscribed scope, from different client act(() => { From 9fb0828b5a6d12c5e9360c4f54f8443e9a859bf5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 30 Jul 2025 15:20:59 +0100 Subject: [PATCH 16/38] fix(storage): do not generate json for default size --- .../config_conversions/to_json_conversions/size.rb | 9 ++++++++- .../config_conversions/to_json_conversions/examples.rb | 9 ++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/service/lib/agama/storage/config_conversions/to_json_conversions/size.rb b/service/lib/agama/storage/config_conversions/to_json_conversions/size.rb index f43d1c44b6..2a266f782c 100644 --- a/service/lib/agama/storage/config_conversions/to_json_conversions/size.rb +++ b/service/lib/agama/storage/config_conversions/to_json_conversions/size.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2024] SUSE LLC +# Copyright (c) [2024-2025] SUSE LLC # # All Rights Reserved. # @@ -33,6 +33,13 @@ def initialize(config) @config = config end + # The size is not generated for default size. + # + # @see Base#convert + def convert + super unless config.default? + end + private # @see Base#conversions diff --git a/service/test/agama/storage/config_conversions/to_json_conversions/examples.rb b/service/test/agama/storage/config_conversions/to_json_conversions/examples.rb index 7d1613a265..c02eee9116 100644 --- a/service/test/agama/storage/config_conversions/to_json_conversions/examples.rb +++ b/service/test/agama/storage/config_conversions/to_json_conversions/examples.rb @@ -386,7 +386,7 @@ end end - context "if size was solved" do + context "if size is default" do before do size_config = config.size size_config.default = true @@ -396,12 +396,7 @@ it "generates the expected JSON" do config_json = subject.convert - expect(config_json[:size]).to eq( - { - min: 5.GiB.to_i, - max: 25.GiB.to_i - } - ) + expect(config_json.keys).to_not include(:size) end end end From 36846c5e7f459bb578e2274036f5500ff78a7f26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 30 Jul 2025 15:22:17 +0100 Subject: [PATCH 17/38] storage: avoid unnecessary signals/events - For example, when there are several attempts to calculate a valid proposal. --- service/lib/agama/dbus/storage/manager.rb | 65 ++++++++------- service/lib/agama/storage/manager.rb | 34 +++++--- service/lib/agama/storage/proposal.rb | 19 ----- .../test/agama/dbus/storage/manager_test.rb | 4 +- .../agama/storage/autoyast_proposal_test.rb | 39 --------- service/test/agama/storage/manager_test.rb | 80 +++++++++++++++++++ service/test/agama/storage/proposal_test.rb | 62 -------------- 7 files changed, 140 insertions(+), 163 deletions(-) diff --git a/service/lib/agama/dbus/storage/manager.rb b/service/lib/agama/dbus/storage/manager.rb index 96eedc978d..e1be330c51 100644 --- a/service/lib/agama/dbus/storage/manager.rb +++ b/service/lib/agama/dbus/storage/manager.rb @@ -35,6 +35,7 @@ require "agama/dbus/storage/volume_conversion" require "agama/dbus/with_progress" require "agama/dbus/with_service_status" +require "agama/storage/config_conversions" require "agama/storage/encryption_settings" require "agama/storage/proposal_settings" require "agama/storage/volume_templates_builder" @@ -70,7 +71,6 @@ def initialize(backend, service_status: nil, logger: nil) @actions = read_actions register_storage_callbacks - register_proposal_callbacks register_progress_callbacks register_service_status_callbacks register_iscsi_callbacks @@ -120,19 +120,44 @@ def probe(keep_config: false, keep_activation: true) # @return [Integer] 0 success; 1 error def apply_config(serialized_config) logger.info("Setting storage config from D-Bus: #{serialized_config}") - config_json = JSON.parse(serialized_config, symbolize_names: true) - proposal.calculate_from_json(config_json) - proposal.success? ? 0 : 1 + configure(config_json) + end + + # Applies the given serialized config model according to the JSON schema. + # + # @param serialized_model [String] Serialized storage config model. + # @return [Integer] 0 success; 1 error + def apply_config_model(serialized_model) + logger.info("Setting storage config model from D-Bus: #{serialized_model}") + + model_json = JSON.parse(serialized_model, symbolize_names: true) + config = Agama::Storage::ConfigConversions::FromModel.new( + model_json, + product_config: product_config, + storage_system: proposal.storage_system + ).convert + config_json = { storage: Agama::Storage::ConfigConversions::ToJSON.new(config).convert } + + configure(config_json) end - # Calculates the initial proposal. + # Resets to the default config. # # @return [Integer] 0 success; 1 error def reset_config logger.info("Reset storage config from D-Bus") - backend.calculate_proposal - backend.proposal.success? ? 0 : 1 + configure + end + + # Configures storage. + # + # @param config_json [Hash, nil] Storage config according to the JSON schema. If nil, then + # the default config is applied. + # @return [Integer] 0 success; 1 error + def configure(config_json = nil) + success = backend.configure(config_json) + success ? 0 : 1 end # Gets and serializes the storage config used for calculating the current proposal. @@ -143,18 +168,6 @@ def recover_config JSON.pretty_generate(json) end - # Applies the given serialized config model according to the JSON schema. - # - # @param serialized_model [String] Serialized storage config model. - # @return [Integer] 0 success; 1 error - def apply_config_model(serialized_model) - logger.info("Setting storage config model from D-Bus: #{serialized_model}") - - model_json = JSON.parse(serialized_model, symbolize_names: true) - proposal.calculate_from_model(model_json) - proposal.success? ? 0 : 1 - end - # Gets and serializes the storage config model. # # @return [String] @@ -312,8 +325,7 @@ def system_device_path(device) end dbus_interface STORAGE_DEVICES_INTERFACE do - # PropertiesChanged signal if a proposal is calculated, see - # {#register_proposal_callbacks}. + # PropertiesChanged signal if storage is configured, see {#register_callbacks}. dbus_reader_attr_accessor :actions, "aa{sv}" dbus_reader :available_drives, "ao" @@ -333,7 +345,7 @@ def calculate_guided_proposal(settings_dbus) logger.info("Calculating guided storage proposal from D-Bus: #{settings_dbus}") settings = ProposalSettingsConversion.from_dbus(settings_dbus, - config: config, logger: logger) + config: product_config, logger: logger) proposal.calculate_guided(settings) proposal.success? ? 0 : 1 @@ -491,10 +503,7 @@ def register_storage_callbacks backend.on_issues_change { issues_properties_changed } backend.on_deprecated_system_change { storage_properties_changed } backend.on_probe { refresh_system_devices } - end - - def register_proposal_callbacks - proposal.on_calculate do + backend.on_configure do export_proposal proposal_properties_changed refresh_staging_devices @@ -594,13 +603,13 @@ def tree_path(tree_root) end # @return [Agama::Config] - def config + def product_config backend.product_config end # @return [Agama::VolumeTemplatesBuilder] def volume_templates_builder - Agama::Storage::VolumeTemplatesBuilder.new_from_config(config) + Agama::Storage::VolumeTemplatesBuilder.new_from_config(product_config) end end end diff --git a/service/lib/agama/storage/manager.rb b/service/lib/agama/storage/manager.rb index a5ea9c31ca..9bce3c5228 100644 --- a/service/lib/agama/storage/manager.rb +++ b/service/lib/agama/storage/manager.rb @@ -70,7 +70,6 @@ def initialize(product_config, logger: nil) @bootloader = Bootloader.new(logger) register_progress_callbacks - register_proposal_callbacks end # Whether the system is in a deprecated status @@ -115,6 +114,14 @@ def on_probe(&block) @on_probe_callbacks << block end + # Registers a callback to be called when storage is configured. + # + # @param block [Proc] + def on_configure(&block) + @on_configure_callbacks ||= [] + @on_configure_callbacks << block + end + # Probes storage devices and performs an initial proposal # # @param keep_config [Boolean] Whether to use the current storage config for calculating the @@ -137,7 +144,10 @@ def probe(keep_config: false, keep_activation: true) progress.step { activate_devices(keep_activation: keep_activation) } progress.step { probe_devices } - progress.step { calculate_proposal(keep_config: keep_config) } + progress.step do + config_json = proposal.storage_json if keep_config + configure(config_json) + end # The system is not deprecated anymore self.deprecated_system = false @@ -169,13 +179,16 @@ def finish Finisher.new(logger, product_config, security).run end - # Calculates the proposal. + # Configures storage. # - # @param keep_config [Boolean] Whether to use the current storage config for calculating the - # proposal. If false, then the default config from the product is used. - def calculate_proposal(keep_config: false) - config_json = proposal.storage_json if keep_config - Configurator.new(proposal).configure(config_json) + # @param config_json [Hash, nil] Storage config according to the JSON schema. If nil, then + # the default config is applied. + # @return [Boolean] Whether storage was successfully configured. + def configure(config_json = nil) + result = Configurator.new(proposal).configure(config_json) + update_issues + @on_configure_callbacks&.each(&:call) + result end # Storage proposal manager @@ -245,11 +258,6 @@ def register_progress_callbacks on_progress_change { logger.info(progress.to_s) } end - # Issues are updated when the proposal is calculated - def register_proposal_callbacks - proposal.on_calculate { update_issues } - end - # Activates the devices, calling activation callbacks if needed # # @param keep_activation [Boolean] Whether to keep the current activation (e.g., provided LUKS diff --git a/service/lib/agama/storage/proposal.rb b/service/lib/agama/storage/proposal.rb index 116edc2c06..236537a72c 100644 --- a/service/lib/agama/storage/proposal.rb +++ b/service/lib/agama/storage/proposal.rb @@ -46,7 +46,6 @@ def initialize(product_config, logger: nil) @product_config = product_config @logger = logger || Logger.new($stdout) - @on_calculate_callbacks = [] end # Whether the proposal was already calculated. @@ -63,11 +62,6 @@ def success? calculated? && !proposal.failed? && issues.none?(&:error?) end - # Stores callbacks to be called after calculating a proposal. - def on_calculate(&block) - @on_calculate_callbacks << block - end - # Default storage config according to the JSON schema. # # The default config depends on the target device. @@ -154,18 +148,6 @@ def calculate_from_json(source_json) success? end - # Calculates a new proposal from a config model. - # - # @param model_json [Hash] Source config model according to the JSON schema. - # @return [Boolean] Whether the proposal successes. - def calculate_from_model(model_json) - config = ConfigConversions::FromModel - .new(model_json, product_config: product_config, storage_system: storage_system) - .convert - - calculate_agama(config) - end - # Calculates a new proposal using the guided strategy. # # @param settings [Agama::Storage::ProposalSettings] @@ -342,7 +324,6 @@ def calculate raise e end - @on_calculate_callbacks.each(&:call) success? end diff --git a/service/test/agama/dbus/storage/manager_test.rb b/service/test/agama/dbus/storage/manager_test.rb index 7ae67f32e6..19d4a119ab 100644 --- a/service/test/agama/dbus/storage/manager_test.rb +++ b/service/test/agama/dbus/storage/manager_test.rb @@ -70,12 +70,12 @@ def serialize(value) # Speed up tests by avoding real check of TPM presence. allow(Y2Storage::EncryptionMethod::TPM_FDE).to receive(:possible?).and_return(true) allow(Yast::Arch).to receive(:s390).and_return false - allow(proposal).to receive(:on_calculate) + allow(backend).to receive(:on_configure) + allow(backend).to receive(:on_issues_change) allow(backend).to receive(:actions).and_return([]) allow(backend).to receive(:iscsi).and_return(iscsi) allow(backend).to receive(:software).and_return(software) allow(backend).to receive(:proposal).and_return(proposal) - allow(backend).to receive(:calculate_proposal) mock_storage(devicegraph: "empty-hd-50GiB.yaml") end diff --git a/service/test/agama/storage/autoyast_proposal_test.rb b/service/test/agama/storage/autoyast_proposal_test.rb index 085a110bea..3a71918e2d 100644 --- a/service/test/agama/storage/autoyast_proposal_test.rb +++ b/service/test/agama/storage/autoyast_proposal_test.rb @@ -141,19 +141,6 @@ def root_filesystem(disk) subject.calculate_autoyast(partitioning) expect(subject.issues).to be_empty end - - it "runs all the callbacks" do - callback1 = proc {} - callback2 = proc {} - - subject.on_calculate(&callback1) - subject.on_calculate(&callback2) - - expect(callback1).to receive(:call) - expect(callback2).to receive(:call) - - subject.calculate_autoyast(partitioning) - end end context "if no root is specified" do @@ -184,19 +171,6 @@ def root_filesystem(disk) ) ) end - - it "runs all the callbacks" do - callback1 = proc {} - callback2 = proc {} - - subject.on_calculate(&callback1) - subject.on_calculate(&callback2) - - expect(callback1).to receive(:call) - expect(callback2).to receive(:call) - - subject.calculate_autoyast(partitioning) - end end end @@ -553,19 +527,6 @@ def root_filesystem(disk) expect(root.snapshots?).to eq(false) end end - - it "runs all the callbacks" do - callback1 = proc {} - callback2 = proc {} - - subject.on_calculate(&callback1) - subject.on_calculate(&callback2) - - expect(callback1).to receive(:call) - expect(callback2).to receive(:call) - - subject.calculate_autoyast(partitioning) - end end end end diff --git a/service/test/agama/storage/manager_test.rb b/service/test/agama/storage/manager_test.rb index 06be2dff27..bd71f04c3e 100644 --- a/service/test/agama/storage/manager_test.rb +++ b/service/test/agama/storage/manager_test.rb @@ -322,6 +322,86 @@ end end + describe "#configure" do + before do + allow(proposal).to receive(:issues).and_return(proposal_issues) + allow(proposal).to receive(:calculate_from_json) + allow(proposal).to receive(:storage_json).and_return(config_json) + allow_any_instance_of(Agama::Storage::Configurator) + .to receive(:generate_configs).and_return([default_config]) + end + + let(:proposal) { Agama::Storage::Proposal.new(config, logger: logger) } + + let(:default_config) do + { + storage: { + drives: [ + search: "/dev/vda1" + ] + } + } + end + + let(:config_json) do + { + storage: { + drives: [ + search: "/dev/vda2" + ] + } + } + end + + let(:proposal_issues) { [Agama::Issue.new("proposal issue")] } + + let(:callback) { proc {} } + + it "calculates a proposal using the default config if no config is given" do + expect(proposal).to receive(:calculate_from_json).with(default_config) + storage.configure + end + + it "calculates a proposal using the given config" do + expect(proposal).to receive(:calculate_from_json).with(config_json) + storage.configure(config_json) + end + + it "adds the proposal issues" do + storage.configure + + expect(storage.issues).to include( + an_object_having_attributes(description: /proposal issue/) + ) + end + + it "executes the on_configure callbacks" do + storage.on_configure(&callback) + expect(callback).to receive(:call) + storage.configure + end + + context "if the proposal was correctly calculated" do + before do + allow(proposal).to receive(:success?).and_return(true) + end + + it "returns true" do + expect(storage.configure).to eq(true) + end + end + + context "if the proposal was not correctly calculated" do + before do + allow(proposal).to receive(:success?).and_return(false) + end + + it "returns false" do + expect(storage.configure).to eq(false) + end + end + end + describe "#install" do before do allow(y2storage_manager).to receive(:staging).and_return(proposed_devicegraph) diff --git a/service/test/agama/storage/proposal_test.rb b/service/test/agama/storage/proposal_test.rb index 7d3c4a7a68..0d1a3f5f34 100644 --- a/service/test/agama/storage/proposal_test.rb +++ b/service/test/agama/storage/proposal_test.rb @@ -592,21 +592,6 @@ def drive(partitions) end end - shared_examples "check proposal callbacks" do |action, settings| - it "runs all the callbacks" do - callback1 = proc {} - callback2 = proc {} - - subject.on_calculate(&callback1) - subject.on_calculate(&callback2) - - expect(callback1).to receive(:call) - expect(callback2).to receive(:call) - - subject.public_send(action, send(settings)) - end - end - shared_examples "check proposal return" do |action, achivable_settings, impossible_settings| it "returns whether the proposal was successful" do result = subject.public_send(action, send(achivable_settings)) @@ -628,19 +613,6 @@ def drive(partitions) expect(Y2Storage::StorageManager.instance.proposal).to be_nil end - it "does not run the callbacks" do - callback1 = proc {} - callback2 = proc {} - - subject.on_calculate(&callback1) - subject.on_calculate(&callback2) - - expect(callback1).to_not receive(:call) - expect(callback2).to_not receive(:call) - - subject.public_send(action, send(settings)) - end - it "returns false" do result = subject.public_send(action, send(settings)) expect(result).to eq(false) @@ -684,8 +656,6 @@ def drive(partitions) ) end - include_examples "check proposal callbacks", :calculate_guided, :achivable_settings - include_examples "check proposal return", :calculate_guided, :achivable_settings, :impossible_settings @@ -741,8 +711,6 @@ def drive(partitions) expect(Y2Storage::StorageManager.instance.proposal).to be_a(Y2Storage::AgamaProposal) end - include_examples "check proposal callbacks", :calculate_agama, :achivable_config - include_examples "check proposal return", :calculate_agama, :achivable_config, :impossible_config @@ -784,8 +752,6 @@ def drive(partitions) expect(Y2Storage::StorageManager.instance.proposal).to be_a(Y2Storage::AutoinstProposal) end - include_examples "check proposal callbacks", :calculate_autoyast, :achivable_settings - include_examples "check proposal return", :calculate_autoyast, :achivable_settings, :impossible_settings @@ -878,34 +844,6 @@ def drive(partitions) end end - describe "#calculate_from_model" do - let(:model_json) do - { - drives: [ - { - name: "/dev/vda", - filesystem: { - type: "xfs" - } - } - ] - } - end - - it "calculates a proposal with the agama strategy and with the expected config" do - expect(subject).to receive(:calculate_agama) do |config| - expect(config).to be_a(Agama::Storage::Config) - expect(config.drives.size).to eq(1) - - drive = config.drives.first - expect(drive.search.name).to eq("/dev/vda") - expect(drive.filesystem.type.fs_type).to eq(Y2Storage::Filesystems::Type::XFS) - end - - subject.calculate_from_model(model_json) - end - end - describe "#actions" do it "returns an empty list if calculate has not been called yet" do expect(subject.actions).to eq([]) From 423a6baf06915d0483fad6576d61ae4f54cf0e9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 30 Jul 2025 15:27:03 +0100 Subject: [PATCH 18/38] storage: send event when storage is configured - Includes the client_id as part of the event. --- rust/agama-lib/src/auth.rs | 12 +++- rust/agama-lib/src/http/event.rs | 3 + rust/agama-lib/src/storage/client.rs | 43 ++++++++++--- .../agama-lib/src/storage/proxies/storage1.rs | 35 ++++++++-- rust/agama-server/src/storage/web.rs | 58 +++++++++++++---- service/lib/agama/dbus/base_object.rb | 23 ++++++- service/lib/agama/dbus/storage/manager.rb | 64 +++++++++++++------ 7 files changed, 186 insertions(+), 52 deletions(-) diff --git a/rust/agama-lib/src/auth.rs b/rust/agama-lib/src/auth.rs index b3f403edaa..ea663a3a18 100644 --- a/rust/agama-lib/src/auth.rs +++ b/rust/agama-lib/src/auth.rs @@ -45,7 +45,7 @@ const USER_TOKEN_PATH: &str = ".local/agama/token"; const AGAMA_TOKEN_FILE: &str = "/run/agama/token"; use std::{ - fmt::Display, + fmt, fs::{self, File}, io::{self, BufRead, BufReader, Write}, os::unix::fs::OpenOptionsExt, @@ -184,8 +184,8 @@ impl AuthToken { } } -impl Display for AuthToken { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { +impl fmt::Display for AuthToken { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", self.0) } } @@ -225,6 +225,12 @@ impl ClientId { } } +impl fmt::Display for ClientId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.0) + } +} + #[cfg(test)] mod tests { use tempfile::tempdir; diff --git a/rust/agama-lib/src/http/event.rs b/rust/agama-lib/src/http/event.rs index 1072a4424e..52f4bbb643 100644 --- a/rust/agama-lib/src/http/event.rs +++ b/rust/agama-lib/src/http/event.rs @@ -103,6 +103,9 @@ pub enum EventPayload { #[serde(flatten)] change: NetworkChange, }, + StorageChanged { + client_id: String, + }, // TODO: it should include the full software proposal or, at least, // all the relevant changes. SoftwareProposalChanged { diff --git a/rust/agama-lib/src/storage/client.rs b/rust/agama-lib/src/storage/client.rs index 34cd4bba93..e36ce23cf7 100644 --- a/rust/agama-lib/src/storage/client.rs +++ b/rust/agama-lib/src/storage/client.rs @@ -155,26 +155,42 @@ impl<'a> StorageClient<'a> { } /// Runs the reprobing process - pub async fn reprobe(&self) -> Result<(), ServiceError> { - Ok(self.storage_proxy.reprobe().await?) + pub async fn reprobe(&self, client_id: String) -> Result<(), ServiceError> { + Ok(self + .storage_proxy + .reprobe(HashMap::from([("client_id", &client_id.into())])) + .await?) } /// Runs the reactivation process - pub async fn reactivate(&self) -> Result<(), ServiceError> { - Ok(self.storage_proxy.reactivate().await?) + pub async fn reactivate(&self, client_id: String) -> Result<(), ServiceError> { + Ok(self + .storage_proxy + .reactivate(HashMap::from([("client_id", &client_id.into())])) + .await?) } /// Set the storage config according to the JSON schema - pub async fn set_config(&self, settings: StorageSettings) -> Result { + pub async fn set_config( + &self, + settings: StorageSettings, + client_id: String, + ) -> Result { Ok(self .storage_proxy - .set_config(serde_json::to_string(&settings)?.as_str()) + .set_config( + serde_json::to_string(&settings)?.as_str(), + HashMap::from([("client_id", &client_id.into())]), + ) .await?) } /// Reset the storage config to the default value - pub async fn reset_config(&self) -> Result { - Ok(self.storage_proxy.reset_config().await?) + pub async fn reset_config(&self, client_id: String) -> Result { + Ok(self + .storage_proxy + .reset_config(HashMap::from([("client_id", &client_id.into())])) + .await?) } /// Get the storage config according to the JSON schema @@ -185,10 +201,17 @@ impl<'a> StorageClient<'a> { } /// Set the storage config model according to the JSON schema - pub async fn set_config_model(&self, model: Box) -> Result { + pub async fn set_config_model( + &self, + model: Box, + client_id: String, + ) -> Result { Ok(self .storage_proxy - .set_config_model(serde_json::to_string(&model).unwrap().as_str()) + .set_config_model( + serde_json::to_string(&model).unwrap().as_str(), + HashMap::from([("client_id", &client_id.into())]), + ) .await?) } diff --git a/rust/agama-lib/src/storage/proxies/storage1.rs b/rust/agama-lib/src/storage/proxies/storage1.rs index 19128aa144..4c707f06a0 100644 --- a/rust/agama-lib/src/storage/proxies/storage1.rs +++ b/rust/agama-lib/src/storage/proxies/storage1.rs @@ -47,6 +47,10 @@ use zbus::proxy; assume_defaults = true )] pub trait Storage1 { + /// Storage configured signal + #[zbus(signal)] + fn configured(&self, client_id: &str) -> zbus::Result<()>; + /// Finish method fn finish(&self) -> zbus::Result<()>; @@ -57,23 +61,40 @@ pub trait Storage1 { fn probe(&self) -> zbus::Result<()>; /// Reprobe method - fn reprobe(&self) -> zbus::Result<()>; + fn reprobe( + &self, + data: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>, + ) -> zbus::Result<()>; /// Reactivate method - fn reactivate(&self) -> zbus::Result<()>; + fn reactivate( + &self, + data: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>, + ) -> zbus::Result<()>; /// Set the storage config according to the JSON schema - fn set_config(&self, settings: &str) -> zbus::Result; + fn set_config( + &self, + settings: &str, + data: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>, + ) -> zbus::Result; /// Reset the storage config to the default value - fn reset_config(&self) -> zbus::Result; + fn reset_config( + &self, + data: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>, + ) -> zbus::Result; + + /// Set the storage config model according to the JSON schema + fn set_config_model( + &self, + model: &str, + data: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>, + ) -> zbus::Result; /// Get the current storage config according to the JSON schema fn get_config(&self) -> zbus::Result; - /// Set the storage config model according to the JSON schema - fn set_config_model(&self, model: &str) -> zbus::Result; - /// Get the storage config model according to the JSON schema fn get_config_model(&self) -> zbus::Result; diff --git a/rust/agama-server/src/storage/web.rs b/rust/agama-server/src/storage/web.rs index b08807d04f..d87bb51778 100644 --- a/rust/agama-server/src/storage/web.rs +++ b/rust/agama-server/src/storage/web.rs @@ -67,10 +67,16 @@ use crate::{ }; pub async fn storage_streams(dbus: zbus::Connection) -> Result { - let mut result: EventStreams = vec![( - "devices_dirty", - Box::pin(devices_dirty_stream(dbus.clone()).await?), - )]; + let mut result: EventStreams = vec![ + ( + "devices_dirty", + Box::pin(devices_dirty_stream(dbus.clone()).await?), + ), + ( + "configured", + Box::pin(configured_stream(dbus.clone()).await?), + ), + ]; let mut iscsi = iscsi_stream(&dbus).await?; let mut dasd = dasd_stream(&dbus).await?; let mut zfcp = zfcp_stream(&dbus).await?; @@ -96,6 +102,19 @@ async fn devices_dirty_stream(dbus: zbus::Connection) -> Result Result, Error> { + let proxy = Storage1Proxy::new(&dbus).await?; + let stream = proxy.receive_configured().await?.filter_map(|signal| { + if let Ok(args) = signal.args() { + return Some(event!(StorageChanged { + client_id: args.client_id.to_string() + })); + } + None + }); + Ok(stream) +} + #[derive(Clone)] struct StorageState<'a> { client: StorageClient<'a>, @@ -197,11 +216,12 @@ async fn get_config( )] async fn set_config( State(state): State>, + Extension(client_id): Extension>, Json(settings): Json, ) -> Result, Error> { let _status: u32 = state .client - .set_config(settings) + .set_config(settings, client_id.to_string()) .await .map_err(Error::Service)?; Ok(Json(())) @@ -246,8 +266,15 @@ async fn get_config_model( (status = 400, description = "The D-Bus service could not perform the action") ) )] -async fn reset_config(State(state): State>) -> Result, Error> { - let _status: u32 = state.client.reset_config().await.map_err(Error::Service)?; +async fn reset_config( + State(state): State>, + Extension(client_id): Extension>, +) -> Result, Error> { + let _status: u32 = state + .client + .reset_config(client_id.to_string()) + .await + .map_err(Error::Service)?; Ok(Json(())) } @@ -268,11 +295,12 @@ async fn reset_config(State(state): State>) -> Result, )] async fn set_config_model( State(state): State>, + Extension(client_id): Extension>, Json(model): Json>, ) -> Result, Error> { let _status: u32 = state .client - .set_config_model(model) + .set_config_model(model, client_id.to_string()) .await .map_err(Error::Service)?; Ok(Json(())) @@ -334,8 +362,11 @@ async fn probe(State(state): State>) -> Result, Error> ), operation_id = "storage_reprobe" )] -async fn reprobe(State(state): State>) -> Result, Error> { - Ok(Json(state.client.reprobe().await?)) +async fn reprobe( + State(state): State>, + Extension(client_id): Extension>, +) -> Result, Error> { + Ok(Json(state.client.reprobe(client_id.to_string()).await?)) } /// Reactivate the storage devices. @@ -349,8 +380,11 @@ async fn reprobe(State(state): State>) -> Result, Erro ), operation_id = "storage_reactivate" )] -async fn reactivate(State(state): State>) -> Result, Error> { - Ok(Json(state.client.reactivate().await?)) +async fn reactivate( + State(state): State>, + Extension(client_id): Extension>, +) -> Result, Error> { + Ok(Json(state.client.reactivate(client_id.to_string()).await?)) } /// Gets whether the system is in a deprecated status. diff --git a/service/lib/agama/dbus/base_object.rb b/service/lib/agama/dbus/base_object.rb index 79c8db4884..07a853cd39 100644 --- a/service/lib/agama/dbus/base_object.rb +++ b/service/lib/agama/dbus/base_object.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -# Copyright (c) [2021] SUSE LLC +# Copyright (c) [2021-2025] SUSE LLC # # All Rights Reserved. # @@ -38,6 +38,27 @@ def initialize(path, logger: nil) # @return [Logger] attr_reader :logger + + # Extra data provided to the D-Bus call (e.g., the client_id requesting the action). + # + # @return [Hash] + def request_data + @request_data ||= {} + end + + # Executes a block ensuring the given request data is available during the process. + # + # Saving the request data is needed in order to have it available while emitting signals as + # part of the block execution. + # + # @param data [Hash] Extra data, see {#request_data}. + # @param block [Proc] + def request(data = {}, &block) + @request_data = data + block.call + ensure + @request_data = {} + end end end end diff --git a/service/lib/agama/dbus/storage/manager.rb b/service/lib/agama/dbus/storage/manager.rb index e1be330c51..2384492b96 100644 --- a/service/lib/agama/dbus/storage/manager.rb +++ b/service/lib/agama/dbus/storage/manager.rb @@ -150,16 +150,6 @@ def reset_config configure end - # Configures storage. - # - # @param config_json [Hash, nil] Storage config according to the JSON schema. If nil, then - # the default config is applied. - # @return [Integer] 0 success; 1 error - def configure(config_json = nil) - success = backend.configure(config_json) - success ? 0 : 1 - end - # Gets and serializes the storage config used for calculating the current proposal. # # @return [String] @@ -208,19 +198,30 @@ def deprecated_system # they should return whether the config was actually applied. # * Methods like #Probe or #Install return nothing. dbus_interface STORAGE_INTERFACE do + dbus_signal :Configured, "client_id:s" dbus_method(:Probe) { probe } - dbus_method(:Reprobe) { probe(keep_config: true) } - dbus_method(:Reactivate) { probe(keep_config: true, keep_activation: false) } - dbus_method(:SetConfig, "in serialized_config:s, out result:u") do |serialized_config| - busy_while { apply_config(serialized_config) } + dbus_method(:Reprobe, "in data:a{sv}") do |data| + busy_request(data) { probe(keep_config: true) } end - dbus_method(:ResetConfig, "out result:u") do - busy_while { reset_config } + dbus_method(:Reactivate, "in data:a{sv}") do |data| + busy_request(data) { probe(keep_config: true, keep_activation: false) } end - dbus_method(:GetConfig, "out serialized_config:s") { recover_config } - dbus_method(:SetConfigModel, "in serialized_model:s, out result:u") do |serialized_model| - busy_while { apply_config_model(serialized_model) } + dbus_method( + :SetConfig, + "in serialized_config:s, in data:a{sv}, out result:u" + ) do |serialized_config, data| + busy_request(data) { apply_config(serialized_config) } end + dbus_method(:ResetConfig, "in data:a{sv}, out result:u") do |data| + busy_request(data) { reset_config } + end + dbus_method( + :SetConfigModel, + "in serialized_model:s, in data:a{sv}, out result:u" + ) do |serialized_model, data| + busy_request(data) { apply_config_model(serialized_model) } + end + dbus_method(:GetConfig, "out serialized_config:s") { recover_config } dbus_method(:GetConfigModel, "out serialized_model:s") { recover_model } dbus_method(:SolveConfigModel, "in sparse_model:s, out solved_model:s") do |sparse_model| solve_model(sparse_model) @@ -483,6 +484,30 @@ def iscsi_delete(path) # @return [DBus::Storage::Proposal, nil] attr_reader :dbus_proposal + # Executes a request setting the service as busy. + # + # @see BaseObject#request + # + # @param data [Hash] see {BaseObject#request_data}. + # @param block [Proc] + def busy_request(data, &block) + busy_while { request(data, &block) } + end + + # Configures storage. + # + # @param config_json [Hash, nil] Storage config according to the JSON schema. If nil, then + # the default config is applied. + # @return [Integer] 0 success; 1 error + def configure(config_json = nil) + success = backend.configure(config_json) + success ? 0 : 1 + end + + def send_configured_signal + self.Configured(request_data["client_id"].to_s) + end + def add_s390_interfaces require "agama/dbus/storage/interfaces/dasd_manager" require "agama/dbus/storage/interfaces/zfcp_manager" @@ -508,6 +533,7 @@ def register_storage_callbacks proposal_properties_changed refresh_staging_devices update_actions + send_configured_signal end end From 2bbc686fa7a65a969b6aca25818b2cf8ffb83cd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 14:30:04 +0100 Subject: [PATCH 19/38] web: fix alert --- web/src/components/core/AlertOutOfSync.test.tsx | 12 ++++++------ web/src/components/core/AlertOutOfSync.tsx | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/web/src/components/core/AlertOutOfSync.test.tsx b/web/src/components/core/AlertOutOfSync.test.tsx index 9e2f76b1aa..db6c5bb8d8 100644 --- a/web/src/components/core/AlertOutOfSync.test.tsx +++ b/web/src/components/core/AlertOutOfSync.test.tsx @@ -86,21 +86,21 @@ describe("AlertOutOfSync", () => { // Simulate a change event for a different scope act(() => { - eventCallback({ type: "NotWatchedChanged", clientId: "other-client" }); + eventCallback({ type: "NotWatchedChanged", client_id: "other-client" }); }); expect(screen.queryByText("Info alert:")).toBeNull(); // Simulate a change event for the subscribed scope, from current client act(() => { - eventCallback({ type: "WatchedChanged", clientId: "current-client" }); + eventCallback({ type: "WatchedChanged", client_id: "current-client" }); }); expect(screen.queryByText("Info alert:")).toBeNull(); // Simulate a change event for the subscribed scope, from different client act(() => { - eventCallback({ type: "WatchedChanged", clientId: "other-client" }); + eventCallback({ type: "WatchedChanged", client_id: "other-client" }); }); screen.getByText("Info alert:"); @@ -120,7 +120,7 @@ describe("AlertOutOfSync", () => { // Simulate a change event for the subscribed scope, from different client act(() => { - eventCallback({ type: "WatchedChanged", clientId: "other-client" }); + eventCallback({ type: "WatchedChanged", client_id: "other-client" }); }); screen.getByText("Info alert:"); @@ -130,7 +130,7 @@ describe("AlertOutOfSync", () => { // Simulate a change event for the subscribed scope, from current client act(() => { - eventCallback({ type: "WatchedChanged", clientId: "current-client" }); + eventCallback({ type: "WatchedChanged", client_id: "current-client" }); }); expect(screen.queryByText("Info alert:")).toBeNull(); @@ -147,7 +147,7 @@ describe("AlertOutOfSync", () => { // Simulate a change event for the subscribed scope, from different client act(() => { - eventCallback({ type: "WatchedChanged", clientId: "other-client" }); + eventCallback({ type: "WatchedChanged", client_id: "other-client" }); }); const reloadButton = screen.getByRole("button", { name: "Reload now" }); diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx index 773bed87c2..3bb12d8bf8 100644 --- a/web/src/components/core/AlertOutOfSync.tsx +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -72,7 +72,7 @@ export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncP if (missingScope) return; return client.onEvent((event) => { - event.type === `${scope}Changed` && setActive(event.clientId !== client.id); + event.type === `${scope}Changed` && setActive(event.client_id !== client.id); }); }); From f362fb5abf785e25934aa42b25fa14fb2b01d79c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Thu, 31 Jul 2025 14:30:25 +0100 Subject: [PATCH 20/38] web: use alert in storage --- web/src/components/storage/ProposalPage.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index ef1f65ef86..2fdf6d15f0 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -45,6 +45,7 @@ import ProposalFailedInfo from "./ProposalFailedInfo"; import ProposalResultSection from "./ProposalResultSection"; import ProposalTransactionalInfo from "./ProposalTransactionalInfo"; import UnsupportedModelInfo from "./UnsupportedModelInfo"; +import AlertOutOfSync from "~/components/core/AlertOutOfSync"; import { useAvailableDevices } from "~/hooks/storage/system"; import { useResetConfigMutation, @@ -255,6 +256,7 @@ export default function ProposalPage(): React.ReactNode { return ( + {_("Storage")} From 355e69a3d0d379d766b6306657deca3203d5b6a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sun, 3 Aug 2025 19:13:01 +0100 Subject: [PATCH 21/38] feat(web): safeguard partition page/form from missing or wrong param Prevent errors when a device no longer exists, whether due to a mistyped URL or changes in the underlying system. It does so by introducing a new core component as a first step toward unifying how Agama handles forms and pages that rely on URL parameters. While this is an initial implementation, further refinement will be needed as adoption grows and ergonomic improvements are identified to make it fully adaptable and developer-friendly. --- .../components/core/ResourceNotFound.test.tsx | 63 +++++++++++++++++++ web/src/components/core/ResourceNotFound.tsx | 61 ++++++++++++++++++ web/src/components/storage/PartitionPage.tsx | 20 ++++-- 3 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 web/src/components/core/ResourceNotFound.test.tsx create mode 100644 web/src/components/core/ResourceNotFound.tsx diff --git a/web/src/components/core/ResourceNotFound.test.tsx b/web/src/components/core/ResourceNotFound.test.tsx new file mode 100644 index 0000000000..c976af9042 --- /dev/null +++ b/web/src/components/core/ResourceNotFound.test.tsx @@ -0,0 +1,63 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { screen } from "@testing-library/dom"; +import { plainRender } from "~/test-utils"; +import ResourceNotFound from "./ResourceNotFound"; +import { ROOT } from "~/routes/paths"; + +describe("ResourceNotFound", () => { + it("renders the default title when none is given", () => { + plainRender(); + screen.getByRole("heading", { name: "Resource not found or lost", level: 3 }); + }); + + it("renders the given title", () => { + plainRender( + , + ); + screen.getByRole("heading", { name: "Not found", level: 3 }); + }); + + it("renders the default body when none is given", () => { + plainRender(); + screen.getByText("It doesn't exist or can't be reached."); + }); + + it("renders the given body", () => { + plainRender( + , + ); + screen.getByText("Unexpected path, nothing to show"); + }); + + it("renders a link with given text and path", () => { + plainRender(); + const link = screen.getByRole("link", { name: "Go to homepage" }); + expect(link).toHaveAttribute("href", ROOT.root); + }); +}); diff --git a/web/src/components/core/ResourceNotFound.tsx b/web/src/components/core/ResourceNotFound.tsx new file mode 100644 index 0000000000..c7b2dfc548 --- /dev/null +++ b/web/src/components/core/ResourceNotFound.tsx @@ -0,0 +1,61 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import React from "react"; +import { + EmptyState, + EmptyStateActions, + EmptyStateBody, + EmptyStateFooter, +} from "@patternfly/react-core"; +import Icon from "../layout/Icon"; +import Link from "../core/Link"; +import { _ } from "~/i18n"; + +type ResourceNotFoundProps = { + title?: string; + body?: React.ReactNode; + linkText: string; + linkPath: string; +}; + +export default function ResourceNotFound({ + title = _("Resource not found or lost"), + body = _("It doesn't exist or can't be reached."), + linkText, + linkPath, +}: ResourceNotFoundProps) { + return ( + }> + {body} + {linkText && linkPath && ( + + + + {linkText} + + + + )} + + ); +} diff --git a/web/src/components/storage/PartitionPage.tsx b/web/src/components/storage/PartitionPage.tsx index 926b98f525..8888072635 100644 --- a/web/src/components/storage/PartitionPage.tsx +++ b/web/src/components/storage/PartitionPage.tsx @@ -47,6 +47,8 @@ import { Page, SelectWrapper as Select, SubtleContent } from "~/components/core/ import { SelectWrapperProps as SelectProps } from "~/components/core/SelectWrapper"; import SelectTypeaheadCreatable from "~/components/core/SelectTypeaheadCreatable"; import AutoSizeText from "~/components/storage/AutoSizeText"; +import SizeModeSelect, { SizeMode, SizeRange } from "~/components/storage/SizeModeSelect"; +import ResourceNotFound from "~/components/core/ResourceNotFound"; import { useAddPartition, useEditPartition } from "~/hooks/storage/partition"; import { useMissingMountPaths } from "~/hooks/storage/product"; import { useModel } from "~/hooks/storage/model"; @@ -62,10 +64,9 @@ import { deviceSize, deviceLabel, filesystemLabel, parseToBytes } from "~/compon import { _ } from "~/i18n"; import { sprintf } from "sprintf-js"; import { apiModel } from "~/api/storage/types"; -import { STORAGE as PATHS } from "~/routes/paths"; -import { unique } from "radashi"; +import { STORAGE as PATHS, STORAGE } from "~/routes/paths"; +import { isUndefined, unique } from "radashi"; import { compact } from "~/utils"; -import SizeModeSelect, { SizeMode, SizeRange } from "~/components/storage/SizeModeSelect"; const NO_VALUE = ""; const NEW_PARTITION = "new"; @@ -688,7 +689,7 @@ function AutoSizeInfo({ value }: AutoSizeInfoProps): React.ReactNode { * @fixme This component has to be adapted to use the new hooks from ~/hooks/storage/ instead of the * deprecated hooks from ~/queries/storage/config-model. */ -export default function PartitionPage() { +const PartitionPageForm = () => { const navigate = useNavigate(); const headingId = useId(); const [mountPoint, setMountPoint] = React.useState(NO_VALUE); @@ -708,6 +709,7 @@ export default function PartitionPage() { const { errors, getVisibleError } = useErrors(value); const device = useModelDevice(); + const unusedMountPoints = useUnusedMountPoints(); const addPartition = useAddPartition(); @@ -905,4 +907,14 @@ export default function PartitionPage() { ); +}; + +export default function PartitionPage() { + const device = useModelDevice(); + + return isUndefined(device) ? ( + + ) : ( + + ); } From ffac6fe36442d4f799a78cee5277e63185d6673f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20D=C3=ADaz=20Gonz=C3=A1lez?= Date: Sun, 3 Aug 2025 19:26:12 +0100 Subject: [PATCH 22/38] feat(web): subscribe PartitionPage to storage changes To show the out-of-sync alert when proceed. --- web/src/components/storage/PartitionPage.tsx | 2 ++ web/src/components/storage/ProposalPage.tsx | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/web/src/components/storage/PartitionPage.tsx b/web/src/components/storage/PartitionPage.tsx index 8888072635..7263e0e633 100644 --- a/web/src/components/storage/PartitionPage.tsx +++ b/web/src/components/storage/PartitionPage.tsx @@ -48,6 +48,7 @@ import { SelectWrapperProps as SelectProps } from "~/components/core/SelectWrapp import SelectTypeaheadCreatable from "~/components/core/SelectTypeaheadCreatable"; import AutoSizeText from "~/components/storage/AutoSizeText"; import SizeModeSelect, { SizeMode, SizeRange } from "~/components/storage/SizeModeSelect"; +import AlertOutOfSync from "~/components/core/AlertOutOfSync"; import ResourceNotFound from "~/components/core/ResourceNotFound"; import { useAddPartition, useEditPartition } from "~/hooks/storage/partition"; import { useMissingMountPaths } from "~/hooks/storage/product"; @@ -814,6 +815,7 @@ const PartitionPageForm = () => { +
diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index 2fdf6d15f0..cd3b786ee5 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -256,11 +256,11 @@ export default function ProposalPage(): React.ReactNode { return ( - {_("Storage")} + {isDeprecated && } {!isDeprecated && !showSections && } {!isDeprecated && showSections && } From ecea98c9eb188ea69e940d53369034cae402f3bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 4 Aug 2025 08:50:29 +0100 Subject: [PATCH 23/38] Revert "web: fix alert" This reverts commit 2bbc686fa7a65a969b6aca25818b2cf8ffb83cd3. --- web/src/components/core/AlertOutOfSync.test.tsx | 12 ++++++------ web/src/components/core/AlertOutOfSync.tsx | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/web/src/components/core/AlertOutOfSync.test.tsx b/web/src/components/core/AlertOutOfSync.test.tsx index db6c5bb8d8..9e2f76b1aa 100644 --- a/web/src/components/core/AlertOutOfSync.test.tsx +++ b/web/src/components/core/AlertOutOfSync.test.tsx @@ -86,21 +86,21 @@ describe("AlertOutOfSync", () => { // Simulate a change event for a different scope act(() => { - eventCallback({ type: "NotWatchedChanged", client_id: "other-client" }); + eventCallback({ type: "NotWatchedChanged", clientId: "other-client" }); }); expect(screen.queryByText("Info alert:")).toBeNull(); // Simulate a change event for the subscribed scope, from current client act(() => { - eventCallback({ type: "WatchedChanged", client_id: "current-client" }); + eventCallback({ type: "WatchedChanged", clientId: "current-client" }); }); expect(screen.queryByText("Info alert:")).toBeNull(); // Simulate a change event for the subscribed scope, from different client act(() => { - eventCallback({ type: "WatchedChanged", client_id: "other-client" }); + eventCallback({ type: "WatchedChanged", clientId: "other-client" }); }); screen.getByText("Info alert:"); @@ -120,7 +120,7 @@ describe("AlertOutOfSync", () => { // Simulate a change event for the subscribed scope, from different client act(() => { - eventCallback({ type: "WatchedChanged", client_id: "other-client" }); + eventCallback({ type: "WatchedChanged", clientId: "other-client" }); }); screen.getByText("Info alert:"); @@ -130,7 +130,7 @@ describe("AlertOutOfSync", () => { // Simulate a change event for the subscribed scope, from current client act(() => { - eventCallback({ type: "WatchedChanged", client_id: "current-client" }); + eventCallback({ type: "WatchedChanged", clientId: "current-client" }); }); expect(screen.queryByText("Info alert:")).toBeNull(); @@ -147,7 +147,7 @@ describe("AlertOutOfSync", () => { // Simulate a change event for the subscribed scope, from different client act(() => { - eventCallback({ type: "WatchedChanged", client_id: "other-client" }); + eventCallback({ type: "WatchedChanged", clientId: "other-client" }); }); const reloadButton = screen.getByRole("button", { name: "Reload now" }); diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx index 3bb12d8bf8..773bed87c2 100644 --- a/web/src/components/core/AlertOutOfSync.tsx +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -72,7 +72,7 @@ export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncP if (missingScope) return; return client.onEvent((event) => { - event.type === `${scope}Changed` && setActive(event.client_id !== client.id); + event.type === `${scope}Changed` && setActive(event.clientId !== client.id); }); }); From bb85d069af898fd6353854ed634b3f0b7654e659 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 4 Aug 2025 08:55:12 +0100 Subject: [PATCH 24/38] storage: properly send client id on storage changed --- rust/agama-lib/src/auth.rs | 4 ++++ rust/agama-lib/src/http/event.rs | 4 +--- rust/agama-server/src/storage/web.rs | 7 ++++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/rust/agama-lib/src/auth.rs b/rust/agama-lib/src/auth.rs index ea663a3a18..b6e972a358 100644 --- a/rust/agama-lib/src/auth.rs +++ b/rust/agama-lib/src/auth.rs @@ -223,6 +223,10 @@ impl ClientId { pub fn new() -> Self { ClientId(Uuid::new_v4()) } + + pub fn new_from_uuid(uuid: Uuid) -> Self { + ClientId(uuid) + } } impl fmt::Display for ClientId { diff --git a/rust/agama-lib/src/http/event.rs b/rust/agama-lib/src/http/event.rs index 52f4bbb643..fb4e92ae21 100644 --- a/rust/agama-lib/src/http/event.rs +++ b/rust/agama-lib/src/http/event.rs @@ -103,9 +103,7 @@ pub enum EventPayload { #[serde(flatten)] change: NetworkChange, }, - StorageChanged { - client_id: String, - }, + StorageChanged, // TODO: it should include the full software proposal or, at least, // all the relevant changes. SoftwareProposalChanged { diff --git a/rust/agama-server/src/storage/web.rs b/rust/agama-server/src/storage/web.rs index d87bb51778..ae7763b743 100644 --- a/rust/agama-server/src/storage/web.rs +++ b/rust/agama-server/src/storage/web.rs @@ -48,6 +48,7 @@ use iscsi::storage_iscsi_service; use serde::{Deserialize, Serialize}; use serde_json::value::RawValue; use tokio_stream::{Stream, StreamExt}; +use uuid::Uuid; use zfcp::{zfcp_service, zfcp_stream}; pub mod dasd; @@ -106,9 +107,9 @@ async fn configured_stream(dbus: zbus::Connection) -> Result Date: Mon, 4 Aug 2025 13:47:10 +0100 Subject: [PATCH 25/38] feat(web): re-render result section only when proceed I.e., only if changes in storage were originated by an action of the same client. It also drops some dead code and an unwanted hasResult condition. --- web/src/components/storage/ProposalPage.tsx | 4 +- .../storage/ProposalResultSection.tsx | 54 +++++++++++-------- 2 files changed, 34 insertions(+), 24 deletions(-) diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index cd3b786ee5..8d24c2c997 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -181,8 +181,6 @@ function ProposalEmptyState(): React.ReactNode { function ProposalSections(): React.ReactNode { const model = useConfigModel({ suspense: true }); - const systemErrors = useSystemErrors("storage"); - const hasResult = !systemErrors.length; return ( @@ -217,7 +215,7 @@ function ProposalSections(): React.ReactNode { )} - {hasResult && } + ); } diff --git a/web/src/components/storage/ProposalResultSection.tsx b/web/src/components/storage/ProposalResultSection.tsx index 6c97fcc5f9..82213c8128 100644 --- a/web/src/components/storage/ProposalResultSection.tsx +++ b/web/src/components/storage/ProposalResultSection.tsx @@ -20,8 +20,8 @@ * find current contact information at www.suse.com. */ -import React, { useState } from "react"; -import { Alert, ExpandableSection, Skeleton, Stack } from "@patternfly/react-core"; +import React, { useEffect, useState } from "react"; +import { Alert, ExpandableSection, Stack } from "@patternfly/react-core"; import { Page } from "~/components/core"; import DevicesManager from "~/components/storage/DevicesManager"; import ProposalResultTable from "~/components/storage/ProposalResultTable"; @@ -29,20 +29,10 @@ import { ProposalActionsDialog } from "~/components/storage"; import { _, n_, formatList } from "~/i18n"; import { useActions, useDevices } from "~/queries/storage"; import { sprintf } from "sprintf-js"; - -/** - * @todo Create a component for rendering a customized skeleton - */ -const ResultSkeleton = () => ( - - - - - -); +import { useInstallerClient } from "~/context/installer"; +import { isNullish } from "radashi"; +import { StorageDevice } from "~/types/storage"; +import { Action } from "~/api/storage/types"; /** * Renders information about delete actions @@ -109,17 +99,39 @@ function ActionsList({ manager }: ActionsListProps) { ); } -export type ProposalResultSectionProps = { - isLoading?: boolean; +type Result = { + system: StorageDevice[]; + staging: StorageDevice[]; + actions: Action[]; }; -export default function ProposalResultSection({ isLoading = false }: ProposalResultSectionProps) { +const useProposalResult = () => { + const client = useInstallerClient(); const system = useDevices("system", { suspense: true }); const staging = useDevices("result", { suspense: true }); const actions = useActions(); - const devicesManager = new DevicesManager(system, staging, actions); + const [result, setResult] = useState(); + + useEffect(() => { + if (isNullish(result) && [system, staging, actions].every((e) => e)) { + setResult({ system, staging, actions }); + } + }, [result, system, staging, actions]); - if (isLoading) return ; + useEffect(() => { + return client.onEvent((event) => { + event.type === "StorageChanged" && + event.clientId === client.id && + setResult({ system, staging, actions }); + }); + }, [client, result, system, staging, actions]); + + return { system, staging, actions }; +}; + +export default function ProposalResultSection() { + const { system, staging, actions } = useProposalResult(); + const devicesManager = new DevicesManager(system, staging, actions); return ( Date: Mon, 4 Aug 2025 13:56:04 +0100 Subject: [PATCH 26/38] feat(web) PoC of hook for using last valid values Using the StorageChanged signal and the clientId prop to determine if the lasd changes are valid or not. --- .../storage/ProposalResultSection.tsx | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/web/src/components/storage/ProposalResultSection.tsx b/web/src/components/storage/ProposalResultSection.tsx index 82213c8128..47d8700b2d 100644 --- a/web/src/components/storage/ProposalResultSection.tsx +++ b/web/src/components/storage/ProposalResultSection.tsx @@ -20,7 +20,7 @@ * find current contact information at www.suse.com. */ -import React, { useEffect, useState } from "react"; +import React, { useEffect, useRef, useState } from "react"; import { Alert, ExpandableSection, Stack } from "@patternfly/react-core"; import { Page } from "~/components/core"; import DevicesManager from "~/components/storage/DevicesManager"; @@ -30,9 +30,6 @@ import { _, n_, formatList } from "~/i18n"; import { useActions, useDevices } from "~/queries/storage"; import { sprintf } from "sprintf-js"; import { useInstallerClient } from "~/context/installer"; -import { isNullish } from "radashi"; -import { StorageDevice } from "~/types/storage"; -import { Action } from "~/api/storage/types"; /** * Renders information about delete actions @@ -99,39 +96,42 @@ function ActionsList({ manager }: ActionsListProps) { ); } -type Result = { - system: StorageDevice[]; - staging: StorageDevice[]; - actions: Action[]; -}; +/** + * Stores the last value that satisfies the given condition. + * Returns undefined until the first valid value is stored. + */ +export function useLastValid(value: T, condition: boolean): T | undefined { + const [initialized, setInitialized] = useState(false); + const lastValidRef = useRef(); + + useEffect(() => { + if (condition) { + lastValidRef.current = value; + if (!initialized) setInitialized(true); + } + }, [value, condition, initialized]); + + return initialized ? lastValidRef.current : undefined; +} -const useProposalResult = () => { +export default function ProposalResultSection() { const client = useInstallerClient(); const system = useDevices("system", { suspense: true }); const staging = useDevices("result", { suspense: true }); const actions = useActions(); - const [result, setResult] = useState(); - - useEffect(() => { - if (isNullish(result) && [system, staging, actions].every((e) => e)) { - setResult({ system, staging, actions }); - } - }, [result, system, staging, actions]); + const [shouldUpdate, setShouldUpdate] = useState(false); useEffect(() => { return client.onEvent((event) => { - event.type === "StorageChanged" && - event.clientId === client.id && - setResult({ system, staging, actions }); + setShouldUpdate(event.type === "StorageChanged" && event.clientId === client.id); }); - }, [client, result, system, staging, actions]); + }, [client]); - return { system, staging, actions }; -}; + const result = useLastValid({ system, staging, actions }, shouldUpdate); -export default function ProposalResultSection() { - const { system, staging, actions } = useProposalResult(); - const devicesManager = new DevicesManager(system, staging, actions); + if (!result) return; + + const devicesManager = new DevicesManager(result.system, result.staging, result.actions); return ( Date: Mon, 4 Aug 2025 16:11:20 +0100 Subject: [PATCH 27/38] feat(web): move useLastValid hook to its own file But still needing test since it is not fully working as expected. --- .../components/storage/ProposalFailedInfo.tsx | 16 ++++++- .../storage/ProposalResultSection.tsx | 28 +++--------- web/src/hooks/use-last-valid.ts | 45 +++++++++++++++++++ 3 files changed, 66 insertions(+), 23 deletions(-) create mode 100644 web/src/hooks/use-last-valid.ts diff --git a/web/src/components/storage/ProposalFailedInfo.tsx b/web/src/components/storage/ProposalFailedInfo.tsx index 8620d65752..6560910227 100644 --- a/web/src/components/storage/ProposalFailedInfo.tsx +++ b/web/src/components/storage/ProposalFailedInfo.tsx @@ -20,7 +20,7 @@ * find current contact information at www.suse.com. */ -import React from "react"; +import React, { useEffect, useState } from "react"; import { Alert, Content } from "@patternfly/react-core"; import { IssueSeverity } from "~/types/issues"; import { useApiModel } from "~/hooks/storage/api-model"; @@ -28,6 +28,8 @@ import { useIssues, useConfigErrors } from "~/queries/issues"; import * as partitionUtils from "~/components/storage/utils/partition"; import { _, formatList } from "~/i18n"; import { sprintf } from "sprintf-js"; +import { useInstallerClient } from "~/context/installer"; +import { useLastValid } from "~/hooks/use-last-valid"; const Description = () => { const model = useApiModel({ suspense: true }); @@ -88,11 +90,21 @@ const Description = () => { * - The generated proposal contains no errors. */ export default function ProposalFailedInfo() { + const client = useInstallerClient(); const configErrors = useConfigErrors("storage"); const errors = useIssues("storage").filter((s) => s.severity === IssueSeverity.Error); + const [shouldUpdate, setShouldUpdate] = useState(false); + + const result = useLastValid(errors, shouldUpdate); + + useEffect(() => { + return client.onEvent((event) => { + setShouldUpdate(event.type === "StorageChanged" && event.clientId === client.id); + }); + }, [client]); if (configErrors.length !== 0) return; - if (errors.length === 0) return; + if (result && result.length === 0) return; return ( diff --git a/web/src/components/storage/ProposalResultSection.tsx b/web/src/components/storage/ProposalResultSection.tsx index 47d8700b2d..e75140c3bc 100644 --- a/web/src/components/storage/ProposalResultSection.tsx +++ b/web/src/components/storage/ProposalResultSection.tsx @@ -20,7 +20,7 @@ * find current contact information at www.suse.com. */ -import React, { useEffect, useRef, useState } from "react"; +import React, { useEffect, useState } from "react"; import { Alert, ExpandableSection, Stack } from "@patternfly/react-core"; import { Page } from "~/components/core"; import DevicesManager from "~/components/storage/DevicesManager"; @@ -30,6 +30,7 @@ import { _, n_, formatList } from "~/i18n"; import { useActions, useDevices } from "~/queries/storage"; import { sprintf } from "sprintf-js"; import { useInstallerClient } from "~/context/installer"; +import { useLastValid } from "~/hooks/use-last-valid"; /** * Renders information about delete actions @@ -96,30 +97,12 @@ function ActionsList({ manager }: ActionsListProps) { ); } -/** - * Stores the last value that satisfies the given condition. - * Returns undefined until the first valid value is stored. - */ -export function useLastValid(value: T, condition: boolean): T | undefined { - const [initialized, setInitialized] = useState(false); - const lastValidRef = useRef(); - - useEffect(() => { - if (condition) { - lastValidRef.current = value; - if (!initialized) setInitialized(true); - } - }, [value, condition, initialized]); - - return initialized ? lastValidRef.current : undefined; -} - export default function ProposalResultSection() { const client = useInstallerClient(); const system = useDevices("system", { suspense: true }); const staging = useDevices("result", { suspense: true }); const actions = useActions(); - const [shouldUpdate, setShouldUpdate] = useState(false); + const [shouldUpdate, setShouldUpdate] = useState(true); useEffect(() => { return client.onEvent((event) => { @@ -129,7 +112,10 @@ export default function ProposalResultSection() { const result = useLastValid({ system, staging, actions }, shouldUpdate); - if (!result) return; + console.log("su", shouldUpdate); + console.log("result is", result); + + if (result?.actions?.length === 0) return; const devicesManager = new DevicesManager(result.system, result.staging, result.actions); diff --git a/web/src/hooks/use-last-valid.ts b/web/src/hooks/use-last-valid.ts new file mode 100644 index 0000000000..99a8f55147 --- /dev/null +++ b/web/src/hooks/use-last-valid.ts @@ -0,0 +1,45 @@ +/* + * Copyright (c) [2025] SUSE LLC + * + * All Rights Reserved. + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, contact SUSE LLC. + * + * To contact SUSE LLC about this file by physical or electronic mail, you may + * find current contact information at www.suse.com. + */ + +import { useEffect, useRef, useState } from "react"; + +/** + * Custom React hook that returns the most recent `value` from when the + * condition was satisfied. + * + * - Returns `undefined` until the first time `condition` is `true`. + * - When `condition` is `true`, the hook updates its internal reference to the + * given `value`; otherwise, it still references the previously stored value. + */ +export function useLastValid(value: T, condition: boolean): T | undefined { + const [initialized, setInitialized] = useState(false); + const lastValidRef = useRef(); + + useEffect(() => { + if (condition) { + lastValidRef.current = value; + if (!initialized) setInitialized(true); + } + }, [value, condition, initialized]); + + return initialized ? lastValidRef.current : undefined; +} From cb0f760a383a6ca2e9f4abc287bc4ec13388fc80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Mon, 4 Aug 2025 16:35:31 +0100 Subject: [PATCH 28/38] storage: include client id while probing through manager --- rust/agama-lib/src/manager.rs | 15 ++++++++--- rust/agama-lib/src/proxies/manager1.rs | 10 +++++-- rust/agama-lib/src/storage/client.rs | 7 +++-- .../agama-lib/src/storage/proxies/storage1.rs | 5 +++- rust/agama-server/src/manager/web.rs | 26 ++++++++++++++----- rust/agama-server/src/storage/web.rs | 7 +++-- service/lib/agama/dbus/clients/storage.rb | 11 +++++--- service/lib/agama/dbus/manager.rb | 11 +++++--- service/lib/agama/dbus/storage/manager.rb | 14 +++------- service/lib/agama/dbus/with_service_status.rb | 8 ++++++ service/lib/agama/manager.rb | 7 +++-- 11 files changed, 82 insertions(+), 39 deletions(-) diff --git a/rust/agama-lib/src/manager.rs b/rust/agama-lib/src/manager.rs index da7fa8b393..56dfc8cfc7 100644 --- a/rust/agama-lib/src/manager.rs +++ b/rust/agama-lib/src/manager.rs @@ -23,6 +23,7 @@ pub mod http_client; pub use http_client::ManagerHTTPClient; use serde::{Deserialize, Serialize}; +use std::collections::HashMap; use crate::error::ServiceError; use crate::proxies::ServiceStatusProxy; @@ -141,15 +142,21 @@ impl<'a> ManagerClient<'a> { } /// Starts the probing process. - pub async fn probe(&self) -> Result<(), ServiceError> { + pub async fn probe(&self, client_id: String) -> Result<(), ServiceError> { self.wait().await?; - Ok(self.manager_proxy.probe().await?) + Ok(self + .manager_proxy + .probe(HashMap::from([("client_id", &client_id.into())])) + .await?) } /// Starts the reprobing process. - pub async fn reprobe(&self) -> Result<(), ServiceError> { + pub async fn reprobe(&self, client_id: String) -> Result<(), ServiceError> { self.wait().await?; - Ok(self.manager_proxy.reprobe().await?) + Ok(self + .manager_proxy + .reprobe(HashMap::from([("client_id", &client_id.into())])) + .await?) } /// Starts the installation. diff --git a/rust/agama-lib/src/proxies/manager1.rs b/rust/agama-lib/src/proxies/manager1.rs index f852a7fa26..81b50d4e66 100644 --- a/rust/agama-lib/src/proxies/manager1.rs +++ b/rust/agama-lib/src/proxies/manager1.rs @@ -39,10 +39,16 @@ pub trait Manager1 { fn finish(&self, method: &str) -> zbus::Result; /// Probe method - fn probe(&self) -> zbus::Result<()>; + fn probe( + &self, + data: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>, + ) -> zbus::Result<()>; /// Reprobe method - fn reprobe(&self) -> zbus::Result<()>; + fn reprobe( + &self, + data: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>, + ) -> zbus::Result<()>; /// BusyServices property #[zbus(property)] diff --git a/rust/agama-lib/src/storage/client.rs b/rust/agama-lib/src/storage/client.rs index e36ce23cf7..6f40de592d 100644 --- a/rust/agama-lib/src/storage/client.rs +++ b/rust/agama-lib/src/storage/client.rs @@ -150,8 +150,11 @@ impl<'a> StorageClient<'a> { } /// Runs the probing process - pub async fn probe(&self) -> Result<(), ServiceError> { - Ok(self.storage_proxy.probe().await?) + pub async fn probe(&self, client_id: String) -> Result<(), ServiceError> { + Ok(self + .storage_proxy + .probe(HashMap::from([("client_id", &client_id.into())])) + .await?) } /// Runs the reprobing process diff --git a/rust/agama-lib/src/storage/proxies/storage1.rs b/rust/agama-lib/src/storage/proxies/storage1.rs index 4c707f06a0..d8b59af15b 100644 --- a/rust/agama-lib/src/storage/proxies/storage1.rs +++ b/rust/agama-lib/src/storage/proxies/storage1.rs @@ -58,7 +58,10 @@ pub trait Storage1 { fn install(&self) -> zbus::Result<()>; /// Probe method - fn probe(&self) -> zbus::Result<()>; + fn probe( + &self, + data: std::collections::HashMap<&str, &zbus::zvariant::Value<'_>>, + ) -> zbus::Result<()>; /// Reprobe method fn reprobe( diff --git a/rust/agama-server/src/manager/web.rs b/rust/agama-server/src/manager/web.rs index 201352876b..3cc7a49bc4 100644 --- a/rust/agama-server/src/manager/web.rs +++ b/rust/agama-server/src/manager/web.rs @@ -26,6 +26,7 @@ //! * `manager_stream` which offers an stream that emits the manager events coming from D-Bus. use agama_lib::{ + auth::ClientId, error::ServiceError, event, logs, manager::{FinishMethod, InstallationPhase, InstallerStatus, ManagerClient}, @@ -38,9 +39,11 @@ use axum::{ http::{header, status::StatusCode, HeaderMap, HeaderValue}, response::IntoResponse, routing::{get, post}, - Json, Router, + Extension, Json, Router, }; +use std::collections::HashMap; use std::pin::Pin; +use std::sync::Arc; use tokio_stream::{Stream, StreamExt}; use tokio_util::io::ReaderStream; @@ -129,7 +132,10 @@ pub async fn manager_service( ) ) )] -async fn probe_action(State(state): State>) -> Result<(), Error> { +async fn probe_action( + State(state): State>, + Extension(client_id): Extension>, +) -> Result<(), Error> { let dbus = state.dbus.clone(); tokio::spawn(async move { let result = dbus @@ -138,7 +144,7 @@ async fn probe_action(State(state): State>) -> Result<(), Error "/org/opensuse/Agama/Manager1", Some("org.opensuse.Agama.Manager1"), "Probe", - &(), + &HashMap::from([("client_id", client_id.to_string())]), ) .await; if let Err(error) = result { @@ -158,8 +164,11 @@ async fn probe_action(State(state): State>) -> Result<(), Error (status = 200, description = "Probing done.") ) )] -async fn probe_sync_action(State(state): State>) -> Result<(), Error> { - state.manager.probe().await?; +async fn probe_sync_action( + State(state): State>, + Extension(client_id): Extension>, +) -> Result<(), Error> { + state.manager.probe(client_id.to_string()).await?; Ok(()) } @@ -172,8 +181,11 @@ async fn probe_sync_action(State(state): State>) -> Result<(), (status = 200, description = "Re-probing done.") ) )] -async fn reprobe_sync_action(State(state): State>) -> Result<(), Error> { - state.manager.reprobe().await?; +async fn reprobe_sync_action( + State(state): State>, + Extension(client_id): Extension>, +) -> Result<(), Error> { + state.manager.reprobe(client_id.to_string()).await?; Ok(()) } diff --git a/rust/agama-server/src/storage/web.rs b/rust/agama-server/src/storage/web.rs index ae7763b743..fbac63e58c 100644 --- a/rust/agama-server/src/storage/web.rs +++ b/rust/agama-server/src/storage/web.rs @@ -348,8 +348,11 @@ struct SolveModelQuery { ), operation_id = "storage_probe" )] -async fn probe(State(state): State>) -> Result, Error> { - Ok(Json(state.client.probe().await?)) +async fn probe( + State(state): State>, + Extension(client_id): Extension>, +) -> Result, Error> { + Ok(Json(state.client.probe(client_id.to_string()).await?)) } /// Reprobes the storage devices. diff --git a/service/lib/agama/dbus/clients/storage.rb b/service/lib/agama/dbus/clients/storage.rb index fcab970a98..66b00ca95d 100644 --- a/service/lib/agama/dbus/clients/storage.rb +++ b/service/lib/agama/dbus/clients/storage.rb @@ -48,14 +48,17 @@ def service_name # If a block is given, the method returns immediately and the probing is performed in an # asynchronous way. # + # @param data [Hash] Extra data provided to the D-Bus call. # @param done [Proc] Block to execute once the probing is done - def probe(&done) - dbus_object[STORAGE_IFACE].Probe(&done) + def probe(data = {}, &done) + dbus_object[STORAGE_IFACE].Probe(data, &done) end # Reprobes (keeps the current settings). - def reprobe - dbus_object.Reprobe + # + # @param data [Hash] Extra data provided to the D-Bus call. + def reprobe(data = {}) + dbus_object.Reprobe(data) end # Performs the packages installation diff --git a/service/lib/agama/dbus/manager.rb b/service/lib/agama/dbus/manager.rb index e847339478..b6b3c65950 100644 --- a/service/lib/agama/dbus/manager.rb +++ b/service/lib/agama/dbus/manager.rb @@ -63,8 +63,8 @@ def initialize(backend, logger) FINISH_PHASE = 3 dbus_interface MANAGER_INTERFACE do - dbus_method(:Probe, "") { config_phase } - dbus_method(:Reprobe, "") { config_phase(reprobe: true) } + dbus_method(:Probe, "in data:a{sv}") { |data| config_phase(data: data) } + dbus_method(:Reprobe, "in data:a{sv}") { |data| config_phase(reprobe: true, data: data) } dbus_method(:Commit, "") { install_phase } dbus_method(:CanInstall, "out result:b") { can_install? } dbus_method(:CollectLogs, "out tarball_filesystem_path:s") { collect_logs } @@ -76,9 +76,12 @@ def initialize(backend, logger) end # Runs the config phase - def config_phase(reprobe: false) + # + # @param reprobe [Boolean] Whether a reprobe should be done instead of a probe. + # @param data [Hash] Extra data provided to the D-Bus calls. + def config_phase(reprobe: false, data: {}) safe_run do - busy_while { backend.config_phase(reprobe: reprobe) } + busy_while { backend.config_phase(reprobe: reprobe, data: data) } end end diff --git a/service/lib/agama/dbus/storage/manager.rb b/service/lib/agama/dbus/storage/manager.rb index 2384492b96..16f41e2660 100644 --- a/service/lib/agama/dbus/storage/manager.rb +++ b/service/lib/agama/dbus/storage/manager.rb @@ -199,7 +199,9 @@ def deprecated_system # * Methods like #Probe or #Install return nothing. dbus_interface STORAGE_INTERFACE do dbus_signal :Configured, "client_id:s" - dbus_method(:Probe) { probe } + dbus_method(:Probe, "in data:a{sv}") do |data| + busy_request(data) { probe } + end dbus_method(:Reprobe, "in data:a{sv}") do |data| busy_request(data) { probe(keep_config: true) } end @@ -484,16 +486,6 @@ def iscsi_delete(path) # @return [DBus::Storage::Proposal, nil] attr_reader :dbus_proposal - # Executes a request setting the service as busy. - # - # @see BaseObject#request - # - # @param data [Hash] see {BaseObject#request_data}. - # @param block [Proc] - def busy_request(data, &block) - busy_while { request(data, &block) } - end - # Configures storage. # # @param config_json [Hash, nil] Storage config according to the JSON schema. If nil, then diff --git a/service/lib/agama/dbus/with_service_status.rb b/service/lib/agama/dbus/with_service_status.rb index 1e9cb169fd..6a01a65c4f 100644 --- a/service/lib/agama/dbus/with_service_status.rb +++ b/service/lib/agama/dbus/with_service_status.rb @@ -39,6 +39,14 @@ def service_status def busy_while(&block) service_status.busy_while(&block) end + + # Executes a block setting the service as busy, see {BaseObject#request}. + # + # @param data [Hash] see {BaseObject#request_data}. + # @param block [Proc] + def busy_request(data, &block) + busy_while { request(data, &block) } + end end end end diff --git a/service/lib/agama/manager.rb b/service/lib/agama/manager.rb index afcc79686f..f23a1df270 100644 --- a/service/lib/agama/manager.rb +++ b/service/lib/agama/manager.rb @@ -91,10 +91,13 @@ def startup_phase end # Runs the config phase - def config_phase(reprobe: false) + # + # @param reprobe [Boolean] Whether a reprobe should be done instead of a probe. + # @param data [Hash] Extra data provided to the D-Bus calls. + def config_phase(reprobe: false, data: {}) installation_phase.config start_progress_with_descriptions(_("Analyze disks"), _("Configure software")) - progress.step { reprobe ? storage.reprobe : storage.probe } + progress.step { reprobe ? storage.reprobe(data) : storage.probe(data) } progress.step { software.probe } logger.info("Config phase done") From ced6db4bf4e8387b37ac6687b00c61ba2e5b4763 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 5 Aug 2025 11:11:52 +0100 Subject: [PATCH 29/38] Revert "feat(web): move useLastValid hook to its own file" This reverts commit 6e5f9309b987436f0aa2f69daba0fc31ba2f2772. --- .../components/storage/ProposalFailedInfo.tsx | 16 +------ .../storage/ProposalResultSection.tsx | 28 +++++++++--- web/src/hooks/use-last-valid.ts | 45 ------------------- 3 files changed, 23 insertions(+), 66 deletions(-) delete mode 100644 web/src/hooks/use-last-valid.ts diff --git a/web/src/components/storage/ProposalFailedInfo.tsx b/web/src/components/storage/ProposalFailedInfo.tsx index 6560910227..8620d65752 100644 --- a/web/src/components/storage/ProposalFailedInfo.tsx +++ b/web/src/components/storage/ProposalFailedInfo.tsx @@ -20,7 +20,7 @@ * find current contact information at www.suse.com. */ -import React, { useEffect, useState } from "react"; +import React from "react"; import { Alert, Content } from "@patternfly/react-core"; import { IssueSeverity } from "~/types/issues"; import { useApiModel } from "~/hooks/storage/api-model"; @@ -28,8 +28,6 @@ import { useIssues, useConfigErrors } from "~/queries/issues"; import * as partitionUtils from "~/components/storage/utils/partition"; import { _, formatList } from "~/i18n"; import { sprintf } from "sprintf-js"; -import { useInstallerClient } from "~/context/installer"; -import { useLastValid } from "~/hooks/use-last-valid"; const Description = () => { const model = useApiModel({ suspense: true }); @@ -90,21 +88,11 @@ const Description = () => { * - The generated proposal contains no errors. */ export default function ProposalFailedInfo() { - const client = useInstallerClient(); const configErrors = useConfigErrors("storage"); const errors = useIssues("storage").filter((s) => s.severity === IssueSeverity.Error); - const [shouldUpdate, setShouldUpdate] = useState(false); - - const result = useLastValid(errors, shouldUpdate); - - useEffect(() => { - return client.onEvent((event) => { - setShouldUpdate(event.type === "StorageChanged" && event.clientId === client.id); - }); - }, [client]); if (configErrors.length !== 0) return; - if (result && result.length === 0) return; + if (errors.length === 0) return; return ( diff --git a/web/src/components/storage/ProposalResultSection.tsx b/web/src/components/storage/ProposalResultSection.tsx index e75140c3bc..47d8700b2d 100644 --- a/web/src/components/storage/ProposalResultSection.tsx +++ b/web/src/components/storage/ProposalResultSection.tsx @@ -20,7 +20,7 @@ * find current contact information at www.suse.com. */ -import React, { useEffect, useState } from "react"; +import React, { useEffect, useRef, useState } from "react"; import { Alert, ExpandableSection, Stack } from "@patternfly/react-core"; import { Page } from "~/components/core"; import DevicesManager from "~/components/storage/DevicesManager"; @@ -30,7 +30,6 @@ import { _, n_, formatList } from "~/i18n"; import { useActions, useDevices } from "~/queries/storage"; import { sprintf } from "sprintf-js"; import { useInstallerClient } from "~/context/installer"; -import { useLastValid } from "~/hooks/use-last-valid"; /** * Renders information about delete actions @@ -97,12 +96,30 @@ function ActionsList({ manager }: ActionsListProps) { ); } +/** + * Stores the last value that satisfies the given condition. + * Returns undefined until the first valid value is stored. + */ +export function useLastValid(value: T, condition: boolean): T | undefined { + const [initialized, setInitialized] = useState(false); + const lastValidRef = useRef(); + + useEffect(() => { + if (condition) { + lastValidRef.current = value; + if (!initialized) setInitialized(true); + } + }, [value, condition, initialized]); + + return initialized ? lastValidRef.current : undefined; +} + export default function ProposalResultSection() { const client = useInstallerClient(); const system = useDevices("system", { suspense: true }); const staging = useDevices("result", { suspense: true }); const actions = useActions(); - const [shouldUpdate, setShouldUpdate] = useState(true); + const [shouldUpdate, setShouldUpdate] = useState(false); useEffect(() => { return client.onEvent((event) => { @@ -112,10 +129,7 @@ export default function ProposalResultSection() { const result = useLastValid({ system, staging, actions }, shouldUpdate); - console.log("su", shouldUpdate); - console.log("result is", result); - - if (result?.actions?.length === 0) return; + if (!result) return; const devicesManager = new DevicesManager(result.system, result.staging, result.actions); diff --git a/web/src/hooks/use-last-valid.ts b/web/src/hooks/use-last-valid.ts deleted file mode 100644 index 99a8f55147..0000000000 --- a/web/src/hooks/use-last-valid.ts +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright (c) [2025] SUSE LLC - * - * All Rights Reserved. - * - * This program is free software; you can redistribute it and/or modify it - * under the terms of the GNU General Public License as published by the Free - * Software Foundation; either version 2 of the License, or (at your option) - * any later version. - * - * This program is distributed in the hope that it will be useful, but WITHOUT - * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or - * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for - * more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, contact SUSE LLC. - * - * To contact SUSE LLC about this file by physical or electronic mail, you may - * find current contact information at www.suse.com. - */ - -import { useEffect, useRef, useState } from "react"; - -/** - * Custom React hook that returns the most recent `value` from when the - * condition was satisfied. - * - * - Returns `undefined` until the first time `condition` is `true`. - * - When `condition` is `true`, the hook updates its internal reference to the - * given `value`; otherwise, it still references the previously stored value. - */ -export function useLastValid(value: T, condition: boolean): T | undefined { - const [initialized, setInitialized] = useState(false); - const lastValidRef = useRef(); - - useEffect(() => { - if (condition) { - lastValidRef.current = value; - if (!initialized) setInitialized(true); - } - }, [value, condition, initialized]); - - return initialized ? lastValidRef.current : undefined; -} From 76ded0a0e2fb057ad30b176b66ce67452bc5ca53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 5 Aug 2025 11:12:09 +0100 Subject: [PATCH 30/38] Revert "feat(web) PoC of hook for using last valid values" This reverts commit eddad299966e49e768457345d66f942d0d64fd30. --- .../storage/ProposalResultSection.tsx | 52 +++++++++---------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/web/src/components/storage/ProposalResultSection.tsx b/web/src/components/storage/ProposalResultSection.tsx index 47d8700b2d..82213c8128 100644 --- a/web/src/components/storage/ProposalResultSection.tsx +++ b/web/src/components/storage/ProposalResultSection.tsx @@ -20,7 +20,7 @@ * find current contact information at www.suse.com. */ -import React, { useEffect, useRef, useState } from "react"; +import React, { useEffect, useState } from "react"; import { Alert, ExpandableSection, Stack } from "@patternfly/react-core"; import { Page } from "~/components/core"; import DevicesManager from "~/components/storage/DevicesManager"; @@ -30,6 +30,9 @@ import { _, n_, formatList } from "~/i18n"; import { useActions, useDevices } from "~/queries/storage"; import { sprintf } from "sprintf-js"; import { useInstallerClient } from "~/context/installer"; +import { isNullish } from "radashi"; +import { StorageDevice } from "~/types/storage"; +import { Action } from "~/api/storage/types"; /** * Renders information about delete actions @@ -96,42 +99,39 @@ function ActionsList({ manager }: ActionsListProps) { ); } -/** - * Stores the last value that satisfies the given condition. - * Returns undefined until the first valid value is stored. - */ -export function useLastValid(value: T, condition: boolean): T | undefined { - const [initialized, setInitialized] = useState(false); - const lastValidRef = useRef(); - - useEffect(() => { - if (condition) { - lastValidRef.current = value; - if (!initialized) setInitialized(true); - } - }, [value, condition, initialized]); - - return initialized ? lastValidRef.current : undefined; -} +type Result = { + system: StorageDevice[]; + staging: StorageDevice[]; + actions: Action[]; +}; -export default function ProposalResultSection() { +const useProposalResult = () => { const client = useInstallerClient(); const system = useDevices("system", { suspense: true }); const staging = useDevices("result", { suspense: true }); const actions = useActions(); - const [shouldUpdate, setShouldUpdate] = useState(false); + const [result, setResult] = useState(); + + useEffect(() => { + if (isNullish(result) && [system, staging, actions].every((e) => e)) { + setResult({ system, staging, actions }); + } + }, [result, system, staging, actions]); useEffect(() => { return client.onEvent((event) => { - setShouldUpdate(event.type === "StorageChanged" && event.clientId === client.id); + event.type === "StorageChanged" && + event.clientId === client.id && + setResult({ system, staging, actions }); }); - }, [client]); + }, [client, result, system, staging, actions]); - const result = useLastValid({ system, staging, actions }, shouldUpdate); - - if (!result) return; + return { system, staging, actions }; +}; - const devicesManager = new DevicesManager(result.system, result.staging, result.actions); +export default function ProposalResultSection() { + const { system, staging, actions } = useProposalResult(); + const devicesManager = new DevicesManager(system, staging, actions); return ( Date: Tue, 5 Aug 2025 11:12:24 +0100 Subject: [PATCH 31/38] Revert "feat(web): re-render result section only when proceed" This reverts commit 65067473fdd33d9ae61b1abdc625dfbe1f488a55. --- web/src/components/storage/ProposalPage.tsx | 4 +- .../storage/ProposalResultSection.tsx | 54 ++++++++----------- 2 files changed, 24 insertions(+), 34 deletions(-) diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index 8d24c2c997..cd3b786ee5 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -181,6 +181,8 @@ function ProposalEmptyState(): React.ReactNode { function ProposalSections(): React.ReactNode { const model = useConfigModel({ suspense: true }); + const systemErrors = useSystemErrors("storage"); + const hasResult = !systemErrors.length; return ( @@ -215,7 +217,7 @@ function ProposalSections(): React.ReactNode { )} - + {hasResult && } ); } diff --git a/web/src/components/storage/ProposalResultSection.tsx b/web/src/components/storage/ProposalResultSection.tsx index 82213c8128..6c97fcc5f9 100644 --- a/web/src/components/storage/ProposalResultSection.tsx +++ b/web/src/components/storage/ProposalResultSection.tsx @@ -20,8 +20,8 @@ * find current contact information at www.suse.com. */ -import React, { useEffect, useState } from "react"; -import { Alert, ExpandableSection, Stack } from "@patternfly/react-core"; +import React, { useState } from "react"; +import { Alert, ExpandableSection, Skeleton, Stack } from "@patternfly/react-core"; import { Page } from "~/components/core"; import DevicesManager from "~/components/storage/DevicesManager"; import ProposalResultTable from "~/components/storage/ProposalResultTable"; @@ -29,10 +29,20 @@ import { ProposalActionsDialog } from "~/components/storage"; import { _, n_, formatList } from "~/i18n"; import { useActions, useDevices } from "~/queries/storage"; import { sprintf } from "sprintf-js"; -import { useInstallerClient } from "~/context/installer"; -import { isNullish } from "radashi"; -import { StorageDevice } from "~/types/storage"; -import { Action } from "~/api/storage/types"; + +/** + * @todo Create a component for rendering a customized skeleton + */ +const ResultSkeleton = () => ( + + + + + +); /** * Renders information about delete actions @@ -99,40 +109,18 @@ function ActionsList({ manager }: ActionsListProps) { ); } -type Result = { - system: StorageDevice[]; - staging: StorageDevice[]; - actions: Action[]; +export type ProposalResultSectionProps = { + isLoading?: boolean; }; -const useProposalResult = () => { - const client = useInstallerClient(); +export default function ProposalResultSection({ isLoading = false }: ProposalResultSectionProps) { const system = useDevices("system", { suspense: true }); const staging = useDevices("result", { suspense: true }); const actions = useActions(); - const [result, setResult] = useState(); - - useEffect(() => { - if (isNullish(result) && [system, staging, actions].every((e) => e)) { - setResult({ system, staging, actions }); - } - }, [result, system, staging, actions]); - - useEffect(() => { - return client.onEvent((event) => { - event.type === "StorageChanged" && - event.clientId === client.id && - setResult({ system, staging, actions }); - }); - }, [client, result, system, staging, actions]); - - return { system, staging, actions }; -}; - -export default function ProposalResultSection() { - const { system, staging, actions } = useProposalResult(); const devicesManager = new DevicesManager(system, staging, actions); + if (isLoading) return ; + return ( Date: Tue, 5 Aug 2025 12:26:13 +0100 Subject: [PATCH 32/38] web: use blocking popup for out of sync alert --- web/src/assets/styles/index.scss | 5 ++ .../components/core/AlertOutOfSync.test.tsx | 21 +++---- web/src/components/core/AlertOutOfSync.tsx | 61 ++++++------------- 3 files changed, 33 insertions(+), 54 deletions(-) diff --git a/web/src/assets/styles/index.scss b/web/src/assets/styles/index.scss index 29723f3686..8826719701 100644 --- a/web/src/assets/styles/index.scss +++ b/web/src/assets/styles/index.scss @@ -529,3 +529,8 @@ button:focus-visible { .storage-structure:has(> li:nth-child(2)) span.action-text { display: none; } + +.agm-backdrop-gray-and-blur { + background-color: rgb(0 0 0 / 30%); + backdrop-filter: grayscale(100%) blur(4px); +} diff --git a/web/src/components/core/AlertOutOfSync.test.tsx b/web/src/components/core/AlertOutOfSync.test.tsx index 9e2f76b1aa..b607a3cd61 100644 --- a/web/src/components/core/AlertOutOfSync.test.tsx +++ b/web/src/components/core/AlertOutOfSync.test.tsx @@ -21,7 +21,7 @@ */ import React, { act } from "react"; -import { screen } from "@testing-library/dom"; +import { screen, within } from "@testing-library/dom"; import { installerRender, plainRender } from "~/test-utils"; import AlertOutOfSync from "./AlertOutOfSync"; @@ -82,31 +82,29 @@ describe("AlertOutOfSync", () => { installerRender(); // Should not render the alert initially - expect(screen.queryByText("Info alert:")).toBeNull(); + expect(screen.queryByRole("dialog")).toBeNull(); // Simulate a change event for a different scope act(() => { eventCallback({ type: "NotWatchedChanged", clientId: "other-client" }); }); - expect(screen.queryByText("Info alert:")).toBeNull(); + expect(screen.queryByRole("dialog")).toBeNull(); // Simulate a change event for the subscribed scope, from current client act(() => { eventCallback({ type: "WatchedChanged", clientId: "current-client" }); }); - expect(screen.queryByText("Info alert:")).toBeNull(); + expect(screen.queryByRole("dialog")).toBeNull(); // Simulate a change event for the subscribed scope, from different client act(() => { eventCallback({ type: "WatchedChanged", clientId: "other-client" }); }); - screen.getByText("Info alert:"); - screen.getByText("Configuration out of sync"); - screen.getByText(/issues or data loss/); - screen.getByRole("button", { name: "Reload now" }); + const dialog = screen.getByRole("dialog", { name: "Configuration out of sync" }); + within(dialog).getByRole("button", { name: "Reload now" }); }); it("dismisses automatically the alert on matching changes event from current client for subscribed scope", () => { @@ -123,17 +121,14 @@ describe("AlertOutOfSync", () => { eventCallback({ type: "WatchedChanged", clientId: "other-client" }); }); - screen.getByText("Info alert:"); - screen.getByText("Configuration out of sync"); - screen.getByText(/issues or data loss/); - screen.getByRole("button", { name: "Reload now" }); + screen.getByRole("dialog", { name: "Configuration out of sync" }); // Simulate a change event for the subscribed scope, from current client act(() => { eventCallback({ type: "WatchedChanged", clientId: "current-client" }); }); - expect(screen.queryByText("Info alert:")).toBeNull(); + expect(screen.queryByRole("dialog")).toBeNull(); }); it("triggers a location relaod when clicking on `Reload now`", async () => { diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx index 773bed87c2..220f16662c 100644 --- a/web/src/components/core/AlertOutOfSync.tsx +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -21,20 +21,14 @@ */ import React, { useEffect, useState } from "react"; -import { - Alert, - AlertActionCloseButton, - AlertGroup, - AlertProps, - Button, - Content, -} from "@patternfly/react-core"; +import { Content } from "@patternfly/react-core"; import { useInstallerClient } from "~/context/installer"; import { isEmpty } from "radashi"; import { _ } from "~/i18n"; import { locationReload } from "~/utils"; +import Popup, { PopupProps } from "~/components/core/Popup"; -type AlertOutOfSyncProps = Partial & { +type AlertOutOfSyncProps = Partial & { /** * The scope to listen for change events on (e.g., `SoftwareProposal`, * `L10nConfig`). @@ -46,14 +40,12 @@ type AlertOutOfSyncProps = Partial & { * Reactive alert shown when the configuration for a given scope has been * changed externally. * - * It warns that the interface may be out of sync and recommends reloading - * before continuing to avoid issues and data loss. Reloading is intentionally - * left up to the user rather than forced automatically, to prevent confusion - * caused by unexpected refreshes. + * It warns that the interface may be out of sync and forces reloading + * before continuing to avoid issues and data loss. * * It works by listening for "Changed" events on the specified scope: * - * - Displays a toast alert if the event originates from a different client + * - Displays a popup if the event originates from a different client * (based on client ID). * - Automatically dismisses the alert if a subsequent event originates from * the current client. @@ -84,32 +76,19 @@ export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncP const title = _("Configuration out of sync"); return ( - - {active && ( - setActive(false)} - /> - } - {...alertProps} - key={`${scope}-out-of-sync`} - > - - {_( - "The configuration has been updated externally. \ -Reload the page to get the latest data and avoid issues or data loss.", - )} - - - - )} - + + {_("The configuration has been updated externally.")} + + {_("The page must be reloaded to get the latest data and avoid issues or data loss.")} + + + {_("Reload now")} + + ); } From b5f77f6ac5a06df2544ae47a21f837ce0a751e00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Tue, 5 Aug 2025 12:27:26 +0100 Subject: [PATCH 33/38] web: move alert to app - The out-of-sync alert is shown in all pages. --- web/src/App.test.tsx | 5 ++++- web/src/App.tsx | 9 ++++++++- web/src/components/storage/ProposalPage.tsx | 2 -- 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index d7d1f58ec9..b796dba67d 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -90,7 +90,10 @@ describe("App", () => { // setting the language through a cookie document.cookie = "agamaLang=en-US; path=/;"; (createClient as jest.Mock).mockImplementation(() => { - return { isConnected: () => true }; + return { + onEvent: jest.fn(), + isConnected: () => true, + }; }); mockProducts = [tumbleweed, microos]; diff --git a/web/src/App.tsx b/web/src/App.tsx index f223b5e60e..6fcb657bf3 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -31,6 +31,7 @@ import { useDeprecatedChanges } from "~/queries/storage"; import { ROOT, PRODUCT } from "~/routes/paths"; import { InstallationPhase } from "~/types/status"; import { useQueryClient } from "@tanstack/react-query"; +import AlertOutOfSync from "~/components/core/AlertOutOfSync"; /** * Main application component. @@ -97,7 +98,13 @@ function App() { return ; }; - return ; + return ( + <> + {/* So far, only the storage backend is able to detect external changes.*/} + + + + ); } export default App; diff --git a/web/src/components/storage/ProposalPage.tsx b/web/src/components/storage/ProposalPage.tsx index cd3b786ee5..ef1f65ef86 100644 --- a/web/src/components/storage/ProposalPage.tsx +++ b/web/src/components/storage/ProposalPage.tsx @@ -45,7 +45,6 @@ import ProposalFailedInfo from "./ProposalFailedInfo"; import ProposalResultSection from "./ProposalResultSection"; import ProposalTransactionalInfo from "./ProposalTransactionalInfo"; import UnsupportedModelInfo from "./UnsupportedModelInfo"; -import AlertOutOfSync from "~/components/core/AlertOutOfSync"; import { useAvailableDevices } from "~/hooks/storage/system"; import { useResetConfigMutation, @@ -260,7 +259,6 @@ export default function ProposalPage(): React.ReactNode { {_("Storage")} - {isDeprecated && } {!isDeprecated && !showSections && } {!isDeprecated && showSections && } From de5b6a6ff87ed193bec4b30aec89b4a69a5692ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Aug 2025 10:49:52 +0100 Subject: [PATCH 34/38] web: improvements in alert --- web/src/components/core/AlertOutOfSync.tsx | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/web/src/components/core/AlertOutOfSync.tsx b/web/src/components/core/AlertOutOfSync.tsx index 220f16662c..29e34fffeb 100644 --- a/web/src/components/core/AlertOutOfSync.tsx +++ b/web/src/components/core/AlertOutOfSync.tsx @@ -28,7 +28,7 @@ import { _ } from "~/i18n"; import { locationReload } from "~/utils"; import Popup, { PopupProps } from "~/components/core/Popup"; -type AlertOutOfSyncProps = Partial & { +type AlertOutOfSyncProps = Partial> & { /** * The scope to listen for change events on (e.g., `SoftwareProposal`, * `L10nConfig`). @@ -37,18 +37,13 @@ type AlertOutOfSyncProps = Partial & { }; /** - * Reactive alert shown when the configuration for a given scope has been - * changed externally. + * Reactive alert shown when the configuration for a given scope has been changed externally. * - * It warns that the interface may be out of sync and forces reloading - * before continuing to avoid issues and data loss. + * It warns that the interface may be out of sync and forces reloading before continuing to avoid + * issues and data loss. * - * It works by listening for "Changed" events on the specified scope: - * - * - Displays a popup if the event originates from a different client - * (based on client ID). - * - Automatically dismisses the alert if a subsequent event originates from - * the current client. + * It works by listening for "Changed" events on the specified scope and displays a popup if the + * event originates from a different client (based on client ID). * * @example * ```tsx @@ -84,7 +79,7 @@ export default function AlertOutOfSync({ scope, ...alertProps }: AlertOutOfSyncP > {_("The configuration has been updated externally.")} - {_("The page must be reloaded to get the latest data and avoid issues or data loss.")} + {_("Reloading is required to get the latest data and avoid issues or data loss.")} {_("Reload now")} From 12947af238240ce6df4e0eef36e7836d009cd80f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Aug 2025 13:57:02 +0100 Subject: [PATCH 35/38] rust: changelog --- rust/package/agama.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rust/package/agama.changes b/rust/package/agama.changes index 9ab09ad3e7..67418416cf 100644 --- a/rust/package/agama.changes +++ b/rust/package/agama.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Wed Aug 6 12:52:41 UTC 2025 - José Iván López González + +- Emit HTTP event when storage is configured, including the client + id (gh#agama-project/agama#2640). + ------------------------------------------------------------------- Mon Aug 4 09:29:29 UTC 2025 - Knut Anderssen From 38489a7906c8b82834318ade476f64a50f2a2ef4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Aug 2025 13:57:21 +0100 Subject: [PATCH 36/38] service: changelog --- service/package/rubygem-agama-yast.changes | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/service/package/rubygem-agama-yast.changes b/service/package/rubygem-agama-yast.changes index 0633a7d19f..a4176587dd 100644 --- a/service/package/rubygem-agama-yast.changes +++ b/service/package/rubygem-agama-yast.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Wed Aug 6 12:47:13 UTC 2025 - José Iván López González + +- Emit D-Bus signal when storage is configured, including the + client id (gh#agama-project/agama#2640). + ------------------------------------------------------------------- Tue Aug 5 14:01:23 UTC 2025 - José Iván López González From 9b587f194e07d9f3d239e7581920072c5e953878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Aug 2025 13:57:36 +0100 Subject: [PATCH 37/38] web: changelog --- web/package/agama-web-ui.changes | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/web/package/agama-web-ui.changes b/web/package/agama-web-ui.changes index 7939dae67b..aee998cd60 100644 --- a/web/package/agama-web-ui.changes +++ b/web/package/agama-web-ui.changes @@ -1,8 +1,8 @@ ------------------------------------------------------------------- Thu Jul 31 09:13:09 UTC 2025 - David Diaz -- Allow displaying out-of-sync alerts by listening to changes - events (gh#agama-project/agama#2630). +- Block UI if storage is configured by any other client + (gh#agama-project/agama#2640). ------------------------------------------------------------------- Fri Jul 25 19:42:18 UTC 2025 - David Diaz From 8cc6ebc9c7e135b7c248ac0e0e41308eed049084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Iv=C3=A1n=20L=C3=B3pez=20Gonz=C3=A1lez?= Date: Wed, 6 Aug 2025 15:18:22 +0100 Subject: [PATCH 38/38] storage: fix tests - EFI partition does not require 1 GiB by default (only if BLS was explicit selected in the control file). --- service/test/agama/storage/autoyast_proposal_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/service/test/agama/storage/autoyast_proposal_test.rb b/service/test/agama/storage/autoyast_proposal_test.rb index 3a71918e2d..24bdbb6851 100644 --- a/service/test/agama/storage/autoyast_proposal_test.rb +++ b/service/test/agama/storage/autoyast_proposal_test.rb @@ -36,7 +36,7 @@ before do mock_storage(devicegraph: scenario) - allow(Y2Storage::StorageManager.instance).to receive(:arch).and_return(arch) + allow(Y2Storage::Arch).to receive(:new).and_return(arch) end let(:scenario) { "windows-linux-pc.yml" } @@ -121,7 +121,7 @@ def root_filesystem(disk) expect(efi).to have_attributes( filesystem_type: Y2Storage::Filesystems::Type::VFAT, filesystem_mountpoint: "/boot/efi", - size: 1.GiB + size: 512.MiB ) expect(root).to have_attributes( @@ -266,7 +266,7 @@ def root_filesystem(disk) filesystem_type: Y2Storage::Filesystems::Type::VFAT, filesystem_mountpoint: "/boot/efi", id: Y2Storage::PartitionId::ESP, - size: 1.GiB + size: 512.MiB ) end