HPCC-31936 WsEcl unable to get sample request or response XML #18728
Conversation
| getServiceSchema(context, request, serviceQName, methodQName, | ||
| version, isWsdl, false, schema); | ||
| // Allow derived classes such as WsEcl to use custom getSchema() logic when there is no serviceXmlFilename | ||
| StringBuffer serviceXmlFilename; |
There was a problem hiding this comment.
Here is a concern I have. This does a copy of the serviceXmlFile name, and if present, calls the getServiceSchema method. That method does another copy of the service XML filename. Perhaps there could be a way to avoid two copies and add a "hasSerivceXML" type method. I don't think it practical to pass the service XML filename as a parameter.
If you feel this is valid, let's open a Jira to make that change and add this one as a dependency. It can be done afterwards. It would require a change to HIDL probably. I don't mind making the change.
As part of that change, the XML filename could be cached, but I don't think that would gain too much.
| @@ -200,6 +203,7 @@ class CWsEclBinding : public CHttpSoapBinding | |||
|
|
|||
| int getXsdDefinition(IEspContext &context, CHttpRequest *request, StringBuffer &content, WsEclWuInfo &wsinfo); | |||
| bool getSchema(StringBuffer& schema, IEspContext &ctx, CHttpRequest* req, WsEclWuInfo &wsinfo) ; | |||
There was a problem hiding this comment.
I would consider renaming this method and moving it to be a protected member, unless there is another component that is calling it.
|
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-31936 Jirabot Action Result: |
|
Another possibility is to implement this fix only in getServiceSchema. If the call to get the service xml filename fails, then call getSchema and return. If the subclass for the binding, as will Ws_ECL, has a getSchema method, it is handled as required. If not, an error is thrown by the base class implementation. This gets rid of the two copies. Thoughts? |
That seems like it might be cleaner, and I can't think of drawbacks. I'll make and test those two changes and re-push. |
| @@ -140,6 +140,12 @@ class CWsEclBinding : public CHttpSoapBinding | |||
| private: | |||
| CWsEclService *wsecl; | |||
|
|
|||
| protected: | |||
| bool getSchema(StringBuffer& schema, IEspContext &ctx, CHttpRequest* req, const char *service, const char *method,bool standalone) override; | |||
There was a problem hiding this comment.
Was there any specific reason this was moved to being protected from public?
There was a problem hiding this comment.
This function with the final bool parameter wasn't overridden in this class before, but it is/was protected in the base class EspHttpBinding- I think it just looks confusing because I renamed & moved the other one.
The other getSchema function (with the final wuinfo parameter) I moved from public to protected in this class and renamed to getSimpleSchema (on your suggestion- seemed reasonable especially now that it makes sense to refactor it away in favor of the other one).
asselitx
force-pushed
the
samplexml-hpcc-31936
branch
from
f5bf94d to
fa7571d
Compare
|
@ghalliday squashed and approved for merge |
|
Jira Issue: https://hpccsystems.atlassian.net//browse/HPCC-31936 Jirabot Action Result: |
Adjust EspHttpBinding and CWsEclBinding to use info from the roxie to generate WSDL/XSD and sample request/response XML rather than attempting to use ESDL. This means moving the old implmenetation of several functions from prior to HPCC-28978 from EspHttpBinding into CWsEclBinding. Specifically: - getSchema is overridden in CWsEclBinding, and called when the ESDL-based generation isn't applicable (there is no xmlServiceFileName). - getWsdlOrXsd is moved into CWsEclBinding, and the onXSD/onWsdl handlers in CWsEclBinding call it directly. Noted that the non-overridden CWsEclBinding member function getSchema was substantially similar to the overridden version, so renamed it getSimpleSchema and made it protected. Created ticket HPCC-32016 to refactor that code. Signed-off-by: Terrence Asselin <[email protected]>
asselitx
force-pushed
the
samplexml-hpcc-31936
branch
from
fa7571d to
17b49b1
Compare
|
Failed job is vcpkg related- downloading package for Windows build. Re-run didn't fix, so rebased and pushed. |
ghalliday
merged commit 58aa3e6
into
hpcc-systems:candidate-9.6.x
Adjust EspHttpBinding and CWsEclBinding to use info from the roxie to generate WSDL/XSD and sample request/response XML, rather than attempting to use ESDL files.
This means moving the old implmenetation of "getSchema" from EspHttpBinding into CWsEclBinding, and allowing the WsEcl override to be called when the ESDL-based generation isn't applicable (there is no xmlServiceFileName).
Type of change:
Checklist:
Smoketest:
Testing:
Tested through ECLWatch UI on local bare metal deployment. Exercised every interface element that seemed likely to be affected and found that wsdl/xsd generation also needed to be fixed. Though valgrind found a likely out-of-bounds string access error in ws_ecl that has a ticket HPCC-31996.