Done with the suggested changes Scott. trunk at rev 1855898 release18.12 at rev 1855899 release17.12 at rev 1855900 release16.11 at rev 1855901
Thanks again for valuable inputs. - Best Regards, Swapnil M Mane, ofbiz.apache.org On Fri, Mar 15, 2019 at 9:52 AM Swapnil M Mane <[email protected]> wrote: > Makes perfect sense, thanks so much for the input Scott. > I will do the suggested changes. > > > - Best Regards, > Swapnil M Mane, > ofbiz.apache.org > > > > On Fri, Mar 15, 2019 at 5:36 AM Scott Gray <[email protected]> > wrote: > >> Hi Swapnil, >> >> A minor thing, but I wonder if it would be better to move this check >> underneath node.getTextContent()? If we assume that most nodes will >> contain text then it might be more efficient to do a null check after >> retrieving nodeValue. >> >> e.g. >> Node node = (Node) obj; >> String nodeValue = node.getTextContent(); >> if (nodeValue == null) { >> // Check node type and return obj; >> } >> if ("String".equals(type) || "java.lang.String".equals(type)) { >> return nodeValue; >> ... >> >> Regards >> Scott >> >> >> On Thu, 14 Mar 2019, 01:27 , <[email protected]> wrote: >> >> > Author: swapnilmmane >> > Date: Wed Mar 13 12:27:13 2019 >> > New Revision: 1855403 >> > >> > URL: http://svn.apache.org/viewvc?rev=1855403&view=rev >> > Log: >> > Fixed: simpleTypeConvert always returns Null for Document, Document Type >> > and Notation Node (OFBIZ-10832) >> > >> > As per the code, getTextContent() method is used get text content of the >> > node and its descendants but the node.getTextContent() always return >> Null >> > for the following Node type >> > DOCUMENT_NODE, DOCUMENT_TYPE_NODE, NOTATION_NODE [1] >> > >> > Since we can't get the text value of Document, Document Type and >> Notation >> > Node, thus simply return the same object. >> > >> > [1] >> > >> https://docs.oracle.com/javase/7/docs/api/org/w3c/dom/Node.html#getTextContent() >> > >> > Thanks: Deepak Dixit for reviewing the code. >> > >> > Modified: >> > >> > >> ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java >> > >> > Modified: >> > >> ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java >> > URL: >> > >> http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java?rev=1855403&r1=1855402&r2=1855403&view=diff >> > >> > >> ============================================================================== >> > --- >> > >> ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java >> > (original) >> > +++ >> > >> ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java >> > Wed Mar 13 12:27:13 2019 >> > @@ -289,6 +289,17 @@ public class ObjectType { >> > } >> > if (obj instanceof Node) { >> > Node node = (Node) obj; >> > + >> > + /* We can't get the text value of Document, Document Type >> and >> > Notation Node, >> > + * thus simply returning the same object from >> > simpleTypeOrObjectConvert method. >> > + * Please refer to OFBIZ-10832 Jira and >> > + * >> > >> https://docs.oracle.com/javase/7/docs/api/org/w3c/dom/Node.html#getTextContent() >> > for more details. >> > + */ >> > + short nodeType = node.getNodeType(); >> > + if (Node.DOCUMENT_NODE == nodeType || >> Node.DOCUMENT_TYPE_NODE >> > == nodeType || Node.NOTATION_NODE == nodeType) { >> > + return obj; >> > + } >> > + >> > String nodeValue = node.getTextContent(); >> > if ("String".equals(type) || >> "java.lang.String".equals(type)) >> > { >> > return nodeValue; >> > >> > >> > >> >
