From 9e5a733a348c00afa340a8a33daa161c829caf62 Mon Sep 17 00:00:00 2001 From: alena Date: Thu, 3 Nov 2011 10:47:59 -0700 Subject: [PATCH] Fixed NPE in updateNetwork - userCaller wasn't passed in to restartNetworkElements call --- .../cloud/api/commands/UpdateNetworkCmd.java | 6 ++++- api/src/com/cloud/network/NetworkService.java | 3 ++- .../com/cloud/network/NetworkManagerImpl.java | 26 ++++++++++++------- .../cloud/network/MockNetworkManagerImpl.java | 2 +- 4 files changed, 24 insertions(+), 13 deletions(-) diff --git a/api/src/com/cloud/api/commands/UpdateNetworkCmd.java b/api/src/com/cloud/api/commands/UpdateNetworkCmd.java index 198ad684a57..2adf841ff65 100644 --- a/api/src/com/cloud/api/commands/UpdateNetworkCmd.java +++ b/api/src/com/cloud/api/commands/UpdateNetworkCmd.java @@ -32,6 +32,8 @@ import com.cloud.exception.ConcurrentOperationException; import com.cloud.exception.InsufficientCapacityException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.network.Network; +import com.cloud.user.Account; +import com.cloud.user.User; import com.cloud.user.UserContext; @Implementation(description="Updates a network", responseObject=NetworkResponse.class) @@ -103,7 +105,9 @@ public class UpdateNetworkCmd extends BaseAsyncCmd { @Override public void execute() throws InsufficientCapacityException, ConcurrentOperationException{ - Network result = _networkService.updateNetwork(getId(), getNetworkName(), getDisplayText(), UserContext.current().getCaller(), getNetworkDomain(), getNetworkOfferingId()); + User callerUser = _accountService.getActiveUser(UserContext.current().getCallerUserId()); + Account callerAccount = _accountService.getActiveAccountById(callerUser.getAccountId()); + Network result = _networkService.updateNetwork(getId(), getNetworkName(), getDisplayText(), callerAccount, callerUser, getNetworkDomain(), getNetworkOfferingId()); if (result != null) { NetworkResponse response = _responseGenerator.createNetworkResponse(result); response.setResponseName(getCommandName()); diff --git a/api/src/com/cloud/network/NetworkService.java b/api/src/com/cloud/network/NetworkService.java index 0cfc45bc5d7..e232d3134dc 100644 --- a/api/src/com/cloud/network/NetworkService.java +++ b/api/src/com/cloud/network/NetworkService.java @@ -35,6 +35,7 @@ import com.cloud.network.Network.Provider; import com.cloud.network.Network.Service; import com.cloud.network.Networks.TrafficType; import com.cloud.user.Account; +import com.cloud.user.User; public interface NetworkService { @@ -77,7 +78,7 @@ public interface NetworkService { Long getDedicatedNetworkDomain(long networkId); - Network updateNetwork(long networkId, String name, String displayText, Account caller, String domainSuffix, Long networkOfferingId); + Network updateNetwork(long networkId, String name, String displayText, Account callerAccount, User callerUser, String domainSuffix, Long networkOfferingId); Integer getNetworkRate(long networkId, Long vmId); diff --git a/server/src/com/cloud/network/NetworkManagerImpl.java b/server/src/com/cloud/network/NetworkManagerImpl.java index 16d86dd6591..be9bb10c87c 100755 --- a/server/src/com/cloud/network/NetworkManagerImpl.java +++ b/server/src/com/cloud/network/NetworkManagerImpl.java @@ -3194,7 +3194,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag @Override @ActionEvent(eventType = EventTypes.EVENT_NETWORK_UPDATE, eventDescription = "updating network", async = true) - public Network updateNetwork(long networkId, String name, String displayText, Account caller, String domainSuffix, Long networkOfferingId) { + public Network updateNetwork(long networkId, String name, String displayText, Account callerAccount, User callerUser, String domainSuffix, Long networkOfferingId) { boolean restartNetwork = false; // verify input parameters @@ -3203,7 +3203,7 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag throw new InvalidParameterValueException("Network id=" + networkId + "doesn't exist in the system"); } - _accountMgr.checkAccess(caller, null, network); + _accountMgr.checkAccess(callerAccount, null, network); // Don't allow to update system network NetworkOffering offering = _networkOfferingDao.findByIdIncludingRemoved(network.getNetworkOfferingId()); @@ -3270,13 +3270,13 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag } } - boolean success = _networksDao.update(networkId, network); - - if (success && restartNetwork && (network.getState() == Network.State.Implemented || network.getState() == Network.State.Setup)) { + _networksDao.update(networkId, network); + boolean success = true; + if (restartNetwork && (network.getState() == Network.State.Implemented || network.getState() == Network.State.Setup)) { s_logger.info("Restarting network " + network + " as a part of update network call"); try { - success = restartNetwork(networkId, caller, null, networkOfferingId, true); + success = restartNetwork(networkId, callerAccount, callerUser, networkOfferingId, true); } catch (Exception e) { success = false; } @@ -3500,11 +3500,17 @@ public class NetworkManagerImpl implements NetworkManager, NetworkService, Manag } //tags should be the same - if (!oldNetworkOffering.getTags().equalsIgnoreCase(newNetworkOffering.getTags())) { - s_logger.debug("Network offerings " + newNetworkOfferingId + " and " + oldNetworkOfferingId + " have different tags, can't upgrade"); - return false; + if (newNetworkOffering.getTags() != null) { + if (oldNetworkOffering.getTags() == null) { + s_logger.debug("New network offering id=" + newNetworkOfferingId + " has tags and old network offering id=" + oldNetworkOfferingId + " doesn't, can't upgrade"); + return false; + } + if (!oldNetworkOffering.getTags().equalsIgnoreCase(newNetworkOffering.getTags())) { + s_logger.debug("Network offerings " + newNetworkOfferingId + " and " + oldNetworkOfferingId + " have different tags, can't upgrade"); + return false; + } } - + //Traffic types should be the same if (oldNetworkOffering.getTrafficType() != newNetworkOffering.getTrafficType()) { s_logger.debug("Network offerings " + newNetworkOfferingId + " and " + oldNetworkOfferingId + " have different traffic types, can't upgrade"); diff --git a/server/test/com/cloud/network/MockNetworkManagerImpl.java b/server/test/com/cloud/network/MockNetworkManagerImpl.java index 4b1182fc3ba..e0c2494e104 100644 --- a/server/test/com/cloud/network/MockNetworkManagerImpl.java +++ b/server/test/com/cloud/network/MockNetworkManagerImpl.java @@ -480,7 +480,7 @@ public class MockNetworkManagerImpl implements NetworkManager, Manager, NetworkS } @Override - public Network updateNetwork(long networkId, String name, String displayText, Account caller, String domainSuffix, Long networkOfferingId) { + public Network updateNetwork(long networkId, String name, String displayText, Account callerAccount, User callerUser, String domainSuffix, Long networkOfferingId) { // TODO Auto-generated method stub return null; }