Re: svn commit: r1340631 - in /ofbiz/trunk/framework: base/src/org/ofbiz/base/util/template/ entity/src/org/ofbiz/entity/util/ webapp/src/org/ofbiz/webapp/control/ webapp/src/org/ofbiz/webapp/event/ webapp/src/org/ofbiz/webapp/ftl/ webapp/src/org/ofbiz/web...

[Adam Heath-2]

On 05/20/2012 02:49 AM, [hidden email] wrote:

> Author: jacopoc
> Date: Sun May 20 07:49:45 2012
> New Revision: 1340631
>
> URL: http://svn.apache.org/viewvc?rev=1340631&view=rev
> Log:
> Refactored code to use the default BeansWrapper defined by FreemarkerWorker; this is important according to comment from one of the Freemarker committers:
>
> "As of creating new BeansWrapper, be sure you create it only once during the application life-cycle and then reuse the same instance, or else it will have to rebuild the class introspection cache again and again."
>
> Thanks to Adam Heath for the comment.
>
> Modified:
>      ofbiz/trunk/framework/base/src/org/ofbiz/base/util/template/FreeMarkerWorker.java
>      ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ControlServlet.java
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/WfsEventHandler.java
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/ftl/FreeMarkerViewHandler.java
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/ftl/FreeMarkerViewRenderer.java
>      ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/view/WfsViewHandler.java
>      ofbiz/trunk/framework/webtools/src/org/ofbiz/webtools/WebToolsServices.java
>      ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ScreenRenderer.java

You missed 2 groovy files in this change.

When you change something project wide, search *all* files, not just
those you have a vetted interest in.

Please, people, be careful.

ps: This was the commit that annoyed me in my other email, the one that
implements what I said I would already be doing.

oos: DO NOT DO THE GROOVY FILES YOURSElF. I have it already done.  And
not to my original commit, but to yours.  Grumble.

[Adam Heath-2]

On 05/20/2012 09:17 PM, Adam Heath wrote:

> On 05/20/2012 02:49 AM, [hidden email] wrote:
>> Author: jacopoc
>> Date: Sun May 20 07:49:45 2012
>> New Revision: 1340631
>>
>> URL: http://svn.apache.org/viewvc?rev=1340631&view=rev
>> Log:
>> Refactored code to use the default BeansWrapper defined by
>> FreemarkerWorker; this is important according to comment from one of
>> the Freemarker committers:
>>
>> "As of creating new BeansWrapper, be sure you create it only once
>> during the application life-cycle and then reuse the same instance, or
>> else it will have to rebuild the class introspection cache again and
>> again."
>>
>> Thanks to Adam Heath for the comment.
>>
>> Modified:
>> ofbiz/trunk/framework/base/src/org/ofbiz/base/util/template/FreeMarkerWorker.java
>>
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/util/EntitySaxReader.java
>>
>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/control/ControlServlet.java
>>
>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/event/WfsEventHandler.java
>>
>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/ftl/FreeMarkerViewHandler.java
>>
>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/ftl/FreeMarkerViewRenderer.java
>>
>> ofbiz/trunk/framework/webapp/src/org/ofbiz/webapp/view/WfsViewHandler.java
>>
>> ofbiz/trunk/framework/webtools/src/org/ofbiz/webtools/WebToolsServices.java
>>
>> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/screen/ScreenRenderer.java
>>
>
> You missed 2 groovy files in this change.
>
> When you change something project wide, search *all* files, not just
> those you have a vetted interest in.
>
> Please, people, be careful.
>
> ps: This was the commit that annoyed me in my other email, the one that
> implements what I said I would already be doing.
>
> oos: DO NOT DO THE GROOVY FILES YOURSElF. I have it already done. And
> not to my original commit, but to yours. Grumble.

And, you also missed ExtendedWrapper in HtmlWidget, which is related to
the original root problem.

[Jacopo Cappellato-4]

On May 21, 2012, at 4:30 AM, Adam Heath wrote:

> And, you also missed ExtendedWrapper in HtmlWidget, which is related to the original root problem.

I may be misunderstanding this comment but I think that was fixed by my first commit:

    protected static Configuration specialConfig = FreeMarkerWorker.makeConfiguration((BeansWrapper)new ExtendedWrapper());

Jacopo

[Adam Heath-2]

On 05/21/2012 01:01 AM, Jacopo Cappellato wrote:
>
> On May 21, 2012, at 4:30 AM, Adam Heath wrote:
>
>> And, you also missed ExtendedWrapper in HtmlWidget, which is related to the original root problem.
>
> I may be misunderstanding this comment but I think that was fixed by my first commit:
>
>      protected static Configuration specialConfig = FreeMarkerWorker.makeConfiguration((BeansWrapper)new ExtendedWrapper());

Sorry, ignore that email, got ahead of myself.

My code added a new method that configures a BeanWrapper instance(or an
extended class), then the configured value is passed to
makeConfiguration().  I don't do the configuration of the wrapper in
makeConfiguration, because it might actually happen many times.

[Jacopo Cappellato-4]

On May 21, 2012, at 8:18 AM, Adam Heath wrote:

> Sorry, ignore that email, got ahead of myself.

ok, no problem.

Jacopo