Status

MailNews Core
LDAP Integration
--
enhancement
16 years ago
7 years ago

People

(Reporter: Rajiv Dayal, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

16 years ago
nsDirPrefs should either be implemented as a singleton or a XPCOM service class
so that it provides a centralized place to access directory preferences. In this
case the functions of nsDirPrefs should be re-enterant. Also it has been copied
from 4x and does not effectively use Mozilla constructs (specially for strings)
using which can make the code cleaner and safer. Further it can be enhanced to
implement an Observer for the directory prefs and notification mechanism for its
clients on changes to the related prefs.
(Reporter)

Comment 1

16 years ago
assigning to myself for now.
Assignee: srilatha → rdayal
Target Milestone: --- → Future
here's a conversation about bug #126134 and DIR_Server and reading prefs 
directly.

cc srilatha, who this will also affect:

to summarize:

until we have a thin, scriptable managing service for addressbook attribute 
management, (like DIR_Server), we should write code that reads prefs directly.

The thin service will just be a thin veneer on top of prefs, but it will keep 
the details of how addressbook attributes are persisted (in our case, with 
prefs) away from the calling code.

examples:  the code to delete a server will just use the pref branch code to 
enumerate and clear all prefs that being with a certain prefix.

we'll need some way to get / set attributes using attribute name, and a key (in 
our case, key is a pref prefix)

I'm sure we'll also need some observer / listener type setup to notify those 
observer who need to know when servers are added or deleted.


dmose2: "so that when we delete servers, all the necessary prefs are delete.
It also provides an good place to put the code that generates 
a unique .mab file name
dmose2: neither of those things require a level of abstraction on top of prefs 
at all
saspitzer: yes, they do.
saspitzer: the first one does, because
saspitzer: well, you could use prefbranch
dmose2: right
saspitzer: so something needs to be told "delete this server with key"
dmose2: sure
saspitzer: but that's all implementation on top of prefs.
dmose2: but there's no reason to make every prefs getter and setter go through 
that code
dmose2: because it's orthogonal
saspitzer: you're going further and saying:
saspitzer: "we don't even need a server object, we can just use prefs"
dmose2: i'm saying "we don't need a server object when getting and setting 
prefs"
dmose2: having an object to do things like generate a file name, and delete 
stuff is ok
dmose2: but inserting that object into the middle of getting and setting prefs 
doesn't gain anything that i can see
saspitzer: how about this:
saspitzer: the new, scriptable service that rdayal (or you, or me or 
srilatha...) creates will allow for this:
saspitzer: given a key, and an "attribute", get me the value.
saspitzer: the impl for that, will be a call right into prefs.
saspitzer: I don't want the caller to have to know how we store the values, I'd 
like to keep it all abstracted.
saspitzer: that won't require an object in the middle
dmose2: i'm confused
saspitzer: but all callers will go through this managing service.
saspitzer: like this:
dmose2: wait
dmose2: take a step ack
dmose2: back
saspitzer: ok
dmose2: if i understand you correctly...
dmose2: the problem you're trying to solve is the fact that the prefs have a 
typing system?
saspitzer: no.  here's what I'm getting at.
dmose2: ie what details are you trying to abstract away?
saspitzer: that we use prefs to implement a persistent way to remember details 
about addressbooks.
saspitzer: prefs is just a convient way to do that.
dmose2: why is abstracting away prefs worth doing?
dmose2: what does it buy us?
saspitzer: one sec, getting to that...
saspitzer: I don't mind if the managing service (for create new ab, delete ab, 
get ab attributes, set ab attributes...)
saspitzer: uses prefs, but here's why I want this service.
saspitzer: it can also use other things, like talk to the windows registry to 
find out stuff, right?
dmose2: i guess
saspitzer: isn't that what the sun guys are hoping to do?  make our code aware 
of outlook settings?
dmose2: i wasn't aware that they were trying to do that
saspitzer: I don't think a thin veneer on top of prefs is a bad thing, as it 
seperates the implementation (use prefs or what ever for persistent storage) 
from the architecture (give me a clean way from C++ and JS manage and get / set 
ab attributes)
saspitzer: what do you think?
dmose2: ok, i'm kinda ok with that
dmose2: but
dmose2: isn't that the whole point of contractids?
dmose2: ie shouldn't someone who wants to do what you're suggesting
dmose2: just implement another class that speaks
dmose2: @mozilla.org/preferences;1
dmose2: or whatever it's called?
saspitzer: I see your point.  I guess I'm hoping that a thin veneer will leave 
the door open, even though I don't have much in mind for what will go there.
saspitzer: clearly, DIR_Server, being heavy weight and not scriptable is not 
what we need.
saspitzer: to work around the non-scriptable-ness
saspitzer: srilatha has written a lot of js to read / write prefs directly
saspitzer: duplicated the C++, basically
dmose2: yeah, ok
dmose2: well, so here's another thing
dmose2: i don't have any big objection to the thin veneer idea
saspitzer: having a managing service would allow us to clean that up.
saspitzer: ok
dmose2: but
dmose2: i'd argue that making this code take the performance hit for DIR_Server 
now is the wrong thing
dmose2: either way we code it, there has to be a cleanup pass later
dmose2: do the efficient thing now (raw prefs reads)
dmose2: and insert the veneer later
saspitzer: so you'd say:  "read prefs now, fix it to use the service later"
dmose2: yeah
dmose2: :-)
saspitzer: ok, I'm cool with that.
saspitzer: I doubt there is really much of a performance impact, but I'm still 
ok with writing less new code, rather than more.
saspitzer: let me paste this entire conversation in the rdayal bug, ok with you?
dmose2: given how much mozilla gets knocked for performance, i'm uncomfortable 
taking steps in the wrong direction
dmose2: ok
saspitzer: you've convinced me:  if I have to leave in (reading, not writing) 
code, might as well use prefs directly and fix it when we fix the rdayal bug 
about implementing a scriptable managing service.
saspitzer: we both know that the code I check in is going to live for a while, 
might as well make it fast and less memory consuming.
dmose2: agreed; thanks
OS: Windows NT → All
Hardware: PC → All

Updated

15 years ago
QA Contact: yulian → gchan
Product: MailNews → Core
Depends on: 287832

Updated

10 years ago
Assignee: rdayal → nobody
QA Contact: grylchan → ldap-integration
(Assignee)

Updated

10 years ago
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.