mirror of
https://github.com/iflytek/skillhub.git
synced 2026-08-28 11:25:00 +00:00
Merge pull request #539 from iflytek/fix/notification-sse-headers
Some checks failed
Security / CodeQL (java-kotlin) (push) Waiting to run
Security / Dependency Review (push) Waiting to run
Security / CodeQL (javascript-typescript) (push) Waiting to run
Deploy Docs / build (push) Has been cancelled
Deploy Docs / Deploy (push) Has been cancelled
Some checks failed
Security / CodeQL (java-kotlin) (push) Waiting to run
Security / Dependency Review (push) Waiting to run
Security / CodeQL (javascript-typescript) (push) Waiting to run
Deploy Docs / build (push) Has been cancelled
Deploy Docs / Deploy (push) Has been cancelled
fix(notification): preserve SSE headers for notification stream
This commit is contained in:
commit
17daf87ab6
2 changed files with 72 additions and 11 deletions
|
|
@ -8,6 +8,8 @@ import org.slf4j.Logger;
|
|||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.core.Ordered;
|
||||
import org.springframework.core.annotation.Order;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.stereotype.Component;
|
||||
import org.springframework.web.filter.OncePerRequestFilter;
|
||||
import org.springframework.web.util.ContentCachingRequestWrapper;
|
||||
|
|
@ -39,6 +41,11 @@ public class RequestLoggingFilter extends OncePerRequestFilter {
|
|||
throws ServletException, IOException {
|
||||
|
||||
String uri = request.getRequestURI();
|
||||
if (isNotificationSse(uri)) {
|
||||
prepareSseResponse(response);
|
||||
filterChain.doFilter(request, response);
|
||||
return;
|
||||
}
|
||||
if (shouldSkip(uri)) {
|
||||
filterChain.doFilter(request, response);
|
||||
return;
|
||||
|
|
@ -100,6 +107,16 @@ public class RequestLoggingFilter extends OncePerRequestFilter {
|
|||
return false;
|
||||
}
|
||||
|
||||
private boolean isNotificationSse(String uri) {
|
||||
return uri != null && uri.endsWith("/notifications/sse");
|
||||
}
|
||||
|
||||
private void prepareSseResponse(HttpServletResponse response) {
|
||||
response.setContentType(MediaType.TEXT_EVENT_STREAM_VALUE);
|
||||
response.setHeader(HttpHeaders.CACHE_CONTROL, "no-cache, no-transform");
|
||||
response.setHeader("X-Accel-Buffering", "no");
|
||||
}
|
||||
|
||||
private String getRequestBody(ContentCachingRequestWrapper request) {
|
||||
byte[] buf = request.getContentAsByteArray();
|
||||
if (buf.length > 0) {
|
||||
|
|
|
|||
|
|
@ -1,22 +1,26 @@
|
|||
package com.iflytek.skillhub.filter;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
|
||||
import ch.qos.logback.classic.Level;
|
||||
import ch.qos.logback.classic.Logger;
|
||||
import ch.qos.logback.classic.spi.ILoggingEvent;
|
||||
import ch.qos.logback.core.read.ListAppender;
|
||||
import jakarta.servlet.FilterChain;
|
||||
import jakarta.servlet.ServletException;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.mock.web.MockHttpServletRequest;
|
||||
import org.springframework.mock.web.MockHttpServletResponse;
|
||||
|
||||
import jakarta.servlet.ServletResponse;
|
||||
import java.io.IOException;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.List;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.slf4j.LoggerFactory;
|
||||
import org.springframework.http.HttpHeaders;
|
||||
import org.springframework.http.MediaType;
|
||||
import org.springframework.mock.web.MockHttpServletRequest;
|
||||
import org.springframework.mock.web.MockHttpServletResponse;
|
||||
import org.springframework.web.util.ContentCachingResponseWrapper;
|
||||
|
||||
class RequestLoggingFilterTest {
|
||||
|
||||
|
|
@ -79,12 +83,12 @@ class RequestLoggingFilterTest {
|
|||
}
|
||||
|
||||
@Test
|
||||
void doFilterInternal_skipsSseEndpointsWithoutWrappingResponse()
|
||||
void doFilterInternal_skipsOtherSseEndpointsWithoutWrappingResponse()
|
||||
throws ServletException, IOException {
|
||||
RequestLoggingFilter filter = new RequestLoggingFilter();
|
||||
attachAppender();
|
||||
|
||||
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/web/notifications/sse");
|
||||
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/web/scan/sse");
|
||||
MockHttpServletResponse response = new MockHttpServletResponse();
|
||||
|
||||
FilterChain filterChain = (req, res) -> {
|
||||
|
|
@ -97,8 +101,10 @@ class RequestLoggingFilterTest {
|
|||
filter.doFilter(request, response, filterChain);
|
||||
|
||||
assertThat(response.getHeader("Content-Length")).isNull();
|
||||
assertThat(response.getHeader("X-Accel-Buffering")).isNull();
|
||||
assertThat(response.getHeader(HttpHeaders.CACHE_CONTROL)).isNull();
|
||||
assertThat(response.getContentAsString()).isEqualTo("event:connected\n");
|
||||
assertThat(loggedMessages()).noneMatch(message -> message.contains("/api/web/notifications/sse"));
|
||||
assertThat(loggedMessages()).noneMatch(message -> message.contains("/api/web/scan/sse"));
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -124,6 +130,44 @@ class RequestLoggingFilterTest {
|
|||
assertThat(loggedMessages()).noneMatch(message -> message.contains("Headers: {"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void doFilterInternal_shouldBypassCachingWrapperForNotificationSse() throws Exception {
|
||||
RequestLoggingFilter filter = new RequestLoggingFilter();
|
||||
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/web/notifications/sse");
|
||||
MockHttpServletResponse response = new MockHttpServletResponse();
|
||||
AtomicReference<ServletResponse> responseSeenByChain = new AtomicReference<>();
|
||||
FilterChain chain = (servletRequest, servletResponse) -> {
|
||||
responseSeenByChain.set(servletResponse);
|
||||
servletResponse.getWriter().write("event: connected\n");
|
||||
servletResponse.flushBuffer();
|
||||
};
|
||||
|
||||
filter.doFilter(request, response, chain);
|
||||
|
||||
assertThat(responseSeenByChain.get()).isSameAs(response);
|
||||
assertThat(response.getHeader("X-Accel-Buffering")).isEqualTo("no");
|
||||
assertThat(response.getHeader(HttpHeaders.CACHE_CONTROL)).isEqualTo("no-cache, no-transform");
|
||||
assertThat(response.getContentType()).isEqualTo(MediaType.TEXT_EVENT_STREAM_VALUE);
|
||||
assertThat(response.getContentAsString()).contains("event: connected");
|
||||
}
|
||||
|
||||
@Test
|
||||
void doFilterInternal_shouldKeepCachingWrapperForRegularApiResponses() throws Exception {
|
||||
RequestLoggingFilter filter = new RequestLoggingFilter();
|
||||
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/api/web/notifications/unread-count");
|
||||
MockHttpServletResponse response = new MockHttpServletResponse();
|
||||
AtomicReference<ServletResponse> responseSeenByChain = new AtomicReference<>();
|
||||
FilterChain chain = (servletRequest, servletResponse) -> {
|
||||
responseSeenByChain.set(servletResponse);
|
||||
servletResponse.getWriter().write("{\"count\":1}");
|
||||
};
|
||||
|
||||
filter.doFilter(request, response, chain);
|
||||
|
||||
assertThat(responseSeenByChain.get()).isInstanceOf(ContentCachingResponseWrapper.class);
|
||||
assertThat(response.getContentAsString()).isEqualTo("{\"count\":1}");
|
||||
}
|
||||
|
||||
private void attachAppender() {
|
||||
logger.setLevel(Level.INFO);
|
||||
appender = new ListAppender<>();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue