Wiser exposes internal representation

[GoogleCodeExporter]

The method Wiser.getMessages() returns a reference to the internal array of 
messages. This can be demonstrated with the following unit test added to 
MessageContentTest

    public void testDoesNotExposeInternalRep() throws Exception
    {
        MimeMessage message = new MimeMessage(this.session);
        message.addRecipient(Message.RecipientType.TO, new InternetAddress("anyone@anywhere.com"));
        message.setFrom(new InternetAddress("someone@somewhereelse.com"));
        Transport.send(message);
        List<WiserMessage> messages = this.wiser.getMessages();
        assertFalse(messages.isEmpty());
        messages.clear();
        assertFalse(this.wiser.getMessages().isEmpty());
    }

This problem is also related to issue 37, in that the client currently needs to 
synchronize on the reference to lower the risk of 
ConcurrentModificationException when iterating the list. If issue 37 is fixed 
by using CopyOnWriteArrayList or similar, then getMessages could be fixed as 
follows.

public List<WiserMessage> getMessages()
    {
        return new ArrayList<WiserMessage>(this.messages);
    }

(However, if the synchronized wrapper class is used as present then the copy 
code needs to in a block synchronised on this.messages).

Original issue reported on code.google.com by s_etheri...@hotmail.com on 19 Dec 2010 at 10:35

[GoogleCodeExporter]

There is no reason for Wiser not to return the internal representation; it 
allows your unit tests to manipulate it as you wish.  Need a copy?  Make one.

Original comment by lhori...@gmail.com on 24 Dec 2010 at 9:23

• Changed state: WontFix
• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

OK. But in order to safely make a copy we would need to subclass Wiser and 
override the messages property. The code we have that currently blows up with 
ConcurrentModificationException is nothing out of the ordinary:

List<MimeMessage> retVal = new ArrayList<MimeMessage>();
List<WiserMessage> wiserMessages = wiser.getMessages();
if (wiserMessages != null) {
    for (WiserMessage wiserMessage : wiserMessages) {
        retVal.add(wiserMessage.getMimeMessage());
    }
}
return retVal;

Original comment by s_etheri...@hotmail.com on 24 Dec 2010 at 11:18

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

Wiser really wasn't intended to be hit from multiple threads - it's really just 
a very simple testing tool for junit-style tests.  If you want something more 
robust, the code for Wiser is almost trivial - and now you can extend it and 
replace the structure used to store messages.

It's tempting to simply replace the ArrayList with a CopyOnWriteList but I am 
hesitant to change the performance profile of a tool that some people very 
possibly use to test thousands of deliveries.

Original comment by lhori...@gmail.com on 24 Dec 2010 at 6:18

• Added labels: ****
• Removed labels: ****