Apply review feedback on service offering categories

Author: Landry JUGE

api/src/main/java/org/apache/cloudstack/api/ApiConstants.java

@@ -551,7 +551,6 @@ public class ApiConstants {
     public static final String SERIAL = "serial";
     public static final String SERVICE_IP = "serviceip";
     public static final String SERVICE_OFFERING_CATEGORY_ID = "categoryid";
-    public static final String SERVICE_OFFERING_CATEGORY_NAME = "categoryname";
     public static final String SERVICE_OFFERING_ID = "serviceofferingid";
     public static final String SERVICE_OFFERING_NAME = "serviceofferingname";
     public static final String SESSIONKEY = "sessionkey";

api/src/main/java/org/apache/cloudstack/api/command/admin/offering/DeleteServiceOfferingCategoryCmd.java

@@ -30,7 +30,7 @@
 @APICommand(name = "deleteServiceOfferingCategory",
         description = "Deletes a service offering category.",
         responseObject = SuccessResponse.class,
-        since = "4.23.0",
+        since = "24.0",
         requestHasSensitiveInfo = false,
         responseHasSensitiveInfo = false)
 public class DeleteServiceOfferingCategoryCmd extends BaseCmd {

api/src/main/java/org/apache/cloudstack/api/command/admin/offering/ListServiceOfferingCategoriesCmd.java

@@ -26,7 +26,7 @@
 @APICommand(name = "listServiceOfferingCategories",
         description = "Lists service offering categories.",
         responseObject = ServiceOfferingCategoryResponse.class,
-        since = "4.23.0",
+        since = "24.0",
         requestHasSensitiveInfo = false,
         responseHasSensitiveInfo = false)
 public class ListServiceOfferingCategoriesCmd extends BaseListCmd {

api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCategoryCmd.java

@@ -30,7 +30,7 @@
 @APICommand(name = "updateServiceOfferingCategory",
         description = "Updates a service offering category",
         responseObject = ServiceOfferingCategoryResponse.class,
-        since = "4.23.0",
+        since = "24.0",
         requestHasSensitiveInfo = false,
         responseHasSensitiveInfo = false)
 public class UpdateServiceOfferingCategoryCmd extends BaseCmd {

api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateServiceOfferingCmd.java

@@ -114,7 +114,7 @@ public class UpdateServiceOfferingCmd extends BaseCmd implements DomainAndZoneId
             entityType = ServiceOfferingCategoryResponse.class,
             required = false,
             description = "the ID of the service offering category to associate",
-            since = "4.23.0")
+            since = "24.0")
     private Long categoryId;
 
     /////////////////////////////////////////////////////

api/src/main/java/org/apache/cloudstack/api/command/user/offering/ListServiceOfferingsCmd.java

@@ -129,7 +129,7 @@ public class ListServiceOfferingsCmd extends BaseListProjectAndAccountResourcesC
             type = CommandType.UUID,
             entityType = ServiceOfferingCategoryResponse.class,
             description = "the ID of the service offering category",
-            since = "4.23.0")
+            since = "24.0")
     private Long categoryId;
 
     /////////////////////////////////////////////////////

api/src/main/java/org/apache/cloudstack/api/response/ServiceOfferingResponse.java

@@ -287,11 +287,11 @@ public class ServiceOfferingResponse extends BaseResponseWithAnnotations {
     private String leaseExpiryAction;
 
     @SerializedName("categoryid")
-    @Param(description = "the ID of the service offering category", since = "4.23.0")
+    @Param(description = "the ID of the service offering category", since = "24.0")
     private String categoryId;
 
     @SerializedName("category")
-    @Param(description = "the name of the service offering category", since = "4.23.0")
+    @Param(description = "the name of the service offering category", since = "24.0")
     private String categoryName;
 
     public ServiceOfferingResponse() {

engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql

@@ -29,9 +29,10 @@ CREATE TABLE IF NOT EXISTS `cloud`.`service_offering_category` (
  CONSTRAINT `uc_service_offering_category__name` UNIQUE (`name`)
 ) ENGINE=InnoDB AUTO_INCREMENT=1 DEFAULT CHARSET=utf8;
 
-INSERT INTO `cloud`.`service_offering_category` (id, name, uuid) VALUES (1, 'Default', UUID());
+INSERT IGNORE INTO `cloud`.`service_offering_category` (id, name, uuid) VALUES (1, 'Default', UUID());
 
-ALTER TABLE `cloud`.`service_offering` ADD COLUMN `category_id` bigint unsigned NOT NULL DEFAULT 1;
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.service_offering', 'category_id', 'bigint unsigned NOT NULL DEFAULT 1');
+CALL `cloud`.`IDEMPOTENT_DROP_FOREIGN_KEY`('cloud.service_offering', 'fk_service_offering__category_id');
 ALTER TABLE `cloud`.`service_offering` ADD CONSTRAINT `fk_service_offering__category_id` FOREIGN KEY (`category_id`) REFERENCES `cloud`.`service_offering_category` (`id`) ON DELETE RESTRICT ON UPDATE CASCADE;
 
 CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.oauth_provider', 'domain_id', 'bigint unsigned DEFAULT NULL COMMENT "NULL for global provider, domain ID for domain-specific" AFTER `redirect_uri`');

server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -3976,6 +3976,13 @@ public ServiceOffering cloneServiceOffering(final CloneServiceOfferingCmd cmd) {
 
         final Long diskOfferingId = getOrDefault(cmd.getDiskOfferingId(), sourceOffering.getDiskOfferingId());
 
+        // validate the overridden category id, otherwise inherit the source offering's category
+        final Long cmdCategoryId = cmd.getCategoryId();
+        if (cmdCategoryId != null && _serviceOfferingCategoryDao.findById(cmdCategoryId) == null) {
+            throw new InvalidParameterValueException("Please specify a valid service offering category id");
+        }
+        final Long categoryId = getOrDefault(cmdCategoryId, sourceOffering.getCategoryId());
+
         return createServiceOffering(userId, systemUse, vmType,
                 name, cpuNumber, memory, cpuSpeed, displayText, provisioningType, localStorageRequired,
                 offerHa, limitCpuUse, isVolatile, tags, domainIds, zoneIds, hostTag, networkRate,
@@ -3987,7 +3994,7 @@ public ServiceOffering cloneServiceOffering(final CloneServiceOfferingCmd cmd) {
                 diskParams.iopsWriteRate, diskParams.iopsWriteRateMax, diskParams.iopsWriteRateMaxLength,
                 diskParams.hypervisorSnapshotReserve, diskParams.cacheMode, customParams.storagePolicy, dynamicScalingEnabled,
                 diskOfferingId, diskOfferingStrictness, isCustomized, encryptRoot,
-                vgpuProfileId, finalGpuCount, gpuDisplay, purgeResources, leaseParams.leaseDuration, leaseParams.leaseExpiryAction, sourceOffering.getCategoryId());
+                vgpuProfileId, finalGpuCount, gpuDisplay, purgeResources, leaseParams.leaseDuration, leaseParams.leaseExpiryAction, categoryId);
     }
 
     private ServiceOfferingVO getAndValidateSourceOffering(Long sourceOfferingId) {

server/src/test/java/com/cloud/configuration/ConfigurationManagerCloneIntegrationTest.java

@@ -30,6 +30,8 @@
 import com.cloud.offerings.dao.NetworkOfferingDao;
 import com.cloud.offerings.dao.NetworkOfferingServiceMapDao;
 import com.cloud.service.ServiceOfferingVO;
+import com.cloud.service.ServiceOfferingCategoryVO;
+import com.cloud.service.dao.ServiceOfferingCategoryDao;
 import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.service.dao.ServiceOfferingDetailsDao;
 import com.cloud.storage.DiskOfferingVO;
@@ -95,6 +97,9 @@ public class ConfigurationManagerCloneIntegrationTest {
     @Mock
     private ServiceOfferingDetailsDao serviceOfferingDetailsDao;
 
+    @Mock
+    private ServiceOfferingCategoryDao serviceOfferingCategoryDao;
+
     @Mock
     private DiskOfferingDao diskOfferingDao;
 
@@ -167,6 +172,9 @@ public void setUp() {
             anyInt(), anyString(), any(), anyLong(), anyBoolean(), anyBoolean());
 
 
+        Mockito.lenient().when(serviceOfferingCategoryDao.findById(anyLong()))
+                .thenReturn(mock(ServiceOfferingCategoryVO.class));
+
         // User/Account DAO stubs used by createDiskOffering
         Mockito.lenient().when(userDao.findById(anyLong())).thenReturn(userVO);
         Mockito.lenient().when(userVO.getAccountId()).thenReturn(1L);
@@ -341,6 +349,7 @@ public void testCloneServiceOfferingOverridesProvidedParameters() {
         Assert.assertEquals("Cloned offering should override HA", Boolean.FALSE, result.isOfferHA());
     }
 
+
     @Test(expected = InvalidParameterValueException.class)
     public void testCloneDiskOfferingFailsWhenSourceNotFound() {
         CloneDiskOfferingCmd cmd = mock(CloneDiskOfferingCmd.class);