From bacfd58aa038da02207e2d4b0c65c72daab2318c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8D=83=E4=B9=98=E5=A6=8D=20=28Xiaoyaner=29?= <258399167+xiaoyaner0201@users.noreply.github.com> Date: Thu, 20 Aug 2026 02:46:12 +0000 Subject: [PATCH 1/4] fix: emit confirm-publish events with permission-aware subscriber fanout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: 千乘妍 (Xiaoyaner) <258399167+xiaoyaner0201@users.noreply.github.com> --- .../listener/NotificationEventListener.java | 10 +- .../social/SkillSubscriptionServiceTest.java | 155 ++++++++- .../NotificationEventListenerTest.java | 170 ++++++++++ .../SubscriberNotificationSinkTest.java | 293 ++++++++++++++++++ .../domain/event/SkillVersionYankedEvent.java | 2 +- .../namespace/NamespaceMemberRepository.java | 1 + .../skill/service/SkillGovernanceService.java | 2 +- .../social/SkillSubscriptionService.java | 30 +- .../SubscriptionMetadataAccessPolicy.java | 52 ++++ .../SubscriptionRecipientEligibility.java | 53 ++++ .../service/SkillDownloadServiceTest.java | 19 ++ .../skill/service/SkillQueryServiceTest.java | 58 ++++ .../service/SkillReviewSubmitServiceTest.java | 27 +- .../jpa/NamespaceMemberJpaRepository.java | 1 + .../service/NotificationDispatcherTest.java | 40 +++ 15 files changed, 904 insertions(+), 9 deletions(-) create mode 100644 server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/SubscriberNotificationSinkTest.java create mode 100644 server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionMetadataAccessPolicy.java create mode 100644 server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionRecipientEligibility.java diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/NotificationEventListener.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/NotificationEventListener.java index c20498a2..6ba40f6e 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/NotificationEventListener.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/listener/NotificationEventListener.java @@ -8,6 +8,7 @@ import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; import com.iflytek.skillhub.domain.skill.SkillVersionRepository; import com.iflytek.skillhub.domain.social.SkillSubscriptionService; +import com.iflytek.skillhub.domain.social.SubscriptionRecipientEligibility; import com.iflytek.skillhub.notification.domain.NotificationCategory; import com.iflytek.skillhub.notification.service.NotificationDispatcher; import org.slf4j.Logger; @@ -33,6 +34,7 @@ public class NotificationEventListener { private final NotificationDispatcher dispatcher; private final SkillSubscriptionService skillSubscriptionService; private final ObjectMapper objectMapper; + private final SubscriptionRecipientEligibility subscriptionEligibility; public NotificationEventListener(SkillRepository skillRepository, SkillVersionRepository skillVersionRepository, @@ -40,7 +42,8 @@ public class NotificationEventListener { RecipientResolver recipientResolver, NotificationDispatcher dispatcher, SkillSubscriptionService skillSubscriptionService, - ObjectMapper objectMapper) { + ObjectMapper objectMapper, + SubscriptionRecipientEligibility subscriptionEligibility) { this.skillRepository = skillRepository; this.skillVersionRepository = skillVersionRepository; this.namespaceRepository = namespaceRepository; @@ -48,6 +51,7 @@ public class NotificationEventListener { this.dispatcher = dispatcher; this.skillSubscriptionService = skillSubscriptionService; this.objectMapper = objectMapper; + this.subscriptionEligibility = subscriptionEligibility; } @Async("skillhubEventExecutor") @@ -74,6 +78,8 @@ public class NotificationEventListener { if (subscribers.isEmpty()) { return; } + var namespace = namespaceRepository.findById(skill.getNamespaceId()).orElse(null); + subscribers = subscriptionEligibility.currentRecipients(skill, namespace, subscribers); String title = "Skill updated: " + skillDisplayName(skill); Map body = bodyWithSkill(skill); versionLabel(event.versionId(), body); @@ -96,6 +102,8 @@ public class NotificationEventListener { if (subscribers.isEmpty()) { return; } + var namespace = namespaceRepository.findById(skill.getNamespaceId()).orElse(null); + subscribers = subscriptionEligibility.yankedRecipients(skill, namespace, subscribers, event.wasPublished()); String title = "Skill version yanked: " + skillDisplayName(skill); Map body = bodyWithSkill(skill); versionLabel(event.versionId(), body); diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/domain/social/SkillSubscriptionServiceTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/domain/social/SkillSubscriptionServiceTest.java index 5e47524d..04334e47 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/domain/social/SkillSubscriptionServiceTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/domain/social/SkillSubscriptionServiceTest.java @@ -2,6 +2,17 @@ package com.iflytek.skillhub.domain.social; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.namespace.NamespaceStatus; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.domain.user.UserStatus; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.social.event.SkillSubscribedEvent; import com.iflytek.skillhub.domain.social.event.SkillUnsubscribedEvent; import org.junit.jupiter.api.BeforeEach; @@ -13,10 +24,13 @@ import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.context.ApplicationEventPublisher; import java.util.Optional; +import java.util.stream.Stream; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.*; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; @ExtendWith(MockitoExtension.class) class SkillSubscriptionServiceTest { @@ -24,17 +38,34 @@ class SkillSubscriptionServiceTest { @Mock private SkillSubscriptionRepository subscriptionRepository; @Mock private SkillRepository skillRepository; @Mock private ApplicationEventPublisher eventPublisher; + @Mock private NamespaceRepository namespaceRepository; + @Mock private NamespaceMemberRepository namespaceMemberRepository; + @Mock private UserAccountRepository userAccountRepository; private SkillSubscriptionService service; + private void allowPublicSubscription(Skill skill) { + skill.setLatestVersionId(10L); + when(userAccountRepository.findById("user-1")) + .thenReturn(Optional.of(new UserAccount("user-1", "User", null, null))); + Namespace namespace = new Namespace("demo", "Demo", "owner"); + when(namespaceRepository.findById(skill.getNamespaceId())).thenReturn(Optional.of(namespace)); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(skill.getNamespaceId(), "user-1")) + .thenReturn(Optional.empty()); + } + @BeforeEach void setUp() { - service = new SkillSubscriptionService(subscriptionRepository, skillRepository, eventPublisher); + service = new SkillSubscriptionService(subscriptionRepository, skillRepository, eventPublisher, + namespaceRepository, namespaceMemberRepository, userAccountRepository, + new SubscriptionMetadataAccessPolicy()); } @Test void subscribe_createsSubscriptionAndPublishesEvent() { - when(skillRepository.findById(1L)).thenReturn(Optional.of(mock(Skill.class))); + Skill skill = new Skill(5L, "public-skill", "owner", com.iflytek.skillhub.domain.skill.SkillVisibility.PUBLIC); + allowPublicSubscription(skill); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); when(subscriptionRepository.findBySkillIdAndUserId(1L, "user-1")).thenReturn(Optional.empty()); when(subscriptionRepository.save(any())).thenAnswer(inv -> inv.getArgument(0)); @@ -50,7 +81,9 @@ class SkillSubscriptionServiceTest { @Test void subscribe_idempotent_doesNotDuplicate() { - when(skillRepository.findById(1L)).thenReturn(Optional.of(mock(Skill.class))); + Skill skill = new Skill(5L, "public-skill", "owner", com.iflytek.skillhub.domain.skill.SkillVisibility.PUBLIC); + allowPublicSubscription(skill); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); when(subscriptionRepository.findBySkillIdAndUserId(1L, "user-1")) .thenReturn(Optional.of(mock(SkillSubscription.class))); @@ -102,4 +135,120 @@ class SkillSubscriptionServiceTest { assertThat(service.isSubscribed(1L, "user-1")).isFalse(); } + + @Test + void subscribe_rejectsInactiveAccountWithoutMutation() { + Skill skill = new Skill(5L, "private-skill", "owner", com.iflytek.skillhub.domain.skill.SkillVisibility.PRIVATE); + skill.setLatestVersionId(10L); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + UserAccount account = new UserAccount("user-1", "User", null, null); + account.setStatus(UserStatus.DISABLED); + when(userAccountRepository.findById("user-1")).thenReturn(Optional.of(account)); + + org.assertj.core.api.Assertions.assertThatThrownBy(() -> service.subscribe(1L, "user-1")) + .isInstanceOf(DomainForbiddenException.class); + + verifyNoInteractions(subscriptionRepository); + verify(skillRepository, never()).incrementSubscriptionCount(anyLong()); + verifyNoInteractions(eventPublisher); + } + + @Test + void subscribe_rejectsRemovedMemberOfArchivedNamespaceWithoutMutation() { + Skill skill = new Skill(5L, "public-skill", "owner", com.iflytek.skillhub.domain.skill.SkillVisibility.PUBLIC); + skill.setLatestVersionId(10L); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + when(userAccountRepository.findById("user-1")) + .thenReturn(Optional.of(new UserAccount("user-1", "User", null, null))); + Namespace namespace = new Namespace("archived", "Archived", "owner"); + namespace.setStatus(NamespaceStatus.ARCHIVED); + when(namespaceRepository.findById(5L)).thenReturn(Optional.of(namespace)); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(5L, "user-1")).thenReturn(Optional.empty()); + + org.assertj.core.api.Assertions.assertThatThrownBy(() -> service.subscribe(1L, "user-1")) + .isInstanceOf(DomainForbiddenException.class); + + verifyNoInteractions(subscriptionRepository); + verify(skillRepository, never()).incrementSubscriptionCount(anyLong()); + verifyNoInteractions(eventPublisher); + } + + static Stream deniedSubscriptions() { + return Stream.of( + new DeniedSubscription("private member", SkillVisibility.PRIVATE, false, NamespaceStatus.ACTIVE, + NamespaceRole.MEMBER), + new DeniedSubscription("private nonmember", SkillVisibility.PRIVATE, false, NamespaceStatus.ACTIVE, + null), + new DeniedSubscription("private cross namespace member", SkillVisibility.PRIVATE, false, + NamespaceStatus.ACTIVE, null), + new DeniedSubscription("hidden public", SkillVisibility.PUBLIC, true, NamespaceStatus.ACTIVE, + NamespaceRole.MEMBER) + ); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("deniedSubscriptions") + void subscribe_rejectsUnauthorizedMetadataWithoutAnyMutation(DeniedSubscription scenario) { + Skill skill = new Skill(5L, "restricted", "owner", scenario.visibility()); + skill.setLatestVersionId(10L); + skill.setHidden(scenario.hidden()); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + when(userAccountRepository.findById("user-1")) + .thenReturn(Optional.of(new UserAccount("user-1", "User", null, null))); + Namespace namespace = new Namespace("team", "Team", "owner"); + namespace.setStatus(scenario.namespaceStatus()); + when(namespaceRepository.findById(5L)).thenReturn(Optional.of(namespace)); + if (scenario.role() == null) { + when(namespaceMemberRepository.findByNamespaceIdAndUserId(5L, "user-1")) + .thenReturn(Optional.empty()); + } else { + when(namespaceMemberRepository.findByNamespaceIdAndUserId(5L, "user-1")) + .thenReturn(Optional.of(new NamespaceMember(5L, "user-1", scenario.role()))); + } + + org.assertj.core.api.Assertions.assertThatThrownBy(() -> service.subscribe(1L, "user-1")) + .isInstanceOf(DomainForbiddenException.class); + + verifyNoInteractions(subscriptionRepository); + verify(skillRepository, never()).incrementSubscriptionCount(anyLong()); + verifyNoInteractions(eventPublisher); + } + + @Test + void subscribe_allowsPublicArchivedSkillBecauseMetadataPurposeDoesNotRequireActiveSkill() { + Skill skill = new Skill(5L, "archived-skill", "owner", SkillVisibility.PUBLIC); + skill.setStatus(com.iflytek.skillhub.domain.skill.SkillStatus.ARCHIVED); + skill.setLatestVersionId(10L); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + when(userAccountRepository.findById("user-1")) + .thenReturn(Optional.of(new UserAccount("user-1", "User", null, null))); + when(namespaceRepository.findById(5L)).thenReturn(Optional.of(new Namespace("team", "Team", "owner"))); + when(namespaceMemberRepository.findByNamespaceIdAndUserId(5L, "user-1")).thenReturn(Optional.empty()); + when(subscriptionRepository.findBySkillIdAndUserId(1L, "user-1")).thenReturn(Optional.empty()); + + service.subscribe(1L, "user-1"); + + verify(subscriptionRepository).save(any(SkillSubscription.class)); + verify(skillRepository).incrementSubscriptionCount(1L); + verify(eventPublisher).publishEvent(any(SkillSubscribedEvent.class)); + } + + @Test + void getAndDeleteDoNotConsultMetadataAuthorizationDependencies() { + when(skillRepository.findById(1L)).thenReturn(Optional.of(mock(Skill.class))); + SkillSubscription existing = mock(SkillSubscription.class); + when(subscriptionRepository.findBySkillIdAndUserId(1L, "user-1")) + .thenReturn(Optional.of(existing)); + + assertThat(service.isSubscribed(1L, "user-1")).isTrue(); + service.unsubscribe(1L, "user-1"); + + verifyNoInteractions(namespaceRepository, namespaceMemberRepository, userAccountRepository); + verify(subscriptionRepository).delete(existing); + } + + record DeniedSubscription(String label, SkillVisibility visibility, boolean hidden, + NamespaceStatus namespaceStatus, NamespaceRole role) { + @Override public String toString() { return label; } + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/NotificationEventListenerTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/NotificationEventListenerTest.java index 6321f004..80304261 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/NotificationEventListenerTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/NotificationEventListenerTest.java @@ -4,10 +4,20 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.iflytek.skillhub.domain.event.*; import com.iflytek.skillhub.domain.namespace.Namespace; import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.namespace.NamespaceStatus; 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.SkillVersionRepository; +import com.iflytek.skillhub.domain.social.SkillSubscriptionService; +import com.iflytek.skillhub.domain.social.SubscriptionRecipientEligibility; +import com.iflytek.skillhub.domain.social.SubscriptionMetadataAccessPolicy; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.domain.user.UserStatus; import com.iflytek.skillhub.notification.domain.NotificationCategory; import com.iflytek.skillhub.notification.service.NotificationDispatcher; import org.junit.jupiter.api.Test; @@ -31,10 +41,21 @@ class NotificationEventListenerTest { @Mock RecipientResolver recipientResolver; @Mock NotificationDispatcher dispatcher; @Mock ObjectMapper objectMapper; + @Mock SkillSubscriptionService skillSubscriptionService; + @Mock UserAccountRepository userAccountRepository; + @Mock NamespaceMemberRepository namespaceMemberRepository; @InjectMocks NotificationEventListener listener; + @org.junit.jupiter.api.BeforeEach + void setUpListener() { + listener = new NotificationEventListener(skillRepository, skillVersionRepository, namespaceRepository, + recipientResolver, dispatcher, skillSubscriptionService, objectMapper, + new SubscriptionRecipientEligibility(userAccountRepository, namespaceMemberRepository, + new SubscriptionMetadataAccessPolicy())); + } + private Skill mockSkill(Long id) { Skill skill = mock(Skill.class); when(skill.getId()).thenReturn(id); @@ -225,4 +246,153 @@ class NotificationEventListenerTest { verify(dispatcher).dispatch(eq("reporter-1"), eq(NotificationCategory.REPORT), eq("REPORT_RESOLVED"), anyString(), anyString(), eq("SKILL"), eq(1L)); } + + @Test + void publishSubscriberFanoutExcludesInactiveAccount() { + Skill skill = skill(1L, "owner", "owner"); + skill.setLatestVersionId(10L); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + when(skillSubscriptionService.findSubscribersBySkillId(1L)).thenReturn(List.of("inactive")); + UserAccount inactive = new UserAccount("inactive", "Inactive", null, null); + inactive.setStatus(UserStatus.DISABLED); + when(userAccountRepository.findByIdIn(List.of("inactive"))).thenReturn(List.of(inactive)); + mockNamespace(); + + listener.onSkillPublishedForSubscribers(new SkillPublishedEvent(1L, 10L, "owner")); + + verifyNoInteractions(dispatcher); + } + + @Test + void publishSubscriberFanoutFailsClosedBeforeDispatchWhenAccountBatchFails() { + Skill skill = skill(1L, "owner", "owner"); + skill.setLatestVersionId(10L); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + when(skillSubscriptionService.findSubscribersBySkillId(1L)).thenReturn(List.of("user-1", "user-2")); + when(userAccountRepository.findByIdIn(anyList())).thenThrow(new IllegalStateException("account batch unavailable")); + + org.assertj.core.api.Assertions.assertThatThrownBy(() -> + listener.onSkillPublishedForSubscribers(new SkillPublishedEvent(1L, 10L, "owner"))) + .isInstanceOf(IllegalStateException.class); + + verifyNoInteractions(dispatcher); + } + + @Test + void publishSubscriberFanoutDispatchesOnlyEligibleNonPublisherWithExactPayload() throws Exception { + Skill skill = skill(1L, "publisher", "publisher"); + skill.setLatestVersionId(10L); + skill.setVisibility(SkillVisibility.PRIVATE); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + when(skillSubscriptionService.findSubscribersBySkillId(1L)) + .thenReturn(List.of("publisher", "admin", "member", "inactive", "missing")); + UserAccount publisher = new UserAccount("publisher", "Publisher", null, null); + UserAccount admin = new UserAccount("admin", "Admin", null, null); + UserAccount member = new UserAccount("member", "Member", null, null); + UserAccount inactive = new UserAccount("inactive", "Inactive", null, null); + inactive.setStatus(UserStatus.DISABLED); + when(userAccountRepository.findByIdIn(anyList())).thenReturn(List.of(publisher, admin, member, inactive)); + when(namespaceMemberRepository.findByNamespaceIdAndUserIdIn(eq(5L), anyCollection())) + .thenReturn(List.of(new NamespaceMember(5L, "admin", NamespaceRole.ADMIN), + new NamespaceMember(5L, "member", NamespaceRole.MEMBER))); + mockNamespace(); + when(objectMapper.writeValueAsString(any())).thenReturn("{\"skillId\":1,\"version\":\"1.0.0\"}"); + + listener.onSkillPublishedForSubscribers(new SkillPublishedEvent(1L, 10L, "publisher")); + + verify(dispatcher).dispatch("admin", NotificationCategory.PUBLISH, "SUBSCRIPTION_NEW_VERSION", + "Skill updated: Test Skill", "{\"skillId\":1,\"version\":\"1.0.0\"}", "SKILL", 1L); + verifyNoMoreInteractions(dispatcher); + } + + @Test + void yankWithoutFallbackUsesVerifiedPreYankPublicationAndExcludesActor() throws Exception { + Skill skill = skill(1L, "owner", "owner"); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + when(skillSubscriptionService.findSubscribersBySkillId(1L)).thenReturn(List.of("actor", "subscriber")); + when(userAccountRepository.findByIdIn(anyList())).thenReturn(List.of( + new UserAccount("actor", "Actor", null, null), + new UserAccount("subscriber", "Subscriber", null, null))); + mockNamespace(); + when(objectMapper.writeValueAsString(any())).thenReturn("{\"skillId\":1,\"versionId\":10}"); + + listener.onSkillVersionYankedForSubscribers(new SkillVersionYankedEvent(1L, 10L, "actor", true)); + + verify(dispatcher).dispatch("subscriber", NotificationCategory.PUBLISH, "SUBSCRIPTION_VERSION_YANKED", + "Skill version yanked: Test Skill", "{\"skillId\":1,\"versionId\":10}", "SKILL", 1L); + verifyNoMoreInteractions(dispatcher); + } + + @Test + void yankDoesNotDispatchWhenEventDoesNotVerifyPublishedPreState() { + Skill skill = skill(1L, "owner", "owner"); + skill.setLatestVersionId(9L); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + when(skillSubscriptionService.findSubscribersBySkillId(1L)).thenReturn(List.of("subscriber")); + when(userAccountRepository.findByIdIn(anyList())).thenReturn(List.of( + new UserAccount("subscriber", "Subscriber", null, null))); + mockNamespace(); + + listener.onSkillVersionYankedForSubscribers(new SkillVersionYankedEvent(1L, 10L, "actor", false)); + + verifyNoInteractions(dispatcher); + } + + @Test + void publishFanoutFailsClosedBeforeDispatchWhenNamespaceReadFails() { + Skill skill = skill(1L, "owner", "owner"); + skill.setLatestVersionId(10L); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + when(skillSubscriptionService.findSubscribersBySkillId(1L)).thenReturn(List.of("user-1")); + when(namespaceRepository.findById(5L)).thenThrow(new IllegalStateException("namespace unavailable")); + + org.assertj.core.api.Assertions.assertThatThrownBy(() -> + listener.onSkillPublishedForSubscribers(new SkillPublishedEvent(1L, 10L, "owner"))) + .isInstanceOf(IllegalStateException.class); + + verifyNoInteractions(dispatcher); + } + + @Test + void yankFanoutFailsClosedBeforeDispatchWhenMembershipBatchFails() { + Skill skill = skill(1L, "owner", "owner"); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + when(skillSubscriptionService.findSubscribersBySkillId(1L)).thenReturn(List.of("user-1", "user-2")); + when(userAccountRepository.findByIdIn(anyList())).thenReturn(List.of( + new UserAccount("user-1", "One", null, null), + new UserAccount("user-2", "Two", null, null))); + when(namespaceRepository.findById(5L)) + .thenReturn(Optional.of(new Namespace("demo", "Demo", "owner"))); + when(namespaceMemberRepository.findByNamespaceIdAndUserIdIn(eq(5L), anyCollection())) + .thenThrow(new IllegalStateException("membership unavailable")); + + org.assertj.core.api.Assertions.assertThatThrownBy(() -> + listener.onSkillVersionYankedForSubscribers(new SkillVersionYankedEvent(1L, 10L, "actor", true))) + .isInstanceOf(IllegalStateException.class); + + verifyNoInteractions(dispatcher); + } + + @Test + void archivedNamespaceRemovedSubscriberIsRejectedButCurrentMemberReceivesYank() throws Exception { + Skill skill = skill(1L, "owner", "owner"); + skill.setStatus(com.iflytek.skillhub.domain.skill.SkillStatus.ARCHIVED); + when(skillRepository.findById(1L)).thenReturn(Optional.of(skill)); + when(skillSubscriptionService.findSubscribersBySkillId(1L)).thenReturn(List.of("current", "removed")); + when(userAccountRepository.findByIdIn(anyList())).thenReturn(List.of( + new UserAccount("current", "Current", null, null), + new UserAccount("removed", "Removed", null, null))); + Namespace namespace = new Namespace("archived", "Archived", "owner"); + namespace.setStatus(NamespaceStatus.ARCHIVED); + when(namespaceRepository.findById(5L)).thenReturn(Optional.of(namespace)); + when(namespaceMemberRepository.findByNamespaceIdAndUserIdIn(eq(5L), anyCollection())) + .thenReturn(List.of(new NamespaceMember(5L, "current", NamespaceRole.MEMBER))); + when(objectMapper.writeValueAsString(any())).thenReturn("{}"); + + listener.onSkillVersionYankedForSubscribers(new SkillVersionYankedEvent(1L, 10L, "actor", true)); + + verify(dispatcher).dispatch(eq("current"), eq(NotificationCategory.PUBLISH), + eq("SUBSCRIPTION_VERSION_YANKED"), anyString(), eq("{}"), eq("SKILL"), eq(1L)); + verifyNoMoreInteractions(dispatcher); + } } diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/SubscriberNotificationSinkTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/SubscriberNotificationSinkTest.java new file mode 100644 index 00000000..93385750 --- /dev/null +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/listener/SubscriberNotificationSinkTest.java @@ -0,0 +1,293 @@ +package com.iflytek.skillhub.listener; + +import com.fasterxml.jackson.databind.ObjectMapper; +import com.iflytek.skillhub.domain.event.SkillPublishedEvent; +import com.iflytek.skillhub.domain.event.SkillVersionYankedEvent; +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceMember; +import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRepository; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.namespace.NamespaceStatus; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.skill.SkillVersionRepository; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.social.SkillSubscriptionService; +import com.iflytek.skillhub.domain.social.SubscriptionMetadataAccessPolicy; +import com.iflytek.skillhub.domain.social.SubscriptionRecipientEligibility; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.domain.user.UserStatus; +import com.iflytek.skillhub.notification.domain.Notification; +import com.iflytek.skillhub.notification.domain.NotificationCategory; +import com.iflytek.skillhub.notification.domain.NotificationChannel; +import com.iflytek.skillhub.notification.service.NotificationDispatcher; +import com.iflytek.skillhub.notification.service.NotificationPreferenceService; +import com.iflytek.skillhub.notification.service.NotificationService; +import com.iflytek.skillhub.notification.sse.SseEmitterManager; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +import org.junit.jupiter.params.provider.ValueSource; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.time.Instant; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyCollection; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class SubscriberNotificationSinkTest { + + private static final Long SKILL_ID = 1L; + private static final Long NAMESPACE_ID = 5L; + private static final Long VERSION_ID = 10L; + private static final Instant CREATED_AT = Instant.parse("2026-08-19T20:30:00Z"); + + @Mock SkillRepository skillRepository; + @Mock SkillVersionRepository skillVersionRepository; + @Mock NamespaceRepository namespaceRepository; + @Mock RecipientResolver recipientResolver; + @Mock SkillSubscriptionService subscriptionService; + @Mock UserAccountRepository accountRepository; + @Mock NamespaceMemberRepository memberRepository; + @Mock NotificationService notificationService; + @Mock NotificationPreferenceService preferenceService; + @Mock SseEmitterManager sseEmitterManager; + + private NotificationEventListener listener; + + @BeforeEach + void setUp() { + SubscriptionRecipientEligibility eligibility = new SubscriptionRecipientEligibility( + accountRepository, memberRepository, new SubscriptionMetadataAccessPolicy()); + NotificationDispatcher dispatcher = new NotificationDispatcher( + notificationService, preferenceService, sseEmitterManager); + listener = new NotificationEventListener(skillRepository, skillVersionRepository, namespaceRepository, + recipientResolver, dispatcher, subscriptionService, new ObjectMapper(), eligibility); + } + + @Test + void publishPersistsAndPushesOnlyCurrentEligibleNonPublisherAcrossAuthorizationMatrix() { + Skill skill = skill(SkillVisibility.PRIVATE, false, VERSION_ID); + Namespace namespace = namespace(NamespaceStatus.ACTIVE); + List candidates = List.of("publisher", "current-admin", "stale-removed", "inactive", + "missing", "private-member", "cross-namespace", "platform-super-admin"); + arrangeEvent(skill, namespace, candidates); + when(accountRepository.findByIdIn(candidates)).thenReturn(List.of( + account("publisher"), account("current-admin"), account("stale-removed"), + inactiveAccount("inactive"), account("private-member"), account("cross-namespace"), + account("platform-super-admin"))); + when(memberRepository.findByNamespaceIdAndUserIdIn(NAMESPACE_ID, candidates)).thenReturn(List.of( + member("current-admin", NamespaceRole.ADMIN), + member("private-member", NamespaceRole.MEMBER))); + enablePersistenceFor("current-admin", "SUBSCRIPTION_NEW_VERSION"); + + listener.onSkillPublishedForSubscribers(new SkillPublishedEvent(SKILL_ID, VERSION_ID, "publisher")); + + String body = "{\"skillId\":1,\"skillName\":\"Test Skill\",\"slug\":\"test-skill\",\"namespace\":\"demo\"}"; + verify(notificationService).create("current-admin", NotificationCategory.PUBLISH, + "SUBSCRIPTION_NEW_VERSION", "Skill updated: Test Skill", body, "SKILL", SKILL_ID); + assertSingleSse("current-admin", "SUBSCRIPTION_NEW_VERSION", body); + verify(accountRepository).findByIdIn(candidates); + verify(memberRepository).findByNamespaceIdAndUserIdIn(NAMESPACE_ID, candidates); + } + + @Test + void hiddenPublishPersistsAndPushesOnlyManagerWhileOrdinaryAndPlatformOnlyCandidatesStayAtZero() { + Skill skill = skill(SkillVisibility.PUBLIC, true, VERSION_ID); + Namespace namespace = namespace(NamespaceStatus.ACTIVE); + List candidates = List.of("manager", "ordinary-member", "platform-super-admin"); + arrangeEvent(skill, namespace, candidates); + when(accountRepository.findByIdIn(candidates)).thenReturn(List.of( + account("manager"), account("ordinary-member"), account("platform-super-admin"))); + when(memberRepository.findByNamespaceIdAndUserIdIn(NAMESPACE_ID, candidates)).thenReturn(List.of( + member("manager", NamespaceRole.ADMIN), member("ordinary-member", NamespaceRole.MEMBER))); + enablePersistenceFor("manager", "SUBSCRIPTION_NEW_VERSION"); + + listener.onSkillPublishedForSubscribers(new SkillPublishedEvent(SKILL_ID, VERSION_ID, "publisher")); + + String body = "{\"skillId\":1,\"skillName\":\"Test Skill\",\"slug\":\"test-skill\",\"namespace\":\"demo\"}"; + verify(notificationService).create("manager", NotificationCategory.PUBLISH, + "SUBSCRIPTION_NEW_VERSION", "Skill updated: Test Skill", body, "SKILL", SKILL_ID); + assertSingleSse("manager", "SUBSCRIPTION_NEW_VERSION", body); + verify(accountRepository).findByIdIn(candidates); + verify(memberRepository).findByNamespaceIdAndUserIdIn(NAMESPACE_ID, candidates); + } + + @ParameterizedTest(name = "yank wasPublished with fallback={0} reaches only current archived-namespace member") + @ValueSource(booleans = {true, false}) + void yankPersistsAndPushesOnlyCurrentMemberForFallbackAndNoFallback(boolean hasFallback) { + Skill skill = skill(SkillVisibility.PUBLIC, false, hasFallback ? 9L : null); + Namespace namespace = namespace(NamespaceStatus.ARCHIVED); + List candidates = List.of("actor", "current", "removed", "inactive", "missing"); + arrangeEvent(skill, namespace, candidates); + when(accountRepository.findByIdIn(candidates)).thenReturn(List.of( + account("actor"), account("current"), account("removed"), inactiveAccount("inactive"))); + when(memberRepository.findByNamespaceIdAndUserIdIn(NAMESPACE_ID, candidates)).thenReturn(List.of( + member("actor", NamespaceRole.ADMIN), member("current", NamespaceRole.MEMBER))); + enablePersistenceFor("current", "SUBSCRIPTION_VERSION_YANKED"); + + listener.onSkillVersionYankedForSubscribers( + new SkillVersionYankedEvent(SKILL_ID, VERSION_ID, "actor", true)); + + String body = "{\"skillId\":1,\"skillName\":\"Test Skill\",\"slug\":\"test-skill\",\"namespace\":\"demo\"}"; + verify(notificationService).create("current", NotificationCategory.PUBLISH, + "SUBSCRIPTION_VERSION_YANKED", "Skill version yanked: Test Skill", body, "SKILL", SKILL_ID); + assertSingleSse("current", "SUBSCRIPTION_VERSION_YANKED", body); + verify(accountRepository).findByIdIn(candidates); + verify(memberRepository).findByNamespaceIdAndUserIdIn(NAMESPACE_ID, candidates); + } + + @Test + void yankWithoutVerifiedPublishedPreStateProducesNoPersistenceOrSse() { + Skill skill = skill(SkillVisibility.PUBLIC, false, null); + Namespace namespace = namespace(NamespaceStatus.ACTIVE); + List candidates = List.of("current"); + arrangeEvent(skill, namespace, candidates); + when(accountRepository.findByIdIn(candidates)).thenReturn(List.of(account("current"))); + when(memberRepository.findByNamespaceIdAndUserIdIn(NAMESPACE_ID, candidates)).thenReturn(List.of()); + + listener.onSkillVersionYankedForSubscribers( + new SkillVersionYankedEvent(SKILL_ID, VERSION_ID, "actor", false)); + + verifyNoInteractions(notificationService, preferenceService, sseEmitterManager); + verify(accountRepository).findByIdIn(candidates); + verify(memberRepository).findByNamespaceIdAndUserIdIn(NAMESPACE_ID, candidates); + } + + @ParameterizedTest(name = "{0} batch failure happens before every final sink") + @EnumSource(BatchFailure.class) + void authoritativeBatchFailureProducesNoPartialPersistenceOrSse(BatchFailure failure) { + Skill skill = skill(SkillVisibility.PUBLIC, false, VERSION_ID); + Namespace namespace = namespace(NamespaceStatus.ACTIVE); + List candidates = List.of("first", "second"); + when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill)); + when(subscriptionService.findSubscribersBySkillId(SKILL_ID)).thenReturn(candidates); + if (failure == BatchFailure.NAMESPACE) { + when(namespaceRepository.findById(NAMESPACE_ID)).thenThrow(new IllegalStateException("namespace batch")); + } else { + when(namespaceRepository.findById(NAMESPACE_ID)).thenReturn(Optional.of(namespace)); + if (failure == BatchFailure.ACCOUNT) { + when(accountRepository.findByIdIn(candidates)).thenThrow(new IllegalStateException("account batch")); + } else { + when(accountRepository.findByIdIn(candidates)).thenReturn(List.of(account("first"), account("second"))); + when(memberRepository.findByNamespaceIdAndUserIdIn(NAMESPACE_ID, candidates)) + .thenThrow(new IllegalStateException("membership batch")); + } + } + + assertThatThrownBy(() -> listener.onSkillPublishedForSubscribers( + new SkillPublishedEvent(SKILL_ID, VERSION_ID, "publisher"))) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining(failure.name().toLowerCase()); + + verifyNoInteractions(notificationService, preferenceService, sseEmitterManager); + verify(namespaceRepository, times(1)).findById(NAMESPACE_ID); + if (failure == BatchFailure.NAMESPACE) { + verify(accountRepository, never()).findByIdIn(anyList()); + verify(memberRepository, never()).findByNamespaceIdAndUserIdIn(any(), anyCollection()); + } else { + verify(accountRepository, times(1)).findByIdIn(candidates); + verify(memberRepository, failure == BatchFailure.MEMBERSHIP ? times(1) : never()) + .findByNamespaceIdAndUserIdIn(eq(NAMESPACE_ID), anyCollection()); + } + } + + private void arrangeEvent(Skill skill, Namespace namespace, List candidates) { + when(skillRepository.findById(SKILL_ID)).thenReturn(Optional.of(skill)); + when(subscriptionService.findSubscribersBySkillId(SKILL_ID)).thenReturn(candidates); + when(namespaceRepository.findById(NAMESPACE_ID)).thenReturn(Optional.of(namespace)); + } + + private void enablePersistenceFor(String recipient, String eventType) { + when(preferenceService.isEnabled(recipient, NotificationCategory.PUBLISH, NotificationChannel.IN_APP)) + .thenReturn(true); + when(notificationService.create(eq(recipient), eq(NotificationCategory.PUBLISH), eq(eventType), + any(String.class), any(String.class), eq("SKILL"), eq(SKILL_ID))) + .thenAnswer(invocation -> notification(recipient, eventType, + invocation.getArgument(3), invocation.getArgument(4))); + } + + private void assertSingleSse(String recipient, String eventType, String body) { + @SuppressWarnings("unchecked") + ArgumentCaptor> payload = ArgumentCaptor.forClass(Map.class); + verify(sseEmitterManager).push(eq(recipient), payload.capture()); + assertThat(payload.getValue()) + .containsEntry("id", 42L) + .containsEntry("category", "PUBLISH") + .containsEntry("eventType", eventType) + .containsEntry("bodyJson", body) + .containsEntry("entityType", "SKILL") + .containsEntry("entityId", SKILL_ID); + } + + private Skill skill(SkillVisibility visibility, boolean hidden, Long latestVersionId) { + Skill skill = new Skill(NAMESPACE_ID, "test-skill", "publisher", visibility); + skill.setDisplayName("Test Skill"); + skill.setHidden(hidden); + skill.setLatestVersionId(latestVersionId); + setId(skill, SKILL_ID); + return skill; + } + + private Namespace namespace(NamespaceStatus status) { + Namespace namespace = new Namespace("demo", "Demo", "publisher"); + namespace.setStatus(status); + return namespace; + } + + private UserAccount account(String id) { + return new UserAccount(id, id, null, null); + } + + private UserAccount inactiveAccount(String id) { + UserAccount account = account(id); + account.setStatus(UserStatus.DISABLED); + return account; + } + + private NamespaceMember member(String userId, NamespaceRole role) { + return new NamespaceMember(NAMESPACE_ID, userId, role); + } + + private Notification notification(String recipient, String eventType, String title, String body) { + Notification notification = new Notification(recipient, NotificationCategory.PUBLISH, eventType, + title, body, "SKILL", SKILL_ID, CREATED_AT); + setId(notification, 42L); + return notification; + } + + private void setId(Object entity, Long id) { + try { + var field = entity.getClass().getDeclaredField("id"); + field.setAccessible(true); + field.set(entity, id); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException(e); + } + } + + private enum BatchFailure { + ACCOUNT, + NAMESPACE, + MEMBERSHIP + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java index 9df82b5d..a9a6d92d 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java @@ -1,3 +1,3 @@ package com.iflytek.skillhub.domain.event; -public record SkillVersionYankedEvent(Long skillId, Long versionId, String actorUserId) {} +public record SkillVersionYankedEvent(Long skillId, Long versionId, String actorUserId, boolean wasPublished) {} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceMemberRepository.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceMemberRepository.java index dc4545b9..9f87782f 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceMemberRepository.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/namespace/NamespaceMemberRepository.java @@ -15,6 +15,7 @@ public interface NamespaceMemberRepository { List findByUserId(String userId); Page findByNamespaceId(Long namespaceId, Pageable pageable); List findByNamespaceIdAndRoleIn(Long namespaceId, Collection roles); + List findByNamespaceIdAndUserIdIn(Long namespaceId, Collection userIds); NamespaceMember save(NamespaceMember member); void deleteByNamespaceId(Long namespaceId); void deleteByNamespaceIdAndUserId(Long namespaceId, String userId); diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java index 3cad927f..552d9e1f 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java @@ -281,7 +281,7 @@ public class SkillGovernanceService { }); auditLogService.record(actorUserId, "YANK_SKILL_VERSION", "SKILL_VERSION", versionId, null, clientIp, userAgent, jsonReason(reason)); eventPublisher.publishEvent(new com.iflytek.skillhub.domain.event.SkillVersionYankedEvent( - version.getSkillId(), versionId, actorUserId)); + version.getSkillId(), versionId, actorUserId, true)); return saved; } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillSubscriptionService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillSubscriptionService.java index 22710e42..a93ea150 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillSubscriptionService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SkillSubscriptionService.java @@ -2,6 +2,10 @@ package com.iflytek.skillhub.domain.social; import com.iflytek.skillhub.domain.shared.exception.DomainNotFoundException; import com.iflytek.skillhub.domain.skill.SkillRepository; +import com.iflytek.skillhub.domain.namespace.*; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; +import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException; import com.iflytek.skillhub.domain.social.event.SkillSubscribedEvent; import com.iflytek.skillhub.domain.social.event.SkillUnsubscribedEvent; import org.springframework.context.ApplicationEventPublisher; @@ -9,24 +13,46 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import java.util.List; +import java.util.Map; @Service public class SkillSubscriptionService { private final SkillSubscriptionRepository subscriptionRepository; private final SkillRepository skillRepository; private final ApplicationEventPublisher eventPublisher; + private final NamespaceRepository namespaceRepository; + private final NamespaceMemberRepository memberRepository; + private final UserAccountRepository accountRepository; + private final SubscriptionMetadataAccessPolicy accessPolicy; public SkillSubscriptionService(SkillSubscriptionRepository subscriptionRepository, SkillRepository skillRepository, - ApplicationEventPublisher eventPublisher) { + ApplicationEventPublisher eventPublisher, + NamespaceRepository namespaceRepository, + NamespaceMemberRepository memberRepository, + UserAccountRepository accountRepository, + SubscriptionMetadataAccessPolicy accessPolicy) { this.subscriptionRepository = subscriptionRepository; this.skillRepository = skillRepository; this.eventPublisher = eventPublisher; + this.namespaceRepository = namespaceRepository; + this.memberRepository = memberRepository; + this.accountRepository = accountRepository; + this.accessPolicy = accessPolicy; } @Transactional public void subscribe(Long skillId, String userId) { - ensureSkillExists(skillId); + var skill = skillRepository.findById(skillId) + .orElseThrow(() -> new DomainNotFoundException("skill.not_found", skillId)); + UserAccount account = accountRepository.findById(userId).orElse(null); + Namespace namespace = namespaceRepository.findById(skill.getNamespaceId()).orElse(null); + var role = memberRepository.findByNamespaceIdAndUserId(skill.getNamespaceId(), userId) + .map(NamespaceMember::getRole); + Map roles = role.map(value -> Map.of(skill.getNamespaceId(), value)).orElse(Map.of()); + if (!accessPolicy.canAccessCurrent(skill, namespace, account, roles)) { + throw new DomainForbiddenException("error.skill.subscription.noPermission"); + } if (subscriptionRepository.findBySkillIdAndUserId(skillId, userId).isPresent()) { return; // idempotent } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionMetadataAccessPolicy.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionMetadataAccessPolicy.java new file mode 100644 index 00000000..f71a0eaa --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionMetadataAccessPolicy.java @@ -0,0 +1,52 @@ +package com.iflytek.skillhub.domain.social; + +import com.iflytek.skillhub.domain.namespace.Namespace; +import com.iflytek.skillhub.domain.namespace.NamespaceRole; +import com.iflytek.skillhub.domain.namespace.NamespaceStatus; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.user.UserAccount; + +import java.util.Map; +import org.springframework.stereotype.Component; + +/** Authorization for metadata exposed by subscriptions and subscriber notifications. */ +@Component +public class SubscriptionMetadataAccessPolicy { + + public boolean canAccessCurrent(Skill skill, Namespace namespace, UserAccount account, + Map namespaceRoles) { + return canAccess(skill, namespace, account, namespaceRoles, false); + } + + public boolean canAccessYankedPublication(Skill skill, Namespace namespace, UserAccount account, + Map namespaceRoles, + boolean wasPublished) { + return wasPublished && canAccess(skill, namespace, account, namespaceRoles, true); + } + + private boolean canAccess(Skill skill, Namespace namespace, UserAccount account, + Map namespaceRoles, boolean yankedPublication) { + if (account == null || !account.isActive() || namespace == null) { + return false; + } + Map roles = namespaceRoles == null ? Map.of() : namespaceRoles; + NamespaceRole role = roles.get(skill.getNamespaceId()); + if (namespace.getStatus() == NamespaceStatus.ARCHIVED && role == null) { + return false; + } + boolean owner = skill.getOwnerId().equals(account.getId()); + boolean manager = role == NamespaceRole.ADMIN || role == NamespaceRole.OWNER; + if (skill.isHidden()) { + return owner || manager; + } + if (!yankedPublication && skill.getLatestVersionId() == null) { + return owner; + } + return switch (skill.getVisibility()) { + case PUBLIC -> true; + case NAMESPACE_ONLY -> role != null; + case PRIVATE -> owner || manager; + }; + } +} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionRecipientEligibility.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionRecipientEligibility.java new file mode 100644 index 00000000..42a2f195 --- /dev/null +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionRecipientEligibility.java @@ -0,0 +1,53 @@ +package com.iflytek.skillhub.domain.social; + +import com.iflytek.skillhub.domain.namespace.*; +import com.iflytek.skillhub.domain.skill.Skill; +import com.iflytek.skillhub.domain.user.UserAccount; +import com.iflytek.skillhub.domain.user.UserAccountRepository; + +import java.util.*; +import java.util.function.BiPredicate; +import org.springframework.stereotype.Component; + +/** Loads authoritative account and membership facts in batches before selecting recipients. */ +@Component +public class SubscriptionRecipientEligibility { + private final UserAccountRepository accountRepository; + private final NamespaceMemberRepository memberRepository; + private final SubscriptionMetadataAccessPolicy policy; + + public SubscriptionRecipientEligibility(UserAccountRepository accountRepository, + NamespaceMemberRepository memberRepository, + SubscriptionMetadataAccessPolicy policy) { + this.accountRepository = accountRepository; + this.memberRepository = memberRepository; + this.policy = policy; + } + + public List currentRecipients(Skill skill, Namespace namespace, List candidateIds) { + return eligible(skill, namespace, candidateIds, + (account, roles) -> policy.canAccessCurrent(skill, namespace, account, roles)); + } + + public List yankedRecipients(Skill skill, Namespace namespace, List candidateIds, + boolean wasPublished) { + return eligible(skill, namespace, candidateIds, + (account, roles) -> policy.canAccessYankedPublication(skill, namespace, account, roles, wasPublished)); + } + + private List eligible(Skill skill, Namespace namespace, List candidateIds, + BiPredicate> predicate) { + List ids = candidateIds.stream().distinct().toList(); + if (ids.isEmpty()) return List.of(); + Map accounts = new HashMap<>(); + accountRepository.findByIdIn(ids).forEach(account -> accounts.put(account.getId(), account)); + Map roles = new HashMap<>(); + memberRepository.findByNamespaceIdAndUserIdIn(skill.getNamespaceId(), ids) + .forEach(member -> roles.put(member.getUserId(), member.getRole())); + return ids.stream().filter(id -> { + NamespaceRole role = roles.get(id); + Map roleMap = role == null ? Map.of() : Map.of(skill.getNamespaceId(), role); + return predicate.test(accounts.get(id), roleMap); + }).toList(); + } +} diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java index 89a2e4cb..56ea05ed 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadServiceTest.java @@ -214,6 +214,25 @@ class SkillDownloadServiceTest { verify(eventPublisher, never()).publishEvent(any(SkillDownloadedEvent.class)); } + @Test + void downloadRejectsPublicArchivedSkillEvenThoughItsMetadataRemainsReadable() throws Exception { + Namespace namespace = new Namespace("global", "Global", "owner-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, "archived-skill", "owner-1", SkillVisibility.PUBLIC); + setId(skill, 9L); + skill.setStatus(SkillStatus.ARCHIVED); + skill.setLatestVersionId(10L); + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "archived-skill")).thenReturn(List.of(skill)); + + assertThrows(com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException.class, () -> + service.downloadLatest("global", "archived-skill", "viewer", Map.of())); + + verify(skillRepository, never()).incrementDownloadCount(anyLong()); + verify(skillVersionStatsRepository, never()).incrementDownloadCount(anyLong(), anyLong()); + verify(eventPublisher, never()).publishEvent(any(SkillDownloadedEvent.class)); + } + @Test void testDownloadLatest_ShouldRejectAnonymousHiddenPrivateAndUnpublishedSkills() throws Exception { Namespace namespace = new Namespace("global", "Global", "owner-1"); diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java index 02cf02f1..dc5782c4 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillQueryServiceTest.java @@ -202,6 +202,64 @@ class SkillQueryServiceTest { service.getSkillDetail(namespaceSlug, skillSlug, null, Map.of())); } + @Test + void getSkillDetailKeepsPublicArchivedSkillDirectlyReadableWithArchivedStatus() throws Exception { + Namespace namespace = new Namespace("global", "Global", "owner-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, "archived-skill", "owner-1", SkillVisibility.PUBLIC); + setId(skill, 9L); + skill.setStatus(SkillStatus.ARCHIVED); + skill.setLatestVersionId(10L); + SkillVersion version = new SkillVersion(9L, "1.0.0", "owner-1"); + setId(version, 10L); + version.setStatus(SkillVersionStatus.PUBLISHED); + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "archived-skill")).thenReturn(List.of(skill)); + when(skillVersionRepository.findById(10L)).thenReturn(Optional.of(version)); + when(userAccountRepository.findById("owner-1")) + .thenReturn(Optional.of(new UserAccount("owner-1", "Owner", null, null))); + + SkillQueryService.SkillDetailDTO detail = + service.getSkillDetail("global", "archived-skill", "viewer", Map.of()); + + assertEquals("ARCHIVED", detail.status()); + assertEquals("archived-skill", detail.slug()); + } + + @Test + void namespaceDiscoveryQueriesOnlyActiveSkillsSoArchivedSkillIsAbsent() throws Exception { + Namespace namespace = new Namespace("global", "Global", "owner-1"); + setId(namespace, 1L); + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndStatus(1L, SkillStatus.ACTIVE)).thenReturn(List.of()); + + Page result = service.listSkillsByNamespace("global", "viewer", Map.of(), PageRequest.of(0, 20)); + + assertTrue(result.isEmpty()); + verify(skillRepository).findByNamespaceIdAndStatus(1L, SkillStatus.ACTIVE); + } + + @Test + void versionContentRejectsPublicArchivedSkillForNonManager() throws Exception { + Namespace namespace = new Namespace("global", "Global", "owner-1"); + setId(namespace, 1L); + Skill skill = new Skill(1L, "archived-skill", "owner-1", SkillVisibility.PUBLIC); + setId(skill, 9L); + skill.setStatus(SkillStatus.ARCHIVED); + skill.setLatestVersionId(10L); + SkillVersion version = new SkillVersion(9L, "1.0.0", "owner-1"); + setId(version, 10L); + version.setStatus(SkillVersionStatus.PUBLISHED); + when(namespaceRepository.findBySlug("global")).thenReturn(Optional.of(namespace)); + when(skillRepository.findByNamespaceIdAndSlug(1L, "archived-skill")).thenReturn(List.of(skill)); + lenient().when(skillVersionRepository.findBySkillIdAndVersion(9L, "1.0.0")) + .thenReturn(Optional.of(version)); + + assertThrows(DomainForbiddenException.class, () -> + service.listFiles("global", "archived-skill", "1.0.0", "viewer", Map.of())); + verifyNoInteractions(skillFileRepository); + } + @Test void testListSkillsByNamespace() throws Exception { // Arrange diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java index 264e129f..0b3b47d0 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java @@ -86,6 +86,7 @@ class SkillReviewSubmitServiceTest { assertEquals(SkillVersionStatus.PENDING_REVIEW, version.getStatus()); assertEquals(SkillVisibility.PUBLIC, version.getRequestedVisibility()); verify(reviewTaskRepository).save(any(ReviewTask.class)); + verify(eventPublisher, never()).publishEvent(any()); } @Test @@ -186,7 +187,7 @@ class SkillReviewSubmitServiceTest { ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(SkillPublishedEvent.class); - verify(eventPublisher).publishEvent(eventCaptor.capture()); + verify(eventPublisher, times(1)).publishEvent(eventCaptor.capture()); SkillPublishedEvent event = eventCaptor.getValue(); assertEquals(skillId, event.skillId()); assertEquals(versionId, event.versionId()); @@ -258,6 +259,30 @@ class SkillReviewSubmitServiceTest { verify(eventPublisher, never()).publishEvent(any()); } + @Test + @DisplayName("should not publish event when version persistence fails") + void shouldNotPublishEventWhenVersionPersistenceFails() { + // Given + Long skillId = 1L; + Long versionId = 100L; + String userId = "user-1"; + + Skill skill = createSkill(skillId, userId, 10L, SkillVisibility.PRIVATE); + SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.UPLOADED); + RuntimeException persistenceFailure = new RuntimeException("version save failed"); + + when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); + when(skillVersionRepository.save(version)).thenThrow(persistenceFailure); + + // When/Then + RuntimeException thrown = assertThrows(RuntimeException.class, + () -> service.confirmPublish(skillId, versionId, userId, Map.of())); + assertSame(persistenceFailure, thrown); + verify(skillRepository, never()).save(any()); + verify(eventPublisher, never()).publishEvent(any()); + } + @Test @DisplayName("should not publish event when skill persistence fails") void shouldNotPublishEventWhenSkillPersistenceFails() { diff --git a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/NamespaceMemberJpaRepository.java b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/NamespaceMemberJpaRepository.java index d1b1be75..492b9afe 100644 --- a/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/NamespaceMemberJpaRepository.java +++ b/server/skillhub-infra/src/main/java/com/iflytek/skillhub/infra/jpa/NamespaceMemberJpaRepository.java @@ -22,6 +22,7 @@ public interface NamespaceMemberJpaRepository List findByUserId(String userId); Page findByNamespaceId(Long namespaceId, Pageable pageable); List findByNamespaceIdAndRoleIn(Long namespaceId, Collection roles); + List findByNamespaceIdAndUserIdIn(Long namespaceId, Collection userIds); void deleteByNamespaceId(Long namespaceId); void deleteByNamespaceIdAndUserId(Long namespaceId, String userId); } diff --git a/server/skillhub-notification/src/test/java/com/iflytek/skillhub/notification/service/NotificationDispatcherTest.java b/server/skillhub-notification/src/test/java/com/iflytek/skillhub/notification/service/NotificationDispatcherTest.java index 10611a40..db6af43b 100644 --- a/server/skillhub-notification/src/test/java/com/iflytek/skillhub/notification/service/NotificationDispatcherTest.java +++ b/server/skillhub-notification/src/test/java/com/iflytek/skillhub/notification/service/NotificationDispatcherTest.java @@ -11,6 +11,11 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; import java.time.Instant; +import java.util.Map; + +import org.mockito.ArgumentCaptor; + +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; @@ -58,6 +63,41 @@ class NotificationDispatcherTest { verify(sseEmitterManager).push(eq("user-1"), any()); } + @Test + void dispatch_persistsExactSubscriberNotificationAndPushesSameRecipientVisiblePayload() { + Notification notification = new Notification("subscriber-1", NotificationCategory.PUBLISH, + "SUBSCRIPTION_NEW_VERSION", "Skill updated: Demo", + "{\"skillId\":1,\"versionId\":10}", "SKILL", 1L, + Instant.parse("2026-08-19T20:30:00Z")); + try { + var id = Notification.class.getDeclaredField("id"); + id.setAccessible(true); + id.set(notification, 42L); + } catch (ReflectiveOperationException e) { + throw new IllegalStateException(e); + } + when(preferenceService.isEnabled("subscriber-1", NotificationCategory.PUBLISH, + NotificationChannel.IN_APP)).thenReturn(true); + when(notificationService.create(any(), any(), any(), any(), any(), any(), any())) + .thenReturn(notification); + + dispatcher.dispatch("subscriber-1", NotificationCategory.PUBLISH, "SUBSCRIPTION_NEW_VERSION", + "Skill updated: Demo", "{\"skillId\":1,\"versionId\":10}", "SKILL", 1L); + + verify(notificationService).create("subscriber-1", NotificationCategory.PUBLISH, + "SUBSCRIPTION_NEW_VERSION", "Skill updated: Demo", + "{\"skillId\":1,\"versionId\":10}", "SKILL", 1L); + @SuppressWarnings("unchecked") + ArgumentCaptor> payload = ArgumentCaptor.forClass(Map.class); + verify(sseEmitterManager).push(eq("subscriber-1"), payload.capture()); + assertThat(payload.getValue()).containsEntry("id", 42L) + .containsEntry("category", "PUBLISH") + .containsEntry("eventType", "SUBSCRIPTION_NEW_VERSION") + .containsEntry("bodyJson", "{\"skillId\":1,\"versionId\":10}") + .containsEntry("entityType", "SKILL") + .containsEntry("entityId", 1L); + } + @Test void dispatch_shouldSkipWhenPreferenceDisabled() { when(preferenceService.isEnabled("user-1", NotificationCategory.REVIEW, NotificationChannel.IN_APP)) From 13316674964ffa115ef3a6089bb01831ad3356c0 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:45:55 +0800 Subject: [PATCH 2/4] fix(subscription): reuse visibility policy and localize denial Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../src/main/resources/messages.properties | 1 + .../src/main/resources/messages_zh.properties | 1 + .../domain/event/SkillVersionYankedEvent.java | 2 +- .../skill/service/SkillGovernanceService.java | 2 +- .../SubscriptionMetadataAccessPolicy.java | 30 ++++++++++++++++++- .../service/SkillReviewSubmitServiceTest.java | 27 +---------------- 6 files changed, 34 insertions(+), 29 deletions(-) diff --git a/server/skillhub-app/src/main/resources/messages.properties b/server/skillhub-app/src/main/resources/messages.properties index 50493763..82b255e5 100644 --- a/server/skillhub-app/src/main/resources/messages.properties +++ b/server/skillhub-app/src/main/resources/messages.properties @@ -185,3 +185,4 @@ promotion.status.invalid=Unsupported promotion status: {0} promotion.sort.field.invalid=Unsupported promotion sort field: {0} promotion.sort.direction.invalid=Unsupported promotion sort direction: {0} promotion.sort.pending_unsupported=Pending promotion requests do not support reviewed-time sorting +error.skill.subscription.noPermission=You do not have permission to subscribe to this skill. diff --git a/server/skillhub-app/src/main/resources/messages_zh.properties b/server/skillhub-app/src/main/resources/messages_zh.properties index febc9c4e..0f2e1808 100644 --- a/server/skillhub-app/src/main/resources/messages_zh.properties +++ b/server/skillhub-app/src/main/resources/messages_zh.properties @@ -185,3 +185,4 @@ promotion.status.invalid=不支持的提升审核状态：{0} promotion.sort.field.invalid=不支持的提升审核排序字段：{0} promotion.sort.direction.invalid=不支持的提升审核排序方向：{0} promotion.sort.pending_unsupported=待审核提升请求不支持按处理时间排序 +error.skill.subscription.noPermission=您没有订阅此技能的权限。 diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java index a9a6d92d..9df82b5d 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java @@ -1,3 +1,3 @@ package com.iflytek.skillhub.domain.event; -public record SkillVersionYankedEvent(Long skillId, Long versionId, String actorUserId, boolean wasPublished) {} +public record SkillVersionYankedEvent(Long skillId, Long versionId, String actorUserId) {} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java index 552d9e1f..3cad927f 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java @@ -281,7 +281,7 @@ public class SkillGovernanceService { }); auditLogService.record(actorUserId, "YANK_SKILL_VERSION", "SKILL_VERSION", versionId, null, clientIp, userAgent, jsonReason(reason)); eventPublisher.publishEvent(new com.iflytek.skillhub.domain.event.SkillVersionYankedEvent( - version.getSkillId(), versionId, actorUserId, true)); + version.getSkillId(), versionId, actorUserId)); return saved; } diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionMetadataAccessPolicy.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionMetadataAccessPolicy.java index f71a0eaa..c166ae21 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionMetadataAccessPolicy.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/social/SubscriptionMetadataAccessPolicy.java @@ -5,18 +5,37 @@ import com.iflytek.skillhub.domain.namespace.NamespaceRole; import com.iflytek.skillhub.domain.namespace.NamespaceStatus; import com.iflytek.skillhub.domain.skill.Skill; import com.iflytek.skillhub.domain.skill.SkillVisibility; +import com.iflytek.skillhub.domain.skill.VisibilityChecker; import com.iflytek.skillhub.domain.user.UserAccount; import java.util.Map; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.stereotype.Component; /** Authorization for metadata exposed by subscriptions and subscriber notifications. */ @Component public class SubscriptionMetadataAccessPolicy { + private final VisibilityChecker visibilityChecker; + + public SubscriptionMetadataAccessPolicy() { + this(new VisibilityChecker()); + } + + @Autowired + public SubscriptionMetadataAccessPolicy(VisibilityChecker visibilityChecker) { + this.visibilityChecker = visibilityChecker; + } public boolean canAccessCurrent(Skill skill, Namespace namespace, UserAccount account, Map namespaceRoles) { - return canAccess(skill, namespace, account, namespaceRoles, false); + if (account == null || !account.isActive() || namespace == null) { + return false; + } + if (namespace.getStatus() == NamespaceStatus.ARCHIVED + && !isOwnerOrManager(skill, account, namespaceRoles)) { + return false; + } + return visibilityChecker.canAccess(skill, account.getId(), namespaceRoles); } public boolean canAccessYankedPublication(Skill skill, Namespace namespace, UserAccount account, @@ -49,4 +68,13 @@ public class SubscriptionMetadataAccessPolicy { case PRIVATE -> owner || manager; }; } + + private boolean isOwnerOrManager(Skill skill, UserAccount account, + Map namespaceRoles) { + if (skill.getOwnerId().equals(account.getId())) { + return true; + } + NamespaceRole role = namespaceRoles == null ? null : namespaceRoles.get(skill.getNamespaceId()); + return role == NamespaceRole.ADMIN || role == NamespaceRole.OWNER; + } } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java index 0b3b47d0..264e129f 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java @@ -86,7 +86,6 @@ class SkillReviewSubmitServiceTest { assertEquals(SkillVersionStatus.PENDING_REVIEW, version.getStatus()); assertEquals(SkillVisibility.PUBLIC, version.getRequestedVisibility()); verify(reviewTaskRepository).save(any(ReviewTask.class)); - verify(eventPublisher, never()).publishEvent(any()); } @Test @@ -187,7 +186,7 @@ class SkillReviewSubmitServiceTest { ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(SkillPublishedEvent.class); - verify(eventPublisher, times(1)).publishEvent(eventCaptor.capture()); + verify(eventPublisher).publishEvent(eventCaptor.capture()); SkillPublishedEvent event = eventCaptor.getValue(); assertEquals(skillId, event.skillId()); assertEquals(versionId, event.versionId()); @@ -259,30 +258,6 @@ class SkillReviewSubmitServiceTest { verify(eventPublisher, never()).publishEvent(any()); } - @Test - @DisplayName("should not publish event when version persistence fails") - void shouldNotPublishEventWhenVersionPersistenceFails() { - // Given - Long skillId = 1L; - Long versionId = 100L; - String userId = "user-1"; - - Skill skill = createSkill(skillId, userId, 10L, SkillVisibility.PRIVATE); - SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.UPLOADED); - RuntimeException persistenceFailure = new RuntimeException("version save failed"); - - when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); - when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); - when(skillVersionRepository.save(version)).thenThrow(persistenceFailure); - - // When/Then - RuntimeException thrown = assertThrows(RuntimeException.class, - () -> service.confirmPublish(skillId, versionId, userId, Map.of())); - assertSame(persistenceFailure, thrown); - verify(skillRepository, never()).save(any()); - verify(eventPublisher, never()).publishEvent(any()); - } - @Test @DisplayName("should not publish event when skill persistence fails") void shouldNotPublishEventWhenSkillPersistenceFails() { From 126f01d75e1c61dab9656f04739702a06afb2aa2 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:50:37 +0800 Subject: [PATCH 3/4] fix(subscription): retain yank visibility context Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../domain/event/SkillVersionYankedEvent.java | 2 +- .../skill/service/SkillGovernanceService.java | 2 +- .../service/SkillReviewSubmitServiceTest.java | 27 ++++++++++++++++++- 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java index 9df82b5d..a9a6d92d 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/event/SkillVersionYankedEvent.java @@ -1,3 +1,3 @@ package com.iflytek.skillhub.domain.event; -public record SkillVersionYankedEvent(Long skillId, Long versionId, String actorUserId) {} +public record SkillVersionYankedEvent(Long skillId, Long versionId, String actorUserId, boolean wasPublished) {} diff --git a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java index 3cad927f..552d9e1f 100644 --- a/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java +++ b/server/skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillGovernanceService.java @@ -281,7 +281,7 @@ public class SkillGovernanceService { }); auditLogService.record(actorUserId, "YANK_SKILL_VERSION", "SKILL_VERSION", versionId, null, clientIp, userAgent, jsonReason(reason)); eventPublisher.publishEvent(new com.iflytek.skillhub.domain.event.SkillVersionYankedEvent( - version.getSkillId(), versionId, actorUserId)); + version.getSkillId(), versionId, actorUserId, true)); return saved; } diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java index 264e129f..0b3b47d0 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java @@ -86,6 +86,7 @@ class SkillReviewSubmitServiceTest { assertEquals(SkillVersionStatus.PENDING_REVIEW, version.getStatus()); assertEquals(SkillVisibility.PUBLIC, version.getRequestedVisibility()); verify(reviewTaskRepository).save(any(ReviewTask.class)); + verify(eventPublisher, never()).publishEvent(any()); } @Test @@ -186,7 +187,7 @@ class SkillReviewSubmitServiceTest { ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(SkillPublishedEvent.class); - verify(eventPublisher).publishEvent(eventCaptor.capture()); + verify(eventPublisher, times(1)).publishEvent(eventCaptor.capture()); SkillPublishedEvent event = eventCaptor.getValue(); assertEquals(skillId, event.skillId()); assertEquals(versionId, event.versionId()); @@ -258,6 +259,30 @@ class SkillReviewSubmitServiceTest { verify(eventPublisher, never()).publishEvent(any()); } + @Test + @DisplayName("should not publish event when version persistence fails") + void shouldNotPublishEventWhenVersionPersistenceFails() { + // Given + Long skillId = 1L; + Long versionId = 100L; + String userId = "user-1"; + + Skill skill = createSkill(skillId, userId, 10L, SkillVisibility.PRIVATE); + SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.UPLOADED); + RuntimeException persistenceFailure = new RuntimeException("version save failed"); + + when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); + when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); + when(skillVersionRepository.save(version)).thenThrow(persistenceFailure); + + // When/Then + RuntimeException thrown = assertThrows(RuntimeException.class, + () -> service.confirmPublish(skillId, versionId, userId, Map.of())); + assertSame(persistenceFailure, thrown); + verify(skillRepository, never()).save(any()); + verify(eventPublisher, never()).publishEvent(any()); + } + @Test @DisplayName("should not publish event when skill persistence fails") void shouldNotPublishEventWhenSkillPersistenceFails() { From a3d1b4c9c533a8348fc5d163174dea60e86746a0 Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Thu, 27 Aug 2026 16:56:01 +0800 Subject: [PATCH 4/4] test(subscription): remove unrelated confirm-publish assertions Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../service/SkillReviewSubmitServiceTest.java | 27 +------------------ 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java index 0b3b47d0..264e129f 100644 --- a/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java +++ b/server/skillhub-domain/src/test/java/com/iflytek/skillhub/domain/skill/service/SkillReviewSubmitServiceTest.java @@ -86,7 +86,6 @@ class SkillReviewSubmitServiceTest { assertEquals(SkillVersionStatus.PENDING_REVIEW, version.getStatus()); assertEquals(SkillVisibility.PUBLIC, version.getRequestedVisibility()); verify(reviewTaskRepository).save(any(ReviewTask.class)); - verify(eventPublisher, never()).publishEvent(any()); } @Test @@ -187,7 +186,7 @@ class SkillReviewSubmitServiceTest { ArgumentCaptor eventCaptor = ArgumentCaptor.forClass(SkillPublishedEvent.class); - verify(eventPublisher, times(1)).publishEvent(eventCaptor.capture()); + verify(eventPublisher).publishEvent(eventCaptor.capture()); SkillPublishedEvent event = eventCaptor.getValue(); assertEquals(skillId, event.skillId()); assertEquals(versionId, event.versionId()); @@ -259,30 +258,6 @@ class SkillReviewSubmitServiceTest { verify(eventPublisher, never()).publishEvent(any()); } - @Test - @DisplayName("should not publish event when version persistence fails") - void shouldNotPublishEventWhenVersionPersistenceFails() { - // Given - Long skillId = 1L; - Long versionId = 100L; - String userId = "user-1"; - - Skill skill = createSkill(skillId, userId, 10L, SkillVisibility.PRIVATE); - SkillVersion version = createVersion(versionId, skillId, SkillVersionStatus.UPLOADED); - RuntimeException persistenceFailure = new RuntimeException("version save failed"); - - when(skillRepository.findById(skillId)).thenReturn(Optional.of(skill)); - when(skillVersionRepository.findById(versionId)).thenReturn(Optional.of(version)); - when(skillVersionRepository.save(version)).thenThrow(persistenceFailure); - - // When/Then - RuntimeException thrown = assertThrows(RuntimeException.class, - () -> service.confirmPublish(skillId, versionId, userId, Map.of())); - assertSame(persistenceFailure, thrown); - verify(skillRepository, never()).save(any()); - verify(eventPublisher, never()).publishEvent(any()); - } - @Test @DisplayName("should not publish event when skill persistence fails") void shouldNotPublishEventWhenSkillPersistenceFails() {