AccountManager/Service: reduced visibility for methods that are called only from AccountManagerImpl itself

Alena Prokharchyk · Alena Prokharchyk · commit 1300fc812867 · 2013-05-30T11:18:32.000-07:00
diff --git a/api/src/com/cloud/user/AccountService.java b/api/src/com/cloud/user/AccountService.java
@@ -16,22 +16,15 @@
 // under the License.
 package com.cloud.user;
 
-import java.util.List;
 import java.util.Map;
 
 import org.apache.cloudstack.acl.ControlledEntity;
 import org.apache.cloudstack.acl.RoleType;
 import org.apache.cloudstack.acl.SecurityChecker.AccessType;
-import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd;
-import org.apache.cloudstack.api.command.admin.user.DeleteUserCmd;
-import org.apache.cloudstack.api.command.admin.user.RegisterCmd;
-import org.apache.cloudstack.api.command.admin.user.UpdateUserCmd;
-
 import org.apache.cloudstack.api.command.admin.user.RegisterCmd;
 
 import com.cloud.domain.Domain;
 import com.cloud.exception.PermissionDeniedException;
-import com.cloud.utils.Pair;
 
 public interface AccountService {
 
@@ -83,13 +76,11 @@ UserAccount createUserAccount(String userName, String password, String firstName
 
     Account finalizeOwner(Account caller, String accountName, Long domainId, Long projectId);
 
-    Pair<List<Long>, Long> finalizeAccountDomainForList(Account caller, String accountName, Long domainId, Long projectId);
-
     Account getActiveAccountByName(String accountName, Long domainId);
 
-    Account getActiveAccountById(Long accountId);
+    Account getActiveAccountById(long accountId);
 
-    Account getAccount(Long accountId);
+    Account getAccount(long accountId);
 
     User getActiveUser(long userId);
 
diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java
@@ -551,6 +551,7 @@ private String queueCommand(BaseCmd cmdObj, Map<String, String> params) throws E
         }
     }
 
+    @SuppressWarnings("unchecked")
     private void buildAsyncListResponse(BaseListCmd command, Account account) {
         List<ResponseObject> responses = ((ListResponse) command.getResponseObject()).getResponses();
         if (responses != null && responses.size() > 0) {
@@ -845,7 +846,7 @@ public void loginUser(HttpSession session, String username, String password, Lon
 
     @Override
     public void logoutUser(long userId) {
-        _accountMgr.logoutUser(Long.valueOf(userId));
+        _accountMgr.logoutUser(userId);
         return;
     }
 
diff --git a/server/src/com/cloud/user/AccountManager.java b/server/src/com/cloud/user/AccountManager.java
@@ -47,21 +47,15 @@ public interface AccountManager extends AccountService {
     
     boolean deleteAccount(AccountVO account, long callerUserId, Account caller);
 
-	boolean cleanupAccount(AccountVO account, long callerUserId, Account caller);
-
 	Long checkAccessAndSpecifyAuthority(Account caller, Long zoneId);
 	
 	Account createAccount(String accountName, short accountType, Long domainId, String networkDomain, Map<String, String> details, String uuid);
-	
-	UserVO createUser(long accountId, String userName, String password, String firstName, String lastName, String email, String timezone, String userUUID);
-	
+		
     /**
      * Logs out a user
      * @param userId
      */
-    void logoutUser(Long userId);
-
-    UserAccount getUserAccount(String username, Long domainId);
+    void logoutUser(long userId);
     
     /**
      * Authenticates a user when s/he logs in.
@@ -87,9 +81,7 @@ public interface AccountManager extends AccountService {
      * @return the user/account pair if one exact match was found, null otherwise
      */
     Pair<User, Account> findUserByApiKey(String apiKey);
-
-    boolean lockAccount(long accountId);
-
+    
 	boolean enableAccount(long accountId);
 
 	void buildACLSearchBuilder(SearchBuilder<? extends ControlledEntity> sb, Long domainId,
diff --git a/server/src/com/cloud/user/AccountManagerImpl.java b/server/src/com/cloud/user/AccountManagerImpl.java
@@ -50,7 +50,6 @@
 import org.apache.log4j.Logger;
 
 import com.cloud.api.ApiDBUtils;
-import com.cloud.api.query.dao.UserAccountJoinDao;
 import com.cloud.api.query.vo.ControlledViewEntity;
 import com.cloud.configuration.Config;
 import com.cloud.configuration.ConfigurationManager;
@@ -105,7 +104,6 @@
 import com.cloud.projects.dao.ProjectAccountDao;
 import com.cloud.projects.dao.ProjectDao;
 import com.cloud.server.auth.UserAuthenticator;
-import com.cloud.storage.StorageManager;
 import com.cloud.storage.VMTemplateVO;
 import com.cloud.storage.Volume;
 import com.cloud.storage.VolumeManager;
@@ -164,8 +162,6 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
     @Inject
     private UserAccountDao _userAccountDao;
     @Inject
-    private UserAccountJoinDao _userAccountJoinDao;
-    @Inject
     private VolumeDao _volumeDao;
     @Inject
     private UserVmDao _userVmDao;
@@ -190,8 +186,6 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M
     @Inject
     private UserVmManager _vmMgr;
     @Inject
-    private StorageManager _storageMgr;
-    @Inject
     private TemplateManager _tmpltMgr;
     @Inject
     private ConfigurationManager _configMgr;
@@ -505,8 +499,8 @@ public boolean enableAccount(long accountId) {
         return success;
     }
 
-    @Override
-    public boolean lockAccount(long accountId) {
+    
+    protected boolean lockAccount(long accountId) {
         boolean success = false;
         Account account = _accountDao.findById(accountId);
         if (account != null) {
@@ -544,8 +538,8 @@ public boolean deleteAccount(AccountVO account, long callerUserId, Account calle
         return cleanupAccount(account, callerUserId, caller);
     }
 
-    @Override
-    public boolean cleanupAccount(AccountVO account, long callerUserId, Account caller) {
+
+    protected boolean cleanupAccount(AccountVO account, long callerUserId, Account caller) {
         long accountId = account.getId();
         boolean accountCleanupNeeded = false;
 
@@ -1617,21 +1611,13 @@ public Account getActiveAccountByName(String accountName, Long domainId) {
     }
 
     @Override
-    public Account getActiveAccountById(Long accountId) {
-        if (accountId == null) {
-            throw new InvalidParameterValueException("AccountId is required by account search");
-        } else {
-            return _accountDao.findById(accountId);
-        }
+    public Account getActiveAccountById(long accountId) {
+        return _accountDao.findById(accountId);
     }
 
     @Override
-    public Account getAccount(Long accountId) {
-        if (accountId == null) {
-            throw new InvalidParameterValueException("AccountId is required by account search");
-        } else {
-            return _accountDao.findByIdIncludingRemoved(accountId);
-        }
+    public Account getAccount(long accountId) {
+        return _accountDao.findByIdIncludingRemoved(accountId);
     }
 
     @Override
@@ -1669,62 +1655,6 @@ public User getUserIncludingRemoved(long userId) {
         return _userDao.findByIdIncludingRemoved(userId);
     }
 
-    @Override
-    public Pair<List<Long>, Long> finalizeAccountDomainForList(Account caller, String accountName, Long domainId, Long projectId) {
-        List<Long> permittedAccounts = new ArrayList<Long>();
-
-        if (isAdmin(caller.getType())) {
-            if (domainId == null && accountName != null) {
-                throw new InvalidParameterValueException("accountName and domainId might be specified together");
-            } else if (domainId != null) {
-                Domain domain = _domainMgr.getDomain(domainId);
-                if (domain == null) {
-                    throw new InvalidParameterValueException("Unable to find the domain by id=" + domainId);
-                }
-
-                checkAccess(caller, domain);
-
-                if (accountName != null) {
-                    Account owner = getActiveAccountByName(accountName, domainId);
-                    if (owner == null) {
-                        throw new InvalidParameterValueException("Unable to find account with name " + accountName + " in domain id=" + domainId);
-                    }
-
-                    permittedAccounts.add(owner.getId());
-                }
-            }
-        } else if (accountName != null && domainId != null) {
-            if (!accountName.equals(caller.getAccountName()) || domainId.longValue() != caller.getDomainId()) {
-                throw new PermissionDeniedException("Can't list port forwarding rules for account " + accountName + " in domain " + domainId + ", permission denied");
-            }
-            permittedAccounts.add(getActiveAccountByName(accountName, domainId).getId());
-        } else {
-            permittedAccounts.add(caller.getAccountId());
-        }
-
-        if (domainId == null && caller.getType() == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) {
-            domainId = caller.getDomainId();
-        }
-
-        // set project information
-        if (projectId != null) {
-            if (projectId.longValue() == -1) {
-                permittedAccounts.addAll(_projectMgr.listPermittedProjectAccounts(caller.getId()));
-            } else {
-                permittedAccounts.clear();
-                Project project = _projectMgr.getProject(projectId);
-                if (project == null) {
-                    throw new InvalidParameterValueException("Unable to find project by id " + projectId);
-                }
-                if (!_projectMgr.canAccessProjectAccount(caller, project.getProjectAccountId())) {
-                    throw new InvalidParameterValueException("Account " + caller + " can't access project id=" + projectId);
-                }
-                permittedAccounts.add(project.getProjectAccountId());
-            }
-        }
-
-        return new Pair<List<Long>, Long>(permittedAccounts, domainId);
-    }
 
     @Override
     public User getActiveUserByRegistrationToken(String registrationToken) {
@@ -1806,9 +1736,8 @@ public AccountVO createAccount(String accountName, short accountType, Long domai
         return account;
     }
 
-    @Override
     @ActionEvent(eventType = EventTypes.EVENT_USER_CREATE, eventDescription = "creating User")
-    public UserVO createUser(long accountId, String userName, String password, String firstName, String lastName, String email, String timezone, String userUUID) {
+    protected UserVO createUser(long accountId, String userName, String password, String firstName, String lastName, String email, String timezone, String userUUID) {
         if (s_logger.isDebugEnabled()) {
             s_logger.debug("Creating user: " + userName + ", accountId: " + accountId + " timezone:" + timezone);
         }
@@ -1833,29 +1762,13 @@ public UserVO createUser(long accountId, String userName, String password, Strin
     }
 
     @Override
-    public void logoutUser(Long userId) {
+    public void logoutUser(long userId) {
         UserAccount userAcct = _userAccountDao.findById(userId);
         if (userAcct != null) {
             ActionEventUtils.onActionEvent(userId, userAcct.getAccountId(), userAcct.getDomainId(), EventTypes.EVENT_USER_LOGOUT, "user has logged out");
         } // else log some kind of error event? This likely means the user doesn't exist, or has been deleted...
     }
 
-    @Override
-    public UserAccount getUserAccount(String username, Long domainId) {
-        if (s_logger.isDebugEnabled()) {
-            s_logger.debug("Retrieiving user: " + username + " in domain " + domainId);
-        }
-
-        UserAccount userAccount = _userAccountDao.getUserAccount(username, domainId);
-        if (userAccount == null) {
-            if (s_logger.isDebugEnabled()) {
-                s_logger.debug("Unable to find user with name " + username + " in domain " + domainId);
-            }
-            return null;
-        }
-
-        return userAccount;
-    }
 
     @Override
     public UserAccount authenticateUser(String username, String password, Long domainId, String loginIpAddress, Map<String, Object[]> requestParameters) {
diff --git a/server/test/com/cloud/network/MockRulesManagerImpl.java b/server/test/com/cloud/network/MockRulesManagerImpl.java
@@ -135,19 +135,6 @@ public boolean disableStaticNat(long ipId)
 		return false;
 	}
 
-	@Override
-	public boolean applyPortForwardingRules(long ipAddressId,
-			boolean continueOnError, Account caller) {
-		// TODO Auto-generated method stub
-		return false;
-	}
-
-	@Override
-	public boolean applyStaticNatRulesForIp(long sourceIpId,
-			boolean continueOnError, Account caller, boolean forRevoke) {
-		// TODO Auto-generated method stub
-		return false;
-	}
 
 	@Override
 	public boolean applyPortForwardingRulesForNetwork(long networkId,
@@ -163,13 +150,6 @@ public boolean applyStaticNatRulesForNetwork(long networkId,
 		return false;
 	}
 
-	@Override
-	public void checkIpAndUserVm(IpAddress ipAddress, UserVm userVm,
-			Account caller) {
-		// TODO Auto-generated method stub
-
-	}
-
 	@Override
 	public void checkRuleAndUserVm(FirewallRule rule, UserVm userVm,
 			Account caller) {
@@ -191,37 +171,13 @@ public boolean revokeAllPFStaticNatRulesForNetwork(long networkId,
 		return false;
 	}
 
-	@Override
-	public List<? extends FirewallRule> listFirewallRulesByIp(long ipAddressId) {
-		// TODO Auto-generated method stub
-		return null;
-	}
-
-	@Override
-	public List<? extends PortForwardingRule> listPortForwardingRulesForApplication(
-			long ipId) {
-		// TODO Auto-generated method stub
-		return null;
-	}
-
-	@Override
-	public List<? extends PortForwardingRule> gatherPortForwardingRulesForApplication(
-			List<? extends IpAddress> addrs) {
-		// TODO Auto-generated method stub
-		return null;
-	}
 
 	@Override
 	public boolean revokePortForwardingRulesForVm(long vmId) {
 		// TODO Auto-generated method stub
 		return false;
 	}
 
-	@Override
-	public boolean revokeStaticNatRulesForVm(long vmId) {
-		// TODO Auto-generated method stub
-		return false;
-	}
 
 	@Override
 	public FirewallRule[] reservePorts(IpAddress ip, String protocol,
@@ -231,25 +187,6 @@ public FirewallRule[] reservePorts(IpAddress ip, String protocol,
 		return null;
 	}
 
-	@Override
-	public boolean releasePorts(long ipId, String protocol, Purpose purpose,
-			int... ports) {
-		// TODO Auto-generated method stub
-		return false;
-	}
-
-	@Override
-	public List<PortForwardingRuleVO> listByNetworkId(long networkId) {
-		// TODO Auto-generated method stub
-		return null;
-	}
-
-	@Override
-	public boolean applyStaticNatForIp(long sourceIpId,
-			boolean continueOnError, Account caller, boolean forRevoke) {
-		// TODO Auto-generated method stub
-		return false;
-	}
 
 	@Override
 	public boolean applyStaticNatsForNetwork(long networkId,
diff --git a/server/test/com/cloud/user/MockAccountManagerImpl.java b/server/test/com/cloud/user/MockAccountManagerImpl.java

Original file line number	Diff line number	Diff line change
`@@ -551,6 +551,7 @@ private String queueCommand(BaseCmd cmdObj, Map<String, String> params) throws E`
`551`	`551`	`}`
`552`	`552`	`}`
`553`	`553`
	`554`	`+ @SuppressWarnings("unchecked")`
`554`	`555`	`private void buildAsyncListResponse(BaseListCmd command, Account account) {`
`555`	`556`	`List<ResponseObject> responses = ((ListResponse) command.getResponseObject()).getResponses();`
`556`	`557`	`if (responses != null && responses.size() > 0) {`
`@@ -845,7 +846,7 @@ public void loginUser(HttpSession session, String username, String password, Lon`
`845`	`846`
`846`	`847`	`@Override`
`847`	`848`	`public void logoutUser(long userId) {`
`848`		`- _accountMgr.logoutUser(Long.valueOf(userId));`
	`849`	`+ _accountMgr.logoutUser(userId);`
`849`	`850`	`return;`
`850`	`851`	`}`
`851`	`852`