Skip to content

[WIP] Fix IMAPMessage violating the IMessage contract#12614

Closed
Copilot wants to merge 1 commit into
mainfrom
copilot/fix-imapmessage-contract-violation
Closed

[WIP] Fix IMAPMessage violating the IMessage contract#12614
Copilot wants to merge 1 commit into
mainfrom
copilot/fix-imapmessage-contract-violation

Conversation

Copilot AI commented Mar 19, 2026

Copy link
Copy Markdown
Contributor

Thanks for assigning this issue to me. I'm starting to work on it and will keep this PR's description up to date as I form a plan and make progress.

Original prompt

This section details on the original issue you should resolve

<issue_title>IMAPMessage violating the IMessage contract</issue_title>
<issue_description>The interface is implemented by OCA\Mail\Model\Message and OCA\Mail\Model\IMAPMessage.

mail/lib/Model/IMessage.php

Lines 119 to 122 in 52214dc

/**
* @return Horde_Mime_Part[]
*/
public function getAttachments(): array;

OCA\Mail\Model\Message is okay:

mail/lib/Model/Message.php

Lines 291 to 305 in 52214dc

/**
* @param string $name
* @param string $content
* @param string $mime
* @return void
*/
private function createAttachmentDetails(string $name, string $content, string $mime): void {
$part = new Horde_Mime_Part();
$part->setCharset('us-ascii');
$part->setDisposition('attachment');
$part->setName($name);
$part->setContents($content);
$part->setType($mime);
$this->attachments[] = $part;
}

OCA\Mail\Model\IMAPMessage is not:

/**
* @return Horde_Mime_Part[]
*/
#[\Override]
public function getAttachments(): array {
return $this->attachments;
}

$this->attachments[] = [
'id' => $p->getMimeId(),
'messageId' => $this->uid,
'fileName' => $p->getName(),
'mime' => $p->getType(),
'size' => $p->getBytes(),
'cid' => $p->getContentId(),
'disposition' => $p->getDisposition()
];

$this->attachments[] = [
'id' => $p->getMimeId(),
'messageId' => $this->uid,
'fileName' => $p->getName(),
'mime' => $p->getType(),
'size' => $p->getBytes(),
'cid' => $p->getContentId(),
'disposition' => $p->getDisposition()
];

Solution

Drop the interface?

It looks like we are always using the actual classes instead of the interface. </issue_description>

<agent_instructions>Hi copilot, please remove the given IMessage interface. </agent_instructions>

Comments on the Issue (you are @copilot in this section)

@kesselb Wdyt @ChristophWurst @DerDreschner? @DerDreschner Hmm, I'm a bit uncertain about dropping the interface... But I'm not entirely sure about what's possible in PHP. @kesselb

In C#, we always used interfaces as the unit testing framework were able to automatically inject mocked classes instead of defining the mocking behavior for the same class over and over again. It looks like we don't have that possibility right now in our projects and I'm uncertain if there is a possibility to do that with phpunit at all.

So, depends on what's possible in theory IMHO?!</comment_new>


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

IMAPMessage violating the IMessage contract

2 participants