MYFACES-4612: FacesMessages don't store in different collections

[melloware]

Because its a LinkedHashMap and the Lists they will remain ordered but not in separate storage containers.

[melloware]

I have to make one more fix.

[melloware]

So Mojarra wrote a custom iterator so it could properly handle this scenario for iterating and removal.

// ---------------------------------------------------------- Inner Classes

    private static final class ComponentMessagesIterator implements Iterator<FacesMessage> {

        private Map<String, List<FacesMessage>> messages;
        private int outerIndex = -1;
        private int messagesSize;
        private Iterator<FacesMessage> inner;
        private Iterator<String> keys;

        // ------------------------------------------------------- Constructors

        ComponentMessagesIterator(Map<String, List<FacesMessage>> messages) {

            this.messages = messages;
            messagesSize = messages.size();
            keys = messages.keySet().iterator();

        }

        // ---------------------------------------------- Methods from Iterator

        @Override
        public boolean hasNext() {

            if (outerIndex == -1) {
                // pop our first List, if any;
                outerIndex++;
                inner = messages.get(keys.next()).iterator();

            }
            while (!inner.hasNext()) {
                outerIndex++;
                if (outerIndex < messagesSize) {
                    inner = messages.get(keys.next()).iterator();
                } else {
                    return false;
                }
            }
            return inner.hasNext();

        }

        @Override
        public FacesMessage next() {

            if (outerIndex >= messagesSize) {
                throw new NoSuchElementException();
            }
            if (inner != null && inner.hasNext()) {
                return inner.next();
            } else {
                // call this.hasNext() to properly initialize/position 'inner'
                if (!hasNext()) {
                    throw new NoSuchElementException();
                } else {
                    return inner.next();
                }
            }

        }

        @Override
        public void remove() {

            if (outerIndex == -1) {
                throw new IllegalStateException();
            }
            inner.remove();

        }

    } // END ComponentMessagesIterator

[tandraschko]

Is remove even supported by Specs?
AFAIR we return a unmodifiable list

[melloware]

On the Iterator versions of getMessages I believe it is but on the List implementions of the return you are right its an unmodifiable List. How can we ask? Should I open a question on the Faces API GitHub?

[tandraschko]

Try to check the javadocs and specs

[melloware]

As far as I can see section 6.1.6 Message Queue deals with messages but it just mentions returning Iterators but not whether messages can be removed from the iterator at any phase?

6.1.6. Message Queue

public void addMessage(String clientId, FacesMessage message);

During the Apply Request Values, Process Validations, Update Model Values, and Invoke Application
phases of the request processing lifecycle, messages can be queued to either the component tree as
a whole (if clientId is null), or related to a specific component based on its client identifier.

public Interator<String> getClientIdsWithMessages();
public Severity getMaximumSeverity();
public Iterator<FacesMessage> getMessages(String clientId);
public Iterator<FacesMessage> getMessages();

The getClientIdsWithMessages() method must return an Iterator over the client identifiers for which
at least one Message has been queued. This method must be implemented so the clientIds are
returned in the order of calls to addMessage(). The getMaximumSeverity() method returns the
highest severity level on any Message that has been queued, regardless of whether or not the
message is associated with a specific client identifier or not. The getMessages(String) method
returns an Iterator over queued Messages, either those associated with the specified client
identifier, or those associated with no client identifier if the parameter is null. The getMessages()
method returns an Iterator over all queued Messages, whether or not they are associated with a
particular client identifier. Both of the getMessage() variants must be implemented such that the
messages are returned in the order in which they were added via calls to addMessage().

[tandraschko]

if this PR fixes the .remove case -> +1
i would only apply it for trunk for now TBH

[melloware]

Collections.emptyList() returns an unmodifiable list by default

[melloware]

OK I fixed it now and updated the PR to handle the scenario of removal and using 1 collection to iterate.