Refactor filtering to start with a universe

This commit is contained in:
Dustin J. Mitchell 2020-12-23 03:39:38 +00:00
parent e6d60524fa
commit a0568f017c
16 changed files with 401 additions and 112 deletions

View file

@ -10,6 +10,20 @@ use nom::{
sequence::*,
Err, IResult,
};
use taskchampion::Uuid;
/// A task identifier, as given in a filter command-line expression
#[derive(Debug, PartialEq, Clone)]
pub(crate) enum TaskId {
/// A small integer identifying a working-set task
WorkingSetId(usize),
/// A full Uuid specifically identifying a task
Uuid(Uuid),
/// A prefix of a Uuid
PartialUuid(String),
}
/// Recognizes any argument
pub(super) fn any(input: &str) -> IResult<&str, &str> {
@ -21,38 +35,60 @@ pub(super) fn literal(literal: &'static str) -> impl Fn(&str) -> IResult<&str, &
move |input: &str| all_consuming(nomtag(literal))(input)
}
/// Recognizes a comma-separated list of ID's (integers or UUID prefixes)
pub(super) fn id_list(input: &str) -> IResult<&str, Vec<&str>> {
/// Recognizes a comma-separated list of TaskIds
pub(super) fn id_list(input: &str) -> IResult<&str, Vec<TaskId>> {
fn hex_n(n: usize) -> impl Fn(&str) -> IResult<&str, &str> {
move |input: &str| recognize(many_m_n(n, n, one_of(&b"0123456789abcdefABCDEF"[..])))(input)
}
fn uuid(input: &str) -> Result<TaskId, ()> {
Ok(TaskId::Uuid(Uuid::parse_str(input).map_err(|_| ())?))
}
fn partial_uuid(input: &str) -> Result<TaskId, ()> {
Ok(TaskId::PartialUuid(input.to_owned()))
}
fn working_set_id(input: &str) -> Result<TaskId, ()> {
Ok(TaskId::WorkingSetId(input.parse().map_err(|_| ())?))
}
all_consuming(separated_list1(
char(','),
alt((
recognize(tuple((
hex_n(8),
char('-'),
hex_n(4),
char('-'),
hex_n(4),
char('-'),
hex_n(4),
char('-'),
hex_n(12),
))),
recognize(tuple((
hex_n(8),
char('-'),
hex_n(4),
char('-'),
hex_n(4),
char('-'),
hex_n(4),
))),
recognize(tuple((hex_n(8), char('-'), hex_n(4), char('-'), hex_n(4)))),
recognize(tuple((hex_n(8), char('-'), hex_n(4)))),
hex_n(8),
digit1,
map_res(
recognize(tuple((
hex_n(8),
char('-'),
hex_n(4),
char('-'),
hex_n(4),
char('-'),
hex_n(4),
char('-'),
hex_n(12),
))),
uuid,
),
map_res(
recognize(tuple((
hex_n(8),
char('-'),
hex_n(4),
char('-'),
hex_n(4),
char('-'),
hex_n(4),
))),
partial_uuid,
),
map_res(
recognize(tuple((hex_n(8), char('-'), hex_n(4), char('-'), hex_n(4)))),
partial_uuid,
),
map_res(
recognize(tuple((hex_n(8), char('-'), hex_n(4)))),
partial_uuid,
),
map_res(hex_n(8), partial_uuid),
// note that an 8-decimal-digit value will be treated as a UUID
map_res(digit1, working_set_id),
)),
))(input)
}
@ -154,29 +190,40 @@ mod test {
#[test]
fn test_id_list_single() {
assert_eq!(id_list("123").unwrap().1, vec!["123".to_owned()]);
assert_eq!(id_list("123").unwrap().1, vec![TaskId::WorkingSetId(123)]);
}
#[test]
fn test_id_list_uuids() {
assert_eq!(id_list("12341234").unwrap().1, vec!["12341234".to_owned()]);
assert_eq!(id_list("1234abcd").unwrap().1, vec!["1234abcd".to_owned()]);
assert_eq!(id_list("abcd1234").unwrap().1, vec!["abcd1234".to_owned()]);
assert_eq!(
id_list("12341234").unwrap().1,
vec![TaskId::PartialUuid("12341234".to_owned())]
);
assert_eq!(
id_list("1234abcd").unwrap().1,
vec![TaskId::PartialUuid("1234abcd".to_owned())]
);
assert_eq!(
id_list("abcd1234").unwrap().1,
vec![TaskId::PartialUuid("abcd1234".to_owned())]
);
assert_eq!(
id_list("abcd1234-1234").unwrap().1,
vec!["abcd1234-1234".to_owned()]
vec![TaskId::PartialUuid("abcd1234-1234".to_owned())]
);
assert_eq!(
id_list("abcd1234-1234-2345").unwrap().1,
vec!["abcd1234-1234-2345".to_owned()]
vec![TaskId::PartialUuid("abcd1234-1234-2345".to_owned())]
);
assert_eq!(
id_list("abcd1234-1234-2345-3456").unwrap().1,
vec!["abcd1234-1234-2345-3456".to_owned()]
vec![TaskId::PartialUuid("abcd1234-1234-2345-3456".to_owned())]
);
assert_eq!(
id_list("abcd1234-1234-2345-3456-0123456789ab").unwrap().1,
vec!["abcd1234-1234-2345-3456-0123456789ab".to_owned()]
vec![TaskId::Uuid(
Uuid::parse_str("abcd1234-1234-2345-3456-0123456789ab").unwrap()
)]
);
}
@ -194,11 +241,11 @@ mod test {
#[test]
fn test_id_list_uuids_mixed() {
assert_eq!(id_list("abcd1234,abcd1234-1234,abcd1234-1234-2345,abcd1234-1234-2345-3456,abcd1234-1234-2345-3456-0123456789ab").unwrap().1,
vec!["abcd1234".to_owned(),
"abcd1234-1234".to_owned(),
"abcd1234-1234-2345".to_owned(),
"abcd1234-1234-2345-3456".to_owned(),
"abcd1234-1234-2345-3456-0123456789ab".to_owned(),
vec![TaskId::PartialUuid("abcd1234".to_owned()),
TaskId::PartialUuid("abcd1234-1234".to_owned()),
TaskId::PartialUuid("abcd1234-1234-2345".to_owned()),
TaskId::PartialUuid("abcd1234-1234-2345-3456".to_owned()),
TaskId::Uuid(Uuid::parse_str("abcd1234-1234-2345-3456-0123456789ab").unwrap()),
]);
}
}

View file

@ -1,48 +1,80 @@
use super::args::{arg_matching, id_list};
use super::args::{arg_matching, id_list, TaskId};
use super::ArgList;
use nom::{combinator::*, multi::fold_many0, IResult};
/// A filter represents a selection of a particular set of tasks.
///
/// A filter has a "universe" of tasks that might match, and a list of conditions
/// all of which tasks must match. The universe can be a set of task IDs, or just
/// pending tasks, or all tasks.
#[derive(Debug, PartialEq, Default, Clone)]
pub(crate) struct Filter {
/// A list of numeric IDs or prefixes of UUIDs
pub(crate) id_list: Option<Vec<String>>,
pub(crate) universe: Universe,
}
/// The universe of tasks over which a filter should be applied.
#[derive(Debug, PartialEq, Clone)]
pub(crate) enum Universe {
/// Only the identified tasks. Note that this may contain duplicates.
IdList(Vec<TaskId>),
/// All tasks in the task database
AllTasks,
/// Only pending tasks (or as an approximation, the working set)
#[allow(dead_code)] // currently only used in tests
PendingTasks,
}
impl Universe {
/// Testing shorthand to construct a simple universe
#[cfg(test)]
pub(super) fn for_ids(mut ids: Vec<usize>) -> Self {
Universe::IdList(ids.drain(..).map(|id| TaskId::WorkingSetId(id)).collect())
}
}
impl Default for Universe {
fn default() -> Self {
Self::AllTasks
}
}
/// Internal struct representing a parsed filter argument
enum FilterArg {
IdList(Vec<String>),
IdList(Vec<TaskId>),
}
impl Filter {
pub(super) fn parse(input: ArgList) -> IResult<ArgList, Filter> {
fn fold(mut acc: Filter, mod_arg: FilterArg) -> Filter {
match mod_arg {
FilterArg::IdList(mut id_list) => {
if let Some(ref mut existing) = acc.id_list {
// given multiple ID lists, concatenate them to represent
// an "OR" between them.
existing.append(&mut id_list);
} else {
acc.id_list = Some(id_list);
}
}
}
acc
}
fold_many0(
Self::id_list,
Filter {
..Default::default()
},
fold,
Self::fold_args,
)(input)
}
/// fold multiple filter args into a single Filter instance
fn fold_args(mut acc: Filter, mod_arg: FilterArg) -> Filter {
match mod_arg {
FilterArg::IdList(mut id_list) => {
// If any IDs are specified, then the filter's universe
// is those IDs. If there are already IDs, append to the
// list.
if let Universe::IdList(ref mut existing) = acc.universe {
existing.append(&mut id_list);
} else {
acc.universe = Universe::IdList(id_list);
}
}
}
acc
}
fn id_list(input: ArgList) -> IResult<ArgList, FilterArg> {
fn to_filterarg(mut input: Vec<&str>) -> Result<FilterArg, ()> {
Ok(FilterArg::IdList(
input.drain(..).map(str::to_owned).collect(),
))
fn to_filterarg(input: Vec<TaskId>) -> Result<FilterArg, ()> {
Ok(FilterArg::IdList(input))
}
map_res(arg_matching(id_list), to_filterarg)(input)
}
@ -71,7 +103,7 @@ mod test {
assert_eq!(
filter,
Filter {
id_list: Some(vec!["1".to_owned()]),
universe: Universe::IdList(vec![TaskId::WorkingSetId(1)]),
..Default::default()
}
);
@ -84,7 +116,29 @@ mod test {
assert_eq!(
filter,
Filter {
id_list: Some(vec!["1".to_owned(), "2".to_owned(), "3".to_owned()]),
universe: Universe::IdList(vec![
TaskId::WorkingSetId(1),
TaskId::WorkingSetId(2),
TaskId::WorkingSetId(3),
]),
..Default::default()
}
);
}
#[test]
fn test_id_list_multi_arg() {
let (input, filter) = Filter::parse(argv!["1,2", "3,4"]).unwrap();
assert_eq!(input.len(), 0);
assert_eq!(
filter,
Filter {
universe: Universe::IdList(vec![
TaskId::WorkingSetId(1),
TaskId::WorkingSetId(2),
TaskId::WorkingSetId(3),
TaskId::WorkingSetId(4),
]),
..Default::default()
}
);
@ -97,7 +151,10 @@ mod test {
assert_eq!(
filter,
Filter {
id_list: Some(vec!["1".to_owned(), "abcd1234".to_owned()]),
universe: Universe::IdList(vec![
TaskId::WorkingSetId(1),
TaskId::PartialUuid("abcd1234".to_owned()),
]),
..Default::default()
}
);

View file

@ -18,8 +18,9 @@ mod modification;
mod report;
mod subcommand;
pub(crate) use args::TaskId;
pub(crate) use command::Command;
pub(crate) use filter::Filter;
pub(crate) use filter::{Filter, Universe};
pub(crate) use modification::{DescriptionMod, Modification};
pub(crate) use report::Report;
pub(crate) use subcommand::Subcommand;

View file

@ -387,6 +387,7 @@ impl Sync {
#[cfg(test)]
mod test {
use super::*;
use crate::argparse::Universe;
const EMPTY: Vec<&str> = vec![];
@ -462,7 +463,8 @@ mod test {
fn test_modify_description_multi() {
let subcommand = Subcommand::Modify {
filter: Filter {
id_list: Some(vec!["123".to_owned()]),
universe: Universe::for_ids(vec![123]),
..Default::default()
},
modification: Modification {
description: DescriptionMod::Set("foo bar".to_owned()),
@ -479,7 +481,8 @@ mod test {
fn test_append() {
let subcommand = Subcommand::Modify {
filter: Filter {
id_list: Some(vec!["123".to_owned()]),
universe: Universe::for_ids(vec![123]),
..Default::default()
},
modification: Modification {
description: DescriptionMod::Append("foo bar".to_owned()),
@ -496,7 +499,8 @@ mod test {
fn test_prepend() {
let subcommand = Subcommand::Modify {
filter: Filter {
id_list: Some(vec!["123".to_owned()]),
universe: Universe::for_ids(vec![123]),
..Default::default()
},
modification: Modification {
description: DescriptionMod::Prepend("foo bar".to_owned()),
@ -513,7 +517,8 @@ mod test {
fn test_done() {
let subcommand = Subcommand::Modify {
filter: Filter {
id_list: Some(vec!["123".to_owned()]),
universe: Universe::for_ids(vec![123]),
..Default::default()
},
modification: Modification {
status: Some(Status::Completed),
@ -530,7 +535,8 @@ mod test {
fn test_done_modify() {
let subcommand = Subcommand::Modify {
filter: Filter {
id_list: Some(vec!["123".to_owned()]),
universe: Universe::for_ids(vec![123]),
..Default::default()
},
modification: Modification {
description: DescriptionMod::Set("now-finished".to_owned()),
@ -548,7 +554,8 @@ mod test {
fn test_start() {
let subcommand = Subcommand::Modify {
filter: Filter {
id_list: Some(vec!["123".to_owned()]),
universe: Universe::for_ids(vec![123]),
..Default::default()
},
modification: Modification {
active: Some(true),
@ -565,7 +572,8 @@ mod test {
fn test_start_modify() {
let subcommand = Subcommand::Modify {
filter: Filter {
id_list: Some(vec!["123".to_owned()]),
universe: Universe::for_ids(vec![123]),
..Default::default()
},
modification: Modification {
active: Some(true),
@ -583,7 +591,8 @@ mod test {
fn test_stop() {
let subcommand = Subcommand::Modify {
filter: Filter {
id_list: Some(vec!["123".to_owned()]),
universe: Universe::for_ids(vec![123]),
..Default::default()
},
modification: Modification {
active: Some(false),
@ -600,7 +609,8 @@ mod test {
fn test_stop_modify() {
let subcommand = Subcommand::Modify {
filter: Filter {
id_list: Some(vec!["123".to_owned()]),
universe: Universe::for_ids(vec![123]),
..Default::default()
},
modification: Modification {
description: DescriptionMod::Set("mod".to_owned()),
@ -632,7 +642,8 @@ mod test {
let subcommand = Subcommand::List {
report: Report {
filter: Filter {
id_list: Some(vec!["12".to_owned(), "13".to_owned()]),
universe: Universe::for_ids(vec![12, 13]),
..Default::default()
},
},
};
@ -647,7 +658,8 @@ mod test {
let subcommand = Subcommand::Info {
debug: false,
filter: Filter {
id_list: Some(vec!["12".to_owned(), "13".to_owned()]),
universe: Universe::for_ids(vec![12, 13]),
..Default::default()
},
};
assert_eq!(
@ -661,7 +673,8 @@ mod test {
let subcommand = Subcommand::Info {
debug: true,
filter: Filter {
id_list: Some(vec!["12".to_owned()]),
universe: Universe::for_ids(vec![12]),
..Default::default()
},
};
assert_eq!(

View file

@ -20,7 +20,7 @@ pub(crate) fn execute<W: WriteColor>(
#[cfg(test)]
mod test {
use super::*;
use crate::invocation::cmd::test::*;
use crate::invocation::test::*;
#[test]
fn test_add() {

View file

@ -11,7 +11,7 @@ pub(crate) fn execute<W: WriteColor>(w: &mut W, replica: &mut Replica) -> Fallib
#[cfg(test)]
mod test {
use super::*;
use crate::invocation::cmd::test::*;
use crate::invocation::test::*;
#[test]
fn test_gc() {

View file

@ -15,7 +15,7 @@ pub(crate) fn execute<W: WriteColor>(
#[cfg(test)]
mod test {
use super::*;
use crate::invocation::cmd::test::*;
use crate::invocation::test::*;
#[test]
fn test_summary() {

View file

@ -45,7 +45,7 @@ pub(crate) fn execute<W: WriteColor>(
#[cfg(test)]
mod test {
use super::*;
use crate::invocation::cmd::test::*;
use crate::invocation::test::*;
use taskchampion::Status;
#[test]

View file

@ -34,7 +34,7 @@ pub(crate) fn execute<W: WriteColor>(
mod test {
use super::*;
use crate::argparse::Filter;
use crate::invocation::cmd::test::*;
use crate::invocation::test::*;
use taskchampion::Status;
#[test]

View file

@ -8,6 +8,3 @@ pub(crate) mod list;
pub(crate) mod modify;
pub(crate) mod sync;
pub(crate) mod version;
#[cfg(test)]
mod test;

View file

@ -23,8 +23,8 @@ pub(crate) fn execute<W: WriteColor>(
mod test {
use super::*;
use crate::argparse::DescriptionMod;
use crate::invocation::cmd::test::test_replica;
use crate::invocation::cmd::test::*;
use crate::invocation::test::test_replica;
use crate::invocation::test::*;
use taskchampion::Status;
#[test]

View file

@ -15,7 +15,7 @@ pub(crate) fn execute<W: WriteColor>(
#[cfg(test)]
mod test {
use super::*;
use crate::invocation::cmd::test::*;
use crate::invocation::test::*;
use tempdir::TempDir;
#[test]

View file

@ -12,7 +12,7 @@ pub(crate) fn execute<W: WriteColor>(w: &mut W) -> Fallible<()> {
#[cfg(test)]
mod test {
use super::*;
use crate::invocation::cmd::test::*;
use crate::invocation::test::*;
#[test]
fn test_version() {

View file

@ -1,38 +1,209 @@
use crate::argparse::Filter;
use crate::argparse::{Filter, TaskId, Universe};
use failure::Fallible;
use std::collections::HashSet;
use taskchampion::{Replica, Task};
/// Return the tasks matching the given filter.
fn match_task(_filter: &Filter, _task: &Task) -> bool {
// TODO: at the moment, only filtering by Universe is supported
true
}
/// Return the tasks matching the given filter. This will return each matching
/// task once, even if the user specified the same task multiple times on the
/// command line.
pub(super) fn filtered_tasks(
replica: &mut Replica,
filter: &Filter,
) -> Fallible<impl Iterator<Item = Task>> {
// For the moment, this gets the entire set of tasks and then iterates
// over the result. A few optimizations are possible:
//
// - id_list could be better parsed (id, uuid-fragment, uuid) in argparse
// - depending on the nature of the filter, we could just scan the working set
// - we could produce the tasks on-demand (but at the cost of holding a ref
// to the replica, preventing modifying tasks..)
let mut res = vec![];
'task: for (uuid, task) in replica.all_tasks()?.drain() {
if let Some(ref ids) = filter.id_list {
for id in ids {
if let Ok(index) = id.parse::<usize>() {
if replica.get_working_set_index(&uuid)? == Some(index) {
res.push(task);
continue 'task;
fn is_partial_uuid(taskid: &TaskId) -> bool {
match taskid {
TaskId::PartialUuid(_) => true,
_ => false,
}
}
// We will enumerate the universe of tasks for this filter, checking
// each resulting task with match_task
match filter.universe {
// A list of IDs, but some are partial so we need to iterate over
// all tasks and pattern-match their Uuids
Universe::IdList(ref ids) if ids.iter().any(is_partial_uuid) => {
'task: for (uuid, task) in replica.all_tasks()?.drain() {
for id in ids {
if match id {
TaskId::WorkingSetId(id) => {
// NOTE: (#108) this results in many reads of the working set; it
// may be better to cache this information here or in the Replica.
replica.get_working_set_index(&uuid)? == Some(*id)
}
TaskId::PartialUuid(prefix) => uuid.to_string().starts_with(prefix),
TaskId::Uuid(id) => id == &uuid,
} {
if match_task(filter, &task) {
res.push(task);
continue 'task;
}
}
}
}
}
// A list of full IDs, which we can fetch directly
Universe::IdList(ref ids) => {
// this is the only case where we might accidentally return the same task
// several times, so we must track the seen tasks.
let mut seen = HashSet::new();
for id in ids {
let task = match id {
TaskId::WorkingSetId(id) => replica.get_working_set_task(*id)?,
TaskId::PartialUuid(_) => unreachable!(), // handled above
TaskId::Uuid(id) => replica.get_task(id)?,
};
if let Some(task) = task {
// if we have already seen this task, skip ahead..
let uuid = *task.get_uuid();
if seen.contains(&uuid) {
continue;
}
seen.insert(uuid);
if match_task(filter, &task) {
res.push(task);
}
}
}
}
// All tasks -- iterate over the full set
Universe::AllTasks => {
for (_, task) in replica.all_tasks()?.drain() {
if match_task(filter, &task) {
res.push(task);
}
}
}
// Pending tasks -- just scan the working set
Universe::PendingTasks => {
for task in replica.working_set()?.drain(..) {
if let Some(task) = task {
if match_task(filter, &task) {
res.push(task);
}
} else if uuid.to_string().starts_with(id) {
res.push(task);
continue 'task;
}
}
} else {
// default to returning all tasks
res.push(task);
continue 'task;
}
}
Ok(res.into_iter())
}
#[cfg(test)]
mod test {
use super::*;
use crate::invocation::test::*;
use taskchampion::Status;
#[test]
fn exact_ids() {
let mut replica = test_replica();
let t1 = replica.new_task(Status::Pending, "A".to_owned()).unwrap();
let t2 = replica.new_task(Status::Completed, "B".to_owned()).unwrap();
let _t = replica.new_task(Status::Pending, "C".to_owned()).unwrap();
replica.gc().unwrap();
let t1uuid = *t1.get_uuid();
let filter = Filter {
universe: Universe::IdList(vec![
TaskId::Uuid(t1uuid), // A
TaskId::WorkingSetId(1), // A (again, dups filtered)
TaskId::Uuid(*t2.get_uuid()), // B
]),
..Default::default()
};
let mut filtered: Vec<_> = filtered_tasks(&mut replica, &filter)
.unwrap()
.map(|t| t.get_description().to_owned())
.collect();
filtered.sort();
assert_eq!(vec!["A".to_owned(), "B".to_owned()], filtered);
}
#[test]
fn partial_ids() {
let mut replica = test_replica();
let t1 = replica.new_task(Status::Pending, "A".to_owned()).unwrap();
let t2 = replica.new_task(Status::Completed, "B".to_owned()).unwrap();
let _t = replica.new_task(Status::Pending, "C".to_owned()).unwrap();
replica.gc().unwrap();
let t1uuid = *t1.get_uuid();
let t2uuid = t2.get_uuid().to_string();
let t2partial = t2uuid[..13].to_owned();
let filter = Filter {
universe: Universe::IdList(vec![
TaskId::Uuid(t1uuid), // A
TaskId::WorkingSetId(1), // A (again, dups filtered)
TaskId::PartialUuid(t2partial), // B
]),
..Default::default()
};
let mut filtered: Vec<_> = filtered_tasks(&mut replica, &filter)
.unwrap()
.map(|t| t.get_description().to_owned())
.collect();
filtered.sort();
assert_eq!(vec!["A".to_owned(), "B".to_owned()], filtered);
}
#[test]
fn all_tasks() {
let mut replica = test_replica();
replica.new_task(Status::Pending, "A".to_owned()).unwrap();
replica.new_task(Status::Completed, "B".to_owned()).unwrap();
replica.new_task(Status::Deleted, "C".to_owned()).unwrap();
replica.gc().unwrap();
let filter = Filter {
universe: Universe::AllTasks,
..Default::default()
};
let mut filtered: Vec<_> = filtered_tasks(&mut replica, &filter)
.unwrap()
.map(|t| t.get_description().to_owned())
.collect();
filtered.sort();
assert_eq!(
vec!["A".to_owned(), "B".to_owned(), "C".to_owned()],
filtered
);
}
#[test]
fn pending_tasks() {
let mut replica = test_replica();
replica.new_task(Status::Pending, "A".to_owned()).unwrap();
replica.new_task(Status::Completed, "B".to_owned()).unwrap();
replica.new_task(Status::Deleted, "C".to_owned()).unwrap();
replica.gc().unwrap();
let filter = Filter {
universe: Universe::PendingTasks,
..Default::default()
};
let mut filtered: Vec<_> = filtered_tasks(&mut replica, &filter)
.unwrap()
.map(|t| t.get_description().to_owned())
.collect();
filtered.sort();
assert_eq!(vec!["A".to_owned()], filtered);
}
}

View file

@ -10,6 +10,9 @@ mod cmd;
mod filter;
mod modify;
#[cfg(test)]
mod test;
use filter::filtered_tasks;
use modify::apply_modification;