Support to add or remove flag(s) on a message

[jbostoen]

Feature Request

Allow this method to specify which flags need to be added or removed:

laminas-mail/src/Storage/Imap.php

Line 550 in bf75042

public function setFlags($id, $flags)

It seems to be available through the storage, but not on the message? Is this a limitation or just not implemented (yet) for some reason?

Q	A
New Feature	yes
RFC	no
BC Break	no

Summary

Allow this method to specify the mode:

laminas-mail/src/Storage/Imap.php

Line 550 in bf75042

public function setFlags($id, $flags)

It seems to be available through the protocol's store() method, but not directly on the message? Is this a limitation or just not implemented (yet) for some reason?

[jbostoen]

I dug around a bit and stumbled upon this comment: #179 (comment)

Is there a reason the "protocol" is protected? If so, would a PR be accepted which introduces this getProtocol() method natively? Or is even in the long term the only proper way to go to create a custom class?

[Ocramius]

In general, exposing internals increases the API surface, and therefore removes flexibility in upgrade/refactoring and increases the likeliness of breaking changes.

What is the use-case for exposing getProtocol()?

[jbostoen]

I'm porting some code which was using a different IMAP implementation that didn't support OAuth 2.0. There, in some cases, it turned out the IMAP "deleted" flag was accidentally set.

So before processing this IMAP folder, I just added a bit of code which removed the "delete" flag. Basically it would do the opposite of this method:

laminas-mail/src/Storage/Imap.php

Line 281 in bf75042

public function removeMessage($id)

The legacy implementation relies on this native PHP method: https://www.php.net/manual/en/function.imap-undelete.php

[Ocramius]

Well, the protocol is given at creation time, so you should be able to get an instance without asking it to the storage:

laminas-mail/src/Storage/Imap.php

Lines 201 to 206 in fe1ac4a

	public function __construct($params)
	{
	$this->has['flags'] = true;
	if ($params instanceof Protocol\Imap) {
	$this->protocol = $params;

The fact that Storage\Imap also creates a protocol (shady - old way of doing things :-( ) is kinda secondary, I'd say:

laminas-mail/src/Storage/Imap.php

Lines 221 to 235 in fe1ac4a

	$host = $params['host'] ?? 'localhost';
	$password = $params['password'] ?? '';
	$port = $params['port'] ?? null;
	$ssl = $params['ssl'] ?? false;
	$folder = $params['folder'] ?? 'INBOX';
	if (null !== $port) {
	$port = (int) $port;
	}
	if (! is_string($ssl)) {
	$ssl = (bool) $ssl;
	}
	$this->protocol = new Protocol\Imap();

My endorsement would be to create the protocol upfront, then keeping a reference to it when you need it: this also removes a lot of responsibilities from Storage\Imap (reads: less space for it to mess it up :D )

Sadly, we can't change that constructor anymore: too many BC implications.

[jbostoen]

Thanks for your time and detailed explanation!

So the best workaround is like the one in the comment I linked to earlier?

[Ocramius]

I think the best workaround would be to operate on the protocol itself, before passing it to the storage: is that workable?

[jbostoen]

Thanks for the advice, but for now it would be too complicated. Currently trying to make the replacement as much "drop in" as possible rather than revising the entire logic :)

[Ocramius]

Two workarounds endorsed:

1. using reflection (ugly, but will get you there, for now)
2. extending the storage to make the transport available

The correct solution is having the transport upfront, but these should work.

Meanwhile, closing as "won't fix" here.