You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Jacopo Cappellato <ja...@hotwaxmedia.com> on 2012/05/16 13:41:18 UTC

Errors in screen while running findByAnd

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

Jacopo

Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
On May 18, 2012, at 7:56 AM, Adam Heath wrote:

> On 05/18/2012 12:52 AM, Jacopo Cappellato wrote:
>> 
>> On May 18, 2012, at 7:02 AM, Adam Heath wrote:
>> 
>>> Imho, we should *not* be calling into the delegator inside freemarker templates.  Templates should not be tied to the backend *at all*.  They should only deal with 100% completely generic collection objects. Because they are currently tied to the delegator, this makes it *impossible* to have a separate web-frontend, that isn't tied to ofbiz at all.
>> 
>> I agree on the general rule; unfortunately there are exceptions where enforcing this rule is too problematic; however I agree that the practice of calling the delegator API from FM is used much more than it should.
>> Apart from moving data preparation code to data preparation scripts where possible, we could also define a FM transform to implement these calls and this will protect the access to the delegator because in the transform we would perform all the necessary null checks etc...
>> The risk is to end up with a big sets of transforms that re-implement the delegator API, but we could start by implementing the calls that are currently clearly broken (findByAnd) and then create new ones (or refactor existing code) *before* you will refactor other delegator methods.
>> In this way we will have a temp fix (waiting for the official fix from FM) and also a better implementation of templates (that execute calls to delegator only thru a transform)
> 
> This problem with null parameter values in freemarker only occurs when there are multiple methods with the same name.  If there is only a single method with a name, then no overridden resolution logic happens.  That's how the freemarker code functions.
> 
> So, if my new findByAnd variant gets a new name, then all can be well. This is how find() and findList() got implemented, and why we never noticed this freemarker problem before.
> 
> So, any suggestions for a new name?

Would it make sense to wrap the delegator before adding it to the context of Freemarker? In this way we could "fix" the call for freemarker only and keep the official API as clean as possible.

Jacopo



Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
On 05/18/2012 12:52 AM, Jacopo Cappellato wrote:
>
> On May 18, 2012, at 7:02 AM, Adam Heath wrote:
>
>> Imho, we should *not* be calling into the delegator inside freemarker templates.  Templates should not be tied to the backend *at all*.  They should only deal with 100% completely generic collection objects. Because they are currently tied to the delegator, this makes it *impossible* to have a separate web-frontend, that isn't tied to ofbiz at all.
>
> I agree on the general rule; unfortunately there are exceptions where enforcing this rule is too problematic; however I agree that the practice of calling the delegator API from FM is used much more than it should.
> Apart from moving data preparation code to data preparation scripts where possible, we could also define a FM transform to implement these calls and this will protect the access to the delegator because in the transform we would perform all the necessary null checks etc...
> The risk is to end up with a big sets of transforms that re-implement the delegator API, but we could start by implementing the calls that are currently clearly broken (findByAnd) and then create new ones (or refactor existing code) *before* you will refactor other delegator methods.
> In this way we will have a temp fix (waiting for the official fix from FM) and also a better implementation of templates (that execute calls to delegator only thru a transform)

This problem with null parameter values in freemarker only occurs when 
there are multiple methods with the same name.  If there is only a 
single method with a name, then no overridden resolution logic happens. 
  That's how the freemarker code functions.

So, if my new findByAnd variant gets a new name, then all can be well. 
This is how find() and findList() got implemented, and why we never 
noticed this freemarker problem before.

So, any suggestions for a new name?

Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
On May 18, 2012, at 7:02 AM, Adam Heath wrote:

> Imho, we should *not* be calling into the delegator inside freemarker templates.  Templates should not be tied to the backend *at all*.  They should only deal with 100% completely generic collection objects. Because they are currently tied to the delegator, this makes it *impossible* to have a separate web-frontend, that isn't tied to ofbiz at all.

I agree on the general rule; unfortunately there are exceptions where enforcing this rule is too problematic; however I agree that the practice of calling the delegator API from FM is used much more than it should.
Apart from moving data preparation code to data preparation scripts where possible, we could also define a FM transform to implement these calls and this will protect the access to the delegator because in the transform we would perform all the necessary null checks etc...
The risk is to end up with a big sets of transforms that re-implement the delegator API, but we could start by implementing the calls that are currently clearly broken (findByAnd) and then create new ones (or refactor existing code) *before* you will refactor other delegator methods.
In this way we will have a temp fix (waiting for the official fix from FM) and also a better implementation of templates (that execute calls to delegator only thru a transform)

Kind regards,

Jacopo

Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
On 05/17/2012 11:50 PM, Adam Heath wrote:
> On 05/17/2012 12:23 PM, Jacopo Cappellato wrote:
>> file are attached; that bug tracker is really awful.
>
> This is all becoming very troubling.
>
> == example methods
> String a(String name, List list) {}
> String a(String name, Map map) {}
> String a(Object[] args) {}
>
> String b(String name, List list) {}
> String b(String name, Map map) {}
> String b(Object... args) {}
>
> == example templates
> ${object.a(string, null)}
> ${object.b(string, null)}
>
> ==
>
> In 2.3.18, freemarker can't find any methods at all. In 2.3.19, it can
> find the b vararg method. In either situation, that is wrong.
>
> The correct situation for both is to say there are 2 matching methods,
> the List and Map variants. Without my patch, when a null is used, any
> non-primitive method will never be considered as a possible candidate.
>
> I'm still discussing with freemarker upstream, but I get the feeling
> that they won't provide a fix for the 2.3 branch, and a true, correct
> fix for 2.4 will be long in coming.
>
> This means we need to come up with a workaround for ofbiz. I'm up for
> suggestions. We really can't undo the change, that just doesn't seem
> right to me.

In detail, using a null in a method call inside freemarker will be 
*extremely* problematic.  Even if I rename my new findByAnd to smoething 
else, passing a null to it will keep freemarker from finding it.

Imho, we should *not* be calling into the delegator inside freemarker 
templates.  Templates should not be tied to the backend *at all*.  They 
should only deal with 100% completely generic collection objects. 
Because they are currently tied to the delegator, this makes it 
*impossible* to have a separate web-frontend, that isn't tied to ofbiz 
at all.


Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
On 05/18/2012 02:45 AM, Adrian Crum wrote:
> On 5/18/2012 5:50 AM, Adam Heath wrote:
>> On 05/17/2012 12:23 PM, Jacopo Cappellato wrote:
>>> file are attached; that bug tracker is really awful.
>>
>> This is all becoming very troubling.
>>
>> == example methods
>> String a(String name, List list) {}
>> String a(String name, Map map) {}
>> String a(Object[] args) {}
>>
>> String b(String name, List list) {}
>> String b(String name, Map map) {}
>> String b(Object... args) {}
>>
>> == example templates
>> ${object.a(string, null)}
>> ${object.b(string, null)}
>>
>> ==
>>
>> In 2.3.18, freemarker can't find any methods at all.  In 2.3.19, it
>> can find the b vararg method.  In either situation, that is wrong.
>>
>> The correct situation for both is to say there are 2 matching
>> methods, the List and Map variants.  Without my patch, when a null
>> is used, any non-primitive method will never be considered as a
>> possible candidate.
>>
>> I'm still discussing with freemarker upstream, but I get the feeling
>> that they won't provide a fix for the 2.3 branch, and a true,
>> correct fix for 2.4 will be long in coming.
>>
>> This means we need to come up with a workaround for ofbiz.  I'm up
>> for suggestions.  We really can't undo the change, that just doesn't
>> seem right to me.
>> .
> 
> In my experience with FreeMarker, there are a lot of things that are
> replaceable - like TemplateLoader and TemplateExceptionHandler. Does
> FreeMarker have a "Object Method Resolver" that we can replace? Would
> a custom BeansWrapper help?

I'm currently in discussion with upstream(1), they may give me commit
rights after I sign a CLA.

Their suggestion right now is a new property on BeansWrapper.

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

Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
On 05/18/2012 03:13 AM, Deepak Dixit wrote:
> Hello Adam,
> 
> We can't pass *null* from ftl if we have to pass null then we can use *NULL* like:
> 
> <#assign productStroe = delegator.findList("ProductStore",NULL,NULL,NULL,NULL, false) />
> 
> Passing *null* is prohibited in NodeListModel class (http://www.docjar.org/docs/api/freemarker/ext/xml/NodeListModel.html). 
> 
> To create an empty model, pass it an empty collection. If we pass an empty list in new findByAnd method then it should work fine:
> 
> <#assign shipmentReceipts = delegator.findByAnd("ShipmentReceipt", {"orderId": orderHeader.getString("orderId"), "orderItemSeqId": orderItem.orderItemSeqId}, [], false)/>

I thought of this, and rejected it.  It will cause more memory churn
than is really nescessary.  All the dummy empty objects.  The same
goes for Static['Collections'].EMPTY_MAP, as that has too much runtime
method invocation overhead.

Altho, maybe if ofbiz auto-inserted EMPTY_MAP and EMPTY_LIST, but even
that would require a context variable lookup, for what really should
be a *real* constant value.

And, I do not want to *have* to modify *every* *single* *ftl* line
again that calls the new findByAnd.

Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
that API is actually rather old and the class has been deprecated and then reimplemented.

Jacopo

On May 18, 2012, at 10:13 AM, Deepak Dixit wrote:

> Hello Adam,
> 
> We can't pass *null* from ftl if we have to pass null then we can use *NULL* like:
> 
> <#assign productStroe = delegator.findList("ProductStore",NULL,NULL,NULL,NULL, false) />
> 
> Passing *null* is prohibited in NodeListModel class (http://www.docjar.org/docs/api/freemarker/ext/xml/NodeListModel.html). 
> 
> To create an empty model, pass it an empty collection. If we pass an empty list in new findByAnd method then it should work fine:
> 
> <#assign shipmentReceipts = delegator.findByAnd("ShipmentReceipt", {"orderId": orderHeader.getString("orderId"), "orderItemSeqId": orderItem.orderItemSeqId}, [], false)/>
> 
> I have tested it and its working fine.
> 
> 
> Thanks & Regards
> -- 
> Deepak Dixit
> 
> On May 18, 2012, at 1:15 PM, Adrian Crum wrote:
> 
>> On 5/18/2012 5:50 AM, Adam Heath wrote:
>>> On 05/17/2012 12:23 PM, Jacopo Cappellato wrote:
>>>> file are attached; that bug tracker is really awful.
>>> 
>>> This is all becoming very troubling.
>>> 
>>> == example methods
>>> String a(String name, List list) {}
>>> String a(String name, Map map) {}
>>> String a(Object[] args) {}
>>> 
>>> String b(String name, List list) {}
>>> String b(String name, Map map) {}
>>> String b(Object... args) {}
>>> 
>>> == example templates
>>> ${object.a(string, null)}
>>> ${object.b(string, null)}
>>> 
>>> ==
>>> 
>>> In 2.3.18, freemarker can't find any methods at all.  In 2.3.19, it can find the b vararg method.  In either situation, that is wrong.
>>> 
>>> The correct situation for both is to say there are 2 matching methods, the List and Map variants.  Without my patch, when a null is used, any non-primitive method will never be considered as a possible candidate.
>>> 
>>> I'm still discussing with freemarker upstream, but I get the feeling that they won't provide a fix for the 2.3 branch, and a true, correct fix for 2.4 will be long in coming.
>>> 
>>> This means we need to come up with a workaround for ofbiz.  I'm up for suggestions.  We really can't undo the change, that just doesn't seem right to me.
>>> .
>> 
>> In my experience with FreeMarker, there are a lot of things that are replaceable - like TemplateLoader and TemplateExceptionHandler. Does FreeMarker have a "Object Method Resolver" that we can replace? Would a custom BeansWrapper help?
>> 
>> -Adrian
>> 
> 


Re: Errors in screen while running findByAnd

Posted by Deepak Dixit <de...@hotwaxmedia.com>.
Hello Adam,

We can't pass *null* from ftl if we have to pass null then we can use *NULL* like:

<#assign productStroe = delegator.findList("ProductStore",NULL,NULL,NULL,NULL, false) />

Passing *null* is prohibited in NodeListModel class (http://www.docjar.org/docs/api/freemarker/ext/xml/NodeListModel.html). 

To create an empty model, pass it an empty collection. If we pass an empty list in new findByAnd method then it should work fine:

<#assign shipmentReceipts = delegator.findByAnd("ShipmentReceipt", {"orderId": orderHeader.getString("orderId"), "orderItemSeqId": orderItem.orderItemSeqId}, [], false)/>

I have tested it and its working fine.


Thanks & Regards
-- 
Deepak Dixit

On May 18, 2012, at 1:15 PM, Adrian Crum wrote:

> On 5/18/2012 5:50 AM, Adam Heath wrote:
>> On 05/17/2012 12:23 PM, Jacopo Cappellato wrote:
>>> file are attached; that bug tracker is really awful.
>> 
>> This is all becoming very troubling.
>> 
>> == example methods
>> String a(String name, List list) {}
>> String a(String name, Map map) {}
>> String a(Object[] args) {}
>> 
>> String b(String name, List list) {}
>> String b(String name, Map map) {}
>> String b(Object... args) {}
>> 
>> == example templates
>> ${object.a(string, null)}
>> ${object.b(string, null)}
>> 
>> ==
>> 
>> In 2.3.18, freemarker can't find any methods at all.  In 2.3.19, it can find the b vararg method.  In either situation, that is wrong.
>> 
>> The correct situation for both is to say there are 2 matching methods, the List and Map variants.  Without my patch, when a null is used, any non-primitive method will never be considered as a possible candidate.
>> 
>> I'm still discussing with freemarker upstream, but I get the feeling that they won't provide a fix for the 2.3 branch, and a true, correct fix for 2.4 will be long in coming.
>> 
>> This means we need to come up with a workaround for ofbiz.  I'm up for suggestions.  We really can't undo the change, that just doesn't seem right to me.
>> .
> 
> In my experience with FreeMarker, there are a lot of things that are replaceable - like TemplateLoader and TemplateExceptionHandler. Does FreeMarker have a "Object Method Resolver" that we can replace? Would a custom BeansWrapper help?
> 
> -Adrian
> 


Re: Errors in screen while running findByAnd

Posted by Adrian Crum <ad...@sandglass-software.com>.
On 5/18/2012 5:50 AM, Adam Heath wrote:
> On 05/17/2012 12:23 PM, Jacopo Cappellato wrote:
>> file are attached; that bug tracker is really awful.
>
> This is all becoming very troubling.
>
> == example methods
> String a(String name, List list) {}
> String a(String name, Map map) {}
> String a(Object[] args) {}
>
> String b(String name, List list) {}
> String b(String name, Map map) {}
> String b(Object... args) {}
>
> == example templates
> ${object.a(string, null)}
> ${object.b(string, null)}
>
> ==
>
> In 2.3.18, freemarker can't find any methods at all.  In 2.3.19, it 
> can find the b vararg method.  In either situation, that is wrong.
>
> The correct situation for both is to say there are 2 matching methods, 
> the List and Map variants.  Without my patch, when a null is used, any 
> non-primitive method will never be considered as a possible candidate.
>
> I'm still discussing with freemarker upstream, but I get the feeling 
> that they won't provide a fix for the 2.3 branch, and a true, correct 
> fix for 2.4 will be long in coming.
>
> This means we need to come up with a workaround for ofbiz.  I'm up for 
> suggestions.  We really can't undo the change, that just doesn't seem 
> right to me.
> .

In my experience with FreeMarker, there are a lot of things that are 
replaceable - like TemplateLoader and TemplateExceptionHandler. Does 
FreeMarker have a "Object Method Resolver" that we can replace? Would a 
custom BeansWrapper help?

-Adrian


Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
On 05/17/2012 12:23 PM, Jacopo Cappellato wrote:
> file are attached; that bug tracker is really awful.

This is all becoming very troubling.

== example methods
String a(String name, List list) {}
String a(String name, Map map) {}
String a(Object[] args) {}

String b(String name, List list) {}
String b(String name, Map map) {}
String b(Object... args) {}

== example templates
${object.a(string, null)}
${object.b(string, null)}

==

In 2.3.18, freemarker can't find any methods at all.  In 2.3.19, it can 
find the b vararg method.  In either situation, that is wrong.

The correct situation for both is to say there are 2 matching methods, 
the List and Map variants.  Without my patch, when a null is used, any 
non-primitive method will never be considered as a possible candidate.

I'm still discussing with freemarker upstream, but I get the feeling 
that they won't provide a fix for the 2.3 branch, and a true, correct 
fix for 2.4 will be long in coming.

This means we need to come up with a workaround for ofbiz.  I'm up for 
suggestions.  We really can't undo the change, that just doesn't seem 
right to me.
.

Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
doing it right now

Jacopo

On May 18, 2012, at 6:01 PM, Adam Heath wrote:

> On 05/17/2012 12:23 PM, Jacopo Cappellato wrote:
>> file are attached; that bug tracker is really awful.
> 
> I've got another one to attach, a more proper fix, that makes the
> change configurable.
> 
> ps: Have I said how much I hate that tracker?
> <freemarker-nullArgs-fix.patch>


Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
Sorry,

it took some time because I was in a call.
The patch is attached now.

Your secretary,

Jacopo

On May 18, 2012, at 6:01 PM, Adam Heath wrote:

> On 05/17/2012 12:23 PM, Jacopo Cappellato wrote:
>> file are attached; that bug tracker is really awful.
> 
> I've got another one to attach, a more proper fix, that makes the
> change configurable.
> 
> ps: Have I said how much I hate that tracker?
> <freemarker-nullArgs-fix.patch>


Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
On 05/17/2012 12:23 PM, Jacopo Cappellato wrote:
> file are attached; that bug tracker is really awful.

I've got another one to attach, a more proper fix, that makes the
change configurable.

ps: Have I said how much I hate that tracker?

Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
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.
> 


Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
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.


Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
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.

Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
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>


Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
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.



Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
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.

Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
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


Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
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?

Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
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>


Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
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.

Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
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

Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
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.

Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
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.
> 


Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
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.


Re: Errors in screen while running findByAnd

Posted by Jacopo Cappellato <ja...@hotwaxmedia.com>.
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.
> 


Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
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.


Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
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).


Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
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.

Re: Errors in screen while running findByAnd

Posted by Adam Heath <do...@brainfood.com>.
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.