HPCC-29554 Use ESDL generated service xml instead of hidl generated c++ code to build the xsds #17585

kenrowland · 2023-07-12T17:35:23Z

Converted form generation to use new schema generation using service
XML and XSLT transforms

Signed-Off-By: Kenneth Rowland [email protected]

Type of change:

This change is a bug fix (non-breaking change which fixes an issue).
This change is a new feature (non-breaking change which adds functionality).
This change improves the code (refactor or other change that does not change the functionality)
This change fixes warnings (the fix does not alter the functionality or the generated code)
This change is a breaking change (fix or feature that will cause existing behavior to change).
This change alters the query API (existing queries will have to be recompiled)

Checklist:

Smoketest:

Send notifications about my Pull Request position in Smoketest queue.
Test my draft Pull Request.

Testing:

github-actions · 2023-07-12T17:35:45Z

https://track.hpccsystems.com/browse/HPCC-29554
Jira updated

afishbeck

@kenrowland some comments about how transform parameters are handled.

afishbeck · 2023-07-13T18:28:50Z

esp/bindings/http/platform/httpbinding.cpp

+}
+
+void  EspHttpBinding::getServiceSchema(IEspContext& context, CHttpRequest* request, const char *serviceQName, const char *methodQName,
+                      Owned<IProperties> &params, double version, EsdlXslTypeId EsdlXslType, StringBuffer &schema)


I don't think params needs to be passed as Owned.
It's also kind of strange that you are modifying the params without knowing how else they might be used by the caller.

Technically it's clear the params are only for this function so looking further, I think it would be better not to pass in the params, but to build the params locally instead. Instead of passing the params maybe add a flags parameter and have flags for thins like "isWsdl" and "AllAnnotations" and based on that add those params here.

afishbeck · 2023-07-14T21:37:34Z

esp/bindings/http/platform/httpbinding.hpp

@@ -461,8 +461,18 @@ class esp_http_decl EspHttpBinding :
                            const char *serviceName, const char* methodName);
    void sortResponse(IEspContext& context, CHttpRequest* request,MemoryBuffer& contentconst,
                            const char *serviceName, const char* methodName);
+
+    enum SchemaType {


I'm not a huge fan of the internal function having to learn about the name and requirements of the external use case. I'd rather see flags where the caller can specify what features it needs.

"wsdl outupt", which wraps the schema in an extra layer and adds the location of the service.

"Add annotations", which adds additional meta data to the output.
etc.

In the end most of these use cases just need the schema to know the basic structure of the request. The use case name shouldn't be exposed to the internals.

kenrowland · 2023-07-18T14:15:45Z

@rpastrana
This PR represents the final replacement of generated XSD and WSDL outputs (forms, xml, json, soap, etc.) with XSLT transforms instead of hard coded output in HIDL generated code. Based on previous discussions of the types and the work to ensure they match previous outputs, Tony and I felt it would be a good idea for you to do any Java client testing that you have to make sure there are no problems.

rpastrana · 2023-07-18T14:21:37Z

@rpastrana This PR represents the final replacement of generated XSD and WSDL outputs (forms, xml, json, soap, etc.) with XSLT transforms instead of hard coded output in HIDL generated code. Based on previous discussions of the types and the work to ensure they match previous outputs, Tony and I felt it would be a good idea for you to do any Java client testing that you have to make sure there are no problems.

@kenrowland to ensure your changes don't break any downstream clients dependent on the ESP WSDLs, you should compare against legacy WSDLs, for instance here are the eclwatch wsdls used by hpcc4j: https://github.com/hpcc-systems/hpcc4j/tree/master/wsclient/src/main/resources/WSDLs

afishbeck · 2023-07-18T14:42:03Z

@kenrowland actually, the title of the PR/Commit should be descriptive of what is actually changing. Something like "Generate native ESP service forms at runtime".. or whatever actually makes sense.

asselitx · 2023-07-21T15:14:05Z

esp/services/ws_resources/CMakeLists.txt

@@ -41,6 +41,8 @@ include_directories (
         ${HPCC_SOURCE_DIR}/esp/bindings/SOAP/xpp
         ${HPCC_SOURCE_DIR}/esp/clients
         ${HPCC_SOURCE_DIR}/esp/smc/SMCLib
+         ${HPCC_SOURCE_DIR}/esp/esdllib
+         ${HPCC_SOURCE_DIR}/esp/esdllib


Same directory listed twice.

ghalliday · 2023-08-11T09:03:49Z

This seemed to be stalled in review/responding.

rpastrana · 2023-08-11T12:02:16Z

This seemed to be stalled in review/responding.

I'm actively helping Ken test WSDL backward compatibility in an interactive manner, eventually Ken's going to provide a running ESP we can target to automatically create all hpcc4j stub code based on the new WSDLs. BTW @kenrowland we don't have to wait for the 160 to have this change, can you provide a dockerhub image?

kenrowland · 2023-08-11T13:05:36Z

@rpastrana Once we are happy with the comparisons of the WSDLs provided to you, I will update the PR and provide a docker image for others to test.

rpastrana · 2023-08-11T15:41:13Z

@rpastrana Once we are happy with the comparisons of the WSDLs provided to you, I will update the PR and provide a docker image for others to test.

As expected, stub code generated fine with WSDL provided, but it seems there's potential for runtime behavior difference (default value directives being ignored).
If the proposed WSDLs no longer honor default value directives, I would ask that we rethink and continue to declare the default vals.

rpastrana · 2023-08-11T16:11:45Z

Based on offline discussion, latest change (honoring "default=0" directive) is fine.

esp/xslt/esp_service_xml2xsd.xslt

afishbeck

Please reverse the httpbinding.cpp header move to httpbinding.hpp and the cmake changes, unless you had a reason I'm not seeing.

Also please update the PR and commit titles to be a little clearer on what is being changed. Using ESDL generated metadata instead of hidl generated c++ code to build the xsd is much more important that "Improve XSD form generation" implies.

esp/bindings/http/platform/httpbinding.cpp

afishbeck

Some extra newlines otherwise looks fine.

afishbeck · 2023-08-28T20:10:13Z

esp/bindings/http/platform/httpbinding.cpp

+    schema.append("</xsd:schema>");
+
+    return true;
+


There's a lot of extra newlines here.

afishbeck · 2023-08-28T20:12:06Z

esp/bindings/http/platform/httpbinding.cpp

@@ -2078,11 +2085,30 @@ static void genSampleXml(StringStack& parent, IXmlType* type, StringBuffer& out,
    }
 }

+


Extra whitespace.

afishbeck · 2023-08-28T20:12:33Z

esp/bindings/http/platform/httpbinding.cpp

 }

+


One extra newline.

afishbeck · 2023-08-28T20:13:38Z

esp/bindings/http/platform/httpbinding.cpp

+        getSchema(schema,ctx,request,serviceQName,methodQName,false);
+    }
+
+


Extra newline.

Converted form generation to use new schema generation using service XML and XSLT transforms Added legacy schema generation to handle non SCM based ESP services that don't have a service XML Signed-Off-By: Kenneth Rowland [email protected]

kenrowland · 2023-08-29T12:44:54Z

@ghalliday Please merge

ghalliday · 2023-09-05T15:35:16Z

esp/bindings/http/platform/httpbinding.cpp

    rpcreq->serialize(*msg);

    StringBuffer req, tag, schema, filtered;
    msg->marshall(req, NULL);

-    getSchema(schema,ctx,request,serv,method,false);
-    getXMLMessageTag(ctx, true, method, tag);
+    if (!serviceXmlFilename.isEmpty())


minor: This block of code is repeated several times. It could be shared.

kenrowland requested a review from afishbeck July 12, 2023 18:06

kenrowland force-pushed the HPCC-29554 branch from 1254bc6 to f5cc2ad Compare July 13, 2023 17:31

afishbeck requested changes Jul 13, 2023

View reviewed changes

afishbeck reviewed Jul 14, 2023

View reviewed changes

kenrowland force-pushed the HPCC-29554 branch from 5f6e2cc to 54efb77 Compare July 17, 2023 20:29

kenrowland requested a review from afishbeck July 17, 2023 22:07

afishbeck approved these changes Jul 18, 2023

View reviewed changes

kenrowland requested review from afishbeck and rpastrana July 18, 2023 14:13

asselitx reviewed Jul 21, 2023

View reviewed changes

kenrowland requested a review from asselitx August 15, 2023 14:26

asselitx reviewed Aug 15, 2023

View reviewed changes

esp/xslt/esp_service_xml2xsd.xslt Show resolved Hide resolved

asselitx approved these changes Aug 15, 2023

View reviewed changes

afishbeck requested changes Aug 16, 2023

View reviewed changes

esp/bindings/http/platform/httpbinding.cpp Show resolved Hide resolved

kenrowland force-pushed the HPCC-29554 branch 2 times, most recently from 854a662 to 6695b0b Compare August 25, 2023 18:27

kenrowland requested a review from afishbeck August 28, 2023 13:44

afishbeck approved these changes Aug 28, 2023

View reviewed changes

HPCC-29554 Improve XSD form generation

9d4ae2d

Converted form generation to use new schema generation using service XML and XSLT transforms Added legacy schema generation to handle non SCM based ESP services that don't have a service XML Signed-Off-By: Kenneth Rowland [email protected]

kenrowland force-pushed the HPCC-29554 branch from 6695b0b to 9d4ae2d Compare August 29, 2023 12:41

kenrowland changed the title ~~HPCC-29554 Improve XSD form generation~~ HPCC-29554 Use ESDL generated service xml instead of hidl generated c++ code to build the xsds Aug 29, 2023

ghalliday reviewed Sep 5, 2023

View reviewed changes

ghalliday merged commit 0e139d8 into hpcc-systems:master Sep 5, 2023
50 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HPCC-29554 Use ESDL generated service xml instead of hidl generated c++ code to build the xsds #17585

HPCC-29554 Use ESDL generated service xml instead of hidl generated c++ code to build the xsds #17585

kenrowland commented Jul 12, 2023 •

edited

Loading

github-actions bot commented Jul 12, 2023

afishbeck left a comment

afishbeck Jul 13, 2023

afishbeck Jul 14, 2023 •

edited

Loading

kenrowland commented Jul 18, 2023

rpastrana commented Jul 18, 2023

afishbeck commented Jul 18, 2023

asselitx Jul 21, 2023

ghalliday commented Aug 11, 2023

rpastrana commented Aug 11, 2023

kenrowland commented Aug 11, 2023

rpastrana commented Aug 11, 2023

rpastrana commented Aug 11, 2023

afishbeck left a comment

afishbeck left a comment

afishbeck Aug 28, 2023

afishbeck Aug 28, 2023

afishbeck Aug 28, 2023

afishbeck Aug 28, 2023

kenrowland commented Aug 29, 2023

ghalliday Sep 5, 2023

		@@ -2078,11 +2085,30 @@ static void genSampleXml(StringStack& parent, IXmlType* type, StringBuffer& out,
		}
		}

		getSchema(schema,ctx,request,serviceQName,methodQName,false);
		}

HPCC-29554 Use ESDL generated service xml instead of hidl generated c++ code to build the xsds #17585

HPCC-29554 Use ESDL generated service xml instead of hidl generated c++ code to build the xsds #17585

Conversation

kenrowland commented Jul 12, 2023 • edited Loading

Type of change:

Checklist:

Smoketest:

Testing:

github-actions bot commented Jul 12, 2023

afishbeck left a comment

Choose a reason for hiding this comment

afishbeck Jul 13, 2023

Choose a reason for hiding this comment

afishbeck Jul 14, 2023 • edited Loading

Choose a reason for hiding this comment

kenrowland commented Jul 18, 2023

rpastrana commented Jul 18, 2023

afishbeck commented Jul 18, 2023

asselitx Jul 21, 2023

Choose a reason for hiding this comment

ghalliday commented Aug 11, 2023

rpastrana commented Aug 11, 2023

kenrowland commented Aug 11, 2023

rpastrana commented Aug 11, 2023

rpastrana commented Aug 11, 2023

afishbeck left a comment

Choose a reason for hiding this comment

afishbeck left a comment

Choose a reason for hiding this comment

afishbeck Aug 28, 2023

Choose a reason for hiding this comment

afishbeck Aug 28, 2023

Choose a reason for hiding this comment

afishbeck Aug 28, 2023

Choose a reason for hiding this comment

afishbeck Aug 28, 2023

Choose a reason for hiding this comment

kenrowland commented Aug 29, 2023

ghalliday Sep 5, 2023

Choose a reason for hiding this comment

kenrowland commented Jul 12, 2023 •

edited

Loading

afishbeck Jul 14, 2023 •

edited

Loading