From 16f76289d203069036a2fb2da1b559f0e888728b Mon Sep 17 00:00:00 2001 From: Alex Clayton Date: Wed, 3 Feb 2016 17:01:38 +0000 Subject: Add CombinedAvatarProviderTest Adds the CombinedAvatarProviderTest class. When I was adding the test I noticed that testProviderUpdateWithAvatarDisappearingTriggersChange kept failing due to a bug in the CombinedAvatarProvider class and testRemoveProviderDisconnectsUpdates could not be implemented as the class was incomplete. I had to make some changes to CombinedAvatarProvider to fix these issues. Test-information: Ran unit tests they all pass. Change-Id: I0bfb68dd2b15df0f220f36c136aceadaf6545893 diff --git a/PortingProgress.txt b/PortingProgress.txt index 77deba3..e486023 100644 --- a/PortingProgress.txt +++ b/PortingProgress.txt @@ -10,9 +10,7 @@ All files ported to 6ca201d0b48f4273e24dd7bff17c4a46eeaddf39. ----- Avatars: -All files ported to 6ca201d0b48f4273e24dd7bff17c4a46eeaddf39 except for: - -CombinedAvatarProviderTest -- Not Yet Ported! +All files ported to 6ca201d0b48f4273e24dd7bff17c4a46eeaddf39 ----- Base: diff --git a/src/com/isode/stroke/avatars/CombinedAvatarProvider.java b/src/com/isode/stroke/avatars/CombinedAvatarProvider.java index 5520736..b7639b3 100755 --- a/src/com/isode/stroke/avatars/CombinedAvatarProvider.java +++ b/src/com/isode/stroke/avatars/CombinedAvatarProvider.java @@ -21,60 +21,87 @@ import java.util.*; public class CombinedAvatarProvider extends AvatarProvider { - private final Vector providers = new Vector(); - private Map avatars = new HashMap(); - private final Map onAvatarChangedConnections_ = new HashMap(); - private Logger logger_ = Logger.getLogger(this.getClass().getName()); + private final Vector providers = new Vector(); + private Map avatars = new HashMap(); + private final Map onAvatarChangedConnections_ = new HashMap(); + private Logger logger_ = Logger.getLogger(this.getClass().getName()); - @Override - public String getAvatarHash(JID jid) { - return getCombinedAvatarAndCache(jid); - } + @Override + public String getAvatarHash(JID jid) { + return getCombinedAvatarAndCache(jid); + } - private final Slot1 onAvatarChangedSlot = new Slot1() { - @Override public void call(JID p1) {handleAvatarChanged(p1);} - }; - - public void addProvider(AvatarProvider provider) { - if (!onAvatarChangedConnections_.containsKey(provider)) { - onAvatarChangedConnections_.put(provider, provider.onAvatarChanged.connect(onAvatarChangedSlot)); - } - providers.add(provider); - } + private final Slot1 onAvatarChangedSlot = new Slot1() { + @Override public void call(JID p1) {handleAvatarChanged(p1);} + }; - public void delete() { - for (SignalConnection connection : onAvatarChangedConnections_.values()) { - connection.disconnect(); - } - for (AvatarProvider provider : providers) { - provider.delete(); - } - } + public void addProvider(AvatarProvider provider) { + if (!onAvatarChangedConnections_.containsKey(provider)) { + onAvatarChangedConnections_.put(provider, provider.onAvatarChanged.connect(onAvatarChangedSlot)); + } + providers.add(provider); + } - private void handleAvatarChanged(JID jid) { - String oldHash = new String(); - if(avatars.containsKey(jid)) { - oldHash = avatars.get(jid); - } - String newHash = getCombinedAvatarAndCache(jid); - if (newHash != null && !newHash.equals(oldHash)) { - logger_.fine("Avatar changed: " + jid + ": " + oldHash + " -> " + ((newHash != null) ? newHash : "NULL") + "\n"); - onAvatarChanged.emit(jid); - } - } + public void removeProvider(AvatarProvider provider) { + while (providers.remove(provider)) { + // Loop will run until no copies of provider in providers + } + SignalConnection avatarChangedConnection = onAvatarChangedConnections_.remove(provider); + if (avatarChangedConnection != null) { + avatarChangedConnection.disconnect(); + } + } - private String getCombinedAvatarAndCache(JID jid) { - logger_.fine("JID: " + jid + "\n"); - String hash = null; - for (int i = 0; i < providers.size() && (hash==null); ++i) { - hash = providers.get(i).getAvatarHash(jid); - logger_.fine("Provider " + providers.get(i) + ": " + ((hash != null) ? hash : "NULL") + "\n"); - } - if (hash != null) { - avatars.put(jid, hash); - } else { - avatars.put(jid, ""); - } - return hash; - } + public void delete() { + for (SignalConnection connection : onAvatarChangedConnections_.values()) { + connection.disconnect(); + } + for (AvatarProvider provider : providers) { + provider.delete(); + } + } + + private void handleAvatarChanged(JID jid) { + String oldHash = new String(); + if(avatars.containsKey(jid)) { + oldHash = avatars.get(jid); + } + String newHash = getCombinedAvatarAndCache(jid); + if (!areHashesEqual(oldHash, newHash)) { + logger_.fine("Avatar changed: " + jid + ": " + oldHash + " -> " + ((newHash != null) ? newHash : "NULL") + "\n"); + onAvatarChanged.emit(jid); + } + } + + /** + * Performs a null safe check if two hashes are equal + * @param hash1 A hash. Can be {@code null}. + * @param hash2 Another hash. Can be {@code null} + * @return {@code true} if the hashes are equal, {@code false} + * otherwise. + */ + private static boolean areHashesEqual(String hash1,String hash2) { + if (hash1 == hash2) { + return true; + } + else if (hash1 == null) { + return false; + } + return hash1.equals(hash2); + } + + private String getCombinedAvatarAndCache(JID jid) { + logger_.fine("JID: " + jid + "\n"); + String hash = null; + for (int i = 0; i < providers.size() && (hash==null); ++i) { + hash = providers.get(i).getAvatarHash(jid); + logger_.fine("Provider " + providers.get(i) + ": " + ((hash != null) ? hash : "NULL") + "\n"); + } + if (hash != null) { + avatars.put(jid, hash); + } else { + avatars.put(jid, ""); + } + return hash; + } } \ No newline at end of file diff --git a/test/com/isode/stroke/avatars/CombinedAvatarProviderTest.java b/test/com/isode/stroke/avatars/CombinedAvatarProviderTest.java new file mode 100644 index 0000000..23fb349 --- /dev/null +++ b/test/com/isode/stroke/avatars/CombinedAvatarProviderTest.java @@ -0,0 +1,374 @@ +/* Copyright (c) 2016, Isode Limited, London, England. + * All rights reserved. + * + * Acquisition and use of this software and related materials for any + * purpose requires a written license agreement from Isode Limited, + * or a written license from an organisation licensed by Isode Limited + * to grant such a license. + * + */ +package com.isode.stroke.avatars; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.junit.Test; + +import com.isode.stroke.base.ByteArray; +import com.isode.stroke.client.DummyStanzaChannel; +import com.isode.stroke.crypto.CryptoProvider; +import com.isode.stroke.crypto.JavaCryptoProvider; +import com.isode.stroke.elements.IQ; +import com.isode.stroke.elements.VCard; +import com.isode.stroke.jid.JID; +import com.isode.stroke.muc.MUCRegistry; +import com.isode.stroke.queries.IQRouter; +import com.isode.stroke.signals.Slot1; +import com.isode.stroke.stringcodecs.Hexify; +import com.isode.stroke.vcards.VCardManager; +import com.isode.stroke.vcards.VCardMemoryStorage; + +/** + * Tests for {@link CombinedAvatarProvider} + */ +public class CombinedAvatarProviderTest { + + private final DummyAvatarProvider avatarProvider1 = new DummyAvatarProvider(); + private final DummyAvatarProvider avatarProvider2 = new DummyAvatarProvider(); + private final JID user1 = new JID("user1@bar.com/bla"); + private final JID user2 = new JID("user2@foo.com/baz"); + private final String avatarHash1 = "ABCDEFG"; + private final String avatarHash2 = "XYZU"; + private final String avatarHash3 = "IDGH"; + private final List changes = new ArrayList(); + + @Test + public void testGetAvatarWithNoAvatarProviderReturnsEmpty() { + CombinedAvatarProvider testling = createProvider(); + String hash = testling.getAvatarHash(user1); + assertNull(hash); + } + + @Test + public void testGetAvatarWithSingleAvatarProvider() { + CombinedAvatarProvider testling = createProvider(); + avatarProvider1.avatars.put(user1, avatarHash1); + testling.addProvider(avatarProvider1); + String hash = testling.getAvatarHash(user1); + assertEquals(avatarHash1, hash); + } + + @Test + public void testGetAvatarWithMultipleAvatarProviderReturnsFirstAvatar() { + CombinedAvatarProvider testling = createProvider(); + avatarProvider1.avatars.put(user1, avatarHash1); + avatarProvider2.avatars.put(user1, avatarHash2); + testling.addProvider(avatarProvider1); + testling.addProvider(avatarProvider2); + String hash = testling.getAvatarHash(user1); + assertEquals(avatarHash1,hash); + } + + @Test + public void testGetAvatarWithMultipleAvatarProviderAndFailingFirstProviderReturnsSecondAvatar() { + CombinedAvatarProvider testling = createProvider(); + avatarProvider2.avatars.put(user1, avatarHash2); + testling.addProvider(avatarProvider1); + testling.addProvider(avatarProvider2); + String hash = testling.getAvatarHash(user1); + assertEquals(avatarHash2,hash); + } + + @Test + public void testProviderUpdateTriggersChange() { + CombinedAvatarProvider testling = createProvider(); + testling.addProvider(avatarProvider1); + avatarProvider1.avatars.put(user1, avatarHash1); + avatarProvider1.onAvatarChanged.emit(user1); + assertEquals(1,changes.size()); + assertEquals(user1,changes.get(0)); + } + + @Test + public void testProviderUpdateWithoutChangeDoesNotTriggerChange() { + CombinedAvatarProvider testling = createProvider(); + testling.addProvider(avatarProvider1); + testling.addProvider(avatarProvider2); + avatarProvider1.avatars.put(user1, avatarHash1); + avatarProvider1.onAvatarChanged.emit(user1); + changes.clear(); + + avatarProvider2.avatars.put(user1, avatarHash2); + avatarProvider2.onAvatarChanged.emit(user1); + + assertEquals(0,changes.size()); + } + + @Test + public void testProviderSecondUpdateTriggersChange() { + CombinedAvatarProvider testling = createProvider(); + testling.addProvider(avatarProvider1); + avatarProvider1.avatars.put(user1, avatarHash1); + avatarProvider1.onAvatarChanged.emit(user1); + changes.clear(); + avatarProvider1.avatars.put(user1, avatarHash2); + avatarProvider1.onAvatarChanged.emit(user1); + + assertEquals(1,changes.size()); + assertEquals(user1,changes.get(0)); + } + + @Test + public void testProviderUpdateWithAvatarDisappearingTriggersChange() { + CombinedAvatarProvider testling = createProvider(); + testling.addProvider(avatarProvider1); + avatarProvider1.avatars.put(user1, avatarHash1); + avatarProvider1.onAvatarChanged.emit(user1); + changes.clear(); + avatarProvider1.avatars.clear(); + avatarProvider1.onAvatarChanged.emit(user1); + assertNull(testling.getAvatarHash(user1)); + assertEquals(1, changes.size()); + assertEquals(user1,changes.get(0)); + } + + @Test + public void testProviderUpdateAfterAvatarDisappearedTriggersChange() { + CombinedAvatarProvider testling = createProvider(); + testling.addProvider(avatarProvider1); + avatarProvider1.avatars.put(user1, avatarHash1); + avatarProvider1.onAvatarChanged.emit(user1); + avatarProvider1.avatars.clear(); + avatarProvider1.onAvatarChanged.emit(user1); + changes.clear(); + avatarProvider1.avatars.put(user1, avatarHash1); + avatarProvider1.onAvatarChanged.emit(user1); + assertEquals(1,changes.size()); + assertEquals(user1,changes.get(0)); + } + + @Test + public void testProviderUpdateAfterGetDoesNotTriggerChange() { + CombinedAvatarProvider testling = createProvider(); + testling.addProvider(avatarProvider1); + avatarProvider1.avatars.put(user1, avatarHash1); + testling.getAvatarHash(user1); + avatarProvider1.onAvatarChanged.emit(user1); + assertEquals(0,changes.size()); + } + + @Test + public void testRemoveProviderDisconnectsUpdates() { + CombinedAvatarProvider testling = createProvider(); + testling.addProvider(avatarProvider1); + testling.addProvider(avatarProvider2); + testling.removeProvider(avatarProvider1); + avatarProvider1.avatars.put(user1, avatarHash1); + avatarProvider2.avatars.put(user1, avatarHash2); + avatarProvider1.onAvatarChanged.emit(user1); + assertEquals(0,changes.size()); + } + + @Test + public void testProviderUpdateBareJIDAfterGetFullJID() { + CombinedAvatarProvider testling = createProvider(); + avatarProvider1.useBare = true; + testling.addProvider(avatarProvider1); + + avatarProvider1.avatars.put(user1.toBare(),avatarHash1); + testling.getAvatarHash(user1); + avatarProvider1.avatars.put(user1.toBare(),avatarHash2); + avatarProvider1.onAvatarChanged.emit(user1.toBare()); + + String hash = testling.getAvatarHash(user1); + assertEquals(avatarHash2,hash); + } + + @Test + public void testAddRemoveFallthrough() { + JID ownJID = new JID("user0@own.com/res"); + JID user1 = new JID("user1@bar.com/bla"); + + CryptoProvider crypto = new JavaCryptoProvider(); + DummyStanzaChannel stanzaChannel = new DummyStanzaChannel(); + stanzaChannel.setAvailable(true); + IQRouter iqRouter = new IQRouter(stanzaChannel); + MUCRegistry mucRegistry = new MUCRegistry(); + AvatarMemoryStorage avatarStorage = new AvatarMemoryStorage(); + VCardMemoryStorage vCardStorage = new VCardMemoryStorage(crypto); + VCardManager vcardManager = new VCardManager(ownJID, iqRouter, vCardStorage); + + VCardUpdateAvatarManager updateManager = + new VCardUpdateAvatarManager(vcardManager, stanzaChannel, avatarStorage, + crypto, mucRegistry); + updateManager.onAvatarChanged.connect(new Slot1() { + + @Override + public void call(JID jid) { + handleAvatarChanged(jid); + } + + }); + + VCardAvatarManager manager = + new VCardAvatarManager(vcardManager, avatarStorage, crypto, mucRegistry); + manager.onAvatarChanged.connect(new Slot1() { + + @Override + public void call(JID jid) { + handleAvatarChanged(jid); + } + + }); + + OfflineAvatarManager offlineManager = new OfflineAvatarManager(avatarStorage); + offlineManager.onAvatarChanged.connect(new Slot1() { + + @Override + public void call(JID jid) { + handleAvatarChanged(jid); + } + + }); + + CombinedAvatarProvider testling = createProvider(); + avatarProvider1.useBare = true; + testling.addProvider(updateManager); + testling.addProvider(manager); + testling.addProvider(offlineManager); + + // Place an avatar in the cache, check that it reads back OK + + assertEquals(0,changes.size()); + + ByteArray avatar1 = new ByteArray("abcdefg"); + String avatar1Hash = Hexify.hexify(crypto.getSHA1Hash(avatar1)); + VCard vcard1 = new VCard(); + vcard1.setPhoto(avatar1); + + vCardStorage.setVCard(user1.toBare(), vcard1); + String testHash = testling.getAvatarHash(user1.toBare()); + assertEquals(avatar1Hash,testHash); + + VCard storedVCard = vCardStorage.getVCard(user1.toBare()); + assertNotNull(storedVCard); + testHash = Hexify.hexify(crypto.getSHA1Hash(storedVCard.getPhoto())); + assertEquals(avatar1Hash, testHash); + + // Change the Avatar by sending a VCard IQ + + vcardManager.requestVCard(user1.toBare()); + assertEquals(1,stanzaChannel.sentStanzas.size()); + IQ request = (IQ) stanzaChannel.sentStanzas.lastElement(); + VCard payload = request.getPayload(new VCard()); + assertNotNull(payload); + stanzaChannel.sentStanzas.clear(); + + ByteArray avatar2 = new ByteArray("1234567"); + String avatar2Hash = Hexify.hexify(crypto.getSHA1Hash(avatar2)); + VCard vcard2 = new VCard(); + vcard2.setPhoto(avatar2); + + IQ reply = new IQ(); + reply.setTo(request.getFrom()); + reply.setFrom(request.getTo()); + reply.setID(request.getID()); + reply.addPayload(vcard2); + reply.setType(IQ.Type.Result); + + stanzaChannel.onIQReceived.emit(reply); + + // Check that we changed the avatar succesfully and that we were notified of the changes + + testHash = testling.getAvatarHash(user1.toBare()); + assertEquals(avatar2Hash,testHash); + assertEquals(3,changes.size()); + assertEquals(user1.toBare(),changes.get(0)); + assertEquals(user1.toBare(),changes.get(1)); + assertEquals(user1.toBare(),changes.get(2)); + changes.clear(); + storedVCard = vCardStorage.getVCard(user1.toBare()); + assertNotNull(storedVCard); + testHash = Hexify.hexify(crypto.getSHA1Hash(storedVCard.getPhoto())); + assertEquals(avatar2Hash,testHash); + + // Change the avatar to an empty avatar + + vcardManager.requestVCard(user1.toBare()); + assertEquals(1,stanzaChannel.sentStanzas.size()); + request = (IQ) stanzaChannel.sentStanzas.lastElement(); + payload = request.getPayload(new VCard()); + assertNotNull(payload); + stanzaChannel.sentStanzas.clear(); + + VCard vcard3 = new VCard(); + reply = new IQ(); + reply.setTo(request.getFrom()); + reply.setFrom(request.getTo()); + reply.setID(request.getID()); + reply.addPayload(vcard3); + reply.setType(IQ.Type.Result); + stanzaChannel.onIQReceived.emit(reply); + + // Check that we changed the avatar successfully + + testHash = testling.getAvatarHash(user1.toBare()); + assertNotNull(testHash); + assertEquals("",testHash); + assertEquals(3,changes.size()); + assertEquals(user1.toBare(),changes.get(0)); + assertEquals(user1.toBare(),changes.get(1)); + assertEquals(user1.toBare(),changes.get(2)); + changes.clear(); + storedVCard = vCardStorage.getVCard(user1.toBare()); + assertNotNull(storedVCard); + assertNull(storedVCard.getPhoto()); + } + + private CombinedAvatarProvider createProvider() { + CombinedAvatarProvider result = new CombinedAvatarProvider(); + result.onAvatarChanged.connect(new Slot1() { + + @Override + public void call(JID jid) { + handleAvatarChanged(jid); + } + + }); + return result; + } + + private void handleAvatarChanged(JID jid) { + changes.add(jid); + } + + private static class DummyAvatarProvider extends AvatarProvider { + + @Override + public String getAvatarHash(JID jid) { + JID actualJID = useBare ? jid.toBare() : jid; + if (avatars.containsKey(actualJID)) { + return avatars.get(actualJID); + } + return null; + } + + @Override + public void delete() { + // Empty Method + } + + private boolean useBare = false; + + private final Map avatars = new HashMap(); + + } + +} -- cgit v0.10.2-6-g49f6