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

Multipart performs blocking call in every instantiation #699 #716

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

jbescos
Copy link
Member

@jbescos jbescos commented Mar 4, 2024

#699

It will try to get the StreamProvider from the session of the Message.

api/src/main/java/jakarta/mail/Multipart.java Outdated Show resolved Hide resolved
api/src/main/java/jakarta/mail/Multipart.java Outdated Show resolved Hide resolved
api/src/main/java/jakarta/mail/Message.java Outdated Show resolved Hide resolved
api/src/main/java/jakarta/mail/Message.java Outdated Show resolved Hide resolved
@jmehrens jmehrens linked an issue Mar 6, 2024 that may be closed by this pull request
@jbescos
Copy link
Member Author

jbescos commented Mar 6, 2024

@jmehrens I am redesigning it, because I found more usages of StreamProvider. I moved the getStreamProvider to the Part interface.

Signed-off-by: Jorge Bescos Gascon <[email protected]>
Copy link
Contributor

@jmehrens jmehrens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like your new approach. Very clever.

api/src/main/java/jakarta/mail/Multipart.java Outdated Show resolved Hide resolved
api/src/main/java/jakarta/mail/Part.java Outdated Show resolved Hide resolved
@jmehrens
Copy link
Contributor

jmehrens commented Mar 7, 2024

jbescos added 2 commits March 7, 2024 09:05
Signed-off-by: Jorge Bescos Gascon <[email protected]>
Signed-off-by: Jorge Bescos Gascon <[email protected]>
@jbescos
Copy link
Member Author

jbescos commented Mar 7, 2024

Need to add entry to https://github.com/jakartaee/mail-api/blob/master/doc/release/CHANGES.txt

Done

@jbescos
Copy link
Member Author

jbescos commented Mar 7, 2024

Lets wait for @lukasj review too. My impression is this requires the new version 2.2.0, and we will need other PR before to upgrade to that version.

@jbescos jbescos requested a review from lukasj March 7, 2024 10:40
Signed-off-by: Jorge Bescos Gascon <[email protected]>
jbescos added 2 commits March 8, 2024 06:27
Signed-off-by: Jorge Bescos Gascon <[email protected]>
Signed-off-by: Jorge Bescos Gascon <[email protected]>
Copy link
Contributor

@jmehrens jmehrens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your approach is awesome! Looks good!

@jmehrens
Copy link
Contributor

jmehrens commented Mar 8, 2024

Side note: we'll have to review some of the Angus packages that also extend the classes here. DSN is an example. I glossed over it and I think it is a case of no changes need but something we should keep an eye on.

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.

Multipart performs blocking call in every instantiation
2 participants