Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wsdl2apigee 69550284 v1 #16

Open
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

Abdul-Kinadiyil
Copy link

Fixed a few bugs in WSDL2Apigee tool and enhanced it to map WSDL constructs more natively to generated OAS spec and the Apigee API proxy.

Abdul-Kinadiyil and others added 26 commits May 22, 2018 12:53
…REST endpoints, send empty root element for GET operation to the SOAP backend service using an optional setting
> 1. Handle SOAP headers for http POST method endpoints
> 2. Mapping of XML Schema extension to JSON entities
> Item 1 requires additional flag to be set -allowSoapHeaders=true
> 1. Incorrect parameters for the OAS REST endpoint when corresponding WSDL operation has just the root element as input.
> 2. When XML data is converted to JSON, optional elements had incorrect default value as NULL string. Corresponding
> JSON attributes will be set to null.
> 1. Incorrect parameters for the OAS REST endpoint when corresponding WSDL operation has just the root element as input.
modified to enable  root element for response definitions in OAS , flag is showRespRootElement = true
modified to enable root element for response definitions in OAS
modified to resolve issue with nillable attribute when values are coming empty. ex:“@nil":true,"TEXT":null will be replaced with null.
added files to address when wsdl have nillable="true"
added java script in target preflow response to resolve nillable issue .
modified to add outer element for POST/PUT resources . flag changed to allowRespRootElement from showRespRootElement
modified to add newly added templates used to fix nillable issue
commented few test cases , need to analyze on that.
reverted commented test cases by fixing white space issue with xml comparisons
added xmlunit dependency to resolve white space issue with xml comparisons
modified to resolve empty headers from wsdl
modified to resolve issue around transformation of Array elements which have Complex and Simple types
added supporting methods to resolve issue with array elements transformation in Open API Spec
modified to resolve issue with transformation of child enum array which is of restriction type
required flag is set to true for attributes whose min and max occurs are 1 in WSDL
required flag is set to true for attributes whose min and max occurs are 1 in WSDL
modified to capture array instances and pass it to TreatAsArray option inside xmltojson policy. 
flag added is allowTreatAsArray=true
modified flag names as allowRespRootElement to enableRespRootElement and allowTreatAsArray - preserveArray 
allowSoapHeaders to enableSoapHeaders as per review comments
Code fix to handle classcast exception while parsing complexcontent
4 new flags have been added as part of wsdl2apigee  tool enhancements. mentioned those details in write-up. These flags are used while creating proxy and OAS from provided WSDL.
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

@Abdul-Kinadiyil
Copy link
Author

I am a Googler, I went through the process of configuring my github ID (abdul-kinadiyil). Let me know if I need to do anything else for this pull request

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

Copy link
Collaborator

@gideongoodwin gideongoodwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very difficult to verify the logic changes overall in this file.

parameter.addProperty("in", "query");
parameter.addProperty("required", false);
if(paramSplit.length>1) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consistent spacing please
if (paramSplit.length > 1) {
etc

same comment throughout this PR.

@@ -71,9 +71,15 @@ public static JsonArray getQueryParameters(ArrayList<String> queryParams) {
JsonObject parameter = null;
for (String queryParam : queryParams) {
parameter = new JsonObject();
parameter.addProperty("name", queryParam);
String paramSplit[] = queryParam.split("_");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if the query param name itself has an underscore in it? where is this behavior documented?

@@ -104,6 +110,90 @@ public static void addObject(JsonObject parent, String parentName, String object
properties.add(objectName, object);
}

public static void addObject(JsonObject parent, String parentName, String objectName,boolean isChildComplexType, String qNameLocal) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

param parentName is unused? can you give a more descriptive name for qNameLocal?

public static void addObject(JsonObject parent, String parentName, String objectName,boolean isChildComplexType, String qNameLocal) {
JsonObject properties = parent.getAsJsonObject("properties");
JsonObject object = new JsonObject();
if(isChildComplexType)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this logic correct? if !isChildComplexType, we're just adding an empty jsonObject to parent's properties?

* inorder to retrieve inner properties attribute to assign output elements
*
*/
public static void addObjectOutputRes(JsonObject parent, String parentName, String objectName,boolean isChildComplexType, String qNameLocal) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this function looks pretty close to the previous one, can we dedupe these and just have the caller pass in the appropriate parent to handle both cases?

required = "true";
}

queryParamStr = queryParamStr.concat(name+"_"+required);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why concat, can we just set queryParamStr's initial value here, or even just return name + "_" + required;

@@ -96,6 +96,9 @@
private static final List<String> primitiveTypes = Arrays.asList(new String[] { "int", "string", "boolean",
"decimal", "float", "double", "duration", "dateTime", "time", "date", "long", "gYearMonth", "gYear",
"gMonthDay", "gDay", "gMonth", "hexBinary", "base64Binary", "anyURI", "QName", "NOTATION" });
private static final List<String> primitiveArrayTypes = Arrays.asList(new String[] {"arrayofint","arrayoffloat","arrayofstring",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we not have an array of every primitive type?

@@ -213,6 +232,21 @@
private String oasContent;
// store openapi query params
private ArrayList<String> queryParams;

private static String soapHeader = "";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this static?

@@ -0,0 +1,40 @@
// js file used to traverse response content , find and replace nil:true
// ------------------------------------------------------------------
var what = Object.prototype.toString;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about using Array.isArray for an array check, and if that's false, you can just do Object.keys(obj) which will resolve to an empty array for non-object-like things. That way you don't have to rely on this toString behavior.

@@ -1,25 +1,21 @@
package com.apigee.proxywriter;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be really cool if you added some tests that verify the functionality you're fixing in this PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants