Skip to content

Commit 9643090

Browse files
authored
Add a new API to handle issues (#2775)
Add a new service to handle issues in the new API. The new service allows any other one to register its issues. ## Registering issues A service that needs to register issues, receives an `agama_utils::actor::Handler<agama_utils::issue::Service>`. It can update the list of issues through the [Update](https://github.com/agama-project/agama/pull/2775/files#diff-473781875264b7e4d4e408a101a973115f312b6305deff38a2d6e135cdc88082R33) action. ## Dealing with D-Bus services The new service reads the issues from the D-Bus services too. When booting, it reads the existing issues and starts listening for changes on the `org.opensuse.Agama1.Issues` interfaces. ## Localization issues As part of this PR, the localization service reports issues now in case of a locale, keymap or timezone is unknown. ## Adapted UI The code to deal with the issues in the web UI has been adapted to the new API. ## Other smaller changes - Make `*_db` functions in `ModelAdapter` to return references instead of mutable references. - In a `TimezoneEntry`, rename `code` to `id` for consistency. Additionally, use `TimezoneId` instead of a string.
2 parents 8d71d27 + 39eb1a0 commit 9643090

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

60 files changed

+1243
-903
lines changed

rust/Cargo.lock

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust/agama-l10n/src/message.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl<T> SetConfig<T> {
7373
pub struct GetProposal;
7474

7575
impl Message for GetProposal {
76-
type Reply = Proposal;
76+
type Reply = Option<Proposal>;
7777
}
7878

7979
pub struct Install;

rust/agama-l10n/src/model.rs

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,13 @@ use std::{env, fs::OpenOptions, io::Write, process::Command};
3939
/// tests.
4040
pub trait ModelAdapter: Send + 'static {
4141
/// Locales database.
42-
fn locales_db(&mut self) -> &mut LocalesDatabase;
42+
fn locales_db(&self) -> &LocalesDatabase;
4343

4444
/// Timezones database.
45-
fn timezones_db(&mut self) -> &mut TimezonesDatabase;
45+
fn timezones_db(&self) -> &TimezonesDatabase;
4646

4747
/// Keymaps database.
48-
fn keymaps_db(&mut self) -> &mut KeymapsDatabase;
48+
fn keymaps_db(&self) -> &KeymapsDatabase;
4949

5050
/// Current system locale.
5151
fn locale(&self) -> LocaleId;
@@ -67,9 +67,9 @@ pub trait ModelAdapter: Send + 'static {
6767
/// at the end of the installation.
6868
fn install(
6969
&self,
70-
_locale: LocaleId,
71-
_keymap: KeymapId,
72-
_timezone: TimezoneId,
70+
_locale: &LocaleId,
71+
_keymap: &KeymapId,
72+
_timezone: &TimezoneId,
7373
) -> Result<(), service::Error> {
7474
Ok(())
7575
}
@@ -110,15 +110,15 @@ impl Model {
110110
}
111111

112112
impl ModelAdapter for Model {
113-
fn locales_db(&mut self) -> &mut LocalesDatabase {
114-
&mut self.locales_db
113+
fn locales_db(&self) -> &LocalesDatabase {
114+
&self.locales_db
115115
}
116-
fn timezones_db(&mut self) -> &mut TimezonesDatabase {
117-
&mut self.timezones_db
116+
fn timezones_db(&self) -> &TimezonesDatabase {
117+
&self.timezones_db
118118
}
119119

120-
fn keymaps_db(&mut self) -> &mut KeymapsDatabase {
121-
&mut self.keymaps_db
120+
fn keymaps_db(&self) -> &KeymapsDatabase {
121+
&self.keymaps_db
122122
}
123123

124124
fn keymap(&self) -> Result<KeymapId, service::Error> {
@@ -172,9 +172,9 @@ impl ModelAdapter for Model {
172172

173173
fn install(
174174
&self,
175-
locale: LocaleId,
176-
keymap: KeymapId,
177-
timezone: TimezoneId,
175+
locale: &LocaleId,
176+
keymap: &KeymapId,
177+
timezone: &TimezoneId,
178178
) -> Result<(), service::Error> {
179179
const ROOT: &str = "/mnt";
180180
const VCONSOLE_CONF: &str = "/etc/vconsole.conf";

rust/agama-l10n/src/model/locale.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ impl LocalesDatabase {
5959
#[cfg(test)]
6060
pub fn with_entries(data: &[LocaleEntry]) -> Self {
6161
Self {
62-
known_locales: vec![],
62+
known_locales: data.iter().map(|l| l.id.clone()).collect(),
6363
locales: data.to_vec(),
6464
}
6565
}

rust/agama-l10n/src/model/timezone.rs

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,15 @@
2020

2121
//! This module provides support for reading the timezones database.
2222
23-
use agama_locale_data::territory::Territories;
24-
use agama_locale_data::timezone_part::TimezoneIdParts;
23+
use agama_locale_data::{territory::Territories, timezone_part::TimezoneIdParts, TimezoneId};
2524
use serde::Serialize;
2625
use std::collections::HashMap;
2726

2827
/// Represents a timezone, including each part as localized.
2928
#[derive(Clone, Debug, Serialize, utoipa::ToSchema)]
3029
pub struct TimezoneEntry {
3130
/// Timezone identifier (e.g. "Atlantic/Canary").
32-
pub code: String,
31+
pub id: TimezoneId,
3332
/// Localized parts (e.g., "Atlántico", "Canarias").
3433
pub parts: Vec<String>,
3534
/// Localized name of the territory this timezone is associated to
@@ -62,8 +61,8 @@ impl TimezonesDatabase {
6261
}
6362

6463
/// Determines whether a timezone exists in the database.
65-
pub fn exists(&self, timezone: &String) -> bool {
66-
self.timezones.iter().any(|t| &t.code == timezone)
64+
pub fn exists(&self, timezone: &TimezoneId) -> bool {
65+
self.timezones.iter().any(|t| &t.id == timezone)
6766
}
6867

6968
/// Returns the list of timezones.
@@ -87,15 +86,17 @@ impl TimezonesDatabase {
8786
let ret = timezones
8887
.into_iter()
8988
.filter_map(|tz| {
90-
let parts = translate_parts(&tz, ui_language, &tz_parts);
91-
let country = translate_country(&tz, ui_language, &tz_countries, &territories);
89+
tz.parse::<TimezoneId>()
90+
.inspect_err(|e| println!("Ignoring timezone {tz}: {e}"))
91+
.ok()
92+
})
93+
.filter_map(|id| {
94+
let parts = translate_parts(id.as_str(), ui_language, &tz_parts);
95+
let country =
96+
translate_country(id.as_str(), ui_language, &tz_countries, &territories);
9297
match country {
93-
None if !COUNTRYLESS.contains(&tz.as_str()) => None,
94-
_ => Some(TimezoneEntry {
95-
code: tz,
96-
parts,
97-
country,
98-
}),
98+
None if !COUNTRYLESS.contains(&id.as_str()) => None,
99+
_ => Some(TimezoneEntry { id, parts, country }),
99100
}
100101
})
101102
.collect();
@@ -143,9 +144,9 @@ mod tests {
143144
let found_timezones = db.entries();
144145
let found = found_timezones
145146
.iter()
146-
.find(|tz| tz.code == "Europe/Berlin")
147+
.find(|tz| tz.id.as_str() == "Europe/Berlin")
147148
.unwrap();
148-
assert_eq!(&found.code, "Europe/Berlin");
149+
assert_eq!(found.id.as_str(), "Europe/Berlin");
149150
assert_eq!(
150151
found.parts,
151152
vec!["Europa".to_string(), "Berlín".to_string()]
@@ -157,7 +158,11 @@ mod tests {
157158
fn test_read_timezone_without_country() {
158159
let mut db = TimezonesDatabase::new();
159160
db.read("es").unwrap();
160-
let timezone = db.entries().iter().find(|tz| tz.code == "UTC").unwrap();
161+
let timezone = db
162+
.entries()
163+
.iter()
164+
.find(|tz| tz.id.as_str() == "UTC")
165+
.unwrap();
161166
assert_eq!(timezone.country, None);
162167
}
163168

@@ -168,7 +173,7 @@ mod tests {
168173
let timezone = db
169174
.entries()
170175
.iter()
171-
.find(|tz| tz.code == "Europe/Kiev")
176+
.find(|tz| tz.id.as_str() == "Europe/Kiev")
172177
.unwrap();
173178
assert_eq!(timezone.country, Some("Ukraine".to_string()));
174179
}
@@ -177,7 +182,9 @@ mod tests {
177182
fn test_timezone_exists() {
178183
let mut db = TimezonesDatabase::new();
179184
db.read("es").unwrap();
180-
assert!(db.exists(&"Atlantic/Canary".to_string()));
181-
assert!(!db.exists(&"Unknown/Unknown".to_string()));
185+
let canary = "Atlantic/Canary".parse().unwrap();
186+
let unknown = "Unknown/Unknown".parse().unwrap();
187+
assert!(db.exists(&canary));
188+
assert!(!db.exists(&unknown));
182189
}
183190
}

rust/agama-l10n/src/service.rs

Lines changed: 94 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ use crate::{
2828
system_info::SystemInfo,
2929
};
3030
use agama_locale_data::{InvalidKeymapId, InvalidLocaleId, InvalidTimezoneId, KeymapId, LocaleId};
31-
use agama_utils::actor::{self, Actor, MessageHandler};
31+
use agama_utils::{
32+
actor::{self, Actor, Handler, MessageHandler},
33+
issue::{self, Issue},
34+
};
3235
use async_trait::async_trait;
3336

3437
#[derive(thiserror::Error, Debug)]
@@ -39,11 +42,11 @@ pub enum Error {
3942
UnknownKeymap(KeymapId),
4043
#[error("Unknown timezone: {0}")]
4144
UnknownTimezone(String),
42-
#[error("Invalid locale: {0}")]
45+
#[error(transparent)]
4346
InvalidLocale(#[from] InvalidLocaleId),
44-
#[error("Invalid keymap: {0}")]
47+
#[error(transparent)]
4548
InvalidKeymap(#[from] InvalidKeymapId),
46-
#[error("Invalid timezone")]
49+
#[error(transparent)]
4750
InvalidTimezone(#[from] InvalidTimezoneId),
4851
#[error("l10n service could not send the event")]
4952
Event,
@@ -53,31 +56,92 @@ pub enum Error {
5356
IO(#[from] std::io::Error),
5457
#[error(transparent)]
5558
Generic(#[from] anyhow::Error),
59+
#[error("There is no proposal for localization")]
60+
MissingProposal,
5661
}
5762

63+
/// Localization service.
64+
///
65+
/// It is responsible for handling the localization part of the installation:
66+
///
67+
/// * Reads the list of known locales, keymaps and timezones.
68+
/// * Keeps track of the localization settings of the underlying system (the installer).
69+
/// * Holds the user configuration.
70+
/// * Applies the user configuration at the end of the installation.
5871
pub struct Service {
5972
state: State,
6073
model: Box<dyn ModelAdapter + Send + 'static>,
74+
issues: Handler<issue::Service>,
6175
events: event::Sender,
6276
}
6377

6478
struct State {
6579
system: SystemInfo,
6680
config: ExtendedConfig,
81+
proposal: Option<Proposal>,
6782
}
6883

6984
impl Service {
70-
pub fn new<T: ModelAdapter + Send + 'static>(mut model: T, events: event::Sender) -> Service {
71-
let system = SystemInfo::read_from(&mut model);
85+
pub fn new<T: ModelAdapter + Send + 'static>(
86+
model: T,
87+
issues: Handler<issue::Service>,
88+
events: event::Sender,
89+
) -> Service {
90+
let system = SystemInfo::read_from(&model);
7291
let config = ExtendedConfig::new_from(&system);
73-
let state = State { system, config };
92+
let proposal = (&config).into();
93+
let state = State {
94+
system,
95+
config,
96+
proposal: Some(proposal),
97+
};
7498

7599
Self {
76100
state,
77101
model: Box::new(model),
102+
issues,
78103
events,
79104
}
80105
}
106+
107+
/// Returns configuration issues.
108+
///
109+
/// It returns an issue for each unknown element (locale, keymap and timezone).
110+
fn find_issues(&self) -> Vec<Issue> {
111+
let config = &self.state.config;
112+
let mut issues = vec![];
113+
if !self.model.locales_db().exists(&config.locale) {
114+
issues.push(Issue {
115+
description: format!("Locale '{}' is unknown", &config.locale),
116+
details: None,
117+
source: issue::IssueSource::Config,
118+
severity: issue::IssueSeverity::Error,
119+
kind: "unknown_locale".to_string(),
120+
});
121+
}
122+
123+
if !self.model.keymaps_db().exists(&config.keymap) {
124+
issues.push(Issue {
125+
description: format!("Keymap '{}' is unknown", &config.keymap),
126+
details: None,
127+
source: issue::IssueSource::Config,
128+
severity: issue::IssueSeverity::Error,
129+
kind: "unknown_keymap".to_string(),
130+
});
131+
}
132+
133+
if !self.model.timezones_db().exists(&config.timezone) {
134+
issues.push(Issue {
135+
description: format!("Timezone '{}' is unknown", &config.timezone),
136+
details: None,
137+
source: issue::IssueSource::Config,
138+
severity: issue::IssueSeverity::Error,
139+
kind: "unknown_timezone".to_string(),
140+
});
141+
}
142+
143+
issues
144+
}
81145
}
82146

83147
impl Actor for Service {
@@ -121,28 +185,43 @@ impl MessageHandler<message::GetConfig> for Service {
121185
impl MessageHandler<message::SetConfig<Config>> for Service {
122186
async fn handle(&mut self, message: message::SetConfig<Config>) -> Result<(), Error> {
123187
let merged = self.state.config.merge(&message.config)?;
124-
if merged != self.state.config {
125-
self.state.config = merged;
126-
_ = self.events.send(Event::ProposalChanged);
188+
if merged == self.state.config {
189+
return Ok(());
127190
}
191+
192+
self.state.config = merged;
193+
let issues = self.find_issues();
194+
195+
self.state.proposal = if issues.is_empty() {
196+
Some((&self.state.config).into())
197+
} else {
198+
None
199+
};
200+
201+
_ = self
202+
.issues
203+
.cast(issue::message::Update::new("localization", issues));
204+
_ = self.events.send(Event::ProposalChanged);
128205
Ok(())
129206
}
130207
}
131208

132209
#[async_trait]
133210
impl MessageHandler<message::GetProposal> for Service {
134-
async fn handle(&mut self, _message: message::GetProposal) -> Result<Proposal, Error> {
135-
Ok((&self.state.config).into())
211+
async fn handle(&mut self, _message: message::GetProposal) -> Result<Option<Proposal>, Error> {
212+
Ok(self.state.proposal.clone())
136213
}
137214
}
138215

139216
#[async_trait]
140217
impl MessageHandler<message::Install> for Service {
141218
async fn handle(&mut self, _message: message::Install) -> Result<(), Error> {
142-
let proposal: Proposal = (&self.state.config).into();
219+
let Some(proposal) = &self.state.proposal else {
220+
return Err(Error::MissingProposal);
221+
};
222+
143223
self.model
144-
.install(proposal.locale, proposal.keymap, proposal.timezone)
145-
.unwrap();
224+
.install(&proposal.locale, &proposal.keymap, &proposal.timezone)?;
146225
Ok(())
147226
}
148227
}

0 commit comments

Comments
 (0)