On Mon, May 12, 2008 at 10:55 AM, Stefano Bagnara <[EMAIL PROTECTED]> wrote: > Robert Burrell Donkin ha scritto: > > > > i've split out most of the mailets remaining in JAMES server into > > mailet-function. a few are tightly coupled to James so i've had to > > leave them in that module. this code looks a little odd to me. > > > > org.apache.james.James uses LocalDelivery mailet to store mail > > supplied via the storeMail interface. > > > > I did this change because we had duplicated code between JAMES.storeMail > and the LocalDelivery mailet.
ok of course, reuse is a double-edged sword... > I'm not sure that it is good to have "storeMail" in the mailet api as there > is no concept of repository/users/remote delivery there I don't see why > there should be the "storeMail" concept. This is something that belongs to > the mailets, ATM. quite possibly > AFAIK MailetContext.storeMail is not used by JAMES code, the method is > deprecated and we have the "LocalDelivery" code as a backward compatibility > hack. The first non-backward compatible release could simply throw an > unsupported exception on storeMail or depends on an updated mailet-api that > will not have this method at all. yes > > LocalDelivery uses > > ToMultiRepository. MailboxDelivery uses ToMultiRepository and the > > sieve mailets. > > > > Yes, this is a refactoring from me. I split the LocalDelivery code into > UsersRepositoryAliasingForwarding and ToMultiRepository to isolate the > aliasing/forwarding logic from the persistence logic. > I did this because we agreed that the current Aliasing/Forwarding method is > a bit weird and it should be better pluggable, so I started isolating that > code. yes: it's weird and should really be pluggable > > ToMultiRepository uses synchronisation on James. i > > expect there are good reasons so i'd like to understand why something > > which seems so simple needs to be so complex. can anyone explain..? > > > > I think synchronization on James.class is an error. > The getId has been copied there from JAMES. But either we use the same > counter (and not 2 different counters) or we simply have to make the > counter++ synchronized on the ToMultiRepository instance itself and use a > different naming scheme to prevent from creating duplicated names. > I just committed a fix (r655453) for this, please review it. i've committed a variation that uses AtomicLong from backport concurrent utils > Is this enough or you better wanted to remove the opposite dependency? probably good enough for now :-) can't see an easy and elegant improvement ATM thanks for the explanation - robert --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
