Implement bulk-move

[particleflux]

Implement a new bulk move feature to allow moving messages between mailboxes at once.

This is able to work with an array of message ids or a MessageIterator. For compatibility with the other bulk commands, there also is the possibility to directly input a list of comma separated message ids as a string.

Fixes #305

[Slamdunk]

The PR looks good.
May you implement this also for imap_mail_copy as the code is almost the same?

[Slamdunk]

I would like to maintain similarity with:

imap/src/Message.php

Lines 260 to 262 in f90d03f

	if (!\imap_mail_move($this->resource->getStream(), (string) $this->getNumber(), $mailbox->getEncodedName(), \CP_UID)) {
	throw new MessageMoveException(\sprintf('Message "%s" cannot be moved to "%s"', $this->getNumber(), $mailbox->getName()));
	}

Please remove any return and throw the Exception.

[particleflux]

I actually was about to implement it with an exception, but decided against, as the other bulk commands (clearFlag and setFlag) use a boolean return too.

[Slamdunk]

Nice, may you also face this:

May you implement this also for imap_mail_copy as the code is almost the same?

please?

[particleflux]

Implemented bulk copy too. I didn't want to produce too much duplicate code, so I refactored the preparation of the message ids into a separate method. This has the nice side effect that the flag commands now work with MessageIterators too.

I'm not sure if a private method is the nicest way to have that implemented. Maybe there could be some kind of MessageList class for that purpose. (That would produce overhead and backward incompatible changes for the caller though $mbox->setFlag(\Seen, new MessageList($array_of_ids))

[Slamdunk]

Implemented bulk copy too. I didn't want to produce too much duplicate code, so I refactored the preparation of the message ids into a separate method. This has the nice side effect that the flag commands now work with MessageIterators too.

Nice 👍

I'm not sure if a private method is the nicest way to have that implemented.

The private method is ok.

Great work. some other few things and 🚢

[Slamdunk]

getArrayCopy isn't a method of MessageIteratorInterface but only of MessageIterator: please change all the doc hints to the latter class

[particleflux]

right. fixed

[particleflux]

Or not. This is now breaking the build since the static analyser complains. I could fix this by typehinting the returned variable in the test - but is that such a good idea?

diff --git a/tests/MailboxTest.php b/tests/MailboxTest.php
index 89cd7c5..84a58da 100644
--- a/tests/MailboxTest.php
+++ b/tests/MailboxTest.php
@@ -10,6 +10,7 @@ use Ddeboer\Imap�xception\MessageDoesNotExistException;
 use Ddeboer\Imap�xception\MessageMoveException;
 use Ddeboer\Imap�xception\ReopenMailboxException;
 use Ddeboer\Imap\MailboxInterface;
+use Ddeboer\Imap\MessageIterator;
 
 /**
  * @covers \Ddeboer\Imap�xception\AbstractException
@@ -234,6 +235,7 @@ final class MailboxTest extends AbstractTest
         $this->assertSame(0, $this->mailbox->count());
 
         // move back by iterator
+        /** @var MessageIterator $messages */
         $messages = $anotherMailbox->getMessages();
         $anotherMailbox->move($messages, $this->mailbox);
         $this->getConnection()->expunge();

[Slamdunk]

but is that such a good idea?

The bad thing is getArrayCopy isn't present in any SPL interface, and this complicates everything a lot.
In a future release we could add it to MessageIteratorInterface but at the time being your solution is enough.

[Slamdunk]

MessageCopyException

[particleflux]

Fixed too.

[Slamdunk]

Thanks @particleflux

[Slamdunk]

Released in 1.3.0