Skip to content

Conversation

@predic8
Copy link
Member

@predic8 predic8 commented Jan 3, 2026

  • update examples and tests

Interceptors are placed directly under global

global:
  - log: {}
  - response:
      - setHeader:
          name: Access-Control-Allow-Origin
          value: "*"
      - setHeader:
          name: Access-Control-Allow-Methods
          value: GET, POST
      - setHeader:
          name: Access-Control-Allow-Headers
          value: Content-Type

Summary by CodeRabbit

  • Documentation

    • Updated global interceptor example documentation with revised API endpoints and configuration references.
  • Tests

    • Refined CORS header validation in global interceptor tests with stricter assertion criteria.
  • Chores

    • Added new YAML configuration file for global interceptor example setup.

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

- update examples and tests
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 3, 2026

📝 Walkthrough

Walkthrough

The GlobalInterceptor class annotation is updated to add noEnvelope = true, signaling changed envelope handling behavior. Documentation and configuration files are updated to reference apis.yaml instead of proxies.xml. Test assertions are adjusted to expect stricter CORS header values.

Changes

Cohort / File(s) Summary
Annotation Update
core/src/main/java/com/predic8/membrane/core/interceptor/GlobalInterceptor.java
Added noEnvelope = true parameter to @MCElement annotation on GlobalInterceptor class
Configuration & Documentation
distribution/examples/extending-membrane/global-interceptor/README.md, distribution/examples/extending-membrane/global-interceptor/apis.yaml
New YAML configuration file defining global interceptor with logging and CORS headers; README updated with new API endpoint paths (/foo, /bar) and reference changed from proxies.xml to apis.yaml
Test Updates
distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/test/GlobalInterceptorExampleTest.java
Test method visibility changed from public to package-private; CORS header assertions updated from wildcard values to specific values (GET, POST for methods; Content-Type for headers); imports reorganized

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

7.x

Suggested reviewers

  • rrayst
  • t-burch

Poem

🐰 A global interceptor hops with grace,
No envelope needed—just the base,
CORS headers dance, strict and true,
New apis.yaml paths break through,
Tests aligned with headers so keen! 🌟

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'global: Remove 'flow' child' accurately describes the main change—removing the 'flow' child element from global configuration. It is specific, concise, and directly reflects the primary objective stated in the PR description.
✨ Finishing touches
  • 📝 Generate docstrings

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

🧹 Nitpick comments (2)
distribution/examples/extending-membrane/global-interceptor/apis.yaml (1)

1-1: Consider using a variable or latest version for the schema reference.

The schema URL is hardcoded to version v7.0.4.json. If this example is intended to work with multiple versions or be maintained long-term, consider whether this should reference a variable, latest, or be updated as part of release processes.

distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/test/GlobalInterceptorExampleTest.java (1)

17-21: Wildcard imports improve brevity.

The switch to wildcard imports and static imports for Header.* and RestAssured.* makes the code more concise. This is acceptable if it aligns with the project's coding style.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 24da0a0 and 61227a2.

📒 Files selected for processing (4)
  • core/src/main/java/com/predic8/membrane/core/interceptor/GlobalInterceptor.java
  • distribution/examples/extending-membrane/global-interceptor/README.md
  • distribution/examples/extending-membrane/global-interceptor/apis.yaml
  • distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/test/GlobalInterceptorExampleTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
core/src/main/java/com/predic8/membrane/core/interceptor/GlobalInterceptor.java (6)
core/src/main/java/com/predic8/membrane/core/interceptor/flow/AbortInterceptor.java (1)
  • MCElement (32-56)
core/src/main/java/com/predic8/membrane/core/interceptor/flow/RequestInterceptor.java (1)
  • MCElement (31-62)
core/src/main/java/com/predic8/membrane/core/interceptor/flow/ResponseInterceptor.java (1)
  • MCElement (28-47)
core/src/main/java/com/predic8/membrane/core/interceptor/flow/IfInterceptor.java (1)
  • MCElement (41-151)
core/src/main/java/com/predic8/membrane/core/interceptor/flow/ForInterceptor.java (1)
  • MCElement (46-147)
core/src/main/java/com/predic8/membrane/core/interceptor/chain/ChainInterceptor.java (1)
  • MCElement (32-73)
⏰ 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 (7)
distribution/examples/extending-membrane/global-interceptor/apis.yaml (2)

3-15: LGTM! Clean global interceptor configuration.

The new structure places interceptors directly under the global: key without the flow: wrapper, which aligns perfectly with the PR objective and the noEnvelope = true annotation change in GlobalInterceptor.java.


8-9: Wildcard CORS origin in example is acceptable.

The Access-Control-Allow-Origin: "*" configuration allows any origin to access the API. This is acceptable for an example/demonstration, but production deployments should restrict this to specific trusted origins.

distribution/examples/extending-membrane/global-interceptor/README.md (2)

14-19: LGTM! Updated endpoints reflect the example APIs.

The curl commands have been updated to include paths /foo and /bar. Since the APIs in apis.yaml use simple return interceptors without specific path restrictions, these paths (or any other paths) should work correctly. The path choice helps demonstrate that the global interceptor applies to all requests regardless of the path.


21-21: Correct update to reference the new YAML configuration.

The documentation now correctly references apis.yaml instead of proxies.xml, which aligns with the shift to YAML-based configuration demonstrated in this PR.

distribution/src/test/java/com/predic8/membrane/examples/withoutinternet/test/GlobalInterceptorExampleTest.java (2)

32-32: LGTM! Package-private visibility is appropriate for JUnit 5.

JUnit 5 doesn't require test methods to be public, so using package-private (default) visibility is a recommended practice that reduces unnecessary boilerplate.

Also applies to: 45-45


39-40: CONTENT_TYPE constant correctly resolves to "Content-Type" and matches the YAML configuration.

The test assertions are correct:

  • Access-Control-Allow-Methods: "GET, POST" matches apis.yaml configuration at line 12
  • Access-Control-Allow-Headers: CONTENT_TYPE correctly uses Header.CONTENT_TYPE, which resolves to "Content-Type" as specified in apis.yaml line 15

Both test methods (lines 39-40 and 52-53) verify the CORS headers properly.

core/src/main/java/com/predic8/membrane/core/interceptor/GlobalInterceptor.java (1)

25-25: LGTM! Annotation change aligns with similar interceptors.

Adding noEnvelope = true is consistent with other flow interceptors like ResponseInterceptor, RequestInterceptor, and AbortInterceptor. This allows child interceptors to be placed directly under the global: key in YAML configurations without requiring a wrapper element. The existing example configuration in ./distribution/examples/extending-membrane/global-interceptor/apis.yaml already demonstrates this pattern in use with direct nesting of interceptors.

@membrane-ci-server
Copy link

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

predic8 added a commit that referenced this pull request Jan 3, 2026
@predic8
Copy link
Member Author

predic8 commented Jan 3, 2026

/ok-to-test

@predic8 predic8 merged commit 89397c8 into master Jan 3, 2026
5 checks passed
@predic8 predic8 deleted the global-configchange-example branch January 3, 2026 19:29
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