mirror of
https://github.com/iflytek/skillhub.git
synced 2026-11-14 09:01:16 +00:00
fix(notification): return client errors for invalid delivery pagination
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
parent
d6ebd66fa7
commit
da1ab25d69
7 changed files with 54 additions and 5 deletions
|
|
@ -25,7 +25,7 @@
|
|||
|---|---|---|
|
||||
| `ObjectOptimisticLockingFailureException` | HTTP 409、响应 `code=409`、本地化冲突提示 | `persistenceConflictsReturn409WithoutLeakingDatabaseDetails` |
|
||||
| `StorageAccessException` | HTTP 503;日志带 requestId 和认证状态,不输出稳定 userId | `handleStorageAccess_shouldLogAuthenticationWithoutStableUserId` |
|
||||
| MVC 缺参、不可读请求体、类型不匹配 | HTTP 400、ApiResponse `code=400`;不记作未处理内部故障 | `handleMvcBadRequest_shouldReturn400WithoutUnhandledErrorLog` |
|
||||
| MVC 缺参、不可读请求体、类型不匹配、原生方法入参约束 | HTTP 400、ApiResponse `code=400`;不记作未处理内部故障 | `handleMvcBadRequest_shouldReturn400WithoutUnhandledErrorLog`、`handleMethodValidationDistinguishesInputFromReturnValueErrors` |
|
||||
| 不支持的 HTTP 方法 | HTTP 405、ApiResponse `code=405`,保留 Allow 响应头 | `handleMethodNotAllowed_shouldReturn405AndAllowHeader` |
|
||||
|
||||
反例:用 `exception.getMessage().contains(...)` 为新业务规则分配状态码;只断言状态而遗漏协议头;
|
||||
|
|
|
|||
|
|
@ -25,6 +25,8 @@ SMTP 连接、TLS 和企业邮箱接入方式见 [SMTP 配置指南](19-smtp-pas
|
|||
|
||||
每封摘要最多展开 50 条,超出的通知通过邮件中的 **批次详情** 入口分页查看;不会为分页再拆发多封。详情路径为 `/dashboard/notifications/email/{id}`,要求登录为对应收件账号,并逐项检查当前权限。退出空间、失去权限或资源删除后,相关项不再展示。保留期结束、批次不存在或不属于当前账号时,页面会提示不可用。此明细不依赖站内通知是否开启、是否已读或已删除。明细采用游标逐页读取原始记录,再过滤当前权限;本页可见项为空但仍有下一游标时,可以继续查看后页。“原始记录数”不是当前可查看条数,“不可访问”数量仅对应本页。已合并的旧批次会提供目标摘要入口。
|
||||
|
||||
详情接口的 `size` 范围为 1–100,管理员投递列表的 `page` 范围为 0–1000000;非法分页参数或非数字游标返回 HTTP 400,不调用查询服务,也不在错误响应中回显原始游标。服务端返回值校验失败仍按 HTTP 500 处理。
|
||||
|
||||
## 管理员如何开启
|
||||
|
||||
1. 在部署侧配置 SMTP、独立通知发件人、有限网络超时和真实公开站点根地址。不要把凭证提交进 Git,也不要在后台填 SMTP 密钥。
|
||||
|
|
|
|||
|
|
@ -7,10 +7,8 @@ import com.iflytek.skillhub.service.notificationemail.NotificationEmailDeliveryQ
|
|||
import jakarta.validation.constraints.*;
|
||||
import org.springframework.security.access.prepost.PreAuthorize;
|
||||
import org.springframework.security.core.annotation.AuthenticationPrincipal;
|
||||
import org.springframework.validation.annotation.Validated;
|
||||
import org.springframework.web.bind.annotation.*;
|
||||
@RestController
|
||||
@Validated
|
||||
@PreAuthorize("hasRole('SUPER_ADMIN')")
|
||||
@RequestMapping({"/api/v1/admin/notification-email-deliveries", "/api/web/admin/notification-email-deliveries"})
|
||||
public class NotificationEmailDeliveryAdminController extends BaseApiController {
|
||||
|
|
|
|||
|
|
@ -4,10 +4,8 @@ import com.iflytek.skillhub.controller.BaseApiController;
|
|||
import com.iflytek.skillhub.dto.*;
|
||||
import com.iflytek.skillhub.service.notificationemail.NotificationEmailDeliveryQueryService;
|
||||
import jakarta.validation.constraints.*;
|
||||
import org.springframework.validation.annotation.Validated;
|
||||
import org.springframework.web.bind.annotation.*;
|
||||
@RestController
|
||||
@Validated
|
||||
@RequestMapping({"/api/v1/notification-email-deliveries", "/api/web/notification-email-deliveries"})
|
||||
public class NotificationEmailDeliveryController extends BaseApiController {
|
||||
private final NotificationEmailDeliveryQueryService service;
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ import org.springframework.web.bind.MethodArgumentNotValidException;
|
|||
import org.springframework.web.bind.MissingRequestHeaderException;
|
||||
import org.springframework.web.bind.MissingServletRequestParameterException;
|
||||
import org.springframework.web.bind.annotation.ExceptionHandler;
|
||||
import org.springframework.web.method.annotation.HandlerMethodValidationException;
|
||||
import org.springframework.web.bind.annotation.RestControllerAdvice;
|
||||
import org.springframework.web.context.request.async.AsyncRequestTimeoutException;
|
||||
import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException;
|
||||
|
|
@ -119,6 +120,13 @@ public class GlobalExceptionHandler {
|
|||
apiResponseFactory.error(400, "error.badRequest"));
|
||||
}
|
||||
|
||||
@ExceptionHandler(HandlerMethodValidationException.class)
|
||||
public ResponseEntity<ApiResponse<Void>> handleMethodValidation(
|
||||
HandlerMethodValidationException ex, HttpServletRequest request) {
|
||||
// Invalid controller results are server faults, not client input errors.
|
||||
return ex.isForReturnValue() ? handleGlobalException(ex, request) : handleMvcBadRequest(ex, request);
|
||||
}
|
||||
|
||||
@ExceptionHandler(HttpRequestMethodNotSupportedException.class)
|
||||
public ResponseEntity<ApiResponse<Void>> handleMethodNotAllowed(
|
||||
HttpRequestMethodNotSupportedException ex,
|
||||
|
|
|
|||
|
|
@ -66,6 +66,36 @@ class NotificationEmailDeliverySecurityTest {
|
|||
.contentType(MediaType.APPLICATION_JSON).content("{\"confirmDuplicateRisk\":true}")).andExpect(status().isUnauthorized());
|
||||
verify(service,never()).retry(eq(2L),any(),anyBoolean());
|
||||
}
|
||||
@Test void invalidDeliveryPaginationReturnsClientErrorsWithoutCallingTheService() throws Exception {
|
||||
for (int size : new int[]{0, 101}) {
|
||||
mvc.perform(get("/api/web/notification-email-deliveries/1").param("size", String.valueOf(size))
|
||||
.with(authentication(auth("ordinary", "USER"))))
|
||||
.andExpect(status().isBadRequest()).andExpect(jsonPath("$.code").value(400));
|
||||
mvc.perform(get("/api/web/admin/notification-email-deliveries").param("size", String.valueOf(size))
|
||||
.with(authentication(auth("admin", "SUPER_ADMIN"))))
|
||||
.andExpect(status().isBadRequest()).andExpect(jsonPath("$.code").value(400));
|
||||
}
|
||||
for (int page : new int[]{-1, 1000001}) {
|
||||
mvc.perform(get("/api/web/admin/notification-email-deliveries").param("page", String.valueOf(page))
|
||||
.with(authentication(auth("admin", "SUPER_ADMIN"))))
|
||||
.andExpect(status().isBadRequest());
|
||||
}
|
||||
mvc.perform(get("/api/web/notification-email-deliveries/1").param("cursor", "private-invalid-cursor")
|
||||
.with(authentication(auth("ordinary", "USER"))))
|
||||
.andExpect(status().isBadRequest())
|
||||
.andExpect(content().string(org.hamcrest.Matchers.not(org.hamcrest.Matchers.containsString("private-invalid-cursor"))));
|
||||
verifyNoInteractions(service);
|
||||
}
|
||||
|
||||
@Test void validPageSizeBoundariesReachTheService() throws Exception {
|
||||
for (int size : new int[]{1, 100}) {
|
||||
mvc.perform(get("/api/web/notification-email-deliveries/1").param("size", String.valueOf(size))
|
||||
.with(authentication(auth("ordinary", "USER"))))
|
||||
.andExpect(status().isOk());
|
||||
verify(service).detail(1L, "ordinary", null, size);
|
||||
}
|
||||
}
|
||||
|
||||
private UsernamePasswordAuthenticationToken auth(String id,String role) {
|
||||
return new UsernamePasswordAuthenticationToken(new PlatformPrincipal(id,id,null,null,null,Set.of(role)),null,
|
||||
List.of(new SimpleGrantedAuthority("ROLE_"+role)));
|
||||
|
|
|
|||
|
|
@ -232,6 +232,19 @@ class GlobalExceptionHandlerTest {
|
|||
.isSameAs(ex);
|
||||
}
|
||||
|
||||
@Test
|
||||
void handleMethodValidationDistinguishesInputFromReturnValueErrors() {
|
||||
var invalidInput = org.mockito.Mockito.mock(
|
||||
org.springframework.web.method.annotation.HandlerMethodValidationException.class);
|
||||
when(invalidInput.isForReturnValue()).thenReturn(false);
|
||||
assertThat(handler.handleMethodValidation(invalidInput, request).getBody().code()).isEqualTo(400);
|
||||
|
||||
var invalidResult = org.mockito.Mockito.mock(
|
||||
org.springframework.web.method.annotation.HandlerMethodValidationException.class);
|
||||
when(invalidResult.isForReturnValue()).thenReturn(true);
|
||||
assertThat(handler.handleMethodValidation(invalidResult, request).getBody().code()).isEqualTo(500);
|
||||
}
|
||||
|
||||
@Test
|
||||
void handleMvcBadRequest_shouldReturn400WithoutUnhandledErrorLog() {
|
||||
attachAppender();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue