fix #3526: correctly filter tasks with 'since' and 'until'

The previous assumption was that the Tasks returned by the Iterator are
sorted by the starttime, but that is not actually the case, and
could never have been, since we append the tasks into the log when
they are finished (not started) and running tasks are always iterated
first.

To correctly filter (and simplify the the api call) we forgo the
combinators, and use a for loop instead. This way we only have to do
the since/until checks only once per Task, but have to do the
start/limit counting ourselves.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
This commit is contained in:
Dominik Csapak 2021-07-14 09:30:26 +02:00 committed by Dietmar Maurer
parent a5d51b0c4f
commit 9517a5759a

View File

@ -451,63 +451,73 @@ pub fn list_tasks(
let list = TaskListInfoIterator::new(running)?;
let limit = if limit > 0 { limit as usize } else { usize::MAX };
let result: Vec<TaskListItem> = list
.skip_while(|info| {
match (info, until) {
(Ok(info), Some(until)) => info.upid.starttime > until,
(Ok(_), None) => false,
(Err(_), _) => false,
}
})
.take_while(|info| {
match (info, since) {
(Ok(info), Some(since)) => info.upid.starttime > since,
(Ok(_), None) => true,
(Err(_), _) => false,
}
})
.filter_map(|info| {
let mut skipped = 0;
let mut result: Vec<TaskListItem> = Vec::new();
for info in list {
let info = match info {
Ok(info) => info,
Err(_) => return None,
Err(_) => break,
};
if let Some(until) = until {
if info.upid.starttime > until {
continue;
}
}
if let Some(since) = since {
if let Some(ref state) = info.state {
if state.endtime() < since {
// we reached the tasks that ended before our 'since'
// so we can stop iterating
break;
}
}
if info.upid.starttime < since {
continue;
}
}
if !list_all && check_task_access(&auth_id, &info.upid).is_err() {
return None;
continue;
}
if let Some(needle) = &userfilter {
if !info.upid.auth_id.to_string().contains(needle) { return None; }
if !info.upid.auth_id.to_string().contains(needle) { continue; }
}
if let Some(store) = store {
if !check_job_store(&info.upid, store) {
return None;
}
if !check_job_store(&info.upid, store) { continue; }
}
if let Some(typefilter) = &typefilter {
if !info.upid.worker_type.contains(typefilter) {
return None;
}
if !info.upid.worker_type.contains(typefilter) { continue; }
}
match (&info.state, &statusfilter) {
(Some(_), _) if running => return None,
(Some(crate::server::TaskState::OK { .. }), _) if errors => return None,
(Some(_), _) if running => continue,
(Some(crate::server::TaskState::OK { .. }), _) if errors => continue,
(Some(state), Some(filters)) => {
if !filters.contains(&state.tasktype()) {
return None;
continue;
}
},
(None, Some(_)) => return None,
(None, Some(_)) => continue,
_ => {},
}
Some(info.into())
}).skip(start as usize)
.take(limit)
.collect();
if skipped < start as usize {
skipped += 1;
continue;
}
result.push(info.into());
if result.len() >= limit {
break;
}
}
let mut count = result.len() + start as usize;
if !result.is_empty() && result.len() >= limit { // we have a 'virtual' entry as long as we have any new