A couple of goodies

[Bron Gondwana]

Sorry about not getting back to reply to the original of this.
I still want to spend some polishing time on these patches.

On Fri, Aug 10, 2007 at 04:51:13PM +0100, David Carter wrote:
> On Fri, 3 Aug 2007, Ken Murchison wrote:
>
>> I applied Bron's patch, with a few small tweaks (hierachy -> hierarchy,
>
> Oops, sorry: my fault.

Yep, blame David :)  I also didn't pick up on it.

>> changed some of the option caching) to my sandbox.
>
> Is your cyr_expire "-D" option keyed on file suffix or a stat() against the 
> cyrus.header file? I guess that file suffix will be faster, but stat() may 
> be more reliable if the naming conventions change at a later date.

I'm happy with either.  Stat was necessary in my original plan, but then
it was too hard to get my hands on uniqueid at the point I wanted, so I
went back to using timestamps.  Ahh, layering.

>> A couple of thoughts before I commit this code:
>
>> - Should we actually have the LIST code explicitly suppress DELETED, or 
>> should we augment the ACLs of DELETED mailboxes to include "-anyone l", 
>> which will hide them from non-admins?
>
> This would certainly be a more natural Cyrus way of doing things if it was 
> easy. I was aiming for the simplest patch that I could get away with.

Yeah, fiddling the ACLs is looking like an option, isn't it.  Only
difficult being knowing for sure what to restore.

>> - Should DELETED be its own namespace?
>
> The only people who can see DELETED are admins, who automatically get the 
> internal namespace. Is there any advantage in adding another namespace?

No opinion here.

>> - Should the "delete_hierarchy" option be changed to "deleteprefix" to 
>> match up with "userprefix" and "sharedprefix"?
>
> If you were going to add another namespace, then definitely yes.
>
> As it stands the argument would seem to be consistency with existing names 
> used for other purposes. We have a number of *prefix names, but no 
> *_hierarchy names, so I guess my vote would be for deleteprefix.

Sounds reasonable to me, though I'd add the proviso that userprefix at
least is broken, and indeed I had to fix one spot where "user." is hard
coded, which is a pain because it also breaks unixhierarchysep.  There
are so many intersecting options which fiddle how the naming structure
works that I actually think the only way to have any confidence that
they are right is to build a decent path-coverage test suite and then go
run it with ALL the different combinations of config flags.

In particular here, I'd like to see a bunch of variable renames (maybe
not going as far as http://www.joelonsoftware.com/articles/Wrong.html
and apps hungarian, but certainly in that style - so just looking at a
string containing a "mailbox name" you can see what namespace that's in,
and know you need to call the converter function if you need to switch
between internal and external representation.

I know I found some assumptions that the partition "default" existed in
the sync code like that, back when we split our servers into two
partitions called "imap1" and "imap2".  Thankfully sanity prevailed and
we changed to smaller instances with a single partition - but still,
we run virtdomains and splitmeta, so we run into many cases where
suggested patches don't work properly on our config thanks to not taking
in to account the effects of these two options.

Bron.