Skip to content

Conversation

@predic8
Copy link
Member

@predic8 predic8 commented Jan 6, 2026

Summary by CodeRabbit

  • Documentation

    • Updated SOAP validation README wording and curl examples (use application/xml); consolidated ROADMAP logging examples; minor tutorial comment formatting.
  • New Features

    • Added a YAML example for running a SOAP proxy with WSDL-based validation and an API tester.
  • Tests

    • Strengthened validation tests to assert presence or absence of SOAP fault content.
  • Refactor

    • Simplified SOAP fault response construction for more consistent error responses.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 6, 2026

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Refactors SOAP fault response construction in SOAPUtil.java to use a fluent Response builder, adds a new SOAP validation example config (apis.yaml) and README edits, tightens test assertions for SOAP fault detection, and makes minor docs/comment formatting updates.

Changes

Cohort / File(s) Summary
SOAP Utility Refactoring
core/src/main/java/com/predic8/membrane/core/util/SOAPUtil.java
Replaced manual raw Response construction with fluent builder (ok().contentType(TEXT_XML_UTF8).body(...).build()); removed helpers createRawSOAPErrorResponse() and getFaultSOAP12Body(); adjusted imports and preserved exception behavior.
SOAP Validation Example
distribution/examples/web-services-soap/soap-wsdl-validation/README.md, distribution/examples/web-services-soap/soap-wsdl-validation/apis.yaml
README title/description updated; removed "How it is done" section; example curl Content-Type changed from application/soap+xml to application/xml; added new apis.yaml configuring soapProxy and validator and sample API tester.
SOAP Proxy Validation Tests
distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/validation/SOAPProxyValidationExampleTest.java
Test methods' visibility changed from public to package-private; success test now asserts absence of "faultcode" and status 200; invalid test asserts presence of "faultcode" and status 200.
Documentation & Comments
distribution/tutorials/getting-started/10-Logging.yaml, docs/ROADMAP.md
Minor comment indentation tweak in tutorial; ROADMAP formatting adjusted and log example consolidated to an aggregated listening line.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Soap proxy wsdl example #2551 — Performs the same refactor in core/src/main/java/com/predic8/membrane/core/util/SOAPUtil.java (switch to Response builder, remove helpers, adjust imports).

Suggested reviewers

  • rrayst
  • t-burch

Poem

🐰 I hop through XML, neat and bright,
Builders stitch faults back into light.
YAML fields sparkle in the dew,
Tests now listen for the clue —
A tiny hop: the pipeline's true.

Pre-merge checks

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The title 'Soap proxy wsdl example' is vague and generic, using non-descriptive terms that don't convey what was actually changed in the codebase. Improve the title to be more specific about the actual changes, such as 'Refactor SOAP response construction to use fluent builder pattern' or 'Add SOAP WSDL validation example with updated configuration' to clearly indicate the main focus.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3e748b2 and 77c6e8e.

📒 Files selected for processing (1)
  • docs/ROADMAP.md

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
core/src/main/java/com/predic8/membrane/core/util/SOAPUtil.java (1)

62-68: Use appropriate HTTP status codes for SOAP faults based on fault type.

The refactored builder pattern uses ok() which returns HTTP 200 for all SOAP faults, regardless of the fault type. According to SOAP 1.1 specification, fault responses should return HTTP 500 (Internal Server Error) for server-side faults and HTTP 400 (Bad Request) for client-side faults.

The method accepts a FaultCode parameter (Server/Client) and correctly serializes it into the SOAP fault XML body, but this distinction is ignored when setting the HTTP response status code. Consider using the FaultCode to determine the appropriate HTTP status:

  • FaultCode.Server → HTTP 500
  • FaultCode.Client → HTTP 400
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4a3d84f and 1e2be1d.

📒 Files selected for processing (6)
  • core/src/main/java/com/predic8/membrane/core/util/SOAPUtil.java
  • distribution/examples/web-services-soap/soap-wsdl-validation/README.md
  • distribution/examples/web-services-soap/soap-wsdl-validation/apis.yaml
  • distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/validation/SOAPProxyValidationExampleTest.java
  • distribution/tutorials/getting-started/10-Logging.yaml
  • docs/ROADMAP.md
🧰 Additional context used
🧬 Code graph analysis (1)
core/src/main/java/com/predic8/membrane/core/util/SOAPUtil.java (1)
core/src/main/java/com/predic8/membrane/core/util/xml/XMLUtil.java (1)
  • XMLUtil (30-111)
🪛 markdownlint-cli2 (0.18.1)
docs/ROADMAP.md

28-28: Spaces inside emphasis markers

(MD037, no-space-in-emphasis)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (java)
🔇 Additional comments (12)
core/src/main/java/com/predic8/membrane/core/util/SOAPUtil.java (2)

31-31: LGTM! Import changes support the builder pattern refactoring.

The static import of Response.* enables the fluent builder pattern used in createSOAPFaultResponse, and the wildcard import for XMLInputFactory.* appropriately covers the constants used elsewhere in the file.

Also applies to: 33-33


62-68: The builder pattern refactoring improves readability.

The switch from manual response construction to the fluent builder pattern makes the code cleaner and more maintainable. Exception handling is preserved appropriately.

distribution/tutorials/getting-started/10-Logging.yaml (1)

18-18: LGTM! Improved comment indentation consistency.

The adjusted indentation aligns with other example commands in the file.

docs/ROADMAP.md (1)

20-28: LGTM! Clear documentation of log consolidation improvement.

The reorganized JMXExporter section and new logs subsection effectively document the planned improvement to consolidate multiple listening port log lines into a single aggregated message.

distribution/examples/web-services-soap/soap-wsdl-validation/README.md (3)

1-3: LGTM! Improved title and description.

The updated title and description more clearly convey the purpose of the example and the validator's role in checking SOAP messages against WSDL.


23-23: LGTM! Updated reference to new configuration file.

The reference now correctly points to apis.yaml, aligning with the new YAML-based configuration approach introduced in this PR.


14-16: Change Content-Type to text/xml for SOAP 1.1 compliance.

The example uses SOAP 1.1 (namespace http://schemas.xmlsoap.org/soap/envelope/). The curl command should use Content-Type: text/xml instead of application/xml to comply with SOAP 1.1 specifications.

Also applies to: 19-21

distribution/examples/web-services-soap/soap-wsdl-validation/apis.yaml (1)

1-23: LGTM! Well-structured SOAP proxy configuration.

The configuration clearly defines:

  • A SOAP proxy with WSDL-based validation on port 2000
  • A test API service on port 2001
  • Clear usage instructions in comments

All referenced files (city-service.wsdl, city-soap.xml, invalid-city-soap.xml) are present in the directory.

distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/validation/SOAPProxyValidationExampleTest.java (4)

25-25: LGTM!

The static import of Hamcrest matchers enables the improved assertions in the test methods.


35-35: LGTM!

The visibility change to package-private follows JUnit 5 best practices, as test methods no longer need to be public.

Also applies to: 51-51


44-45: LGTM!

The assertions correctly validate that a valid SOAP request returns a successful response without a fault code.


61-62: LGTM. The assertions correctly validate that an invalid SOAP request produces a fault response. This example uses SOAP 1.1 (indicated by the s11 namespace prefix and http://schemas.xmlsoap.org/soap/envelope/ namespace), where HTTP 200 for fault responses is the correct behavior, confirming the status code expectation and the code comment are both accurate.

@predic8 predic8 requested a review from rrayst January 6, 2026 19:13
@membrane-ci-server
Copy link

This pull request needs "/ok-to-test" from an authorized committer.

rrayst
rrayst previously approved these changes Jan 7, 2026
@predic8 predic8 merged commit 36b5bdd into master Jan 7, 2026
2 of 3 checks passed
@predic8 predic8 deleted the soap-proxy-wsdl-example branch January 7, 2026 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants