Skip to content
This repository was archived by the owner on Apr 29, 2024. It is now read-only.

Commit 7cc0cf3

Browse files
committed
Add updates subcommand
This implements checks for possible updates to the knowledge base from external data. At the moment there are two checks: has a platform_issues bug been closed as a dupe of another bug, and is there a `see_also` entry in one of the linked bugs that isn't in the `breakage` column. To be really useful there might need to be a way to allow list known issues.
1 parent 9fe0232 commit 7cc0cf3

File tree

3 files changed

+263
-3
lines changed

3 files changed

+263
-3
lines changed

tools/kbcheck/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
pub mod bugzilla;
22
pub mod data;
33
pub mod entry;
4+
pub mod updates;
45
pub mod validate;

tools/kbcheck/src/main.rs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use clap::{Parser, Subcommand};
2-
use kbcheck::{data, validate};
3-
use miette::{Diagnostic, IntoDiagnostic, Result};
2+
use kbcheck::{data, updates, validate};
3+
use miette::{Diagnostic, Result};
44
use std::collections::BTreeMap;
55
use std::path::{Path, PathBuf};
66

@@ -20,6 +20,7 @@ enum Commands {
2020
Tags,
2121
/// Validate data files against schema
2222
Validate,
23+
Updates,
2324
}
2425

2526
fn get_tags(root_path: &Path) -> Result<BTreeMap<String, (String, u64)>> {
@@ -66,13 +67,26 @@ fn validate(root_path: &Path) -> Result<()> {
6667
}
6768
}
6869

70+
fn updates(root_path: &Path) -> Result<()> {
71+
let updates = updates::check_updates(root_path)?;
72+
if !updates.is_empty() {
73+
for (path, updates) in updates.iter() {
74+
println!("{}", path.display());
75+
for update in updates.iter() {
76+
println!(" {}\n Try: {}", update.error, update.suggestion);
77+
}
78+
}
79+
}
80+
Ok(())
81+
}
82+
6983
fn main() -> Result<()> {
7084
let cli = Cli::parse();
7185
let root_path = cli.root_path.unwrap_or_default();
7286
match &cli.command {
7387
Commands::Tags => tags(&root_path),
7488
Commands::Validate => validate(&root_path),
75-
Commands::Bug { path } => bug(&path),
89+
Commands::Updates => updates(&root_path),
7690
}?;
7791
Ok(())
7892
}

tools/kbcheck/src/updates.rs

Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,245 @@
1+
use crate::bugzilla;
2+
use crate::data::load_all;
3+
use crate::entry::Entry;
4+
use miette::Diagnostic;
5+
use std::collections::{BTreeMap, BTreeSet};
6+
use std::path::{Path, PathBuf};
7+
use thiserror::Error;
8+
use url::Url;
9+
10+
#[derive(Error, Debug, Diagnostic)]
11+
pub enum UpdateError {
12+
#[error(transparent)]
13+
DataError(#[from] crate::data::DataError),
14+
#[error(transparent)]
15+
BugzillaError(#[from] crate::bugzilla::BugzillaError),
16+
}
17+
18+
pub struct Update {
19+
pub error: String,
20+
pub suggestion: String,
21+
}
22+
23+
/// Extract a bugzilla bug id from a URL
24+
fn bugzilla_id(url: &Url) -> Option<u64> {
25+
if url.host_str() == Some("bugzilla.mozilla.org") && url.path() == "/show_bug.cgi" {
26+
if let Some((_, id_str)) = url.query_pairs().find(|(name, _)| name == "id") {
27+
return id_str.parse().ok();
28+
}
29+
}
30+
None
31+
}
32+
33+
/// Extract a webcompat issue number from a URL
34+
fn webcompat_id(url: &Url) -> Option<u64> {
35+
if (url.host_str() == Some("github.com")
36+
&& url.path().starts_with("/webcompat/web-bugs/issues/"))
37+
|| (url.host_str() == Some("webcompat.com") && url.path().starts_with("/issues/"))
38+
{
39+
if let Some(segments) = url.path_segments() {
40+
if let Some(id) = segments.last() {
41+
return id.parse().ok();
42+
}
43+
}
44+
}
45+
None
46+
}
47+
48+
/// Check for possible updates relative to the bugzilla bugs listed in `references.platform_issues`
49+
fn check_bug(bugzilla: &bugzilla::Client, entry: &Entry) -> Result<Vec<Update>, UpdateError> {
50+
let mut updates = Vec::new();
51+
let platform_issue_ids = entry
52+
.references
53+
.platform_issues
54+
.iter()
55+
.filter_map(bugzilla_id);
56+
let mut bugs_data = BTreeMap::new();
57+
for bug_id in platform_issue_ids {
58+
let bug_data = bugzilla.get_bug(bug_id)?;
59+
check_is_dupe(&bug_data, &mut updates);
60+
bugs_data.insert(bug_id, bug_data);
61+
}
62+
check_missing_breakage(entry, &bugs_data, &mut updates);
63+
Ok(updates)
64+
}
65+
66+
/// Check if the bug was closed as a dupe
67+
fn check_is_dupe(bug_data: &bugzilla::Bug, updates: &mut Vec<Update>) {
68+
if let Some(dupe) = bug_data.dupe_of {
69+
updates.push(Update {
70+
error: format!("Bug {} closed as a duplicate of {}", bug_data.id, dupe),
71+
suggestion: format!(
72+
"Update platform_issue entry to https://bugzilla.mozilla.org/show_bug.cgi?id={}",
73+
dupe
74+
),
75+
});
76+
}
77+
}
78+
79+
/// Check for see_also links to webcompat issues that aren't in the breakage data
80+
fn check_missing_breakage(
81+
entry: &Entry,
82+
bugs_data: &BTreeMap<u64, bugzilla::Bug>,
83+
updates: &mut Vec<Update>,
84+
) {
85+
let entry_breakage =
86+
BTreeSet::from_iter(entry.references.breakage.iter().filter_map(webcompat_id));
87+
let mut bugs_see_also = BTreeSet::new();
88+
for bug_data in bugs_data.values() {
89+
bugs_see_also.extend(
90+
bug_data
91+
.see_also
92+
.iter()
93+
.filter_map(|url_str| Url::parse(url_str).ok().as_ref().and_then(webcompat_id)),
94+
)
95+
}
96+
for webcompat_id in bugs_see_also.difference(&entry_breakage) {
97+
updates.push(Update {
98+
error: "Missing see also entry from linked bug in breakage".into(),
99+
suggestion: format!(
100+
"Add https://webcompat.com/issues/{} to references.breakage",
101+
webcompat_id
102+
),
103+
});
104+
}
105+
}
106+
107+
/// Check knowledge base entries against other data sources for possible updates
108+
pub fn check_updates(root_path: &Path) -> Result<BTreeMap<PathBuf, Vec<Update>>, UpdateError> {
109+
let entries = load_all(root_path)?;
110+
let bugzilla_client = bugzilla::Client::new(
111+
Url::parse("https://bugzilla.mozilla.org").expect("Failed to parse bugzilla URL"),
112+
);
113+
let mut updates = BTreeMap::new();
114+
for (path, entry) in entries.iter() {
115+
let path_updates = check_bug(&bugzilla_client, entry)?;
116+
if !path_updates.is_empty() {
117+
updates.insert(path.to_owned(), path_updates);
118+
}
119+
}
120+
Ok(updates)
121+
}
122+
123+
#[cfg(test)]
124+
mod test {
125+
use super::*;
126+
use crate::bugzilla::Bug;
127+
use crate::entry::{Entry, References, Severity, Solutions};
128+
use std::default::Default;
129+
130+
#[test]
131+
fn test_bugzilla_id() {
132+
assert_eq!(
133+
bugzilla_id(&Url::parse("https://bugzilla.mozilla.org/show_bug.cgi?id=1234").unwrap()),
134+
Some(1234)
135+
);
136+
assert_eq!(
137+
bugzilla_id(
138+
&Url::parse("https://bugzilla.mozilla.org/show_bug.cgi?id=1234&foo=bar#baz")
139+
.unwrap()
140+
),
141+
Some(1234)
142+
);
143+
assert_eq!(
144+
bugzilla_id(&Url::parse("https://bugzilla.mozilla.org/other.cgi?id=1234").unwrap()),
145+
None
146+
);
147+
assert_eq!(
148+
bugzilla_id(&Url::parse("https://bugzilla.mozilla.com/show_bug.cgi?id=1234").unwrap()),
149+
None
150+
);
151+
assert_eq!(
152+
bugzilla_id(&Url::parse("https://bugzilla.mozilla.org/show_bug.cgi?id=1234a").unwrap()),
153+
None
154+
);
155+
}
156+
157+
#[test]
158+
fn test_webcompat_id() {
159+
assert_eq!(
160+
webcompat_id(&Url::parse("https://webcompat.com/issues/1234").unwrap()),
161+
Some(1234)
162+
);
163+
assert_eq!(
164+
webcompat_id(&Url::parse("https://github.com/webcompat/web-bugs/issues/1234").unwrap()),
165+
Some(1234)
166+
);
167+
assert_eq!(
168+
webcompat_id(&Url::parse("https://webcompat.com/issues/1234?foo=bar#baz").unwrap()),
169+
Some(1234)
170+
);
171+
assert_eq!(
172+
webcompat_id(
173+
&Url::parse("https://github.com/webcompat/web-bugs/issues/1234?foo=bar#baz")
174+
.unwrap()
175+
),
176+
Some(1234)
177+
);
178+
assert_eq!(
179+
webcompat_id(&Url::parse("https://other.webcompat.com/issues/1234").unwrap()),
180+
None
181+
);
182+
assert_eq!(
183+
webcompat_id(&Url::parse("https://github.org/webcompat/web-bugs/issues/1234").unwrap()),
184+
None
185+
);
186+
assert_eq!(
187+
webcompat_id(&Url::parse("https://webcompat.com/other/1234").unwrap()),
188+
None
189+
);
190+
assert_eq!(
191+
webcompat_id(&Url::parse("https://github.com/webcompat/web-bugs/pulls/1234").unwrap()),
192+
None
193+
);
194+
assert_eq!(
195+
webcompat_id(&Url::parse("https://github.com/other/web-bugs/issues/1234").unwrap()),
196+
None
197+
);
198+
}
199+
200+
#[test]
201+
fn test_is_dupe() {
202+
let mut updates = Vec::new();
203+
let mut bug = Bug {
204+
..Default::default()
205+
};
206+
check_is_dupe(&bug, &mut updates);
207+
assert_eq!(updates.len(), 0);
208+
bug.dupe_of = Some(1234);
209+
check_is_dupe(&bug, &mut updates);
210+
assert_eq!(updates.len(), 1);
211+
}
212+
213+
#[test]
214+
fn test_missing_breakage() {
215+
let mut updates = Vec::new();
216+
let bug = Bug {
217+
see_also: vec![
218+
"https://webcompat.com/issues/1234".into(),
219+
"https://webcompat.com/issues/1235".into(),
220+
],
221+
..Default::default()
222+
};
223+
let entry = Entry {
224+
title: "test".into(),
225+
severity: Severity::Normal,
226+
user_base_impact: None,
227+
notes: None,
228+
tags: vec![],
229+
symptoms: vec![],
230+
console_messages: vec![],
231+
references: References {
232+
breakage: vec![Url::parse("https://webcompat.com/issues/1234").unwrap()],
233+
..Default::default()
234+
},
235+
solutions: Solutions {
236+
..Default::default()
237+
},
238+
};
239+
let mut bugs_data = BTreeMap::new();
240+
bugs_data.insert(123, bug);
241+
check_missing_breakage(&entry, &bugs_data, &mut updates);
242+
assert_eq!(updates.len(), 1);
243+
assert!(updates[0].suggestion.contains("1235"))
244+
}
245+
}

0 commit comments

Comments
 (0)