You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2010/10/01 19:59:14 UTC
svn commit: r1003596 -
/ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
Author: jleroux
Date: Fri Oct 1 17:59:13 2010
New Revision: 1003596
URL: http://svn.apache.org/viewvc?rev=1003596&view=rev
Log:
Excerpt from a BJ's patch at "Message: In field [headerString] less-than (<) and greater-than (>) symbols are not allowed.In field [messageId] less-than (<) and greater-than (>) symbols are not allowed." (https://issues.apache.org/jira/browse/OFBIZ-2544) - OFBIZ-2544
I kept only the replacing lines (there were conflicts)
Modified:
ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
Modified: ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java?rev=1003596&r1=1003595&r2=1003596&view=diff
==============================================================================
--- ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java (original)
+++ ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java Fri Oct 1 17:59:13 2010
@@ -668,7 +668,7 @@ public class CommunicationEventServices
Address[] addressesTo = wrapper.getTo();
Address[] addressesCC = wrapper.getCc();
Address[] addressesBCC = wrapper.getBcc();
- String messageId = wrapper.getMessageId();
+ String messageId = wrapper.getMessageId().replaceAll("<|>", "");;
String aboutThisEmail = "message [" + messageId + "] from [" +
(addressesFrom[0] == null? "not found" : addressesFrom[0].toString()) + "] to [" +
@@ -794,7 +794,7 @@ public class CommunicationEventServices
if (inReplyTo != null && inReplyTo[0] != null) {
GenericValue parentCommEvent = null;
try {
- List<GenericValue> events = delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId", inReplyTo[0]));
+ List<GenericValue> events = delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId", inReplyTo[0].replaceAll("<|>", "")));
parentCommEvent = EntityUtil.getFirst(events);
} catch (GenericEntityException e) {
Debug.logError(e, module);
@@ -854,7 +854,8 @@ public class CommunicationEventServices
headerString.append(System.getProperty("line.separator"));
headerString.append(headerLines.nextElement());
}
- commEventMap.put("headerString", headerString.toString());
+ String header = headerString.toString();
+ commEventMap.put("headerString", header.replaceAll("<|>", ""));
result = dispatcher.runSync("createCommunicationEvent", commEventMap);
communicationEventId = (String)result.get("communicationEventId");
Re: svn commit: r1003596 - /ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
Posted by BJ Freeman <bj...@free-man.net>.
regex has to compile then do a lot of segmenting.
were a single char compare is a singl2 segment twice.
it would seem that a walkthrough twice is less machine code steps than a
compile and multiple segmentation.
I was just looking a the code in the java.util.pattern.java
but I could have not read it right.
Adam Heath sent the following on 10/1/2010 12:27 PM:
> On 10/01/2010 02:07 PM, Jacques Le Roux wrote:
>> From: "Adam Heath" <do...@brainfood.com>
>>> On 10/01/2010 12:59 PM, jleroux@apache.org wrote:
>>>> Author: jleroux
>>>> Date: Fri Oct 1 17:59:13 2010
>>>> New Revision: 1003596
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1003596&view=rev
>>>> Log:
>>>> Excerpt from a BJ's patch at "Message: In field [headerString]
>>>> less-than (<) and greater-than (>) symbols are not allowed.In field
>>>> [messageId] less-than (<) and greater-than (>) symbols are not
>>>> allowed." (https://issues.apache.org/jira/browse/OFBIZ-2544) -
>>>> OFBIZ-2544
>>>>
>>>> I kept only the replacing lines (there were conflicts)
>>>>
>>>> Modified:
>>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>>>
>>>>
>>>>
>>>> Modified:
>>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>>>
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java?rev=1003596&r1=1003595&r2=1003596&view=diff
>>>>
>>>>
>>>> ==============================================================================
>>>>
>>>>
>>>> ---
>>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>>>
>>>> (original)
>>>> +++
>>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>>>
>>>> Fri Oct 1 17:59:13 2010
>>>> @@ -668,7 +668,7 @@ public class CommunicationEventServices
>>>> Address[] addressesTo = wrapper.getTo();
>>>> Address[] addressesCC = wrapper.getCc();
>>>> Address[] addressesBCC = wrapper.getBcc();
>>>> - String messageId = wrapper.getMessageId();
>>>> + String messageId = wrapper.getMessageId().replaceAll("<|>", "");;
>>>>
>>>> String aboutThisEmail = "message [" + messageId + "] from [" +
>>>> (addressesFrom[0] == null? "not found" : addressesFrom[0].toString())
>>>> + "] to [" +
>>>> @@ -794,7 +794,7 @@ public class CommunicationEventServices
>>>> if (inReplyTo != null&& inReplyTo[0] != null) {
>>>> GenericValue parentCommEvent = null;
>>>> try {
>>>> - List<GenericValue> events =
>>>> delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId",
>>>> inReplyTo[0]));
>>>> + List<GenericValue> events =
>>>> delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId",
>>>> inReplyTo[0].replaceAll("<|>", "")));
>>>> parentCommEvent = EntityUtil.getFirst(events);
>>>> } catch (GenericEntityException e) {
>>>> Debug.logError(e, module);
>>>> @@ -854,7 +854,8 @@ public class CommunicationEventServices
>>>> headerString.append(System.getProperty("line.separator"));
>>>> headerString.append(headerLines.nextElement());
>>>> }
>>>> - commEventMap.put("headerString", headerString.toString());
>>>> + String header = headerString.toString();
>>>> + commEventMap.put("headerString", header.replaceAll("<|>", ""));
>>>
>>> I think that [<>] would be faster.
>>
>> Why? since it's only one character and we need to try to replace both,
>> it will be alternation in both cases (Alternation or Character Classes)
>> in the end, isn'it ?
>>
>> Jacques
>
> Because the '|' /a|b/, with no other explicit bounds applied, means to
> scan the entire string twice. Once to see if there is an 'a', then start
> over at the beginning to find the 'b'. Once it finds it, it repeats
> is(as that what replaceAll does).
>
> But a character class /[ab]/ has no unlimit bounds constraint, and the
> internal matcher can walk each character one at a time, instead of
> scanning the string from the beginning again.
>
> /a|b/ functions like this. At the start of the, when attempting to match
> the group /a|b/, the position in the string is remembered. This is 0.
> Then, the first alternation is tried; in this case, 'a'. A full string
> scan is performed, until the first one is found. If so, it is removed,
> and you start over completely.
>
> If 'a' is not found, however, the position remembered at the beginning
> of the alternation group is returned to, and the second alternation is
> tried. And so on for any such alternations.
>
> This is how alternation is defined to work.
>
>
>>
>
>
Re: svn commit: r1003596 - /ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "Adam Heath" <do...@brainfood.com>
> On 10/01/2010 02:07 PM, Jacques Le Roux wrote:
>> From: "Adam Heath" <do...@brainfood.com>
>>> On 10/01/2010 12:59 PM, jleroux@apache.org wrote:
>>>> Author: jleroux
>>>> Date: Fri Oct 1 17:59:13 2010
>>>> New Revision: 1003596
>>>>
>>>> URL: http://svn.apache.org/viewvc?rev=1003596&view=rev
>>>> Log:
>>>> Excerpt from a BJ's patch at "Message: In field [headerString]
>>>> less-than (<) and greater-than (>) symbols are not allowed.In field
>>>> [messageId] less-than (<) and greater-than (>) symbols are not
>>>> allowed." (https://issues.apache.org/jira/browse/OFBIZ-2544) -
>>>> OFBIZ-2544
>>>>
>>>> I kept only the replacing lines (there were conflicts)
>>>>
>>>> Modified:
>>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>>>
>>>>
>>>> Modified:
>>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>>>
>>>> URL:
>>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java?rev=1003596&r1=1003595&r2=1003596&view=diff
>>>>
>>>> ==============================================================================
>>>>
>>>> ---
>>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>>> (original)
>>>> +++
>>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>>> Fri Oct 1 17:59:13 2010
>>>> @@ -668,7 +668,7 @@ public class CommunicationEventServices
>>>> Address[] addressesTo = wrapper.getTo();
>>>> Address[] addressesCC = wrapper.getCc();
>>>> Address[] addressesBCC = wrapper.getBcc();
>>>> - String messageId = wrapper.getMessageId();
>>>> + String messageId = wrapper.getMessageId().replaceAll("<|>", "");;
>>>>
>>>> String aboutThisEmail = "message [" + messageId + "] from [" +
>>>> (addressesFrom[0] == null? "not found" : addressesFrom[0].toString())
>>>> + "] to [" +
>>>> @@ -794,7 +794,7 @@ public class CommunicationEventServices
>>>> if (inReplyTo != null&& inReplyTo[0] != null) {
>>>> GenericValue parentCommEvent = null;
>>>> try {
>>>> - List<GenericValue> events =
>>>> delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId",
>>>> inReplyTo[0]));
>>>> + List<GenericValue> events =
>>>> delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId",
>>>> inReplyTo[0].replaceAll("<|>", "")));
>>>> parentCommEvent = EntityUtil.getFirst(events);
>>>> } catch (GenericEntityException e) {
>>>> Debug.logError(e, module);
>>>> @@ -854,7 +854,8 @@ public class CommunicationEventServices
>>>> headerString.append(System.getProperty("line.separator"));
>>>> headerString.append(headerLines.nextElement());
>>>> }
>>>> - commEventMap.put("headerString", headerString.toString());
>>>> + String header = headerString.toString();
>>>> + commEventMap.put("headerString", header.replaceAll("<|>", ""));
>>>
>>> I think that [<>] would be faster.
>>
>> Why? since it's only one character and we need to try to replace both,
>> it will be alternation in both cases (Alternation or Character Classes)
>> in the end, isn'it ?
>>
>> Jacques
>
> Because the '|' /a|b/, with no other explicit bounds applied, means to scan the entire string twice. Once to see if there is an
> 'a', then start over at the beginning to find the 'b'. Once it finds it, it repeats is(as that what replaceAll does).
>
> But a character class /[ab]/ has no unlimit bounds constraint, and the internal matcher can walk each character one at a time,
> instead of scanning the string from the beginning again.
>
> /a|b/ functions like this. At the start of the, when attempting to match the group /a|b/, the position in the string is
> remembered. This is 0. Then, the first alternation is tried; in this case, 'a'. A full string scan is performed, until the
> first one is found. If so, it is removed, and you start over completely.
>
> If 'a' is not found, however, the position remembered at the beginning of the alternation group is returned to, and the second
> alternation is tried. And so on for any such alternations.
>
> This is how alternation is defined to work.
Interesting, then I agree it's worth the change indeed. I did it at r1003655
Jacques
>
>>
>
Re: svn commit: r1003596 - /ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
Posted by Adam Heath <do...@brainfood.com>.
On 10/01/2010 02:07 PM, Jacques Le Roux wrote:
> From: "Adam Heath" <do...@brainfood.com>
>> On 10/01/2010 12:59 PM, jleroux@apache.org wrote:
>>> Author: jleroux
>>> Date: Fri Oct 1 17:59:13 2010
>>> New Revision: 1003596
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1003596&view=rev
>>> Log:
>>> Excerpt from a BJ's patch at "Message: In field [headerString]
>>> less-than (<) and greater-than (>) symbols are not allowed.In field
>>> [messageId] less-than (<) and greater-than (>) symbols are not
>>> allowed." (https://issues.apache.org/jira/browse/OFBIZ-2544) -
>>> OFBIZ-2544
>>>
>>> I kept only the replacing lines (there were conflicts)
>>>
>>> Modified:
>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>>
>>>
>>> Modified:
>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>>
>>> URL:
>>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java?rev=1003596&r1=1003595&r2=1003596&view=diff
>>>
>>> ==============================================================================
>>>
>>> ---
>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>> (original)
>>> +++
>>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>> Fri Oct 1 17:59:13 2010
>>> @@ -668,7 +668,7 @@ public class CommunicationEventServices
>>> Address[] addressesTo = wrapper.getTo();
>>> Address[] addressesCC = wrapper.getCc();
>>> Address[] addressesBCC = wrapper.getBcc();
>>> - String messageId = wrapper.getMessageId();
>>> + String messageId = wrapper.getMessageId().replaceAll("<|>", "");;
>>>
>>> String aboutThisEmail = "message [" + messageId + "] from [" +
>>> (addressesFrom[0] == null? "not found" : addressesFrom[0].toString())
>>> + "] to [" +
>>> @@ -794,7 +794,7 @@ public class CommunicationEventServices
>>> if (inReplyTo != null&& inReplyTo[0] != null) {
>>> GenericValue parentCommEvent = null;
>>> try {
>>> - List<GenericValue> events =
>>> delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId",
>>> inReplyTo[0]));
>>> + List<GenericValue> events =
>>> delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId",
>>> inReplyTo[0].replaceAll("<|>", "")));
>>> parentCommEvent = EntityUtil.getFirst(events);
>>> } catch (GenericEntityException e) {
>>> Debug.logError(e, module);
>>> @@ -854,7 +854,8 @@ public class CommunicationEventServices
>>> headerString.append(System.getProperty("line.separator"));
>>> headerString.append(headerLines.nextElement());
>>> }
>>> - commEventMap.put("headerString", headerString.toString());
>>> + String header = headerString.toString();
>>> + commEventMap.put("headerString", header.replaceAll("<|>", ""));
>>
>> I think that [<>] would be faster.
>
> Why? since it's only one character and we need to try to replace both,
> it will be alternation in both cases (Alternation or Character Classes)
> in the end, isn'it ?
>
> Jacques
Because the '|' /a|b/, with no other explicit bounds applied, means to
scan the entire string twice. Once to see if there is an 'a', then
start over at the beginning to find the 'b'. Once it finds it, it
repeats is(as that what replaceAll does).
But a character class /[ab]/ has no unlimit bounds constraint, and the
internal matcher can walk each character one at a time, instead of
scanning the string from the beginning again.
/a|b/ functions like this. At the start of the, when attempting to
match the group /a|b/, the position in the string is remembered. This
is 0. Then, the first alternation is tried; in this case, 'a'. A
full string scan is performed, until the first one is found. If so,
it is removed, and you start over completely.
If 'a' is not found, however, the position remembered at the beginning
of the alternation group is returned to, and the second alternation is
tried. And so on for any such alternations.
This is how alternation is defined to work.
>
Re: svn commit: r1003596 - /ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
Posted by Jacques Le Roux <ja...@les7arts.com>.
From: "Adam Heath" <do...@brainfood.com>
> On 10/01/2010 12:59 PM, jleroux@apache.org wrote:
>> Author: jleroux
>> Date: Fri Oct 1 17:59:13 2010
>> New Revision: 1003596
>>
>> URL: http://svn.apache.org/viewvc?rev=1003596&view=rev
>> Log:
>> Excerpt from a BJ's patch at "Message: In field [headerString] less-than (<) and greater-than (>) symbols are not allowed.In
>> field [messageId] less-than (<) and greater-than (>) symbols are not allowed."
>> (https://issues.apache.org/jira/browse/OFBIZ-2544) - OFBIZ-2544
>>
>> I kept only the replacing lines (there were conflicts)
>>
>> Modified:
>> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>>
>> Modified: ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java?rev=1003596&r1=1003595&r2=1003596&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java (original)
>> +++ ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java Fri Oct 1 17:59:13 2010
>> @@ -668,7 +668,7 @@ public class CommunicationEventServices
>> Address[] addressesTo = wrapper.getTo();
>> Address[] addressesCC = wrapper.getCc();
>> Address[] addressesBCC = wrapper.getBcc();
>> - String messageId = wrapper.getMessageId();
>> + String messageId = wrapper.getMessageId().replaceAll("<|>", "");;
>>
>> String aboutThisEmail = "message [" + messageId + "] from [" +
>> (addressesFrom[0] == null? "not found" : addressesFrom[0].toString()) + "] to [" +
>> @@ -794,7 +794,7 @@ public class CommunicationEventServices
>> if (inReplyTo != null&& inReplyTo[0] != null) {
>> GenericValue parentCommEvent = null;
>> try {
>> - List<GenericValue> events = delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId",
>> inReplyTo[0]));
>> + List<GenericValue> events = delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId",
>> inReplyTo[0].replaceAll("<|>", "")));
>> parentCommEvent = EntityUtil.getFirst(events);
>> } catch (GenericEntityException e) {
>> Debug.logError(e, module);
>> @@ -854,7 +854,8 @@ public class CommunicationEventServices
>> headerString.append(System.getProperty("line.separator"));
>> headerString.append(headerLines.nextElement());
>> }
>> - commEventMap.put("headerString", headerString.toString());
>> + String header = headerString.toString();
>> + commEventMap.put("headerString", header.replaceAll("<|>", ""));
>
> I think that [<>] would be faster.
Why? since it's only one character and we need to try to replace both, it will be alternation in both cases (Alternation or
Character Classes) in the end, isn'it ?
Jacques
Re: svn commit: r1003596 - /ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
Posted by Adam Heath <do...@brainfood.com>.
On 10/01/2010 12:59 PM, jleroux@apache.org wrote:
> Author: jleroux
> Date: Fri Oct 1 17:59:13 2010
> New Revision: 1003596
>
> URL: http://svn.apache.org/viewvc?rev=1003596&view=rev
> Log:
> Excerpt from a BJ's patch at "Message: In field [headerString] less-than (<) and greater-than (>) symbols are not allowed.In field [messageId] less-than (<) and greater-than (>) symbols are not allowed." (https://issues.apache.org/jira/browse/OFBIZ-2544) - OFBIZ-2544
>
> I kept only the replacing lines (there were conflicts)
>
> Modified:
> ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
>
> Modified: ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java?rev=1003596&r1=1003595&r2=1003596&view=diff
> ==============================================================================
> --- ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java (original)
> +++ ofbiz/trunk/applications/party/src/org/ofbiz/party/communication/CommunicationEventServices.java Fri Oct 1 17:59:13 2010
> @@ -668,7 +668,7 @@ public class CommunicationEventServices
> Address[] addressesTo = wrapper.getTo();
> Address[] addressesCC = wrapper.getCc();
> Address[] addressesBCC = wrapper.getBcc();
> - String messageId = wrapper.getMessageId();
> + String messageId = wrapper.getMessageId().replaceAll("<|>", "");;
>
> String aboutThisEmail = "message [" + messageId + "] from [" +
> (addressesFrom[0] == null? "not found" : addressesFrom[0].toString()) + "] to [" +
> @@ -794,7 +794,7 @@ public class CommunicationEventServices
> if (inReplyTo != null&& inReplyTo[0] != null) {
> GenericValue parentCommEvent = null;
> try {
> - List<GenericValue> events = delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId", inReplyTo[0]));
> + List<GenericValue> events = delegator.findByAnd("CommunicationEvent", UtilMisc.toMap("messageId", inReplyTo[0].replaceAll("<|>", "")));
> parentCommEvent = EntityUtil.getFirst(events);
> } catch (GenericEntityException e) {
> Debug.logError(e, module);
> @@ -854,7 +854,8 @@ public class CommunicationEventServices
> headerString.append(System.getProperty("line.separator"));
> headerString.append(headerLines.nextElement());
> }
> - commEventMap.put("headerString", headerString.toString());
> + String header = headerString.toString();
> + commEventMap.put("headerString", header.replaceAll("<|>", ""));
I think that [<>] would be faster.