Skip to content

Commit fc92e39

Browse files
winterhazelwinterhazel
committed
Apply suggestions + some adjustments
1 parent 589a07e commit fc92e39

File tree

8 files changed

+61
-115
lines changed

8 files changed

+61
-115
lines changed

‎api/src/main/java/com/cloud/user/AccountService.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,8 @@ User createUser(String userName, String password, String firstName, String lastN
126126

127127
Long finalizeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly);
128128

129+
Long finalizeAccountId(Long accountId, String accountName, Long domainId, Long projectId);
130+
129131
/**
130132
* returns the user account object for a given user id
131133
* @param userId user id

‎framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaAccountStateFilter.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,20 @@
1616
// under the License.
1717
package org.apache.cloudstack.quota;
1818

19+
import org.apache.commons.lang3.StringUtils;
20+
1921
public enum QuotaAccountStateFilter {
2022
ALL, ACTIVE, REMOVED;
2123

2224
public static QuotaAccountStateFilter getValue(String value) {
25+
if (StringUtils.isBlank(value)) {
26+
return null;
27+
}
2328
for (QuotaAccountStateFilter state : values()) {
2429
if (state.name().equalsIgnoreCase(value)) {
2530
return state;
2631
}
2732
}
28-
2933
return null;
3034
}
3135
}

‎plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaSummaryCmd.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public class QuotaSummaryCmd extends BaseListCmd {
5151
QuotaService quotaService;
5252

5353
@ACL
54-
@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, entityType = AccountResponse.class, description = "ID of the account for which balance will be listed. Can not be specified with projectId.", since = "4.21.0")
54+
@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, entityType = AccountResponse.class, description = "ID of the account for which balance will be listed. Can not be specified with projectId.", since = "4.23.0")
5555
private Long accountId;
5656

5757
@Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, required = false, description = "Optional, Account Id for which statement needs to be generated")
@@ -60,18 +60,17 @@ public class QuotaSummaryCmd extends BaseListCmd {
6060
@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, required = false, entityType = DomainResponse.class, description = "Optional, If domain Id is given and the caller is domain admin then the statement is generated for domain.")
6161
private Long domainId;
6262

63-
@Deprecated
64-
@Parameter(name = ApiConstants.LIST_ALL, type = CommandType.BOOLEAN, description = "False (default) lists balance summary for account. True lists balance summary for " +
63+
@Parameter(name = ApiConstants.LIST_ALL, type = CommandType.BOOLEAN, description = "False (default) lists the Quota balance summary for calling Account. True lists balance summary for " +
6564
"Accounts which the caller has access.")
6665
private Boolean listAll;
6766

6867
@Parameter(name = ApiConstants.ACCOUNT_STATE_TO_SHOW, type = CommandType.STRING, description = "Possible values are [ALL, ACTIVE, REMOVED]. ALL will list summaries for " +
6968
"active and removed accounts; ACTIVE will list summaries only for active accounts; REMOVED will list summaries only for removed accounts. The default value is ACTIVE.",
70-
since = "4.21.0")
69+
since = "4.23.0")
7170
private String accountStateToShow;
7271

7372
@ACL
74-
@Parameter(name = ApiConstants.PROJECT_ID, type = CommandType.UUID, entityType = ProjectResponse.class, description = "ID of the project for which balance will be listed. Can not be specified with accountId.", since = "4.21.0")
73+
@Parameter(name = ApiConstants.PROJECT_ID, type = CommandType.UUID, entityType = ProjectResponse.class, description = "ID of the project for which balance will be listed. Can not be specified with accountId.", since = "4.23.0")
7574
private Long projectId;
7675

7776
@Override
@@ -136,6 +135,6 @@ public long getEntityOwnerId() {
136135
if (ObjectUtils.allNull(accountId, accountName, projectId)) {
137136
return -1;
138137
}
139-
return quotaService.finalizeAccountId(accountId, accountName, domainId, projectId);
138+
return _accountService.finalizeAccountId(accountId, accountName, domainId, projectId);
140139
}
141140
}

‎plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -198,21 +198,7 @@ public QuotaTariffResponse createQuotaTariffResponse(QuotaTariffVO tariff, boole
198198
public Pair<List<QuotaSummaryResponse>, Integer> createQuotaSummaryResponse(QuotaSummaryCmd cmd) {
199199
Account caller = CallContext.current().getCallingAccount();
200200

201-
if (cmd.getProjectId() != null && !cmd.isListAll() && accountTypesThatCanListAllQuotaSummaries.contains(caller.getType())) {
202-
ProjectVO projectVO = projectDao.findById(cmd.getProjectId());
203-
Long projectAccountId = projectVO.getProjectAccountId();
204-
return getQuotaSummaryResponse(projectAccountId, null, null, null, cmd);
205-
}
206-
207-
else if (cmd.getAccountId() != null && !cmd.isListAll() && accountTypesThatCanListAllQuotaSummaries.contains(caller.getType())) {
208-
return getQuotaSummaryResponse(cmd.getAccountId(), null, null, null, cmd);
209-
}
210-
211-
else if (cmd.getDomainId() != null && !cmd.isListAll() && accountTypesThatCanListAllQuotaSummaries.contains(caller.getType())) {
212-
return getQuotaSummaryResponse(null, null, cmd.getDomainId(), null, cmd);
213-
}
214-
215-
else if (!accountTypesThatCanListAllQuotaSummaries.contains(caller.getType()) || !cmd.isListAll()) {
201+
if (!accountTypesThatCanListAllQuotaSummaries.contains(caller.getType()) || !cmd.isListAll()) {
216202
return getQuotaSummaryResponse(caller.getAccountId(), null, null, null, cmd);
217203
}
218204

@@ -222,7 +208,7 @@ else if (!accountTypesThatCanListAllQuotaSummaries.contains(caller.getType()) ||
222208
protected Pair<List<QuotaSummaryResponse>, Integer> getQuotaSummaryResponseWithListAll(QuotaSummaryCmd cmd, Account caller) {
223209
Long accountId = cmd.getEntityOwnerId();
224210
if (accountId == -1) {
225-
accountId = null;
211+
accountId = cmd.isListAll() ? null : caller.getAccountId();
226212
}
227213

228214
Long domainId = cmd.getDomainId();
@@ -276,7 +262,7 @@ protected String getDomainPathByDomainIdForDomainAdmin(Account caller) {
276262
_accountMgr.checkAccess(caller, domain);
277263

278264
if (domain == null) {
279-
throw new InvalidParameterValueException(String.format("Domain id [%s] is invalid.", domainId));
265+
throw new InvalidParameterValueException(String.format("Domain ID [%s] is invalid.", domainId));
280266
}
281267

282268
return domain.getPath();
@@ -288,8 +274,7 @@ protected Pair<List<QuotaSummaryResponse>, Integer> getQuotaSummaryResponse(Long
288274
List<QuotaSummaryVO> summaries = pairSummaries.first();
289275

290276
if (CollectionUtils.isEmpty(summaries)) {
291-
logger.info(String.format("There are no summaries to list for parameters [%s].",
292-
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(cmd, "accountName", "domainId", "listAll", "page", "pageSize")));
277+
logger.info("There are no summaries to list for parameters [{}].", ReflectionToStringBuilderUtils.reflectOnlySelectedFields(cmd, "accountName", "domainId", "listAll", "page", "pageSize"));
293278
return new Pair<>(new ArrayList<>(), 0);
294279
}
295280

‎plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaService.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,6 @@ public interface QuotaService extends PluggableService {
4040

4141
boolean saveQuotaAccount(AccountVO account, BigDecimal aggrUsage, Date endDate);
4242

43-
Long finalizeAccountId(Long accountId, String accountName, Long domainId, Long projectId);
44-
4543
boolean isJsInterpretationEnabled();
4644

4745
}

‎plugins/database/quota/src/main/java/org/apache/cloudstack/quota/QuotaServiceImpl.java‎

Lines changed: 0 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,8 @@
2626
import javax.inject.Inject;
2727
import javax.naming.ConfigurationException;
2828

29-
import com.cloud.projects.Project;
3029
import com.cloud.projects.ProjectManager;
3130
import com.cloud.user.AccountService;
32-
import org.apache.cloudstack.api.ApiConstants;
33-
import org.apache.cloudstack.api.ApiErrorCode;
34-
import org.apache.cloudstack.api.ServerApiException;
3531
import org.apache.cloudstack.api.command.QuotaBalanceCmd;
3632
import org.apache.cloudstack.api.command.QuotaConfigureEmailCmd;
3733
import org.apache.cloudstack.api.command.QuotaCreditsCmd;
@@ -290,48 +286,6 @@ public boolean saveQuotaAccount(final AccountVO account, final BigDecimal aggrUs
290286
}
291287
}
292288

293-
/**
294-
* Returns the Id of the account that will be used when provided with either accountId, projectId or accountName and domainId.
295-
*/
296-
@Override
297-
public Long finalizeAccountId(Long accountId, String accountName, Long domainId, Long projectId) {
298-
if (projectId != null) {
299-
if (accountId != null || accountName != null) {
300-
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Project and account can not be specified together.");
301-
}
302-
final Project project = projectMgr.getProject(projectId);
303-
if (project == null) {
304-
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Unable to find project with id: [%s].", projectId));
305-
}
306-
if (project.getState() != Project.State.Active) {
307-
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Project with projectId [%s] is not active.", projectId));
308-
}
309-
return project.getProjectAccountId();
310-
}
311-
312-
if (accountId != null) {
313-
if (accountService.getActiveAccountById(accountId) != null) {
314-
return accountId;
315-
}
316-
throw new InvalidParameterValueException(String.format("Unable to find account with accountId: [%s].", accountId));
317-
}
318-
319-
if (accountName == null && domainId == null) {
320-
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Either %s or %s is required.", ApiConstants.ACCOUNT_ID, ApiConstants.PROJECT_ID));
321-
}
322-
try {
323-
Account activeAccount = accountService.getActiveAccountByName(accountName, domainId);
324-
if (activeAccount != null) {
325-
return activeAccount.getId();
326-
}
327-
} catch (InvalidParameterValueException exception) {
328-
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Both %s and %s are needed if using either. Consider using %s instead.",
329-
ApiConstants.ACCOUNT, ApiConstants.DOMAIN_ID, ApiConstants.ACCOUNT_ID));
330-
}
331-
throw new InvalidParameterValueException(String.format("Unable to find account by name: [%s] on domain: [%s]", accountName, domainId));
332-
}
333-
334-
335289
@Override
336290
public void setMinBalance(Long accountId, Double balance) {
337291
QuotaAccountVO acc = _quotaAcc.findByIdQuotaAccount(accountId);
@@ -345,36 +299,6 @@ public void setMinBalance(Long accountId, Double balance) {
345299
}
346300
}
347301

348-
protected Long getAccountToWhomQuotaBalancesWillBeListed(Long accountId, String accountName, Long domainId) {
349-
if (accountId != null) {
350-
Account account = _accountDao.findByIdIncludingRemoved(accountId);
351-
if (account == null) {
352-
throw new InvalidParameterValueException(String.format("Unable to find account [%s].", accountId));
353-
}
354-
return accountId;
355-
}
356-
357-
validateIsChildDomain(accountName, domainId);
358-
359-
Account account = _accountDao.findActiveAccount(accountName, domainId);
360-
if (account == null) {
361-
throw new InvalidParameterValueException(String.format("Unable to find active account [%s] in domain [%s].", accountName, domainId));
362-
}
363-
return account.getAccountId();
364-
}
365-
366-
protected void validateIsChildDomain(String accountName, Long domainId) {
367-
Account caller = CallContext.current().getCallingAccount();
368-
369-
long callerDomainId = caller.getDomainId();
370-
if (_domainDao.isChildDomain(callerDomainId, domainId)) {
371-
return;
372-
}
373-
374-
logger.debug(String.format("Domain with ID [%s] is not a child of the caller's domain [%s].", domainId, callerDomainId));
375-
throw new PermissionDeniedException(String.format("Account [%s] or domain [%s] is invalid.", accountName, domainId));
376-
}
377-
378302
@Override
379303
public boolean isJsInterpretationEnabled() {
380304
return jsInterpretationEnabled;

‎plugins/database/quota/src/test/java/org/apache/cloudstack/quota/QuotaServiceImplTest.java‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
import org.apache.cloudstack.quota.vo.QuotaAccountVO;
3232
import org.apache.cloudstack.quota.vo.QuotaBalanceVO;
3333
import org.joda.time.DateTime;
34-
import org.junit.Assert;
3534
import org.junit.Before;
3635
import org.junit.Test;
3736
import org.junit.runner.RunWith;
@@ -142,8 +141,6 @@ public void testGetQuotaUsage() {
142141
final Date startDate = new DateTime().minusDays(2).toDate();
143142
final Date endDate = new Date();
144143

145-
Mockito.lenient().doReturn(accountId).when(quotaServiceImplSpy).getAccountToWhomQuotaBalancesWillBeListed(Mockito.anyLong(), Mockito.anyString(), Mockito.anyLong());
146-
147144
quotaServiceImplSpy.getQuotaUsage(accountId, accountName, domainId, QuotaTypes.IP_ADDRESS, startDate, endDate);
148145
Mockito.verify(quotaUsageDao, Mockito.times(1)).findQuotaUsage(Mockito.eq(accountId), Mockito.eq(domainId), Mockito.eq(QuotaTypes.IP_ADDRESS), Mockito.any(Date.class), Mockito.any(Date.class));
149146
}
@@ -181,12 +178,4 @@ public void testSetMinBalance() {
181178
Mockito.verify(quotaAcc, Mockito.times(1)).persistQuotaAccount(Mockito.any(QuotaAccountVO.class));
182179
}
183180

184-
@Test
185-
public void getAccountToWhomQuotaBalancesWillBeListedTestAccountIdIsNotNullReturnsExistingAccount() {
186-
long expected = 1L;
187-
Mockito.doReturn(accountVoMock).when(accountDaoMock).findByIdIncludingRemoved(Mockito.anyLong());
188-
long result = quotaServiceImplSpy.getAccountToWhomQuotaBalancesWillBeListed(expected, "test", 2L);
189-
Assert.assertEquals(expected, result);
190-
}
191-
192181
}

‎server/src/main/java/com/cloud/user/AccountManagerImpl.java‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@
5656
import org.apache.cloudstack.api.APICommand;
5757
import org.apache.cloudstack.api.ApiCommandResourceType;
5858
import org.apache.cloudstack.api.ApiConstants;
59+
import org.apache.cloudstack.api.ApiErrorCode;
60+
import org.apache.cloudstack.api.ServerApiException;
5961
import org.apache.cloudstack.api.command.admin.account.CreateAccountCmd;
6062
import org.apache.cloudstack.api.command.admin.account.UpdateAccountCmd;
6163
import org.apache.cloudstack.api.command.admin.user.DeleteUserCmd;
@@ -86,6 +88,7 @@
8688
import org.apache.commons.codec.binary.Base64;
8789
import org.apache.commons.collections.CollectionUtils;
8890
import org.apache.commons.lang3.BooleanUtils;
91+
import org.apache.commons.lang3.ObjectUtils;
8992
import org.apache.commons.lang3.StringUtils;
9093
import org.jetbrains.annotations.NotNull;
9194
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
@@ -3506,6 +3509,48 @@ public Long finalizeAccountId(final String accountName, final Long domainId, fin
35063509
return null;
35073510
}
35083511

3512+
@Override
3513+
public Long finalizeAccountId(Long accountId, String accountName, Long domainId, Long projectId) {
3514+
if (projectId != null) {
3515+
if (ObjectUtils.anyNotNull(accountId, accountName)) {
3516+
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Project and account can not be specified together.");
3517+
}
3518+
return getActiveProjectAccountByProjectId(projectId);
3519+
}
3520+
if (accountId != null) {
3521+
if (getActiveAccountById(accountId) != null) {
3522+
return accountId;
3523+
}
3524+
throw new InvalidParameterValueException(String.format("Unable to find account with ID [%s].", accountId));
3525+
}
3526+
3527+
if (accountName == null && domainId == null) {
3528+
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Either %s or %s is required.", ApiConstants.ACCOUNT_ID, ApiConstants.PROJECT_ID));
3529+
}
3530+
3531+
try {
3532+
Account activeAccount = getActiveAccountByName(accountName, domainId);
3533+
if (activeAccount != null) {
3534+
return activeAccount.getId();
3535+
}
3536+
} catch (InvalidParameterValueException exception) {
3537+
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Both %s and %s are needed if using either. Consider using %s instead.",
3538+
ApiConstants.ACCOUNT, ApiConstants.DOMAIN_ID, ApiConstants.ACCOUNT_ID));
3539+
}
3540+
throw new InvalidParameterValueException(String.format("Unable to find account by name [%s] on domain [%s].", accountName, domainId));
3541+
}
3542+
3543+
protected long getActiveProjectAccountByProjectId(long projectId) {
3544+
Project project = _projectMgr.getProject(projectId);
3545+
if (project == null) {
3546+
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Unable to find project with ID [%s].", projectId));
3547+
}
3548+
if (project.getState() != Project.State.Active) {
3549+
throw new ServerApiException(ApiErrorCode.PARAM_ERROR, String.format("Project with ID [%s] is not active.", projectId));
3550+
}
3551+
return project.getProjectAccountId();
3552+
}
3553+
35093554
@Override
35103555
public UserAccount getUserAccountById(Long userId) {
35113556
UserAccount userAccount = _userAccountDao.findById(userId);

0 commit comments

Comments
 (0)