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 6 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,127 @@
/**
*
* 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;


/**
* Implementation of XEP-0172.
*
* @author Miguel Hincapie 2018
* @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<Message> 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;

asyncButOrdered.performAsyncButOrdered(message, new Runnable() {
Copy link
Member

Choose a reason for hiding this comment

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

The AsyncButOrdered API is not correctly used. Right now, it assures the runnable is ordered with respect to the message, when it should be ordered with respect to the originating entity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you explain me a little bit more about what could be the originating entiy?
Because there is no much doc about that AsyncButOrdered I have been using it as I saw in others parts of Smack, so it's not much clear to me how should I use it or when or the options I have instead it.

Copy link
Member

Choose a reason for hiding this comment

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

The originating entity is the entity that send the message stanza. So you need to replace message with message.getFrom(). But make sure to add a null check.

Copy link
Contributor Author

@miguelhincapie miguelhincapie Mar 30, 2019

Choose a reason for hiding this comment

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

and what if it is null? I mean, should I throw an exception?

@Override
public void run() {
for (NickListener listener : nickListeners) {
Copy link
Member

Choose a reason for hiding this comment

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

nickListeners is not synchronized although it should be

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean something like:

synchronized (nickListeners) {
     for (NickListener listener : nickListeners) {
         listener.newNickMessage(message);
     }
 }

am I right?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, or some other sort of means to ensure access to the datastructure is properly synchronized.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, added a synchronized, tell me if that is enough or you like another kind of synchronized.

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 Message createNickMessage(EntityBareJid to, String nickName) {
miguelhincapie marked this conversation as resolved.
Show resolved Hide resolved
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;