Skip to content

Add skipPaths to selectively skip requests from Chucker#970

Merged
cortinico merged 11 commits into
ChuckerTeam:developfrom
ArjanSM:Issue-266
Apr 8, 2023
Merged

Add skipPaths to selectively skip requests from Chucker#970
cortinico merged 11 commits into
ChuckerTeam:developfrom
ArjanSM:Issue-266

Conversation

@ArjanSM

@ArjanSM ArjanSM commented Feb 24, 2023

Copy link
Copy Markdown
Contributor

📄 Context

Fixes #266

📝 Changes

I added a public API, skipPaths(paths:List<String>), to the ChuckerInterceptor.Builer.
The ChuckerInterceptor decides to process the transaction based on the outcome of the lambda in skipEndpoints.

🚫 Breaking

Adding skipPaths(..) to the ChuckerInterceptor.Build will be a breaking change.

🛠️ How to test

  • Added tests to ChuckerInterceptorTest
  • I've also added another endpoint /anything to the HttpBinHttpTask.kt and added /anything to the skip paths list in the sample app.

@cortinico

Copy link
Copy Markdown
Member

I'd like to test this out @ArjanSM on the sample app but I've just noticed it instacrash due to a StrictMode violation :|

@cortinico cortinico left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for sending this over @ArjanSM

I don't think we want to evolve the API in this direction as this will expose the Request object from OkHTTP to the user.
A user can manipulate the request (i.e. Read the body) in way we cannot prevent.

I think the right way to approach this feature is either:

  • Provide a list of paths to use for the skip (a List<String>)
  • Provide a list of header to strip and use for the skip (again a List<String>)

Comment thread library/src/main/kotlin/com/chuckerteam/chucker/api/ChuckerInterceptor.kt Outdated
ArjanSM added 2 commits March 3, 2023 00:50
- adds example in sample app.
- adds tests
@ArjanSM ArjanSM requested a review from a team as a code owner March 3, 2023 06:35
@ArjanSM ArjanSM requested a review from cortinico March 3, 2023 07:26

@cortinico cortinico left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the new approach goes in the right direction 👍
Left a couple of comments.

The CI is still red though, can you look into it @ArjanSM

Comment thread library/src/main/kotlin/com/chuckerteam/chucker/api/ChuckerInterceptor.kt Outdated
Comment thread library/src/main/kotlin/com/chuckerteam/chucker/api/ChuckerInterceptor.kt Outdated
@ArjanSM ArjanSM requested a review from cortinico March 8, 2023 13:49
Comment thread library/src/test/kotlin/com/chuckerteam/chucker/api/ChuckerInterceptorTest.kt Outdated
Comment thread library/src/main/kotlin/com/chuckerteam/chucker/api/ChuckerInterceptor.kt Outdated
@ArjanSM ArjanSM requested a review from cortinico March 12, 2023 21:36
@cortinico cortinico changed the title Issue-266: An alternate approach Add skipPaths to selectively skip requests from Chucker Apr 8, 2023
Comment on lines +85 to +87
return if(shouldProcessTheRequest)
responseProcessor.process(response,transaction)
else response

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's merge it as it is, but I believe KtLint is broken here as this is not properly formatted

@cortinico cortinico merged commit fd213c1 into ChuckerTeam:develop Apr 8, 2023
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.

Allow to selectively skip Chucker interception

2 participants