OFBiz › OFBiz - Dev

Re: svn commit: r1855403 - /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

4 messages Options

Scott Gray-3

Re: svn commit: r1855403 - /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java

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 , <[hidden email]> 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;
>
>
>

Swapnil M Mane

Re: svn commit: r1855403 - /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java

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 <[hidden email]>
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 , <[hidden email]> 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;
> >
> >
> >
>

Swapnil M Mane

Re: svn commit: r1855403 - /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java

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 <[hidden email]>
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 <[hidden email]>
> 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 , <[hidden email]> 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;
>> >
>> >
>> >
>>
>

Scott Gray-3

Re: svn commit: r1855403 - /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/ObjectType.java

Nice one, thanks Swapnil!

Regards
Scott

On Thu, 21 Mar 2019, 01:25 Swapnil M Mane, <[hidden email]> wrote:

> 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 <[hidden email]>
> 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 <[hidden email]
> >
> > 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 , <[hidden email]> 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;
> >> >
> >> >
> >> >
> >>
> >
>