Skip to content

INT-4299: Add AbstractMailReceiver.autoCloseFolder #2887

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

Merged
merged 2 commits into from
Apr 9, 2019

Conversation

artembilan
Copy link
Member

JIRA: https://jira.spring.io/browse/INT-4299

  • Optimize AbstractMessageSource do not evaluate headers when polling
    result is null
  • Optimize MailReceivingMessageSource do not wrap polling result to
    the message: the AbstractMessageSource will do that later

JIRA: https://jira.spring.io/browse/INT-4299

* Optimize `AbstractMessageSource` do not evaluate headers when polling
result is `null`
* Optimize `MailReceivingMessageSource` do not wrap polling result to
the message: the `AbstractMessageSource` will do that later
@artembilan
Copy link
Member Author

If this is OK, I'll add some docs on the matter.

Thanks

Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

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

Just minor javadoc polishing.

LGTM - awaiting docs.

/**
* Configure a {@code boolean} flag to close the folder automatically after fetch (default) or
* populate additional {@link IntegrationMessageHeaderAccessor#CLOSEABLE_RESOURCE} message header instead.
* It is downstream flow responsibility to obtain this header and call its {@code close()} whenever
Copy link
Contributor

Choose a reason for hiding this comment

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

after a fetch

populate an additional

is the downstream flow's responsibility

* server when parsing maltipart content of the email with attachments.
* <p> The {@link #setSimpleContent(boolean)} and {@link #setHeaderMapper(HeaderMapper)} options are not
* affected by this flag.
* @param autoCloseFolder {@code false} do not close the folder automatically after fetch.
Copy link
Contributor

Choose a reason for hiding this comment

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

Keeping the folder open is useful in cases where communication with the server is needed when parsing multipart (typo)

@@ -269,6 +269,21 @@ public ImapIdleChannelAdapterSpec embeddedPartsAsBytes(boolean embeddedPartsAsBy
return _this();
}

/**
* When configured to {@code false}, the folder is not closed automatically after fetch.
* It is target application responsibility to close it used
Copy link
Contributor

Choose a reason for hiding this comment

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

the target application's

@@ -253,6 +253,22 @@ public S simpleContent(boolean simpleContent) {
return _this();
}

/**
* When configured to {@code false}, the folder is not closed automatically after fetch.
* It is target application responsibility to close it used
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

* Document `autoCloseFolder` option
@@ -357,7 +378,9 @@ private Folder obtainFolderInstance() throws MessagingException {
return convertMessagesIfNecessary(searchAndFilterMessages());
}
finally {
MailTransportUtils.closeFolder(this.folder, this.shouldDeleteMessages);
if (this.autoCloseFolder) {
closeFolder();
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to close here if no messages were retrieved (needs a test),

Copy link
Member Author

Choose a reason for hiding this comment

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

Why? Why do we need to close the folder at all?
I don't see any comment in the Internet requiring to close the folder after each fetch.
😕

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@garyrussell garyrussell merged commit c05876e into spring-projects:master Apr 9, 2019
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.

2 participants