Hi Eric..
2010/8/22 Eric Charles <e...@apache.org>: > Hi, > > So Tim, you was also ok with optin 2 and finally Norman reverted to option 1 > ? (not sure to be in line :) ?) > Seems so.. > I've looked at revision 987821: > - SubscriptionManager is not used anymore in MailboxManager hirerachy > (StoreMailboxManager,...) > - The 3 delegate methods present in DelegatingMailboxManager are removed > (so, yes, DelegatingMailboxManager is not delegating anymore, and we could > rename it :), or merge with StoreMailboxManager > - The ImapProcess is now responsible for the SubscriptionManager > > I suppose the spring-beans.xml is not committed according to these changes > (imapProcessor and mailboxmanager constructors should be updated) yeah I will update the springs-beans.xml once we know howto process. > > I like the idea to leave an interface thats clearly describe the > Subscription methods. > Is there a particular reason to have now the DefaultImapProcessorFactory > responsible for the SubscriptionManager ? > I can see it's // with the MailboxManager. > NioImapServer finally uses the imapProcessor bean. We need the Subscription stuff in some Processors, thats why we need it here. > > btw, if we talk Gof patterns, we went from a "Strategy" pattern to a "Chain > of responsibility" pattern. > So the question I'm wondering is "is it the responsibility of the the > NioImapServer to process the mailbox, then the subscription" (it does it via > the DefaultProcessorChain). > I remain in favour of "strategy or chain", rather then inheritance. > > Tim, are the mapper considerations your're talking here related to IMAP-206 > ? (not sure to understand the relations, maybe a separate thread would help) > > Tks, > > Eric > > > On 21/08/2010 22:53, Tim-Christian Mundt wrote: >> >> Norman, >> >> I have not yet reviewed your recent commit. However, I'd like to remark >> that I was in favor of your last attempt. I just wanted to >> _additionally_: >> >>>>>> I think we should also merge them which >>>>>> would also simplify the package structure because we wouldn't need >>>>>> the .mail and .user packages anymore. >> >> Which means, not only merge the SubscriptionManager into the >> MailboxManager like you did, but also the SubscriptionMapper into the >> MailboxMapper. That would make the separation between .user and .mail >> superfluous. >> >> Does that make sense? Maybe your current attempt is still better, I >> haven't checked yet. >> >> Best >> Tim >> >> Am Samstag, den 21.08.2010, 22:08 +0200 schrieb Norman Maurer: >>> >>> Ok another attempt was made.. please review changes made in revision >>> 987821 >>> >>> Thx, >>> Norman >>> >>> 2010/8/21 Norman Maurer<nor...@apache.org>: >>>> >>>> Well I need to revert it ;) I will do so then.. >>>> >>>> Bye, >>>> Norman >>>> >>>> >>>> 2010/8/21 Norman Maurer<nor...@apache.org>: >>>>> >>>>> Hi Tim, >>>>> >>>>> comments inside.. >>>>> >>>>> 2010/8/21 Tim-Christian Mundt<d...@tim-erwin.de>: >>>>>> >>>>>> Norman, >>>>>> >>>>>> you are right in that it was kinda double, so there should be some >>>>>> cleanup. My first attempt would have been to remove the subscription >>>>>> stuff from the MailboxManager (your option #1). The reason is that we >>>>>> always have a manager and its respective mapper. Now we have the >>>>>> MailboxManager with two Mappers. I think we should also merge them >>>>>> which >>>>>> would also simplify the package structure because we wouldn't need >>>>>> the .mail and .user packages anymore. >>>>> >>>>> Thats true I just thought it would be more easy to have not to many >>>>> interfaces to implement. Anyway I would also be happy to move the >>>>> subscripe stuff to any extra interface. I just don't like to have it >>>>> duplicated so feel free to revert... >>>>> >>>>>> One more thing concerning naming and stuff: Now the >>>>>> DelegatingMailboxManager is not really delegating anymore. Is there >>>>>> any >>>>>> good reason we should keep it separate from StoreMailboxManager? If >>>>>> not >>>>>> I'd rather have a little bigger class but fewer hierarchy levels. >>>>> >>>>> I need to review.. >>>>> >>>>>> Any thoughts? >>>>>> >>>>>> Tim >>>>>> >>>>>> Am Samstag, den 21.08.2010, 10:23 +0200 schrieb Norman Maurer: >>>>>>> >>>>>>> I just committed the changes.. If anyone thinks its a bad idea we can >>>>>>> revert it anyway.. >>>>>>> >>>>>>> https://issues.apache.org/jira/browse/IMAP-205 >>>>>>> >>>>>>> Bye, >>>>>>> Norman >>>>>>> >>>>>>> 2010/8/21 Norman Maurer<nor...@apache.org>: >>>>>>>> >>>>>>>> Hi there, >>>>>>>> >>>>>>>> after looking again at the IMAP api I'm in favor of removing the >>>>>>>> org.apache.james.imap.store.Subscriper interface and merge the >>>>>>>> implementations with the MailboxManager implementations. Thats >>>>>>>> because >>>>>>>> the Subsciper interface has 3 methods, all of the methods are >>>>>>>> already >>>>>>>> in MailboxManager. So the MailboxManager just wraps the Subscriper >>>>>>>> implementation and delegate the call to it. >>>>>>>> >>>>>>>> So there are two solutions to this: >>>>>>>> >>>>>>>> 1) Remove the methods from MailboxManager and move the Subscriper >>>>>>>> interface to the mailbox api >>>>>>>> 2) Remove the Subscriper interface from store api and merge the >>>>>>>> implementations >>>>>>>> >>>>>>>> As I stated before I would prefer 2). >>>>>>>> >>>>>>>> WDYT ? >>>>>>>> >>>>>>>> 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 >>>>>> >>>>>> >>>>> Bye, >>>>> Norman >>>>> Bye, Norman --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
