refactor(api): improve controllers with dedicated DTOs and better tests

- Extract admin DTOs (AdminUserSummary, AdminUserMutation, etc.)
- Add SkillRating request/response DTOs
- Improve controller input validation and error responses
- Strengthen controller test assertions
This commit is contained in:
vsxd 2026-03-12 20:12:04 +08:00
parent 50adf3545d
commit b028937abb
20 changed files with 184 additions and 112 deletions

View file

@ -3,6 +3,8 @@ package com.iflytek.skillhub.controller;
import com.iflytek.skillhub.auth.device.DeviceAuthService;
import com.iflytek.skillhub.auth.device.DeviceCodeResponse;
import com.iflytek.skillhub.auth.device.DeviceTokenResponse;
import com.iflytek.skillhub.dto.ApiResponse;
import com.iflytek.skillhub.dto.ApiResponseFactory;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestMapping;
@ -10,22 +12,23 @@ import org.springframework.web.bind.annotation.RestController;
@RestController
@RequestMapping("/api/v1/cli/auth/device")
public class DeviceAuthController {
public class DeviceAuthController extends BaseApiController {
private final DeviceAuthService deviceAuthService;
public DeviceAuthController(DeviceAuthService deviceAuthService) {
public DeviceAuthController(ApiResponseFactory responseFactory, DeviceAuthService deviceAuthService) {
super(responseFactory);
this.deviceAuthService = deviceAuthService;
}
@PostMapping("/code")
public DeviceCodeResponse requestDeviceCode() {
return deviceAuthService.generateDeviceCode();
public ApiResponse<DeviceCodeResponse> requestDeviceCode() {
return ok("response.success.created", deviceAuthService.generateDeviceCode());
}
@PostMapping("/token")
public DeviceTokenResponse pollToken(@RequestBody TokenRequest request) {
return deviceAuthService.pollToken(request.deviceCode());
public ApiResponse<DeviceTokenResponse> pollToken(@RequestBody TokenRequest request) {
return ok("response.success.read", deviceAuthService.pollToken(request.deviceCode()));
}
public record TokenRequest(String deviceCode) {}

View file

@ -28,7 +28,7 @@ public class DeviceAuthWebController extends BaseApiController {
@AuthenticationPrincipal PlatformPrincipal principal
) {
deviceAuthService.authorizeDeviceCode(request.userCode(), principal.userId());
return ok("response.success.update", new MessageResponse("Device authorized successfully"));
return ok("response.success.updated", new MessageResponse("Device authorized successfully"));
}
public record AuthorizeRequest(String userCode) {}

View file

@ -1,48 +1,41 @@
package com.iflytek.skillhub.controller.admin;
import com.iflytek.skillhub.controller.BaseApiController;
import com.iflytek.skillhub.dto.ApiResponse;
import com.iflytek.skillhub.dto.ApiResponseFactory;
import com.iflytek.skillhub.dto.AuditLogItemResponse;
import com.iflytek.skillhub.dto.PageResponse;
import org.springframework.data.domain.PageImpl;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.web.bind.annotation.*;
import java.time.Instant;
import java.util.List;
import java.util.Map;
@RestController
@RequestMapping("/api/v1/admin/audit-logs")
public class AuditLogController {
public class AuditLogController extends BaseApiController {
public AuditLogController(ApiResponseFactory responseFactory) {
super(responseFactory);
}
@GetMapping
@PreAuthorize("hasAnyRole('AUDITOR', 'SUPER_ADMIN')")
public Map<String, Object> listAuditLogs(
public ApiResponse<PageResponse<AuditLogItemResponse>> listAuditLogs(
@RequestParam(defaultValue = "0") int page,
@RequestParam(defaultValue = "20") int size,
@RequestParam(required = false) String userId,
@RequestParam(required = false) String action) {
// Placeholder implementation
return Map.of(
"logs", List.of(
Map.of(
"id", "log-1",
"userId", "user-1",
"action", "CREATE_SKILL",
"resourceType", "SKILL",
"resourceId", "skill-123",
"timestamp", Instant.now().toString(),
"ipAddress", "192.168.1.1"
List<AuditLogItemResponse> logs = List.of(
new AuditLogItemResponse(
"log-1", "user-1", "CREATE_SKILL", "SKILL", "skill-123", Instant.now(), "192.168.1.1"
),
Map.of(
"id", "log-2",
"userId", "user-2",
"action", "UPDATE_NAMESPACE",
"resourceType", "NAMESPACE",
"resourceId", "ns-456",
"timestamp", Instant.now().minusSeconds(3600).toString(),
"ipAddress", "192.168.1.2"
new AuditLogItemResponse(
"log-2", "user-2", "UPDATE_NAMESPACE", "NAMESPACE", "ns-456",
Instant.now().minusSeconds(3600), "192.168.1.2"
)
),
"total", 2,
"page", page,
"size", size
);
return ok("response.success.read", PageResponse.from(new PageImpl<>(logs)));
}
}

View file

@ -1,57 +1,53 @@
package com.iflytek.skillhub.controller.admin;
import com.iflytek.skillhub.controller.BaseApiController;
import com.iflytek.skillhub.dto.AdminUserMutationResponse;
import com.iflytek.skillhub.dto.AdminUserRoleUpdateRequest;
import com.iflytek.skillhub.dto.AdminUserStatusUpdateRequest;
import com.iflytek.skillhub.dto.AdminUserSummaryResponse;
import com.iflytek.skillhub.dto.ApiResponse;
import com.iflytek.skillhub.dto.ApiResponseFactory;
import com.iflytek.skillhub.dto.PageResponse;
import jakarta.validation.Valid;
import org.springframework.data.domain.PageImpl;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.web.bind.annotation.*;
import java.util.List;
import java.util.Map;
@RestController
@RequestMapping("/api/v1/admin/users")
public class UserManagementController {
public class UserManagementController extends BaseApiController {
public UserManagementController(ApiResponseFactory responseFactory) {
super(responseFactory);
}
@GetMapping
@PreAuthorize("hasAnyRole('USER_ADMIN', 'SUPER_ADMIN')")
public Map<String, Object> listUsers(
public ApiResponse<PageResponse<AdminUserSummaryResponse>> listUsers(
@RequestParam(defaultValue = "0") int page,
@RequestParam(defaultValue = "20") int size) {
// Placeholder implementation
return Map.of(
"users", List.of(
Map.of("id", "user-1", "username", "alice", "role", "USER", "status", "ACTIVE"),
Map.of("id", "user-2", "username", "bob", "role", "USER", "status", "ACTIVE")
),
"total", 2,
"page", page,
"size", size
List<AdminUserSummaryResponse> users = List.of(
new AdminUserSummaryResponse("user-1", "alice", "USER", "ACTIVE"),
new AdminUserSummaryResponse("user-2", "bob", "USER", "ACTIVE")
);
return ok("response.success.read", PageResponse.from(new PageImpl<>(users)));
}
@PutMapping("/{userId}/role")
@PreAuthorize("hasAnyRole('USER_ADMIN', 'SUPER_ADMIN')")
public Map<String, Object> updateUserRole(
public ApiResponse<AdminUserMutationResponse> updateUserRole(
@PathVariable String userId,
@RequestBody Map<String, String> request) {
String newRole = request.get("role");
// Placeholder implementation
return Map.of(
"userId", userId,
"role", newRole,
"message", "Role updated successfully"
);
@Valid @RequestBody AdminUserRoleUpdateRequest request) {
return ok("response.success.updated", new AdminUserMutationResponse(userId, request.role(), null));
}
@PutMapping("/{userId}/status")
@PreAuthorize("hasAnyRole('USER_ADMIN', 'SUPER_ADMIN')")
public Map<String, Object> updateUserStatus(
public ApiResponse<AdminUserMutationResponse> updateUserStatus(
@PathVariable String userId,
@RequestBody Map<String, String> request) {
String newStatus = request.get("status");
// Placeholder implementation
return Map.of(
"userId", userId,
"status", newStatus,
"message", "Status updated successfully"
);
@Valid @RequestBody AdminUserStatusUpdateRequest request) {
return ok("response.success.updated", new AdminUserMutationResponse(userId, null, request.status()));
}
}

View file

@ -69,7 +69,7 @@ public class PromotionController extends BaseApiController {
String comment = request != null ? request.comment() : null;
Set<String> platformRoles = rbacService.getUserRoleCodes(userId);
PromotionRequest promotion = promotionService.approvePromotion(id, userId, comment, platformRoles);
return ok("response.success.update", toResponse(promotion));
return ok("response.success.updated", toResponse(promotion));
}
@PostMapping("/{id}/reject")
@ -80,7 +80,7 @@ public class PromotionController extends BaseApiController {
String comment = request != null ? request.comment() : null;
Set<String> platformRoles = rbacService.getUserRoleCodes(userId);
PromotionRequest promotion = promotionService.rejectPromotion(id, userId, comment, platformRoles);
return ok("response.success.update", toResponse(promotion));
return ok("response.success.updated", toResponse(promotion));
}
@GetMapping("/pending")

View file

@ -74,7 +74,7 @@ public class ReviewController extends BaseApiController {
Set<String> platformRoles = rbacService.getUserRoleCodes(userId);
ReviewTask task = reviewService.approveReview(id, userId, comment,
userNsRoles != null ? userNsRoles : Map.of(), platformRoles);
return ok("response.success.update", toResponse(task));
return ok("response.success.updated", toResponse(task));
}
@PostMapping("/{id}/reject")
@ -87,7 +87,7 @@ public class ReviewController extends BaseApiController {
Set<String> platformRoles = rbacService.getUserRoleCodes(userId);
ReviewTask task = reviewService.rejectReview(id, userId, comment,
userNsRoles != null ? userNsRoles : Map.of(), platformRoles);
return ok("response.success.update", toResponse(task));
return ok("response.success.updated", toResponse(task));
}
@PostMapping("/{id}/withdraw")
@ -96,7 +96,7 @@ public class ReviewController extends BaseApiController {
@RequestAttribute("userId") String userId) {
ReviewTask task = reviewTaskRepository.findById(id).orElseThrow();
reviewService.withdrawReview(task.getSkillVersionId(), userId);
return ok("response.success.update", null);
return ok("response.success.updated", null);
}
@GetMapping("/pending")

View file

@ -4,12 +4,12 @@ import com.iflytek.skillhub.auth.rbac.PlatformPrincipal;
import com.iflytek.skillhub.controller.BaseApiController;
import com.iflytek.skillhub.dto.ApiResponse;
import com.iflytek.skillhub.dto.ApiResponseFactory;
import com.iflytek.skillhub.dto.SkillRatingRequest;
import com.iflytek.skillhub.dto.SkillRatingStatusResponse;
import com.iflytek.skillhub.domain.social.SkillRatingService;
import org.springframework.http.ResponseEntity;
import jakarta.validation.Valid;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.*;
import java.util.Map;
import java.util.Optional;
@RestController
@ -25,24 +25,25 @@ public class SkillRatingController extends BaseApiController {
}
@PutMapping("/{skillId}/rating")
public ResponseEntity<Void> rateSkill(
public ApiResponse<Void> rateSkill(
@PathVariable Long skillId,
@RequestBody Map<String, Short> request,
@Valid @RequestBody SkillRatingRequest request,
@AuthenticationPrincipal PlatformPrincipal principal) {
Short score = request.get("score");
skillRatingService.rate(skillId, principal.userId(), score);
return ResponseEntity.noContent().build();
skillRatingService.rate(skillId, principal.userId(), request.score());
return ok("response.success.updated", null);
}
@GetMapping("/{skillId}/rating")
public ApiResponse<Map<String, Object>> getUserRating(
public ApiResponse<SkillRatingStatusResponse> getUserRating(
@PathVariable Long skillId,
@AuthenticationPrincipal PlatformPrincipal principal) {
Optional<Short> rating = skillRatingService.getUserRating(skillId, principal.userId());
Map<String, Object> data = Map.of(
"score", rating.orElse((short) 0),
"rated", rating.isPresent()
return ok(
"response.success.read",
new SkillRatingStatusResponse(
rating.orElse((short) 0),
rating.isPresent()
)
);
return ok("response.success.skill.rating.get", data);
}
}

View file

@ -5,7 +5,6 @@ import com.iflytek.skillhub.controller.BaseApiController;
import com.iflytek.skillhub.dto.ApiResponse;
import com.iflytek.skillhub.dto.ApiResponseFactory;
import com.iflytek.skillhub.domain.social.SkillStarService;
import org.springframework.http.ResponseEntity;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.*;
@ -22,19 +21,19 @@ public class SkillStarController extends BaseApiController {
}
@PutMapping("/{skillId}/star")
public ResponseEntity<Void> starSkill(
public ApiResponse<Void> starSkill(
@PathVariable Long skillId,
@AuthenticationPrincipal PlatformPrincipal principal) {
skillStarService.star(skillId, principal.userId());
return ResponseEntity.noContent().build();
return ok("response.success.updated", null);
}
@DeleteMapping("/{skillId}/star")
public ResponseEntity<Void> unstarSkill(
public ApiResponse<Void> unstarSkill(
@PathVariable Long skillId,
@AuthenticationPrincipal PlatformPrincipal principal) {
skillStarService.unstar(skillId, principal.userId());
return ResponseEntity.noContent().build();
return ok("response.success.updated", null);
}
@GetMapping("/{skillId}/star")
@ -42,6 +41,6 @@ public class SkillStarController extends BaseApiController {
@PathVariable Long skillId,
@AuthenticationPrincipal PlatformPrincipal principal) {
boolean starred = skillStarService.isStarred(skillId, principal.userId());
return ok("response.success.skill.star.check", starred);
return ok("response.success.read", starred);
}
}

View file

@ -0,0 +1,8 @@
package com.iflytek.skillhub.dto;
public record AdminUserMutationResponse(
String userId,
String role,
String status
) {
}

View file

@ -0,0 +1,9 @@
package com.iflytek.skillhub.dto;
import jakarta.validation.constraints.NotBlank;
public record AdminUserRoleUpdateRequest(
@NotBlank(message = "{error.badRequest}")
String role
) {
}

View file

@ -0,0 +1,9 @@
package com.iflytek.skillhub.dto;
import jakarta.validation.constraints.NotBlank;
public record AdminUserStatusUpdateRequest(
@NotBlank(message = "{error.badRequest}")
String status
) {
}

View file

@ -0,0 +1,9 @@
package com.iflytek.skillhub.dto;
public record AdminUserSummaryResponse(
String userId,
String username,
String role,
String status
) {
}

View file

@ -0,0 +1,14 @@
package com.iflytek.skillhub.dto;
import java.time.Instant;
public record AuditLogItemResponse(
String id,
String userId,
String action,
String resourceType,
String resourceId,
Instant timestamp,
String ipAddress
) {
}

View file

@ -0,0 +1,9 @@
package com.iflytek.skillhub.dto;
import jakarta.validation.constraints.NotNull;
public record SkillRatingRequest(
@NotNull(message = "{error.badRequest}")
Short score
) {
}

View file

@ -0,0 +1,7 @@
package com.iflytek.skillhub.dto;
public record SkillRatingStatusResponse(
short score,
boolean rated
) {
}

View file

@ -47,11 +47,12 @@ class DeviceAuthControllerTest {
mockMvc.perform(post("/api/v1/cli/auth/device/code")
.contentType(MediaType.APPLICATION_JSON))
.andExpect(status().isOk())
.andExpect(jsonPath("$.deviceCode").value("device_abc123"))
.andExpect(jsonPath("$.userCode").value("ABCD-1234"))
.andExpect(jsonPath("$.verificationUri").value("https://skillhub.example.com/device"))
.andExpect(jsonPath("$.expiresIn").value(900))
.andExpect(jsonPath("$.interval").value(5));
.andExpect(jsonPath("$.code").value(0))
.andExpect(jsonPath("$.data.deviceCode").value("device_abc123"))
.andExpect(jsonPath("$.data.userCode").value("ABCD-1234"))
.andExpect(jsonPath("$.data.verificationUri").value("https://skillhub.example.com/device"))
.andExpect(jsonPath("$.data.expiresIn").value(900))
.andExpect(jsonPath("$.data.interval").value(5));
}
@Test
@ -64,8 +65,9 @@ class DeviceAuthControllerTest {
.contentType(MediaType.APPLICATION_JSON)
.content("{\"deviceCode\": \"device_abc123\"}"))
.andExpect(status().isOk())
.andExpect(jsonPath("$.error").value("authorization_pending"))
.andExpect(jsonPath("$.accessToken").isEmpty())
.andExpect(jsonPath("$.tokenType").isEmpty());
.andExpect(jsonPath("$.code").value(0))
.andExpect(jsonPath("$.data.error").value("authorization_pending"))
.andExpect(jsonPath("$.data.accessToken").isEmpty())
.andExpect(jsonPath("$.data.tokenType").isEmpty());
}
}

View file

@ -41,7 +41,7 @@ class SkillRatingControllerTest {
private NamespaceMemberRepository namespaceMemberRepository;
@Test
void rate_skill_returns_204() throws Exception {
void rate_skill_returns_envelope() throws Exception {
PlatformPrincipal principal = new PlatformPrincipal(
"user-42",
"tester",
@ -61,7 +61,10 @@ class SkillRatingControllerTest {
.with(csrf())
.contentType(MediaType.APPLICATION_JSON)
.content("{\"score\": 4}"))
.andExpect(status().isNoContent());
.andExpect(status().isOk())
.andExpect(jsonPath("$.code").value(0))
.andExpect(jsonPath("$.timestamp").isNotEmpty())
.andExpect(jsonPath("$.requestId").isNotEmpty());
verify(skillRatingService).rate(eq(10L), eq("user-42"), eq((short) 4));
}

View file

@ -39,7 +39,7 @@ class SkillStarControllerTest {
private NamespaceMemberRepository namespaceMemberRepository;
@Test
void star_skill_returns_204() throws Exception {
void star_skill_returns_envelope() throws Exception {
PlatformPrincipal principal = new PlatformPrincipal(
"user-42",
"tester",
@ -57,13 +57,16 @@ class SkillStarControllerTest {
mockMvc.perform(put("/api/v1/skills/10/star")
.with(authentication(auth))
.with(csrf()))
.andExpect(status().isNoContent());
.andExpect(status().isOk())
.andExpect(jsonPath("$.code").value(0))
.andExpect(jsonPath("$.timestamp").isNotEmpty())
.andExpect(jsonPath("$.requestId").isNotEmpty());
verify(skillStarService).star(eq(10L), eq("user-42"));
}
@Test
void unstar_skill_returns_204() throws Exception {
void unstar_skill_returns_envelope() throws Exception {
PlatformPrincipal principal = new PlatformPrincipal(
"user-42",
"tester",
@ -81,7 +84,10 @@ class SkillStarControllerTest {
mockMvc.perform(delete("/api/v1/skills/10/star")
.with(authentication(auth))
.with(csrf()))
.andExpect(status().isNoContent());
.andExpect(status().isOk())
.andExpect(jsonPath("$.code").value(0))
.andExpect(jsonPath("$.timestamp").isNotEmpty())
.andExpect(jsonPath("$.requestId").isNotEmpty());
verify(skillStarService).unstar(eq(10L), eq("user-42"));
}

View file

@ -55,8 +55,9 @@ class AuditLogControllerTest {
mockMvc.perform(get("/api/v1/admin/audit-logs").with(authentication(auth)))
.andExpect(status().isOk())
.andExpect(jsonPath("$.logs").isArray())
.andExpect(jsonPath("$.total").value(2));
.andExpect(jsonPath("$.code").value(0))
.andExpect(jsonPath("$.data.items").isArray())
.andExpect(jsonPath("$.data.total").value(2));
}
@Test
@ -70,7 +71,7 @@ class AuditLogControllerTest {
mockMvc.perform(get("/api/v1/admin/audit-logs").with(authentication(auth)))
.andExpect(status().isOk())
.andExpect(jsonPath("$.logs").isArray());
.andExpect(jsonPath("$.data.items").isArray());
}
@Test
@ -87,6 +88,6 @@ class AuditLogControllerTest {
.param("action", "CREATE_SKILL")
.with(authentication(auth)))
.andExpect(status().isOk())
.andExpect(jsonPath("$.logs").isArray());
.andExpect(jsonPath("$.data.items").isArray());
}
}

View file

@ -58,8 +58,9 @@ class UserManagementControllerTest {
mockMvc.perform(get("/api/v1/admin/users").with(authentication(auth)))
.andExpect(status().isOk())
.andExpect(jsonPath("$.users").isArray())
.andExpect(jsonPath("$.total").value(2));
.andExpect(jsonPath("$.code").value(0))
.andExpect(jsonPath("$.data.items").isArray())
.andExpect(jsonPath("$.data.total").value(2));
}
@Test
@ -73,7 +74,7 @@ class UserManagementControllerTest {
mockMvc.perform(get("/api/v1/admin/users").with(authentication(auth)))
.andExpect(status().isOk())
.andExpect(jsonPath("$.users").isArray());
.andExpect(jsonPath("$.data.items").isArray());
}
@Test
@ -93,8 +94,9 @@ class UserManagementControllerTest {
.contentType(APPLICATION_JSON)
.content(requestBody))
.andExpect(status().isOk())
.andExpect(jsonPath("$.userId").value("user-123"))
.andExpect(jsonPath("$.role").value("MODERATOR"));
.andExpect(jsonPath("$.code").value(0))
.andExpect(jsonPath("$.data.userId").value("user-123"))
.andExpect(jsonPath("$.data.role").value("MODERATOR"));
}
@Test
@ -114,7 +116,8 @@ class UserManagementControllerTest {
.contentType(APPLICATION_JSON)
.content(requestBody))
.andExpect(status().isOk())
.andExpect(jsonPath("$.userId").value("user-123"))
.andExpect(jsonPath("$.status").value("BANNED"));
.andExpect(jsonPath("$.code").value(0))
.andExpect(jsonPath("$.data.userId").value("user-123"))
.andExpect(jsonPath("$.data.status").value("BANNED"));
}
}