Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

problem with ->moveToFolder() and multiple moves #31

Closed
mundanet opened this issue Oct 17, 2017 · 24 comments
Closed

problem with ->moveToFolder() and multiple moves #31

mundanet opened this issue Oct 17, 2017 · 24 comments
Assignees

Comments

@mundanet
Copy link

->moveToFolder() works perfectly when moving a single email in a single session.
But it fails to move correct emails when moving more than one email per session.
Debugging with xdebug I can confirm that the sequence numbers got confused when expunging messages.
I find the affirmations at the following link to be correct:
http://gabijack.com/moving-and-deleting-messages-with-php-imap/

May I request a fix?

@Webklex
Copy link
Owner

Webklex commented Oct 17, 2017

Hi @mundanet ,
thanks for your report. You are always welcome to suggest any fix or feature :)
I will verify this issue and provide an update as soon as possible.

Best regards

@Webklex Webklex self-assigned this Oct 17, 2017
@Webklex Webklex added the bug label Oct 17, 2017
@mundanet
Copy link
Author

mundanet commented Oct 17, 2017

HI @Webklex I have tested a solution:
Client.php

    /**
     * Messages to be deleted
     *
     * @var array
     */
    public $msglist = [];

    /**
     * Move marked messages to Folder.
     *
     * @param string $mailbox
     *
     * @return boolean
     */
    public function moveToFolder($mailbox = 'INBOX') {
        $this->checkConnection();
        $this->createFolder($mailbox);

        if(imap_mail_move($this->connection, implode(',', $this->msglist), $mailbox) == true){
            return $this->expunge();
        }
        return false;
    }

in Message.php

    /**
     * mark the Message to be moved into another Folder
     *
     *
     */
    public function markToBeMoved(){
        array_push($this->client->msglist, $this->msglist);
    }

and it is working.
Not the nicest code but it is a good starting point

@Webklex
Copy link
Owner

Webklex commented Oct 18, 2017

Hi @mundanet ,
I just released a new version. Thanks for your suggestions and support :)

Please reffer to: v1.0.3.5

@Webklex Webklex closed this as completed Oct 18, 2017
@theparker
Copy link

@Webklex
I am having a same issue with $message->moveToFolder()
The error I get from imap_last_error() is [CLOSED] IMAP connection lost
The first few messages are moved successfully but after moving roughly 8 messages, I consistently get the connection lost error.
Is there a way to say increase connection timeout? I was thinking the checkConnection() function which is called in createFolder() should reestablish connection but it doesn't seem like that works like it is supposed to.
What would you suggest?

@Webklex Webklex reopened this Apr 3, 2019
@theparker
Copy link

It seems the problem is coming from calling $client->createFolder(); inside $message->moveToFolder() as commenting it out works.

I don't know if it is a bug, i.e. should have been $client->checkConnection() instead of $client->createFolder() but you may want to consider removing createFolder() from moveToFolder() as calling moveToFolder() a thousand times would mean a thousand attempt to create the same folder.

PS; Thanks for the effort in creating and maintaining this library. I like the idea of having message and attachment objects.

@gareth-ib
Copy link
Contributor

I'm having a similar problem.
Both folders get created when the code runs, but the email it finds gets moved into the processed folder, but doesn't move into the review folder.
I don't see anything in the code indicating why this would be happening. Any ideas?
Thanks!

namespace App\Console\Commands;

use Illuminate\Console\Command;
use Webklex\IMAP\Client as ImapClient;

class ImapTest extends Command
{
    protected $signature = 'ImapTest';

    public function handle()
    {
        
        $oClient = new ImapClient([
            'host'          => 'imap.gmail.com',
            'port'          => 993,
            'encryption'    => 'ssl',
            'validate_cert' => true,
            'username'      => env( 'EMAIL_USERNAME' ),
            'password'      => env( 'EMAIL_APP_PASSWORD' ),
            'protocol'      => 'imap'
        ]);
        
        $folder = $oClient->getFolder('INBOX');
        
        /**
         * @var \Webklex\IMAP\Support\MessageCollection $messages
         */
        $messages = $folder->messages()->limit(1)->get();
        
        /**
          * @var \Webklex\IMAP\Message $message
          */
        foreach( $messages as $message ) {
            $message->moveToFolder('processing');
            sleep(5);
            $message->moveToFolder('review');
        }

    }

}

@theparker
Copy link

@gareth-ib Use imap_last_error() after the last call to moveToFolder() to see what errors are returned.

I think after moving the message to the processing folder, you could reload the message object (as it likely will be referencing the message in the initial folder) before moving to the review folder or you could call $client->expunge() before the second call to moveToFolder()

Either way, checking imap errors should help you identify the problem.

@gareth-ib
Copy link
Contributor

I tried your idea...

            $message->moveToFolder('processing');

            sleep(5);

            $oClient->expunge();

            $message->moveToFolder('review');
            $error = imap_last_error();
            dump($error);

            $oClient->expunge();

            $message->move('review');
            $error = imap_last_error();
            dump($error);

and got...

"[ALREADYEXISTS] Duplicate folder name review (Failure)"
"Could not parse command"

with the email still sitting in the processing folder

@theparker
Copy link

Comment out $this->client->createFolder($mailbox); in the moveToFolder() function in webklex/php-imap/src/Message.php and try again.

I fixed the issue I was having with lost connection by commenting that code out. I don't think a call to moveToFolder() should attempt creating a folder.

/**
     * Move the Message into an other Folder
     * @param string $mailbox
     *
     * @return bool
     * @throws Exceptions\ConnectionFailedException
     */
    public function moveToFolder($mailbox = 'INBOX') {
        //$this->client->createFolder($mailbox); /*Todo: Reference: https://github.com/Webklex/laravel-imap/issues/31*/

        return imap_mail_move($this->client->getConnection(), $this->uid, $mailbox, CP_UID);
    }

@gareth-ib
Copy link
Contributor

with the createFolder commented out...

false
"Could not parse command"

and the mail is still in the processing (first) folder

@Webklex
Copy link
Owner

Webklex commented Apr 12, 2019

Hi @gareth-ib ,
plase try to use INBOX.processing or INBOX/processing depending on your delimiter as $mailbox instead.

You might also want to update to the latest version or perhaps the master for futher investigation :)

Best regards

@gareth-ib
Copy link
Contributor

@Webklex ok I apparently was on 1.2, so I upgraded to 1.4...
I ran it with the variations of the INBOX included in the folder name, and the results were the same. But into new folders as seen

image

no prefix...

"[ALREADYEXISTS] Duplicate folder name review (Failure)"
"Could not parse command"

INBOX/ prefix

false
"Could not parse command"

INBOX. prefix ( these folders already existed )

"[ALREADYEXISTS] Duplicate folder name INBOX.review (Failure)"
"Could not parse command"

@Webklex
Copy link
Owner

Webklex commented Apr 12, 2019

Hi @gareth-ib,
I found the issue and provided a fix.

The MessageID or UID changes by moving the message. Therefore it wasn't possible to move the same Message/UID again.

The provided patch does exactly that.
The target mailbox will return the next UID and return a new message instance based on it if the message could be moved. The returned message has now a valid and fresh UID which can be moved again.

Please give the current master version a try. If it works as expected I will release a new version :)

Best regards

@gareth-ib
Copy link
Contributor

okay I've got it updated...

  - Removing webklex/laravel-imap (1.4.0)
  - Installing webklex/laravel-imap (dev-master 6990abb): Downloading (100%)

and the code...

            $message->moveToFolder('processing');

            sleep(5);

            $oClient->expunge();

            $message->moveToFolder('review');
            $error = imap_last_error();
            dump($error);

            $oClient->expunge();

            $message->move('review');
            $error = imap_last_error();
            dump($error);

output ( same result if I remove the $oClient->expunge(); commands )

"[ALREADYEXISTS] Duplicate folder name review (Failure)"
"Could not parse command"

@gareth-ib
Copy link
Contributor

this is on Gmail by the way, if that helps

@Webklex
Copy link
Owner

Webklex commented Apr 12, 2019

Remember Message::moveToFolder() returns the new Message (the moved instance). The old instance is now no longer valid.

echo "message UID: ".$message->uid."\n";

$processing_message = $message->moveToFolder('processing', true, false);
sleep(5);

if($processing_message !== null){
    echo "processing UID: ". $processing_message->uid."\n";

    $review_message = $processing_message->moveToFolder('review', true, false);
    if($review_message !== null){
        echo "review UID: ". $review_message->uid."\n";
    }else{
        dd(imap_last_error());
    }
}else{
    dd(imap_last_error());
}

Best regards

@gareth-ib
Copy link
Contributor

I added an echo into the moveToFolder

        if($status === true){
            if($expunge) $this->client->expunge();
echo __LINE__.':'.$target_status->uidnext."\n";
            return $target_folder->getMessage($target_status->uidnext);
        }

and ran the code...

            $movedMessage = $message->moveToFolder('processing');
            dd($movedMessage);
            sleep(5);
            $movedMessage->moveToFolder('review');

and the result is...

967:15
null

@Webklex
Copy link
Owner

Webklex commented Apr 12, 2019

And if you use this it's still null?

$movedMessage = $message->moveToFolder('processing', true, false);

By the way do you do anything else with $message before you move it?

@gareth-ib
Copy link
Contributor

yeah ran it with your demo test code and the result is...

message UID: 906
false

@gareth-ib
Copy link
Contributor

gareth-ib commented Apr 12, 2019

oh no I don't do anything to the $message before the move, it's just like the loop I posted originally

@Webklex
Copy link
Owner

Webklex commented Apr 12, 2019

I found an other issue when handling with multiple connections. This should get rid of it and finally solve your problem :)

@gareth-ib
Copy link
Contributor

hooraayy! thanks! looks like it was a lot of changes! haha

@gareth-ib
Copy link
Contributor

oh I confirmed it works btw :)

@Webklex Webklex closed this as completed Apr 12, 2019
@gareth-ib
Copy link
Contributor

found a problem with the returned message not having attachments like the orig message did. submitted PR #209

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants