citrusframework / citrus

Framework for automated integration tests with focus on messaging integration
https://citrusframework.org
Apache License 2.0
458 stars 136 forks source link

Custom SqlResultSetScriptValidator for validating with callback #619

Open mariodavid opened 5 years ago

mariodavid commented 5 years ago

Citrus Version 2.8.0

Question I would like to understand what is the default way of using the SQL validation with the Java DSL. Additionally I would like to have type ensurance of the result set (with POJOs). Therefore I wanted not to do a groovy based validation script. Instead I decided to use the custom validator approach (see: https://github.com/citrusframework/citrus/blob/citrus-2.8.0/modules/citrus-java-dsl/src/main/java/com/consol/citrus/dsl/builder/ExecuteSQLQueryBuilder.java#L240) via the following snippet:

designer
        .query(myDbDatasource)
        .sqlResource(resource)
        .validator(new SqlResultSetScriptValidator() {
          @Override
          public void validateSqlResultSet(List<Map<String, Object>> resultSet,
              ScriptValidationContext validationContext, TestContext context)
              throws ValidationException {
            assert false;
          }
        });

What I've tried so far Unfortunately this does not work, where work in this case means: the test are green, which should not be the case because of the assert false. Instead what works is the following snippet:

designer
        .query(myDbDatasource)
        .sqlResource(resource)
        .validateScript("assert true == true", "groovy")
        .validator(new SqlResultSetScriptValidator() {
          @Override
          public void validateSqlResultSet(List<Map<String, Object>> resultSet,
              ScriptValidationContext validationContext, TestContext context)
              throws ValidationException {
            assert false;
          }
        });

It seems that I either don't fully understand the usage of the validator() method, or there is some hidden dependency that the validator is only used in the context of the script validation. Perhaps I'm also missing the point: What is the default way of programmatically validate the result set (ideally not only with type List<Map<String, Object>> but rather as it can be defined in the HttpClientResponseActionBuilder with the validationCallback like this:

.validationCallback(new JsonMappingValidationCallback<MyJsonResultPojo>(MyJsonResultPojo.class, objectMapper) {
          @Override
          public void validate(
              MyJsonResultPojo myJsonResultPojo,
              Map<String, Object> map,
              TestContext testContext
          ) {
            assertThat(myJsonResultPojo.getMyAttribute()).isEqualTo(123);
          }
        });

I mean I could do it with groovy (and I love groovy :)) but honestly it is super hard to debug when the execution of the script is somewhere hidden within the framework. Therefore I would prefer proper Java POJOs

Additional information The identified piece of code that causes this situation is here: https://github.com/citrusframework/citrus/blob/citrus-2.8.0/modules/citrus-core/src/main/java/com/consol/citrus/actions/ExecuteSQLQueryAction.java#L242

Thanks & Bye, Mario

svettwer commented 5 years ago

Hi!

Thank you for the detailed explanation and the refinement. :+1: It seems that you've found a bug, unfortunately. As you pointed out, the ExecuteSQLQueryAction checks the existence of a scriptValidationContext within the action. This context is only created while using the validateScript method. As a workaround, I recommend to leave the .validateScript("assert true == true", "groovy") to trigger the SqlResultSetScriptValidator execution until this is fixed, just as you already did.

Concerning the result set Type: The List<Map<String, Object>> structure returning the results is exactly what the spring jdbc template returns on a request. Citrus does not provide an automated mapping currently but you could add a conversion into a POJO to the SqlResultSetScriptValidator implementation as a first step. If you are interested in such a feature performing an automated conversion, please open another issue with a feature requests.

BR, Sven

svettwer commented 5 years ago

BTW: As it seems that you're using the Designer, right? We're currently switching to the Runner as it provides a lot of advantages compared to the Designer. I would recommend to write new tests using the runner and start migrating the Designer-Tests in 2019, as the Designer will be removed by the end of the year.

We're currently switching from the Designer to the Runner in the samples to prevent any inconveniences, but this has not been finished yet. For more information, please have a look at the Roadmap Blogpost.

BR, Sven

mariodavid commented 5 years ago

Hi,

thanks for the answer. I'll keep the .validateScript("assert true == true", "groovy") hack for now then.

Do you have any documentation about the Runner version available besides the annoucement in the blog?

One addition: implementing SqlResultSetScriptValidator seems to be a little bit misleaded in our case - because we are not doing scripting at all. Perhaps it would be a good idea to have another interface SqlResultSetValidator that should be implemented in this case?

Bye Mario

mariodavid commented 5 years ago

BTW: isn't that a common thing to do these kinds of assertions directly in java code? Or is the main use case really checking the SQL via SQL expectation files or groovy code? I had the impression that doing it in java code is the most natural choice, since it is a java / junit / cucumber thing anyways - right?

I actually had the same question back when I used citrus a year ago in a SOAP checking environment.

svettwer commented 5 years ago

Hi!

Do you have any documentation about the Runner version available besides the annoucement in the blog?

Please have a look at the manuals java-dsl-test-runner section. In addition there are some syntax samples provided as in the sending messages section. We're also going to review the documentation this year to provide more examples.

One addition: implementing SqlResultSetScriptValidator seems to be a little bit misleaded in our case - because we are not doing scripting at all. Perhaps it would be a good idea to have another interface SqlResultSetValidator that should be implemented in this case?

We should think about that, yes.

BTW: isn't that a common thing to do these kinds of assertions directly in java code? Or is the main use case really checking the SQL via SQL expectation files or groovy code? I had the impression that doing it in java code is the most natural choice, since it is a java / junit / cucumber thing anyways - right?

Short answer: It depends! :wink: But from what I've seen so far, a lot of validation is done with groovy.

BR, Sven

erikonthenet commented 1 year ago

Hi, I was about to file a bug report, but this issue seems to be exactly describe my problem, except I'm using citrus 3.4.0. In the above example, it is recommended to use .validateScript("assert true == true", "groovy") in combination with .validator(). When using citrus 2.8.0 we had another workaround, the setter method:

testRunner.query(query -> {
    query.dataSource(dataSource)
        .statement(queryString)
        .validator((resultSet, validationContext, context) -> {
            // custom validation
        })
        .build()
        .setScriptValidationContext(
            new ScriptValidationContext("Custom"));
});

At the moment, we're migrating to citrus 3.4.0 and the setter method no longer exists. Simply removing this method made the code compile again, but the validator didn't run anymore. So I found that adding .validateScript("assert rows.size() == 1", "groovy") would result in the validator being run again, just as described above by @svettwer .

By testing some more, it became clear that script/type passed to validateScript() is not evaluated, assert rows.size() == 2 also works for 1 result. At last, passing empty strings to the .validateScript() method also works. So the difference in using the validateScript method is that this method adds the ScriptValidationContext, and the passed script/type seem to be ignored when also passing a validator().

My working example in Citrus 3.4.0:

testRunner.run(
    query(dataSource)
        .statement(queryString)
        .validateScript("", "")
        .validator((resultSet, validationContext, context) -> {
            // custom validation
        }));