Hi All,
Comments inside.
Tks,
Eric
On 08/20/2010 04:53 PM, Tim-Christian Mundt wrote:
Hi guys,
I began looking into the API stuff. I'd like to simplify the Mapper
hierarchy, which however will have no direct impact on the API. Will
upload a patch tomorrow.
Will look at it :)
I agree, we should name the interface MessageManager, that's what I
thought.
+1
The "proposal_v3" is reasonable, I think. To make it clearer, we should
maybe call "AbstractStoreMessageManager" "StoreMessageManager" and
qualify "StoreMessageManager" to "MapperStoreMessageManager". I think,
that's what Eric also proposed.
Sounds good
(loosely proposal : MappingStoreMessageManager instead of
MapperStoreMessageManager :)
Concerning composition: I think the composition part in the Store stuff
is the mappers which get plugged into the Managers. The only reason so
far for extending Store*Manager is the creation of certain objects.
Nonetheless, you are right, Eric, there are quite extensive hierarchies
in James and maybe we can simplify them. However, inheritance here is
not used to change behavior but to model different stages of abstraction
which is not the matter of composition. Where/How do you think we should
introduce composition?
I'm talking about composition in the sense of the Gof "Strategy" pattern.
I think we can say that "James has different store strategies".
I also can be in line with you when you say the behaviour is not really
changed (we always talk about "storing" stuff).
However, fine-grained behaviour is quite different depending on the
store (jpa/jcr/file/nosql such as cassandra).
Another argument for composition in that underlying api can change
without disturbing the existing 3rd party store impl (think what will
happen if you rename a method in the abstract class). The enormeous
advantage of composition is that the interface of the composed class is
the published contract and should not change (between minor releases).
So you can change anything in the abstract without breaking 3rd party
implementations.
But I'd better take my IDE and to create a patch with a composed API....
However, I've got a number of waiting JIRAs, and if you don't see
anything from me soon, let's not wait to implement this refactoring in
the inheritance way.
We can also come on this later on if appropriate.
Regards
Tim
Am Freitag, den 20.08.2010, 14:17 +0200 schrieb Eric Charles:
Hi Norman,
The following hierarchy is obvious and gives services such as create,
login, logout, search on *mailboxes*:
MailboxManager (interface)
/|\
|
DelegatingMailboxManagerimpl (abstract)
/|\
|
StoreMailboxManager (abstract)
/|\
|
JPA/JCR/Maildir/...MailboxManager (concrete)
This one gives services to append, set flags', expunge on *messages*:
Mailbox (interface)
/|\
|
AbstractStoreMessageManager<Id> (abstract)
/|\
|
StoreMessageManager<Id> (abstract)
/|\
|
JPA/JCR/Maildir/...MessageManager (concrete)
Wouldn't that Mailbox interface finally be named MessageManager ?
But np for me to live with the existing names.
Will StoreMessageManager remain abstract (it is in the patch) ? (if yes,
we will end up with a AbstractStoreMessageManager and a
StoreMessageManager, both abstract,...).
I suppose the criteria to implement in AbstractStoreMessageManager was
'no need for messageMapper'. Is there a common characteristic to these
methods (so we can QualifyStoreMessageManager) ?
If I understand well, store implementors could reuse or simply override
methods of *StoreMessageManager.
I was also talking about composition, so a store implementator would
know directly which methods he should implement to take control on a
certain behaviour aspect.
With the inheritance mecanism, the implementator has to go "deep" in the
class hierarchy to know it.
Would you rename Document to Message in IMAP-202 ?
(btw, in my first reply, I confused somewhere ...Mapper with ...Manager,
sorry)
Tks,
Eric
On 20/08/2010 12:07, Norman Maurer wrote:
Comments inline..
2010/8/20 Eric Charles<e...@apache.org>:
OK
So AbstractStoreMessageManager doesn't implement MessageMapper (and
it was
in your javadoc :)
... and implements Mailbox (the
org.apache.james.imap.mailbox.Mailbox, not
the org.apache.james.imap.store.mail.model.Mailbox...)
yep ..
The org.apache.james.imap.mailbox.Mailbox has much too do with
MaiboxSession
and not with domain classes, so it respects your goal.
Maybe the domain independence could be also documented in the javadoc.
Fair enough..
Talking about domain model, I'm sometimes a bit confused. We have a
nice
package org.apache.james.imap.store.mail.model that contains the
model, but
my 2-cents:
- There are a few utils classes (PropertyBuilder,...) that could be
moved
somewhere else.
I see no value here..
- Document could be renamed to Message (I think we talked about it
some time
ago)
+1
- When you read code, Mailbox,... is sometimes confusing because it
exists
somewhere else, for example in
org.apache.james.imap.mailbox.Mailbox. This
last one has more to do with "service" than with "domain". I don't
have a
good name right now, but Maiboxable seems the closer to what I'm
looking
for.
Thats why its still called Mailbox ;)
Hope I'm not too niggling,
Tks,
Eric
On 20/08/2010 07:36, Norman Maurer wrote:
Let me try to explain it....
The idea is to let the MessageMapper untouched, because its API is
really easy to understand and use. Its not the most Performant todo
but thats the price to pay.
So I introduced a new abstract class which can be used if you want to
write a custom store which is not forced to use a MessageMapper
and so
is a good fit for other implementations which not works so well with
the Domain model and lazy loading (nosql for example).
Hope this explains it...
Thx
Norman
2010/8/20, Eric Charles<e...@apache.org>:
Hi Norman,
I applied the last uploaded file (proposal_v3.diff):
- patch works, which is good :)
- I don't see any change on MessageMapper. Probably the modif to
return
only uid was already committed (IMAP-203,...) ?
- You migrated methods from StoreMessageManager to
AbstractMessageMapper. So now, I'm a bit confused about the class
"responsiblity". Could you summary us the
"roles/responsibilities" of
these 2 classes. That will help to define which methods goes
where...
- Abstract/extend is widely used in james. I sometimes force
myself to
thin to "favor composition over inheritance". Did you consider
composition instead of AbstractMessageMapper ? (maybe stupid
question
and I should have tried to implement before asking... :) Of
course, you
can find plenty of discussions and explanations
(http://www.google.com/search?q=favor+composition+over+inheritance)
on
the net
- I like the new UpdatedFlag class. When you read it, you understand
what it is aimed for. Separating "data domain" from "service
domain" is
always good.
Tks,
Eric
On 08/19/2010 12:20 PM, Norman Maurer wrote:
Hi there,
I' currently looking for ways to make it easier to create high
performant Mailbox implementations while reusing the store api. I
think there are some things we should change to allow this.
* We should return only the uid of the message if nothing more
is needed
* We should move some stuff to an abstract MessageMapper
implementation to make it easier to override and handle it
different
and still be able to use the store api.
For this I opened a jira and attached a patch whic hshows the
idea..
I'm still not 100% sure if its really a good idea, so feedback is
welcome :)
https://issues.apache.org/jira/browse/IMAP-202
Bye,
Norman
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
Bye,
Norman
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org
For additional commands, e-mail: server-dev-h...@james.apache.org
