Re: [jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!

2005-09-12 Thread Stefano Bagnara
> > IMHO the DEFAULT behaviour should be the deep copy.
> 
> I disagree, what we're trying to do is to manage routing the message.
> 
> Message can be routed down two different routes to its 
> destination, but is still the same message.

-1 on that!
James developers already did mistakes due to mimemessage shared and this bug
report is the proof.
Performance can be achieved with copyonwrite proxies or different approach
but we should avoid that mailet writer run a mail.duplicate() and without
the knowledge that it will share the mimemessage.
NOONE will ever understand that running a matcher in a processor will
duplicate the envelope but will share the mimemessage so if you change one
of the 2 mails you don't know wether you will receive 2 identical messages
or 2 different ones!

> The default should be the least resource intensive.

The default should be the SAFEST. The unsafe optimization should be applied
understanding possible problems of the optimization.
As I already said we can achieve optimization later or in a different way
but first we should ensure that message handling is consistent and SAFE.

> In fact I would tend to say that the issue is not technical, 
> but one of documentation. The docs should clearly explain 
> that and any changes should be made only to ensure that there 
> is a simple and consistent method which can be used to branch 
> the message itself.

Please reread the bug report and not only the subject.
The issue is REAL and is a critical BUG: if you think that MimeMessage
sharing is good then it is a bug in the LinearProcessor.
I don't care wether the message is shared or not, but the scenario I
submitted should not give that result.
The developer that wrote the LinearProcessor did not consider that the
MailImpl.duplicate was returning a new Mail with the same MimeMessage.

The developer that wrote the AbstractRedirect did the bugfree thing (clone
the mimemessage).

This is the "easy" code that should be documented to do the right thing:

newMail.setMessage(new MimeMessage(originalMail.getMessage()) {
protected void updateHeaders() throws MessagingException
{
if (getMessageID() == null) super.updateHeaders();
else {
modified = false;
}
}
}); 


IMHO adding a copyonwrite proxy over the MimeMessageWrapper we can remove
also the deepcopy in the abstractRedirect and achieve better performance
than now, btw I don't care how we decide to fix it. I just want it to be
fixed.

Stefano


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: [jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!

2005-09-12 Thread Danny Angus
> IMHO the DEFAULT behaviour should be the deep copy.

I disagree, what we're trying to do is to manage routing the message.

Message can be routed down two different routes to its destination, but is
still the same message.

The default should be the least resource intensive.

In fact I would tend to say that the issue is not technical, but one of
documentation. The docs should clearly explain that and any changes should
be made only to ensure that there is a simple and consistent method which
can be used to branch the message itself.

d.




***
The information in this e-mail is confidential and for use by the addressee(s) 
only. If you are not the intended recipient (or responsible for delivery of the 
message to the intended recipient) please notify us immediately on 0141 306 
2050 and delete the message from your computer. You may not copy or forward it 
or use or disclose its contents to any other person. As Internet communications 
are capable of data corruption Student Loans Company Limited does not accept 
any  responsibility for changes made to this message after it was sent. For 
this reason it may be inappropriate to rely on advice or opinions contained in 
an e-mail without obtaining written confirmation of it. Neither Student Loans 
Company Limited or the sender accepts any liability or responsibility for 
viruses as it is your responsibility to scan attachments (if any). Opinions and 
views expressed in this e-mail are those of the sender and may not reflect the 
opinions and views of The Student Loans Company Limit
 ed.

This footnote also confirms that this email message has been swept for the 
presence of computer viruses.

**

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: [jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!

2005-09-09 Thread Stefano Bagnara
> This is the usual deep versus shallow copy discussion, and 
> actually both methods make sense for different purposes. 
> I think we should provide a deep-duplicate or such method, as 
> there might be someone out there, who has coded against the 
> shallow copy behaviour.

IMHO the DEFAULT behaviour should be the deep copy.
We can provide a method that share the MimeMessage but the user should
understand what he's doing.

Currently also james core developers used it the wrong way because it's too
easy to do the wrong thing with the current behaviour.
Mailet writers can do the wrong thing too easily.

Stefano


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Re: [jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!

2005-09-09 Thread Soren Hilmer
This is the usual deep versus shallow copy discussion, and actually both 
methods make sense for different purposes. 
I think we should provide a deep-duplicate or such method, as there might be 
someone out there, who has coded against the shallow copy behaviour.

--Søren

On Friday 09 September 2005 09:45, Stefano Bagnara (JIRA) wrote:
> [
> http://issues.apache.org/jira/browse/JAMES-421?page=comments#action_1232300
>6 ]
>
> Stefano Bagnara commented on JAMES-421:
> ---
>
> Here is what it happens in my test:
> I send a mail "mail1" to [EMAIL PROTECTED] and [EMAIL PROTECTED] and "original
> body" body. The first matcher split the mail1 by duplicating it to
> "mail1-!27120": it then remove a recipient from mail1 and the other from
> mail1-!27120. mail1 run to the fist MyMailet that change its body to new
> text 0. Both Mails run then through the second MyMailet that should change
> the body of one mail to "new text 1" and the body of the other mail to "new
> text 2". I then print the 2 bodies: I have 2 messages with "new text 2"
> body and NO ONE with "new text 1".
>
> - AbstractRedirect does correctly clone the MimeMessage after the call to
> MailImpl.duplicate - Many mailets just use sendMail with the shared
> MimeMessage: we should add a comment to mailetContext sendMails to let the
> use know that we lock the MimeMessage until we processed it and we never
> change it. - LinearProcessor (after partial matching) does duplicate and
> this way sends multiple Mails with sharing MimeMessage to the following
> mailets.
>
> We could solve the issue by cloning the MimeMessage in LinearProcessor but
> this is too easy to exploit: IMHO we should change the duplicate so that we
> wrap the object with a "CopyOnWrite" shield: so we can safely share the
> MimeMessage also when storing it and after multiple operations.
>
> > MailImpls sharing MimeMessage's!
> > 
> >
> >  Key: JAMES-421
> >  URL: http://issues.apache.org/jira/browse/JAMES-421
> >  Project: James
> > Type: Bug
> >   Components: James Core
> > Versions: 2.3.0, 2.2.0
> > Reporter: Stefano Bagnara
> > Assignee: Stefano Bagnara
> > Priority: Critical
> >  Fix For: 2.3.0
> >  Attachments: LinearProcessorTest.java
> >
> > LinearProcessor match a single recipient for a 2 recipient mail.
> > it run "MailImpl.duplicate". duplicate DOES NOT clone the "MimeMessage".
> > The following mailet will handle 2 different MailImpl sharing the same
> > MimeMessage. Attached is the proving test.

-- 
Søren Hilmer, M.Sc.
R&D manager Phone:  +45 72 30 64 00
TietoEnator IT+ A/S Fax:+45 72 30 64 40
Ved Lunden 12   Direct: +45 72 30 64 57
DK-8230 Åbyhøj  Email:  soren.hilmer  tietoenator.com

-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



[jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!

2005-09-09 Thread Stefano Bagnara (JIRA)
[ 
http://issues.apache.org/jira/browse/JAMES-421?page=comments#action_12323006 ] 

Stefano Bagnara commented on JAMES-421:
---

Here is what it happens in my test:
I send a mail "mail1" to [EMAIL PROTECTED] and [EMAIL PROTECTED] and "original 
body" body.
The first matcher split the mail1 by duplicating it to "mail1-!27120": it then 
remove a recipient from mail1 and the other from mail1-!27120.
mail1 run to the fist MyMailet that change its body to new text 0.
Both Mails run then through the second MyMailet that should change the body of 
one mail to "new text 1" and the body of the other mail to "new text 2".
I then print the 2 bodies: I have 2 messages with "new text 2" body and NO ONE 
with "new text 1".

- AbstractRedirect does correctly clone the MimeMessage after the call to 
MailImpl.duplicate
- Many mailets just use sendMail with the shared MimeMessage: we should add a 
comment to mailetContext sendMails to let the use know that we lock the 
MimeMessage until we processed it and we never change it.
- LinearProcessor (after partial matching) does duplicate and this way sends 
multiple Mails with sharing MimeMessage to the following mailets.

We could solve the issue by cloning the MimeMessage in LinearProcessor but this 
is too easy to exploit: IMHO we should change the duplicate so that we wrap the 
object with a "CopyOnWrite" shield: so we can safely share the MimeMessage also 
when storing it and after multiple operations.



> MailImpls sharing MimeMessage's!
> 
>
>  Key: JAMES-421
>  URL: http://issues.apache.org/jira/browse/JAMES-421
>  Project: James
> Type: Bug
>   Components: James Core
> Versions: 2.3.0, 2.2.0
> Reporter: Stefano Bagnara
> Assignee: Stefano Bagnara
> Priority: Critical
>  Fix For: 2.3.0
>  Attachments: LinearProcessorTest.java
>
> LinearProcessor match a single recipient for a 2 recipient mail.
> it run "MailImpl.duplicate". duplicate DOES NOT clone the "MimeMessage".
> The following mailet will handle 2 different MailImpl sharing the same 
> MimeMessage.
> Attached is the proving test.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



[jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!

2005-09-09 Thread Stefano Bagnara (JIRA)
[ 
http://issues.apache.org/jira/browse/JAMES-421?page=comments#action_12323003 ] 

Stefano Bagnara commented on JAMES-421:
---

It doesn't matter what Mail is supposed to do: if this is not a bug in MailImpl 
it is a bug in LinearProcessor!
And a critical one: James send a different mail from what it's supposed to send.

> MailImpls sharing MimeMessage's!
> 
>
>  Key: JAMES-421
>  URL: http://issues.apache.org/jira/browse/JAMES-421
>  Project: James
> Type: Bug
>   Components: James Core
> Versions: 2.3.0, 2.2.0
> Reporter: Stefano Bagnara
> Assignee: Stefano Bagnara
> Priority: Critical
>  Fix For: 2.3.0
>  Attachments: LinearProcessorTest.java
>
> LinearProcessor match a single recipient for a 2 recipient mail.
> it run "MailImpl.duplicate". duplicate DOES NOT clone the "MimeMessage".
> The following mailet will handle 2 different MailImpl sharing the same 
> MimeMessage.
> Attached is the proving test.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



[jira] Commented: (JAMES-421) MailImpls sharing MimeMessage's!

2005-09-08 Thread Noel J. Bergman (JIRA)
[ 
http://issues.apache.org/jira/browse/JAMES-421?page=comments#action_12322983 ] 

Noel J. Bergman commented on JAMES-421:
---

Mail objects are just addressing carriers for the message.  They are not 
supposed to clone the message, since we may simply be effecting routing within 
the pipeline.  The overhead would be huge if every split in routing caused a 
clone of the underlying message.

> MailImpls sharing MimeMessage's!
> 
>
>  Key: JAMES-421
>  URL: http://issues.apache.org/jira/browse/JAMES-421
>  Project: James
> Type: Bug
>   Components: James Core
> Versions: 2.3.0, 2.2.0
> Reporter: Stefano Bagnara
> Assignee: Stefano Bagnara
> Priority: Critical
>  Fix For: 2.3.0
>  Attachments: LinearProcessorTest.java
>
> LinearProcessor match a single recipient for a 2 recipient mail.
> it run "MailImpl.duplicate". duplicate DOES NOT clone the "MimeMessage".
> The following mailet will handle 2 different MailImpl sharing the same 
> MimeMessage.
> Attached is the proving test.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]