Skip to content
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.

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

add @ContentDisposition annotation for setting Content-Disposition he… #11488

Open
wants to merge 2 commits into
base: 4.8.x
Choose a base branch
from

Conversation

ChaimaaeROUAI
Copy link
Contributor

issue #9480

@ChaimaaeROUAI ChaimaaeROUAI added the type: improvement A minor improvement to an existing feature label Jan 6, 2025
@@ -26,6 +26,8 @@ dependencies {
testImplementation(libs.micronaut.test.junit5) {
exclude(group= "io.micronaut")
}
testImplementation("io.micronaut:micronaut-http-client")
Copy link
Member

Choose a reason for hiding this comment

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

please move the test to the tck

import reactor.core.publisher.Flux;

/**
* A filter that adds the `Content-Disposition` header to HTTP responses for specific endpoints.
Copy link
Member

Choose a reason for hiding this comment

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

Add @since

* @return A publisher that emits the modified or unmodified HTTP response.
*/
@Override
public Publisher<MutableHttpResponse<?>> doFilter(HttpRequest<?> request, ServerFilterChain chain) {
Copy link
Member

Choose a reason for hiding this comment

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

Add nullability annotations

*/
@Singleton
@Filter("/**")
public class ContentDispositionFilter implements HttpServerFilter {
Copy link
Member

Choose a reason for hiding this comment

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

Use @ServerFilter API

Copy link
Member

Choose a reason for hiding this comment

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

In fact I'm not sure we can have a general filter like this. It will affect performance of all controllers, even those without the annotation. Potentially this change needs to go into ResponseLifecycle instead (after #11342 )

@dstepanov
Copy link
Contributor

I think you can simply implement ContentDisposition as an alias for Header

*/
@Override
public Publisher<MutableHttpResponse<?>> doFilter(HttpRequest<?> request, ServerFilterChain chain) {
if (request.getPath().startsWith("/report/result")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this class is in src/main/java but seems to be hard coded to some test classes (ReportController) this doesn't seem to belong here at all. What is this PR trying to achieve?

* Provides an endpoint to generate and download a CSV report.
*/
@Controller("/report")
public class ReportController {
Copy link
Contributor

Choose a reason for hiding this comment

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

this should not be in src/main/java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: improvement A minor improvement to an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants