Suggested adds

[Ian]

Hey.

I am happy see that there are several bug hunters in the woods.

I have suggestions for the following items:

1. Simplemessenger Add / Remove
2. Setting personal message bug
   - UUX not supported
   - PM without Current Media

1. Simplemessenger Add / Remove

This would prevent some unnessary messaging towards MSN NF. Contact's participation in given list could be checked before any messaging when adding and removing. Methods:
  
   private void addFriend
   private void removeFriend

For instance private addFriend is called from 3 methods (blockFriend, unblockFriend, public addFriend). Only in public addFiend it is checked whether the contact is already in the specified list. Subversion implementation for block/unblockFriend causes unnessary messaging.

My suggestion is to modify private add/removeFriend methods to see whether the contact is already in the specified list.

[private SimpleMessenger.addFriend() AND private SimpleMessenger.removeFriend()]
   // add this after parameter checks   
   MsnContact contact = getContactList().getContactByEmail(email);
   if (contact.isInList(list)){
      return ;
   }

2. Setting personal message bug

There seems to be some bug with setting personal message.

First of all the UUX do not work with protocols protocol.before(MsnProtocol.MSNP11)).

    The UUX is not support in protocols 8 - 10.
    The Java JML should informs it in case the protocol in wrong.
    In case the protocol is set to (< 11) then the NS will
    logout the user at once, without giving any notification.

    http://www.messenger-blog.com/?p=116

PM without Current Media
   
    Personal Message (PM) should be able to send without Current Media.

    http://msnpiki.msnfanatic.com/index.php/MSNP11:Changes#UUX

I made following fixes:

    [MsnOwnerImpl]
       public void setPersonalMessage(String info) {
          if (info != null) {
             OutgoingUUX message = new OutgoingUUX(messenger.getActualMsnProtocol());
         /* removed - end */
         message.setPersonalMessage(info);
         messenger.send(message);
          }
       }
   
    [MsnOwnerImpl]
       public void setCurrentMedia(String info) {
          if (info != null) {
        OutgoingUUX message = new OutgoingUUX(messenger.getActualMsnProtocol());
            message.setPersonalMessage(info);
            /* added */
            message.setCurrentMedia(info, null, null, null);
            /* end */
        messenger.send(message);
          }
       }

    [OutgoingUUX]
             public OutgoingUUX(MsnProtocol protocol) {
                super(protocol);
                if (protocol.before(MsnProtocol.MSNP11)){
                   throw new UnsupportedProtocolException(new MsnProtocol[] { protocol }) ;
                }
                setCommand("UUX");
             }

    [OutgoingUUX]
   
        private void buildData() {
           StringBuffer buffer = new StringBuffer();
           buffer.append("<Data>");
           buffer.append("<PSM>");
           buffer.append(StringUtils.xmlEscaping(personalMessage));
           buffer.append("</PSM>");
           if (currentMedia != null){
              buffer.append("<CurrentMedia>");
              buffer.append(StringUtils.xmlEscaping(currentMedia));
              buffer.append("</CurrentMedia>");
           }
           buffer.append("</Data>");
           setChunkData(buffer.toString());
        }

When calling the constructor and the protocol version is wrong an exception is send. This is only a suggestion how the wrong protocol usage could be handled. I suggest that in overall we will take some kind of policy how we will inform the client program that is using the JML stack with a wrong protocol version.

Sincerelly, Ian

[Daniel Henninger]

Hey please check out what I just committed to SVN.  =)  I went with a lot of your suggestions and added a lot of other things.

[Ian]

Looks nice..

Here are couple of new suggestions:

1. Null check to IncomingPRP
2. Checking displayName in MsnOwnerImpl

1. Null check to IncomingPRP

There seems to arise a NullpointerException when setting a displayname. This is pretty bizarre cause It seems that I will receive multiple PRP packets, or at least the ImcomingPRP.messageReceived() is processed multiple times.

With a following fix a managed to void the NullpointerException:

   if (propType != null && prop != null){
      // a plain null check before using these variables

   }

2. displayName check to MsnOwnerImpl

When we are changing the displayname, I suggest that we check the current display name before sending any signalling towards Msn NS:

   [in MsnOwnerImpl.setDisplayName()]
   if (displayName.equals(getDisplayName())){
      return ;
   }

[Daniel Henninger]

Excellent suggestions, committed.

[Ian]

Hello JML,

I have couple of changes pending in my local datastore which could be added to next release.

1. MsnContact Interface
2. IncomingREM

1. MsnContact Interface

   public String getPersonalMessage();

   I added this method definition also to interface (exists already on implementation class)

2. IncomingREM
  
   I am using MSNP12 and I have noticed that the removing contacts do now work. With the below fix I got it running.

    <!-- IncomingREM.getEmail() -->
    public Email getEmail() {
        if (getList() == MsnList.RL || protocol.after(MsnProtocol.MSNP10)) {
            if (protocol.before(MsnProtocol.MSNP10)) {
                return Email.parseStr(getParam(2));
            }
            return Email.parseStr(getParam(1));
        }
        else {
            return null;
        }
    }
    <!-- end -->

    <!-- part of IncomingREM.messageReceived() -->

            if (list == MsnList.FL) {
                contact = (MsnContactImpl) contactList.getContactById(getId());
            } else {
                contact = (MsnContactImpl) contactList.getContactByEmail(getEmail());
            }

    <!-- old-code -->

            if (list == MsnList.AL) {
                contact = (MsnContactImpl) contactList.getContactByEmail(getEmail());
            }
            else {
                contact = (MsnContactImpl) contactList.getContactById(getId());
            }

    <!-- end -->
  

[Daniel Henninger]

Hi Ian!  I just committed your updates.  I'm pretty sure I got the last part right by your suggestions but I wasn't 100% sure.  Would you confirm?

[EricZheng]

It doesn't work that fix for Null exception of OutgoingUUX. The codes are from SubVersion.

java.lang.NullPointerException
    at net.sf.jml.protocol.outgoing.OutgoingUUX.<init>(OutgoingUUX.java:40)
    at net.sf.jml.impl.MsnOwnerImpl.setPersonalMessageAndCurrentMedia(MsnOwnerImpl.java:143)

[EricZheng]

ianlamberg，

Why does not it work to follow your suggestion? I followed your comments to do the code modification. But null exceptions are still thrown.

Don't know the reason?

Could you please attach the file here? Thanks a lot.

[Ian]

Have you downloaded the latest sources from the Subversion? The only thing that is missing from the SVN is the modifications I suggested in my '2007-02-07 07:57' reply. Well to be honest.. I havent tested to current SVN release what it comes to setting CurrentMedia. The personalMessage definetely works.

Sincerelly, Ian

[EricZheng]

hi, Ian.

I download the newest one from subversion. It wokrs fine, including setCurrentMedia. Thanks