Apply Vishesh's suggestions

Author: weizhouapache

engine/orchestration/src/main/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestrator.java

@@ -479,7 +479,7 @@ public void setDhcpProviders(final List<DhcpServiceProvider> dhcpProviders) {
      * Spring bean list free from {@code NetworkExtensionElement} and allows
      * dynamic discovery of extensions without a restart.</p>
      */
-    private List<NetworkElement> getNetworkElementsIncludingExtensions() {
+    protected List<NetworkElement> getNetworkElementsIncludingExtensions() {
         List<Extension> extensions = extensionHelper.listExtensionsByType(Extension.Type.NetworkOrchestrator);
         if (CollectionUtils.isEmpty(extensions)) {
             return networkElements;

engine/orchestration/src/test/java/org/apache/cloudstack/engine/orchestration/NetworkOrchestratorTest.java

@@ -398,7 +398,7 @@ private void configureTestConfigureNicProfileBasedOnRequestedIpTests(NicProfile
             when(testOrchestrator._ipAddressDao.findByIpAndSourceNetworkId(Mockito.anyLong(), Mockito.anyString())).thenReturn(ipVoSpy);
         }
 
-        VlanVO vlanSpy = Mockito.spy(new VlanVO(Vlan.VlanType.DirectAttached, "vlanTag", vlanGateway, vlanNetmask, 0l, "192.168.100.100 - 192.168.100.200", 0l, new Long(0l),
+        VlanVO vlanSpy = Mockito.spy(new VlanVO(Vlan.VlanType.DirectAttached, "vlanTag", vlanGateway, vlanNetmask, 0l, "192.168.100.100 - 192.168.100.200", 0l, 0l,
                 "ip6Gateway", "ip6Cidr", "ip6Range"));
 
         Mockito.doReturn(0l).when(vlanSpy).getId();
@@ -1034,10 +1034,8 @@ public void getNetworkElementsIncludingExtensionsReturnsBaseListWhenNoExtensions
         List<NetworkElement> elements = new ArrayList<>(List.of(dhcpProvider));
         testOrchestrator.networkElements = elements;
 
-        @SuppressWarnings("unchecked")
-        List<NetworkElement> result =
-                (List<NetworkElement>) ReflectionTestUtils
-                        .invokeMethod(testOrchestrator, "getNetworkElementsIncludingExtensions");
+        List<NetworkElement> result = testOrchestrator.getNetworkElementsIncludingExtensions();
+
         assertNotNull(result);
         assertEquals(elements.size(), result.size());
     }
@@ -1055,10 +1053,8 @@ public void getNetworkElementsIncludingExtensionsAddsExtensionElements() {
         DhcpServiceProvider dhcpProvider = mock(DhcpServiceProvider.class);
         testOrchestrator.networkElements = new ArrayList<>(List.of(dhcpProvider));
 
-        @SuppressWarnings("unchecked")
-        List<NetworkElement> result =
-                (List<NetworkElement>) ReflectionTestUtils
-                        .invokeMethod(testOrchestrator, "getNetworkElementsIncludingExtensions");
+        List<NetworkElement> result = testOrchestrator.getNetworkElementsIncludingExtensions();
+
         assertNotNull(result);
         assertEquals(2, result.size());
         assertTrue(result.contains(extElement));
@@ -1072,10 +1068,8 @@ public void getNetworkElementsIncludingExtensionsReturnsBaseListWhenExtensionHel
         DhcpServiceProvider dhcpProvider = mock(DhcpServiceProvider.class);
         testOrchestrator.networkElements = new ArrayList<>(List.of(dhcpProvider));
 
-        @SuppressWarnings("unchecked")
-        List<NetworkElement> result =
-                (List<NetworkElement>) ReflectionTestUtils
-                        .invokeMethod(testOrchestrator, "getNetworkElementsIncludingExtensions");
+        List<NetworkElement> result = testOrchestrator.getNetworkElementsIncludingExtensions();
+
         assertNotNull(result);
         assertEquals(1, result.size());
     }

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

@@ -155,4 +155,4 @@ CALL `cloud`.`IDEMPOTENT_CHANGE_COLUMN`('cloud.extension_details', 'value', 'val
 CALL `cloud`.`IDEMPOTENT_CHANGE_COLUMN`('cloud.extension_resource_map_details', 'value', 'value', 'VARCHAR(4096)');
 
 -- Add CustomAction service support to physical_network_service_providers
-CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.physical_network_service_providers', 'custom_action_service_provided', 'tinyint(1) unsigned NOT NULL DEFAULT 0 COMMENT "Is Custom Action service provided"');
+CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.physical_network_service_providers', 'custom_action_service_provided', 'tinyint(1) unsigned NOT NULL DEFAULT 0 COMMENT "Is Custom Action service provided" AFTER `networkacl_service_provided`');

framework/extensions/src/main/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImpl.java

@@ -996,7 +996,7 @@ public Extension registerExtensionWithResource(RegisterExtensionCmd cmd) {
         if (extension == null) {
             throw new InvalidParameterValueException("Invalid extension specified");
         }
-        ExtensionResourceMap.ResourceType resType = ExtensionResourceMap.ResourceType.valueOf(resourceType);
+        ExtensionResourceMap.ResourceType resType = EnumUtils.getEnum(ExtensionResourceMap.ResourceType.class, resourceType);
         if (ExtensionResourceMap.ResourceType.PhysicalNetwork.equals(resType)) {
             PhysicalNetworkVO physicalNetwork = physicalNetworkDao.findByUuid(resourceId);
             if (physicalNetwork == null) {
@@ -1035,16 +1035,10 @@ public Extension updateRegisteredExtensionWithResource(UpdateRegisteredExtension
             throw new InvalidParameterValueException("Invalid extension specified");
         }
 
-        ExtensionResourceMap.ResourceType resType = ExtensionResourceMap.ResourceType.valueOf(resourceType);
+        ExtensionResourceMap.ResourceType resType = EnumUtils.getEnum(ExtensionResourceMap.ResourceType.class, resourceType);
         long resolvedResourceId;
         if (ExtensionResourceMap.ResourceType.PhysicalNetwork.equals(resType)) {
             PhysicalNetworkVO physicalNetwork = physicalNetworkDao.findByUuid(resourceId);
-            if (physicalNetwork == null) {
-                try {
-                    physicalNetwork = physicalNetworkDao.findById(Long.parseLong(resourceId));
-                } catch (NumberFormatException ignored) {
-                }
-            }
             if (physicalNetwork == null) {
                 throw new InvalidParameterValueException("Invalid physical network ID specified");
             }
@@ -1378,7 +1372,7 @@ public Extension unregisterExtensionWithResource(UnregisterExtensionCmd cmd) {
                     String.format("Currently only [%s] can be used to unregister an extension",
                             EnumSet.allOf(ExtensionResourceMap.ResourceType.class)));
         }
-        ExtensionResourceMap.ResourceType resType = ExtensionResourceMap.ResourceType.valueOf(resourceType);
+        ExtensionResourceMap.ResourceType resType = EnumUtils.getEnum(ExtensionResourceMap.ResourceType.class, resourceType);
         if (ExtensionResourceMap.ResourceType.PhysicalNetwork.equals(resType)) {
             unregisterExtensionWithPhysicalNetwork(resourceId, extensionId);
         } else {

framework/extensions/src/test/java/org/apache/cloudstack/framework/extensions/manager/ExtensionsManagerImplTest.java

@@ -156,7 +156,7 @@
 import com.cloud.vm.VmDetailConstants;
 import com.cloud.vm.dao.VMInstanceDao;
 
-@RunWith(MockitoJUnitRunner.Silent.class)
+@RunWith(MockitoJUnitRunner.class)
 public class ExtensionsManagerImplTest {
 
     @Spy

server/src/main/java/com/cloud/network/NetworkModelImpl.java

@@ -240,7 +240,7 @@ public void setNetworkElements(List<NetworkElement> networkElements) {
     @Inject
     ExtensionHelper extensionHelper;
     @Inject
-    private NetworkExtensionElement networkExtensionElement;
+    NetworkExtensionElement networkExtensionElement;
 
 
     private final HashMap<String, NetworkOfferingVO> _systemNetworks = new HashMap<String, NetworkOfferingVO>(5);

server/src/test/java/com/cloud/network/NetworkModelImplTest.java

@@ -116,10 +116,10 @@ public void setUp() {
         networkModel._networkOfferingDao = networkOfferingDao;
         networkModel._ntwkSrvcDao = networkServiceMapDao;
         networkModel._ntwkOfferingSrvcDao = networkOfferingServiceMapDao;
-        ReflectionTestUtils.setField(networkModel, "extensionHelper", extensionHelper);
-        ReflectionTestUtils.setField(networkModel, "networkExtensionElement", networkExtensionElement);
-        ReflectionTestUtils.setField(networkModel, "_physicalNetworkDao", physicalNetworkDao);
-        ReflectionTestUtils.setField(networkModel, "_pNSPDao", physicalNetworkServiceProviderDao);
+        networkModel.extensionHelper = extensionHelper;
+        networkModel.networkExtensionElement = networkExtensionElement;
+        networkModel._physicalNetworkDao = physicalNetworkDao;
+        networkModel._pNSPDao = physicalNetworkServiceProviderDao;
         Mockito.lenient().when(extensionHelper.isNetworkExtensionProvider(Mockito.anyString())).thenReturn(false);
     }
 

setup/db/create-schema.sql

@@ -2120,7 +2120,6 @@ CREATE TABLE `cloud`.`physical_network_service_providers` (
   `user_data_service_provided` tinyint(1) unsigned NOT NULL DEFAULT 0 COMMENT 'Is UserData service provided',
   `security_group_service_provided` tinyint(1) unsigned NOT NULL DEFAULT 0 COMMENT 'Is SG service provided',
   `networkacl_service_provided` tinyint(1) unsigned NOT NULL DEFAULT 0 COMMENT 'Is Network ACL service provided',
-  `custom_action_service_provided` tinyint(1) unsigned NOT NULL DEFAULT 0 COMMENT 'Is Custom Action service provided',
   `removed` datetime COMMENT 'date removed if not null',
   PRIMARY KEY (`id`),
   CONSTRAINT `fk_pnetwork_service_providers__physical_network_id` FOREIGN KEY (`physical_network_id`) REFERENCES `physical_network`(`id`) ON DELETE CASCADE,