Permalink
Browse files

Fix performance bug when converting POJO to xml (#968)

Fix bug where converting PJO to xml created too may calls to findChildSchemaObject and caused a significant performance hit.
See: CumberlandGroup/node-ews#58

Fix is for findChildSchemaObject to avoid recurring to irrelevant parts
of the schema.
  • Loading branch information...
1 parent 0d5a2ec commit 01d8585b9f33381821c5ba4e10acf3fb6ab98f19 @zarend zarend committed with jsdevel Sep 2, 2017
Showing with 130 additions and 2 deletions.
  1. +3 −1 lib/wsdl.js
  2. +32 −1 test/wsdl-test.js
  3. +20 −0 test/wsdl/perf/ns1.xsd
  4. +67 −0 test/wsdl/perf/order.wsdl
  5. +5 −0 test/wsdl/perf/request.json
  6. +3 −0 test/wsdl/perf/request.xml.js
View
@@ -2018,7 +2018,9 @@ WSDL.prototype.findChildSchemaObject = function(parameterTypeObj, childName, bac
}
var childNsURI;
- if (object.$type) {
+
+ // want to avoid unecessary recursion to improve performance
+ if (object.$type && backtrace.length === 1) {
var typeInfo = splitQName(object.$type);
if (typeInfo.prefix === TNS_PREFIX) {
childNsURI = parameterTypeObj.$targetNamespace;
View
@@ -2,7 +2,8 @@
var fs = require('fs'),
soap = require('..'),
- assert = require('assert');
+ assert = require('assert'),
+ sinon = require('sinon');
var wsdlStrictTests = {},
wsdlNonStrictTests = {};
@@ -110,6 +111,36 @@ wsdlStrictTests['should handle type ref'] = function(done) {
});
};
+wsdlStrictTests['should parse POJO into xml without making unnecessary recursion'] = function(done) {
+ var expectedMsg = require('./wsdl/perf/request.xml.js');
+ var reqJson = require('./wsdl/perf/request.json');
+ var spy = sinon.spy(soap.WSDL.prototype, "findChildSchemaObject");
+
+ soap.createClient(__dirname + '/wsdl/perf/order.wsdl', {strict: true}, function(err, client) {
+ var i, spyCall;
+
+ assert.ok(!err);
+ client.order(reqJson, function(err, result) {
+ assert.equal(client.lastMessage, expectedMsg);
+
+ // since the reqJson does not use the element named "thing", then findChildSchemaObject should never get to the type named RabbitHole
+ // see perf/ns1.xsd
+ // this tests the fix for the performance problem where we too many calls to findChildSchemaObject
+ // https://github.com/CumberlandGroup/node-ews/issues/58
+ assert.ok(spy.callCount);
+ for (i = 0; i < spy.callCount; i++) {
+ spyCall = spy.getCall(i);
+ if (spyCall.args[0]) {
+ assert.notEqual(spyCall.args[0].$type, "RabbitHole");
+ }
+ }
+
+ spy.restore();
+ done();
+ });
+ });
+};
+
wsdlStrictTests['should get empty namespace prefix'] = function(done) {
var expectedMsg = '<ns1:fooRq xmlns:ns1="http://example.com/bar/xsd"' +
' xmlns="http://example.com/bar/xsd"><bar1:paymentRq' +
View
@@ -0,0 +1,20 @@
+<xsd:schema targetNamespace="http://example.org/ns1"
+ elementFormDefault="qualified"
+ xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+ xmlns:ns1="http://example.org/ns1">
+ <xsd:element name="orderRq">
+ <xsd:complexType>
+ <xsd:sequence>
+ <xsd:element name="thing" type="OptionalThing"/>
+ <xsd:element name="item" type="xsd:string"/>
+ </xsd:sequence>
+ </xsd:complexType>
+ </xsd:element>
+ <xsd:complexType name="OptionalThing">
+ <xsd:sequence>
+ <xsd:element name="rabbitHole" type="RabbitHole"/>
+ </xsd:sequence>
+ </xsd:complexType>
+ <xsd:element name="RabbitHole" type="xsd:string"/>
+ <xsd:element name="orderRs" type="xsd:string"/>
+</xsd:schema>
View
@@ -0,0 +1,67 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<wsdl:definitions name="orderDef"
+ targetNamespace="http://example.org/wsdl"
+ xmlns:wsdl="http://schemas.xmlsoap.org/wsdl/"
+ xmlns:ns1="http://example.org/ns1"
+ xmlns:tns="http://example.org/wsdl"
+ xmlns:xsd="http://www.w3.org/2001/XMLSchema"
+ xmlns:soap="http://schemas.xmlsoap.org/wsdl/soap/">
+ <wsdl:types>
+ <xsd:schema>
+ <xsd:import
+ namespace="http://example.org/ns1"
+ schemaLocation="ns1.xsd"/>
+ <xsd:element name="orderRq">
+ <xsd:complexType>
+ <xsd:sequence>
+ <xsd:element name="thing" type="OptionalThing"/>
+ <xsd:element name="item" type="xsd:string"/>
+ </xsd:sequence>
+ </xsd:complexType>
+ </xsd:element>
+ <xsd:complexType name="OptionalThing">
+ <xsd:sequence>
+ <xsd:element name="rabbitHole" type="RabbitHole"/>
+ </xsd:sequence>
+ </xsd:complexType>
+ <xsd:element name="RabbitHole" type="xsd:string"/>
+ <xsd:element name="orderRs" type="xsd:string"/>
+ </xsd:schema>
+ </wsdl:types>
+ <wsdl:message name="orderRs">
+ <wsdl:part name="orderRs" element="ns1:orderRs">
+ </wsdl:part>
+ </wsdl:message>
+ <wsdl:message name="orderRq">
+ <wsdl:part name="orderRq" element="ns1:orderRq">
+ </wsdl:part>
+ </wsdl:message>
+ <wsdl:portType name="order_PortType">
+ <wsdl:operation name="order">
+ <wsdl:input message="tns:orderRq">
+ </wsdl:input>
+ <wsdl:output message="tns:orderRs">
+ </wsdl:output>
+ </wsdl:operation>
+ </wsdl:portType>
+ <wsdl:binding name="order_Binding" type="tns:order_PortType">
+ <soap:binding style="document"
+ transport="http://schemas.xmlsoap.org/soap/http"/>
+ <wsdl:operation name="order">
+ <soap:operation soapAction="order"/>
+ <wsdl:input>
+ <soap:body use="literal"/>
+ </wsdl:input>
+ <wsdl:output>
+ <soap:body use="literal"/>
+ </wsdl:output>
+ </wsdl:operation>
+ </wsdl:binding>
+ <wsdl:service name="order_Service">
+ <wsdl:port name="order_SoapPort"
+ binding="tns:order_Binding">
+ <soap:address
+ location="http://localhost:8888/mock_Order_Binding"/>
+ </wsdl:port>
+ </wsdl:service>
+</wsdl:definitions>
@@ -0,0 +1,5 @@
+{
+ "itemRq": {
+ "item": "bread"
+ }
+}
@@ -0,0 +1,3 @@
+module.exports = '<ns1:orderRq xmlns:ns1="http://example.org/ns1" xmlns="http://example.org/ns1">' +
+ '<ns1:itemRq><ns1:item>bread</ns1:item></ns1:itemRq>' +
+ '</ns1:orderRq>';

0 comments on commit 01d8585

Please sign in to comment.