ofbiz-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From BJ Freeman <bjf...@free-man.net>
Subject Re: Bugs in OrderView.groovy
Date Mon, 01 Jun 2009 12:44:41 GMT
ok why don't you explain what the whole script is doing and how you see
this not being correct, in the context of an order created by ofbiz. not
your import.

Cimballi sent the following on 6/1/2009 5:16 AM:
> My email was not about if an order must have a productStore or not,
> but about the fact that in the same file, you first accept a null
> productStore, and then you don't.
> 
> Cimballi
> 
> 
> On Sun, May 31, 2009 at 11:20 PM, Scott Gray <scott.gray@hotwaxmedia.com> wrote:
>> Just because the data model allows an order without a product store doesn't
>> mean that the code does.  There are a million ways that you can cause errors
>> in the system with incorrectly loaded data.
>>
>> Regards
>> Scott
>>
>> On 1/06/2009, at 8:56 AM, Cimballi wrote:
>>
>>> Here is a data file which you can import and which will generate the
>>> null pointer exception when trying to view the order :
>>>
>>> <?xml version="1.0" encoding="UTF-8"?>
>>>
>>> <entity-engine-xml>
>>>
>>>  <OrderType description="Special Sales" hasTable="N"
>>>   orderTypeId="SPECIAL_SALES_ORDER" parentTypeId="SALES_ORDER" />
>>>
>>>  <OrderHeader orderId="OH0001" orderTypeId="SPECIAL_SALES_ORDER"
>>>   orderDate="2009-01-01 12:00:00.0" entryDate="2009-01-01 12:00:00.0"
>>>   statusId="ORDER_CREATED" />
>>>
>>>  <OrderRole orderId="OH0001" partyId="DemoCustomer"
>>>   roleTypeId="PLACING_CUSTOMER" />
>>>
>>> </entity-engine-xml>
>>>
>>> And the stack trace (the beginning) :
>>>
>>> 2009-05-31 15:54:22,417 (http-0.0.0.0-8443-1) [
>>> ControlServlet.java:204:ERROR]
>>> ---- exception report
>>> ----------------------------------------------------------
>>> Error in request handler:
>>> Exception: org.ofbiz.widget.screen.ScreenRenderException
>>> Message: Error rendering screen
>>> [component://order/widget/ordermgr/OrderViewScreens.xml#OrderHeaderView]:
>>> java.lang.NullPointerException (null)
>>> ---- cause
>>> ---------------------------------------------------------------------
>>> Exception: java.lang.NullPointerException
>>> Message: null
>>> ---- stack trace
>>> ---------------------------------------------------------------
>>> java.lang.NullPointerException
>>>
>>> org.codehaus.groovy.runtime.InvokerHelper.getProperty(InvokerHelper.java:178)
>>>
>>> org.codehaus.groovy.runtime.ScriptBytecodeAdapter.getProperty(ScriptBytecodeAdapter.java:477)
>>> OrderView.run(OrderView.groovy:380)
>>> org.ofbiz.base.util.GroovyUtil.runScriptAtLocation(GroovyUtil.java:117)
>>> ...
>>>
>>> Cimballi
>>>
>>>
>>> On Sun, May 31, 2009 at 5:28 AM, Divesh Dutta
>>> <divesh.dutta@hotwaxmedia.com> wrote:
>>>> +1
>>>>
>>>> Thanks
>>>> --
>>>> Divesh
>>>>
>>>>
>>>> Ray wrote:
>>>>> Lots of talk about different types of contributors etc and it should
be
>>>>> noted there are also lots of types of bugs.
>>>>>
>>>>> Your other posts highlight specific: do this, do that, it
>>>>> crashes/doesn't
>>>>> provide the expected result. That's helpful and will tend to get a
>>>>> response
>>>>> fairly quickly as they may not require as much time to verify and fix.
>>>>>
>>>>> This post is very different and could be titled "Potential bugs in
>>>>> OrderView.groovy". I don't think anybody involved in OFBiz can answer
>>>>> your
>>>>> post with out doing a full code review of the file.
>>>>>
>>>>> There are numerous possible reasons for including the first check and
>>>>> not
>>>>> the second, it depends on lots of things and the file is split in to
>>>>> several
>>>>> 'if' sections that may have a lot of impact on whether a productStore
is
>>>>> expected to be found. And in a file that is over 400 lines long it could
>>>>> take some effort to assess and justify the one thing you've highlighted
>>>>> before dealing with the "I didn't note all of them" others.
>>>>>
>>>>> I think with the:
>>>>>> If yes, why the first test ?
>>>>>> If no, there is missing a test in the second case.
>>>>> you might be over simplifying the problem as there are always other
>>>>> dependencies.
>>>>>
>>>>> So although it's a reasonable post to suggest there are problems in the
>>>>> file the difficulty is you need someone else to volunteer a reasonable
>>>>> amount of their own time to investigate, justify and fix a potential
>>>>> issue.
>>>>> It's basically a retrospective code review that will take a lot of
>>>>> effort,
>>>>> and carries it's own risks of introducing new problems.
>>>>>
>>>>> Generally code quality gets looked at when someone is working on a file.
>>>>>
>>>>> Don't take it personally on this post but I suspect you won't get
>>>>> someone
>>>>> jumping in and adding/removing a speculative 'if' wrapper as you are
>>>>> indirectly asking for quite a lot.
>>>>>
>>>>> On the other hand if you read the code and can produce a test case that
>>>>> triggers a null pointer exception on line 380....
>>>>>
>>>>> Ray
>>>>>
>>>>>
>>>>> Cimballi wrote:
>>>>>> Well, I don't know how to explain you, it seems evident to me...
>>>>>> In the first case you check if "productStore" is null or not. In
the
>>>>>> second case, you don't check.
>>>>>> So, what is the correct behaviour ? Should an order be linked to
a
>>>>>> productStore or not ?
>>>>>> If yes, why the first test ?
>>>>>> If no, there is missing a test in the second case.
>>>>>>
>>>>>>> From my point of view I would say no because an order with products
of
>>>>>> type "service" don't need productStore.
>>>>>>
>>>>>> To Scott : you should consider there are different kind of
>>>>>> contributors on open source projects, I'm the kind of contributor
who
>>>>>> send emails when I find something I think is a bug, I'm still not
in
>>>>>> the category of "patch providers" ! :-)
>>>>>>
>>>>>> Cimballi
>>>>>>
>>>>>>
>>>>>> On Fri, May 29, 2009 at 7:16 PM, Scott Gray
>>>>>> <scott.gray@hotwaxmedia.com>
>>>>>> wrote:
>>>>>>> Hi Cimballi
>>>>>>>
>>>>>>> Inconsistencies aren't necessarily bugs, but you are most welcome
to
>>>>>>> create
>>>>>>> a patch and jira issue for the corrections you think should be
made.
>>>>>>>
>>>>>>> Thanks
>>>>>>> Scott
>>>>>>>
>>>>>>> On 30/05/2009, at 10:39 AM, Cimballi wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> There are several inconsistancies in the file
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> "applications/order/webapp/ordermgr/WEB-INF/actions/order/OrderView.groovy".
>>>>>>>>
>>>>>>>> I didn't note all of them, but here is an example :
>>>>>>>>
>>>>>>>> Line 260, productStore can be null :
>>>>>>>>  productStore = orderHeader.getRelatedOne("ProductStore");
>>>>>>>>  if (productStore) {
>>>>>>>>     facility = productStore.getRelatedOne("Facility");
>>>>>>>>
>>>>>>>> Line 380, here productStore cannot be null :
>>>>>>>>  productStoreId =
>>>>>>>> orderHeader.getRelatedOne("ProductStore").productStoreId;
>>>>>>>>
>>>>>>>> Cimballi
>>>>
>>
> 

-- 
BJ Freeman
http://www.businessesnetwork.com/automation
http://bjfreeman.elance.com
http://www.linkedin.com/profile?viewProfile=&key=1237480&locale=en_US&trk=tab_pro
Systems Integrator.


Mime
View raw message