Possible bug in interaction between FeedbackMessages and FeedbackMessagesModel

[Per Lundholm <pe...@crisp.se>]

Hi!

I've bumped into a problem with feedback messages not being rendered and
given the warning about it in the log.

While tracing down how this really works, it is complicated I think, I
bumped into some code that looks a bit nasty to me. It may have effect on my
problem.

If the getObjcet method is called on FeedbackMessageModel is called, it
checks whether it has set its internal "messages" variable, else it asks the
current Session for messages.

    public final List<FeedbackMessage> getObject()
    {
        if (messages == null)
        {
            // Get filtered messages from page where component lives
            messages = Session.get().getFeedbackMessages().messages(filter);

            // Sort the list before returning it
            if (sortingComparator != null)
            {
                Collections.sort(messages, sortingComparator);
            }

            // Let subclass do any extra processing it wants to on the
messages.
            // It may want to do something special, such as removing a given
            // message under some special condition or perhaps eliminate
            // duplicate messages. It could even add a message under certain
            // conditions.
            messages = processMessages(messages);
        }
        return messages;
    }

That means that it actually *cache* the address to the list of message. Ok,
why is that a good idea?

However, if we look how the Session returns the list of messages we see that
it will return what FeedbackMessages returns. And here comes the
interaction, FeedbackMessages returns the Collections.emptyList() if there
are no messages. In effect, FeedbackMessageModel is *caching* the address of
the Collections.emptyList() result.

If somebody later adds a message, FeedbackMesssageModel will be unaware of
that since it will keep looking at Collections.emtyList() that it has cached
and not take the trouble to ask FeedbackMessages again.

    public final List<FeedbackMessage> messages(final IFeedbackMessageFilter
filter)
    {
        if (messages.size() == 0)
        {
            return Collections.emptyList();
        }

        final List<FeedbackMessage> list = new ArrayList<FeedbackMessage>();
        for (final Iterator<FeedbackMessage> iterator = messages.iterator();
iterator.hasNext();)
        {
            final FeedbackMessage message = iterator.next();
            if (filter == null || filter.accept(message))
            {
                list.add(message);
            }
        }
        return list;
    }

Now, I think it is complicated with how these feedback messages work, but
this does not look good, IMHO.

I am talking about 1.4rc6 and earlier versions as well.

Regards,
  Per