From 99fc21c915f17592eb82eefc164feb98376789c9 Mon Sep 17 00:00:00 2001 From: Danny van Heumen Date: Thu, 24 Jul 2014 23:27:40 +0200 Subject: [PATCH] More code clean-up and added tests for new role management approach. --- .../protocol/irc/ChatRoomMemberIrcImpl.java | 43 ++++++---- .../irc/ChatRoomMemberIrcImplTest.java | 80 +++++++++++++++---- .../irc/FormattedTextBuilderTest.java | 5 ++ .../impl/protocol/irc/ModeParserTest.java | 8 +- .../impl/protocol/irc/ModeTest.java | 7 +- 5 files changed, 108 insertions(+), 35 deletions(-) diff --git a/src/net/java/sip/communicator/impl/protocol/irc/ChatRoomMemberIrcImpl.java b/src/net/java/sip/communicator/impl/protocol/irc/ChatRoomMemberIrcImpl.java index f79de9630..ab872bd22 100644 --- a/src/net/java/sip/communicator/impl/protocol/irc/ChatRoomMemberIrcImpl.java +++ b/src/net/java/sip/communicator/impl/protocol/irc/ChatRoomMemberIrcImpl.java @@ -41,7 +41,7 @@ public class ChatRoomMemberIrcImpl * The provider that created us. */ private final ProtocolProviderService parentProvider; - + /** * Set of active roles. */ @@ -61,23 +61,31 @@ public class ChatRoomMemberIrcImpl * @param chatRoomMemberRole the role that this member has in the * corresponding chat room */ - public ChatRoomMemberIrcImpl(ProtocolProviderService parentProvider, - ChatRoom chatRoom, String contactID, - ChatRoomMemberRole chatRoomMemberRole) + public ChatRoomMemberIrcImpl(final ProtocolProviderService parentProvider, + final ChatRoom chatRoom, final String contactID, + final ChatRoomMemberRole chatRoomMemberRole) { if (parentProvider == null) + { throw new IllegalArgumentException( "parent protocol provider cannot be null"); + } this.parentProvider = parentProvider; if (chatRoom == null) + { throw new IllegalArgumentException( "chat room instance cannot be null"); + } this.chatRoom = chatRoom; if (contactID == null) + { throw new IllegalArgumentException("contact ID cannot be null"); + } this.contactID = contactID; if (chatRoomMemberRole == null) + { throw new IllegalArgumentException("member role cannot be null"); + } this.roles.add(chatRoomMemberRole); } @@ -131,13 +139,15 @@ public String getName() /** * Set a new name for this ChatRoomMember. - * - * @param newName + * + * @param newName new name to set for chat room member */ - public void setName(String newName) + public void setName(final String newName) { if (newName == null) + { throw new IllegalArgumentException("newName cannot be null"); + } this.contactID = newName; } @@ -157,31 +167,30 @@ public ChatRoomMemberRole getRole() * * @param chatRoomMemberRole the role to be set */ - public void setRole(ChatRoomMemberRole chatRoomMemberRole) + public void setRole(final ChatRoomMemberRole chatRoomMemberRole) { // Ignore explicit set role operations, since we only allow // modifications from the IRC server. - LOGGER.debug("Ignoring request to set role to " - + chatRoomMemberRole.getRoleName()); + LOGGER.debug("Ignoring request to set member role."); return; } /** * Add a role. - * + * * @param role the new role */ - void addRole(ChatRoomMemberRole role) + void addRole(final ChatRoomMemberRole role) { this.roles.add(role); } /** * Remove a role. - * + * * @param role the revoked role */ - void removeRole(ChatRoomMemberRole role) + void removeRole(final ChatRoomMemberRole role) { this.roles.remove(role); } @@ -226,15 +235,15 @@ public int hashCode() /** * equality by provider protocol instance and contact ID. - * + * * Enables the possibility to check if chat room member is same member in * different chat rooms. Values are only reliable for the same connection, * so also check protocol provider instance. - * + * * {@inheritDoc} */ @Override - public boolean equals(Object obj) + public boolean equals(final Object obj) { if (this == obj) return true; diff --git a/test/net/java/sip/communicator/impl/protocol/irc/ChatRoomMemberIrcImplTest.java b/test/net/java/sip/communicator/impl/protocol/irc/ChatRoomMemberIrcImplTest.java index d853fe0d2..2761f9dc1 100644 --- a/test/net/java/sip/communicator/impl/protocol/irc/ChatRoomMemberIrcImplTest.java +++ b/test/net/java/sip/communicator/impl/protocol/irc/ChatRoomMemberIrcImplTest.java @@ -1,9 +1,14 @@ +/* + * Jitsi, the OpenSource Java VoIP and Instant Messaging client. + * + * Distributable under LGPL license. See terms of license at gnu.org. + */ package net.java.sip.communicator.impl.protocol.irc; -import org.easymock.*; - -import net.java.sip.communicator.service.protocol.*; import junit.framework.*; +import net.java.sip.communicator.service.protocol.*; + +import org.easymock.*; public class ChatRoomMemberIrcImplTest extends TestCase @@ -139,19 +144,10 @@ public void testRoleNull() ChatRoomMemberIrcImpl member = new ChatRoomMemberIrcImpl(provider, chatroom, "user", ChatRoomMemberRole.SILENT_MEMBER); - Assert.assertSame(ChatRoomMemberRole.SILENT_MEMBER, member.getRole()); - try - { - member.setRole(null); - Assert.fail("expected IAE because of null role"); - } - catch (IllegalArgumentException e) - { - // this is good - } + member.setRole(null); } - public void testRoleChange() + public void testRoleUnchange() { ChatRoom chatroom = EasyMock.createMock(ChatRoom.class); ProtocolProviderService provider = @@ -160,9 +156,63 @@ public void testRoleChange() "user", ChatRoomMemberRole.SILENT_MEMBER); Assert.assertSame(ChatRoomMemberRole.SILENT_MEMBER, member.getRole()); member.setRole(ChatRoomMemberRole.ADMINISTRATOR); - Assert.assertSame(ChatRoomMemberRole.ADMINISTRATOR, member.getRole()); + Assert.assertSame(ChatRoomMemberRole.SILENT_MEMBER, member.getRole()); } + public void testAddSignificantRole() + { + ChatRoom chatroom = EasyMock.createMock(ChatRoom.class); + ProtocolProviderService provider = + EasyMock.createMock(ProtocolProviderService.class); + ChatRoomMemberIrcImpl member = + new ChatRoomMemberIrcImpl(provider, chatroom, "user", + ChatRoomMemberRole.SILENT_MEMBER); + Assert.assertSame(ChatRoomMemberRole.SILENT_MEMBER, member.getRole()); + member.addRole(ChatRoomMemberRole.ADMINISTRATOR); + Assert.assertSame(ChatRoomMemberRole.ADMINISTRATOR, member.getRole()); + } + + public void testRemoveSignificantRole() + { + ChatRoom chatroom = EasyMock.createMock(ChatRoom.class); + ProtocolProviderService provider = + EasyMock.createMock(ProtocolProviderService.class); + ChatRoomMemberIrcImpl member = + new ChatRoomMemberIrcImpl(provider, chatroom, "user", + ChatRoomMemberRole.SILENT_MEMBER); + member.addRole(ChatRoomMemberRole.ADMINISTRATOR); + Assert.assertSame(ChatRoomMemberRole.ADMINISTRATOR, member.getRole()); + member.removeRole(ChatRoomMemberRole.ADMINISTRATOR); + Assert.assertSame(ChatRoomMemberRole.SILENT_MEMBER, member.getRole()); + } + + public void testAddInsignificantRole() + { + ChatRoom chatroom = EasyMock.createMock(ChatRoom.class); + ProtocolProviderService provider = + EasyMock.createMock(ProtocolProviderService.class); + ChatRoomMemberIrcImpl member = + new ChatRoomMemberIrcImpl(provider, chatroom, "user", + ChatRoomMemberRole.ADMINISTRATOR); + Assert.assertSame(ChatRoomMemberRole.ADMINISTRATOR, member.getRole()); + member.addRole(ChatRoomMemberRole.MEMBER); + Assert.assertSame(ChatRoomMemberRole.ADMINISTRATOR, member.getRole()); + } + + public void testRemoveInsignificantRole() + { + ChatRoom chatroom = EasyMock.createMock(ChatRoom.class); + ProtocolProviderService provider = + EasyMock.createMock(ProtocolProviderService.class); + ChatRoomMemberIrcImpl member = + new ChatRoomMemberIrcImpl(provider, chatroom, "user", + ChatRoomMemberRole.ADMINISTRATOR); + member.addRole(ChatRoomMemberRole.MEMBER); + Assert.assertSame(ChatRoomMemberRole.ADMINISTRATOR, member.getRole()); + member.removeRole(ChatRoomMemberRole.MEMBER); + Assert.assertSame(ChatRoomMemberRole.ADMINISTRATOR, member.getRole()); + } + public void testGetContact() { ChatRoom chatroom = EasyMock.createMock(ChatRoom.class); diff --git a/test/net/java/sip/communicator/impl/protocol/irc/FormattedTextBuilderTest.java b/test/net/java/sip/communicator/impl/protocol/irc/FormattedTextBuilderTest.java index a3d317334..d2f6b92a5 100644 --- a/test/net/java/sip/communicator/impl/protocol/irc/FormattedTextBuilderTest.java +++ b/test/net/java/sip/communicator/impl/protocol/irc/FormattedTextBuilderTest.java @@ -1,3 +1,8 @@ +/* + * Jitsi, the OpenSource Java VoIP and Instant Messaging client. + * + * Distributable under LGPL license. See terms of license at gnu.org. + */ package net.java.sip.communicator.impl.protocol.irc; import junit.framework.*; diff --git a/test/net/java/sip/communicator/impl/protocol/irc/ModeParserTest.java b/test/net/java/sip/communicator/impl/protocol/irc/ModeParserTest.java index eddf06645..e584fb3e0 100644 --- a/test/net/java/sip/communicator/impl/protocol/irc/ModeParserTest.java +++ b/test/net/java/sip/communicator/impl/protocol/irc/ModeParserTest.java @@ -1,10 +1,14 @@ +/* + * Jitsi, the OpenSource Java VoIP and Instant Messaging client. + * + * Distributable under LGPL license. See terms of license at gnu.org. + */ package net.java.sip.communicator.impl.protocol.irc; import java.util.*; -import net.java.sip.communicator.impl.protocol.irc.ModeParser.ModeEntry; - import junit.framework.*; +import net.java.sip.communicator.impl.protocol.irc.ModeParser.ModeEntry; public class ModeParserTest extends TestCase diff --git a/test/net/java/sip/communicator/impl/protocol/irc/ModeTest.java b/test/net/java/sip/communicator/impl/protocol/irc/ModeTest.java index 47c23ec43..843bbe432 100644 --- a/test/net/java/sip/communicator/impl/protocol/irc/ModeTest.java +++ b/test/net/java/sip/communicator/impl/protocol/irc/ModeTest.java @@ -1,7 +1,12 @@ +/* + * Jitsi, the OpenSource Java VoIP and Instant Messaging client. + * + * Distributable under LGPL license. See terms of license at gnu.org. + */ package net.java.sip.communicator.impl.protocol.irc; -import net.java.sip.communicator.service.protocol.*; import junit.framework.*; +import net.java.sip.communicator.service.protocol.*; public class ModeTest extends TestCase