mirror of
https://github.com/iflytek/skillhub.git
synced 2026-09-08 22:21:09 +00:00
Merge pull request #782 from FenjuFu/fix/audit-detail-json
fix(audit): render audit detail JSON with Jackson instead of string concatenation
This commit is contained in:
commit
d1cd3d2afe
16 changed files with 309 additions and 48 deletions
|
|
@ -12,6 +12,7 @@ import com.iflytek.skillhub.compat.dto.ClawHubUnstarResponse;
|
|||
import com.iflytek.skillhub.compat.dto.ClawHubWhoamiResponse;
|
||||
import com.iflytek.skillhub.controller.support.MultipartPackageExtractor;
|
||||
import com.iflytek.skillhub.controller.support.ZipPackageExtractor;
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import com.iflytek.skillhub.domain.namespace.NamespaceRole;
|
||||
import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException;
|
||||
|
|
@ -345,7 +346,7 @@ public class ClawHubCompatAppService {
|
|||
confirmWarnings
|
||||
);
|
||||
recordCompatPublishAudit(principal.userId(), result.version().getId(), clientIp, userAgent,
|
||||
"{\"namespace\":\"" + namespace + "\",\"slug\":\"" + extracted.payload().slug() + "\"}");
|
||||
AuditDetail.of("namespace", namespace, "slug", extracted.payload().slug()));
|
||||
return new ClawHubPublishResponse(result.skillId().toString(), result.version().getId().toString());
|
||||
}
|
||||
|
||||
|
|
@ -364,7 +365,7 @@ public class ClawHubCompatAppService {
|
|||
confirmWarnings
|
||||
);
|
||||
recordCompatPublishAudit(principal.userId(), result.version().getId(), clientIp, userAgent,
|
||||
"{\"namespace\":\"" + namespace + "\"}");
|
||||
AuditDetail.of("namespace", namespace));
|
||||
return new ClawHubPublishResponse(result.skillId().toString(), result.version().getId().toString());
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ package com.iflytek.skillhub.controller;
|
|||
|
||||
import com.iflytek.skillhub.auth.device.DeviceAuthService;
|
||||
import com.iflytek.skillhub.auth.rbac.PlatformPrincipal;
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import com.iflytek.skillhub.dto.ApiResponse;
|
||||
import com.iflytek.skillhub.dto.ApiResponseFactory;
|
||||
|
|
@ -51,7 +52,7 @@ public class DeviceAuthWebController extends BaseApiController {
|
|||
requestIdAccessor.current(),
|
||||
httpRequest.getRemoteAddr(),
|
||||
httpRequest.getHeader("User-Agent"),
|
||||
"{\"userCode\":\"" + request.userCode() + "\"}"
|
||||
AuditDetail.of("userCode", request.userCode())
|
||||
);
|
||||
return ok("response.success.updated", new MessageResponse("Device authorized successfully"));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ package com.iflytek.skillhub.controller.admin;
|
|||
|
||||
import com.iflytek.skillhub.auth.rbac.PlatformPrincipal;
|
||||
import com.iflytek.skillhub.controller.BaseApiController;
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.dto.ApiResponse;
|
||||
import com.iflytek.skillhub.dto.ApiResponseFactory;
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
|
|
@ -48,7 +49,7 @@ public class AdminSearchController extends BaseApiController {
|
|||
requestIdAccessor.current(),
|
||||
httpRequest.getRemoteAddr(),
|
||||
httpRequest.getHeader("User-Agent"),
|
||||
"{\"scope\":\"ALL\"}"
|
||||
AuditDetail.of("scope", "ALL")
|
||||
);
|
||||
return ok("response.success.updated", null);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package com.iflytek.skillhub.service;
|
||||
|
||||
import com.iflytek.skillhub.auth.rbac.RbacService;
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import com.iflytek.skillhub.domain.label.LabelDefinition;
|
||||
import com.iflytek.skillhub.domain.label.LabelDefinitionService;
|
||||
|
|
@ -62,7 +63,7 @@ public class LabelAdminAppService {
|
|||
userId,
|
||||
platformRoles(userId)
|
||||
);
|
||||
recordAudit("LABEL_CREATE", userId, labelDefinition.getId(), auditContext, "{\"slug\":\"" + labelDefinition.getSlug() + "\"}");
|
||||
recordAudit("LABEL_CREATE", userId, labelDefinition.getId(), auditContext, AuditDetail.of("slug", labelDefinition.getSlug()));
|
||||
return toResponse(labelDefinition);
|
||||
}
|
||||
|
||||
|
|
@ -87,7 +88,7 @@ public class LabelAdminAppService {
|
|||
if (!affectedSkillIds.isEmpty()) {
|
||||
afterCommit(() -> labelSearchSyncService.rebuildSkills(affectedSkillIds));
|
||||
}
|
||||
recordAudit("LABEL_UPDATE", userId, updated.getId(), auditContext, "{\"slug\":\"" + updated.getSlug() + "\"}");
|
||||
recordAudit("LABEL_UPDATE", userId, updated.getId(), auditContext, AuditDetail.of("slug", updated.getSlug()));
|
||||
return toResponse(updated);
|
||||
}
|
||||
|
||||
|
|
@ -102,7 +103,7 @@ public class LabelAdminAppService {
|
|||
if (!affectedSkillIds.isEmpty()) {
|
||||
afterCommit(() -> labelSearchSyncService.rebuildSkills(affectedSkillIds));
|
||||
}
|
||||
recordAudit("LABEL_DELETE", userId, existing.getId(), auditContext, "{\"slug\":\"" + slug + "\"}");
|
||||
recordAudit("LABEL_DELETE", userId, existing.getId(), auditContext, AuditDetail.of("slug", slug));
|
||||
}
|
||||
|
||||
@Transactional
|
||||
|
|
@ -118,7 +119,7 @@ public class LabelAdminAppService {
|
|||
List<LabelDefinitionResponse> responses = labelDefinitionService.updateSortOrders(updates, platformRoles(userId)).stream()
|
||||
.map(this::toResponse)
|
||||
.toList();
|
||||
recordAudit("LABEL_SORT_ORDER_UPDATE", userId, null, auditContext, "{\"count\":" + request.items().size() + "}");
|
||||
recordAudit("LABEL_SORT_ORDER_UPDATE", userId, null, auditContext, AuditDetail.of("count", request.items().size()));
|
||||
return responses;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package com.iflytek.skillhub.service;
|
||||
|
||||
import com.iflytek.skillhub.auth.rbac.RbacService;
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import com.iflytek.skillhub.domain.namespace.NamespaceRole;
|
||||
import com.iflytek.skillhub.domain.review.PromotionRequest;
|
||||
|
|
@ -67,7 +68,7 @@ public class PromotionPortalAppService {
|
|||
userId,
|
||||
promotion.getId(),
|
||||
auditContext,
|
||||
"{\"sourceSkillId\":" + sourceSkillId + ",\"sourceVersionId\":" + sourceVersionId + "}"
|
||||
AuditDetail.of("sourceSkillId", sourceSkillId, "sourceVersionId", sourceVersionId)
|
||||
);
|
||||
return governanceQueryRepository.getPromotionResponse(promotion);
|
||||
}
|
||||
|
|
@ -247,24 +248,9 @@ public class PromotionPortalAppService {
|
|||
|
||||
private String detailWithComment(String comment, boolean selfReview) {
|
||||
boolean hasComment = comment != null && !comment.isBlank();
|
||||
if (!hasComment && !selfReview) {
|
||||
return null;
|
||||
}
|
||||
StringBuilder detail = new StringBuilder("{");
|
||||
if (hasComment) {
|
||||
detail.append("\"comment\":\"").append(escapeJson(comment)).append("\"");
|
||||
}
|
||||
if (selfReview) {
|
||||
if (hasComment) {
|
||||
detail.append(",");
|
||||
}
|
||||
detail.append("\"selfReview\":true");
|
||||
}
|
||||
detail.append("}");
|
||||
return detail.toString();
|
||||
}
|
||||
|
||||
private String escapeJson(String value) {
|
||||
return value.replace("\\", "\\\\").replace("\"", "\\\"");
|
||||
return AuditDetail.builder()
|
||||
.put("comment", hasComment ? comment : null)
|
||||
.put("selfReview", selfReview ? Boolean.TRUE : null)
|
||||
.build();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package com.iflytek.skillhub.service;
|
||||
|
||||
import com.iflytek.skillhub.auth.rbac.RbacService;
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import com.iflytek.skillhub.domain.namespace.Namespace;
|
||||
import com.iflytek.skillhub.domain.namespace.NamespaceRepository;
|
||||
|
|
@ -62,7 +63,7 @@ public class ReviewPortalAppService {
|
|||
normalizeRoles(userNsRoles),
|
||||
platformRoles(userId)
|
||||
);
|
||||
recordAudit("REVIEW_SUBMIT", userId, task.getId(), auditContext, "{\"skillVersionId\":" + skillVersionId + "}");
|
||||
recordAudit("REVIEW_SUBMIT", userId, task.getId(), auditContext, AuditDetail.of("skillVersionId", skillVersionId));
|
||||
return governanceQueryRepository.getReviewTaskResponse(task);
|
||||
}
|
||||
|
||||
|
|
@ -109,7 +110,7 @@ public class ReviewPortalAppService {
|
|||
userId,
|
||||
reviewTaskId,
|
||||
auditContext,
|
||||
"{\"skillVersionId\":" + task.getSkillVersionId() + "}"
|
||||
AuditDetail.of("skillVersionId", task.getSkillVersionId())
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -270,6 +271,6 @@ public class ReviewPortalAppService {
|
|||
if (comment == null || comment.isBlank()) {
|
||||
return null;
|
||||
}
|
||||
return "{\"comment\":\"" + comment.replace("\"", "\\\"") + "\"}";
|
||||
return AuditDetail.of("comment", comment);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package com.iflytek.skillhub.service;
|
||||
|
||||
import com.iflytek.skillhub.auth.rbac.RbacService;
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import com.iflytek.skillhub.domain.label.LabelDefinition;
|
||||
import com.iflytek.skillhub.domain.label.LabelDefinitionService;
|
||||
|
|
@ -96,7 +97,7 @@ public class SkillLabelAppService {
|
|||
platformRoles(userId)
|
||||
);
|
||||
afterCommit(() -> labelSearchSyncService.rebuildSkill(skill.getId()));
|
||||
recordAudit("SKILL_LABEL_ATTACH", userId, skill.getId(), auditContext, "{\"labelSlug\":\"" + labelSlug + "\"}");
|
||||
recordAudit("SKILL_LABEL_ATTACH", userId, skill.getId(), auditContext, AuditDetail.of("labelSlug", labelSlug));
|
||||
return toDtos(List.of(attached)).getFirst();
|
||||
}
|
||||
|
||||
|
|
@ -116,7 +117,7 @@ public class SkillLabelAppService {
|
|||
platformRoles(userId)
|
||||
);
|
||||
afterCommit(() -> labelSearchSyncService.rebuildSkill(skill.getId()));
|
||||
recordAudit("SKILL_LABEL_DETACH", userId, skill.getId(), auditContext, "{\"labelSlug\":\"" + labelSlug + "\"}");
|
||||
recordAudit("SKILL_LABEL_DETACH", userId, skill.getId(), auditContext, AuditDetail.of("labelSlug", labelSlug));
|
||||
return new MessageResponse("Label detached");
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
package com.iflytek.skillhub.service;
|
||||
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import com.iflytek.skillhub.domain.namespace.Namespace;
|
||||
import com.iflytek.skillhub.domain.namespace.NamespaceRepository;
|
||||
|
|
@ -128,7 +129,7 @@ public class SkillLifecycleAppService {
|
|||
null,
|
||||
auditContext.clientIp(),
|
||||
auditContext.userAgent(),
|
||||
"{\"version\":\"" + version.replace("\"", "\\\"") + "\"}"
|
||||
AuditDetail.of("version", version)
|
||||
);
|
||||
return new SkillLifecycleMutationResponse(
|
||||
skill.getId(),
|
||||
|
|
@ -165,8 +166,7 @@ public class SkillLifecycleAppService {
|
|||
null,
|
||||
auditContext.clientIp(),
|
||||
auditContext.userAgent(),
|
||||
"{\"sourceVersion\":\"" + version.replace("\"", "\\\"")
|
||||
+ "\",\"targetVersion\":\"" + targetVersion.replace("\"", "\\\"") + "\"}"
|
||||
AuditDetail.of("sourceVersion", version, "targetVersion", targetVersion)
|
||||
);
|
||||
return new SkillLifecycleMutationResponse(
|
||||
result.skillId(),
|
||||
|
|
@ -201,7 +201,7 @@ public class SkillLifecycleAppService {
|
|||
null,
|
||||
auditContext.clientIp(),
|
||||
auditContext.userAgent(),
|
||||
"{\"version\":\"" + version.replace("\"", "\\\"") + "\",\"targetVisibility\":\"" + targetVisibility + "\"}"
|
||||
AuditDetail.of("version", version, "targetVisibility", targetVisibility)
|
||||
);
|
||||
return new SkillLifecycleMutationResponse(
|
||||
skill.getId(),
|
||||
|
|
@ -234,7 +234,7 @@ public class SkillLifecycleAppService {
|
|||
null,
|
||||
auditContext.clientIp(),
|
||||
auditContext.userAgent(),
|
||||
"{\"version\":\"" + version.replace("\"", "\\\"") + "\"}"
|
||||
AuditDetail.of("version", version)
|
||||
);
|
||||
return new SkillLifecycleMutationResponse(
|
||||
skill.getId(),
|
||||
|
|
|
|||
|
|
@ -134,6 +134,37 @@ class PromotionPortalAppServiceTest {
|
|||
);
|
||||
}
|
||||
|
||||
@Test
|
||||
void approvePromotion_escapesMultiLineReviewCommentIntoValidJson() {
|
||||
// Regression: the detail column is JSONB. A reviewer pressing Enter used
|
||||
// to produce a raw newline inside the JSON string, so the audit insert
|
||||
// failed after the promotion had already been approved.
|
||||
String comment = "looks good\nbut rename it \"foo\"\tfirst\\done";
|
||||
PromotionRequest promotion = promotionRequest(PROMOTION_ID, SUBMITTER_ID);
|
||||
when(rbacService.getUserRoleCodes(REVIEWER_ID)).thenReturn(Set.of("SKILL_ADMIN"));
|
||||
when(promotionService.approvePromotion(PROMOTION_ID, REVIEWER_ID, comment, Set.of("SKILL_ADMIN")))
|
||||
.thenReturn(promotion);
|
||||
when(governanceQueryRepository.getPromotionResponse(promotion)).thenReturn(response(promotion));
|
||||
|
||||
service.approvePromotion(
|
||||
PROMOTION_ID,
|
||||
comment,
|
||||
REVIEWER_ID,
|
||||
new AuditRequestContext("127.0.0.1", "JUnit")
|
||||
);
|
||||
|
||||
verify(auditLogService).record(
|
||||
eq(REVIEWER_ID),
|
||||
eq("PROMOTION_APPROVE"),
|
||||
eq("PROMOTION_REQUEST"),
|
||||
eq(PROMOTION_ID),
|
||||
eq(null),
|
||||
eq("127.0.0.1"),
|
||||
eq("JUnit"),
|
||||
eq("{\"comment\":\"looks good\\nbut rename it \\\"foo\\\"\\tfirst\\\\done\"}")
|
||||
);
|
||||
}
|
||||
|
||||
private PromotionResponseDto response(PromotionRequest request) {
|
||||
return new PromotionResponseDto(
|
||||
request.getId(),
|
||||
|
|
|
|||
|
|
@ -0,0 +1,86 @@
|
|||
package com.iflytek.skillhub.domain.audit;
|
||||
|
||||
import com.fasterxml.jackson.core.JsonProcessingException;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.Map;
|
||||
|
||||
/**
|
||||
* Builds the JSON stored in {@code audit_log.detail_json}.
|
||||
*
|
||||
* <p>That column is PostgreSQL {@code JSONB}, so the value has to be valid JSON or the
|
||||
* insert fails. Hand-concatenating it does not survive real input: a value containing a
|
||||
* newline, tab, backslash, or any other control character produces a string PostgreSQL
|
||||
* rejects, and the audit write then fails after the domain mutation has already
|
||||
* committed. Escaping only {@code "} — or only {@code "} and {@code \} — is not enough.
|
||||
*
|
||||
* <p>Every audit detail payload should be built here so there is exactly one place where
|
||||
* that escaping is decided.
|
||||
*
|
||||
* <p>The mapper is a private static instance rather than the injected application bean on
|
||||
* purpose: audit records are a stored format, and they should not change shape because
|
||||
* someone reconfigures Jackson elsewhere in the application.
|
||||
*/
|
||||
public final class AuditDetail {
|
||||
|
||||
private static final ObjectMapper MAPPER = new ObjectMapper();
|
||||
|
||||
private AuditDetail() {
|
||||
}
|
||||
|
||||
/**
|
||||
* Renders a single-field detail payload, e.g. {@code {"slug":"my-skill"}}.
|
||||
*/
|
||||
public static String of(String key, Object value) {
|
||||
return builder().put(key, value).build();
|
||||
}
|
||||
|
||||
/**
|
||||
* Renders a two-field detail payload, preserving argument order.
|
||||
*/
|
||||
public static String of(String key1, Object value1, String key2, Object value2) {
|
||||
return builder().put(key1, value1).put(key2, value2).build();
|
||||
}
|
||||
|
||||
public static Builder builder() {
|
||||
return new Builder();
|
||||
}
|
||||
|
||||
/**
|
||||
* Accumulates fields in insertion order. Use it when a field is conditional.
|
||||
*/
|
||||
public static final class Builder {
|
||||
|
||||
private final Map<String, Object> fields = new LinkedHashMap<>();
|
||||
|
||||
private Builder() {
|
||||
}
|
||||
|
||||
/**
|
||||
* Adds a field. A {@code null} value is skipped, so an optional field can be
|
||||
* offered unconditionally.
|
||||
*/
|
||||
public Builder put(String key, Object value) {
|
||||
if (value != null) {
|
||||
fields.put(key, value);
|
||||
}
|
||||
return this;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the rendered JSON, or {@code null} when no field was set — the audit
|
||||
* log stores {@code null} rather than an empty object for "no detail".
|
||||
*/
|
||||
public String build() {
|
||||
if (fields.isEmpty()) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
return MAPPER.writeValueAsString(fields);
|
||||
} catch (JsonProcessingException e) {
|
||||
throw new IllegalStateException("Failed to serialize audit detail JSON", e);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,5 +1,6 @@
|
|||
package com.iflytek.skillhub.domain.namespace;
|
||||
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import com.iflytek.skillhub.domain.shared.exception.DomainBadRequestException;
|
||||
import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException;
|
||||
|
|
@ -206,7 +207,7 @@ public class NamespaceGovernanceService {
|
|||
requestId,
|
||||
clientIp,
|
||||
userAgent,
|
||||
reason == null || reason.isBlank() ? null : "{\"reason\":\"" + reason.replace("\"", "\\\"") + "\"}"
|
||||
reason == null || reason.isBlank() ? null : AuditDetail.of("reason", reason)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
package com.iflytek.skillhub.domain.report;
|
||||
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import com.iflytek.skillhub.domain.event.ReportResolvedEvent;
|
||||
import com.iflytek.skillhub.domain.event.ReportSubmittedEvent;
|
||||
|
|
@ -78,7 +79,7 @@ public class SkillReportService {
|
|||
normalize(details)
|
||||
));
|
||||
auditLogService.record(reporterId, "REPORT_SKILL", "SKILL", skillId, null, clientIp, userAgent,
|
||||
"{\"reportId\":" + saved.getId() + "}");
|
||||
AuditDetail.of("reportId", saved.getId()));
|
||||
eventPublisher.publishEvent(new ReportSubmittedEvent(
|
||||
saved.getId(), saved.getSkillId(), saved.getReporterId()));
|
||||
return saved;
|
||||
|
|
@ -120,7 +121,7 @@ public class SkillReportService {
|
|||
"SKILL_REPORT",
|
||||
reportId,
|
||||
"Report handled",
|
||||
"{\"status\":\"RESOLVED\"}"
|
||||
AuditDetail.of("status", "RESOLVED")
|
||||
);
|
||||
return saved;
|
||||
}
|
||||
|
|
@ -146,7 +147,7 @@ public class SkillReportService {
|
|||
"SKILL_REPORT",
|
||||
reportId,
|
||||
"Report dismissed",
|
||||
"{\"status\":\"DISMISSED\"}"
|
||||
AuditDetail.of("status", "DISMISSED")
|
||||
);
|
||||
return saved;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
package com.iflytek.skillhub.domain.review;
|
||||
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.event.PromotionApprovedEvent;
|
||||
import com.iflytek.skillhub.domain.event.PromotionRejectedEvent;
|
||||
import com.iflytek.skillhub.domain.event.PromotionSubmittedEvent;
|
||||
|
|
@ -266,7 +267,7 @@ public class PromotionService {
|
|||
"PROMOTION_REQUEST",
|
||||
promotionId,
|
||||
"Promotion approved",
|
||||
"{\"status\":\"APPROVED\"}"
|
||||
AuditDetail.of("status", "APPROVED")
|
||||
);
|
||||
|
||||
return savedRequest;
|
||||
|
|
@ -323,7 +324,7 @@ public class PromotionService {
|
|||
"PROMOTION_REQUEST",
|
||||
promotionId,
|
||||
"Promotion rejected",
|
||||
"{\"status\":\"REJECTED\"}"
|
||||
AuditDetail.of("status", "REJECTED")
|
||||
);
|
||||
|
||||
return request;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
package com.iflytek.skillhub.domain.review;
|
||||
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.namespace.Namespace;
|
||||
import com.iflytek.skillhub.domain.namespace.NamespaceRepository;
|
||||
import com.iflytek.skillhub.domain.namespace.NamespaceRole;
|
||||
|
|
@ -241,7 +242,7 @@ public class ReviewService {
|
|||
"REVIEW_TASK",
|
||||
reviewTaskId,
|
||||
"Review approved",
|
||||
"{\"status\":\"APPROVED\"}"
|
||||
AuditDetail.of("status", "APPROVED")
|
||||
);
|
||||
|
||||
return task;
|
||||
|
|
@ -294,7 +295,7 @@ public class ReviewService {
|
|||
"REVIEW_TASK",
|
||||
reviewTaskId,
|
||||
"Review rejected",
|
||||
"{\"status\":\"REJECTED\"}"
|
||||
AuditDetail.of("status", "REJECTED")
|
||||
);
|
||||
|
||||
return task;
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
package com.iflytek.skillhub.domain.skill.service;
|
||||
|
||||
import com.iflytek.skillhub.domain.audit.AuditDetail;
|
||||
import com.iflytek.skillhub.domain.audit.AuditLogService;
|
||||
import com.iflytek.skillhub.domain.event.SkillStatusChangedEvent;
|
||||
import com.iflytek.skillhub.domain.namespace.NamespaceRole;
|
||||
|
|
@ -205,7 +206,7 @@ public class SkillGovernanceService {
|
|||
null,
|
||||
clientIp,
|
||||
userAgent,
|
||||
"{\"version\":\"" + version.getVersion().replace("\"", "\\\"") + "\"}"
|
||||
AuditDetail.of("version", version.getVersion())
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -311,7 +312,7 @@ public class SkillGovernanceService {
|
|||
if (reason == null || reason.isBlank()) {
|
||||
return null;
|
||||
}
|
||||
return "{\"reason\":\"" + reason.replace("\"", "\\\"") + "\"}";
|
||||
return AuditDetail.of("reason", reason);
|
||||
}
|
||||
|
||||
private Instant currentInstant() {
|
||||
|
|
|
|||
|
|
@ -0,0 +1,147 @@
|
|||
package com.iflytek.skillhub.domain.audit;
|
||||
|
||||
import com.fasterxml.jackson.databind.JsonNode;
|
||||
import com.fasterxml.jackson.databind.ObjectMapper;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.junit.jupiter.params.ParameterizedTest;
|
||||
import org.junit.jupiter.params.provider.ValueSource;
|
||||
|
||||
import static org.assertj.core.api.Assertions.assertThat;
|
||||
import static org.assertj.core.api.Assertions.assertThatCode;
|
||||
|
||||
class AuditDetailTest {
|
||||
|
||||
private static final ObjectMapper MAPPER = new ObjectMapper();
|
||||
|
||||
// ------------------------------------------------------------------
|
||||
// Shape — the rendered JSON must match what callers wrote by hand
|
||||
// ------------------------------------------------------------------
|
||||
|
||||
@Test
|
||||
void of_rendersSingleField() {
|
||||
assertThat(AuditDetail.of("slug", "my-skill")).isEqualTo("{\"slug\":\"my-skill\"}");
|
||||
}
|
||||
|
||||
@Test
|
||||
void of_preservesArgumentOrderForTwoFields() {
|
||||
assertThat(AuditDetail.of("version", "1.0.0", "targetVisibility", "PUBLIC"))
|
||||
.isEqualTo("{\"version\":\"1.0.0\",\"targetVisibility\":\"PUBLIC\"}");
|
||||
}
|
||||
|
||||
@Test
|
||||
void of_keepsNumbersAndBooleansUnquoted() {
|
||||
assertThat(AuditDetail.of("count", 3)).isEqualTo("{\"count\":3}");
|
||||
assertThat(AuditDetail.of("reportId", 42L)).isEqualTo("{\"reportId\":42}");
|
||||
assertThat(AuditDetail.of("selfReview", Boolean.TRUE)).isEqualTo("{\"selfReview\":true}");
|
||||
}
|
||||
|
||||
@Test
|
||||
void builder_preservesInsertionOrder() {
|
||||
assertThat(AuditDetail.builder()
|
||||
.put("comment", "ship")
|
||||
.put("selfReview", Boolean.TRUE)
|
||||
.build())
|
||||
.isEqualTo("{\"comment\":\"ship\",\"selfReview\":true}");
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------
|
||||
// Escaping — the regression this class exists for
|
||||
// ------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* Every one of these breaks a hand-rolled builder. The audit column is JSONB,
|
||||
* so an unescaped control character makes the insert fail after the domain
|
||||
* mutation has already committed.
|
||||
*/
|
||||
@ParameterizedTest
|
||||
@ValueSource(strings = {
|
||||
"line one\nline two",
|
||||
"carriage\rreturn",
|
||||
"tab\tseparated",
|
||||
"back\\slash",
|
||||
"quote \" inside",
|
||||
"both \\ and \"",
|
||||
"form\ffeed",
|
||||
"backspace\bhere",
|
||||
"中文评审意见\n第二行",
|
||||
"emoji 🚀 and \"quotes\"",
|
||||
})
|
||||
void escapedValuesRoundTripAsValidJson(String raw) throws Exception {
|
||||
String json = AuditDetail.of("comment", raw);
|
||||
|
||||
JsonNode parsed = MAPPER.readTree(json);
|
||||
assertThat(parsed.get("comment").asText()).isEqualTo(raw);
|
||||
}
|
||||
|
||||
/**
|
||||
* Built at runtime rather than in the annotation: a raw control character in
|
||||
* a source literal is fragile, and these are exactly the bytes JSON forbids
|
||||
* unescaped inside a string.
|
||||
*/
|
||||
@Test
|
||||
void lowControlCharactersRoundTripAsValidJson() throws Exception {
|
||||
for (char c = 0; c < 0x20; c++) {
|
||||
String raw = "before" + c + "after";
|
||||
String json = AuditDetail.of("comment", raw);
|
||||
|
||||
JsonNode parsed = MAPPER.readTree(json);
|
||||
assertThat(parsed.get("comment").asText())
|
||||
.as("control character U+%04X", (int) c)
|
||||
.isEqualTo(raw);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void newlineIsEscapedRatherThanEmbeddedRaw() {
|
||||
// The literal two-character sequence backslash-n, not a raw 0x0A.
|
||||
assertThat(AuditDetail.of("comment", "a\nb")).isEqualTo("{\"comment\":\"a\\nb\"}");
|
||||
}
|
||||
|
||||
@Test
|
||||
void multiFieldPayloadWithControlCharactersStaysParseable() throws Exception {
|
||||
String json = AuditDetail.of("sourceVersion", "1.0.0\n", "targetVersion", "2.0.0\t\"x\"");
|
||||
|
||||
JsonNode parsed = MAPPER.readTree(json);
|
||||
assertThat(parsed.get("sourceVersion").asText()).isEqualTo("1.0.0\n");
|
||||
assertThat(parsed.get("targetVersion").asText()).isEqualTo("2.0.0\t\"x\"");
|
||||
}
|
||||
|
||||
@Test
|
||||
void everyAwkwardCharacterAtOnceStillSerializes() throws Exception {
|
||||
// Assembled from char casts: a \\uXXXX escape is expanded by the Java
|
||||
// lexer before parsing, which would put the raw byte back in the literal.
|
||||
String awkward = "" + (char) 0x00 + (char) 0x1F + "\\\"" + "\n\r\t" + "中文";
|
||||
|
||||
assertThatCode(() -> AuditDetail.of("reason", awkward)).doesNotThrowAnyException();
|
||||
assertThat(MAPPER.readTree(AuditDetail.of("reason", awkward)).get("reason").asText())
|
||||
.isEqualTo(awkward);
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------------
|
||||
// Null handling — "no detail" must stay null, not become "{}"
|
||||
// ------------------------------------------------------------------
|
||||
|
||||
@Test
|
||||
void nullValueIsSkipped() {
|
||||
assertThat(AuditDetail.of("comment", null, "selfReview", Boolean.TRUE))
|
||||
.isEqualTo("{\"selfReview\":true}");
|
||||
}
|
||||
|
||||
@Test
|
||||
void allNullValuesProduceNull() {
|
||||
assertThat(AuditDetail.of("comment", null)).isNull();
|
||||
assertThat(AuditDetail.of("comment", null, "selfReview", null)).isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
void emptyBuilderProducesNull() {
|
||||
assertThat(AuditDetail.builder().build()).isNull();
|
||||
}
|
||||
|
||||
@Test
|
||||
void emptyStringIsStillARecordedValue() {
|
||||
// Only null is treated as absent; callers that want to drop blanks keep
|
||||
// their own isBlank() guard.
|
||||
assertThat(AuditDetail.of("comment", "")).isEqualTo("{\"comment\":\"\"}");
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue