james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eric Charles <e...@apache.org>
Subject Re: Improved MailboxPath and how to handle the resulting change set
Date Fri, 29 Jun 2012 06:49:11 GMT
comments inline, Eric

On 06/28/2012 11:02 PM, Jochen Gazda wrote:
> Hi Eric,
>
> see inline.
>
> Best,
>
> gazda
>
> On Thu, Jun 28, 2012 at 4:57 PM, Eric Charles<eric@apache.org>  wrote:
>> On 06/28/2012 04:00 PM, Jochen Gazda wrote:
>>>>
>>>> 1.- profile noTest on mailbox-integration-tester: ok
>>>>
>>>> 2.- LocalAndVirtualMailboxLocatorChain: what's the goal?
>>>
>>>
>>> MailDir is a file system based storage. There is the MaildirLocator
>>> interface for mapping of MailboxNames to file system directories and
>>> back. There are two basic MaildirLocator implementations:
>>> LocalSystemMaildirLocator (maps to /home/<user>/MailDir) and
>>> VirtualMailboxLocator (maps to<virtualRoot>/<domain>/<user>).
The
>>> third one, LocalAndVirtualMailboxLocatorChain, cobines the two.
>>>
>>
>> Ok, got it.
>> (naming is just not consistent across LocalSystemMaildirLocator /
>> VirtualMailboxLocator / LocalAndVirtualMailboxLocatorChain (you have Maildir
>> in the first, not in the others).
>>
>>
>>
>>>> 3.- MailboxPath is now MailboxName: Path sounded more like
>>>> folder/subfolder/subfolder.
>>>
>>>
>>> Yes, I agree, that is the case for common English. But there is no
>>> single occurence of 'path' in RFC3501. It speaks only about
>>> (hierarchical) names. On the other hand, MailboxPath/MailboxName is
>>> our internal term which does not map 1:1 to the IMAP hierachical name.
>>> We can name it however we want. I am not against going back to
>>> MailboxPath.
>>>
>>
>> The key is 'hierarchical'. I find MailboxPath correctly reflect this point.
>> With MailboxName, you loose that, and MailboxHierarchicalName is just too
>> long.
>>
>> At first sight, I would prefer to revert to MailboxPath.
>
> There are several MailboxName* classes. I will have a look if renaming
> them all to MailboxPath* sounds plausible to me.
>
>>>> 4.- MailboxNameResolver on MailboxManager
>>>
>>> You mean that the methods from MailboxNameResolver should better move
>>> to MailboxManager? - Well, MailboxManager is defines storage
>>> operations, but MailboxNameResolver interprets names. That is
>>> something different. Two distinct MailboxNameResolvers are conceivable
>>> for a single MailboxManager; see (5) in my previous post (
>>> /users/<username>    vs. /users/<username>/INBOX ).
>>>
>>
>> I was just pointing an additional method MailboxNameResolver
>> getMailboxNameResolver(); on the MailboxManager interface.
>>
>> So good as is.
>>
>>
>>>> 5.- MailboxSession has no more PersonalSpace nor UserSpace but a
>>>> MailboxOwner.
>>>
>>>
>>> The capability to list namespace prefixes has moved from
>>> MailboxSession to MailboxNameResolver, which is now a member of
>>> MailboxSession. Listing namespace prefixes belongs to the mailbox
>>> hierarchy definition entailed in MailboxNameResolver.
>>>
>>
>> OK
>>
>>
>>> MailboxOwner is needed to distinguish groups from persons and 'normal'
>>> (domain-less) users from virtual users.
>>>
>>
>> So you assume a mailbox has always an owner.
>> Sounds logical, but is it sustained by the RFC?
>
> I have done it that way without thinking of RFC. It is quite practical
> to have it like that because one can set reasonable default ACLs which
> rely on the notion of owner. Dovecot and Cyrus do that. From
> http://wiki.dovecot.org/ACL :
>
>> The default ACL for mailboxes is to give the mailbox
>> owner all permissions and other users none. Mailboxes
>> in public namespaces don't have owners, so by default
>> no-one can access them.
>
> To have owners on mailboxes is surely not against the spirit of RFC
> 4314, which names owner in one of its examples:
>
>> For example,
>> in an implementation that uses UNIX mode bits, the rights "swite" are
>> tied, the "a" right is always granted to the owner of a mailbox and
>> is never granted to another user.
>
> It would probably be possible to do without mailbox owners but it
> seems really impractical to me.
>

 From your previous explanation, it sounds like MailboxOvwner is a good 
fit for us (specifically for ACL and potentially for the rest).

>>>> 8. mmh, MailboxPath is still there.
>>>
>>>
>>> Yes, it is still there because there are still references to it from
>>> mailets project which I was not able to fix.
>>> Otherwise there are only refs in comments which can generally be
>>> replaced by MailboxName.
>>>
>>
>> Would it be solved if we revert from MailboxName to MailboxPath?
>
> Yes, perhaps.
>
>>>> 9. MailboxNameSerializer, MailboxNameBuilder, MailboxNameCodec,
>>>> MailboxNamespaceType, MailboxNameEscaper
>>>
>>>
>>> MailboxNameSerializer belongs to my first attempts. It could be made
>>> parent of MailboxNameCodec or replaced altogether. As for
>>> MailboxNameCodec cf. (6) of my previous post.
>>> MailboxNameBuilder - allows for saving some memory and string-copying
>>> when creating a new MailboxName.
>>>
>>> MailboxNameEscaper - ancillary interface for MailboxNameCodec.
>>> MailboxNameCodec implementations mostly differ only in the
>>> MailboxNameEscaper they use.
>>>
>>
>> I need to read back all this.
>> Is the goal of all these classes to better support mailbox name charsets and
>> to build the hierachical mailbox name structure in a efficient way?
>
> No, currently it is not about mailbox name charsets, though
> MailboxNameCodec could serve also that purpose.
> MailboxNameCodec (and its usual member MailboxNameEscaper) are about
> parsing raw string names like "INBOX/asf" to segment collections like
> {"INBOX", "asf"} and serializing those collections back to strings.
> Their central task (when decoding) is to recognoze segment boundaries.
> For that purpose they define the delimiter (e.g. '/') that will be
> used for the given purpose.
>

So MaiboxNameCodec has two responsibilities:
- Tokenize a 'Path' to an ordered list of 'PathSegment' (or 'PathToken')
- Build a 'Path' from an ordered list of 'PathSegment' (or 'PathToken')

But we need to take car to not over-engineer, after all, 
PathSegment/Token are just String.

Still the fact is that it didn't popup to me reading the name 
MailboxNameCodec. I also like the mantra 'one class, one 
responsibility', so having two different classes coulb be an option 
(MailboxPathTokenizer and MailboxPathBuilder). I am also fine leaving as 
such the MailboxNameCodec (or MailboxPathCodec depending on your 
decision to revert on not to MailboxPath).

>>>> 10. LikeSearchPatternEscaper: why deal with JCR, SQL in the API?
>>>
>>>
>>> It is not API, but common to many API consumers. Can you see a better
>>> place for it?
>>>
>>
>> I would prefer not to have them in the API, but we can leave it as such for
>> now.
>>
>>
>>>> 11. More and more unit tests... :)
>>>
>>>
>>> Finite verb?
>>>
>>
>> ... is good :)
>>
>>
>>>
>>> Well, yes, definitely more typing - that is con.
>>> Pros are:
>>>   - Handling mailbox names more reliably
>>>   - Mailbox names are interpreted on one central place.
>>>   - Namespace prefixes other than personal are now possible, esp. group
>>> folders are possible
>>>
>>
>> As con, you will end with a few more object to instantiate, mainly on
>> mailbox selection, but IMHO I this won't be an issue at all
>>
>>
>>>> To be honest, I didn't see well if and how the ACL are applied... :)
>>>
>>>
>>> New in my proposal is only that the ACLs can now be stored in JPA, JCR
>>> and HBase in addition to MailDir. They are still not enforced though.
>>> https://issues.apache.org/jira/browse/IMAP-358 is still open for that
>>> matter.
>>>
>>
>> OK, with these changes, you are now in a better place to implement the ACL.
>>
>>
>>>> [1]
>>>>
>>>> https://github.com/gazdahimself/current/commit/ae75a54400bc7aa93763657a48596d09ec357f98,
>>>
>>>
>>> [1] is now outdated, one can use
>>> https://github.com/gazdahimself/current/compare/master...MAILBOX-175
>>> to see the relevant diffs.
>>>
>>>
>>>>
>>>> On 06/27/2012 06:55 PM, Jochen Gazda wrote:
>>>>>
>>>>>
>>>>> Gentlemen,
>>>>>
>>>>> I have finally managed it to publish my changeset on GitHub:
>>>>> https://github.com/gazdahimself/current/tree/MAILBOX-175
>>>>> The state of my brach MAILBOX-175 is in sync with the currently latest
>>>>> svn revision 1354581. My brach MAILBOX-175 can also be diffed against
>>>>> svn revision 1354581 directly on GitHub.
>>>>>
>>>>> Sorry for the delay, I am new to git and I have a new job.
>>>>>
>>>>> Please comment.
>>>>>
>>>>> Best,
>>>>>
>>>>> gazda
>>>>>
>>>>> On Wed, Jun 13, 2012 at 1:51 PM, Ioan Eugen Stan<stan.ieugen@gmail.com>
>>>>>   wrote:
>>>>>>
>>>>>>
>>>>>> HI Gazda,
>>>>>>
>>>>>> Git is great.
>>>>>>
>>>>>> Cheers,
>>>>>>
>>>>>> 2012/6/13 Eric Charles<eric@apache.org>:
>>>>>>>
>>>>>>>
>>>>>>> Hi Gazda,
>>>>>>>
>>>>>>> I'm fine with the push to your personal github. It offers nice
ui to
>>>>>>> show
>>>>>>> diffs.
>>>>>>>
>>>>>>> Thx, Eric
>>>>>>>
>>>>>>>
>>>>>>> On 06/13/2012 10:32 AM, Jochen Gazda wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Gentlemen,
>>>>>>>>
>>>>>>>> I could invest about 6 weeks of my time into solving
>>>>>>>> https://issues.apache.org/jira/browse/MAILBOX-175 and
>>>>>>>> https://issues.apache.org/jira/browse/MAILBOX-167.
>>>>>>>> The result is quite a huge and deep changeset. There is a
de facto
>>>>>>>> replacement for MailboxPath - so you can imagine, how many
classes
>>>>>>>> were changed.
>>>>>>>>
>>>>>>>> Before going too much into details I wanted to agree on a
way how my
>>>>>>>> changeset could find its way into SVN.
>>>>>>>>
>>>>>>>> The changes should be discussed before they are committed
to trunk.
>>>>>>>> But I am not sure what is the best way to share my changes
before
>>>>>>>> they
>>>>>>>> are committed to trunk.
>>>>>>>> Would cloning from http://git.apache.org/ and pushing changes
to my
>>>>>>>> personal GitHub repo be a viable solution?
>>>>>>>> I am also ready to send my zipped workspace (~30MB) to anybody
who
>>>>>>>> wants to have a quick look.
>>>>>>>>
>>>>>>>> Best,
>>>>>>>>
>>>>>>>> gazda
>>>>>>>>
>>>>>>>> ---------------------------------------------------------------------
>>>>>>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>>>>>>>> For additional commands, e-mail: server-dev-help@james.apache.org
>>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> eric | http://about.echarles.net | @echarles
>>>>>>>
>>>>>>>
>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>>>>>>> For additional commands, e-mail: server-dev-help@james.apache.org
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> Ioan Eugen Stan / http://axemblr.com / Tools for Clouds
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>>>>>> For additional commands, e-mail: server-dev-help@james.apache.org
>>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>>>>> For additional commands, e-mail: server-dev-help@james.apache.org
>>>>>
>>>>
>>>> --
>>>> eric | http://about.echarles.net | @echarles
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>>>> For additional commands, e-mail: server-dev-help@james.apache.org
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>>> For additional commands, e-mail: server-dev-help@james.apache.org
>>>
>>
>> --
>> eric | http://about.echarles.net | @echarles
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
>> For additional commands, e-mail: server-dev-help@james.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>

-- 
eric | http://about.echarles.net | @echarles

---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message