You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2021/08/29 06:14:23 UTC
[GitHub] [cxf] ellepsis opened a new pull request #839: CXF-8587: Don't check the validation flag and get the schema on every iteration for SoapHeader.
ellepsis opened a new pull request #839:
URL: https://github.com/apache/cxf/pull/839
It seems like we don't need to check the schema enabled flag and get schema on every iteration for a SoapHeder.
Removing these actions outside of the loop can provide a slight performance improvement if multiple headers are present in the request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on a change in pull request #839: CXF-8587: Don't check the validation flag and get the schema on every iteration for SoapHeader.
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #839:
URL: https://github.com/apache/cxf/pull/839#discussion_r698441977
##########
File path: rt/bindings/soap/src/main/java/org/apache/cxf/binding/soap/interceptor/SoapHeaderInterceptor.java
##########
@@ -107,20 +107,12 @@ public void handleMessage(Message m) throws Fault {
boolean supportsNode = this.supportsDataReader(message, Node.class);
Service service = ServiceModelUtil.getService(message.getExchange());
+
+ validateHeaders(message, headers, service);
+
for (SoapHeaderInfo header : headers) {
MessagePartInfo mpi = header.getPart();
- try {
- if (ServiceUtils.isSchemaValidationEnabled(SchemaValidationType.IN, message)) {
Review comment:
Thank you, @ellepsis!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta merged pull request #839: CXF-8587: Don't check the validation flag and get the schema on every iteration for SoapHeader.
Posted by GitBox <gi...@apache.org>.
reta merged pull request #839:
URL: https://github.com/apache/cxf/pull/839
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] ellepsis commented on a change in pull request #839: CXF-8587: Don't check the validation flag and get the schema on every iteration for SoapHeader.
Posted by GitBox <gi...@apache.org>.
ellepsis commented on a change in pull request #839:
URL: https://github.com/apache/cxf/pull/839#discussion_r698235537
##########
File path: rt/bindings/soap/src/main/java/org/apache/cxf/binding/soap/interceptor/SoapHeaderInterceptor.java
##########
@@ -107,20 +107,12 @@ public void handleMessage(Message m) throws Fault {
boolean supportsNode = this.supportsDataReader(message, Node.class);
Service service = ServiceModelUtil.getService(message.getExchange());
+
+ validateHeaders(message, headers, service);
+
for (SoapHeaderInfo header : headers) {
MessagePartInfo mpi = header.getPart();
- try {
- if (ServiceUtils.isSchemaValidationEnabled(SchemaValidationType.IN, message)) {
Review comment:
I think that it would not make almost any difference. We are not using any variables which are received in the main cycle (except `MessagePartInfo mpi = header.getPart();`, but it is just a getter call). Besides code style/structure, the only difference will be to get headers from `ArrayList`, which highly likely will be cached in a CPU cache. From another side we introducing additional checks for each iteration (hopefully will be optimized by JIT). But anyway I believe that difference is so small for time spend to further discussion)
I've pushed changes. Thank you for review!
Btw `schemaValidationEnabled && schema != null` could be simplified to `schema != null`. But I agree, that with the `schemaValidationEnabled` flag check, it's much more readable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on a change in pull request #839: CXF-8587: Don't check the validation flag and get the schema on every iteration for SoapHeader.
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #839:
URL: https://github.com/apache/cxf/pull/839#discussion_r698031709
##########
File path: rt/bindings/soap/src/main/java/org/apache/cxf/binding/soap/interceptor/SoapHeaderInterceptor.java
##########
@@ -107,20 +107,12 @@ public void handleMessage(Message m) throws Fault {
boolean supportsNode = this.supportsDataReader(message, Node.class);
Service service = ServiceModelUtil.getService(message.getExchange());
+
+ validateHeaders(message, headers, service);
+
for (SoapHeaderInfo header : headers) {
MessagePartInfo mpi = header.getPart();
- try {
- if (ServiceUtils.isSchemaValidationEnabled(SchemaValidationType.IN, message)) {
Review comment:
I agree with you that removing schema and validation flag check out of the loop does make sense, but you still have to go over each header and now we have to do this twice (in the loop and `validateHeaders`).
I would suggest a bit different change:
```
Schema schema = null;
final boolean schemaValidationEnabled = ServiceUtils.isSchemaValidationEnabled(SchemaValidationType.IN, message);
if (schemaValidationEnabled) {
schema = EndpointReferenceUtils.getSchema(service.getServiceInfos().get(0),
message.getExchange().getBus());
}
for (SoapHeaderInfo header : headers) {
MessagePartInfo mpi = header.getPart();
try {
if (schemaValidationEnabled && schema != null) {
validateHeader(message, mpi, schema);
}
} catch (Fault f) {
if (!isRequestor(message)) {
f.setFaultCode(Fault.FAULT_CODE_CLIENT);
}
throw f;
}
...
}
```
WDYT? Thank you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta merged pull request #839: CXF-8587: Don't check the validation flag and get the schema on every iteration for SoapHeader.
Posted by GitBox <gi...@apache.org>.
reta merged pull request #839:
URL: https://github.com/apache/cxf/pull/839
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org