Merge pull request #57 from cobratbq/master

Various minor fixes w.r.t. IRC protocol support and chat room listing.

src/net/java/sip/communicator/impl/muc/ServerChatRoomQuery.java

@@ -107,18 +107,26 @@ protected void run()
      * @param addQueryResult indicates whether we should add the chat room to
      * the query results or fire an event without adding it to the results.
      */
-    private void providerAdded(ChatRoomProviderWrapper provider,
-        boolean addQueryResult)
+    private void providerAdded(final ChatRoomProviderWrapper provider,
+        final boolean addQueryResult)
     {
-        List<String> chatRoomNames
-            = MUCActivator.getMUCService().getExistingChatRooms(provider);
-        if(chatRoomNames == null)
+        final ProtocolProviderService pps = provider.getProtocolProvider();
+        List<String> chatRoomNames =
+            MUCActivator.getMUCService().getExistingChatRooms(provider);
+        if (chatRoomNames == null)
+        {
             return;
-        for(String chatRoomName : chatRoomNames)
+        }
+
+        // Already create all the BaseChatRoomSourceContact instances since all
+        // the data is already available.
+        final Set<BaseChatRoomSourceContact> chatRooms =
+            new HashSet<BaseChatRoomSourceContact>(chatRoomNames.size());
+        for (final String name : chatRoomNames)
         {
-            addChatRoom( provider.getProtocolProvider(), chatRoomName,
-                chatRoomName, addQueryResult);
+            chatRooms.add(new BaseChatRoomSourceContact(name, name, this, pps));
         }
+        addChatRooms(pps, chatRooms, addQueryResult);
     }
 
 
@@ -159,6 +167,55 @@ private void addChatRoom(ProtocolProviderService pps,
         }
     }
 
+    /**
+     * Adds found results to the query results.
+     *
+     * @param pps the protocol provider associated with the found chat room.
+     * @param chatRooms The set of chat rooms based on
+     *            BaseChatRoomSourceContact. This is the full set and it will be
+     *            filtered according to demands of the queryString.
+     * @param addQueryResult indicates whether we should add the chat room to
+     *            the query results or fire an event without adding it to the
+     *            results.
+     */
+    private void addChatRooms(final ProtocolProviderService pps,
+        final Set<BaseChatRoomSourceContact> chatRooms,
+        final boolean addQueryResult)
+    {
+        BaseChatRoomSourceContact room;
+        Iterator<BaseChatRoomSourceContact> iterator = chatRooms.iterator();
+        while (iterator.hasNext())
+        {
+            room = iterator.next();
+
+            // Notice the NOT operator at the start ...
+            if (!((queryString == null || (room.getChatRoomName().contains(
+                queryString) || room.getChatRoomID().contains(queryString)))
+                    && isMatching(room.getChatRoomID(), pps)))
+            {
+                iterator.remove();
+            }
+        }
+
+        synchronized (contactResults)
+        {
+            contactResults.addAll(chatRooms);
+        }
+
+        if (addQueryResult)
+        {
+            addQueryResults(chatRooms);
+        }
+        else
+        {
+            // TODO Need something to fire one event for multiple contacts.
+            for (SourceContact contact : chatRooms)
+            {
+                fireContactReceived(contact, false);
+            }
+        }
+    }
+
     @Override
     public void chatRoomProviderWrapperAdded(ChatRoomProviderWrapper provider)
     {
@@ -273,4 +330,4 @@ public int indexOf(BaseChatRoomSourceContact contact)
         }
         return -1;
     }
-}
+}

src/net/java/sip/communicator/impl/protocol/irc/ChatRoomIrcImpl.java

@@ -152,7 +152,9 @@ public ChatRoomIrcImpl(final String chatRoomName,
             throw new IllegalArgumentException(
                 "chatRoomName cannot be null or empty string");
         }
-        this.chatRoomName = verifyName(chatRoomName);
+        this.chatRoomName =
+            verifyName(this.parentProvider.getIrcStack().getChannelTypes(),
+                chatRoomName);
         this.isSystem = isSystem;
     }
 
@@ -164,24 +166,27 @@ public ChatRoomIrcImpl(final String chatRoomName,
      * @throws IllegalArgumentException if name/identifier contains invalid
      *             characters
      */
-    private String verifyName(final String name)
+    private static String verifyName(final Set<Character> channelTypes,
+        final String name)
     {
         final char prefix = name.charAt(0);
-        if (!this.parentProvider.getIrcStack().getChannelTypes()
-            .contains(prefix))
+        if (channelTypes.contains(prefix))
         {
-            throw new IllegalArgumentException("invalid channel prefix: "
-                + prefix);
-        }
-        for (char c : IrcStack.SPECIAL_CHARACTERS)
-        {
-            if (name.contains("" + c))
+            for (char c : IrcStack.SPECIAL_CHARACTERS)
             {
-                throw new IllegalArgumentException(
-                    "chat room identifier contains illegal character: " + c);
+                if (name.contains("" + c))
+                {
+                    throw new IllegalArgumentException(
+                       "chat room identifier contains illegal character: " + c);
+                }
             }
+            return name;
+        }
+        else
+        {
+            LOGGER.trace("Automatically added # channel prefix.");
+            return verifyName(channelTypes, "#" + name);
         }
-        return name;
     }
 
     /**

src/net/java/sip/communicator/impl/protocol/irc/IrcStack.java

@@ -514,6 +514,9 @@ public List<String> getServerChatRoomList()
                 }
                 list = listSignal.getValue();
                 this.channellist.set(list);
+                // TODO Set a timer for past channel expiration delay and clean
+                // up outdated cached channel list. Because it won't be cleaned
+                // up if a user won't list server chat rooms again.
                 LOGGER.trace("Finished retrieving server chat room list.");
             }
             else

src/net/java/sip/communicator/impl/protocol/irc/OperationSetMultiUserChatIrcImpl.java

@@ -170,7 +170,19 @@ public ChatRoom createChatRoom(
         throws OperationFailedException,
                OperationNotSupportedException
     {
-        return findOrCreateRoom(roomName);
+        try
+        {
+            return findOrCreateRoom(roomName);
+        }
+        catch (IllegalArgumentException e)
+        {
+            String message =
+                IrcActivator.getResources().getI18NString(
+                    "service.gui.CREATE_CHAT_ROOM_ERROR", new String[]
+                    { roomName });
+            throw new OperationFailedException(message,
+                OperationFailedException.ILLEGAL_ARGUMENT, e);
+        }
     }
 
     /**

src/net/java/sip/communicator/service/contactsource/AsyncContactQuery.java

@@ -116,7 +116,7 @@ protected boolean addQueryResult(SourceContact sourceContact,
 
         return changed;
     }
-    
+
     /**
      * Adds a specific <tt>SourceContact</tt> to the list of
      * <tt>SourceContact</tt>s to be returned by this <tt>ContactQuery</tt> in
@@ -141,6 +141,39 @@ protected boolean addQueryResult(SourceContact sourceContact)
         return changed;
     }
 
+    /**
+     * Adds a set of <tt>SourceContact</tt> instances to the list of
+     * <tt>SourceContact</tt>s to be returned by this <tt>ContactQuery</tt> in
+     * response to {@link #getQueryResults()}.
+     *
+     * @param sourceContacts the set of <tt>SourceContact</tt> to be added to
+     * the <tt>queryResults</tt> of this <tt>ContactQuery</tt>
+     * @return <tt>true</tt> if the <tt>queryResults</tt> of this
+     * <tt>ContactQuery</tt> has changed in response to the call
+     */
+    protected boolean addQueryResults(
+        final Set<? extends SourceContact> sourceContacts)
+    {
+        final boolean changed;
+
+        synchronized (queryResults)
+        {
+            changed = queryResults.addAll(sourceContacts);
+        }
+
+        if (changed)
+        {
+            // TODO Need something to fire one event for multiple contacts.
+            for (SourceContact contact : sourceContacts)
+            {
+                fireContactReceived(contact, false);
+            }
+        }
+
+        return changed;
+    }
+
+
     /**
      * Gets the {@link #query} of this <tt>AsyncContactQuery</tt> as a
      * <tt>String</tt> which represents a phone number (if possible).

test/net/java/sip/communicator/impl/protocol/irc/ChatRoomIrcImplTest.java

@@ -25,7 +25,7 @@ public void setUp() throws Exception
         this.stackMock = EasyMock.createMock(IrcStack.class);
         EasyMock.expect(this.providerMock.getIrcStack()).andReturn(stackMock);
         EasyMock.expect(this.stackMock.getChannelTypes()).andReturn(
-            Collections.unmodifiableSet(Sets.newHashSet('#', '$')));
+            Collections.unmodifiableSet(Sets.newHashSet('#', '&')));
     }
 
     //@Test
@@ -77,18 +77,12 @@ public void testEmptyName()
         }
     }
 
-    //@Test(expected = IllegalArgumentException.class)
-    public void testIllegalNameBadPrefix()
+    //@Test
+    public void testAutoPrefixBadChannelName()
     {
         EasyMock.replay(this.providerMock, this.stackMock);
-        try
-        {
-            new ChatRoomIrcImpl("!test", this.providerMock);
-            fail("Should have failed with IAE.");
-        }
-        catch (IllegalArgumentException e)
-        {
-        }
+        ChatRoomIrcImpl room = new ChatRoomIrcImpl("!test", this.providerMock);
+        Assert.assertEquals("#!test", room.getIdentifier());
     }
 
     //@Test(expected = IllegalArgumentException.class)
@@ -431,4 +425,19 @@ public void testSetChatRoomSubjectFailedByOtherRuntimeException()
         {
         }
     }
+
+    /**
+     * Test creating chat room with alternative prefix. Special check to ensure
+     * that we don't forget about less often used prefixes.
+     */
+    // @Test
+    public void testChatRoomWithAlternativePrefix()
+    {
+        EasyMock.replay(this.providerMock, this.stackMock);
+        ChatRoomIrcImpl alternative =
+            new ChatRoomIrcImpl("&MyAlternative-channel-prefix",
+                this.providerMock);
+        Assert.assertEquals("&MyAlternative-channel-prefix",
+            alternative.getIdentifier());
+    }
 }