mirror of
https://github.com/iflytek/skillhub.git
synced 2026-10-09 03:17:52 +00:00
fix(audit): render audit detail JSON with Jackson instead of concatenation
audit_log.detail_json is a JSONB column, so the value has to be valid
JSON or the insert fails. It was built by string concatenation at every
call site, with three inconsistent levels of escaping: none at all in
ClawHubCompatAppService, DeviceAuthWebController, LabelAdminAppService
and SkillLabelAppService; quotes only in ReviewPortalAppService,
SkillLifecycleAppService, NamespaceGovernanceService and
SkillGovernanceService; quotes and backslashes in
PromotionPortalAppService.escapeJson.
None of the three escapes control characters, which JSON forbids raw
inside a string. A reviewer pressing Enter in a review comment therefore
produced a payload PostgreSQL rejects, and because the audit write
happens after the domain mutation, the review was already approved when
the request returned 500.
Add AuditDetail, which renders the payload through Jackson, and route
all 29 construction sites through it. 17 of those interpolate a string
value and are the actual defect surface; the numeric and constant ones
are converted too so there is one way to build audit detail and no
hand-rolled example left to copy.
SkillHardDeleteService.toAuditPayload already did this correctly with a
LinkedHashMap and an ObjectMapper; AuditDetail is that shape extracted.
The service itself is left alone rather than changing its constructor
signature for no behavior gain.
Output is byte-identical for values that were already escaped correctly,
so the existing exact-string assertions in AdminSearchControllerTest and
PromotionPortalAppServiceTest are unchanged. null still means "no
detail": the builder returns null rather than {} when no field is set.
Addresses the JSON half of #615. The transaction half -- the domain
mutation and the audit write not sharing one transaction -- is a
separate design decision about whether an audit failure should roll back
a review, and is not bundled here.
Signed-off-by: FenjuFu <fufenjupku@gmail.com>
This commit is contained in:
parent
b896c698cd
commit
215ab11b09
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