-
Couldn't load subscription status.
- Fork 287
feat: provide attachement names for messages #11596
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
base: main
Are you sure you want to change the base?
Conversation
|
I realized this can be merged independently from #11529, rebasing |
Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>
c8079fb to
185eb96
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works, but I'm not a big fan. It's using the distributed cache as storage. Cons:
- Does not work when there is no distributed cache. It will still do the expensive fetching, but the results are losts.
- For small installations that use APCu for local+distributed caching this will inflate cache memory quite a bit and risk that the cache is fully evicted because it's running full.
| $result = array_map(static function (array $attachment) { | ||
| return $attachment['fileName']; | ||
| }, $attachments); | ||
| $this->cache->set($message->getUid(), $result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UIDs are unique in the scope of a single mailbox. You'll run into conflicts across mailboxes, and worse, across user accounts. Use the primary key or find another unique identifier.
Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Read the code wrong. Does exactly what we discussed ![]()
No description provided.