From 98bacb144d126bfbcbee41650a49e1f3b1077d90 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Wed, 31 Aug 2016 08:20:13 +0200 Subject: [PATCH] Rework blocking command code. Mostly remove the helper utils. The server is required to present the client with a consisent state of the block list and corresponding modifications, so we should not end up with duplicate entires if we don't check for them. SMACK-731 --- .../blocking/BlockingCommandManager.java | 39 +++---------------- .../smackx/blocking/element/BlockListIQ.java | 8 ++-- .../blocking/element/UnblockContactsIQ.java | 10 ++++- .../provider/BlockListIQProvider.java | 5 ++- .../smackx/blocking/GetBlockingListTest.java | 2 +- 5 files changed, 24 insertions(+), 40 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/BlockingCommandManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/BlockingCommandManager.java index 5a364e0ba..6ee6bdce7 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/BlockingCommandManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/BlockingCommandManager.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016 Fernando Ramirez + * Copyright 2016 Fernando Ramirez, Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -44,6 +44,7 @@ import org.jxmpp.jid.Jid; * Blocking command manager class. * * @author Fernando Ramirez + * @author Florian Schmaus * @see XEP-0191: Blocking * Command */ @@ -96,7 +97,7 @@ public final class BlockingCommandManager extends Manager { } List blockedJids = blockContactIQ.getJids(); - addToBlockList(blockedJids); + blockListCached.addAll(blockedJids); return IQ.createResultIQ(blockContactIQ); } @@ -117,7 +118,7 @@ public final class BlockingCommandManager extends Manager { if (unblockedJids == null) { // remove all blockListCached.clear(); } else { // remove only some - removeFromBlockList(unblockedJids); + blockListCached.removeAll(unblockedJids); } return IQ.createResultIQ(unblockContactIQ); @@ -136,33 +137,6 @@ public final class BlockingCommandManager extends Manager { }); } - private void addToBlockList(List blockedJids) { - for (Jid jid : blockedJids) { - if (searchJid(jid) == -1) { - blockListCached.add(jid); - } - } - } - - private void removeFromBlockList(List unblockedJids) { - for (Jid jid : unblockedJids) { - int position = searchJid(jid); - if (position != -1) { - blockListCached.remove(position); - } - } - } - - private int searchJid(Jid jid) { - int i = -1; - for (int j = 0; j < blockListCached.size(); j++) { - if (blockListCached.get(j).equals(jid)) { - i = j; - } - } - return i; - } - /** * Returns true if Blocking Command is supported by the server. * @@ -197,8 +171,7 @@ public final class BlockingCommandManager extends Manager { BlockListIQ blockListIQResult = connection().createPacketCollectorAndSend(blockListIQ).nextResultOrThrow(); blockListCached = blockListIQResult.getJids(); - List emptyList = Collections.emptyList(); - return (blockListCached == null) ? emptyList : Collections.unmodifiableList(blockListCached); + return Collections.unmodifiableList(blockListCached); } /** @@ -241,7 +214,7 @@ public final class BlockingCommandManager extends Manager { */ public void unblockAll() throws NoResponseException, XMPPErrorException, NotConnectedException, InterruptedException { - UnblockContactsIQ unblockContactIQ = new UnblockContactsIQ(null); + UnblockContactsIQ unblockContactIQ = new UnblockContactsIQ(); connection().createPacketCollectorAndSend(unblockContactIQ).nextResultOrThrow(); } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/element/BlockListIQ.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/element/BlockListIQ.java index 52de9f04d..c24da5718 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/element/BlockListIQ.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/element/BlockListIQ.java @@ -16,6 +16,7 @@ */ package org.jivesoftware.smackx.blocking.element; +import java.util.Collections; import java.util.List; import org.jivesoftware.smack.packet.IQ; @@ -50,6 +51,9 @@ public class BlockListIQ extends IQ { */ public BlockListIQ(List jids) { super(ELEMENT, NAMESPACE); + if (jids == null) { + jids = Collections.emptyList(); + } this.jids = jids; } @@ -71,10 +75,8 @@ public class BlockListIQ extends IQ { @Override protected IQChildElementXmlStringBuilder getIQChildElementBuilder(IQChildElementXmlStringBuilder xml) { - - if (jids == null) { + if (jids.isEmpty()) { xml.setEmptyElement(); - } else { xml.rightAngleBracket(); diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/element/UnblockContactsIQ.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/element/UnblockContactsIQ.java index d0269222e..5b2260b37 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/element/UnblockContactsIQ.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/element/UnblockContactsIQ.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016 Fernando Ramirez + * Copyright 2016 Fernando Ramirez, Florian Schmaus * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,7 @@ import org.jxmpp.jid.Jid; * Unblock contact IQ class. * * @author Fernando Ramirez + * @author Florian Schmaus * @see XEP-0191: Blocking * Command */ @@ -54,6 +55,13 @@ public class UnblockContactsIQ extends IQ { this.jids = jids; } + /** + * Constructs a new unblock IQ which will unblock all JIDs. + */ + public UnblockContactsIQ() { + this(null); + } + /** * Get the JIDs. * diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/provider/BlockListIQProvider.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/provider/BlockListIQProvider.java index 2073f5528..520c22b52 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/provider/BlockListIQProvider.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/blocking/provider/BlockListIQProvider.java @@ -21,9 +21,9 @@ import java.util.List; import org.jivesoftware.smack.packet.IQ.Type; import org.jivesoftware.smack.provider.IQProvider; +import org.jivesoftware.smack.util.ParserUtils; import org.jivesoftware.smackx.blocking.element.BlockListIQ; import org.jxmpp.jid.Jid; -import org.jxmpp.jid.impl.JidCreate; import org.xmlpull.v1.XmlPullParser; /** @@ -49,7 +49,8 @@ public class BlockListIQProvider extends IQProvider { if (jids == null) { jids = new ArrayList<>(); } - jids.add(JidCreate.from(parser.getAttributeValue("", "jid"))); + Jid jid = ParserUtils.getJidAttribute(parser); + jids.add(jid); } break; diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/blocking/GetBlockingListTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/blocking/GetBlockingListTest.java index dd3062414..a14381ccf 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/smackx/blocking/GetBlockingListTest.java +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/blocking/GetBlockingListTest.java @@ -53,7 +53,7 @@ public class GetBlockingListTest { IQ iq2 = (IQ) PacketParserUtils.parseStanza(emptyBlockListIQExample); BlockListIQ emptyBlockListIQ = (BlockListIQ) iq2; - Assert.assertNull(emptyBlockListIQ.getJids()); + Assert.assertEquals(0, emptyBlockListIQ.getJids().size()); } }