New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issue/821/access variable by xpath json #827

Merged

christophd merged 5 commits into citrusframework:main from postfinance:issue/821/access-variable-by-xpath-json

Jan 25, 2022

Collaborator

tschlat commented Dec 16, 2021 •

edited

Loading

Added the feature requested with issue #821.

I have implemented the feature into the changes of #820.

The code got more complex than I thought it would get. The reason is the separation of JsonPath/Xpath functionality into different modules. So I wasn't able to access the JSonPath/Xpath utillities from the api-module that hosts the TestContext. I had to decouple the extraction mechanism for JsonPath and Xpath into the modules. Although there is a bunch of new files, the implemented approach pretty much follows the decoupling mechanisms already used thought the framework.

There is one toDo in com.consol.citrus.xml.XpathSegmentVariableExtractor for which I request advice from you @christophd.

tschlat mentioned this pull request

Issue/820/allow access to indexed variables #826

Closed

bbortt force-pushed the issue/821/access-variable-by-xpath-json branch from 203527e to def6130 Compare

December 16, 2021 15:42

bbortt requested changes

View reviewed changes

core/citrus-api/src/main/java/com/consol/citrus/variable/SegmentVariableExtractorRegistry.java Outdated Show resolved Hide resolved

core/citrus-api/src/main/java/com/consol/citrus/variable/SegmentVariableExtractor.java Outdated Show resolved Hide resolved

core/citrus-api/src/main/java/com/consol/citrus/variable/VariableExpressionSegmentMatcher.java Outdated Show resolved Hide resolved

...ion-json/src/test/java/com/consol/citrus/integration/JsonPathSegmentVariableExtractorIT.java

+              public class JsonPathSegmentVariableExtractorIT extends TestNGCitrusSpringSupport {
+                  @Test
+                  @CitrusXmlTest

Collaborator

bbortt Dec 16, 2021

I just had an idea.. could we have the @Ignore of the "no empty test methods" sonar rule on the @CitrusXmlTest annotation? :D

...citrus-validation-xml/src/main/java/com/consol/citrus/xml/XpathSegmentVariableExtractor.java Show resolved Hide resolved

...citrus-validation-xml/src/main/java/com/consol/citrus/xml/XpathSegmentVariableExtractor.java Outdated Show resolved Hide resolved

...citrus-validation-xml/src/main/java/com/consol/citrus/xml/XpathSegmentVariableExtractor.java Outdated Show resolved Hide resolved

Collaborator

bbortt commented Dec 16, 2021

@tschlat pretty cool enhancement! but the build didn't like your changes..

tschlat commented

View reviewed changes

Collaborator Author

tschlat left a comment

Included feedback of bbortt.

...citrus-validation-xml/src/main/java/com/consol/citrus/xml/XpathSegmentVariableExtractor.java Outdated Show resolved Hide resolved

core/citrus-api/src/main/java/com/consol/citrus/variable/SegmentVariableExtractor.java Outdated Show resolved Hide resolved

core/citrus-api/src/main/java/com/consol/citrus/variable/VariableExpressionSegmentMatcher.java Outdated Show resolved Hide resolved

...citrus-validation-xml/src/main/java/com/consol/citrus/xml/XpathSegmentVariableExtractor.java Show resolved Hide resolved

...citrus-validation-xml/src/main/java/com/consol/citrus/xml/XpathSegmentVariableExtractor.java Outdated Show resolved Hide resolved

christophd reviewed

View reviewed changes

Member

christophd left a comment

This is really a great enhancement! Many thanks! I like the configuration via resource lookup, well done!

I'd just argue the document caching mechanism and we need to also add this to Spring based configuration

...itrus-api/src/test/java/com/consol/citrus/variable/VariableExpressionSegmentMatcherTest.java Outdated Show resolved Hide resolved

...citrus-validation-xml/src/main/java/com/consol/citrus/xml/XpathSegmentVariableExtractor.java Outdated Show resolved Hide resolved

...citrus-validation-xml/src/main/java/com/consol/citrus/xml/XpathSegmentVariableExtractor.java Outdated Show resolved Hide resolved

core/citrus-base/src/main/java/com/consol/citrus/context/TestContextFactory.java Show resolved Hide resolved

tschlat force-pushed the issue/821/access-variable-by-xpath-json branch 2 times, most recently from 0561a18 to cb553a6 Compare

December 20, 2021 09:36

Collaborator Author

tschlat commented Dec 20, 2021

I have added a commit with the needed changes identified by @christophd

tschlat commented

View reviewed changes

Collaborator Author

tschlat left a comment

I have added fixes for your review @christophd

...citrus-validation-xml/src/main/java/com/consol/citrus/xml/XpathSegmentVariableExtractor.java Outdated Show resolved Hide resolved

christophd approved these changes

View reviewed changes

Member

christophd commented Dec 20, 2021

LGTM Many thanks! Unit tests in CI job failing though

Collaborator Author

tschlat commented Dec 22, 2021 •

edited

Loading

@christophd please carefully check changes of this commit which I provided as fix to issue#831

Collaborator Author

tschlat commented Dec 22, 2021 •

edited

Loading

@christophd please carefully check changes on this commit which I provided as fix to issue#832.

tschlat force-pushed the issue/821/access-variable-by-xpath-json branch 2 times, most recently from 6203c8a to 2786da3 Compare

December 22, 2021 13:55

tschlat requested a review from christophd

December 22, 2021 13:56

bbortt requested changes

View reviewed changes

Collaborator

bbortt left a comment

a review for christmas 🎄

...citrus-base/src/test/java/com/consol/citrus/functions/core/ReadFileResourceFunctionTest.java Outdated Show resolved Hide resolved

core/citrus-api/src/main/java/com/consol/citrus/util/IsJsonPredicate.java Outdated Show resolved Hide resolved

core/citrus-api/src/main/java/com/consol/citrus/util/IsXmlPredicate.java Outdated Show resolved Hide resolved

core/citrus-api/src/main/java/com/consol/citrus/variable/VariableExpressionIterator.java Outdated Show resolved Hide resolved

core/citrus-api/src/main/java/com/consol/citrus/variable/VariableExpressionSegmentMatcher.java Outdated Show resolved Hide resolved

vintage/citrus-java-dsl/src/main/java/com/consol/citrus/dsl/builder/HttpActionBuilder.java

@@ @@ -26,7 +27,7 @@ public HttpClientActionBuilder client(String httpClient) { @@
                       return new HttpClientActionBuilder(delegate.client(httpClient));
                   }
-                  public HttpServerActionBuilder server(HttpServer httpServer) {
+                  public HttpServerActionBuilder server(Endpoint httpServer) {

Collaborator

bbortt Dec 22, 2021

thanks @tschlat! that's the right one - follow up for #817. my bad.

Member

christophd Jan 10, 2022

yes, that's the right place. 👍

@bbortt what about reverting the changes made in #817? I think the builder should only accept HttpServer endpoints. I need to review the use case in the citrus-simulator to understand why another endpoint type is required here.

It is ok to provide the Endpoint option in the vintage modules but I think using the builder with generic endpoint type is not the way we should go in the long run.

Collaborator

bbortt Jan 27, 2022

see #840.

Collaborator Author

tschlat commented Dec 23, 2021 •

edited

Loading

@bbortt I accepted all your suggestions.

@christophd if you could have a look at the two commits concerning other issues and could kick the workflow we could eventually make it for christmas.

If approved, I will squash and reorganize the commits into three commits namely: fix(#820), fix(#821) and fix(#832)

bbortt approved these changes

View reviewed changes

Collaborator

bbortt left a comment

/lgtm, many thanks.

Member

christophd commented Jan 10, 2022

@tschlat sorry, did not make it before the holidays. just started the workflow

christophd reviewed

View reviewed changes

Member

christophd left a comment

I see a problem with non textual file content and the variable replacement. I have described in more detail in the comments

core/citrus-base/src/main/java/com/consol/citrus/functions/core/ReadFileResourceFunction.java Outdated Show resolved Hide resolved

core/citrus-base/src/main/java/com/consol/citrus/functions/core/ReadFileResourceFunction.java Outdated Show resolved Hide resolved

tschlat force-pushed the issue/821/access-variable-by-xpath-json branch 3 times, most recently from 376bedd to 5e8923c Compare

January 17, 2022 09:01

Thorsten Schlathölter added 4 commits

January 17, 2022 10:13


          fix(citrusframework#820): Allow access to indexed variables

dc0a960


          fix(citrusframework#821): Allow direct access to path/jsonPath in var…

5008a6e

…iables


          fix(citrusframework#831): readFile() function should repl. dynamic co…

da033ae

…ntent when encod. base64

- Adjusted file.txt
  - to only contain constant expressions (currentDate())
  - Removed newLine to support testing on windows


          fix(citrusframework#832): Changes constructor to allow Endpoint arg r…

d195eca

…ather than HttpServer

tschlat force-pushed the issue/821/access-variable-by-xpath-json branch from 5e8923c to d195eca Compare

January 17, 2022 09:20

Collaborator Author

tschlat commented Jan 18, 2022

@christophd magst Du noch mal meinen Change für #831 reviewen und den Workflow noch mal triggern?

Collaborator

bbortt commented Jan 19, 2022

@christophd maybe one of us (@tschlat or me) can be maintainer here as well as in citrusframework/citrus-simulator? would be faster for us, if we could trigger the workflow ourselfs.

Member

christophd commented Jan 24, 2022

@bbortt @tschlat we have a green build! 🥳 @tschlat do you still want to squash and reorganize the commits before merge?

christophd reviewed

View reviewed changes

endpoints/citrus-ws/src/main/java/com/consol/citrus/ws/message/SoapAttachment.java Outdated Show resolved Hide resolved


          fix(837): Fix platform specific test failures

516ce14

Tests that fail on os=win

fix(837): Fix platform specific test failures

Tests that fail on os=win

tschlat force-pushed the issue/821/access-variable-by-xpath-json branch from ab01258 to 516ce14 Compare

January 25, 2022 06:09

Collaborator Author

tschlat commented Jan 25, 2022

@bbortt @tschlat we have a green build! 🥳 @tschlat do you still want to squash and reorganize the commits before merge?

This has already been done. I have a commit per issue. Any objections?

Collaborator Author

tschlat commented Jan 25, 2022

@christophd could you make me a maintainer so that I can trigger the build myself?

Collaborator

bbortt commented Jan 25, 2022

@christophd could you make me a maintainer so that I can trigger the build myself?

I can help you.. just ping me 😉

christophd merged commit f6359fc into citrusframework:main

bbortt deleted the issue/821/access-variable-by-xpath-json branch

January 25, 2022 07:50

Collaborator

bbortt commented Jan 25, 2022

thanks @christophd 🚀

Collaborator Author

tschlat commented Jan 25, 2022

Yes thanks a lot for the support @christophd

Collaborator Author

tschlat commented Jan 25, 2022

bbortt mentioned this pull request

Revert "changed to endpoint" #840

Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet