Skip to content

Conversation

@onobc
Copy link
Contributor

@onobc onobc commented Jul 2, 2025

This is the 2 commits in my PR for server interceptor filters on top of main.

Then I explore moving the filters as follows:

Server Interceptor Filter

  • move filter instance from configuruer to InProcessGrpcServerFactory
  • add supports(interceptor, service) on GrpcServerFactory w/ default return true
  • modify configurer to ask the factory being configured if it supports the interceptor/service
  • override supports in InProcessGrpcServerFactory to ask its filter instance if it supports it

Tradeoffs
✅ This removes the server factory from the interceptor filter contract

❌ Currently the ServerInterceptorFilter is only used by InProcessGrpcServerFactory so it may be misleading by its name for users thinking it may be used by all servers

Service Filter

  • move filter instance from DefaultGrpcServerFactory to InProcessGrpcServerFactory
  • remove use of filter in addService of DefaultGrpcServerFactory
  • override addService in InProcessGrpcServerFactory to ask its filter instance if it should add it

Tradeoffs
✅ This removes the server factory from the interceptor filter contract

❌ Currently the ServerServiceDefinitionFilter is only used by InProcessGrpcServerFactory so it may be misleading by its name for users thinking it may be used by all servers

Note

This will not compile fully - only playing w/ the contract a bit before going further

therepanic and others added 3 commits July 2, 2025 15:32
This commit adds the ability to filter out services from being bound
to server factories.

Resolves spring-projects#207

Signed-off-by: Andrey Litvitski <andrey1010102008@gmail.com>
Signed-off-by: Chris Bono <chris.bono@gmail.com>
This commit updates some tests and the contract slightly of the
previous commit to allow a serverFactory parameter as part of
the inputs to the service filter and tests all variants of
server factories via `@ParameterizedTest`

See spring-projects#207

Signed-off-by: Chris Bono <chris.bono@gmail.com>
Signed-off-by: Chris Bono <chris.bono@gmail.com>
@dsyer dsyer merged commit a0f5c55 into spring-projects:main Jul 3, 2025
3 of 5 checks passed
@dsyer
Copy link
Member

dsyer commented Jul 3, 2025

I took this suggestion and extended it a bit in ac3ecb5. Also closes #216.

@therepanic
Copy link
Contributor

Hi, @onobc. You may have noticed that I deliberately did not include documentation in my commit, but it is needed. I can try to write it if you don't mind (I assume you noticed this and decided to do it yourself a little later). If so, can you advise which tab in the documentation to include it in? I can guess what it will look like, as there is documentation in previous changes with filters, and I have a rough idea of how to do it.

@onobc
Copy link
Contributor Author

onobc commented Jul 4, 2025

Thanks for the reminder @therepanic . I will make sure that the client, server, and service filter docs all exist and are all up-to-date (reflect the latest changes @dsyer made).

@onobc onobc deleted the GH-207-208-explore-filter-options branch July 4, 2025 20:26
@onobc
Copy link
Contributor Author

onobc commented Jul 4, 2025

Thanks for the reminder @therepanic . I will make sure that the client, server, and service filter docs all exist and are all up-to-date (reflect the latest changes @dsyer made).

The client/server interceptor docs were already updated by Dave to reflect reality.

I added a section for service filtering in this PR PTAL

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