370074 - Tidy up some pref related code in the nsAbLDAP* classes

Status: RESOLVED FIXED

(MailNews Core :: LDAP Integration, defect)

Description

[Mark Banner (:standard8)]

Attachment: Part 1 v1

There's some tidy up and optimisations that we could do in the nsAbLDAP* classes to make the address book code more consistent and easier to understand.

The part one patch reuses some defined functions for getting items which means the directory uses the m_DirPrefId rather than working it out again. Additionally I've put a new function into nsIAbLDAPDirectory that allows us to get the specific attribute map for that LDAP server directly from the directory class (which also knows the base pref id) rather than having to QI to nsIAbDirectory and then use the base pref id.

Also part 1 fixes a problem with Get/SetAuthDn due to incorrect capitalisation of the pref name.

maxHits is still obtained through the helper functions as it's defined in the nsIAbDirectoryProperties class so I don't want to duplicate that in nsIAbLDAPDirectory just yet.

There's some other places that the optimisation could be done, but I need to think about those before I do that.

Comment 1

[David :Bienvenu]

Comment on attachment 254736 [details] [diff] [review]
Part 1 v1

Looks good, thx, Mark.

One nit:

+  nsCOMPtr<nsIAbLDAPAttributeMapService> mapSvc = 
+    do_GetService("@mozilla.org/addressbook/ldap-attribute-map-service;1");
+  if (!mapSvc)
+    return NS_ERROR_FAILURE;

Here, I'd prefer returning the actual error from do_GetService...

Comment 2

[Mark Banner (:standard8)]

Attachment: Part 2 v2

The second part:

- Fixes nit from part 1 (that I forgot on check in)
- sorts out remaining pref usage in nsAbLDAPDirectory
- removes unnecessary headers
- corrects nsAbLDAPDirectory::GetURI (in the uri is empty case).

Comment 3

[David :Bienvenu]

Comment on attachment 254860 [details] [diff] [review]
Part 2 v2

thx, Mark

Comment 4

[Mark Banner (:standard8)]

Both patches checked in, nothing more to do for this bug -> fixed.