Mail: Proposal for removing constructors in `AbstractMailReceiver` which support `url` to be null

[mjd507]

current in AbstractMailReceiver, we have 3 constructors.

        private final URLName url;

	public AbstractMailReceiver() {
		this.url = null;
	}

	public AbstractMailReceiver(URLName urlName) {
		Assert.notNull(urlName, "urlName must not be null");
		this.url = urlName;
	}

	public AbstractMailReceiver(String url) {
		if (url != null) {
			this.url = new URLName(url);
		}
		else {
			this.url = null;
		}
	}

it says url could be null in some cases.

however, inside the AbstractMailReceiver.receive(), we will call below:

	private Folder obtainFolderInstance() throws MessagingException {
		return this.store.getFolder(this.url);
	}

here, if this.url is null, getFolder will throw an NPE.

So, I think it's safe to remove this constructors.

[anthologia]

I agree that it could cause NPE.

However, instead of removing the ctors (which would affect ImapMailReceiver...), what about improving the obtainFolderInstance() method to handle the null case gracefully?

private Folder obtainFolderInstance() throws MessagingException {
    if (this.url == null) {
        return this.store.getDefaultFolder();
    }
    return this.store.getFolder(this.url);
}

[mjd507]

yes, that would fix, let's discuss.
originally I was thinking to fix all the implementations like 'Pop3MailReceiver' and 'ImapMailReceiver', so raised this issue,
Give everything(host, port, user, pwd, folder) default may not happen frequently in actual end-users, let's discuss.

[artembilan]

I also think that this.store.getDefaultFolder(); is the way to go if url is not provided.
Treating this a bug an has to be back-ported to all the supported versions.
I still would keep all those ctors for compatibility.

[mjd507]

okay, because I think its no harm if we force user to provide url, since currently if they don't provide, they can not use at all.
but ok.. let me keep that.