Skip to content

fix(cli): align anonymous installability rules (ISSUE-39) #515

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
dongmucat wants to merge 2 commits into from
Closed

fix(cli): align anonymous installability rules (ISSUE-39) #515

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3a111dc
fix(cli): align anonymous installability rules
dongmucat Jun 12, 2026
7f8d2aa
fix(cli): require installable latest in search
dongmucat Jun 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions server/skillhub-app/src/main/java/com/iflytek/skillhub/service/cli/CliSkillAppService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,16 @@ public CliSearchResult search(String q, int limit, String userId, Map<Long, Name
);

List<CliSearchItem> items = response.items().stream()
.filter(item -> item.publishedVersion() != null)
.map(item -> new CliSearchItem(
item.namespace(),
item.slug(),
item.publishedVersion() != null ? item.publishedVersion().version() : null,
item.publishedVersion().version(),
item.summary()
))
.toList();

return new CliSearchResult(items, response.total(), limit);
return new CliSearchResult(items, items.size(), limit);
}

public CliResolveResponse resolve(String namespace, String slug, String version, String userId, Map<Long, NamespaceRole> userNsRoles) {
Expand Down
121 changes: 113 additions & 8 deletions ...er/skillhub-app/src/test/java/com/iflytek/skillhub/service/SkillSearchAppServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
import com.iflytek.skillhub.domain.namespace.NamespaceService;
import com.iflytek.skillhub.domain.skill.Skill;
import com.iflytek.skillhub.domain.skill.SkillRepository;
import com.iflytek.skillhub.domain.skill.SkillVersion;
import com.iflytek.skillhub.domain.skill.SkillVersionRepository;
import com.iflytek.skillhub.domain.skill.SkillVersionStatus;
import com.iflytek.skillhub.domain.skill.SkillVisibility;
import com.iflytek.skillhub.domain.skill.service.SkillLifecycleProjectionService;
import com.iflytek.skillhub.search.SearchQuery;
Expand All @@ -22,11 +24,13 @@
import org.mockito.ArgumentCaptor;
import org.mockito.junit.jupiter.MockitoExtension;

import java.time.Instant;
import java.util.List;
import java.util.Map;
import java.util.Set;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyList;
Expand Down Expand Up @@ -96,8 +100,6 @@ void search_shouldFillVisiblePageAcrossArchivedNamespaceResults() {
when(skillRepository.findByIdIn(List.of(11L))).thenReturn(List.of(visibleSkill));
when(namespaceRepository.findByIdIn(List.of(2L))).thenReturn(List.of(activeNamespace));
when(skillVersionRepository.findByIdIn(List.of(111L))).thenReturn(List.of());
when(skillVersionRepository.findBySkillIdInAndStatus(List.of(11L), com.iflytek.skillhub.domain.skill.SkillVersionStatus.PUBLISHED))
.thenReturn(List.of());

SkillSearchAppService.SearchResponse response = service.search("skill", null, "newest", 0, 1, null, null);

Expand Down Expand Up @@ -145,8 +147,6 @@ void search_shouldExcludeHiddenSkillsForRegularUsers() {
when(skillRepository.findByIdIn(List.of(10L))).thenReturn(List.of(visibleSkill));
when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace));
when(skillVersionRepository.findByIdIn(List.of(101L))).thenReturn(List.of());
when(skillVersionRepository.findBySkillIdInAndStatus(List.of(10L), com.iflytek.skillhub.domain.skill.SkillVersionStatus.PUBLISHED))
.thenReturn(List.of());

SkillSearchAppService.SearchResponse response = service.search("skill", null, "newest", 0, 20, "user-9", Map.of());

Expand All @@ -164,6 +164,9 @@ void search_shouldResolvePublishedVersionsInBatch() {
setField(second, "id", 11L);
second.setLatestVersionId(102L);

SkillVersion firstVersion = publishedVersion(10L, 101L, "1.0.0");
SkillVersion secondVersion = publishedVersion(11L, 102L, "2.0.0");

Namespace namespace = new Namespace("team-a", "Team A", "owner-1");
setField(namespace, "id", 1L);
namespace.setStatus(NamespaceStatus.ACTIVE);
Expand All @@ -172,18 +175,112 @@ void search_shouldResolvePublishedVersionsInBatch() {
.thenReturn(new SearchResult(List.of(10L, 11L), 2, 0, 20));
when(skillRepository.findByIdIn(List.of(10L, 11L))).thenReturn(List.of(first, second));
when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace));
when(skillVersionRepository.findByIdIn(List.of(101L, 102L))).thenReturn(List.of());
when(skillVersionRepository.findBySkillIdInAndStatus(List.of(10L, 11L), com.iflytek.skillhub.domain.skill.SkillVersionStatus.PUBLISHED))
.thenReturn(List.of());
when(skillVersionRepository.findByIdIn(List.of(101L, 102L))).thenReturn(List.of(firstVersion, secondVersion));

SkillSearchAppService.SearchResponse response = service.search(null, null, "newest", 0, 20, null, null);

assertEquals(2, response.items().size());
assertEquals("1.0.0", response.items().get(0).publishedVersion().version());
assertEquals("2.0.0", response.items().get(1).publishedVersion().version());
verify(skillVersionRepository, times(1)).findByIdIn(List.of(101L, 102L));
verify(skillVersionRepository, times(1))
verify(skillVersionRepository, times(0))
.findBySkillIdInAndStatus(List.of(10L, 11L), com.iflytek.skillhub.domain.skill.SkillVersionStatus.PUBLISHED);
}

@Test
void search_shouldNotFallbackToOlderPublishedVersionWhenLatestIsMissing() {
Skill skill = new Skill(1L, "missing-latest", "owner-1", SkillVisibility.PUBLIC);
setField(skill, "id", 10L);

SkillVersion oldInstallable = publishedVersion(10L, 100L, "0.9.0");

Namespace namespace = new Namespace("global", "Global", "owner-1");
setField(namespace, "id", 1L);
namespace.setStatus(NamespaceStatus.ACTIVE);

when(searchQueryService.search(any()))
.thenReturn(new SearchResult(List.of(10L), 1, 0, 20));
when(skillRepository.findByIdIn(List.of(10L))).thenReturn(List.of(skill));
when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace));
org.mockito.Mockito.lenient()
.when(skillVersionRepository.findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED))
.thenReturn(List.of(oldInstallable));

SkillSearchAppService.SearchResponse response = service.search(null, null, "newest", 0, 20, null, null);

assertEquals(1, response.items().size());
assertEquals("missing-latest", response.items().getFirst().slug());
assertNull(response.items().getFirst().publishedVersion());
verify(skillVersionRepository, times(0))
.findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED);
}

@Test
void search_shouldNotFallbackToOlderPublishedVersionWhenLatestIsYanked() {
Skill skill = new Skill(1L, "yanked-latest", "owner-1", SkillVisibility.PUBLIC);
setField(skill, "id", 10L);
skill.setLatestVersionId(101L);

SkillVersion latest = publishedVersion(10L, 101L, "1.0.0");
latest.setYankedAt(Instant.parse("2026-06-12T00:00:00Z"));
SkillVersion oldInstallable = publishedVersion(10L, 100L, "0.9.0");

Namespace namespace = new Namespace("global", "Global", "owner-1");
setField(namespace, "id", 1L);
namespace.setStatus(NamespaceStatus.ACTIVE);

when(searchQueryService.search(any()))
.thenReturn(new SearchResult(List.of(10L), 1, 0, 20));
when(skillRepository.findByIdIn(List.of(10L))).thenReturn(List.of(skill));
when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace));
when(skillVersionRepository.findByIdIn(List.of(101L))).thenReturn(List.of(latest));
org.mockito.Mockito.lenient()
.when(skillVersionRepository.findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED))
.thenReturn(List.of(oldInstallable));

SkillSearchAppService.SearchResponse response = service.search(null, null, "newest", 0, 20, null, null);

assertEquals(1, response.items().size());
assertEquals("yanked-latest", response.items().getFirst().slug());
assertNull(response.items().getFirst().publishedVersion());
verify(skillVersionRepository, times(0))
.findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED);
}

@Test
void search_shouldNotFallbackToOlderPublishedVersionWhenLatestDownloadUnavailable() {
Skill skill = new Skill(1L, "not-ready", "owner-1", SkillVisibility.PUBLIC);
setField(skill, "id", 10L);
skill.setLatestVersionId(101L);

SkillVersion version = new SkillVersion(10L, "1.0.0", "owner-1");
setField(version, "id", 101L);
version.setStatus(SkillVersionStatus.PUBLISHED);
version.setDownloadReady(false);
SkillVersion oldInstallable = publishedVersion(10L, 100L, "0.9.0");

Namespace namespace = new Namespace("global", "Global", "owner-1");
setField(namespace, "id", 1L);
namespace.setStatus(NamespaceStatus.ACTIVE);

when(searchQueryService.search(any()))
.thenReturn(new SearchResult(List.of(10L), 1, 0, 20));
when(skillRepository.findByIdIn(List.of(10L))).thenReturn(List.of(skill));
when(namespaceRepository.findByIdIn(List.of(1L))).thenReturn(List.of(namespace));
when(skillVersionRepository.findByIdIn(List.of(101L))).thenReturn(List.of(version));
org.mockito.Mockito.lenient()
.when(skillVersionRepository.findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED))
.thenReturn(List.of(oldInstallable));

SkillSearchAppService.SearchResponse response = service.search(null, null, "newest", 0, 20, null, null);

assertEquals(1, response.items().size());
assertEquals("not-ready", response.items().getFirst().slug());
assertNull(response.items().getFirst().publishedVersion());
verify(skillVersionRepository, times(0))
.findBySkillIdInAndStatus(List.of(10L), SkillVersionStatus.PUBLISHED);
}

@Test
void search_shouldNormalizeAndPassLabelSlugs() {
when(searchQueryService.search(any()))
Expand Down Expand Up @@ -240,4 +337,12 @@ private void setField(Object target, String fieldName, Object value) {
throw new RuntimeException(e);
}
}

private SkillVersion publishedVersion(Long skillId, Long versionId, String versionNumber) {
SkillVersion version = new SkillVersion(skillId, versionNumber, "owner-1");
setField(version, "id", versionId);
version.setStatus(SkillVersionStatus.PUBLISHED);
version.setDownloadReady(true);
return version;
}
}
31 changes: 31 additions & 0 deletions ...r/skillhub-app/src/test/java/com/iflytek/skillhub/service/cli/CliSkillAppServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,37 @@ void search_mapsResultsToCliFormat() {
assertEquals(20, result.limit());
}

@Test
void search_filtersResultsWithoutInstallablePublishedVersion() {
var searchResponse = new SkillSearchAppService.SearchResponse(
List.of(
new SkillSummaryResponse(
1L, "draft-only", "Draft Only", "No installable version",
"PUBLIC", "ACTIVE", 0L, 0, BigDecimal.ZERO, 0,
"global", Instant.now(), false,
null, null, null, "NONE"
),
new SkillSummaryResponse(
2L, "ready", "Ready", "Installable",
"PUBLIC", "ACTIVE", 0L, 0, BigDecimal.ZERO, 0,
"global", Instant.now(), false,
new SkillLifecycleVersionResponse(2L, "1.0.0", "PUBLISHED"),
new SkillLifecycleVersionResponse(2L, "1.0.0", "PUBLISHED"),
null, "PUBLISHED"
)
),
2L, 0, 20
);
given(skillSearchAppService.search("demo", null, "newest", 0, 20, null, null))
.willReturn(searchResponse);

var result = service.search("demo", 20, null, null);

assertEquals(1, result.items().size());
assertEquals("ready", result.items().getFirst().slug());
assertEquals(1L, result.total());
}

@Test
void resolve_delegatesToQueryService() {
given(skillQueryService.resolveVersion("global", "demo", "2.0.0", null, null, "user-1", Map.of()))
Expand Down
18 changes: 18 additions & 0 deletions .../skillhub-domain/src/main/java/com/iflytek/skillhub/domain/skill/SkillInstallability.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.iflytek.skillhub.domain.skill;

/**
* Defines whether a skill version can be installed through public download
* paths. Storage object presence is checked later by the download service so
* fallback bundle behavior stays separate from domain publication state.
*/
public final class SkillInstallability {
private SkillInstallability() {
}

public static boolean isInstallableVersion(SkillVersion version) {
return version != null
&& version.getStatus() == SkillVersionStatus.PUBLISHED
&& version.isDownloadReady()
&& version.getYankedAt() == null;
}
}
29 changes: 13 additions & 16 deletions ...-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillDownloadService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import com.iflytek.skillhub.domain.namespace.Namespace;
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.shared.exception.DomainBadRequestException;
import com.iflytek.skillhub.domain.shared.exception.DomainForbiddenException;
import com.iflytek.skillhub.domain.skill.*;
Expand Down Expand Up @@ -163,7 +164,7 @@ public DownloadResult downloadReviewVersion(Skill skill, SkillVersion version) {

private DownloadResult downloadVersion(Skill skill, SkillVersion version) {
assertPublishedAccessible(skill);
assertDownloadableVersion(skill, version);
assertDownloadableVersion(version);
DownloadResult result = buildDownloadResult(skill, version);

// Only increment download count for PUBLISHED versions
Expand Down Expand Up @@ -281,12 +282,20 @@ private void assertCanDownload(Namespace namespace,
if (!visibilityChecker.canAccess(skill, currentUserId, userNsRoles)) {
throw new DomainForbiddenException("error.skill.access.denied", skill.getSlug());
}
if (namespace.getStatus() == NamespaceStatus.ARCHIVED
&& !isNamespaceMember(namespace.getId(), currentUserId, userNsRoles)) {
throw new DomainForbiddenException("error.namespace.archived", namespace.getSlug());
}
}

private boolean isAnonymousDownloadAllowed(Skill skill) {
return skill.getVisibility() == SkillVisibility.PUBLIC;
}

private boolean isNamespaceMember(Long namespaceId, String currentUserId, Map<Long, NamespaceRole> userNsRoles) {
return currentUserId != null && userNsRoles != null && userNsRoles.containsKey(namespaceId);
}

private Skill resolveVisibleSkill(Long namespaceId, String slug, String currentUserId) {
return skillSlugResolutionService.resolve(
namespaceId,
Expand All @@ -301,21 +310,9 @@ private void assertPublishedAccessible(Skill skill) {
}
}

/**
* Asserts that the version can be downloaded.
* - PUBLISHED: anyone with skill access can download
* - UPLOADED/PENDING_REVIEW: only skill owner can download
*/
private void assertDownloadableVersion(Skill skill, SkillVersion version) {
switch (version.getStatus()) {
case PUBLISHED -> {
// Anyone with skill access can download published versions
}
case UPLOADED, PENDING_REVIEW -> {
// Only owner can download UPLOADED/PENDING_REVIEW versions
// Note: This check is already done in assertCanDownload via visibilityChecker
}
default -> throw new DomainBadRequestException("error.skill.version.notDownloadable", version.getVersion());
private void assertDownloadableVersion(SkillVersion version) {
if (!SkillInstallability.isInstallableVersion(version)) {
throw new DomainBadRequestException("error.skill.version.notDownloadable", version.getVersion());
}
}
}
16 changes: 2 additions & 14 deletions .../main/java/com/iflytek/skillhub/domain/skill/service/SkillLifecycleProjectionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.iflytek.skillhub.domain.namespace.NamespaceRole;
import com.iflytek.skillhub.domain.skill.Skill;
import com.iflytek.skillhub.domain.skill.SkillInstallability;
import com.iflytek.skillhub.domain.skill.SkillVersion;
import com.iflytek.skillhub.domain.skill.SkillVersionRepository;
import com.iflytek.skillhub.domain.skill.SkillVersionStatus;
Expand Down Expand Up @@ -88,19 +89,10 @@ public Map<Long, Projection> projectPublishedSummaries(List<Skill> skills) {
.collect(Collectors.toMap(SkillVersion::getId, Function.identity()));

Map<Long, SkillVersion> publishedBySkillId = new java.util.HashMap<>();
List<Long> unresolvedSkillIds = new java.util.ArrayList<>();
for (Skill skill : skills) {
SkillVersion latestVersion = latestVersionsById.get(skill.getLatestVersionId());
if (latestVersion != null && latestVersion.getStatus() == SkillVersionStatus.PUBLISHED) {
if (SkillInstallability.isInstallableVersion(latestVersion)) {
publishedBySkillId.put(skill.getId(), latestVersion);
} else {
unresolvedSkillIds.add(skill.getId());
}
}

if (!unresolvedSkillIds.isEmpty()) {
for (SkillVersion version : skillVersionRepository.findBySkillIdInAndStatus(unresolvedSkillIds, SkillVersionStatus.PUBLISHED)) {
publishedBySkillId.merge(version.getSkillId(), version, this::newerVersion);
}
}

Expand Down Expand Up @@ -157,10 +149,6 @@ private Comparator<SkillVersion> versionComparator() {
.thenComparing(SkillVersion::getId, Comparator.nullsLast(Comparator.naturalOrder()));
}

private SkillVersion newerVersion(SkillVersion left, SkillVersion right) {
return versionComparator().compare(left, right) >= 0 ? left : right;
}

private VersionProjection toProjection(SkillVersion version) {
if (version == null) {
return null;
Expand Down
15 changes: 8 additions & 7 deletions ...hub-domain/src/main/java/com/iflytek/skillhub/domain/skill/service/SkillQueryService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,13 +487,7 @@ public Page<SkillVersion> listVersions(String namespaceSlug,
}

public boolean isDownloadAvailable(SkillVersion version) {
if (version == null) {
return false;
}
if (version.getStatus() != SkillVersionStatus.PUBLISHED) {
return false;
}
return version.isDownloadReady();
return SkillInstallability.isInstallableVersion(version);
}

public ReviewSkillSnapshotDTO getReviewSkillSnapshot(Long skillVersionId) {
Expand Down Expand Up @@ -565,6 +559,7 @@ public ResolvedVersionDTO resolveVersion(
Skill skill = resolveVisibleSkill(namespace.getId(), skillSlug, currentUserId);
assertPublishedAccessible(namespace, skill, currentUserId, userNsRoles);
SkillVersion resolved = resolveVersionEntity(skill, version, tag, hash);
assertInstallableVersion(resolved, resolved.getVersion());
String fingerprint = computeFingerprint(resolved);
Boolean matched = hash == null || hash.isBlank() ? null : Objects.equals(hash, fingerprint);

Expand Down Expand Up @@ -916,6 +911,12 @@ private void assertPublishedVersion(SkillVersion version, String versionStr) {
}
}

private void assertInstallableVersion(SkillVersion version, String versionStr) {
if (!SkillInstallability.isInstallableVersion(version)) {
throw new DomainBadRequestException("error.skill.version.notDownloadable", versionStr);
}
}

/**
* Checks whether the caller may preview a specific version's files and metadata.
* Published versions are visible to everyone; all other statuses are restricted
Expand Down
Loading
Loading