Errors in screen while running findByAnd

[Jacopo Cappellato-4]

https://demo-trunk.ofbiz.apache.org/ordermgr/control/orderview?orderId=Demo1002

Jacopo

[Adam Heath-2]

On 05/16/2012 06:41 AM, Jacopo Cappellato wrote:
> https://demo-trunk.ofbiz.apache.org/ordermgr/control/orderview?orderId=Demo1002

This may be a stupid question, but why doesn't "admin/ofbiz" work as a
login?  (right now, the admin account is disabled for 5 minutes,
that's my fault).

I can't yet see this error.  I'm updating to trunk, and will attempt
the same thing locally.

[Adam Heath-2]

On 05/16/2012 11:05 AM, Adam Heath wrote:
> On 05/16/2012 06:41 AM, Jacopo Cappellato wrote:
>> https://demo-trunk.ofbiz.apache.org/ordermgr/control/orderview?orderId=Demo1002
>
> This may be a stupid question, but why doesn't "admin/ofbiz" work as a
> login?  (right now, the admin account is disabled for 5 minutes,
> that's my fault).
>
> I can't yet see this error.  I'm updating to trunk, and will attempt
> the same thing locally.

Is freemarker really this stupid?  Really?  That line is:

shipmentRouteSegments=delegator.findByAnd("ShipmentRouteSegment",
{"shipmentId" : shipment.shipmentId}, null, false) [on line 581,
column 25 in
component://order/webapp/ordermgr/order/ordershippinginfo.ftl]

And there *is* a findByAnd in Delegator that has (String, Map, List,
boolean).  Why is thinking that findByAnd(String, Object...) is the
correct one to use?  I'm leaning towards this being a freemarker bug.

If one other person agrees with me, then I'll attempt to simulate it
as a real freemark bug(new freemarker template, new object that
implements similiar interfaces, isolated from any/all ofbiz code), to
make reporting a freemark bug simpler.  And then fix freemarker myself.

[Adam Heath-2]

On 05/16/2012 11:18 AM, Adam Heath wrote:

> On 05/16/2012 11:05 AM, Adam Heath wrote:
>> On 05/16/2012 06:41 AM, Jacopo Cappellato wrote:
>>> https://demo-trunk.ofbiz.apache.org/ordermgr/control/orderview?orderId=Demo1002
>>
>> This may be a stupid question, but why doesn't "admin/ofbiz" work as a
>> login?  (right now, the admin account is disabled for 5 minutes,
>> that's my fault).
>>
>> I can't yet see this error.  I'm updating to trunk, and will attempt
>> the same thing locally.
>
> Is freemarker really this stupid?  Really?  That line is:
>
> shipmentRouteSegments=delegator.findByAnd("ShipmentRouteSegment",
> {"shipmentId" : shipment.shipmentId}, null, false) [on line 581,
> column 25 in
> component://order/webapp/ordermgr/order/ordershippinginfo.ftl]
>
> And there *is* a findByAnd in Delegator that has (String, Map, List,
> boolean).  Why is thinking that findByAnd(String, Object...) is the
> correct one to use?  I'm leaning towards this being a freemarker bug.
>
> If one other person agrees with me, then I'll attempt to simulate it
> as a real freemark bug(new freemarker template, new object that
> implements similiar interfaces, isolated from any/all ofbiz code), to
> make reporting a freemark bug simpler.  And then fix freemarker myself.

Gah.  As a test, I tried placing the Object... variant last in both
the interface and the class.  No change, it still picks the wrong method.

Velocity, Freemarker, Groovy, Asm, and other similiar tools all go
from free-form text, to eventually call a method.  Every single tool
needs to be the most correct method to call.  And they all implement
their own way of figuring that out.  Does anyone know of something
that does that correctly in a simple library?  There is nothing in the
default java spec that does this(which is an oversite, imho).

[Adam Heath-2]

On 05/16/2012 11:30 AM, Adam Heath wrote:

> On 05/16/2012 11:18 AM, Adam Heath wrote:
>> On 05/16/2012 11:05 AM, Adam Heath wrote:
>>> On 05/16/2012 06:41 AM, Jacopo Cappellato wrote:
>>>> https://demo-trunk.ofbiz.apache.org/ordermgr/control/orderview?orderId=Demo1002
>>>
>>> This may be a stupid question, but why doesn't "admin/ofbiz" work as a
>>> login?  (right now, the admin account is disabled for 5 minutes,
>>> that's my fault).
>>>
>>> I can't yet see this error.  I'm updating to trunk, and will attempt
>>> the same thing locally.
>>
>> Is freemarker really this stupid?  Really?  That line is:
>>
>> shipmentRouteSegments=delegator.findByAnd("ShipmentRouteSegment",
>> {"shipmentId" : shipment.shipmentId}, null, false) [on line 581,
>> column 25 in
>> component://order/webapp/ordermgr/order/ordershippinginfo.ftl]
>>
>> And there *is* a findByAnd in Delegator that has (String, Map, List,
>> boolean).  Why is thinking that findByAnd(String, Object...) is the
>> correct one to use?  I'm leaning towards this being a freemarker bug.
>>
>> If one other person agrees with me, then I'll attempt to simulate it
>> as a real freemark bug(new freemarker template, new object that
>> implements similiar interfaces, isolated from any/all ofbiz code), to
>> make reporting a freemark bug simpler.  And then fix freemarker myself.
>
> Gah.  As a test, I tried placing the Object... variant last in both
> the interface and the class.  No change, it still picks the wrong method.
>
> Velocity, Freemarker, Groovy, Asm, and other similiar tools all go
> from free-form text, to eventually call a method.  Every single tool
> needs to be the most correct method to call.  And they all implement
> their own way of figuring that out.  Does anyone know of something
> that does that correctly in a simple library?  There is nothing in the
> default java spec that does this(which is an oversite, imho).

Yes, absolute bug in freemarker.

s/null/[]/ in the ftl, and it picks the correct method.  I read the
freemarker code to come to a workaround.  But I do not want to commit
the workaround into ofbiz.

[Jacopo Cappellato-4]

Adam, all,

this is interesting, thanks for the research.

I would suggest the following approach:

1) identify which one of the recent changes we did caused this error to happen
2) if possible, commit a temporary workaround for this to let the system work as before
3) file a bug in the Freemarker issue tracker
4) contact the Freemarker community to discuss the issue and try to schedule with them a new release date for the new release (2.3.20); ideally this will also contain my fixes for the deadlock condition affecting Freemarker
5) when the new Freemarker release will be issued, we upgrade OFBiz to it and we revert #2

I can help with #4 as I recently have been in touch with them.
Did you get a chance to figure out how many screens could are affected? If there are just a few then, instead of #2,we could commit a temporaray fix for the screens rather than a global one.

Kind regards,

Jacopo

On May 16, 2012, at 9:55 PM, Adam Heath wrote:

> On 05/16/2012 11:30 AM, Adam Heath wrote:
>> On 05/16/2012 11:18 AM, Adam Heath wrote:
>>> On 05/16/2012 11:05 AM, Adam Heath wrote:
>>>> On 05/16/2012 06:41 AM, Jacopo Cappellato wrote:
>>>>> https://demo-trunk.ofbiz.apache.org/ordermgr/control/orderview?orderId=Demo1002
>>>>
>>>> This may be a stupid question, but why doesn't "admin/ofbiz" work as a
>>>> login?  (right now, the admin account is disabled for 5 minutes,
>>>> that's my fault).
>>>>
>>>> I can't yet see this error.  I'm updating to trunk, and will attempt
>>>> the same thing locally.
>>>
>>> Is freemarker really this stupid?  Really?  That line is:
>>>
>>> shipmentRouteSegments=delegator.findByAnd("ShipmentRouteSegment",
>>> {"shipmentId" : shipment.shipmentId}, null, false) [on line 581,
>>> column 25 in
>>> component://order/webapp/ordermgr/order/ordershippinginfo.ftl]
>>>
>>> And there *is* a findByAnd in Delegator that has (String, Map, List,
>>> boolean).  Why is thinking that findByAnd(String, Object...) is the
>>> correct one to use?  I'm leaning towards this being a freemarker bug.
>>>
>>> If one other person agrees with me, then I'll attempt to simulate it
>>> as a real freemark bug(new freemarker template, new object that
>>> implements similiar interfaces, isolated from any/all ofbiz code), to
>>> make reporting a freemark bug simpler.  And then fix freemarker myself.
>>
>> Gah.  As a test, I tried placing the Object... variant last in both
>> the interface and the class.  No change, it still picks the wrong method.
>>
>> Velocity, Freemarker, Groovy, Asm, and other similiar tools all go
>> from free-form text, to eventually call a method.  Every single tool
>> needs to be the most correct method to call.  And they all implement
>> their own way of figuring that out.  Does anyone know of something
>> that does that correctly in a simple library?  There is nothing in the
>> default java spec that does this(which is an oversite, imho).
>
> Yes, absolute bug in freemarker.
>
> s/null/[]/ in the ftl, and it picks the correct method.  I read the
> freemarker code to come to a workaround.  But I do not want to commit
> the workaround into ofbiz.
>

[Adam Heath-2]

On 05/17/2012 01:11 AM, Jacopo Cappellato wrote:
> Adam, all,
>
> this is interesting, thanks for the research.
>
> I would suggest the following approach:
>
> 1) identify which one of the recent changes we did caused this error to happen

I know *exactly* which commit(s) it was.  The findByAnd changes I did,
the huge commits.

> 2) if possible, commit a temporary workaround for this to let the system work as before

The temporary workaround is to s/null/[]/ in all ftl files.  That's a
very large commit, that will just have to be reverted again when it is
fixed properly by freemarker.

> 3) file a bug in the Freemarker issue tracker

not yet, waiting, see below

> 4) contact the Freemarker community to discuss the issue and try to schedule with them a new release date for the new release (2.3.20); ideally this will also contain my fixes for the deadlock condition affecting Freemarker

not yet, waiting, see below

> 5) when the new Freemarker release will be issued, we upgrade OFBiz to it and we revert #2

I've already got the 2.4 branch git cloned, and have verified the bug
still exists.  But I need to clone 2.3.19, and work there.

When I find these kinds of bugs, I don't file the issue until I have a
*very* good handle on it, and in many cases, depending on the severity
of the problem, and how easy a workaround might be, I'll even send a
patch fixing the problem.

I probably would have had a fix yesterday, but we had a client
in-house and I had to make some progress for her, while she was here,
in case I had any questions.

> I can help with #4 as I recently have been in touch with them.
> Did you get a chance to figure out how many screens could are affected? If there are just a few then, instead of #2,we could commit a temporaray fix for the screens rather than a global one.

Every single ftl file I touched that has a null parameter.  In fact,
this bug *could* affect any method call by freemarker, we've just been
lucky.

[Jacopo Cappellato-4]

Adam,

I am sorry but a few minutes ago I filed the bug in the Freemarker issue tracker:

https://sourceforge.net/tracker/?func=detail&aid=3527625&group_id=794&atid=100794

I think I have added enough details for them to comment; if you feel like you can provide more of them (or even a patch) please feel free to comment on the same ticket.

Jacopo

On May 17, 2012, at 5:11 PM, Adam Heath wrote:

> On 05/17/2012 01:11 AM, Jacopo Cappellato wrote:
>> Adam, all,
>>
>> this is interesting, thanks for the research.
>>
>> I would suggest the following approach:
>>
>> 1) identify which one of the recent changes we did caused this error to happen
>
> I know *exactly* which commit(s) it was.  The findByAnd changes I did,
> the huge commits.
>
>> 2) if possible, commit a temporary workaround for this to let the system work as before
>
> The temporary workaround is to s/null/[]/ in all ftl files.  That's a
> very large commit, that will just have to be reverted again when it is
> fixed properly by freemarker.
>
>> 3) file a bug in the Freemarker issue tracker
>
> not yet, waiting, see below
>
>> 4) contact the Freemarker community to discuss the issue and try to schedule with them a new release date for the new release (2.3.20); ideally this will also contain my fixes for the deadlock condition affecting Freemarker
>
> not yet, waiting, see below
>
>> 5) when the new Freemarker release will be issued, we upgrade OFBiz to it and we revert #2
>
> I've already got the 2.4 branch git cloned, and have verified the bug
> still exists.  But I need to clone 2.3.19, and work there.
>
> When I find these kinds of bugs, I don't file the issue until I have a
> *very* good handle on it, and in many cases, depending on the severity
> of the problem, and how easy a workaround might be, I'll even send a
> patch fixing the problem.
>
> I probably would have had a fix yesterday, but we had a client
> in-house and I had to make some progress for her, while she was here,
> in case I had any questions.
>
>> I can help with #4 as I recently have been in touch with them.
>> Did you get a chance to figure out how many screens could are affected? If there are just a few then, instead of #2,we could commit a temporaray fix for the screens rather than a global one.
>
> Every single ftl file I touched that has a null parameter.  In fact,
> this bug *could* affect any method call by freemarker, we've just been
> lucky.
>

[Adam Heath-2]

On 05/17/2012 10:22 AM, Jacopo Cappellato wrote:
> Adam,
>
> I am sorry but a few minutes ago I filed the bug in the Freemarker issue tracker:
>
> https://sourceforge.net/tracker/?func=detail&aid=3527625&group_id=794&atid=100794
>
> I think I have added enough details for them to comment; if you feel like you can provide more of them (or even a patch) please feel free to comment on the same ticket.

At the very least, when I get around to working on it, I'll create a
much smaller example.  I've got a git svn clone in progress(just got
in to work).

* must use the 'null' keyword in the ftl, I don't think using a
variable that resolves to null will trigger it; it might, depending on
how tight freemarker keeps the type with the variable.
* it's possible that you need to have a vararg method involved.  That
means using '...', not just Object[].
* Of course, all the methods need to have the same name.
* based on my reading of the code, it doesn't matter which raw order
the methods are returned in when doing reflection on the class.
* It's not just related to Static['foo']; again, based on my reading
of the code.

[Jacopo Cappellato-4]

On May 17, 2012, at 5:32 PM, Adam Heath wrote:
>
> * It's not just related to Static['foo']; again, based on my reading
> of the code.

I can confirm that Static is not involved in the issue

[Adam Heath-2]

On 05/17/2012 10:40 AM, Jacopo Cappellato wrote:
>
> On May 17, 2012, at 5:32 PM, Adam Heath wrote:
>>
>> * It's not just related to Static['foo']; again, based on my reading
>> of the code.
>
> I can confirm that Static is not involved in the issue

See attached, very simple.  It *should* call the first m(), but it
really calls the second.

[Jacopo Cappellato-4]

Looks good to me, are you going to attach it to the ticket I have created in the Freemarker bug tracker?

Jacopo

On May 17, 2012, at 5:51 PM, Adam Heath wrote:

> On 05/17/2012 10:40 AM, Jacopo Cappellato wrote:
>>
>> On May 17, 2012, at 5:32 PM, Adam Heath wrote:
>>>
>>> * It's not just related to Static['foo']; again, based on my reading
>>> of the code.
>>
>> I can confirm that Static is not involved in the issue
>
> See attached, very simple.  It *should* call the first m(), but it
> really calls the second.
> <FreemarkerBug.java>

[Adam Heath-2]

On 05/17/2012 10:56 AM, Jacopo Cappellato wrote:
> Looks good to me, are you going to attach it to the ticket I have created in the Freemarker bug tracker?

I'll do it; what's the other bug#?  Do we want to bump the severity?

[Jacopo Cappellato-4]

On May 17, 2012, at 5:57 PM, Adam Heath wrote:

> On 05/17/2012 10:56 AM, Jacopo Cappellato wrote:
>> Looks good to me, are you going to attach it to the ticket I have created in the Freemarker bug tracker?
>
> I'll do it; what's the other bug#?  Do we want to bump the severity?


The bug# is 3527625; here is the direct link:

https://sourceforge.net/tracker/?func=detail&aid=3527625&group_id=794&atid=100794

Feel free to edit it but, based on recent experience, I expect a quick feedback from them regardless.

Jacopo

[Adam Heath-2]

On 05/17/2012 10:57 AM, Adam Heath wrote:
> On 05/17/2012 10:56 AM, Jacopo Cappellato wrote:
>> Looks good to me, are you going to attach it to the ticket I have created in the Freemarker bug tracker?
>
> I'll do it; what's the other bug#?  Do we want to bump the severity?

I'm just waiting on greylisting on our mailserver to allow it thru,
then'll the attachment will be there.

[Adam Heath-2]

On 05/17/2012 11:20 AM, Adam Heath wrote:
> On 05/17/2012 10:57 AM, Adam Heath wrote:
>> On 05/17/2012 10:56 AM, Jacopo Cappellato wrote:
>>> Looks good to me, are you going to attach it to the ticket I have created in the Freemarker bug tracker?
>>
>> I'll do it; what's the other bug#?  Do we want to bump the severity?
>
> I'm just waiting on greylisting on our mailserver to allow it thru,
> then'll the attachment will be there.

While waiting, I made a patch, seems to fix it.

[Jacopo Cappellato-4]

wonderful

Jacopo

On May 17, 2012, at 6:45 PM, Adam Heath wrote:

> On 05/17/2012 11:20 AM, Adam Heath wrote:
>> On 05/17/2012 10:57 AM, Adam Heath wrote:
>>> On 05/17/2012 10:56 AM, Jacopo Cappellato wrote:
>>>> Looks good to me, are you going to attach it to the ticket I have created in the Freemarker bug tracker?
>>>
>>> I'll do it; what's the other bug#?  Do we want to bump the severity?
>>
>> I'm just waiting on greylisting on our mailserver to allow it thru,
>> then'll the attachment will be there.
>
> While waiting, I made a patch, seems to fix it.
>
>
> <fix-freemarker.patch>

[Adam Heath-2]

On 05/17/2012 11:48 AM, Jacopo Cappellato wrote:
> wonderful

I can't seem to attach files to that issue, maybe because I didn't
originally file it.  Could you see if you can attach both files?

If you can, but I can't, then the sf tracker is really stupid.

[Jacopo Cappellato-4]

sure, I am going to try right now

Jacopo

On May 17, 2012, at 7:18 PM, Adam Heath wrote:

> On 05/17/2012 11:48 AM, Jacopo Cappellato wrote:
>> wonderful
>
> I can't seem to attach files to that issue, maybe because I didn't
> originally file it.  Could you see if you can attach both files?
>
> If you can, but I can't, then the sf tracker is really stupid.

[Jacopo Cappellato-4]

file are attached; that bug tracker is really awful.

Jacopo

On May 17, 2012, at 7:20 PM, Jacopo Cappellato wrote:

> sure, I am going to try right now
>
> Jacopo
>
> On May 17, 2012, at 7:18 PM, Adam Heath wrote:
>
>> On 05/17/2012 11:48 AM, Jacopo Cappellato wrote:
>>> wonderful
>>
>> I can't seem to attach files to that issue, maybe because I didn't
>> originally file it.  Could you see if you can attach both files?
>>
>> If you can, but I can't, then the sf tracker is really stupid.
>