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<[email protected]> 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<[email protected]>
wrote:
HI Gazda,
Git is great.
Cheers,
2012/6/13 Eric Charles<[email protected]>:
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: [email protected]
For additional commands, e-mail: [email protected]
--
eric | http://about.echarles.net | @echarles
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
--
Ioan Eugen Stan / http://axemblr.com / Tools for Clouds
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
--
eric | http://about.echarles.net | @echarles
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
--
eric | http://about.echarles.net | @echarles
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
--
eric | http://about.echarles.net | @echarles
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
