Details

    • Type: Feature Request Feature Request
    • Status: Contributed Solution
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 6.1.0 CE RC1, 6.2.0 CE M3
    • Fix Version/s: None
    • Component/s: Chat
    • Labels:
      None
    • Business Value:
      5
    • Similar Issues:
      Show 5 results 

      Description

      We needed to use the latest trunk chat-portlet sooner than later, and as part of that, we had to make some modifications for it to work for us. Please let us know if these modifications could be added into the mainline trunk for 6.1 support.

      Below they are described and included in a patch file, and hopefully these changes can be included/supported in the official 6.1 release whenever it comes out.

      Please note the patch file also includes some service generation code which is not different than the trunk. The primary changes are all in the jabber.* package. and the main.js for 6.0 backport support.

      In general our use case is that we needed to use it with Google Apps corporate accounts. Our users have corresponding accounts in our google apps corporate account and there are more than one @domainname (sub-domains) under our google apps account so assumptions currently in the trunk about all user's screen names existing within one domain, or screen name reliance only, does not work with Smack -> Google Talk for a corporate account.

      Overall we are asking that configurable support for the changes we made be rolled into the planned mainline release of the chat-portlet so that we do not have to maintain our own branch of it.

      OVERVIEW OF CHANGES WE MADE
      -------------------------------------------------------------

      (#1) Changed JabberImpl's sendMessage() to set the TO (destination user's) jabberId = the user's email address rather than just the screen name.

      Can an config option be added for this behavior?

      (i.e.) String jabberId = toUser.getEmailAddress();

      (#2) Altered JabblerImpl.connect()'s Smack ConnectionConfiguration to set the service name equal to the user's email domain (right side of @) which is what is needed to work with google apps accounts.

      Can an config option be added for this kind of behavior?

      ConnectionConfiguration config = getConnectionConfiguration();
      String serviceName = StringUtil.extractLast(user.getEmailAddress(), StringPool.AT);
      config.setServiceName(serviceName);
      connection = new XMPPConnection(config);

      (#3) Altered the JabberImp.connect()'s smack connection login call to pass email address instead of screen name

      Can an config option be added for this kind of behavior? Likely could read from the same config for the above.

      connection.login(user.getEmailAddress(), password, PortletPropsValues.JABBER_RESOURCE);

      (#4) Adjusted JabberMessageListener.processMessage() to retrieve the sender user by email address rather than screen name due to screen names being different (potentially) than the email for the user (which must match the jabberid MINUS the /resource). This change also added a new method on the Jabber interface: public String getEmailAddress(String jabberIdWithResource); plus on the implementing classes. Again the use of looking up by email might need to be controlled by a config property.

      (#5) Adjusted the JabberImpl's getStatuses() method to fetch by email instead of screen name (same reasons as above) as well as catch Exceptions and do a null check in calls to "UserLocalServiceUtil.getUserByEmailAddress()" when a user returned by the XMPP server does not exist in the local liferay install. It seems legitimate that someone might connect this to an XMPP server where users in someone's roster are not in the local Liferay install. This should fail without error, (log/warn maybe) but not cause it to bomb out. Just filter the user from the roster returned. (i.e. _log.warn("XMPP Roster user: " + rosterEntry.getUser() + " not in local Liferay... skipping adding to chat roster..")

      (#6) We front our liferay install via an SSO mechanism and have a custom AuthLogin hook implemented. Subsequently the LoginPostAction in com.liferay.chat.hook.events.LoginPostAction has no password available to us for Smack to consume in its login calls. We made a modification in our copy to fetch it via other means as Smack requires it.

      *Please continue by looking at the attachments included in reading the description. The post was unexpectedly cut off.

      -------------------------------------------------------------

      1. chat-portlet.main.js.20110719.patch
        2 kB
        Ira Chui
      2. com.liferay.chat.jabber.20110719.patch
        6 kB
        Ira Chui
      3. jabberChangesFinal.rtf
        5 kB
        Ira Chui
      4. xmpp.duplicate.fix.20110803-omg.txt
        2 kB
        AaronZee

        Issue Links

          Activity

          Hide
          AaronZee added a comment -

          One thing to be aware of off the bat, is that one bug we have seen already is that sometimes when 2 users are talking to one another, the messages TO/FROM get mixed up and multiple are sent, sometimes dozens repeatedly. In some instances this repeats into the thousands, killing the browser, sort of like a DOS. These can be seen in the UI + the chat entry table. Not sure where or what is originating this issue (message listener? bug in entries into database? JS polling/comet side?). We will investigate on our end as well to try to reproduce/narrow down the source of it as well.

          Show
          AaronZee added a comment - One thing to be aware of off the bat, is that one bug we have seen already is that sometimes when 2 users are talking to one another, the messages TO/FROM get mixed up and multiple are sent, sometimes dozens repeatedly. In some instances this repeats into the thousands, killing the browser, sort of like a DOS. These can be seen in the UI + the chat entry table. Not sure where or what is originating this issue (message listener? bug in entries into database? JS polling/comet side?). We will investigate on our end as well to try to reproduce/narrow down the source of it as well.
          Hide
          AaronZee added a comment -

          Update on looping issue:

          EntryLocalServiceImpl's addEntry(date,from,to,content) was triggering a loop when called from the JabberMessageListener.processMessage() method. The scenario being that in some instances when a message arrives for the User, the process message method would call addEntry() to persist the message locally, the side effect of method was an assumption to call JabberUtil.sendMessage() which then resent the message back to the recipient again.

          To fix, we added variants of EntryLocalServiceImpl.addEntry() that take a "sendViaJabber" boolean which should be set to false when called by JabberMessageListener.processMessage() to prevent the loop. This required a regeneration of services as well. Once services are regenerated, you will also need to update ChatPollerProcessor.addEntry() to call the new method variant with "true" for sendViaJabber.

          public Entry addEntry(long fromUserId, long toUserId, String content) throws SystemException

          { return addEntry(fromUserId, toUserId, content, false); }

          public Entry addEntry(long createDate, long fromUserId, long toUserId, String content) throws SystemException

          { return addEntry(createDate, fromUserId, toUserId, content, false); }

          public Entry addEntry(long fromUserId, long toUserId, String content, boolean sendViaJabber)
          throws SystemException

          { long createDate = System.currentTimeMillis(); return addEntry(createDate, fromUserId, toUserId, content, sendViaJabber); }

          public Entry addEntry(
          long createDate, long fromUserId, long toUserId, String content, boolean sendViaJabber)
          throws SystemException {

          if (Validator.isNull(content)) {
          content = StringPool.BLANK;

          List<Entry> entries = entryFinder.findByEmptyContent(
          fromUserId, toUserId, 0, 5);

          for (Entry entry : entries)

          { entryPersistence.remove(entry); }

          }
          else {
          if (content.length() > 500)

          { content = content.substring(0, 500); }

          }

          long entryId = counterLocalService.increment();

          Entry entry = entryPersistence.create(entryId);

          entry.setCreateDate(createDate);
          entry.setFromUserId(fromUserId);
          entry.setToUserId(toUserId);
          entry.setContent(content);

          entryPersistence.update(entry, false);

          if (sendViaJabber)

          { JabberUtil.sendMessage(fromUserId, toUserId, content); }

          return entry;
          }

          Show
          AaronZee added a comment - Update on looping issue: EntryLocalServiceImpl's addEntry(date,from,to,content) was triggering a loop when called from the JabberMessageListener.processMessage() method. The scenario being that in some instances when a message arrives for the User, the process message method would call addEntry() to persist the message locally, the side effect of method was an assumption to call JabberUtil.sendMessage() which then resent the message back to the recipient again. To fix, we added variants of EntryLocalServiceImpl.addEntry() that take a "sendViaJabber" boolean which should be set to false when called by JabberMessageListener.processMessage() to prevent the loop. This required a regeneration of services as well. Once services are regenerated, you will also need to update ChatPollerProcessor.addEntry() to call the new method variant with "true" for sendViaJabber. public Entry addEntry(long fromUserId, long toUserId, String content) throws SystemException { return addEntry(fromUserId, toUserId, content, false); } public Entry addEntry(long createDate, long fromUserId, long toUserId, String content) throws SystemException { return addEntry(createDate, fromUserId, toUserId, content, false); } public Entry addEntry(long fromUserId, long toUserId, String content, boolean sendViaJabber) throws SystemException { long createDate = System.currentTimeMillis(); return addEntry(createDate, fromUserId, toUserId, content, sendViaJabber); } public Entry addEntry( long createDate, long fromUserId, long toUserId, String content, boolean sendViaJabber) throws SystemException { if (Validator.isNull(content)) { content = StringPool.BLANK; List<Entry> entries = entryFinder.findByEmptyContent( fromUserId, toUserId, 0, 5); for (Entry entry : entries) { entryPersistence.remove(entry); } } else { if (content.length() > 500) { content = content.substring(0, 500); } } long entryId = counterLocalService.increment(); Entry entry = entryPersistence.create(entryId); entry.setCreateDate(createDate); entry.setFromUserId(fromUserId); entry.setToUserId(toUserId); entry.setContent(content); entryPersistence.update(entry, false); if (sendViaJabber) { JabberUtil.sendMessage(fromUserId, toUserId, content); } return entry; }
          Hide
          AaronZee added a comment -
                1. @See attached xmpp.duplicate.fix.20110803-omg.txt file for code change excerpts.####

          Recently fixed an additional issue with this. When 2 users chatting (jabber/xmpp) with one another are both logged in locally to liferay:

          (1) the message is added to the Chat_Entry table. Both users see this in their chat window via the ChatPollerProcessor picking up the new message from Chat_Entry.

          (2) Then the message is sent out over XMPP, which is then received back via JabberMessageListener.processMessage() for the recipient (the recipient also logged in locally to LF, with a smack xmpp connection listener). The processMessage() method then blindly re-adds the message to Chat_Entry, creating a double entry given it was already logged via (1) above.

          (3) Both users see the message 2x in their chat windows and double entries are evident in the Chat_Entry table

          What needed to be fixed was a few things, mainly properly handling inbound messages from the remote XMPP server to detect the senders full XMPP resource identifier, and correlate it with the local registry to avoid duplicate entries in Chat_Entry. This also permits people to be logged into liferay + other XMPP clients to their remote XMPP server (that Liferay itself is also proxying) and receive/send messages in either client and everyone is happy.

          (A) We exposed a new method on the Jabber interface: public String getLocalJabberResourceForUserId(long userId); This method is intended to return the local full jabber resource id (user@domain/xmppresource) for the given user that is registered on the local Liferay instance via the Smack connection listings located in the static JabberImpl._connections registry

          (B) Create a hook on JabberUtil with the same name, relaying the call to the Jabber instance: such as

          public static String getLocalJabberResourceForUserId(long userId)

          { return getJabber().getLocalJabberResourceForUserId(userId); }

          (C) Implement the method on JabberImpl which looks up the user's full local jabber xmpp resource identifier in Liferay. (below)

          public String getLocalJabberResourceForUserId(long userId) {
          Connection xmppConn = _connections.get(userId);
          if (xmppConn == null)

          { return null; }

          return xmppConn.getUser();
          }

          (D) Finally adjust JabberMessageListener.processMessage() to consult this new method before calling EntryLocalServiceUtil.addEntry(), to avoid double entries in the Chat_Entry table.

                1. @See attached xmpp.duplicate.fix.20110803-omg.txt file for code change excerpts.####
          Show
          AaronZee added a comment - @See attached xmpp.duplicate.fix.20110803-omg.txt file for code change excerpts.#### Recently fixed an additional issue with this. When 2 users chatting (jabber/xmpp) with one another are both logged in locally to liferay: (1) the message is added to the Chat_Entry table. Both users see this in their chat window via the ChatPollerProcessor picking up the new message from Chat_Entry. (2) Then the message is sent out over XMPP, which is then received back via JabberMessageListener.processMessage() for the recipient (the recipient also logged in locally to LF, with a smack xmpp connection listener). The processMessage() method then blindly re-adds the message to Chat_Entry, creating a double entry given it was already logged via (1) above. (3) Both users see the message 2x in their chat windows and double entries are evident in the Chat_Entry table What needed to be fixed was a few things, mainly properly handling inbound messages from the remote XMPP server to detect the senders full XMPP resource identifier, and correlate it with the local registry to avoid duplicate entries in Chat_Entry. This also permits people to be logged into liferay + other XMPP clients to their remote XMPP server (that Liferay itself is also proxying) and receive/send messages in either client and everyone is happy. (A) We exposed a new method on the Jabber interface: public String getLocalJabberResourceForUserId(long userId); This method is intended to return the local full jabber resource id (user@domain/xmppresource) for the given user that is registered on the local Liferay instance via the Smack connection listings located in the static JabberImpl._connections registry (B) Create a hook on JabberUtil with the same name, relaying the call to the Jabber instance: such as public static String getLocalJabberResourceForUserId(long userId) { return getJabber().getLocalJabberResourceForUserId(userId); } (C) Implement the method on JabberImpl which looks up the user's full local jabber xmpp resource identifier in Liferay. (below) public String getLocalJabberResourceForUserId(long userId) { Connection xmppConn = _connections.get(userId); if (xmppConn == null) { return null; } return xmppConn.getUser(); } (D) Finally adjust JabberMessageListener.processMessage() to consult this new method before calling EntryLocalServiceUtil.addEntry(), to avoid double entries in the Chat_Entry table. @See attached xmpp.duplicate.fix.20110803-omg.txt file for code change excerpts.####
          Hide
          Juan Fernández added a comment -

          This should be reviewed by a core engineer.
          Added to product backlog
          Thanks for the feedback

          Show
          Juan Fernández added a comment - This should be reviewed by a core engineer. Added to product backlog Thanks for the feedback
          Hide
          Billy added a comment -

          How to use it ? Let me test!

          Show
          Billy added a comment - How to use it ? Let me test!
          Hide
          Billy added a comment -

          abc

          Show
          Billy added a comment - abc
          Hide
          Randy Zhu added a comment -

          In preparation for Ideation; we are merging New Feature and Improvement tickets into a singular ticket type called “Feature Request”. Additional information to follow soon.

          Show
          Randy Zhu added a comment - In preparation for Ideation; we are merging New Feature and Improvement tickets into a singular ticket type called “Feature Request”. Additional information to follow soon.

            People

            • Votes:
              9 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Days since last comment:
                2 years, 24 weeks, 3 days ago

                Development

                  Structure Helper Panel