diff --git a/contract/openapi.yaml b/contract/openapi.yaml index f303dbc8..176ba747 100644 --- a/contract/openapi.yaml +++ b/contract/openapi.yaml @@ -4401,7 +4401,7 @@ info: description: "부산대학교 클라우드 플랫폼 Pickle의 REST API. 인증은 JWT Bearer, 오류 응답은 RFC 9457 problem+json(Problem\ \ 스키마)을 따릅니다." title: "Pickle API" - version: "0.31.0" + version: "0.31.1" openapi: "3.1.0" paths: /admin/announcements: diff --git a/src/main/java/kr/ac/pusan/pickle/admin/AdminInventoryService.java b/src/main/java/kr/ac/pusan/pickle/admin/AdminInventoryService.java index b0b56715..a75c3d7e 100644 --- a/src/main/java/kr/ac/pusan/pickle/admin/AdminInventoryService.java +++ b/src/main/java/kr/ac/pusan/pickle/admin/AdminInventoryService.java @@ -25,7 +25,6 @@ import kr.ac.pusan.pickle.inventory.dto.VmFlavorResponse; import kr.ac.pusan.pickle.security.AuthenticatedUser; import org.springframework.dao.DataIntegrityViolationException; -import org.springframework.data.domain.Sort; import org.springframework.http.HttpStatus; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; @@ -64,7 +63,7 @@ public AdminInventoryService(OsImageRepository osImageRepository, /** Contract {@code listAdminOsImages}: every OS image, retired revisions included. */ @Transactional(readOnly = true) public List listOsImages() { - return osImageRepository.findAll(Sort.by("id")).stream() + return osImageRepository.findAllInDisplayOrder().stream() .map(AdminOsImageResponse::from) .toList(); } @@ -104,7 +103,7 @@ public NodeSummaryResponse updateNodeStatus(AuthenticatedUser actor, long nodeId /** Contract {@code listAdminVmFlavors}: every preset, retired ones included. */ @Transactional(readOnly = true) public List listFlavors() { - return vmFlavorRepository.findAll(Sort.by("id")).stream() + return vmFlavorRepository.findAllInDisplayOrder().stream() .map(VmFlavorResponse::from) .toList(); } diff --git a/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java b/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java index bc75b433..b762864a 100644 --- a/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java +++ b/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java @@ -41,7 +41,7 @@ public class OpenApiConfig { /** Contract version served in {@code info.version}; bump on any contract change. */ - public static final String CONTRACT_VERSION = "0.31.0"; + public static final String CONTRACT_VERSION = "0.31.1"; /** Name of the bearer-JWT security scheme in the published spec. */ private static final String BEARER_SCHEME = "bearerAuth"; diff --git a/src/main/java/kr/ac/pusan/pickle/inventory/OsImageController.java b/src/main/java/kr/ac/pusan/pickle/inventory/OsImageController.java index 9c1a7617..e097fa30 100644 --- a/src/main/java/kr/ac/pusan/pickle/inventory/OsImageController.java +++ b/src/main/java/kr/ac/pusan/pickle/inventory/OsImageController.java @@ -28,7 +28,7 @@ public OsImageController(OsImageRepository osImageRepository) { @GetMapping @Transactional(readOnly = true) public List listOsImages() { - return osImageRepository.findByStatusOrderByIdAsc(CatalogStatus.ACTIVE).stream() + return osImageRepository.findByStatusInDisplayOrder(CatalogStatus.ACTIVE).stream() .map(OsImageResponse::from) .toList(); } diff --git a/src/main/java/kr/ac/pusan/pickle/inventory/OsImageRepository.java b/src/main/java/kr/ac/pusan/pickle/inventory/OsImageRepository.java index 165cc4b6..c17ece7f 100644 --- a/src/main/java/kr/ac/pusan/pickle/inventory/OsImageRepository.java +++ b/src/main/java/kr/ac/pusan/pickle/inventory/OsImageRepository.java @@ -2,10 +2,42 @@ import java.util.List; import org.springframework.data.jpa.repository.JpaRepository; +import org.springframework.data.jpa.repository.Query; +import org.springframework.data.repository.query.Param; public interface OsImageRepository extends JpaRepository { - List findByStatusOrderByIdAsc(CatalogStatus status); + /** + * Display order of the OS catalog: distribution alphabetically, then release + * ascending, then id. + * + *

Insertion order (the id) is an artifact of when an operator registered + * a row and says nothing to whoever reads the list, so it survives only as + * the tie-break between rows the first two keys cannot separate (per-node + * copies and superseded revisions of one release).

+ * + *

The release is sorted as the number sequence it is, not as text: + * {@code '9' > '10'} in text order, which would misplace Rocky the moment + * it enters the catalog. The {@code chk_os_images_os_version} check + * constraint (V62) restricts the column to dotted digits, which is what + * makes this cast total for every row the table can hold.

+ */ + String DISPLAY_ORDER = + " order by os_family asc, string_to_array(os_version, '.')::int[] asc, id asc"; + + @Query(value = "select * from os_images where status = cast(:status as catalog_status)" + + DISPLAY_ORDER, nativeQuery = true) + List findByStatusInDisplayOrder(@Param("status") String status); + + default List findByStatusInDisplayOrder(CatalogStatus status) { + return findByStatusInDisplayOrder(status.name()); + } + + @Query(value = "select * from os_images" + DISPLAY_ORDER, nativeQuery = true) + List findAllInDisplayOrder(); + + /** Rows of one status with no display intent — order carries no meaning here. */ + List findByStatus(CatalogStatus status); /** * Whether a node hosts a usable copy of an OS image. Image rows are diff --git a/src/main/java/kr/ac/pusan/pickle/inventory/VmFlavorController.java b/src/main/java/kr/ac/pusan/pickle/inventory/VmFlavorController.java index 996d0529..2bca89c3 100644 --- a/src/main/java/kr/ac/pusan/pickle/inventory/VmFlavorController.java +++ b/src/main/java/kr/ac/pusan/pickle/inventory/VmFlavorController.java @@ -21,7 +21,7 @@ public VmFlavorController(VmFlavorRepository vmFlavorRepository) { @GetMapping @Transactional(readOnly = true) public List listVmFlavors() { - return vmFlavorRepository.findByStatusOrderByIdAsc(CatalogStatus.ACTIVE).stream() + return vmFlavorRepository.findByStatusInDisplayOrder(CatalogStatus.ACTIVE).stream() .map(VmFlavorResponse::from) .toList(); } diff --git a/src/main/java/kr/ac/pusan/pickle/inventory/VmFlavorRepository.java b/src/main/java/kr/ac/pusan/pickle/inventory/VmFlavorRepository.java index d9cc8bf1..5e6995fa 100644 --- a/src/main/java/kr/ac/pusan/pickle/inventory/VmFlavorRepository.java +++ b/src/main/java/kr/ac/pusan/pickle/inventory/VmFlavorRepository.java @@ -1,11 +1,35 @@ package kr.ac.pusan.pickle.inventory; import java.util.List; +import org.springframework.data.domain.Sort; import org.springframework.data.jpa.repository.JpaRepository; public interface VmFlavorRepository extends JpaRepository { - List findByStatusOrderByIdAsc(CatalogStatus status); + /** + * Display order of the spec presets: smallest first, on the three numbers + * that make a preset what it is, with the id as the last tie-break. + * + *

The presets have no family axis to group by; size is the axis the + * student is choosing along, so the list reads as a scale instead of as the + * order an operator happened to create them in. Ascending also puts the + * modest preset in front of the generous one, which is the direction the + * quota policy wants a hesitant requester nudged.

+ */ + Sort DISPLAY_ORDER = Sort.by("vcpu", "memoryMb", "diskGb", "id"); + + default List findByStatusInDisplayOrder(CatalogStatus status) { + return findByStatus(status, DISPLAY_ORDER); + } + + default List findAllInDisplayOrder() { + return findAll(DISPLAY_ORDER); + } + + List findByStatus(CatalogStatus status, Sort sort); + + /** Rows of one status with no display intent — order carries no meaning here. */ + List findByStatus(CatalogStatus status); boolean existsByName(String name); } diff --git a/src/main/java/kr/ac/pusan/pickle/provisioning/NodePlacementService.java b/src/main/java/kr/ac/pusan/pickle/provisioning/NodePlacementService.java index 9f4b4bfb..38799ead 100644 --- a/src/main/java/kr/ac/pusan/pickle/provisioning/NodePlacementService.java +++ b/src/main/java/kr/ac/pusan/pickle/provisioning/NodePlacementService.java @@ -77,7 +77,7 @@ public Node place(Vm vm, OsImage image, Long forcedNodeId) { // Nodes hosting an ACTIVE image of the same name (image rows are // per-node; a multi-node cluster clones the image under one name). Set imageNodeIds = imageRepository - .findByStatusOrderByIdAsc(CatalogStatus.ACTIVE).stream() + .findByStatus(CatalogStatus.ACTIVE).stream() .filter(candidate -> candidate.getName().equals(image.getName())) .map(OsImage::getNodeId) .collect(Collectors.toSet()); diff --git a/src/test/java/kr/ac/pusan/pickle/admin/AdminInventoryTest.java b/src/test/java/kr/ac/pusan/pickle/admin/AdminInventoryTest.java index 2652256a..4d294c72 100644 --- a/src/test/java/kr/ac/pusan/pickle/admin/AdminInventoryTest.java +++ b/src/test/java/kr/ac/pusan/pickle/admin/AdminInventoryTest.java @@ -6,7 +6,9 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; +import com.jayway.jsonpath.JsonPath; import java.time.Instant; +import java.util.List; import java.util.UUID; import kr.ac.pusan.pickle.security.JwtService; import kr.ac.pusan.pickle.support.EmbeddedPostgresConfig; @@ -101,6 +103,31 @@ void adminOsImageListShowsRetiredRevisionsThePublicListHides() throws Exception .andExpect(jsonPath(byId(imageId)).doesNotExist()); } + /** + * The admin catalog is the same catalog the wizard shows, so it is read in + * the same order — the admin decides what students see and should not have + * to translate between two orderings. The retired revision sorts by its own + * family and release like any other row; status is a column, not a section. + */ + @Test + void adminOsImageListFollowsTheWizardDisplayOrder() throws Exception { + long nodeId = jdbcTemplate.queryForObject("select min(id) from nodes", Long.class); + // registered newest-release-first and retired, so id order and text + // order on the release string would both put Rocky 10 ahead of Rocky 9 + String rocky10 = insertImage(nodeId, "rocky", "10", "DISABLED"); + String rocky9 = insertImage(nodeId, "rocky", "9", "ACTIVE"); + String debian13 = insertImage(nodeId, "debian", "13", "ACTIVE"); + + String body = mockMvc.perform(get("/api/v1/admin/os-images") + .header("Authorization", "Bearer " + sysManagerToken)) + .andExpect(status().isOk()) + .andReturn().getResponse().getContentAsString(); + List names = JsonPath.read(body, "$[*].name"); + + // imageName is the setUp row, an ubuntu 24.04 — last of the four families + assertThat(names).containsSubsequence(debian13, rocky9, rocky10, imageName); + } + @Test void osImageToggleIsSysAdminOnlyAndAuditsRealTransitionsOnly() throws Exception { mockMvc.perform(patch("/api/v1/admin/os-images/{id}", imageId) @@ -199,6 +226,16 @@ private static String byId(long id) { return "$[?(@.id == %d)]".formatted(id); } + private String insertImage(long nodeId, String family, String version, String status) { + String name = family + "-" + version + "-" + UUID.randomUUID().toString().substring(0, 8); + jdbcTemplate.update(""" + insert into os_images (name, display_name, os_family, os_version, ssh_username, + proxmox_vmid, node_id, version, min_disk_gb, status) + values (?, '정렬 확인용', ?, ?, ?, 990002, ?, 1, 10, cast(? as catalog_status)) + """, name, family, version, family, nodeId, status); + return name; + } + private long auditCount(String action, long targetId) { return jdbcTemplate.queryForObject( "select count(*) from audit_logs where action = ? and target_id = ?", diff --git a/src/test/java/kr/ac/pusan/pickle/admin/AdminVmFlavorTest.java b/src/test/java/kr/ac/pusan/pickle/admin/AdminVmFlavorTest.java index f791f61f..03225655 100644 --- a/src/test/java/kr/ac/pusan/pickle/admin/AdminVmFlavorTest.java +++ b/src/test/java/kr/ac/pusan/pickle/admin/AdminVmFlavorTest.java @@ -98,13 +98,16 @@ void adminFlavorListShowsRetiredPresetsThePublicListHides() throws Exception { } @Test - void publicFlavorListIsActiveOnlyAndOrderedById() throws Exception { - // every authenticated role reads the wizard list; the V58 seed leads it + void publicFlavorListIsActiveOnlyAndOrderedBySize() throws Exception { + // every authenticated role reads the wizard list, smallest preset first. + // The row this class creates carries basic's exact spec, so it lands on + // the id tie-break right behind basic rather than at the end of the list. mockMvc.perform(get("/api/v1/vm-flavors").header("Authorization", "Bearer " + userToken)) .andExpect(status().isOk()) .andExpect(jsonPath("$[0].name").value("small")) .andExpect(jsonPath("$[1].name").value("basic")) - .andExpect(jsonPath("$[2].name").value("large")) + .andExpect(jsonPath("$[2].name").value(flavorName)) + .andExpect(jsonPath("$[3].name").value("large")) .andExpect(jsonPath(byId(flavorId) + ".status").value("ACTIVE")); jdbcTemplate.update("update vm_flavors set status = 'DISABLED'::catalog_status where id = ?", diff --git a/src/test/java/kr/ac/pusan/pickle/reference/ReferenceDataTest.java b/src/test/java/kr/ac/pusan/pickle/reference/ReferenceDataTest.java index 7e030ee6..732fc0a5 100644 --- a/src/test/java/kr/ac/pusan/pickle/reference/ReferenceDataTest.java +++ b/src/test/java/kr/ac/pusan/pickle/reference/ReferenceDataTest.java @@ -126,7 +126,7 @@ void managerTierSeesHiddenOrgs() throws Exception { @Test void listsOnlyActiveOsImagesAsAPureOsCatalog() throws Exception { // a DISABLED version row must not surface in the wizard list - if (osImageRepository.findByStatusOrderByIdAsc(CatalogStatus.DISABLED).isEmpty()) { + if (osImageRepository.findByStatus(CatalogStatus.DISABLED).isEmpty()) { Long nodeId = osImageRepository.findAll().getFirst().getNodeId(); osImageRepository.save(new OsImage("ubuntu-22.04", "Ubuntu 22.04 LTS (구버전)", "ubuntu", "22.04", "ubuntu", 1001, nodeId, 1, 10, @@ -155,10 +155,66 @@ void listsOnlyActiveOsImagesAsAPureOsCatalog() throws Exception { .andExpect(jsonPath("$[0].defaultDiskGb").doesNotExist()); } + /** + * The wizard's OS axis reads as a rule, not as the order an operator + * happened to register rows in: distribution alphabetically, release + * ascending as a number. The rows below are inserted in an order that + * defeats both of the ways this can be got wrong — Rocky 10 lands before + * Rocky 9 (so id order would show 10 first) and the release strings sort + * the same way as text ('10' < '9'), so only numeric release order puts 9 + * ahead of 10. The older Ubuntu is registered last for the same reason. + */ + @Test + void osCatalogIsOrderedByFamilyThenReleaseAsNumbers() throws Exception { + Long nodeId = osImageRepository.findAll().getFirst().getNodeId(); + List added = osImageRepository.saveAll(List.of( + new OsImage("rocky-10", "Rocky Linux 10", "rocky", "10", "rocky", + 1901, nodeId, 1, 10, CatalogStatus.ACTIVE, "정렬 확인용"), + new OsImage("rocky-9", "Rocky Linux 9", "rocky", "9", "rocky", + 1902, nodeId, 1, 10, CatalogStatus.ACTIVE, "정렬 확인용"), + new OsImage("debian-13", "Debian 13", "debian", "13", "debian", + 1903, nodeId, 1, 10, CatalogStatus.ACTIVE, "정렬 확인용"), + new OsImage("ubuntu-20.04", "Ubuntu 20.04 LTS", "ubuntu", "20.04", "ubuntu", + 1904, nodeId, 1, 10, CatalogStatus.ACTIVE, "정렬 확인용"))); + try { + mockMvc.perform(get("/api/v1/os-images") + .header("Authorization", "Bearer " + accessToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$[*].name").value(org.hamcrest.Matchers.contains( + "debian-13", "rocky-9", "rocky-10", + "ubuntu-20.04", "ubuntu-24.04"))); + } finally { + osImageRepository.deleteAll(added); + } + } + + /** + * The spec axis has no family to group by, so it reads as a scale: the + * three preset numbers ascending. The two rows below are registered after + * the presets and must still bracket them. + */ + @Test + void flavorsAreOrderedBySizeNotByRegistration() throws Exception { + List added = vmFlavorRepository.saveAll(List.of( + new VmFlavor("ref-order-huge", "정렬 확인용 특대형", 8, 16384, 80, + CatalogStatus.ACTIVE, "정렬 확인용"), + new VmFlavor("ref-order-tiny", "정렬 확인용 초소형", 1, 512, 5, + CatalogStatus.ACTIVE, "정렬 확인용"))); + try { + mockMvc.perform(get("/api/v1/vm-flavors") + .header("Authorization", "Bearer " + accessToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$[*].name").value(org.hamcrest.Matchers.contains( + "ref-order-tiny", "small", "basic", "large", "ref-order-huge"))); + } finally { + vmFlavorRepository.deleteAll(added); + } + } + @Test void listsOnlyActiveFlavorsWithTheirSpecs() throws Exception { // a retired preset must not surface in the wizard list - if (vmFlavorRepository.findByStatusOrderByIdAsc(CatalogStatus.DISABLED).isEmpty()) { + if (vmFlavorRepository.findByStatus(CatalogStatus.DISABLED).isEmpty()) { vmFlavorRepository.save(new VmFlavor("ref-retired", "은퇴 프리셋", 8, 16384, 80, CatalogStatus.DISABLED, null)); } diff --git a/src/test/java/kr/ac/pusan/pickle/vmrequest/VmRequestTest.java b/src/test/java/kr/ac/pusan/pickle/vmrequest/VmRequestTest.java index 82ca558b..a6abec04 100644 --- a/src/test/java/kr/ac/pusan/pickle/vmrequest/VmRequestTest.java +++ b/src/test/java/kr/ac/pusan/pickle/vmrequest/VmRequestTest.java @@ -296,7 +296,7 @@ void retiredFlavorIsRejectedAndBothAxesReportIndependently() throws Exception { @Test void emptyOsCatalogListsNothingAndRefusesEverySubmission() throws Exception { long groupId = createTeam(requesterToken, "vmr-empty-x1"); - List active = imageRepository.findByStatusOrderByIdAsc(CatalogStatus.ACTIVE); + List active = imageRepository.findByStatus(CatalogStatus.ACTIVE); assertThat(active).isNotEmpty(); // Catalog rows are shared state across test classes on this context — // the ACTIVE set is restored in the finally block below.