From da9cfa6f4a1aa6cea532ae576f8321344bbb03be Mon Sep 17 00:00:00 2001 From: Bron Gondwana Date: Tue, 20 Aug 2024 00:06:27 +1000 Subject: [PATCH 1/4] sync_support: find the intermediate rename --- imap/sync_support.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/imap/sync_support.c b/imap/sync_support.c index a6a2e63230..0c01d1bcea 100644 --- a/imap/sync_support.c +++ b/imap/sync_support.c @@ -659,6 +659,18 @@ struct sync_folder *sync_folder_lookup(struct sync_folder_list *l, return NULL; } +const struct sync_folder *sync_folder_lookup_byname(struct sync_folder_list *l, + const char *name) +{ + struct sync_folder *p; + + for (p = l->head; p; p = p->next) { + if (!strcmp(p->name, name)) + return p; + } + return NULL; +} + void sync_folder_list_free(struct sync_folder_list **lp) { struct sync_folder_list *l = *lp; @@ -6811,6 +6823,32 @@ static int do_folders(struct sync_client_state *sync_cs, * short and contain few dependancies. Algorithm is to simply pick a * rename operation which has no dependancy and repeat until done */ + struct sync_rename *item; + for (item = rename_folders->head; item; item = item->next) { + if (!strcmp(item->oldname, item->newname)) continue; + if (!sync_folder_lookup_byname(replica_folders, item->oldname)) continue; + if (!sync_folder_lookup_byname(replica_folders, item->newname)) continue; + + // ok, it's a rename and both source and destination names exist already, + // is there an intermediate we can rename to? + + mbentry_t *mbentry_byid = NULL; + if (mboxlist_lookup_by_uniqueid(item->uniqueid, &mbentry_byid, NULL)) continue; + int i; + for (i = 0; i < ptrarray_size(&mbentry_byid->name_history); i++) { + const former_name_t *histitem = ptrarray_nth(&mbentry_byid->name_history, i); + if (sync_folder_lookup_byname(replica_folders, histitem->name)) continue; + // add a rename from old name to the temporary name + sync_rename_list_add(rename_folders, item->uniqueid, item->oldname, + histitem->name, item->part, histitem->uidvalidity); + // and then reuse this item for the rename from temporary to final + free(item->oldname); + item->oldname = xstrdup(histitem->name); + break; + } + mboxlist_entry_free(&mbentry_byid); + } + while (rename_folders->done < rename_folders->count) { int rename_success = 0; struct sync_rename *item, *item2 = NULL; @@ -6821,7 +6859,7 @@ static int do_folders(struct sync_client_state *sync_cs, /* don't skip rename to different partition */ if (strcmp(item->oldname, item->newname)) { item2 = sync_rename_lookup(rename_folders, item->newname); - if (item2 && !item2->done) continue; + if (item2 && !item2->done && sync_folder_lookup_byname(replica_folders, item->newname)) continue; } /* Found unprocessed item which should rename cleanly */ From 29e7975269e8a187d6bb554598ad3f7c9047de43 Mon Sep 17 00:00:00 2001 From: Bron Gondwana Date: Tue, 20 Aug 2024 08:43:30 +1000 Subject: [PATCH 2/4] mailbox: don't statuscache if deleted or renamed --- imap/mailbox.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/imap/mailbox.c b/imap/mailbox.c index d21ca18977..4f248da95a 100644 --- a/imap/mailbox.c +++ b/imap/mailbox.c @@ -2670,19 +2670,21 @@ EXPORTED void mailbox_unlock_index(struct mailbox *mailbox, struct statusdata *s abort(); } + // we always write if given new statusdata, or if we changed the mailbox + int write_status = !!sdata; if (mailbox->has_changed) { sync_log_mailbox(mailbox_name(mailbox)); - if (!sdata) { + if (!sdata && !(mailbox->i.options & OPT_MAILBOX_DELETED) && !strcmpsafe(mailbox_name(mailbox), mailbox->h.name)) { status_fill_mailbox(mailbox, &mysdata); sdata = &mysdata; } mailbox->has_changed = 0; + write_status = 1; } - // we always write if given new statusdata, or if we changed the mailbox - if (sdata) + if (write_status) statuscache_invalidate(mailbox_name(mailbox), sdata); if (mailbox->index_locktype) { From ff2c56d3475cbeb88e64354b9a498fc7f872b2fb Mon Sep 17 00:00:00 2001 From: Bron Gondwana Date: Mon, 19 Aug 2024 23:20:21 +1000 Subject: [PATCH 3/4] FastMail: test that rename a->c, b->a, c->b works Even if replication didn't catch each step of the way! --- .../FastMail/mailbox_rename_switch_ab | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 cassandane/tiny-tests/FastMail/mailbox_rename_switch_ab diff --git a/cassandane/tiny-tests/FastMail/mailbox_rename_switch_ab b/cassandane/tiny-tests/FastMail/mailbox_rename_switch_ab new file mode 100644 index 0000000000..1df0c98a7a --- /dev/null +++ b/cassandane/tiny-tests/FastMail/mailbox_rename_switch_ab @@ -0,0 +1,31 @@ +#!perl +use Cassandane::Tiny; + +sub test_mailbox_rename_switch_ab + :AllowMoves :Replication :SyncLog :min_version_3_3 + :needs_component_replication +{ + my ($self) = @_; + + my $mtalk = $self->{master_store}->get_client(); + + $mtalk->create('INBOX.Foo'); + $mtalk->create('INBOX.Bar'); + + # replicate and check initial state + my $synclogfname = "$self->{instance}->{basedir}/conf/sync/log"; + $self->run_replication(rolling => 1, inputfile => $synclogfname); + unlink($synclogfname); + $self->check_replication('cassandane'); + + # perform the three way rename + $mtalk = $self->{master_store}->get_client(); + $mtalk->rename('INBOX.Foo', 'Inbox.Foo2'); + $mtalk->rename('INBOX.Bar', 'Inbox.Foo'); + $mtalk->rename('INBOX.Foo2', 'Inbox.Bar'); + + # replicate and check that it syncs ok + $self->run_replication(rolling => 1, inputfile => $synclogfname); + unlink($synclogfname); + $self->check_replication('cassandane'); +} From 1cdad7a29f83c0f193f26e477d8f70382eda8483 Mon Sep 17 00:00:00 2001 From: Bron Gondwana Date: Tue, 20 Aug 2024 10:18:29 +1000 Subject: [PATCH 4/4] Fastmail: a long chain of renames test --- .../FastMail/mailbox_rename_long_chain | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 cassandane/tiny-tests/FastMail/mailbox_rename_long_chain diff --git a/cassandane/tiny-tests/FastMail/mailbox_rename_long_chain b/cassandane/tiny-tests/FastMail/mailbox_rename_long_chain new file mode 100644 index 0000000000..a8b14e4eb2 --- /dev/null +++ b/cassandane/tiny-tests/FastMail/mailbox_rename_long_chain @@ -0,0 +1,40 @@ +#!perl +use Cassandane::Tiny; + +sub test_mailbox_rename_long_chain + :AllowMoves :Replication :SyncLog :min_version_3_3 + :needs_component_replication +{ + my ($self) = @_; + + my $mtalk = $self->{master_store}->get_client(); + + $mtalk->create('INBOX.Foo'); + $mtalk->create('INBOX.Bar'); + + # replicate and check initial state + my $synclogfname = "$self->{instance}->{basedir}/conf/sync/log"; + $self->run_replication(rolling => 1, inputfile => $synclogfname); + unlink($synclogfname); + $self->check_replication('cassandane'); + + # perform the multi-path rename + $mtalk = $self->{master_store}->get_client(); + $mtalk->rename('INBOX.Bar', 'Inbox.Old'); + $mtalk->rename('INBOX.Foo', 'Inbox.Foo2'); + $mtalk->rename('INBOX.Foo2', 'Inbox.Foo3'); + $mtalk->rename('INBOX.Foo3', 'Inbox.Foo4'); + $mtalk->rename('INBOX.Foo4', 'Inbox.Foo5'); + $mtalk->rename('INBOX.Foo5', 'Inbox.Foo6'); + $mtalk->rename('INBOX.Foo6', 'Inbox.Foo7'); + $mtalk->rename('INBOX.Foo7', 'Inbox.Foo8'); + $mtalk->rename('INBOX.Foo8', 'Inbox.Bar'); + # Create a couple of intermediates again + $mtalk->create('INBOX.Foo5'); + $mtalk->create('INBOX.Foo2'); + + # replicate and check that it syncs ok + $self->run_replication(rolling => 1, inputfile => $synclogfname); + unlink($synclogfname); + $self->check_replication('cassandane'); +}