feature: mail provider backend

[SebastianKrupinski]

• Resolves: Calendar invites: Administrator shall choose if server's or user's email adress is used #5080

Summary

Initial Mail Provider Implementation

TODO

• Copyright
• Comments
• Minor code improvements

[github-advanced-security]

Psalm found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

[SebastianKrupinski]

TODO: Separate attachment handling for local files (saved on the system) and streamed files (generate in memory)

[SebastianKrupinski]

TODO: unit tests

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

[ChristophWurst]

Looks sane

[kesselb]

• IAddress.getAddress
• IAddress.getLabel
• IAttachment.getName
• IAttachment.getType
• IAttachment.getContents
• IMessage.getFrom
• IMessage.getReplyTo
• IMessage.getTo
• IMessage.getCC
• IMessage.getBcc
• IMessage.getSubject
• IMessage.getBody
• IMessage.getBodyHtml
• IMessage.getBodyPlain
• IMessage.getAttachments

Each of the above methods can return null.
This should be reworked.

For example, IAttachment:

What's the use case for having an attachment object with a name and a type but content's null, and what is the consumer supposed to do with such an object?

• https://github.com/nextcloud/mail/pull/9651/files#diff-396c7c8ae7dacf5cd102f6d592a4dc8b485833c22e9265d4b24b062164d2db72R47-R52
• https://github.com/nextcloud/mail/blob/dd160e64d854ef915f4eab9217b1c84b9902bfa4/lib/Service/Attachment/AttachmentService.php#L95

<?xml version="1.0" encoding="utf-8"?>
<d:error xmlns:d="DAV:" xmlns:s="http://sabredav.org/ns">
  <s:exception>TypeError</s:exception>
  <s:message>OCA\Mail\Service\Attachment\AttachmentService::addFileFromString(): Argument #4 ($fileContents) must be of type string, null given, called in /var/www/html/apps-extra/mail/lib/Provider/Command/MessageSend.php on line 47</s:message>
</d:error>

The requirements for the public api should be as strict as possible and can be extended when necessary. We don't have a use case for having a "null" attachment and therefore should not implement it. The same for every other method from the list above.

[kesselb]

• IProvider.createService
• IProvider.modifyService
• IProvider.deleteService

Are unused and the implemention in https://github.com/nextcloud/mail/pull/9651/files#diff-863bf0fae4943be717ec0db9eba3ff621d28665362e192e0b6d45a2b917426f7R190-R236 does nothing and therefore should be removed.

[st3iny]

The API looks solid to me and I like the design overall.

However, I agree with kesselb's latest points and have some additional minor high level feedback:

1. Please get rid of IServiceIdentity and IServiceLocation interfaces. We can always add new interfaces once those two features are needed and ready. Currently, they seem to be unused.
2. All methods inside IMessage that return an array or null should only return an (empty) array. This makes consuming the API more convenient as both cases don't need to be handled separately despite being semantically identical. (As kesselb mentioned, there are some more methods inside IAttachment where this also applies.)

[SebastianKrupinski]

@st3iny @kesselb

I've changed the empty return type on the functions that return an array to an empty array, you have a valid point about the easier consumption of the array.

I have also removed the extra IServiceIdentity and IServiceLocation interfaces and methods, even tho I have a feeling I might be adding them back in next month from a conversion I had this week, but I save the code so that should not be a big deal.

[miaulalala]

Aside from the comments by Daniel and Richard this looks fine.

[st3iny]

Feedback has been addressed.

[kesselb]

#45383 (comment)

[kesselb]

I'm aware that Beta 1 is due on 2024-07-25, but we can always backport changes if necessary.

• Most methods still accept null values for no reason. I would prefer to sort that out before merging. It's easier to make an API less strict later, than the other way around.
• https://github.com/nextcloud/server/blob/cf432cf1c31908a45adde486ff4353a37254e8ed/lib/public/Mail/Provider/Message.php Please use class properties rather then $this->data.

[kesselb]

• Sebastian and I discussed the nullable return types and agreed to keep the nullable return types.
• We've learned that there's a need for api consumers to signal back that an entity is unprocessable and therefore will add an exception to ocp.

Sorted out

[ChristophWurst]

@SebastianKrupinski this PR adds new APIs to lib/public aka OCP, so we need developer docs. You can take https://docs.nextcloud.com/server/latest/developer_manual/digging_deeper/groupware/calendar.html as inspiration.

[susnux]

Documentation was added: nextcloud/documentation#12083