diff --git a/docs/superpowers/specs/2026-03-20-skill-detail-hard-delete-design.md b/docs/superpowers/specs/2026-03-20-skill-detail-hard-delete-design.md new file mode 100644 index 00000000..b88c80a8 --- /dev/null +++ b/docs/superpowers/specs/2026-03-20-skill-detail-hard-delete-design.md @@ -0,0 +1,264 @@ +# Skill Detail Hard Delete Design + +## Context + +The project already has a hard-delete API for whole skills at `DELETE /api/v1/skills/{namespace}/{slug}`. That API is token-oriented and restricted to `SUPER_ADMIN` callers with the `skill:delete` scope. The product now needs a skill-detail-page deletion flow that lets skill owners delete their own skills from the UI while keeping the existing token API restricted to super administrators. + +This deletion must remain a physical delete of the whole skill and all versions. The UI must require multiple confirmations, and the backend must be resilient if object storage cleanup fails after the database delete commits. + +## Goals + +- Add a skill-detail-page delete flow under lifecycle management. +- Allow deletion from the UI for: + - the skill owner + - `SUPER_ADMIN` +- Keep the existing token hard-delete API restricted to `SUPER_ADMIN`. +- Require two-stage user confirmation in the UI. +- Continue performing whole-skill physical deletion. +- Make storage cleanup failure recoverable through persistent compensation. + +## Non-Goals + +- No change to the existing token API permission model. +- No soft delete. +- No partial delete of a single version. +- No change to the user-confirmed idempotent semantics for missing skills. + +## User Experience + +### Entry Point + +In the skill detail page lifecycle management section, add a destructive action for deleting the skill. + +Visibility rules: +- visible to the skill owner +- visible to `SUPER_ADMIN` +- hidden for all other users + +### Confirmation Flow + +The deletion flow uses two steps: + +1. Danger confirmation dialog +- explains that the delete is permanent +- explains that all versions, files, and download bundles will be removed +- explains that the action cannot be undone + +2. Input confirmation dialog +- requires the user to type the skill `slug` +- the destructive confirm button remains disabled until the input exactly matches the current skill slug + +### Success Behavior + +After success: +- show a success toast +- navigate away from the deleted detail page +- prefer returning to the logical source page if available +- otherwise fall back to the current default listing route + +### Failure Behavior + +If the delete request fails: +- keep the user on the detail page +- show a destructive error toast with the backend message when available +- keep the skill data unchanged on screen until a successful delete happens + +## API Design + +### Existing API (unchanged) + +`DELETE /api/v1/skills/{namespace}/{slug}` +- for API token callers +- still requires `SUPER_ADMIN` +- still requires `skill:delete` + +### New Portal API + +Add a session-authenticated lifecycle endpoint under the existing skill lifecycle controller family. + +Recommended route: +- `DELETE /api/web/skills/{namespace}/{slug}` + +Authorization: +- allow if current user is the skill owner +- allow if current user has `SUPER_ADMIN` +- reject all others + +Behavior: +- normalize `@namespace` input the same way as current lifecycle endpoints +- if the skill is missing, return a success envelope with `deleted=false` +- if present and authorized, perform whole-skill hard delete and return `deleted=true` + +Response shape: +- reuse the existing delete response DTO shape when practical +- include at minimum: + - `skillId` nullable + - `namespace` + - `slug` + - `deleted` + +## Backend Design + +### App Layer Split + +Keep the current split between controller, app service, and domain delete service. + +- controller: request extraction and response envelope only +- app service: authorization-aware orchestration for the portal use case +- domain service: actual delete of database-linked skill resources + +### Authorization Model + +Portal delete authorization in app service: +- load the skill by namespace and slug +- if missing, return idempotent result +- allow if `principal.userId == skill.ownerId` +- allow if roles contain `SUPER_ADMIN` +- otherwise throw `DomainForbiddenException` + +This keeps owner-aware logic out of the token controller and preserves the existing admin-only token endpoint. + +### Deletion Scope + +The delete still removes: +- the skill row +- all skill versions +- all skill files +- all stored bundles +- all stored file objects +- tags +- reports +- review tasks for the deleted versions +- promotion requests referencing the skill +- stars +- ratings +- version stats +- search index document + +### Storage Failure Compensation + +Current implementation deletes storage after transaction commit and only logs failures. That is not enough for this feature. + +Replace that behavior with persistent compensation: + +1. During deletion, collect all storage keys that must be removed. +2. Commit the database transaction first. +3. After commit, try deleting those storage keys. +4. If storage deletion fails, persist a compensation record containing: +- skill identifier context +- storage keys still pending deletion +- retry count +- last error +- timestamps +5. A scheduled cleanup task retries pending compensation records until success. +6. On successful retry, mark the compensation record completed or delete it. + +This avoids coupling database commit to S3/network availability while still guaranteeing that failed storage cleanup remains actionable and retryable. + +## Data Model + +Add a persistent compensation table for hard-delete storage cleanup. + +Suggested fields: +- `id` +- `skill_id` nullable +- `namespace_slug` +- `skill_slug` +- `storage_keys_json` +- `status` (`PENDING`, `FAILED`, `COMPLETED`) +- `attempt_count` +- `last_error` +- `created_at` +- `updated_at` +- `last_attempt_at` + +Keep the model narrow and purpose-built for storage cleanup retries. + +## Frontend Design + +### Skill Detail Page + +Update the lifecycle section in `skill-detail.tsx`: +- add delete action only when caller is owner or super admin +- keep archive/unarchive behavior unchanged +- visually separate delete from archive/unarchive because it is destructive and permanent + +### Dialog State + +Add two pieces of dialog state: +- first confirmation open/closed +- second input-confirmation open/closed plus typed slug value + +The second dialog opens only after the first is confirmed. + +### Mutation Handling + +Add a dedicated delete mutation hook using the new portal endpoint. + +On success: +- invalidate affected skill queries +- toast success +- navigate away from the deleted page + +On failure: +- toast error +- keep dialogs closed after failure only if that matches the current destructive-action convention; otherwise preserve the second dialog for retry + +## Testing Strategy + +### Backend Tests + +Add tests for: +- portal delete allows owner deleting own skill +- portal delete allows `SUPER_ADMIN` +- portal delete rejects non-owner non-admin +- portal delete remains idempotent for missing skill +- token delete path remains `SUPER_ADMIN`-only +- hard delete still removes all linked data +- storage cleanup failure creates compensation record +- compensation retry task succeeds and clears pending work + +### Frontend Tests + +Add tests for: +- delete button visibility for owner / super admin / unauthorized user +- first confirmation dialog opens from lifecycle section +- second confirmation requires exact slug input +- mutation is not called before exact slug input +- success toast and navigation happen after successful delete +- failure toast appears when delete fails + +## Risks and Mitigations + +### Risk: widening delete permissions accidentally affects token callers +Mitigation: +- keep existing token controller and route rules unchanged +- add owner-aware behavior only on the new portal delete path + +### Risk: deleted page navigation becomes confusing +Mitigation: +- reuse existing return-source helpers where possible +- define one clear fallback route + +### Risk: storage cleanup failure leaves orphaned files +Mitigation: +- persist compensation tasks and retry until cleanup succeeds + +## Files Expected To Change + +Backend: +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleController.java` or a dedicated portal delete controller +- `server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillDeleteAppService.java` +- `server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteService.java` +- new compensation entity/repository/service/task files +- migration file for compensation table +- route and auth tests if needed + +Frontend: +- `web/src/pages/skill-detail.tsx` +- `web/src/shared/hooks/use-skill-queries.ts` +- locale files and new tests as needed + +## Recommendation + +Implement this as a new portal-only delete path while preserving the existing token path. Reuse the current hard-delete core, but upgrade storage cleanup from best-effort logging to persistent compensation and retry. This keeps the security model narrow, meets the UI requirement cleanly, and closes the current operational gap around storage failures. diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillController.java index 3c0f87b8..b88807eb 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillController.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillController.java @@ -75,6 +75,7 @@ public class SkillController extends BaseApiController { detail.id(), detail.slug(), detail.displayName(), + detail.ownerId(), detail.ownerDisplayName(), detail.summary(), detail.visibility(), diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillDeleteController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillDeleteController.java new file mode 100644 index 00000000..675cee83 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillDeleteController.java @@ -0,0 +1,52 @@ +package com.iflytek.skillhub.controller.portal; + +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.SkillDeleteResponse; +import com.iflytek.skillhub.service.AuditRequestContext; +import com.iflytek.skillhub.service.SkillDeleteAppService; +import jakarta.servlet.http.HttpServletRequest; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.security.core.annotation.AuthenticationPrincipal; +import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +/** + * API-token-friendly hard-delete endpoint reserved for super administrators. + */ +@RestController +@RequestMapping("/api/v1/skills") +public class SkillDeleteController extends BaseApiController { + + private final SkillDeleteAppService skillDeleteAppService; + + public SkillDeleteController(SkillDeleteAppService skillDeleteAppService, + ApiResponseFactory responseFactory) { + super(responseFactory); + this.skillDeleteAppService = skillDeleteAppService; + } + + @DeleteMapping("/{namespace}/{slug}") + @PreAuthorize("hasRole('SUPER_ADMIN')") + public ApiResponse deleteSkill(@PathVariable String namespace, + @PathVariable String slug, + @AuthenticationPrincipal PlatformPrincipal principal, + HttpServletRequest request) { + SkillDeleteAppService.DeleteResult result = skillDeleteAppService.deleteSkill( + namespace, + slug, + principal.userId(), + AuditRequestContext.from(request) + ); + return ok("response.success.deleted", new SkillDeleteResponse( + result.skillId(), + result.namespace(), + result.slug(), + result.deleted() + )); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleDeleteController.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleDeleteController.java new file mode 100644 index 00000000..8be92ff2 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/controller/portal/SkillLifecycleDeleteController.java @@ -0,0 +1,47 @@ +package com.iflytek.skillhub.controller.portal; + +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.SkillDeleteResponse; +import com.iflytek.skillhub.service.AuditRequestContext; +import com.iflytek.skillhub.service.SkillDeleteAppService; +import jakarta.servlet.http.HttpServletRequest; +import org.springframework.security.core.annotation.AuthenticationPrincipal; +import org.springframework.web.bind.annotation.DeleteMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RestController; + +@RestController +@RequestMapping("/api/web/skills") +public class SkillLifecycleDeleteController extends BaseApiController { + + private final SkillDeleteAppService skillDeleteAppService; + + public SkillLifecycleDeleteController(SkillDeleteAppService skillDeleteAppService, + ApiResponseFactory responseFactory) { + super(responseFactory); + this.skillDeleteAppService = skillDeleteAppService; + } + + @DeleteMapping("/{namespace}/{slug}") + public ApiResponse deleteSkill(@PathVariable String namespace, + @PathVariable String slug, + @AuthenticationPrincipal PlatformPrincipal principal, + HttpServletRequest httpRequest) { + SkillDeleteAppService.DeleteResult result = skillDeleteAppService.deleteSkillFromPortal( + namespace, + slug, + principal, + AuditRequestContext.from(httpRequest) + ); + return ok("response.success.deleted", new SkillDeleteResponse( + result.skillId(), + result.namespace(), + result.slug(), + result.deleted() + )); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillDeleteResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillDeleteResponse.java new file mode 100644 index 00000000..ce569158 --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillDeleteResponse.java @@ -0,0 +1,9 @@ +package com.iflytek.skillhub.dto; + +public record SkillDeleteResponse( + Long skillId, + String namespace, + String slug, + boolean deleted +) { +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillDetailResponse.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillDetailResponse.java index 1af0c753..8f74efdf 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillDetailResponse.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/dto/SkillDetailResponse.java @@ -6,6 +6,7 @@ public record SkillDetailResponse( Long id, String slug, String displayName, + String ownerId, String ownerDisplayName, String summary, String visibility, diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewSkillDetailAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewSkillDetailAppService.java index 1d36e75d..c2f029d7 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewSkillDetailAppService.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/ReviewSkillDetailAppService.java @@ -59,6 +59,7 @@ public class ReviewSkillDetailAppService { snapshot.skill().getId(), snapshot.skill().getSlug(), snapshot.skill().getDisplayName(), + snapshot.skill().getOwnerId(), snapshot.ownerDisplayName(), snapshot.skill().getSummary(), snapshot.skill().getVisibility().name(), diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillDeleteAppService.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillDeleteAppService.java new file mode 100644 index 00000000..25bd1b6b --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/service/SkillDeleteAppService.java @@ -0,0 +1,97 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.service.SkillHardDeleteService; +import com.iflytek.skillhub.search.SearchIndexService; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +/** + * Orchestrates the API-facing hard-delete flow for whole skills. + */ +@Service +public class SkillDeleteAppService { + + public record DeleteResult(Long skillId, String namespace, String slug, boolean deleted) { + } + + private final SkillRepository skillRepository; + private final SkillHardDeleteService skillHardDeleteService; + private final SearchIndexService searchIndexService; + + public SkillDeleteAppService(SkillRepository skillRepository, + SkillHardDeleteService skillHardDeleteService, + SearchIndexService searchIndexService) { + this.skillRepository = skillRepository; + this.skillHardDeleteService = skillHardDeleteService; + this.searchIndexService = searchIndexService; + } + + @Transactional + public DeleteResult deleteSkill(String namespace, + String slug, + String actorUserId, + AuditRequestContext auditRequestContext) { + return deleteSkillForActor(namespace, slug, actorUserId, auditRequestContext); + } + + @Transactional + public DeleteResult deleteSkillFromPortal(String namespace, + String slug, + PlatformPrincipal principal, + AuditRequestContext auditRequestContext) { + String normalizedNamespace = normalizeNamespace(namespace); + return skillRepository.findByNamespaceSlugAndSlug(normalizedNamespace, slug) + .map(skill -> deleteExistingSkill(skill, normalizedNamespace, slug, principal.userId(), auditRequestContext, true, principal)) + .orElseGet(() -> new DeleteResult(null, normalizedNamespace, slug, false)); + } + + private DeleteResult deleteSkillForActor(String namespace, + String slug, + String actorUserId, + AuditRequestContext auditRequestContext) { + String normalizedNamespace = normalizeNamespace(namespace); + return skillRepository.findByNamespaceSlugAndSlug(normalizedNamespace, slug) + .map(skill -> deleteExistingSkill(skill, normalizedNamespace, slug, actorUserId, auditRequestContext, false, null)) + .orElseGet(() -> new DeleteResult(null, normalizedNamespace, slug, false)); + } + + private DeleteResult deleteExistingSkill(Skill skill, + String namespace, + String slug, + String actorUserId, + AuditRequestContext auditRequestContext, + boolean enforcePortalOwnership, + PlatformPrincipal principal) { + if (enforcePortalOwnership && !canDeleteFromPortal(skill, principal)) { + throw new DomainForbiddenException("error.forbidden"); + } + skillHardDeleteService.hardDeleteSkill( + skill, + namespace, + actorUserId, + auditRequestContext != null ? auditRequestContext.clientIp() : null, + auditRequestContext != null ? auditRequestContext.userAgent() : null + ); + searchIndexService.remove(skill.getId()); + return new DeleteResult(skill.getId(), namespace, slug, true); + } + + private String normalizeNamespace(String namespace) { + if (namespace == null) { + return null; + } + return namespace.startsWith("@") ? namespace.substring(1) : namespace; + } + + private boolean canDeleteFromPortal(Skill skill, PlatformPrincipal principal) { + if (principal == null) { + return false; + } + return principal.platformRoles().contains("SUPER_ADMIN") + || principal.userId().equals(skill.getOwnerId()); + } +} diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillStorageDeletionCompensationTask.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillStorageDeletionCompensationTask.java new file mode 100644 index 00000000..f0eaa08c --- /dev/null +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/task/SkillStorageDeletionCompensationTask.java @@ -0,0 +1,29 @@ +package com.iflytek.skillhub.task; + +import com.iflytek.skillhub.domain.skill.service.SkillStorageDeletionCompensationService; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.scheduling.annotation.Scheduled; +import org.springframework.stereotype.Component; +import org.springframework.transaction.annotation.Transactional; + +@Component +public class SkillStorageDeletionCompensationTask { + + private static final Logger logger = LoggerFactory.getLogger(SkillStorageDeletionCompensationTask.class); + + private final SkillStorageDeletionCompensationService compensationService; + + public SkillStorageDeletionCompensationTask(SkillStorageDeletionCompensationService compensationService) { + this.compensationService = compensationService; + } + + @Scheduled(fixedDelay = 300000) + @Transactional + public void retryPendingCleanup() { + int retried = compensationService.retryPendingCleanup(); + if (retried > 0) { + logger.info("Retried {} pending hard-delete storage cleanup records", retried); + } + } +} diff --git a/server/skillhub-app/src/main/resources/db/migration/V33__skill_delete_storage_compensation.sql b/server/skillhub-app/src/main/resources/db/migration/V33__skill_delete_storage_compensation.sql new file mode 100644 index 00000000..59c13a86 --- /dev/null +++ b/server/skillhub-app/src/main/resources/db/migration/V33__skill_delete_storage_compensation.sql @@ -0,0 +1,16 @@ +CREATE TABLE skill_storage_delete_compensation ( + id BIGSERIAL PRIMARY KEY, + skill_id BIGINT, + namespace VARCHAR(128) NOT NULL, + slug VARCHAR(128) NOT NULL, + storage_keys_json TEXT NOT NULL, + status VARCHAR(20) NOT NULL DEFAULT 'PENDING', + attempt_count INTEGER NOT NULL DEFAULT 0, + last_error TEXT, + last_attempt_at TIMESTAMPTZ, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +CREATE INDEX idx_skill_storage_delete_comp_status_created + ON skill_storage_delete_compensation (status, created_at); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java index b48c439f..5bc3b960 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/ReviewPortalControllerTest.java @@ -178,6 +178,7 @@ class ReviewPortalControllerTest { 30L, "skill-a", "Skill A", + "owner-1", "Owner", "Summary", "PUBLIC", diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillControllerTest.java index 90388da2..b6d8eea1 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillControllerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/SkillControllerTest.java @@ -149,6 +149,7 @@ class SkillControllerTest { 1L, "demo", "Demo", + "owner-1", "Alice", "Pending preview", "PUBLIC", diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillDeleteControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillDeleteControllerTest.java new file mode 100644 index 00000000..350b4d5f --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillDeleteControllerTest.java @@ -0,0 +1,94 @@ +package com.iflytek.skillhub.controller.portal; + +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.service.SkillDeleteAppService; +import java.util.List; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; + +import static org.mockito.BDDMockito.given; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class SkillDeleteControllerTest { + + @Autowired + private MockMvc mockMvc; + + @MockBean + private SkillDeleteAppService skillDeleteAppService; + + @MockBean + private NamespaceMemberRepository namespaceMemberRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @Test + void deleteSkill_allowsSuperAdminAndReturnsDeletedResponse() throws Exception { + given(skillDeleteAppService.deleteSkill( + org.mockito.ArgumentMatchers.eq("global"), + org.mockito.ArgumentMatchers.eq("demo-skill"), + org.mockito.ArgumentMatchers.eq("super-1"), + org.mockito.ArgumentMatchers.any())) + .willReturn(new SkillDeleteAppService.DeleteResult(11L, "global", "demo-skill", true)); + + PlatformPrincipal principal = new PlatformPrincipal( + "super-1", "Super", "super@example.com", "", "api_token", Set.of("SUPER_ADMIN")); + var auth = new UsernamePasswordAuthenticationToken( + principal, + null, + List.of( + new SimpleGrantedAuthority("ROLE_SUPER_ADMIN"), + new SimpleGrantedAuthority("SCOPE_skill:delete") + )); + + mockMvc.perform(delete("/api/v1/skills/global/demo-skill") + .with(authentication(auth)) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.deleted").value(true)) + .andExpect(jsonPath("$.data.skillId").value(11)) + .andExpect(jsonPath("$.data.namespace").value("global")) + .andExpect(jsonPath("$.data.slug").value("demo-skill")); + } + + @Test + void deleteSkill_rejectsNonSuperAdmin() throws Exception { + PlatformPrincipal principal = new PlatformPrincipal( + "skill-1", "Skill Admin", "skill@example.com", "", "api_token", Set.of("SKILL_ADMIN")); + var auth = new UsernamePasswordAuthenticationToken( + principal, + null, + List.of( + new SimpleGrantedAuthority("ROLE_SKILL_ADMIN"), + new SimpleGrantedAuthority("SCOPE_skill:delete") + )); + + mockMvc.perform(delete("/api/v1/skills/global/demo-skill") + .with(authentication(auth)) + .with(csrf())) + .andExpect(status().isForbidden()) + .andExpect(jsonPath("$.code").value(403)); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillLifecycleDeleteControllerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillLifecycleDeleteControllerTest.java new file mode 100644 index 00000000..742a69eb --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/controller/portal/SkillLifecycleDeleteControllerTest.java @@ -0,0 +1,81 @@ +package com.iflytek.skillhub.controller.portal; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.authentication; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.csrf; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +import com.iflytek.skillhub.TestRedisConfig; +import com.iflytek.skillhub.auth.device.DeviceAuthService; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.service.SkillDeleteAppService; +import java.util.List; +import java.util.Set; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.context.annotation.Import; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.web.servlet.MockMvc; + +@SpringBootTest +@AutoConfigureMockMvc +@ActiveProfiles("test") +@Import(TestRedisConfig.class) +class SkillLifecycleDeleteControllerTest { + + @Autowired + private MockMvc mockMvc; + + @MockBean + private SkillDeleteAppService skillDeleteAppService; + + @MockBean + private NamespaceMemberRepository namespaceMemberRepository; + + @MockBean + private DeviceAuthService deviceAuthService; + + @Test + void deleteSkill_allowsPortalOwnerAndReturnsUnifiedEnvelope() throws Exception { + given(skillDeleteAppService.deleteSkillFromPortal( + eq("global"), + eq("demo-skill"), + any(), + any())) + .willReturn(new SkillDeleteAppService.DeleteResult(1L, "global", "demo-skill", true)); + + mockMvc.perform(delete("/api/web/skills/global/demo-skill") + .with(authentication(portalAuth("usr_1", "USER"))) + .with(csrf())) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.code").value(0)) + .andExpect(jsonPath("$.data.skillId").value(1)) + .andExpect(jsonPath("$.data.deleted").value(true)) + .andExpect(jsonPath("$.data.slug").value("demo-skill")); + } + + private UsernamePasswordAuthenticationToken portalAuth(String userId, String... roles) { + PlatformPrincipal principal = new PlatformPrincipal( + userId, + userId, + userId + "@example.com", + "", + "session", + Set.of(roles) + ); + List authorities = java.util.Arrays.stream(roles) + .map(role -> new SimpleGrantedAuthority("ROLE_" + role)) + .toList(); + return new UsernamePasswordAuthenticationToken(principal, null, authorities); + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillDeleteAppServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillDeleteAppServiceTest.java new file mode 100644 index 00000000..177b5ab9 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillDeleteAppServiceTest.java @@ -0,0 +1,143 @@ +package com.iflytek.skillhub.service; + +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.service.SkillHardDeleteService; +import com.iflytek.skillhub.search.SearchIndexService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +@ExtendWith(MockitoExtension.class) +class SkillDeleteAppServiceTest { + + @Mock + private SkillRepository skillRepository; + @Mock + private SkillHardDeleteService skillHardDeleteService; + @Mock + private SearchIndexService searchIndexService; + + private SkillDeleteAppService service; + + @BeforeEach + void setUp() { + service = new SkillDeleteAppService(skillRepository, skillHardDeleteService, searchIndexService); + } + + @Test + void deleteSkill_deletesExistingSkillAndSearchDocument() { + Skill skill = new Skill(1L, "demo-skill", "owner-1", SkillVisibility.PUBLIC); + setField(skill, "id", 11L); + given(skillRepository.findByNamespaceSlugAndSlug("global", "demo-skill")).willReturn(Optional.of(skill)); + + SkillDeleteAppService.DeleteResult result = + service.deleteSkill("global", "demo-skill", "super-1", new AuditRequestContext("127.0.0.1", "JUnit")); + + assertThat(result.deleted()).isTrue(); + assertThat(result.skillId()).isEqualTo(11L); + verify(skillHardDeleteService).hardDeleteSkill(skill, "global", "super-1", "127.0.0.1", "JUnit"); + verify(searchIndexService).remove(11L); + } + + @Test + void deleteSkill_isIdempotentWhenSkillDoesNotExist() { + given(skillRepository.findByNamespaceSlugAndSlug("global", "missing-skill")).willReturn(Optional.empty()); + + SkillDeleteAppService.DeleteResult result = + service.deleteSkill("global", "missing-skill", "super-1", new AuditRequestContext("127.0.0.1", "JUnit")); + + assertThat(result.deleted()).isFalse(); + assertThat(result.skillId()).isNull(); + verify(skillHardDeleteService, never()).hardDeleteSkill(org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any()); + verify(searchIndexService, never()).remove(org.mockito.ArgumentMatchers.anyLong()); + } + + @Test + void deleteSkillFromPortal_allowsOwner() { + Skill skill = new Skill(1L, "demo-skill", "owner-1", SkillVisibility.PUBLIC); + setField(skill, "id", 11L); + given(skillRepository.findByNamespaceSlugAndSlug("global", "demo-skill")).willReturn(Optional.of(skill)); + + SkillDeleteAppService.DeleteResult result = service.deleteSkillFromPortal( + "global", + "demo-skill", + new PlatformPrincipal("owner-1", "Owner", "owner@example.com", "", "session", java.util.Set.of("USER")), + new AuditRequestContext("127.0.0.1", "JUnit") + ); + + assertThat(result.deleted()).isTrue(); + verify(skillHardDeleteService).hardDeleteSkill(skill, "global", "owner-1", "127.0.0.1", "JUnit"); + verify(searchIndexService).remove(11L); + } + + @Test + void deleteSkillFromPortal_allowsSuperAdmin() { + Skill skill = new Skill(1L, "demo-skill", "owner-1", SkillVisibility.PUBLIC); + setField(skill, "id", 11L); + given(skillRepository.findByNamespaceSlugAndSlug("global", "demo-skill")).willReturn(Optional.of(skill)); + + SkillDeleteAppService.DeleteResult result = service.deleteSkillFromPortal( + "global", + "demo-skill", + new PlatformPrincipal("super-1", "Super", "super@example.com", "", "session", java.util.Set.of("SUPER_ADMIN")), + new AuditRequestContext("127.0.0.1", "JUnit") + ); + + assertThat(result.deleted()).isTrue(); + verify(skillHardDeleteService).hardDeleteSkill(skill, "global", "super-1", "127.0.0.1", "JUnit"); + } + + @Test + void deleteSkillFromPortal_rejectsNonOwnerWithoutSuperAdmin() { + Skill skill = new Skill(1L, "demo-skill", "owner-1", SkillVisibility.PUBLIC); + setField(skill, "id", 11L); + given(skillRepository.findByNamespaceSlugAndSlug("global", "demo-skill")).willReturn(Optional.of(skill)); + + assertThatThrownBy(() -> service.deleteSkillFromPortal( + "global", + "demo-skill", + new PlatformPrincipal("user-2", "User", "user@example.com", "", "session", java.util.Set.of("USER")), + new AuditRequestContext("127.0.0.1", "JUnit") + )).isInstanceOf(DomainForbiddenException.class); + } + + @Test + void deleteSkillFromPortal_isIdempotentWhenSkillDoesNotExist() { + given(skillRepository.findByNamespaceSlugAndSlug("global", "missing-skill")).willReturn(Optional.empty()); + + SkillDeleteAppService.DeleteResult result = service.deleteSkillFromPortal( + "global", + "missing-skill", + new PlatformPrincipal("owner-1", "Owner", "owner@example.com", "", "session", java.util.Set.of("USER")), + new AuditRequestContext("127.0.0.1", "JUnit") + ); + + assertThat(result.deleted()).isFalse(); + assertThat(result.skillId()).isNull(); + verify(skillHardDeleteService, never()).hardDeleteSkill(org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any()); + } + + private void setField(Object target, String fieldName, Object value) { + try { + java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } catch (ReflectiveOperationException e) { + throw new AssertionError(e); + } + } +} diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/task/SkillStorageDeletionCompensationTaskTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/task/SkillStorageDeletionCompensationTaskTest.java new file mode 100644 index 00000000..e3d85233 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/task/SkillStorageDeletionCompensationTaskTest.java @@ -0,0 +1,34 @@ +package com.iflytek.skillhub.task; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.iflytek.skillhub.domain.skill.service.SkillStorageDeletionCompensationService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class SkillStorageDeletionCompensationTaskTest { + + @Mock + private SkillStorageDeletionCompensationService compensationService; + + private SkillStorageDeletionCompensationTask task; + + @BeforeEach + void setUp() { + task = new SkillStorageDeletionCompensationTask(compensationService); + } + + @Test + void retryPendingCleanup_delegatesToCompensationService() { + when(compensationService.retryPendingCleanup()).thenReturn(2); + + task.retryPendingCleanup(); + + verify(compensationService).retryPendingCleanup(); + } +} diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java index 297de089..ecdeaf91 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java @@ -63,6 +63,8 @@ public class RouteSecurityPolicyRegistry { RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/tags/*/download"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/tags/*/files"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/skills/*/*/tags/*/file"), + RouteAuthorizationPolicy.roles(HttpMethod.DELETE, "/api/v1/skills/*/*", "SUPER_ADMIN"), + RouteAuthorizationPolicy.authenticated(HttpMethod.DELETE, "/api/web/skills/*/*"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/namespaces"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/namespaces/*"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/web/namespaces"), @@ -94,6 +96,7 @@ public class RouteSecurityPolicyRegistry { ApiTokenPolicy.allow(null, "/swagger-ui/**"), ApiTokenPolicy.require(null, "/api/v1/tokens", "token:manage"), ApiTokenPolicy.require(null, "/api/v1/tokens/**", "token:manage"), + ApiTokenPolicy.require(HttpMethod.DELETE, "/api/v1/skills/*/*", "skill:delete"), ApiTokenPolicy.require(HttpMethod.POST, "/api/v1/skills", "skill:publish"), ApiTokenPolicy.require(HttpMethod.POST, "/api/v1/skills/*/publish", "skill:publish"), ApiTokenPolicy.require(HttpMethod.POST, "/api/web/skills/*/publish", "skill:publish"), @@ -131,15 +134,11 @@ public class RouteSecurityPolicyRegistry { if (path == null) { return false; } - return path.startsWith("/api/") - || path.equals("/api/v1/publish") - || path.startsWith("/api/v1/auth/device/"); + return path.startsWith("/api/"); } public boolean shouldProjectRequestContext(String path) { - return path != null && (path.startsWith("/api/v1/") - || path.startsWith("/api/web/") - || path.startsWith("/api/")); + return path != null && path.startsWith("/api/"); } private boolean isApiPath(String path) { diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java index e97961f0..3b785021 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import org.springframework.http.HttpMethod; import java.util.Set; import org.junit.jupiter.api.Test; @@ -21,6 +22,27 @@ class RouteSecurityPolicyRegistryTest { assertTrue(allowed.allowed()); } + @Test + void authorizeApiToken_requiresDeleteScopeForHardDeleteEndpoint() { + var denied = registry.authorizeApiToken("DELETE", "/api/v1/skills/global/demo-skill", Set.of("skill:publish")); + var allowed = registry.authorizeApiToken("DELETE", "/api/v1/skills/global/demo-skill", Set.of("skill:delete")); + + assertFalse(denied.allowed()); + assertEquals("skill:delete", denied.requiredScope()); + assertTrue(allowed.allowed()); + } + + @Test + void authorizationPolicies_shouldDeclareSuperAdminDeleteRuleForHardDeleteEndpoint() { + boolean matched = registry.authorizationPolicies().stream() + .anyMatch(policy -> policy.method() == HttpMethod.DELETE + && "/api/v1/skills/*/*".equals(policy.pattern()) + && policy.accessLevel() == RouteSecurityPolicyRegistry.AccessLevel.ROLE_PROTECTED + && Set.of(policy.roles()).contains("SUPER_ADMIN")); + + assertTrue(matched); + } + @Test void shouldIgnoreCsrf_forBearerAndApiPaths() { assertTrue(registry.shouldIgnoreCsrf("/api/v1/admin/users", null)); diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilterDeleteTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilterDeleteTest.java new file mode 100644 index 00000000..f7285129 --- /dev/null +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeFilterDeleteTest.java @@ -0,0 +1,69 @@ +package com.iflytek.skillhub.auth.token; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.auth.policy.RouteSecurityPolicyRegistry; +import com.iflytek.skillhub.auth.rbac.PlatformPrincipal; +import jakarta.servlet.FilterChain; +import jakarta.servlet.http.HttpServletResponse; +import java.util.List; +import java.util.Set; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; +import org.springframework.security.core.authority.SimpleGrantedAuthority; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.access.AccessDeniedHandler; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +class ApiTokenScopeFilterDeleteTest { + + private final ApiTokenScopeService scopeService = + new ApiTokenScopeService(new ObjectMapper(), new RouteSecurityPolicyRegistry()); + + @AfterEach + void clearSecurityContext() { + SecurityContextHolder.clearContext(); + } + + @Test + void shouldDenyHardDeleteWithoutDeleteScope() throws Exception { + AccessDeniedHandler handler = (request, response, accessDeniedException) -> + response.sendError(HttpServletResponse.SC_FORBIDDEN, accessDeniedException.getMessage()); + ApiTokenScopeFilter filter = new ApiTokenScopeFilter(scopeService, handler); + + PlatformPrincipal principal = new PlatformPrincipal( + "super-1", + "Super", + "super@example.com", + "", + "api_token", + Set.of("SUPER_ADMIN") + ); + var authentication = new UsernamePasswordAuthenticationToken( + principal, + null, + List.of( + new SimpleGrantedAuthority("ROLE_SUPER_ADMIN"), + new SimpleGrantedAuthority("SCOPE_skill:publish") + ) + ); + SecurityContextHolder.getContext().setAuthentication(authentication); + + MockHttpServletRequest request = new MockHttpServletRequest("DELETE", "/api/v1/skills/global/demo-skill"); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain chain = mock(FilterChain.class); + + filter.doFilter(request, response, chain); + + assertEquals(HttpServletResponse.SC_FORBIDDEN, response.getStatus()); + assertTrue(response.getErrorMessage().contains("Missing API token scope: skill:delete")); + verify(chain, never()).doFilter(request, response); + } +} diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeServiceTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeServiceTest.java index cc877bf3..58ccedc9 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeServiceTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/token/ApiTokenScopeServiceTest.java @@ -53,6 +53,26 @@ class ApiTokenScopeServiceTest { assertTrue(allowed.allowed()); } + @Test + void authorizeShouldRequireDeleteScopeForHardDeleteEndpoint() { + ApiTokenScopeService.AuthorizationDecision denied = scopeService.authorize( + "DELETE", + "/api/v1/skills/team-a/demo-skill", + Set.of("skill:publish") + ); + + assertFalse(denied.allowed()); + assertEquals("skill:delete", denied.requiredScope()); + + ApiTokenScopeService.AuthorizationDecision allowed = scopeService.authorize( + "DELETE", + "/api/v1/skills/team-a/demo-skill", + Set.of("skill:delete") + ); + + assertTrue(allowed.allowed()); + } + @Test void authorizeShouldRequireTokenManageScopeForTokenEndpoints() { ApiTokenScopeService.AuthorizationDecision decision = scopeService.authorize( diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/report/SkillReportRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/report/SkillReportRepository.java index 62349668..9e06d0d4 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/report/SkillReportRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/report/SkillReportRepository.java @@ -15,4 +15,5 @@ public interface SkillReportRepository { boolean existsBySkillIdAndReporterIdAndStatus(Long skillId, String reporterId, SkillReportStatus status); Page findByStatus(SkillReportStatus status, Pageable pageable); List findBySkillIdIn(Collection skillIds); + void deleteBySkillId(Long skillId); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionRequestRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionRequestRepository.java index a6dc87c9..64b8fd53 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionRequestRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/PromotionRequestRepository.java @@ -14,6 +14,7 @@ public interface PromotionRequestRepository { Optional findBySourceVersionIdAndStatus(Long sourceVersionId, ReviewTaskStatus status); Optional findBySourceSkillIdAndStatus(Long sourceSkillId, ReviewTaskStatus status); Page findByStatus(ReviewTaskStatus status, Pageable pageable); + void deleteBySourceSkillIdOrTargetSkillId(Long sourceSkillId, Long targetSkillId); int updateStatusWithVersion(Long id, ReviewTaskStatus status, String reviewedBy, String reviewComment, Long targetSkillId, Integer expectedVersion); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTaskRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTaskRepository.java index 5596f60a..c825d994 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTaskRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/review/ReviewTaskRepository.java @@ -2,6 +2,7 @@ package com.iflytek.skillhub.domain.review; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; +import java.util.Collection; import java.util.Optional; /** @@ -14,6 +15,7 @@ public interface ReviewTaskRepository { Page findByStatus(ReviewTaskStatus status, Pageable pageable); Page findByNamespaceIdAndStatus(Long namespaceId, ReviewTaskStatus status, Pageable pageable); Page findBySubmittedByAndStatus(String submittedBy, ReviewTaskStatus status, Pageable pageable); + void deleteBySkillVersionIdIn(Collection skillVersionIds); void delete(ReviewTask reviewTask); int updateStatusWithVersion(Long id, ReviewTaskStatus status, String reviewedBy, String reviewComment, Integer expectedVersion); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillStorageDeletionCompensation.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillStorageDeletionCompensation.java new file mode 100644 index 00000000..9098741d --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillStorageDeletionCompensation.java @@ -0,0 +1,131 @@ +package com.iflytek.skillhub.domain.skill; + +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.EnumType; +import jakarta.persistence.Enumerated; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.PrePersist; +import jakarta.persistence.Table; +import java.time.Clock; +import java.time.Instant; + +@Entity +@Table(name = "skill_storage_delete_compensation") +public class SkillStorageDeletionCompensation { + + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Long id; + + @Column(name = "skill_id") + private Long skillId; + + @Column(nullable = false, length = 128) + private String namespace; + + @Column(nullable = false, length = 128) + private String slug; + + @Column(name = "storage_keys_json", nullable = false, columnDefinition = "TEXT") + private String storageKeysJson; + + @Enumerated(EnumType.STRING) + @Column(nullable = false, length = 20) + private SkillStorageDeletionCompensationStatus status = SkillStorageDeletionCompensationStatus.PENDING; + + @Column(name = "attempt_count", nullable = false) + private int attemptCount; + + @Column(name = "last_error", columnDefinition = "TEXT") + private String lastError; + + @Column(name = "last_attempt_at") + private Instant lastAttemptAt; + + @Column(name = "created_at", nullable = false, updatable = false) + private Instant createdAt; + + @Column(name = "updated_at", nullable = false) + private Instant updatedAt; + + protected SkillStorageDeletionCompensation() { + } + + public SkillStorageDeletionCompensation(Long skillId, + String namespace, + String slug, + String storageKeysJson, + String lastError) { + this.skillId = skillId; + this.namespace = namespace; + this.slug = slug; + this.storageKeysJson = storageKeysJson; + this.lastError = lastError; + } + + @PrePersist + protected void onCreate() { + Instant now = Instant.now(Clock.systemUTC()); + createdAt = now; + updatedAt = now; + } + + public Long getId() { + return id; + } + + public Long getSkillId() { + return skillId; + } + + public String getNamespace() { + return namespace; + } + + public String getSlug() { + return slug; + } + + public String getStorageKeysJson() { + return storageKeysJson; + } + + public SkillStorageDeletionCompensationStatus getStatus() { + return status; + } + + public int getAttemptCount() { + return attemptCount; + } + + public String getLastError() { + return lastError; + } + + public Instant getLastAttemptAt() { + return lastAttemptAt; + } + + public Instant getCreatedAt() { + return createdAt; + } + + public Instant getUpdatedAt() { + return updatedAt; + } + + public void markAttempt(String error) { + attemptCount += 1; + lastAttemptAt = Instant.now(Clock.systemUTC()); + lastError = error; + updatedAt = lastAttemptAt; + } + + public void markCompleted() { + status = SkillStorageDeletionCompensationStatus.COMPLETED; + updatedAt = Instant.now(Clock.systemUTC()); + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillStorageDeletionCompensationRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillStorageDeletionCompensationRepository.java new file mode 100644 index 00000000..f0ac4510 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillStorageDeletionCompensationRepository.java @@ -0,0 +1,8 @@ +package com.iflytek.skillhub.domain.skill; + +import java.util.List; + +public interface SkillStorageDeletionCompensationRepository { + SkillStorageDeletionCompensation save(SkillStorageDeletionCompensation compensation); + List findTop100ByStatusOrderByCreatedAtAsc(SkillStorageDeletionCompensationStatus status); +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillStorageDeletionCompensationStatus.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillStorageDeletionCompensationStatus.java new file mode 100644 index 00000000..b6d930cc --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillStorageDeletionCompensationStatus.java @@ -0,0 +1,6 @@ +package com.iflytek.skillhub.domain.skill; + +public enum SkillStorageDeletionCompensationStatus { + PENDING, + COMPLETED +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillTagRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillTagRepository.java index e45b09e6..68ae001c 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillTagRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillTagRepository.java @@ -11,4 +11,5 @@ public interface SkillTagRepository { List findBySkillId(Long skillId); SkillTag save(SkillTag tag); void delete(SkillTag tag); + void deleteBySkillId(Long skillId); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java index c98e2364..33878b0e 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionRepository.java @@ -16,4 +16,5 @@ public interface SkillVersionRepository { List findBySkillIdAndStatus(Long skillId, SkillVersionStatus status); SkillVersion save(SkillVersion version); void delete(SkillVersion version); + void deleteBySkillId(Long skillId); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionStatsRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionStatsRepository.java index 905621f9..5def2c4e 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionStatsRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillVersionStatsRepository.java @@ -8,4 +8,5 @@ import java.util.Optional; public interface SkillVersionStatsRepository { Optional findBySkillVersionId(Long skillVersionId); void incrementDownloadCount(Long skillVersionId, Long skillId); + void deleteBySkillId(Long skillId); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteService.java new file mode 100644 index 00000000..f862c320 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteService.java @@ -0,0 +1,178 @@ +package com.iflytek.skillhub.domain.skill.service; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.report.SkillReportRepository; +import com.iflytek.skillhub.domain.review.PromotionRequestRepository; +import com.iflytek.skillhub.domain.review.ReviewTaskRepository; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillFile; +import com.iflytek.skillhub.domain.skill.SkillFileRepository; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillTagRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVersionStatsRepository; +import com.iflytek.skillhub.domain.social.SkillRatingRepository; +import com.iflytek.skillhub.domain.social.SkillStarRepository; +import com.iflytek.skillhub.storage.ObjectStorageService; +import java.util.LinkedHashMap; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; + +/** + * Permanently deletes a skill and all of its persisted artifacts so the slug + * may be uploaded again without residual conflicts. + */ +@Service +public class SkillHardDeleteService { + + private static final Logger log = LoggerFactory.getLogger(SkillHardDeleteService.class); + + private final SkillRepository skillRepository; + private final SkillVersionRepository skillVersionRepository; + private final SkillFileRepository skillFileRepository; + private final SkillTagRepository skillTagRepository; + private final ReviewTaskRepository reviewTaskRepository; + private final PromotionRequestRepository promotionRequestRepository; + private final SkillStarRepository skillStarRepository; + private final SkillRatingRepository skillRatingRepository; + private final SkillReportRepository skillReportRepository; + private final SkillVersionStatsRepository skillVersionStatsRepository; + private final ObjectStorageService objectStorageService; + private final SkillStorageDeletionCompensationService compensationService; + private final AuditLogService auditLogService; + private final ObjectMapper objectMapper; + + public SkillHardDeleteService(SkillRepository skillRepository, + SkillVersionRepository skillVersionRepository, + SkillFileRepository skillFileRepository, + SkillTagRepository skillTagRepository, + ReviewTaskRepository reviewTaskRepository, + PromotionRequestRepository promotionRequestRepository, + SkillStarRepository skillStarRepository, + SkillRatingRepository skillRatingRepository, + SkillReportRepository skillReportRepository, + SkillVersionStatsRepository skillVersionStatsRepository, + ObjectStorageService objectStorageService, + SkillStorageDeletionCompensationService compensationService, + AuditLogService auditLogService, + ObjectMapper objectMapper) { + this.skillRepository = skillRepository; + this.skillVersionRepository = skillVersionRepository; + this.skillFileRepository = skillFileRepository; + this.skillTagRepository = skillTagRepository; + this.reviewTaskRepository = reviewTaskRepository; + this.promotionRequestRepository = promotionRequestRepository; + this.skillStarRepository = skillStarRepository; + this.skillRatingRepository = skillRatingRepository; + this.skillReportRepository = skillReportRepository; + this.skillVersionStatsRepository = skillVersionStatsRepository; + this.objectStorageService = objectStorageService; + this.compensationService = compensationService; + this.auditLogService = auditLogService; + this.objectMapper = objectMapper; + } + + @Transactional + public void hardDeleteSkill(Skill skill, String namespaceSlug, String actorUserId, String clientIp, String userAgent) { + List versions = skillVersionRepository.findBySkillId(skill.getId()); + List versionIds = versions.stream().map(SkillVersion::getId).toList(); + + List storageKeys = new ArrayList<>(); + for (SkillVersion version : versions) { + List files = skillFileRepository.findByVersionId(version.getId()); + files.stream() + .map(SkillFile::getStorageKey) + .filter(key -> key != null && !key.isBlank()) + .forEach(storageKeys::add); + storageKeys.add(buildBundleStorageKey(skill.getId(), version.getId())); + } + deleteStorageAfterCommit(skill, namespaceSlug, storageKeys); + + skill.setLatestVersionId(null); + skill.setUpdatedBy(actorUserId); + skillRepository.save(skill); + + if (!versionIds.isEmpty()) { + reviewTaskRepository.deleteBySkillVersionIdIn(versionIds); + } + promotionRequestRepository.deleteBySourceSkillIdOrTargetSkillId(skill.getId(), skill.getId()); + skillTagRepository.deleteBySkillId(skill.getId()); + skillStarRepository.deleteBySkillId(skill.getId()); + skillRatingRepository.deleteBySkillId(skill.getId()); + skillReportRepository.deleteBySkillId(skill.getId()); + skillVersionStatsRepository.deleteBySkillId(skill.getId()); + + for (Long versionId : versionIds) { + skillFileRepository.deleteByVersionId(versionId); + } + skillVersionRepository.deleteBySkillId(skill.getId()); + skillRepository.delete(skill); + + auditLogService.record( + actorUserId, + "DELETE_SKILL_HARD", + "SKILL", + skill.getId(), + null, + clientIp, + userAgent, + toAuditPayload(skill) + ); + } + + private void deleteStorageAfterCommit(Skill skill, String namespaceSlug, List storageKeys) { + if (storageKeys.isEmpty()) { + return; + } + if (!TransactionSynchronizationManager.isSynchronizationActive()) { + deleteStorageWithCompensation(skill, namespaceSlug, storageKeys); + return; + } + TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() { + @Override + public void afterCommit() { + deleteStorageWithCompensation(skill, namespaceSlug, storageKeys); + } + }); + } + + private void deleteStorageWithCompensation(Skill skill, String namespaceSlug, List storageKeys) { + try { + objectStorageService.deleteObjects(storageKeys); + } catch (RuntimeException ex) { + compensationService.recordFailure( + skill.getId(), + namespaceSlug, + skill.getSlug(), + storageKeys, + ex.getMessage() + ); + log.error("Failed to delete storage objects after hard delete commit [keys={}]", storageKeys, ex); + } + } + + private String buildBundleStorageKey(Long skillId, Long versionId) { + return String.format("packages/%d/%d/bundle.zip", skillId, versionId); + } + + private String toAuditPayload(Skill skill) { + Map payload = new LinkedHashMap<>(); + payload.put("namespaceId", skill.getNamespaceId()); + payload.put("slug", skill.getSlug()); + try { + return objectMapper.writeValueAsString(payload); + } catch (JsonProcessingException e) { + throw new IllegalStateException("Failed to serialize hard-delete audit payload", e); + } + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java index 5c04f153..dcb679d1 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java @@ -82,6 +82,7 @@ public class SkillQueryService { Long id, String slug, String displayName, + String ownerId, String ownerDisplayName, String summary, String visibility, @@ -166,6 +167,7 @@ public class SkillQueryService { skill.getId(), skill.getSlug(), skill.getDisplayName(), + skill.getOwnerId(), ownerDisplayName, skill.getSummary(), skill.getVisibility().name(), diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillStorageDeletionCompensationService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillStorageDeletionCompensationService.java new file mode 100644 index 00000000..3c7b2df0 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillStorageDeletionCompensationService.java @@ -0,0 +1,76 @@ +package com.iflytek.skillhub.domain.skill.service; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensation; +import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensationRepository; +import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensationStatus; +import com.iflytek.skillhub.storage.ObjectStorageService; +import java.util.List; +import org.springframework.stereotype.Service; +import org.springframework.transaction.annotation.Transactional; + +@Service +public class SkillStorageDeletionCompensationService { + + private static final TypeReference> STORAGE_KEYS_TYPE = new TypeReference<>() {}; + + private final SkillStorageDeletionCompensationRepository repository; + private final ObjectStorageService objectStorageService; + private final ObjectMapper objectMapper; + + public SkillStorageDeletionCompensationService(SkillStorageDeletionCompensationRepository repository, + ObjectStorageService objectStorageService, + ObjectMapper objectMapper) { + this.repository = repository; + this.objectStorageService = objectStorageService; + this.objectMapper = objectMapper; + } + + @Transactional + public void recordFailure(Long skillId, + String namespace, + String slug, + List storageKeys, + String lastError) { + repository.save(new SkillStorageDeletionCompensation( + skillId, + namespace, + slug, + serialize(storageKeys), + lastError + )); + } + + @Transactional + public int retryPendingCleanup() { + List records = + repository.findTop100ByStatusOrderByCreatedAtAsc(SkillStorageDeletionCompensationStatus.PENDING); + for (SkillStorageDeletionCompensation record : records) { + try { + objectStorageService.deleteObjects(deserialize(record.getStorageKeysJson())); + record.markCompleted(); + } catch (RuntimeException ex) { + record.markAttempt(ex.getMessage()); + } + } + return records.size(); + } + + private String serialize(List storageKeys) { + try { + return objectMapper.writeValueAsString(storageKeys); + } catch (JsonProcessingException e) { + throw new IllegalStateException("Failed to serialize storage deletion compensation keys", e); + } + } + + private List deserialize(String storageKeysJson) { + try { + return objectMapper.readValue(storageKeysJson, STORAGE_KEYS_TYPE); + } catch (JsonProcessingException e) { + throw new IllegalStateException("Failed to deserialize storage deletion compensation keys", e); + } + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingRepository.java index b628e598..f0a8a245 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillRatingRepository.java @@ -10,4 +10,5 @@ public interface SkillRatingRepository { Optional findBySkillIdAndUserId(Long skillId, String userId); double averageScoreBySkillId(Long skillId); int countBySkillId(Long skillId); + void deleteBySkillId(Long skillId); } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillStarRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillStarRepository.java index 2eabe344..bbff050d 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillStarRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillStarRepository.java @@ -11,6 +11,7 @@ public interface SkillStarRepository { SkillStar save(SkillStar star); Optional findBySkillIdAndUserId(Long skillId, String userId); void delete(SkillStar star); + void deleteBySkillId(Long skillId); Page findByUserId(String userId, Pageable pageable); long countBySkillId(Long skillId); } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteServiceTest.java new file mode 100644 index 00000000..7c800aef --- /dev/null +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillHardDeleteServiceTest.java @@ -0,0 +1,214 @@ +package com.iflytek.skillhub.domain.skill.service; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.audit.AuditLogService; +import com.iflytek.skillhub.domain.report.SkillReportRepository; +import com.iflytek.skillhub.domain.review.PromotionRequestRepository; +import com.iflytek.skillhub.domain.review.ReviewTaskRepository; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillFile; +import com.iflytek.skillhub.domain.skill.SkillFileRepository; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillTagRepository; +import com.iflytek.skillhub.domain.skill.SkillVersion; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVersionStatsRepository; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.social.SkillRatingRepository; +import com.iflytek.skillhub.domain.social.SkillStarRepository; +import com.iflytek.skillhub.storage.ObjectStorageService; +import java.util.List; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.transaction.support.TransactionSynchronization; +import org.springframework.transaction.support.TransactionSynchronizationManager; + +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +@ExtendWith(MockitoExtension.class) +class SkillHardDeleteServiceTest { + + @Mock + private SkillRepository skillRepository; + @Mock + private SkillVersionRepository skillVersionRepository; + @Mock + private SkillFileRepository skillFileRepository; + @Mock + private SkillTagRepository skillTagRepository; + @Mock + private ReviewTaskRepository reviewTaskRepository; + @Mock + private PromotionRequestRepository promotionRequestRepository; + @Mock + private SkillStarRepository skillStarRepository; + @Mock + private SkillRatingRepository skillRatingRepository; + @Mock + private SkillReportRepository skillReportRepository; + @Mock + private SkillVersionStatsRepository skillVersionStatsRepository; + @Mock + private ObjectStorageService objectStorageService; + @Mock + private SkillStorageDeletionCompensationService compensationService; + @Mock + private AuditLogService auditLogService; + + private SkillHardDeleteService service; + + @AfterEach + void tearDown() { + if (TransactionSynchronizationManager.isSynchronizationActive()) { + TransactionSynchronizationManager.clearSynchronization(); + } + } + + @BeforeEach + void setUp() { + service = new SkillHardDeleteService( + skillRepository, + skillVersionRepository, + skillFileRepository, + skillTagRepository, + reviewTaskRepository, + promotionRequestRepository, + skillStarRepository, + skillRatingRepository, + skillReportRepository, + skillVersionStatsRepository, + objectStorageService, + compensationService, + auditLogService, + new ObjectMapper() + ); + } + + @Test + void hardDeleteSkill_removesStorageArtifactsAndRelatedRecords() { + Skill skill = new Skill(9L, "demo-skill", "owner-1", SkillVisibility.PUBLIC); + setField(skill, "id", 7L); + skill.setLatestVersionId(22L); + + SkillVersion pending = new SkillVersion(7L, "1.0.0-rc1", "owner-1"); + setField(pending, "id", 21L); + SkillVersion published = new SkillVersion(7L, "1.0.0", "owner-1"); + setField(published, "id", 22L); + + given(skillVersionRepository.findBySkillId(7L)).willReturn(List.of(pending, published)); + given(skillFileRepository.findByVersionId(21L)).willReturn(List.of( + new SkillFile(21L, "SKILL.md", 12L, "text/markdown", "sha1", "skills/7/21/SKILL.md") + )); + given(skillFileRepository.findByVersionId(22L)).willReturn(List.of( + new SkillFile(22L, "README.md", 20L, "text/markdown", "sha2", "skills/7/22/README.md") + )); + + service.hardDeleteSkill(skill, "global", "super-1", "127.0.0.1", "JUnit"); + + verify(skillRepository).save(skill); + verify(reviewTaskRepository).deleteBySkillVersionIdIn(List.of(21L, 22L)); + verify(promotionRequestRepository).deleteBySourceSkillIdOrTargetSkillId(7L, 7L); + verify(skillTagRepository).deleteBySkillId(7L); + verify(skillStarRepository).deleteBySkillId(7L); + verify(skillRatingRepository).deleteBySkillId(7L); + verify(skillReportRepository).deleteBySkillId(7L); + verify(skillVersionStatsRepository).deleteBySkillId(7L); + verify(objectStorageService).deleteObjects(argThat(keys -> + keys.contains("skills/7/21/SKILL.md") + && keys.contains("skills/7/22/README.md") + && keys.contains("packages/7/21/bundle.zip") + && keys.contains("packages/7/22/bundle.zip") + && keys.size() == 4)); + verify(skillFileRepository).deleteByVersionId(21L); + verify(skillFileRepository).deleteByVersionId(22L); + verify(skillVersionRepository).deleteBySkillId(7L); + verify(skillRepository).delete(skill); + verify(auditLogService).record( + "super-1", + "DELETE_SKILL_HARD", + "SKILL", + 7L, + null, + "127.0.0.1", + "JUnit", + "{\"namespaceId\":9,\"slug\":\"demo-skill\"}" + ); + } + + @Test + void hardDeleteSkill_deletesStorageAfterCommitWhenSynchronizationIsActive() { + Skill skill = new Skill(9L, "demo-skill", "owner-1", SkillVisibility.PUBLIC); + setField(skill, "id", 7L); + SkillVersion version = new SkillVersion(7L, "1.0.0", "owner-1"); + setField(version, "id", 22L); + + given(skillVersionRepository.findBySkillId(7L)).willReturn(List.of(version)); + given(skillFileRepository.findByVersionId(22L)).willReturn(List.of( + new SkillFile(22L, "README.md", 20L, "text/markdown", "sha2", "skills/7/22/README.md") + )); + + TransactionSynchronizationManager.initSynchronization(); + + service.hardDeleteSkill(skill, "global", "super-1", "127.0.0.1", "JUnit"); + + verify(objectStorageService, never()).deleteObjects(argThat(keys -> !keys.isEmpty())); + + for (TransactionSynchronization synchronization : TransactionSynchronizationManager.getSynchronizations()) { + synchronization.afterCommit(); + } + + verify(objectStorageService).deleteObjects(argThat(keys -> + keys.contains("skills/7/22/README.md") + && keys.contains("packages/7/22/bundle.zip") + && keys.size() == 2)); + } + + @Test + void hardDeleteSkill_recordsCompensationWhenStorageDeleteFails() { + Skill skill = new Skill(9L, "demo-skill", "owner-1", SkillVisibility.PUBLIC); + setField(skill, "id", 7L); + SkillVersion version = new SkillVersion(7L, "1.0.0", "owner-1"); + setField(version, "id", 22L); + + given(skillVersionRepository.findBySkillId(7L)).willReturn(List.of(version)); + given(skillFileRepository.findByVersionId(22L)).willReturn(List.of( + new SkillFile(22L, "README.md", 20L, "text/markdown", "sha2", "skills/7/22/README.md") + )); + doThrow(new RuntimeException("s3 down")).when(objectStorageService).deleteObjects(anyList()); + + TransactionSynchronizationManager.initSynchronization(); + + service.hardDeleteSkill(skill, "global", "super-1", "127.0.0.1", "JUnit"); + + for (TransactionSynchronization synchronization : TransactionSynchronizationManager.getSynchronizations()) { + synchronization.afterCommit(); + } + + verify(compensationService).recordFailure( + org.mockito.ArgumentMatchers.eq(7L), + org.mockito.ArgumentMatchers.eq("global"), + org.mockito.ArgumentMatchers.eq("demo-skill"), + argThat(keys -> keys.contains("skills/7/22/README.md") && keys.contains("packages/7/22/bundle.zip")), + org.mockito.ArgumentMatchers.contains("s3 down") + ); + } + + private void setField(Object target, String fieldName, Object value) { + try { + java.lang.reflect.Field field = target.getClass().getDeclaredField(fieldName); + field.setAccessible(true); + field.set(target, value); + } catch (ReflectiveOperationException e) { + throw new AssertionError(e); + } + } +} diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillStorageDeletionCompensationServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillStorageDeletionCompensationServiceTest.java new file mode 100644 index 00000000..62a3623d --- /dev/null +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillStorageDeletionCompensationServiceTest.java @@ -0,0 +1,94 @@ +package com.iflytek.skillhub.domain.skill.service; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.verify; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensation; +import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensationRepository; +import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensationStatus; +import com.iflytek.skillhub.storage.ObjectStorageService; +import java.util.List; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class SkillStorageDeletionCompensationServiceTest { + + @Mock + private SkillStorageDeletionCompensationRepository repository; + @Mock + private ObjectStorageService objectStorageService; + + private SkillStorageDeletionCompensationService service; + + @BeforeEach + void setUp() { + service = new SkillStorageDeletionCompensationService(repository, objectStorageService, new ObjectMapper()); + } + + @Test + void recordFailure_persistsPendingKeysAndError() { + service.recordFailure(11L, "global", "demo-skill", List.of("skills/11/101/SKILL.md"), "boom"); + + ArgumentCaptor captor = + ArgumentCaptor.forClass(SkillStorageDeletionCompensation.class); + verify(repository).save(captor.capture()); + SkillStorageDeletionCompensation saved = captor.getValue(); + assertThat(saved.getSkillId()).isEqualTo(11L); + assertThat(saved.getNamespace()).isEqualTo("global"); + assertThat(saved.getSlug()).isEqualTo("demo-skill"); + assertThat(saved.getStorageKeysJson()).contains("SKILL.md"); + assertThat(saved.getLastError()).isEqualTo("boom"); + assertThat(saved.getStatus()).isEqualTo(SkillStorageDeletionCompensationStatus.PENDING); + } + + @Test + void retryPendingCleanup_marksCompletedAfterSuccessfulDelete() { + SkillStorageDeletionCompensation record = new SkillStorageDeletionCompensation( + 11L, + "global", + "demo-skill", + "[\"skills/11/101/SKILL.md\"]", + "boom" + ); + given(repository.findTop100ByStatusOrderByCreatedAtAsc(SkillStorageDeletionCompensationStatus.PENDING)) + .willReturn(List.of(record)); + + int retried = service.retryPendingCleanup(); + + assertThat(retried).isEqualTo(1); + verify(objectStorageService).deleteObjects(List.of("skills/11/101/SKILL.md")); + assertThat(record.getStatus()).isEqualTo(SkillStorageDeletionCompensationStatus.COMPLETED); + } + + @Test + void retryPendingCleanup_keepsPendingAndTracksAttemptWhenDeleteFails() { + SkillStorageDeletionCompensation record = new SkillStorageDeletionCompensation( + 11L, + "global", + "demo-skill", + "[\"skills/11/101/SKILL.md\"]", + "boom" + ); + given(repository.findTop100ByStatusOrderByCreatedAtAsc(SkillStorageDeletionCompensationStatus.PENDING)) + .willReturn(List.of(record)); + doThrow(new RuntimeException("s3 down")).when(objectStorageService).deleteObjects(anyList()); + + int retried = service.retryPendingCleanup(); + + assertThat(retried).isEqualTo(1); + assertThat(record.getStatus()).isEqualTo(SkillStorageDeletionCompensationStatus.PENDING); + assertThat(record.getAttemptCount()).isEqualTo(1); + assertThat(record.getLastError()).contains("s3 down"); + verify(repository).findTop100ByStatusOrderByCreatedAtAsc(SkillStorageDeletionCompensationStatus.PENDING); + } +} diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/JpaSkillRatingRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/JpaSkillRatingRepository.java index 8996e226..6b613ce3 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/JpaSkillRatingRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/JpaSkillRatingRepository.java @@ -18,4 +18,6 @@ public interface JpaSkillRatingRepository extends JpaRepository, SkillStarRepository { Optional findBySkillIdAndUserId(Long skillId, String userId); + void deleteBySkillId(Long skillId); Page findByUserId(String userId, Pageable pageable); long countBySkillId(Long skillId); } diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/JpaSkillStorageDeletionCompensationRepositoryAdapter.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/JpaSkillStorageDeletionCompensationRepositoryAdapter.java new file mode 100644 index 00000000..65d1ed28 --- /dev/null +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/JpaSkillStorageDeletionCompensationRepositoryAdapter.java @@ -0,0 +1,32 @@ +package com.iflytek.skillhub.infra.jpa; + +import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensation; +import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensationRepository; +import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensationStatus; +import java.util.List; +import org.springframework.context.annotation.Primary; +import org.springframework.stereotype.Repository; + +@Repository +@Primary +public class JpaSkillStorageDeletionCompensationRepositoryAdapter + implements SkillStorageDeletionCompensationRepository { + + private final SkillStorageDeletionCompensationJpaRepository delegate; + + public JpaSkillStorageDeletionCompensationRepositoryAdapter( + SkillStorageDeletionCompensationJpaRepository delegate) { + this.delegate = delegate; + } + + @Override + public SkillStorageDeletionCompensation save(SkillStorageDeletionCompensation compensation) { + return delegate.save(compensation); + } + + @Override + public List findTop100ByStatusOrderByCreatedAtAsc( + SkillStorageDeletionCompensationStatus status) { + return delegate.findTop100ByStatusOrderByCreatedAtAsc(status); + } +} diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/PromotionRequestJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/PromotionRequestJpaRepository.java index d3ab5db9..b04a7c63 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/PromotionRequestJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/PromotionRequestJpaRepository.java @@ -25,6 +25,8 @@ public interface PromotionRequestJpaRepository extends JpaRepository findByStatus(ReviewTaskStatus status, Pageable pageable); + void deleteBySourceSkillIdOrTargetSkillId(Long sourceSkillId, Long targetSkillId); + @Modifying(clearAutomatically = true, flushAutomatically = true) @Query(""" UPDATE PromotionRequest p diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/ReviewTaskJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/ReviewTaskJpaRepository.java index 1fc4f7ea..1efd038e 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/ReviewTaskJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/ReviewTaskJpaRepository.java @@ -10,6 +10,7 @@ import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param; import org.springframework.stereotype.Repository; +import java.util.Collection; import java.util.Optional; /** @@ -27,6 +28,8 @@ public interface ReviewTaskJpaRepository extends JpaRepository Page findBySubmittedByAndStatus(String submittedBy, ReviewTaskStatus status, Pageable pageable); + void deleteBySkillVersionIdIn(Collection skillVersionIds); + @Modifying @Query(""" UPDATE ReviewTask t diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillReportJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillReportJpaRepository.java index a88a0f47..67e0fce8 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillReportJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillReportJpaRepository.java @@ -16,6 +16,7 @@ public interface SkillReportJpaRepository extends JpaRepository findByStatusOrderByCreatedAtDesc(SkillReportStatus status, Pageable pageable); List findBySkillIdIn(Collection skillIds); + void deleteBySkillId(Long skillId); @Override default Page findByStatus(SkillReportStatus status, Pageable pageable) { diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillStorageDeletionCompensationJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillStorageDeletionCompensationJpaRepository.java new file mode 100644 index 00000000..72411f44 --- /dev/null +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillStorageDeletionCompensationJpaRepository.java @@ -0,0 +1,13 @@ +package com.iflytek.skillhub.infra.jpa; + +import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensation; +import com.iflytek.skillhub.domain.skill.SkillStorageDeletionCompensationStatus; +import java.util.List; +import org.springframework.data.jpa.repository.JpaRepository; + +interface SkillStorageDeletionCompensationJpaRepository + extends JpaRepository { + + List findTop100ByStatusOrderByCreatedAtAsc( + SkillStorageDeletionCompensationStatus status); +} diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillTagJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillTagJpaRepository.java index a232ebe6..55e01245 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillTagJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillTagJpaRepository.java @@ -15,4 +15,5 @@ import java.util.Optional; public interface SkillTagJpaRepository extends JpaRepository, SkillTagRepository { Optional findBySkillIdAndTagName(Long skillId, String tagName); List findBySkillId(Long skillId); + void deleteBySkillId(Long skillId); } diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java index 8bddd442..eabb01e4 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionJpaRepository.java @@ -34,4 +34,5 @@ public interface SkillVersionJpaRepository extends JpaRepository findBySkillIdAndStatusOrderByCreatedAtDesc(Long skillId, SkillVersionStatus status); Page findBySkillIdAndStatus(Long skillId, SkillVersionStatus status, Pageable pageable); + void deleteBySkillId(Long skillId); } diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionStatsJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionStatsJpaRepository.java index 8dae9601..0f633241 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionStatsJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/SkillVersionStatsJpaRepository.java @@ -35,4 +35,10 @@ public interface SkillVersionStatsJpaRepository extends JpaRepository { + const cleanNamespace = namespace.startsWith('@') ? namespace.slice(1) : namespace + await fetchJson(`${WEB_API_PREFIX}/skills/${cleanNamespace}/${slug}`, { + method: 'DELETE', + headers: await ensureCsrfHeaders(), + }) + }, + async deleteVersion(namespace: string, slug: string, version: string): Promise { const cleanNamespace = namespace.startsWith('@') ? namespace.slice(1) : namespace await fetchJson(`${WEB_API_PREFIX}/skills/${cleanNamespace}/${slug}/versions/${encodeURIComponent(version)}`, { diff --git a/web/src/api/types.ts b/web/src/api/types.ts index 9e805d59..d376ed03 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -157,6 +157,7 @@ export interface SkillDetail { id: number slug: string displayName: string + ownerId?: string ownerDisplayName?: string summary?: string visibility: string diff --git a/web/src/features/skill/skill-delete-flow.test.ts b/web/src/features/skill/skill-delete-flow.test.ts new file mode 100644 index 00000000..3b9be00f --- /dev/null +++ b/web/src/features/skill/skill-delete-flow.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'vitest' +import { isDeleteSlugConfirmationValid, resolveDeletedSkillReturnTo } from './skill-delete-flow' + +describe('isDeleteSlugConfirmationValid', () => { + it('requires an exact slug match', () => { + expect(isDeleteSlugConfirmationValid('demo-skill', 'demo-skill')).toBe(true) + expect(isDeleteSlugConfirmationValid('demo-skill', 'Demo-Skill')).toBe(false) + expect(isDeleteSlugConfirmationValid('demo-skill', 'demo')).toBe(false) + }) +}) + +describe('resolveDeletedSkillReturnTo', () => { + it('prefers a safe in-app return path', () => { + expect(resolveDeletedSkillReturnTo('/dashboard/skills')).toBe('/dashboard/skills') + }) + + it('falls back to search when return path is unsafe or missing', () => { + expect(resolveDeletedSkillReturnTo('https://example.com')).toBe('/search') + expect(resolveDeletedSkillReturnTo(undefined)).toBe('/search') + }) +}) diff --git a/web/src/features/skill/skill-delete-flow.ts b/web/src/features/skill/skill-delete-flow.ts new file mode 100644 index 00000000..f31078b7 --- /dev/null +++ b/web/src/features/skill/skill-delete-flow.ts @@ -0,0 +1,9 @@ +import { normalizeSkillDetailReturnTo } from '@/shared/lib/skill-navigation' + +export function isDeleteSlugConfirmationValid(expectedSlug: string, typedSlug: string) { + return typedSlug === expectedSlug +} + +export function resolveDeletedSkillReturnTo(returnTo?: string) { + return normalizeSkillDetailReturnTo(returnTo) ?? '/search' +} diff --git a/web/src/i18n/locales/en.json b/web/src/i18n/locales/en.json index d0a2368c..b458ccab 100644 --- a/web/src/i18n/locales/en.json +++ b/web/src/i18n/locales/en.json @@ -682,6 +682,10 @@ "governance": "Governance", "processing": "Processing...", "archiveSkill": "Archive Skill", + "unarchiveSkill": "Restore Skill", + "deleteSkill": "Delete Skill", + "deleteSkillContinue": "Continue", + "deleteSkillFinal": "Delete Permanently", "withdrawReview": "Withdraw Review", "hideSkill": "Hide Skill", "unhideSkill": "Unhide Skill", @@ -689,12 +693,21 @@ "archiveConfirmDescription": "After archiving, regular users will no longer be able to view or download \"{{skill}}\". Continue?", "unarchiveConfirmTitle": "Restore skill", "unarchiveConfirmDescription": "\"{{skill}}\" will become visible again and can publish new versions after being restored.", + "deleteSkillConfirmTitle": "Permanently delete skill", + "deleteSkillConfirmDescription": "This removes every version, file, and download bundle for \"{{skill}}\" and cannot be undone.", + "deleteSkillInputTitle": "Type the skill slug to confirm", + "deleteSkillInputDescription": "Enter the skill slug \"{{slug}}\" to continue.", + "deleteSkillInputPlaceholder": "Enter skill slug", + "deleteSkillWarning": "This is a physical hard delete. Historical versions and bundles will be removed permanently.", "archiveSuccessTitle": "Skill archived", "archiveSuccessDescription": "\"{{skill}}\" has been archived.", "archiveErrorTitle": "Failed to archive skill", "unarchiveSuccessTitle": "Skill restored", "unarchiveSuccessDescription": "\"{{skill}}\" has been restored.", "unarchiveErrorTitle": "Failed to restore skill", + "deleteSkillSuccessTitle": "Skill deleted", + "deleteSkillSuccessDescription": "\"{{skill}}\" has been permanently deleted.", + "deleteSkillErrorTitle": "Failed to delete skill", "withdrawReviewConfirmTitle": "Withdraw review", "withdrawReviewConfirmDescription": "After withdrawal, version {{version}} will leave the review queue and return to draft so it can be submitted again later.", "withdrawReviewSuccessTitle": "Review withdrawn", diff --git a/web/src/i18n/locales/zh.json b/web/src/i18n/locales/zh.json index fcfc02ae..a8c645ea 100644 --- a/web/src/i18n/locales/zh.json +++ b/web/src/i18n/locales/zh.json @@ -682,6 +682,10 @@ "governance": "治理操作", "processing": "处理中...", "archiveSkill": "归档技能", + "unarchiveSkill": "恢复技能", + "deleteSkill": "删除技能", + "deleteSkillContinue": "继续删除", + "deleteSkillFinal": "永久删除", "withdrawReview": "撤销审核", "hideSkill": "隐藏技能", "unhideSkill": "恢复技能", @@ -689,12 +693,21 @@ "archiveConfirmDescription": "归档后普通用户将无法看到或下载“{{skill}}”，确定继续吗？", "unarchiveConfirmTitle": "确认恢复技能", "unarchiveConfirmDescription": "恢复后“{{skill}}”会重新对外可见，并允许继续发布新版本。", + "deleteSkillConfirmTitle": "确认永久删除技能", + "deleteSkillConfirmDescription": "删除后会彻底移除“{{skill}}”的所有版本、文件和下载包，且无法恢复。", + "deleteSkillInputTitle": "输入技能标识以确认删除", + "deleteSkillInputDescription": "请输入技能 slug“{{slug}}”后继续。", + "deleteSkillInputPlaceholder": "输入技能 slug", + "deleteSkillWarning": "这是物理彻底删除操作。删除后不会保留历史版本，也不会保留下载包。", "archiveSuccessTitle": "技能已归档", "archiveSuccessDescription": "“{{skill}}”已归档。", "archiveErrorTitle": "归档技能失败", "unarchiveSuccessTitle": "技能已恢复", "unarchiveSuccessDescription": "“{{skill}}”已恢复。", "unarchiveErrorTitle": "恢复技能失败", + "deleteSkillSuccessTitle": "技能已删除", + "deleteSkillSuccessDescription": "“{{skill}}”已被永久删除。", + "deleteSkillErrorTitle": "删除技能失败", "withdrawReviewConfirmTitle": "确认撤销审核", "withdrawReviewConfirmDescription": "撤销后，版本 {{version}} 将退出审核流程并回到草稿状态，之后可再次提交审核。", "withdrawReviewSuccessTitle": "已撤销审核", diff --git a/web/src/i18n/skill-detail-locale.test.ts b/web/src/i18n/skill-detail-locale.test.ts new file mode 100644 index 00000000..6434f4d5 --- /dev/null +++ b/web/src/i18n/skill-detail-locale.test.ts @@ -0,0 +1,10 @@ +import { describe, expect, it } from 'vitest' +import en from './locales/en.json' +import zh from './locales/zh.json' + +describe('skill detail lifecycle locales', () => { + it('defines the unarchive label in both locales', () => { + expect(zh.skillDetail.unarchiveSkill).toBe('恢复技能') + expect(en.skillDetail.unarchiveSkill).toBe('Restore Skill') + }) +}) diff --git a/web/src/pages/skill-detail.test.tsx b/web/src/pages/skill-detail.test.tsx new file mode 100644 index 00000000..d7ea5db5 --- /dev/null +++ b/web/src/pages/skill-detail.test.tsx @@ -0,0 +1,176 @@ +import { renderToStaticMarkup } from 'react-dom/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const navigateMock = vi.fn() + +vi.mock('@tanstack/react-router', () => ({ + useNavigate: () => navigateMock, + useParams: () => ({ namespace: 'global', slug: 'demo-skill' }), + useRouterState: () => ({ pathname: '/space/global/demo-skill', searchStr: '', hash: '' }), + useSearch: () => ({ returnTo: '/dashboard/skills' }), +})) + +vi.mock('react-i18next', async () => { + const actual = await vi.importActual('react-i18next') + return { + ...actual, + useTranslation: () => ({ + t: (key: string) => key, + i18n: { language: 'zh' }, + }), + } +}) + +vi.mock('@tanstack/react-query', () => ({ + useMutation: () => ({ mutate: vi.fn(), isPending: false }), + useQueryClient: () => ({ invalidateQueries: vi.fn() }), +})) + +vi.mock('@/features/auth/use-auth', () => ({ + useAuth: () => ({ + user: { userId: 'owner-1', platformRoles: ['USER'] }, + hasRole: (role: string) => role === 'USER', + }), +})) + +vi.mock('@/features/report/use-skill-reports', () => ({ + useSubmitSkillReport: () => ({ mutateAsync: vi.fn(), isPending: false }), +})) + +vi.mock('@/shared/lib/toast', () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})) + +vi.mock('@/api/client', () => ({ + adminApi: { + hideSkill: vi.fn(), + unhideSkill: vi.fn(), + yankVersion: vi.fn(), + }, + ApiError: class ApiError extends Error { + serverMessageKey?: string + }, + buildApiUrl: (value: string) => value, + WEB_API_PREFIX: '/api/web', +})) + +vi.mock('@/shared/lib/date-time', () => ({ + formatLocalDateTime: (value: string) => value, +})) + +vi.mock('@/shared/lib/skill-download-cache', () => ({ + incrementSkillDownloadCount: vi.fn(), +})) + +vi.mock('@/shared/lib/number-format', () => ({ + formatCompactCount: (value: number) => String(value), +})) + +vi.mock('@/features/skill/markdown-renderer', () => ({ + MarkdownRenderer: () =>
markdown
, +})) + +vi.mock('@/features/skill/file-tree', () => ({ + FileTree: () =>
files
, +})) + +vi.mock('@/features/skill/install-command', () => ({ + InstallCommand: () =>
install
, +})) + +vi.mock('@/features/social/rating-input', () => ({ + RatingInput: () =>
rating
, +})) + +vi.mock('@/features/social/star-button', () => ({ + StarButton: () =>
star
, +})) + +const useSkillDetailMock = vi.fn() + +vi.mock('@/shared/hooks/use-skill-queries', () => ({ + useSkillDetail: () => useSkillDetailMock(), + useSkillVersions: () => ({ + data: [ + { + id: 10, + version: '1.0.0', + status: 'PUBLISHED', + changelog: '', + fileCount: 1, + totalSize: 12, + publishedAt: '2026-03-20T00:00:00Z', + downloadAvailable: true, + }, + ], + }), + useSkillVersionDetail: () => ({ data: undefined }), + useSkillFiles: () => ({ data: [] }), + useSkillReadme: () => ({ data: '# Demo', error: null }), + useArchiveSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), + useDeleteSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), + useDeleteSkillVersion: () => ({ mutateAsync: vi.fn(), isPending: false }), + useRereleaseSkillVersion: () => ({ mutateAsync: vi.fn(), isPending: false }), + useSubmitPromotion: () => ({ mutateAsync: vi.fn(), isPending: false }), + useUnarchiveSkill: () => ({ mutateAsync: vi.fn(), isPending: false }), + useWithdrawSkillReview: () => ({ mutateAsync: vi.fn(), isPending: false }), +})) + +import { SkillDetailPage } from './skill-detail' + +function createSkill(overrides: Record = {}) { + return { + id: 1, + slug: 'demo-skill', + displayName: 'Demo Skill', + ownerId: 'owner-1', + ownerDisplayName: 'Owner One', + summary: 'summary', + visibility: 'PUBLIC', + status: 'ACTIVE', + downloadCount: 12, + starCount: 2, + ratingAvg: 4.5, + ratingCount: 2, + hidden: false, + namespace: 'global', + canManageLifecycle: true, + canSubmitPromotion: false, + canInteract: true, + canReport: true, + headlineVersion: { id: 10, version: '1.0.0', status: 'PUBLISHED' }, + publishedVersion: { id: 10, version: '1.0.0', status: 'PUBLISHED' }, + ownerPreviewVersion: undefined, + resolutionMode: 'PUBLISHED', + ...overrides, + } +} + +describe('SkillDetailPage', () => { + beforeEach(() => { + navigateMock.mockReset() + useSkillDetailMock.mockReturnValue({ + data: createSkill(), + isLoading: false, + error: null, + }) + }) + + it('shows hard delete action for the skill owner', () => { + const html = renderToStaticMarkup() + + expect(html).toContain('skillDetail.deleteSkill') + }) + + it('hides hard delete action when the viewer is not the owner or super admin', () => { + useSkillDetailMock.mockReturnValue({ + data: createSkill({ ownerId: 'someone-else' }), + isLoading: false, + error: null, + }) + + const html = renderToStaticMarkup() + + expect(html).not.toContain('skillDetail.deleteSkill') + }) +}) diff --git a/web/src/pages/skill-detail.tsx b/web/src/pages/skill-detail.tsx index 5866d4eb..3f5ffdfa 100644 --- a/web/src/pages/skill-detail.tsx +++ b/web/src/pages/skill-detail.tsx @@ -12,6 +12,7 @@ import { shouldCollapseOverview, } from '@/features/skill/overview-collapse' import { resolveSkillActionErrorTitle } from '@/features/skill/skill-action-error' +import { isDeleteSlugConfirmationValid, resolveDeletedSkillReturnTo } from '@/features/skill/skill-delete-flow' import { RatingInput } from '@/features/social/rating-input' import { StarButton } from '@/features/social/star-button' import { useAuth } from '@/features/auth/use-auth' @@ -40,6 +41,7 @@ import { useSkillFiles, useSkillReadme, useArchiveSkill, + useDeleteSkill, useDeleteSkillVersion, useRereleaseSkillVersion, useSubmitPromotion, @@ -97,6 +99,9 @@ export function SkillDetailPage() { const [archiveConfirmOpen, setArchiveConfirmOpen] = useState(false) const [unarchiveConfirmOpen, setUnarchiveConfirmOpen] = useState(false) const [promotionConfirmOpen, setPromotionConfirmOpen] = useState(false) + const [deleteSkillConfirmOpen, setDeleteSkillConfirmOpen] = useState(false) + const [deleteSkillInputOpen, setDeleteSkillInputOpen] = useState(false) + const [deleteSkillInput, setDeleteSkillInput] = useState('') const [deleteVersionTarget, setDeleteVersionTarget] = useState(null) const [withdrawVersionTarget, setWithdrawVersionTarget] = useState(null) const [rereleaseTarget, setRereleaseTarget] = useState(null) @@ -137,6 +142,7 @@ export function SkillDetailPage() { const hasPublishedPendingReview = Boolean(publishedVersion && hasPendingOwnerPreview) const canInteract = skill?.canInteract ?? true const canReport = skill?.canReport ?? true + const canHardDeleteSkill = Boolean(skill && user && (skill.ownerId === user.userId || hasRole('SUPER_ADMIN'))) const isVersionDownloadable = selectedVersionEntry?.status === 'PUBLISHED' && (selectedVersionEntry?.downloadAvailable ?? false) useEffect(() => { @@ -221,6 +227,7 @@ export function SkillDetailPage() { }) const archiveMutation = useArchiveSkill() const unarchiveMutation = useUnarchiveSkill() + const deleteSkillMutation = useDeleteSkill() const deleteVersionMutation = useDeleteSkillVersion() const withdrawReviewMutation = useWithdrawSkillReview() const rereleaseVersionMutation = useRereleaseSkillVersion() @@ -388,6 +395,30 @@ export function SkillDetailPage() { } } + const handleOpenDeleteSkillInput = async () => { + setDeleteSkillConfirmOpen(false) + setDeleteSkillInput('') + setDeleteSkillInputOpen(true) + } + + const handleDeleteSkill = async () => { + if (!skill || !isDeleteSlugConfirmationValid(skill.slug, deleteSkillInput)) { + return + } + try { + await deleteSkillMutation.mutateAsync({ namespace, slug }) + toast.success( + t('skillDetail.deleteSkillSuccessTitle'), + t('skillDetail.deleteSkillSuccessDescription', { skill: skill.displayName }), + ) + setDeleteSkillInputOpen(false) + navigate({ to: resolveDeletedSkillReturnTo(search.returnTo) }) + } catch (error) { + toast.error(t('skillDetail.deleteSkillErrorTitle'), error instanceof Error ? error.message : '') + throw error + } + } + const handleDeleteVersion = async () => { if (!deleteVersionTarget) { return @@ -921,6 +952,15 @@ export function SkillDetailPage() { {archiveMutation.isPending ? t('skillDetail.processing') : t('skillDetail.archiveSkill')} )} + {canHardDeleteSkill && ( + + )} )} @@ -1022,6 +1062,55 @@ export function SkillDetailPage() { onConfirm={handleUnarchive} /> + + + { + setDeleteSkillInputOpen(open) + if (!open) { + setDeleteSkillInput('') + } + }} + > + + + {t('skillDetail.deleteSkillInputTitle')} + {t('skillDetail.deleteSkillInputDescription', { slug: skill.slug })} + +
+
+ {t('skillDetail.deleteSkillWarning')} +
+ setDeleteSkillInput(event.target.value)} + placeholder={t('skillDetail.deleteSkillInputPlaceholder')} + /> +
+ + + + + + + { diff --git a/web/src/shared/hooks/use-skill-queries.ts b/web/src/shared/hooks/use-skill-queries.ts index c04b47b2..5e540d69 100644 --- a/web/src/shared/hooks/use-skill-queries.ts +++ b/web/src/shared/hooks/use-skill-queries.ts @@ -346,6 +346,21 @@ export function useDeleteSkillVersion() { }) } +export function useDeleteSkill() { + const queryClient = useQueryClient() + + return useMutation({ + mutationFn: ({ namespace, slug }: { namespace: string; slug: string }) => + skillLifecycleApi.deleteSkill(namespace, slug), + onSuccess: (_data, variables) => { + queryClient.invalidateQueries({ queryKey: ['skills', 'my'] }) + queryClient.invalidateQueries({ queryKey: ['skills', variables.namespace, variables.slug] }) + queryClient.invalidateQueries({ queryKey: ['skills', variables.namespace, variables.slug, 'versions'] }) + queryClient.invalidateQueries({ queryKey: ['skills'] }) + }, + }) +} + export function useWithdrawSkillReview() { const queryClient = useQueryClient()