Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XEP-0172 Improvement #276

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/**
*
* Copyright 2018 Miguel Hincapie.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jivesoftware.smackx.nick;

import org.jivesoftware.smack.packet.Message;

/**
* Used with NickManager.
*
* @author Miguel Hincapie 2018
miguelhincapie marked this conversation as resolved.
Show resolved Hide resolved
* @see <a href="http://xmpp.org/extensions/xep-0172.html">XEP-0172: User Nickname</a>
*/
public interface NickListener {
void newNickMessage(Message message);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
/**
*
* Copyright 2018 Miguel Hincapie.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jivesoftware.smackx.nick;

import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.WeakHashMap;

import org.jivesoftware.smack.AsyncButOrdered;
import org.jivesoftware.smack.Manager;
import org.jivesoftware.smack.SmackException;
import org.jivesoftware.smack.StanzaListener;
import org.jivesoftware.smack.XMPPConnection;
import org.jivesoftware.smack.filter.StanzaFilter;
import org.jivesoftware.smack.packet.Message;
import org.jivesoftware.smack.packet.Stanza;
import org.jivesoftware.smackx.nick.filter.NickFilter;
import org.jivesoftware.smackx.nick.packet.Nick;

import org.jxmpp.jid.EntityBareJid;
import org.jxmpp.jid.Jid;


/**
* Implementation of XEP-0172.
*
* @author Miguel Hincapie
* @see <a href="http://xmpp.org/extensions/xep-0172.html">XEP-0172: User Nickname</a>
*/
public final class NickManager extends Manager {

private static final Map<XMPPConnection, NickManager> INSTANCES = new WeakHashMap<>();

private static final StanzaFilter INCOMING_MESSAGE_FILTER = NickFilter.INSTANCE;

private final Set<NickListener> nickListeners = new HashSet<>();

private final AsyncButOrdered<Jid> asyncButOrdered = new AsyncButOrdered<>();

private NickManager(XMPPConnection connection) {
super(connection);

connection.addSyncStanzaListener(new StanzaListener() {
@Override
public void processStanza(Stanza packet)
throws
SmackException.NotConnectedException,
InterruptedException,
SmackException.NotLoggedInException {
final Message message = (Message) packet;

Jid from = message.getFrom();
if (from != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In order to avoid an extra level of indentation use if (from == null) return; here.

asyncButOrdered.performAsyncButOrdered(from, new Runnable() {
@Override
public void run() {
synchronized (nickListeners) {
for (NickListener listener : nickListeners) {
listener.newNickMessage(message);
}
}
}
});
}

}
}, INCOMING_MESSAGE_FILTER);
}

public static synchronized NickManager getInstanceFor(XMPPConnection connection) {
NickManager nickManager = INSTANCES.get(connection);
if (nickManager == null) {
nickManager = new NickManager(connection);
INSTANCES.put(connection, nickManager);
}
return nickManager;
}

public synchronized boolean addNickMessageListener(NickListener listener) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That synchronization is broken: You are protecting nickListeners with the Manager's object monitor and above you use the nickListeners's monitor.

return nickListeners.add(listener);
}

public synchronized boolean removeNickMessageListener(NickListener listener) {
return nickListeners.remove(listener);
}

public void sendNickMessage(EntityBareJid to, String nickname) throws
SmackException.NotLoggedInException,
InterruptedException,
SmackException.NotConnectedException {
sendStanza(createNickMessage(to, nickname));
}

/**
* Create a Smack's message stanza to update the user's nickName.
*
* @param to the receiver.
* @param nickName the new nickName.
* @return instance of Message stanza.
*/
private static Message createNickMessage(EntityBareJid to, String nickName) {
Message message = new Message();
message.setTo(to);
message.setType(Message.Type.chat);
message.setStanzaId();
message.addExtension(new Nick(nickName));
return message;
}

private void sendStanza(Message message)
throws
SmackException.NotLoggedInException,
SmackException.NotConnectedException,
InterruptedException {
XMPPConnection connection = getAuthenticatedConnectionOrThrow();
connection.sendStanza(message);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/**
*
* Copyright 2018 Miguel Hincapie.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jivesoftware.smackx.nick.filter;

import org.jivesoftware.smack.filter.StanzaExtensionFilter;
import org.jivesoftware.smack.filter.StanzaFilter;
import org.jivesoftware.smackx.nick.packet.Nick;

/**
* Used with NickManager.
*
* @author Miguel Hincapie 2018
* @see <a href="http://xmpp.org/extensions/xep-0172.html">XEP-0172: User Nickname</a>
*/
public final class NickFilter extends StanzaExtensionFilter {

public static final StanzaFilter INSTANCE = new NickFilter(Nick.ELEMENT_NAME, Nick.NAMESPACE);

private NickFilter(String elementName, String namespace) {
super(elementName, namespace);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
*
* Copyright 2018 Miguel Hincapie
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* Smacks implementation of XEP-0172: User Nickname.
*/
package org.jivesoftware.smackx.nick.filter;
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
*
* Copyright Miguel Hincapie
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.jivesoftware.smackx.nick;

import static org.junit.Assert.assertNotNull;

import org.jivesoftware.smack.DummyConnection;
import org.jivesoftware.smackx.InitExtensions;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;



public class NickManagerTest extends InitExtensions {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Flowdalic I was searching in current tests and I couldn't find anything as a guide or to follow, the most close to it was DeliveryReceiptTest, so please be patient with this part :).

Can I use Mockito?
i.e. for getInstanceFor I wish to:

  • test if INSTANCES is really working.
  • test if addSyncStanzaListener is being called inside constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm feeling lonely with this u_u'

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I use Mockito?

Yes, but it is usually a good idea to start with tests that do not require mocking.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please migrate this to JUnit 5.


@Before
public void setUp() {
}

@After
public void tearDown() {
}

@Test
public void getInstanceFor() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test methods should be prefixed (or postfixed) with "test".

DummyConnection dummyConnection = new DummyConnection();
dummyConnection.connect();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason for calling connect()?

NickManager nickManager = NickManager.getInstanceFor(dummyConnection);
assertNotNull(nickManager);
}

@Test
public void addNickMessageListener() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in order to be able to do this test I will need to add Set<NickListener> nickListeners to parameters constructor because I need to mock it first. What do you think @Flowdalic?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"to parameters constructor"? Depends on what you want to test within the test.

}

@Test
public void removeNickMessageListener() {
}

@Test
public void sendNickMessage() {
}
}