fix(ipc): thread active_prefix through Query so daemon honours it (Phase 3.4.5)
The smoke test in Phase 3.4 surfaced a long-standing gap: parse_query
already routes `:smoke foo` → Plugin("smoke") via the dynamic-prefix
fallback (filter.rs:319-347), but the UI never told the daemon. The
UI's filter.active_prefix was set locally yet build_modes_param only
serialised filter.enabled, so the daemon rebuilt a fresh filter with
no active_prefix and searched across every provider. Same gap also
affected hardcoded plugin prefixes (`:emoji heart` etc.) — they only
appeared to work because their items outranked others on fuzzy score.
ipc.rs:
- Request::Query gains optional `prefix: Option<String>`, default None,
skip_serializing_if = "Option::is_none". Wire-format omits the field
when None so existing 2.0.x clients keep deserialising on a 2.1
daemon and vice-versa.
filter.rs:
- New test parse_query_routes_unknown_prefix_to_plugin_type_id locks
in the existing dynamic-fallback behaviour for `:smoke item`.
- New test build_prefix_param_round_trips_through_provider_type
proves Display + FromStr agree on the wire format.
client.rs:
- `query(text, modes, prefix)` — third arg added; 2 existing tests
updated. Two new tests with a capturing mock_server assert the
prefix field is serialised when set and elided when None.
backend.rs:
- QueryParams grows a `prefix: Option<String>`. New helper
build_prefix_param(filter) -> Option<String> mirrors set_prefix's
active_prefix to the wire. All three client.query call sites updated
(search, search_with_tag, query_async).
server.rs:
- Query handler destructures the new prefix field and applies it via
filter.set_prefix(ProviderType::from_str(prefix)). Unknown ids fall
through to ProviderType::Plugin(_) per the existing FromStr impl —
symmetric with the UI's dynamic-fallback path.
Live smoke (release build, isolated XDG + socket):
- prefix=smoke + text="item" → 3 items, all smoke (was: 100, mixed)
- prefix=smoke + text="one" → 1 item: "smoke one item"
- prefix=smoke + text="" → 3 smoke items, frecency-sorted
- prefix=app + text="firefox" → 1 firefox app item (regression
check; hardcoded prefix path still works)
7 new tests (ipc:3, client:2, filter:2). 283/283 lib tests green with
--features full. Both clippy configurations silent. Closes task #28.
This commit is contained in:
@@ -18,6 +18,11 @@ pub struct QueryParams {
|
||||
#[allow(dead_code)]
|
||||
pub max_results: usize,
|
||||
pub modes: Option<Vec<String>>,
|
||||
/// Provider-narrowing prefix (e.g. `"smoke"`, `"calc"`). Mirrors the
|
||||
/// `prefix` field of [`crate::filter::ParsedQuery`] after `parse_query`
|
||||
/// strips the `:foo ` from the entry. Daemon applies it as
|
||||
/// `filter.set_prefix` before searching.
|
||||
pub prefix: Option<String>,
|
||||
pub tag_filter: Option<String>,
|
||||
}
|
||||
|
||||
@@ -62,7 +67,7 @@ impl DaemonHandle {
|
||||
} else {
|
||||
params.query
|
||||
};
|
||||
match c.query(&effective_query, params.modes) {
|
||||
match c.query(&effective_query, params.modes, params.prefix) {
|
||||
Ok(items) => items.into_iter().map(result_to_launch_item).collect(),
|
||||
Err(e) => {
|
||||
warn!("IPC query failed: {}", e);
|
||||
@@ -115,6 +120,15 @@ impl SearchBackend {
|
||||
}
|
||||
}
|
||||
|
||||
/// Extract the active-prefix parameter for IPC. Mirrors the
|
||||
/// `active_prefix` on the local filter so the daemon's filter can be
|
||||
/// narrowed identically. `set_prefix` doesn't touch `accept_all` /
|
||||
/// `enabled`, so without this the prefix would be silently lost in
|
||||
/// translation (task #28).
|
||||
fn build_prefix_param(filter: &ProviderFilter) -> Option<String> {
|
||||
filter.active_prefix().map(|p| p.to_string())
|
||||
}
|
||||
|
||||
/// Search for items matching the query.
|
||||
///
|
||||
/// In daemon mode, sends query over IPC. The modes list is derived from
|
||||
@@ -131,8 +145,9 @@ impl SearchBackend {
|
||||
match self {
|
||||
SearchBackend::Daemon(handle) => {
|
||||
let modes_param = Self::build_modes_param(filter);
|
||||
let prefix_param = Self::build_prefix_param(filter);
|
||||
match handle.client.lock() {
|
||||
Ok(mut client) => match client.query(query, modes_param) {
|
||||
Ok(mut client) => match client.query(query, modes_param, prefix_param) {
|
||||
Ok(items) => items.into_iter().map(result_to_launch_item).collect(),
|
||||
Err(e) => {
|
||||
warn!("IPC query failed: {}", e);
|
||||
@@ -194,8 +209,9 @@ impl SearchBackend {
|
||||
};
|
||||
|
||||
let modes_param = Self::build_modes_param(filter);
|
||||
let prefix_param = Self::build_prefix_param(filter);
|
||||
match handle.client.lock() {
|
||||
Ok(mut client) => match client.query(&effective_query, modes_param) {
|
||||
Ok(mut client) => match client.query(&effective_query, modes_param, prefix_param) {
|
||||
Ok(items) => items.into_iter().map(result_to_launch_item).collect(),
|
||||
Err(e) => {
|
||||
warn!("IPC query failed: {}", e);
|
||||
@@ -255,6 +271,7 @@ impl SearchBackend {
|
||||
query: query.to_string(),
|
||||
max_results,
|
||||
modes: Self::build_modes_param(filter),
|
||||
prefix: Self::build_prefix_param(filter),
|
||||
tag_filter: tag_filter.map(|s| s.to_string()),
|
||||
};
|
||||
Some(handle.query_async(params))
|
||||
|
||||
@@ -116,10 +116,21 @@ impl CoreClient {
|
||||
}
|
||||
|
||||
/// Send a search query and return matching results.
|
||||
pub fn query(&mut self, text: &str, modes: Option<Vec<String>>) -> io::Result<Vec<ResultItem>> {
|
||||
///
|
||||
/// `prefix` (when `Some`) narrows the daemon-side search to a single
|
||||
/// provider — typically the `prefix` field from
|
||||
/// [`crate::filter::ProviderFilter::parse_query`]. Pass `None` for the
|
||||
/// unrestricted case (the daemon falls back to `modes` or accept-all).
|
||||
pub fn query(
|
||||
&mut self,
|
||||
text: &str,
|
||||
modes: Option<Vec<String>>,
|
||||
prefix: Option<String>,
|
||||
) -> io::Result<Vec<ResultItem>> {
|
||||
self.send(&Request::Query {
|
||||
text: text.to_string(),
|
||||
modes,
|
||||
prefix,
|
||||
})?;
|
||||
|
||||
match self.receive()? {
|
||||
@@ -245,6 +256,38 @@ mod tests {
|
||||
|
||||
static COUNTER: AtomicU32 = AtomicU32::new(0);
|
||||
|
||||
/// Spawn a mock server that accepts one connection, captures the request,
|
||||
/// and replies with the given canned response. Returns the socket path and
|
||||
/// a Receiver that will yield the request line after the test sends.
|
||||
fn mock_server_capturing(response: Response) -> (PathBuf, std::sync::mpsc::Receiver<String>) {
|
||||
let n = COUNTER.fetch_add(1, Ordering::Relaxed);
|
||||
let dir = std::env::temp_dir().join(format!("owlry-test-{}-{}", std::process::id(), n));
|
||||
let _ = std::fs::create_dir_all(&dir);
|
||||
let sock = dir.join("test.sock");
|
||||
let _ = std::fs::remove_file(&sock);
|
||||
|
||||
let listener = UnixListener::bind(&sock).expect("bind mock socket");
|
||||
let sock_clone = sock.clone();
|
||||
let (tx, rx) = std::sync::mpsc::channel();
|
||||
|
||||
thread::spawn(move || {
|
||||
let (stream, _) = listener.accept().expect("accept");
|
||||
let mut reader = BufReader::new(stream.try_clone().unwrap());
|
||||
let mut writer = stream;
|
||||
let mut line = String::new();
|
||||
reader.read_line(&mut line).expect("read request");
|
||||
let _ = tx.send(line);
|
||||
let mut json = serde_json::to_string(&response).unwrap();
|
||||
json.push('\n');
|
||||
writer.write_all(json.as_bytes()).unwrap();
|
||||
writer.flush().unwrap();
|
||||
let _ = std::fs::remove_file(&sock_clone);
|
||||
let _ = std::fs::remove_dir(dir);
|
||||
});
|
||||
|
||||
(sock, rx)
|
||||
}
|
||||
|
||||
/// Spawn a mock server that accepts one connection, reads one request,
|
||||
/// and replies with the given canned response. Each call gets a unique
|
||||
/// socket path to avoid collisions when tests run in parallel.
|
||||
@@ -303,7 +346,7 @@ mod tests {
|
||||
thread::sleep(Duration::from_millis(50));
|
||||
|
||||
let mut client = CoreClient::connect(&sock).expect("connect");
|
||||
let results = client.query("fire", None).expect("query");
|
||||
let results = client.query("fire", None, None).expect("query");
|
||||
|
||||
assert_eq!(results.len(), 1);
|
||||
assert_eq!(results[0].id, "firefox");
|
||||
@@ -311,6 +354,44 @@ mod tests {
|
||||
assert_eq!(results[0].score, 100);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn query_sends_prefix_field_in_request() {
|
||||
let canned = Response::Results { items: vec![] };
|
||||
let (sock, rx) = mock_server_capturing(canned);
|
||||
thread::sleep(Duration::from_millis(50));
|
||||
|
||||
let mut client = CoreClient::connect(&sock).expect("connect");
|
||||
let _ = client
|
||||
.query("foo", None, Some("smoke".into()))
|
||||
.expect("query");
|
||||
|
||||
let request_line = rx.recv_timeout(Duration::from_secs(2)).expect("captured");
|
||||
let parsed: serde_json::Value = serde_json::from_str(&request_line).expect("parse json");
|
||||
assert_eq!(parsed["type"], "query");
|
||||
assert_eq!(parsed["text"], "foo");
|
||||
assert_eq!(
|
||||
parsed["prefix"], "smoke",
|
||||
"prefix must be present in the request"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn query_without_prefix_omits_field_from_request() {
|
||||
let canned = Response::Results { items: vec![] };
|
||||
let (sock, rx) = mock_server_capturing(canned);
|
||||
thread::sleep(Duration::from_millis(50));
|
||||
|
||||
let mut client = CoreClient::connect(&sock).expect("connect");
|
||||
let _ = client.query("foo", None, None).expect("query");
|
||||
|
||||
let request_line = rx.recv_timeout(Duration::from_secs(2)).expect("captured");
|
||||
assert!(
|
||||
!request_line.contains("prefix"),
|
||||
"prefix field must be omitted when None; got: {}",
|
||||
request_line
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn toggle_returns_ack() {
|
||||
let sock = mock_server(Response::Ack);
|
||||
@@ -392,7 +473,7 @@ mod tests {
|
||||
thread::sleep(Duration::from_millis(50));
|
||||
|
||||
let mut client = CoreClient::connect(&sock).expect("connect");
|
||||
let err = client.query("test", None).unwrap_err();
|
||||
let err = client.query("test", None, None).unwrap_err();
|
||||
|
||||
let msg = err.to_string();
|
||||
assert!(
|
||||
|
||||
@@ -483,6 +483,39 @@ mod tests {
|
||||
assert_eq!(result.query, "5+3");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_query_routes_unknown_prefix_to_plugin_type_id() {
|
||||
// Locks in the dynamic-prefix-fallback path (filter.rs:319-347): a
|
||||
// prefix not in the hardcoded core/plugin tables — e.g. a user
|
||||
// provider's `:smoke` — must still produce Plugin("smoke"). This is
|
||||
// what makes user-defined providers reachable by prefix.
|
||||
let result = ProviderFilter::parse_query(":smoke item");
|
||||
assert_eq!(
|
||||
result.prefix,
|
||||
Some(ProviderType::Plugin("smoke".to_string()))
|
||||
);
|
||||
assert_eq!(result.query, "item");
|
||||
|
||||
let bare = ProviderFilter::parse_query(":smoke");
|
||||
assert_eq!(
|
||||
bare.prefix,
|
||||
Some(ProviderType::Plugin("smoke".to_string()))
|
||||
);
|
||||
assert_eq!(bare.query, "");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn build_prefix_param_round_trips_through_provider_type() {
|
||||
// The wire format is ProviderType's Display impl; the daemon parses
|
||||
// it back via FromStr. Verify both ends agree for a user provider.
|
||||
use std::str::FromStr;
|
||||
let plugin_smoke = ProviderType::Plugin("smoke".to_string());
|
||||
let wire = plugin_smoke.to_string();
|
||||
assert_eq!(wire, "smoke");
|
||||
let parsed = ProviderType::from_str(&wire).unwrap();
|
||||
assert_eq!(parsed, plugin_smoke);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_toggle_ensures_one_enabled() {
|
||||
let mut filter = ProviderFilter::apps_only();
|
||||
|
||||
@@ -7,6 +7,15 @@ pub enum Request {
|
||||
text: String,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
modes: Option<Vec<String>>,
|
||||
/// Active prefix narrowing — the daemon restricts results to this
|
||||
/// single provider. Carries the [`ProviderType`] as a string
|
||||
/// (e.g. `"app"`, `"cmd"`, `"smoke"` for a user-defined provider
|
||||
/// with id `"smoke"`). The UI populates this from
|
||||
/// `ProviderFilter::parse_query`'s `prefix` field after stripping
|
||||
/// the `:foo ` from the query text. Older clients omit the field;
|
||||
/// the daemon treats `None` as "no narrowing".
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
prefix: Option<String>,
|
||||
},
|
||||
Launch {
|
||||
item_id: String,
|
||||
@@ -73,3 +82,52 @@ pub struct ProviderDesc {
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub search_noun: Option<String>,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn query_request_without_prefix_serializes_without_field() {
|
||||
// Pre-3.4.5 clients didn't have the field; new schema must omit it
|
||||
// from JSON when None so on-wire payloads stay identical.
|
||||
let req = Request::Query {
|
||||
text: "firefox".into(),
|
||||
modes: None,
|
||||
prefix: None,
|
||||
};
|
||||
let json = serde_json::to_string(&req).unwrap();
|
||||
assert!(!json.contains("prefix"), "prefix must be omitted; got: {}", json);
|
||||
assert!(json.contains(r#""type":"query""#));
|
||||
assert!(json.contains(r#""text":"firefox""#));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn query_request_with_prefix_serializes_and_roundtrips() {
|
||||
let req = Request::Query {
|
||||
text: "foo".into(),
|
||||
modes: None,
|
||||
prefix: Some("smoke".into()),
|
||||
};
|
||||
let json = serde_json::to_string(&req).unwrap();
|
||||
assert!(json.contains(r#""prefix":"smoke""#));
|
||||
let back: Request = serde_json::from_str(&json).unwrap();
|
||||
assert_eq!(back, req);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_clients_without_prefix_field_still_deserialize() {
|
||||
// A 2.0.x client sends Query without the new prefix field. The 2.1
|
||||
// daemon must still accept it (default = None).
|
||||
let json = r#"{"type":"query","text":"foo"}"#;
|
||||
let req: Request = serde_json::from_str(json).unwrap();
|
||||
match req {
|
||||
Request::Query { text, modes, prefix } => {
|
||||
assert_eq!(text, "foo");
|
||||
assert!(modes.is_none());
|
||||
assert!(prefix.is_none());
|
||||
}
|
||||
other => panic!("expected Query, got {:?}", other),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -352,11 +352,24 @@ impl Server {
|
||||
config: &Arc<RwLock<Config>>,
|
||||
) -> Response {
|
||||
match request {
|
||||
Request::Query { text, modes } => {
|
||||
let filter = match modes {
|
||||
Request::Query {
|
||||
text,
|
||||
modes,
|
||||
prefix,
|
||||
} => {
|
||||
let mut filter = match modes {
|
||||
Some(m) => ProviderFilter::from_mode_strings(m),
|
||||
None => ProviderFilter::all(),
|
||||
};
|
||||
if let Some(prefix_str) = prefix {
|
||||
// Mirrors UI-side parse_query → set_prefix. Tolerates
|
||||
// unknown ids by emitting Plugin(prefix_str) — that's
|
||||
// what the dynamic-prefix-fallback in parse_query does.
|
||||
use std::str::FromStr;
|
||||
if let Ok(prefix_type) = crate::providers::ProviderType::from_str(prefix_str) {
|
||||
filter.set_prefix(Some(prefix_type));
|
||||
}
|
||||
}
|
||||
let (max, weight) = {
|
||||
let cfg = match config.read() {
|
||||
Ok(g) => g,
|
||||
|
||||
Reference in New Issue
Block a user