You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/08/03 07:49:53 UTC
[GitHub] [camel-quarkus] svkcemk opened a new pull request, #3953: Increase test coverage (file/http) for camel-quarkus-validator extens…
svkcemk opened a new pull request, #3953:
URL: https://github.com/apache/camel-quarkus/pull/3953
…ion.
<!-- Uncomment and fill this section if your PR is not trivial
[ ] An issue should be filed for the change unless this is a trivial change (fixing a typo or similar). One issue should ideally be fixed by not more than one commit and the other way round, each commit should fix just one issue, without pulling in other changes.
[ ] Each commit in the pull request should have a meaningful and properly spelled subject line and body. Copying the title of the associated issue is typically enough. Please include the issue number in the commit message prefixed by #.
[ ] The pull request description should explain what the pull request does, how, and why. If the info is available in the associated issue or some other external document, a link is enough.
[ ] Phrases like Fix #<issueNumber> or Fixes #<issueNumber> will auto-close the named issue upon merging the pull request. Using them is typically a good idea.
[ ] Please run mvn process-resources -Pformat (and amend the changes if necessary) before sending the pull request.
[ ] Contributor guide is your good friend: https://camel.apache.org/camel-quarkus/latest/contributor-guide.html
-->
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] ppalaga commented on a diff in pull request #3953: Increase test coverage (file/http) for camel-quarkus-validator extens…
Posted by GitBox <gi...@apache.org>.
ppalaga commented on code in PR #3953:
URL: https://github.com/apache/camel-quarkus/pull/3953#discussion_r936617797
##########
integration-tests/validator/src/main/resources/application.properties:
##########
@@ -15,4 +15,5 @@
## limitations under the License.
## ---------------------------------------------------------------------------
-quarkus.native.additional-build-args =-H:ResourceConfigurationFiles=resources-config.json
\ No newline at end of file
+quarkus.native.additional-build-args =-H:ResourceConfigurationFiles=resources-config.json
+quarkus.ssl.native=true
Review Comment:
I see that this is needed for accessing the GitHub https URL. But I wonder how should we help end users to get around this. Maybe we should just document it in `usage.adoc`, something like
```
If you are accessing XSD resources via HTTPS (e.g. `from(...).to("validator:https://acme.org/foo/schema.xsd")`) than you need to add `quarkus.ssl.native = true` to your `application.properties`.
```
Hardcoding SSL support (like we do in some other extensions) seems to be an overkill in this case, IMO. I guess users will mostly use classpath resources. WDYT @jamesnetherton?
##########
integration-tests/validator/src/main/resources/application.properties:
##########
@@ -15,4 +15,5 @@
## limitations under the License.
## ---------------------------------------------------------------------------
-quarkus.native.additional-build-args =-H:ResourceConfigurationFiles=resources-config.json
\ No newline at end of file
+quarkus.native.additional-build-args =-H:ResourceConfigurationFiles=resources-config.json
Review Comment:
I see that this is an old code, but we could perhaps use this occasion to change it towards Quarkus way of doing things:
```suggestion
quarkus.native.resources.includes = *.xsd
```
##########
integration-tests/validator/src/main/java/org/apache/camel/quarkus/component/validator/it/ValidatorRouteBuilder.java:
##########
@@ -21,7 +21,14 @@
public class ValidatorRouteBuilder extends RouteBuilder {
@Override
public void configure() {
- from("direct:start")
+ from("direct:classpath")
.to("validator:message.xsd");
+
+ from("direct:filesystem")
+ .to("validator:file:src/main/resources/message.xsd");
+
+ from("direct:http")
+ .toD("validator:https://raw.githubusercontent.com/apache/camel-quarkus/main/integration-tests/validator/src/main/resources/message.xsd");
Review Comment:
This might be risky for several reasons:
* If we change message.xsd in the future, the tests in older branches may start failing
* GitHub might be inaccessible in some environments
* GitHub's availability will impact the reliability of this test
I'd vote for serving message.xsd from an ad hoc WireMock server. There are plenty of examples in the source tree.
##########
integration-tests/validator/src/test/java/org/apache/camel/quarkus/component/validator/it/ValidatorTest.java:
##########
@@ -25,26 +25,75 @@
class ValidatorTest {
@Test
- public void validXML() {
+ public void validXMLFromClassPath() {
RestAssured.given()
.contentType(ContentType.XML)
.body("<message><firstName>MyFirstname</firstName><lastName>MyLastname</lastName></message>")
- .post("/validator/xml")
+ .post("/validator/classpath")
.then()
.statusCode(200);
}
Review Comment:
I'd vote for having one parameterized test for the valid case and one for invalid case. The params being `"classpath", "filesystem", "http"`. Just grep for `@ParameterizedTest` in the source tree to see some examples.
Less code is perhaps easier to maintain.
##########
integration-tests/validator/src/main/java/org/apache/camel/quarkus/component/validator/it/ValidatorResource.java:
##########
@@ -32,11 +32,28 @@ public class ValidatorResource {
@Inject
ProducerTemplate producerTemplate;
- @Path("/xml")
+ @Path("/classpath")
@POST
@Consumes(MediaType.APPLICATION_XML)
@Produces(MediaType.TEXT_PLAIN)
- public String processOrder(String statement) {
- return producerTemplate.requestBody("direct:start", statement, String.class);
+ public String processOrderInXmlFromClassPath(String statement) {
+ return producerTemplate.requestBody("direct:classpath", statement, String.class);
}
+
+ @Path("/filesystem")
+ @POST
+ @Consumes(MediaType.APPLICATION_XML)
+ @Produces(MediaType.TEXT_PLAIN)
+ public String processOrderInXmlFromFileSystem(String statement) {
+ return producerTemplate.requestBody("direct:filesystem", statement, String.class);
+ }
+
+ @Path("/http")
+ @POST
+ @Consumes(MediaType.APPLICATION_XML)
+ @Produces(MediaType.TEXT_PLAIN)
+ public String processOrderInXmlFromHTTP(String statement) {
+ return producerTemplate.requestBody("direct:http", statement, String.class);
+ }
Review Comment:
I'd vote for having just one parameterized endpoint instead of three non-parameterized. Something like
```
@Path("/validate/{scheme}")
@POST
@Consumes(MediaType.APPLICATION_XML)
@Produces(MediaType.TEXT_PLAIN)
public String processOrderInXmlFromFileSystem(String statement, @PathParam("scheme") String scheme) {
return producerTemplate.requestBody("direct:" + scheme, statement, String.class);
}
```
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zhfeng commented on a diff in pull request #3953: Increase test coverage (file/http) for camel-quarkus-validator extens…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on code in PR #3953:
URL: https://github.com/apache/camel-quarkus/pull/3953#discussion_r936408330
##########
integration-tests/validator/src/main/java/org/apache/camel/quarkus/component/validator/it/ValidatorResource.java:
##########
@@ -32,11 +32,28 @@ public class ValidatorResource {
@Inject
ProducerTemplate producerTemplate;
- @Path("/xml")
+ @Path("/classpath")
@POST
@Consumes(MediaType.APPLICATION_XML)
@Produces(MediaType.TEXT_PLAIN)
- public String processOrder(String statement) {
- return producerTemplate.requestBody("direct:start", statement, String.class);
+ public String processOrderInXmlFromClassPath(String statement) {
+ return producerTemplate.requestBody("direct:classpath", statement, String.class);
}
+
+ @Path("/filesystem")
+ @POST
+ @Consumes(MediaType.APPLICATION_XML)
+ @Produces(MediaType.TEXT_PLAIN)
+ public String processOrderInXmlFromFileSystem(String statement) {
+ return producerTemplate.requestBody("direct:filesystem", statement, String.class);
+ }
+
+ @Path("/http")
+ @POST
+ @Consumes(MediaType.APPLICATION_XML)
+ @Produces(MediaType.TEXT_PLAIN)
+ public String processOrderInXmlFromHTTP(String statement) {
+ return producerTemplate.requestBody("direct:http", statement, String.class);
+ }
Review Comment:
This test is working in the native mode?
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zhfeng merged pull request #3953: Increase test coverage (file/http) for camel-quarkus-validator extens…
Posted by GitBox <gi...@apache.org>.
zhfeng merged PR #3953:
URL: https://github.com/apache/camel-quarkus/pull/3953
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] zhfeng commented on pull request #3953: Increase test coverage (file/http) for camel-quarkus-validator extens…
Posted by GitBox <gi...@apache.org>.
zhfeng commented on PR #3953:
URL: https://github.com/apache/camel-quarkus/pull/3953#issuecomment-1203724269
Thanks @svkcemk for your contribution!
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] svkcemk commented on a diff in pull request #3953: Increase test coverage (file/http) for camel-quarkus-validator extens…
Posted by GitBox <gi...@apache.org>.
svkcemk commented on code in PR #3953:
URL: https://github.com/apache/camel-quarkus/pull/3953#discussion_r936435943
##########
integration-tests/validator/src/main/java/org/apache/camel/quarkus/component/validator/it/ValidatorResource.java:
##########
@@ -32,11 +32,28 @@ public class ValidatorResource {
@Inject
ProducerTemplate producerTemplate;
- @Path("/xml")
+ @Path("/classpath")
@POST
@Consumes(MediaType.APPLICATION_XML)
@Produces(MediaType.TEXT_PLAIN)
- public String processOrder(String statement) {
- return producerTemplate.requestBody("direct:start", statement, String.class);
+ public String processOrderInXmlFromClassPath(String statement) {
+ return producerTemplate.requestBody("direct:classpath", statement, String.class);
}
+
+ @Path("/filesystem")
+ @POST
+ @Consumes(MediaType.APPLICATION_XML)
+ @Produces(MediaType.TEXT_PLAIN)
+ public String processOrderInXmlFromFileSystem(String statement) {
+ return producerTemplate.requestBody("direct:filesystem", statement, String.class);
+ }
+
+ @Path("/http")
+ @POST
+ @Consumes(MediaType.APPLICATION_XML)
+ @Produces(MediaType.TEXT_PLAIN)
+ public String processOrderInXmlFromHTTP(String statement) {
+ return producerTemplate.requestBody("direct:http", statement, String.class);
+ }
Review Comment:
Sorry my mistake you are right the http one is failing I can see that now, thanks for pointing out.
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] svkcemk commented on a diff in pull request #3953: Increase test coverage (file/http) for camel-quarkus-validator extens…
Posted by GitBox <gi...@apache.org>.
svkcemk commented on code in PR #3953:
URL: https://github.com/apache/camel-quarkus/pull/3953#discussion_r936451639
##########
integration-tests/validator/src/main/java/org/apache/camel/quarkus/component/validator/it/ValidatorResource.java:
##########
@@ -32,11 +32,28 @@ public class ValidatorResource {
@Inject
ProducerTemplate producerTemplate;
- @Path("/xml")
+ @Path("/classpath")
@POST
@Consumes(MediaType.APPLICATION_XML)
@Produces(MediaType.TEXT_PLAIN)
- public String processOrder(String statement) {
- return producerTemplate.requestBody("direct:start", statement, String.class);
+ public String processOrderInXmlFromClassPath(String statement) {
+ return producerTemplate.requestBody("direct:classpath", statement, String.class);
}
+
+ @Path("/filesystem")
+ @POST
+ @Consumes(MediaType.APPLICATION_XML)
+ @Produces(MediaType.TEXT_PLAIN)
+ public String processOrderInXmlFromFileSystem(String statement) {
+ return producerTemplate.requestBody("direct:filesystem", statement, String.class);
+ }
+
+ @Path("/http")
+ @POST
+ @Consumes(MediaType.APPLICATION_XML)
+ @Produces(MediaType.TEXT_PLAIN)
+ public String processOrderInXmlFromHTTP(String statement) {
+ return producerTemplate.requestBody("direct:http", statement, String.class);
+ }
Review Comment:
Added `quarkus.ssl.native=true` for https endpoint, now tests are working in native mode as well, thanks @zhfeng
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [camel-quarkus] svkcemk commented on a diff in pull request #3953: Increase test coverage (file/http) for camel-quarkus-validator extens…
Posted by GitBox <gi...@apache.org>.
svkcemk commented on code in PR #3953:
URL: https://github.com/apache/camel-quarkus/pull/3953#discussion_r936430928
##########
integration-tests/validator/src/main/java/org/apache/camel/quarkus/component/validator/it/ValidatorResource.java:
##########
@@ -32,11 +32,28 @@ public class ValidatorResource {
@Inject
ProducerTemplate producerTemplate;
- @Path("/xml")
+ @Path("/classpath")
@POST
@Consumes(MediaType.APPLICATION_XML)
@Produces(MediaType.TEXT_PLAIN)
- public String processOrder(String statement) {
- return producerTemplate.requestBody("direct:start", statement, String.class);
+ public String processOrderInXmlFromClassPath(String statement) {
+ return producerTemplate.requestBody("direct:classpath", statement, String.class);
}
+
+ @Path("/filesystem")
+ @POST
+ @Consumes(MediaType.APPLICATION_XML)
+ @Produces(MediaType.TEXT_PLAIN)
+ public String processOrderInXmlFromFileSystem(String statement) {
+ return producerTemplate.requestBody("direct:filesystem", statement, String.class);
+ }
+
+ @Path("/http")
+ @POST
+ @Consumes(MediaType.APPLICATION_XML)
+ @Produces(MediaType.TEXT_PLAIN)
+ public String processOrderInXmlFromHTTP(String statement) {
+ return producerTemplate.requestBody("direct:http", statement, String.class);
+ }
Review Comment:
@zhfeng yes I tested with `mvn clean install -Pnative` as well and not seeing any failures on native mode. Thanks!
--
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: commits-unsubscribe@camel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org