Patch that corrects the generation of Nicknames in search when using LDAP directory

The popup dialog is actually populated from a column pulled straight from the search table. The actual column name is ‘Name’ so the fix (diff) is actually as follows.

With this diff applied the Spark client picks up the Name column so as long as you have ‘ldap.nameField = displayName’ on the server that sorts out field mapping properly as the displayName

The original implementation code doesn’t make a whole lot of sense as it tries ‘Username’ as a field which always exists and then on the catch tries ‘nick’ as a field which doesn’t exist.

This patch changes that so it tries ‘Name’ as a field and if it doesn’t exist it fails back to ‘Username’.

Speaking to the developer now. Wasn’t that what you intended in the first place?

— UserSearchResults.java.orig 2009-08-24 12:24:25.000000000 +0100
+++ UserSearchResults.java 2009-08-24 12:36:09.000000000 +0100
@@ -139,11 +139,11 @@

             TableColumn column = null;
             try {
  •                column = resultsTable.getColumn("Username");
    
  •                column = resultsTable.getColumn("Name");
               }
               catch (Exception ex) {
                   try {
    
  •                    column = resultsTable.getColumn("nick");
    
  •                    column = resultsTable.getColumn("Username");
                   }
                   catch (Exception e1) {
                       // Nothing to do

Hi,

Isn’t that more or less the same like http://www.igniterealtime.org/community/thread/37867?

But here you can yourself choose which column to use for the nickname …

wroot wants me to a some description text field - but I’ve no glue about programming this, that patch above happend by accident

Mueller,

I hazarded a guess initially (you can see it over on the OpenFire support forum) about the nature of the problem after I had done some packet analysis to make sure the VCard tags coming over were anticipated.

I also noted that the behaviour of the WebSpark client was to display the Name when a search selection was added rather than Username.

Then I saw your patch too which was very similar to the changes I originally put forward.

The reason for putting this patch forward is that it seems more sensible in terms of general usability and consistent with what WebSpark does so I imaged that is what the developer had in mind. Generally people just want a sensible name to be displayed in the Nickname box when they have searched out the user from the directory.

I can see the merits of having the value that the dialog is populated with as a preference but having folks highlight the value each time? No, I think that may be a little too far off in terms of obvious usability.

The section that I pointed out in its original form doesn’t make a great deal of sense to me since, as explained, the code try(tries)s the column get on the value ‘Username’ which to be returned in the column set MUST be present. The exception handler (catch statement) then tries ‘nick’ as a fieldname which actually doesn’t exist in the columns. Doesn’t make a heap of sense since the catch clause is pretty much guaranteed to never happen so you will always get ‘Username’.

This way round makes much more sense, the ‘Name’ field is tried and if it can’t be read (maybe the field isn’t populated) the catch clause should tidy it up and pick up ‘Username’. The user stands a much better chance at getting a more meaningful name out of the directory with a suitably specced VCard and ldap.nameField.

I think we are both suggesting the same thing - it’s interesting that independently we both arrived at the same conclusion so that should help with a little bit of attention at the main trunk (here’s hoping). It looks like the root cause of a lot of queries on this board so I think the next thing is to see if the devs can explain if there is a good reason that the original code is as it is or if either patch has merit. I don’t really want to maintain a private fork each time a release comes out myself and I’m sure not many do.

Kind regards,

-Andy

OK, here’s an update.

I did some more work on this and it helps to come at the problem understanding a full XMPP infrastructure.

When you add somebody to a Roster from an LDAP search that’s easy from the originator end because the server can easily provide a VCard - you are after all searching a service that does that.

What happens then internally is that the Client requests a ‘subscription’ for presence information that goes through your server - or remember that this could route outside - and gets routed to the user that you added to your Roster. The subscription request that hits the user being added is the initiator of that dialog that pops up saying ‘xxx wants to add you…’ and is titled ‘Subscription Request’ under Spark.

Here is where the limitations come in.

The Subscription request could actually come from anywhere even external if your OpenFire server/infrastructure is thus configured.

The Subscription request is very lean. It is a presence message that contains a type of ‘subscribe’, a unique id, the recipient JID and the requestors JID. The Spark client also appears to send a VERY small VCard tagged on the XML namespace ‘vcard-temp:x:update’ which carries only the avatar as a ‘photo’ field. No nickname. This means that the receiving client has nothing to base a Nickname suggestion on other than the JID.

I would have to do some more digging to see if it is legitimate to try to make the presence subscription request carry a nickname too which seems to be the way of resolving the fact that Spark translates the Nickname to the username part of the JID for users receiving these subscription requests.

Simply doing it isn’t the right way. I need to check that this is legitimate and in spec before doing it.

OK, here’s the rub I suspect.

Some IM server/client systems view the Nickname as something personal to the user concerned and updates to it should go to everyone elses Rosters.

This system views the Nickname as personal to others so they are at liberty to Nickname the person concerned anything they like in their own Rosters.

I suspect that if I add to that ‘vcard-temp:x:update’ the Nickname it will get changed to the first behaviour described as the user signs on and reaffirms their subscription.

The Avatar is of course something that users push to people and would want updated at remote Rosters.

This may be more suitable to some organisations but may not suit individual preferences. Any devs wanting to chip in with their considered views on this?

EDITI just checked the receiving code, it only has special handling for this one attribute and won’t touch the Nickname. Again I think I need to consult specs or have somebody clarify what is acceptable and standard to push as an extra attributeEDIT

Speaking to the developer now. Wasn’t that what you intended in the first place?

Who knows. The original developer is not here anymore. I cant say whether your solution is ok with the specs or no, but it seems ok and similar to the suggestions here: SPARK-1021

I suggest to attach your patch here as a .diff file, so i can attach it to that ticket.

Is a diff from the current directory fine (since it relates to just one file) or do you want a diff -nur? The text of the original diff is embedded in the first posting here.

I will look into the receipt of nicknames via extra hints in the subscription request seperately since this is a little contentious.

I want to see what and how some other client software implements since there doesn’t seem to be an awful lot of guidance on the subject in the XMPP documentation around. At the end of the day making that sort of choice should be down to using the standard approach, extending the spec isn’t something I think should be done regardless of how convenient it seems.

Regards,

-Andy

I’m not a coder myself, but i think we need only a diff of that file you have changed. Also i didnt want to save it myself from your first post, as i may save some unneeded chars or brake something It’s better when developer provides the diff.

No worries, here you go ^^
UserSearchResults-username-on-roster-add.patch.zip (441 Bytes)

Ok. I’ve attached it to the above mentioned ticket.

OK, however, I highlighted this issue in response to giving a better field on an LDAP directory integrated copy of OpenFire and Spark.

The issue that this patch got attached to is titled around non LDAP deployments. The nature of the patch is likely to get lost possibly because of that.

Regards,

-Andy

Ok. I’ve removed the attachment. Maybe your patch needs another ticket. But what about comments in that ticket. Todd Getz says it’s ok with the LDAP setup:

The issue comes into play when trying to manually add a user to a
roster. If there is no nickname field for spark to use it defaults the
username as it was entered during account creation. This could be
anything tg2471 or something like that. Shared groups on the other hand
default to the Name data as entered when creating an account. This is
the same for LDAP which uses which can be set to what
ever the admin likes (displayName).
In short the add contact to roster needs to change to auto reference
some field that is present in every account created on the server, like
the name field. Right now if an LDAP openfire server has the vCard
field for nickname configured
({displayName} ) that info
auto-populates to the new contact dialog box when the user account is
entered. We need a similar functionality for those accounts that do not
have data there. It is partially there in that it auto completes with
the username.