fix repeated listing of same Store with different flags
multiple Channels can call driver_t::list_store() with different LIST_*
flags. assuming the flags are actually taken into consideration, using a
single boolean 'listed' flag to track whether the Store still needs to
be listed obviously wouldn't cut it - if INBOX does not live right under
Path and the Channels used entirely disjoint Patterns (say, * and
INBOX*), the second Channel in a single run (probably a Group) would
fail to match anything.
to fix this, make store_t::listed more granular. this also requires
moving its handling back into the drivers (thus reverting c66afdc0
),
because the actually performed queries and their possible implicit
results are driver-specific.
note that this slightly pessimizes some cases - e.g., an IMAP Store with
Path "" will now list the entire namespace even if there is only one
Channel with Pattern "INBOX*" (because a hypothetical Pattern "*" would
also include INBOX*, and the queries are kept disjoint to avoid the need
for de-duplication). this isn't expected to be a problem, as listing
mailboxes is generally cheap.
This commit is contained in:
parent
b9505301cc
commit
416ced25dd
|
@ -2759,23 +2759,35 @@ imap_list_store( store_t *gctx, int flags,
|
||||||
imap_cmd_refcounted_state_t *sts = imap_refcounted_new_state( cb, aux );
|
imap_cmd_refcounted_state_t *sts = imap_refcounted_new_state( cb, aux );
|
||||||
|
|
||||||
// ctx->prefix may be empty, "INBOX.", or something else.
|
// ctx->prefix may be empty, "INBOX.", or something else.
|
||||||
// 'flags' may be LIST_INBOX, LIST_PATH (or LIST_PATH_MAYBE), or both.
|
// 'flags' may be LIST_INBOX, LIST_PATH (or LIST_PATH_MAYBE), or both. 'listed'
|
||||||
// This matrix determines what to query, and what comes out as a side effect:
|
// already containing a particular value effectively removes it from 'flags'.
|
||||||
|
// This matrix determines what to query, and what comes out as a side effect.
|
||||||
|
// The lowercase letters indicate unnecessary results; the queries are done
|
||||||
|
// this way to have non-overlapping result sets, so subsequent calls create
|
||||||
|
// no duplicates:
|
||||||
//
|
//
|
||||||
// qry \ pfx | empty | inbox | other
|
// qry \ pfx | empty | inbox | other
|
||||||
// ----------+-------+-------+-------
|
// ----------+-------+-------+-------
|
||||||
// inbox | i | i [p] | i
|
// inbox | p [I] | I [p] | I
|
||||||
// both | p [i] | i [p] | i + p
|
// both | P [I] | I [P] | I + P
|
||||||
// path | p [i] | p {i} | p
|
// path | P [i] | i [P] | P
|
||||||
//
|
//
|
||||||
// {i} => This doesn't actually contain INBOX itself, only its subfolders.
|
int pfx_is_empty = !*ctx->prefix;
|
||||||
//
|
int pfx_is_inbox = !pfx_is_empty && is_inbox( ctx, ctx->prefix, -1 );
|
||||||
if ((flags & (LIST_PATH | LIST_PATH_MAYBE)) && (!(flags & LIST_INBOX) || !is_inbox( ctx, ctx->prefix, -1 )))
|
if (((flags & (LIST_PATH | LIST_PATH_MAYBE)) || pfx_is_empty) && !pfx_is_inbox && !(ctx->gen.listed & LIST_PATH)) {
|
||||||
|
ctx->gen.listed |= LIST_PATH;
|
||||||
|
if (pfx_is_empty)
|
||||||
|
ctx->gen.listed |= LIST_INBOX;
|
||||||
imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box,
|
imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box,
|
||||||
"LIST \"\" \"%\\s*\"", ctx->prefix );
|
"LIST \"\" \"%\\s*\"", ctx->prefix );
|
||||||
if ((flags & LIST_INBOX) && (!(flags & (LIST_PATH | LIST_PATH_MAYBE)) || *ctx->prefix))
|
}
|
||||||
|
if (((flags & LIST_INBOX) || pfx_is_inbox) && !pfx_is_empty && !(ctx->gen.listed & LIST_INBOX)) {
|
||||||
|
ctx->gen.listed |= LIST_INBOX;
|
||||||
|
if (pfx_is_inbox)
|
||||||
|
ctx->gen.listed |= LIST_PATH;
|
||||||
imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box,
|
imap_exec( ctx, imap_refcounted_new_cmd( sts ), imap_refcounted_done_box,
|
||||||
"LIST \"\" INBOX*" );
|
"LIST \"\" INBOX*" );
|
||||||
|
}
|
||||||
imap_refcounted_done( sts );
|
imap_refcounted_done( sts );
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -300,6 +300,10 @@ maildir_list_maildirpp( store_t *gctx, int flags, const char *inbox )
|
||||||
int warned = 0;
|
int warned = 0;
|
||||||
struct stat st;
|
struct stat st;
|
||||||
|
|
||||||
|
if (gctx->listed & LIST_PATH) // Implies LIST_INBOX
|
||||||
|
return 0;
|
||||||
|
|
||||||
|
if (!(gctx->listed & LIST_INBOX))
|
||||||
add_string_list( &gctx->boxes, "INBOX" );
|
add_string_list( &gctx->boxes, "INBOX" );
|
||||||
|
|
||||||
char path[_POSIX_PATH_MAX];
|
char path[_POSIX_PATH_MAX];
|
||||||
|
@ -317,6 +321,8 @@ maildir_list_maildirpp( store_t *gctx, int flags, const char *inbox )
|
||||||
char name[_POSIX_PATH_MAX];
|
char name[_POSIX_PATH_MAX];
|
||||||
char *effName = name;
|
char *effName = name;
|
||||||
if (*ent == '.') {
|
if (*ent == '.') {
|
||||||
|
if (gctx->listed & LIST_INBOX)
|
||||||
|
continue;
|
||||||
if (!*++ent)
|
if (!*++ent)
|
||||||
continue;
|
continue;
|
||||||
// The Maildir++ Inbox is technically not under Path (as there is none), so
|
// The Maildir++ Inbox is technically not under Path (as there is none), so
|
||||||
|
@ -346,6 +352,10 @@ maildir_list_maildirpp( store_t *gctx, int flags, const char *inbox )
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
closedir (dir);
|
closedir (dir);
|
||||||
|
|
||||||
|
if (flags & (LIST_PATH | LIST_PATH_MAYBE))
|
||||||
|
gctx->listed |= LIST_PATH;
|
||||||
|
gctx->listed |= LIST_INBOX;
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -384,14 +394,14 @@ maildir_list_recurse( store_t *gctx, int isBox, int flags,
|
||||||
continue;
|
continue;
|
||||||
pl += pathLen;
|
pl += pathLen;
|
||||||
if (inbox && equals( path, pl, inbox, inboxLen )) {
|
if (inbox && equals( path, pl, inbox, inboxLen )) {
|
||||||
/* Inbox nested into Path. List now if it won't be listed separately anyway. */
|
// Inbox nested into Path.
|
||||||
if (!(flags & LIST_INBOX) && maildir_list_inbox( gctx, flags, 0 ) < 0) {
|
if (maildir_list_inbox( gctx, flags, 0 ) < 0) {
|
||||||
closedir( dir );
|
closedir( dir );
|
||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
} else if (basePath && equals( path, pl, basePath, basePathLen )) {
|
} else if (basePath && equals( path, pl, basePath, basePathLen )) {
|
||||||
/* Path nested into Inbox. List now if it won't be listed separately anyway. */
|
// Path nested into Inbox.
|
||||||
if (!(flags & (LIST_PATH | LIST_PATH_MAYBE)) && maildir_list_path( gctx, flags, 0 ) < 0) {
|
if (maildir_list_path( gctx, flags, 0 ) < 0) {
|
||||||
closedir( dir );
|
closedir( dir );
|
||||||
return -1;
|
return -1;
|
||||||
}
|
}
|
||||||
|
@ -433,6 +443,10 @@ maildir_list_inbox( store_t *gctx, int flags, const char *basePath )
|
||||||
{
|
{
|
||||||
char path[_POSIX_PATH_MAX], name[_POSIX_PATH_MAX];
|
char path[_POSIX_PATH_MAX], name[_POSIX_PATH_MAX];
|
||||||
|
|
||||||
|
if (gctx->listed & LIST_INBOX)
|
||||||
|
return 0;
|
||||||
|
gctx->listed |= LIST_INBOX;
|
||||||
|
|
||||||
add_string_list( &gctx->boxes, "INBOX" );
|
add_string_list( &gctx->boxes, "INBOX" );
|
||||||
return maildir_list_recurse(
|
return maildir_list_recurse(
|
||||||
gctx, 1, flags, 0, 0, basePath, basePath ? strlen( basePath ) - 1 : 0,
|
gctx, 1, flags, 0, 0, basePath, basePath ? strlen( basePath ) - 1 : 0,
|
||||||
|
@ -445,6 +459,10 @@ maildir_list_path( store_t *gctx, int flags, const char *inbox )
|
||||||
{
|
{
|
||||||
char path[_POSIX_PATH_MAX], name[_POSIX_PATH_MAX];
|
char path[_POSIX_PATH_MAX], name[_POSIX_PATH_MAX];
|
||||||
|
|
||||||
|
if (gctx->listed & LIST_PATH)
|
||||||
|
return 0;
|
||||||
|
gctx->listed |= LIST_PATH;
|
||||||
|
|
||||||
if (maildir_ensure_path( (maildir_store_conf_t *)gctx->conf ) < 0)
|
if (maildir_ensure_path( (maildir_store_conf_t *)gctx->conf ) < 0)
|
||||||
return -1;
|
return -1;
|
||||||
return maildir_list_recurse(
|
return maildir_list_recurse(
|
||||||
|
|
|
@ -945,7 +945,7 @@ store_connected( int sts, void *aux )
|
||||||
case DRV_CANCELED:
|
case DRV_CANCELED:
|
||||||
return;
|
return;
|
||||||
case DRV_OK:
|
case DRV_OK:
|
||||||
if (!mvars->skip && !mvars->chanptr->boxlist && mvars->chan->patterns && !mvars->ctx[t]->listed) {
|
if (!mvars->skip && !mvars->chanptr->boxlist && mvars->chan->patterns) {
|
||||||
for (cflags = 0, cpat = mvars->chan->patterns; cpat; cpat = cpat->next) {
|
for (cflags = 0, cpat = mvars->chan->patterns; cpat; cpat = cpat->next) {
|
||||||
const char *pat = cpat->string;
|
const char *pat = cpat->string;
|
||||||
if (*pat != '!') {
|
if (*pat != '!') {
|
||||||
|
@ -1004,7 +1004,6 @@ store_listed( int sts, void *aux )
|
||||||
case DRV_CANCELED:
|
case DRV_CANCELED:
|
||||||
return;
|
return;
|
||||||
case DRV_OK:
|
case DRV_OK:
|
||||||
mvars->ctx[t]->listed = 1;
|
|
||||||
if (DFlags & DEBUG_MAIN) {
|
if (DFlags & DEBUG_MAIN) {
|
||||||
debug( "got mailbox list from %s:\n", str_ms[t] );
|
debug( "got mailbox list from %s:\n", str_ms[t] );
|
||||||
for (bx = mvars->ctx[t]->boxes; bx; bx = bx->next)
|
for (bx = mvars->ctx[t]->boxes; bx; bx = bx->next)
|
||||||
|
|
Loading…
Reference in New Issue
Block a user