355363 - Make a base class for LDAP Message Listener code

Status: RESOLVED FIXED

(MailNews Core :: LDAP Integration, defect)

Description

[Mark Banner (:standard8)]

Currently, in the address book we have a nsAbQueryLDAPMessageListener that is used to handle messages being received from the ldap interface. This includes the code used when binding.

To handle modifing LDAP address books we'll need similar code. Therefore I suggest separating the generic parts of the nsAbQueryLDAPMessageListener into a nsAbLDAPListenerBase class ready for use when we start being able to modify address books.

I've got a patch in progress for this it still needs some tweaks, but I should be able to post it soon.

Comment 1

[Mark Banner (:standard8)]

Attachment: Patch v1

This still needs a bit of work to check for regressions but essentially I think that the base class should look roughly like this for now. This will form the base class, I'm then thinking we'll have a nsAbLDAPDirectoryModify class/file to handle the actual modifications separate to the nsAbLDAPDirectoryQuery and the nsAbLDAPDirectory implementations.

Comment 2

[Mark Banner (:standard8)]

Attachment: Patch v2

This patch has a few more improvements in it so that ldap auth should work correctly in all instances, I've also included the recent changes that we did for dropping pointers at the right times. This should get us what we need to help with implementing writeable ldap directory listeners etc.

Comment 3

[David :Bienvenu]

Comment on attachment 244880 [details] [diff] [review]
Patch v2

In general, this looks good. One thing to check is that we haven't introduced any new cycles in the ldap objects structure, such that ldap connections aren't closed when ab windows are closed.

Comment 4

[Mark Banner (:standard8)]

(In reply to comment #3)
> (From update of attachment 244880 [details] [diff] [review] [edit])
> In general, this looks good. One thing to check is that we haven't introduced
> any new cycles in the ldap objects structure, such that ldap connections aren't
> closed when ab windows are closed.
> 
I don't believe I've done this, as the base class just separates the code out. I've tested various scenarios and not had any problems yet.

Comment 5

[Scott MacGregor]

Comment on attachment 244880 [details] [diff] [review]
Patch v2

should this:

+  rv = ldapOperation->SimpleBind(NS_ConvertUTF16toUTF8(passwd));
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  return rv;

just be

return ldapOperation->SimpleBind(NS_ConvertUTF16toUTF8(passwd));

Comment 6

[Mark Banner (:standard8)]

Patch checked in.