From aecfb2f6d016851c15d6e9085b0faa263ea1c20f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Tue, 5 Dec 2023 14:40:22 -0300 Subject: [PATCH 1/6] quota tariff positions --- .../META-INF/db/schema-41810to41900.sql | 3 ++ .../cloudstack/quota/QuotaManagerImpl.java | 20 ++++++- .../presetvariables/Tariff.java | 33 ++++++++++++ .../cloudstack/quota/vo/QuotaTariffVO.java | 14 +++++ .../quota/QuotaManagerImplTest.java | 53 +++++++++++++------ .../api/command/QuotaTariffCreateCmd.java | 12 +++++ .../api/command/QuotaTariffUpdateCmd.java | 11 ++++ .../response/QuotaResponseBuilderImpl.java | 18 +++++-- .../api/response/QuotaTariffResponse.java | 13 +++++ .../QuotaResponseBuilderImplTest.java | 18 ++++++- 10 files changed, 174 insertions(+), 21 deletions(-) create mode 100644 framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Tariff.java diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql b/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql index 27170fcac149..982dce037fe9 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41810to41900.sql @@ -314,3 +314,6 @@ CREATE TABLE `cloud_usage`.`bucket_statistics` ( `size` bigint unsigned COMMENT 'total size of bucket objects', PRIMARY KEY(`id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; + +-- Quota inject tariff result into subsequent ones +ALTER TABLE `cloud_usage`.`quota_tariff` ADD COLUMN `position` bigint(20) NOT NULL DEFAULT 1 COMMENT 'Position in the execution sequence for tariffs of the same type' ; diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java index 56a6edf5db47..2112731949da 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/QuotaManagerImpl.java @@ -20,6 +20,7 @@ import java.math.RoundingMode; import java.util.ArrayList; import java.util.Arrays; +import java.util.Comparator; import java.util.Date; import java.util.HashMap; import java.util.List; @@ -35,6 +36,7 @@ import org.apache.cloudstack.quota.activationrule.presetvariables.GenericPresetVariable; import org.apache.cloudstack.quota.activationrule.presetvariables.PresetVariableHelper; import org.apache.cloudstack.quota.activationrule.presetvariables.PresetVariables; +import org.apache.cloudstack.quota.activationrule.presetvariables.Tariff; import org.apache.cloudstack.quota.constant.QuotaConfig; import org.apache.cloudstack.quota.constant.QuotaTypes; import org.apache.cloudstack.quota.dao.QuotaAccountDao; @@ -375,9 +377,22 @@ protected BigDecimal aggregateQuotaTariffsValues(UsageVO usageRecord, List lastTariffs = new ArrayList<>(); + + for (QuotaTariffVO quotaTariff : quotaTariffs) { if (isQuotaTariffInPeriodToBeApplied(usageRecord, quotaTariff, accountToString)) { - aggregatedQuotaTariffsValue = aggregatedQuotaTariffsValue.add(getQuotaTariffValueToBeApplied(quotaTariff, jsInterpreter, presetVariables)); + + BigDecimal tariffValue = getQuotaTariffValueToBeApplied(quotaTariff, jsInterpreter, presetVariables, lastTariffs); + + aggregatedQuotaTariffsValue = aggregatedQuotaTariffsValue.add(tariffValue); + + Tariff tariffPresetVariable = new Tariff(); + tariffPresetVariable.setId(quotaTariff.getUuid()); + tariffPresetVariable.setValue(tariffValue); + lastTariffs.add(tariffPresetVariable); } } @@ -405,7 +420,7 @@ protected PresetVariables getPresetVariables(boolean hasAnyQuotaTariffWithActiva *
  • If the activation rule result in something else, returns {@link BigDecimal#ZERO}.
  • * */ - protected BigDecimal getQuotaTariffValueToBeApplied(QuotaTariffVO quotaTariff, JsInterpreter jsInterpreter, PresetVariables presetVariables) { + protected BigDecimal getQuotaTariffValueToBeApplied(QuotaTariffVO quotaTariff, JsInterpreter jsInterpreter, PresetVariables presetVariables, List lastAppliedTariffsList) { String activationRule = quotaTariff.getActivationRule(); BigDecimal quotaTariffValue = quotaTariff.getCurrencyValue(); String quotaTariffToString = quotaTariff.toString(); @@ -417,6 +432,7 @@ protected BigDecimal getQuotaTariffValueToBeApplied(QuotaTariffVO quotaTariff, J } injectPresetVariablesIntoJsInterpreter(jsInterpreter, presetVariables); + jsInterpreter.injectVariable("lastTariffs", lastAppliedTariffsList.toString()); String scriptResult = jsInterpreter.executeScript(activationRule).toString(); diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Tariff.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Tariff.java new file mode 100644 index 000000000000..3703820a1a40 --- /dev/null +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/activationrule/presetvariables/Tariff.java @@ -0,0 +1,33 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.cloudstack.quota.activationrule.presetvariables; + +import java.math.BigDecimal; + +public class Tariff extends GenericPresetVariable { + private BigDecimal value; + + public BigDecimal getValue() { + return value; + } + + public void setValue(BigDecimal value) { + this.value = value; + fieldNamesToIncludeInToString.add("value"); + } +} diff --git a/framework/quota/src/main/java/org/apache/cloudstack/quota/vo/QuotaTariffVO.java b/framework/quota/src/main/java/org/apache/cloudstack/quota/vo/QuotaTariffVO.java index 7bc2870e36a1..2eca8793b241 100644 --- a/framework/quota/src/main/java/org/apache/cloudstack/quota/vo/QuotaTariffVO.java +++ b/framework/quota/src/main/java/org/apache/cloudstack/quota/vo/QuotaTariffVO.java @@ -90,6 +90,10 @@ public class QuotaTariffVO implements InternalIdentity { @Temporal(value = TemporalType.TIMESTAMP) private Date endDate; + @Column(name = "position") + protected Integer position; + + public QuotaTariffVO() { } @@ -117,6 +121,7 @@ public QuotaTariffVO(QuotaTariffVO that) { this.setDescription(that.getDescription()); this.setActivationRule(that.getActivationRule()); this.setEndDate(that.getEndDate()); + this.setPosition(that.getPosition()); } public void setId(Long id) { @@ -259,6 +264,15 @@ public boolean setUsageTypeData(int usageType) { return true; } + public Integer getPosition() { + return position; + } + + public void setPosition(Integer position) { + this.position = position; + } + + @Override public String toString() { return ReflectionToStringBuilderUtils.reflectOnlySelectedFields(this, "uuid", "name", "effectiveOn", "endDate"); diff --git a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java index e53051f2925b..5dfc12f7ef89 100644 --- a/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java +++ b/framework/quota/src/test/java/org/apache/cloudstack/quota/QuotaManagerImplTest.java @@ -29,6 +29,7 @@ import org.apache.cloudstack.quota.activationrule.presetvariables.GenericPresetVariable; import org.apache.cloudstack.quota.activationrule.presetvariables.PresetVariableHelper; import org.apache.cloudstack.quota.activationrule.presetvariables.PresetVariables; +import org.apache.cloudstack.quota.activationrule.presetvariables.Tariff; import org.apache.cloudstack.quota.activationrule.presetvariables.Value; import org.apache.cloudstack.quota.constant.QuotaTypes; import org.apache.cloudstack.quota.dao.QuotaTariffDao; @@ -395,7 +396,7 @@ public void getQuotaTariffValueToBeAppliedTestActivationRuleIsNullReturnTariffVa Mockito.doReturn(null).when(quotaTariffVoMock).getActivationRule(); Mockito.doReturn(BigDecimal.ONE).when(quotaTariffVoMock).getCurrencyValue(); - BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, null, null); + BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, null, null, null); Assert.assertEquals(BigDecimal.ONE, result); } @@ -405,7 +406,7 @@ public void getQuotaTariffValueToBeAppliedTestActivationRuleIsEmptyReturnTariffV Mockito.doReturn("").when(quotaTariffVoMock).getActivationRule(); Mockito.doReturn(BigDecimal.TEN).when(quotaTariffVoMock).getCurrencyValue(); - BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, null, null); + BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, null, null, null); Assert.assertEquals(BigDecimal.TEN, result); } @@ -413,13 +414,15 @@ public void getQuotaTariffValueToBeAppliedTestActivationRuleIsEmptyReturnTariffV @Test public void getQuotaTariffValueToBeAppliedTestScriptResultIsNumberReturnIt() { BigDecimal expected = new BigDecimal(50.1); + List lastTariffs = createLastAppliedTariffsPresetVariableList(0); + Mockito.doReturn(" ").when(quotaTariffVoMock).getActivationRule(); Mockito.doReturn(BigDecimal.TEN).when(quotaTariffVoMock).getCurrencyValue(); Mockito.doNothing().when(quotaManagerImplSpy).injectPresetVariablesIntoJsInterpreter(Mockito.any(), Mockito.any()); Mockito.doReturn(expected).when(jsInterpreterMock).executeScript(Mockito.anyString()); - BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, jsInterpreterMock, presetVariablesMock); + BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, jsInterpreterMock, presetVariablesMock, lastTariffs); Assert.assertEquals(expected, result); } @@ -427,37 +430,42 @@ public void getQuotaTariffValueToBeAppliedTestScriptResultIsNumberReturnIt() { @Test public void getQuotaTariffValueToBeAppliedTestScriptResultIsTrueReturnTariffValue() { BigDecimal expected = new BigDecimal(236.84); + List lastTariffs = createLastAppliedTariffsPresetVariableList(0); Mockito.doReturn(" ").when(quotaTariffVoMock).getActivationRule(); Mockito.doReturn(expected).when(quotaTariffVoMock).getCurrencyValue(); Mockito.doNothing().when(quotaManagerImplSpy).injectPresetVariablesIntoJsInterpreter(Mockito.any(), Mockito.any()); Mockito.doReturn(true).when(jsInterpreterMock).executeScript(Mockito.anyString()); - BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, jsInterpreterMock, presetVariablesMock); + BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, jsInterpreterMock, presetVariablesMock, lastTariffs); Assert.assertEquals(expected, result); } @Test public void getQuotaTariffValueToBeAppliedTestScriptResultIsFalseReturnZero() { + List lastTariffs = createLastAppliedTariffsPresetVariableList(0); + Mockito.doReturn(" ").when(quotaTariffVoMock).getActivationRule(); Mockito.doReturn(BigDecimal.TEN).when(quotaTariffVoMock).getCurrencyValue(); Mockito.doNothing().when(quotaManagerImplSpy).injectPresetVariablesIntoJsInterpreter(Mockito.any(), Mockito.any()); Mockito.doReturn(false).when(jsInterpreterMock).executeScript(Mockito.anyString()); - BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, jsInterpreterMock, presetVariablesMock); + BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, jsInterpreterMock, presetVariablesMock, lastTariffs); Assert.assertEquals(BigDecimal.ZERO, result); } @Test public void getQuotaTariffValueToBeAppliedTestScriptResultIsNotBooleanNorNumericReturnZero() { + List lastTariffs = createLastAppliedTariffsPresetVariableList(0); + Mockito.doReturn(" ").when(quotaTariffVoMock).getActivationRule(); Mockito.doReturn(BigDecimal.TEN).when(quotaTariffVoMock).getCurrencyValue(); Mockito.doNothing().when(quotaManagerImplSpy).injectPresetVariablesIntoJsInterpreter(Mockito.any(), Mockito.any()); Mockito.doReturn("test").when(jsInterpreterMock).executeScript(Mockito.anyString()); - BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, jsInterpreterMock, presetVariablesMock); + BigDecimal result = quotaManagerImplSpy.getQuotaTariffValueToBeApplied(quotaTariffVoMock, jsInterpreterMock, presetVariablesMock, lastTariffs); Assert.assertEquals(BigDecimal.ZERO, result); } @@ -477,10 +485,7 @@ public void getPresetVariablesTestHasTariffsWithActivationRuleReturnPresetVariab @Test public void aggregateQuotaTariffsValuesTestTariffsWereNotInPeriodToBeAppliedReturnZero() { - List tariffs = new ArrayList<>(); - tariffs.add(new QuotaTariffVO()); - tariffs.add(new QuotaTariffVO()); - tariffs.add(new QuotaTariffVO()); + List tariffs = createTariffList(); Mockito.doReturn(false).when(quotaManagerImplSpy).isQuotaTariffInPeriodToBeApplied(Mockito.any(), Mockito.any(), Mockito.anyString()); BigDecimal result = quotaManagerImplSpy.aggregateQuotaTariffsValues(usageVoMock, tariffs, false, jsInterpreterMock, ""); @@ -497,13 +502,10 @@ public void aggregateQuotaTariffsValuesTestTariffsIsEmptyReturnZero() { @Test public void aggregateQuotaTariffsValuesTestTariffsAreInPeriodToBeAppliedReturnAggregation() { - List tariffs = new ArrayList<>(); - tariffs.add(new QuotaTariffVO()); - tariffs.add(new QuotaTariffVO()); - tariffs.add(new QuotaTariffVO()); + List tariffs = createTariffList(); Mockito.doReturn(true, false, true).when(quotaManagerImplSpy).isQuotaTariffInPeriodToBeApplied(Mockito.any(), Mockito.any(), Mockito.anyString()); - Mockito.doReturn(BigDecimal.TEN).when(quotaManagerImplSpy).getQuotaTariffValueToBeApplied(Mockito.any(), Mockito.any(), Mockito.any()); + Mockito.doReturn(BigDecimal.TEN).when(quotaManagerImplSpy).getQuotaTariffValueToBeApplied(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()); BigDecimal result = quotaManagerImplSpy.aggregateQuotaTariffsValues(usageVoMock, tariffs, false, jsInterpreterMock, ""); Assert.assertEquals(BigDecimal.TEN.multiply(new BigDecimal(2)), result); @@ -528,4 +530,25 @@ public void persistUsagesAndQuotaUsagesAndRetrievePersistedQuotaUsagesTestReturn Assert.assertEquals(quotaUsageVoMock1, result.get(0)); Assert.assertEquals(quotaUsageVoMock2, result.get(1)); } + + private static List createTariffList() { + List tariffs = new ArrayList<>(); + tariffs.add(new QuotaTariffVO()); + tariffs.add(new QuotaTariffVO()); + tariffs.add(new QuotaTariffVO()); + tariffs.forEach(quotaTariffVO -> quotaTariffVO.setPosition(1)); + return tariffs; + } + + private static List createLastAppliedTariffsPresetVariableList(int numberOfTariffs) { + List lastTariffs = new ArrayList<>(); + for (int i = 0; i < numberOfTariffs; i++) { + Tariff tariff = new Tariff(); + tariff.setId(String.valueOf(i)); + tariff.setValue(BigDecimal.valueOf(i)); + lastTariffs.add(tariff); + } + return lastTariffs; + } + } diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffCreateCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffCreateCmd.java index 2bbdb57fa7e7..eaf5afc1478c 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffCreateCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffCreateCmd.java @@ -68,6 +68,9 @@ public class QuotaTariffCreateCmd extends BaseCmd { + " endDate=2009-06-03.") private Date endDate; + @Parameter(name = ApiConstants.POSITION, type = CommandType.INTEGER, description = "Position in the execution sequence for tariffs of the same type", since = "4.19.0.0") + private Integer position; + @Override public void execute() { QuotaTariffVO result = responseBuilder.createQuotaTariff(this); @@ -134,4 +137,13 @@ public void setEndDate(Date endDate) { this.endDate = endDate; } + public Integer getPosition() { + return position; + } + + public void setPosition(Integer position) { + this.position = position; + } + + } diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffUpdateCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffUpdateCmd.java index e2aad3a86f31..e2fdf06a5d92 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffUpdateCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffUpdateCmd.java @@ -69,6 +69,9 @@ public class QuotaTariffUpdateCmd extends BaseCmd { "value will be applied. Inform empty to remove the activation rule.", length = 65535, since = "4.18.0.0") private String activationRule; + @Parameter(name = ApiConstants.POSITION, type = CommandType.INTEGER, description = "Position in the execution sequence for tariffs of the same type", since = "4.19.0.0") + private Integer position; + public Integer getUsageType() { return usageType; } @@ -125,4 +128,12 @@ public long getEntityOwnerId() { return Account.ACCOUNT_ID_SYSTEM; } + public Integer getPosition() { + return position; + } + + public void setPosition(Integer position) { + this.position = position; + } + } diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java index 32b49a72ae41..b7d548e7dee7 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImpl.java @@ -61,6 +61,7 @@ import org.apache.cloudstack.quota.vo.QuotaTariffVO; import org.apache.cloudstack.quota.vo.QuotaUsageVO; import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils; +import org.apache.commons.lang3.ObjectUtils; import org.apache.log4j.Logger; import org.springframework.stereotype.Component; @@ -125,6 +126,7 @@ public QuotaTariffResponse createQuotaTariffResponse(QuotaTariffVO tariff) { response.setDescription(tariff.getDescription()); response.setUuid(tariff.getUuid()); response.setRemoved(tariff.getRemoved()); + response.setPosition(tariff.getPosition()); return response; } @@ -387,6 +389,7 @@ public QuotaTariffVO updateQuotaTariffPlan(QuotaTariffUpdateCmd cmd) { String description = cmd.getDescription(); String activationRule = cmd.getActivationRule(); Date now = _quotaService.computeAdjustedTime(new Date()); + Integer position = cmd.getPosition(); warnQuotaTariffUpdateDeprecatedFields(cmd); @@ -401,7 +404,7 @@ public QuotaTariffVO updateQuotaTariffPlan(QuotaTariffUpdateCmd cmd) { currentQuotaTariff.setRemoved(now); QuotaTariffVO newQuotaTariff = persistNewQuotaTariff(currentQuotaTariff, name, 0, currentQuotaTariffStartDate, cmd.getEntityOwnerId(), endDate, value, description, - activationRule); + activationRule, position); _quotaTariffDao.updateQuotaTariff(currentQuotaTariff); return newQuotaTariff; } @@ -419,7 +422,7 @@ protected void warnQuotaTariffUpdateDeprecatedFields(QuotaTariffUpdateCmd cmd) { } protected QuotaTariffVO persistNewQuotaTariff(QuotaTariffVO currentQuotaTariff, String name, int usageType, Date startDate, Long entityOwnerId, Date endDate, Double value, - String description, String activationRule) { + String description, String activationRule, Integer position) { QuotaTariffVO newQuotaTariff = getNewQuotaTariffObject(currentQuotaTariff, name, usageType); @@ -431,6 +434,7 @@ protected QuotaTariffVO persistNewQuotaTariff(QuotaTariffVO currentQuotaTariff, validateValueOnCreatingNewQuotaTariff(newQuotaTariff, value); validateStringsOnCreatingNewQuotaTariff(newQuotaTariff::setDescription, description); validateStringsOnCreatingNewQuotaTariff(newQuotaTariff::setActivationRule, activationRule); + validatePositionOnCreatingNewQuotaTariff(newQuotaTariff, position); _quotaTariffDao.addQuotaTariff(newQuotaTariff); return newQuotaTariff; @@ -451,6 +455,13 @@ protected QuotaTariffVO getNewQuotaTariffObject(QuotaTariffVO currentQuotaTariff return newQuotaTariff; } + protected void validatePositionOnCreatingNewQuotaTariff(QuotaTariffVO newQuotaTariff, Integer position) { + if (position != null) { + newQuotaTariff.setPosition(position); + } + } + + protected void validateStringsOnCreatingNewQuotaTariff(Consumer method, String value){ if (value != null) { method.accept(value.isBlank() ? null : value); @@ -629,6 +640,7 @@ public QuotaTariffVO createQuotaTariff(QuotaTariffCreateCmd cmd) { Double value = cmd.getValue(); String description = cmd.getDescription(); String activationRule = cmd.getActivationRule(); + Integer position = ObjectUtils.defaultIfNull(cmd.getPosition(), 1); QuotaTariffVO currentQuotaTariff = _quotaTariffDao.findByName(name); @@ -640,7 +652,7 @@ public QuotaTariffVO createQuotaTariff(QuotaTariffCreateCmd cmd) { throw new InvalidParameterValueException(String.format("The quota tariff's start date [%s] cannot be less than now [%s]", startDate, now)); } - return persistNewQuotaTariff(null, name, usageType, startDate, cmd.getEntityOwnerId(), endDate, value, description, activationRule); + return persistNewQuotaTariff(null, name, usageType, startDate, cmd.getEntityOwnerId(), endDate, value, description, activationRule, position); } public boolean deleteQuotaTariff(String quotaTariffUuid) { diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaTariffResponse.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaTariffResponse.java index ce4c5953641d..ce2d0d8f20d3 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaTariffResponse.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/response/QuotaTariffResponse.java @@ -82,6 +82,11 @@ public class QuotaTariffResponse extends BaseResponse { @Param(description = "when the quota tariff was removed") private Date removed; + @SerializedName("position") + @Param(description = "position in the execution sequence for tariffs of the same type") + private Integer position; + + public QuotaTariffResponse() { super(); this.setObjectName("quotatariff"); @@ -204,4 +209,12 @@ public void setRemoved(Date removed) { this.removed = removed; } + public Integer getPosition() { + return position; + } + + public void setPosition(Integer position) { + this.position = position; + } + } diff --git a/plugins/database/quota/src/test/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImplTest.java b/plugins/database/quota/src/test/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImplTest.java index b960a1be5692..b835b2a4ff1b 100644 --- a/plugins/database/quota/src/test/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImplTest.java +++ b/plugins/database/quota/src/test/java/org/apache/cloudstack/api/response/QuotaResponseBuilderImplTest.java @@ -352,8 +352,10 @@ public void persistNewQuotaTariffTestpersistNewQuotaTariff() { Mockito.doNothing().when(quotaResponseBuilderSpy).validateValueOnCreatingNewQuotaTariff(Mockito.any(QuotaTariffVO.class), Mockito.anyDouble()); Mockito.doNothing().when(quotaResponseBuilderSpy).validateStringsOnCreatingNewQuotaTariff(Mockito.any(Consumer.class), Mockito.anyString()); Mockito.doReturn(quotaTariffVoMock).when(quotaTariffDaoMock).addQuotaTariff(Mockito.any(QuotaTariffVO.class)); + Mockito.doNothing().when(quotaResponseBuilderSpy).validatePositionOnCreatingNewQuotaTariff(Mockito.any(QuotaTariffVO.class), Mockito.anyInt()); - quotaResponseBuilderSpy.persistNewQuotaTariff(quotaTariffVoMock, "", 1, date, 1l, date, 1.0, "", ""); + + quotaResponseBuilderSpy.persistNewQuotaTariff(quotaTariffVoMock, "", 1, date, 1l, date, 1.0, "", "", 2); Mockito.verify(quotaTariffDaoMock).addQuotaTariff(Mockito.any(QuotaTariffVO.class)); } @@ -403,4 +405,18 @@ public void getQuotaSummaryResponseTestAccountIsNotNullQuotaIsEnabledShouldRetur assertTrue(quotaSummaryResponse.getQuotaEnabled()); } + + @Test + public void validatePositionOnCreatingNewQuotaTariffTestNullValueDoNothing() { + quotaResponseBuilderSpy.validatePositionOnCreatingNewQuotaTariff(quotaTariffVoMock, null); + Mockito.verify(quotaTariffVoMock, Mockito.never()).setPosition(Mockito.any()); + } + + @Test + public void validatePositionOnCreatingNewQuotaTariffTestAnyValueIsSet() { + Integer position = 1; + quotaResponseBuilderSpy.validatePositionOnCreatingNewQuotaTariff(quotaTariffVoMock, position); + Mockito.verify(quotaTariffVoMock).setPosition(position); + } + } From 72f0cc2669db095a6bee3278b563f404c986fd95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Tue, 26 Dec 2023 10:49:06 -0300 Subject: [PATCH 2/6] Create integration test --- .../plugins/test_quota_tariff_order.py | 189 ++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 test/integration/plugins/test_quota_tariff_order.py diff --git a/test/integration/plugins/test_quota_tariff_order.py b/test/integration/plugins/test_quota_tariff_order.py new file mode 100644 index 000000000000..6ef0f0a64495 --- /dev/null +++ b/test/integration/plugins/test_quota_tariff_order.py @@ -0,0 +1,189 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +""" Test cases for checking quota API +""" + +# Import Local Modules +import tools.marvin.marvin +from tools.marvin.marvin.cloudstackTestCase import * +from tools.marvin.marvin.cloudstackAPI import * +from tools.marvin.marvin.lib.utils import * +from tools.marvin.marvin.lib.base import * +from tools.marvin.marvin.lib.common import * +from nose.plugins.attrib import attr + +# Import System modules +import time + + +class TestQuotaTariffOrder(cloudstackTestCase): + + @classmethod + def setUpClass(cls): + testClient = super(TestQuotaTariffOrder, cls).getClsTestClient() + cls.api_client = testClient.getApiClient() + cls.services = testClient.getParsedTestDataConfig() + + # Get Zone, Domain and templates + cls.domain = get_domain(cls.api_client) + cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests()) + + # Create Account + cls.account = Account.create( + cls.api_client, + cls.services["account"], + domainid=cls.domain.id + ) + cls._cleanup = [ + cls.account, + ] + + cls.services["account"] = cls.account.name + + return + + @classmethod + def tearDownClass(cls): + try: + # Cleanup resources used + cleanup_resources(cls.api_client, cls._cleanup) + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def setUp(self): + self.apiclient = self.testClient.getApiClient() + self.dbclient = self.testClient.getDbConnection() + self.cleanup = [] + self.tariffs = [] + return + + def tearDown(self): + try: + cleanup_resources(self.api_client, self.cleanup) + self.delete_tariffs() + except Exception as e: + raise Exception("Warning: Exception during cleanup : %s" % e) + return + + def delete_tariffs(self): + for tariff in self.tariffs: + cmd = quotaTariffDelete.quotaTariffDeleteCmd() + cmd.id = tariff.uuid + self.api_client.quotaTariffDelete(cmd) + + @attr( + tags=[ + "advanced", + "smoke"], + required_hardware="false") + def test_01_quota_tariff_order(self): + """Test Quota Tariff Order + """ + + cmd = quotaTariffCreate.quotaTariffCreateCmd() + cmd.name = 'tf1' + cmd.value = '1' + cmd.activationrule = '10' + cmd.usagetype = '22' + cmd.position = '2' + self.tariffs.append(self.api_client.quotaTariffCreate(cmd)) + + cmd = quotaTariffCreate.quotaTariffCreateCmd() + cmd.name = 'tf2' + cmd.value = '1' + cmd.activationrule = 'lastTariffs[lastTariffs.length -1].value + 7' + cmd.usagetype = '22' + cmd.position = '3' + self.tariffs.append(self.api_client.quotaTariffCreate(cmd)) + + cmd = quotaTariffCreate.quotaTariffCreateCmd() + cmd.name = 'tf3' + cmd.value = '1' + cmd.activationrule = 'lastTariffs[lastTariffs.length -2].value + lastTariffs[lastTariffs.length -1].value' + cmd.usagetype = '22' + cmd.position = '4' + self.tariffs.append(self.api_client.quotaTariffCreate(cmd)) + + cmd = quotaCredits.quotaCreditsCmd() + cmd.account = self.account.name + cmd.domainid = self.domain.id + cmd.value = 54 + self.api_client.quotaCredits(cmd) + + # Fetch account ID from account_uuid + self.debug("select id from account where uuid = '%s';" + % self.account.id) + + qresultset = self.dbclient.execute( + "select id from account where uuid = '%s';" + % self.account.id + ) + + account_id = qresultset[0][0] + + self.debug("SELECT id from `domain` d WHERE uuid = '%s';" + % self.domain.id) + + qresultset = self.dbclient.execute( + "SELECT id from `domain` d WHERE uuid = '%s';" + % self.domain.id + ) + + domain_id = qresultset[0][0] + + self.debug("SELECT id from data_center dc where dc.uuid = '%s';" + % self.zone.id) + + qresultset = self.dbclient.execute( + "SELECT id from data_center dc where dc.uuid = '%s';" + % self.zone.id + ) + + zone_id = qresultset[0][0] + + start = datetime.datetime.now() + datetime.timedelta(seconds=1) + end = datetime.datetime.now() + datetime.timedelta(hours=1) + + self.debug("INSERT INTO cloud_usage.cloud_usage (zone_id,account_id,domain_id,description,usage_display," + "usage_type,raw_usage,vm_instance_id,vm_name,offering_id,template_id,usage_id,`type`,`size`," + "network_id,start_date,end_date,virtual_size,cpu_speed,cpu_cores,memory,quota_calculated," + "is_hidden,state) VALUES ('{}','{}','{}','Test','1 Hrs',22,1,NULL,NULL,NULL,NULL,NULL," + "'VirtualMachine',NULL,NULL,'{}','{}',NULL,NULL,NULL,NULL,0,0,NULL);" + .format(zone_id, account_id, domain_id, start, end)) + + self.dbclient.execute( + "INSERT INTO cloud_usage.cloud_usage (zone_id,account_id,domain_id,description,usage_display,usage_type," + "raw_usage,vm_instance_id,vm_name,offering_id,template_id,usage_id,`type`,`size`,network_id," + "start_date,end_date,virtual_size,cpu_speed,cpu_cores,memory,quota_calculated,is_hidden," + "state) VALUES ('{}','{}','{}','Test','1 Hrs',22,1,NULL,NULL,NULL,NULL,NULL,'VirtualMachine'," + "NULL,NULL,'{}','{}',NULL,NULL,NULL,NULL,0,0,NULL);" + .format(zone_id, account_id, domain_id, start, end)) + + cmd = quotaUpdate.quotaUpdateCmd() + self.api_client.quotaUpdate(cmd) + + cmd = quotaBalance.quotaBalanceCmd() + cmd.domainid = self.account.domainid + cmd.account = self.account.name + response = self.apiclient.quotaBalance(cmd) + + self.debug(f"Quota Balance: {response.balance}") + + self.assertEqual(response.balance.startquota, 0, f"startQuota is supposed to be 0 but was {response.balance.startquota}") + + return From c388bea0b61595c576215d8bf4637fde0a38dd24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Mon, 5 Feb 2024 13:29:28 -0300 Subject: [PATCH 3/6] change sql file --- .../src/main/resources/META-INF/db/schema-41900to42000.sql | 3 +++ 1 file changed, 3 insertions(+) diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41900to42000.sql b/engine/schema/src/main/resources/META-INF/db/schema-41900to42000.sql index 1c368a2fbee2..465229e1f4ff 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41900to42000.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41900to42000.sql @@ -18,3 +18,6 @@ --; -- Schema upgrade from 4.19.0.0 to 4.20.0.0 --; + +-- Quota inject tariff result into subsequent ones +ALTER TABLE `cloud_usage`.`quota_tariff` ADD COLUMN `position` bigint(20) NOT NULL DEFAULT 1 COMMENT 'Position in the execution sequence for tariffs of the same type' ; From 779e79f4cfc39a5d3128f853e42a3b79d2782b1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Fri, 16 Feb 2024 10:34:12 -0300 Subject: [PATCH 4/6] Address reviews --- .../api/command/QuotaTariffCreateCmd.java | 2 +- .../api/command/QuotaTariffUpdateCmd.java | 2 +- .../plugins/test_quota_tariff_order.py | 20 ++++++++----------- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffCreateCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffCreateCmd.java index b67430669962..7fe5e7ac7c69 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffCreateCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffCreateCmd.java @@ -66,7 +66,7 @@ public class QuotaTariffCreateCmd extends BaseCmd { + " endDate=2009-06-03.") private Date endDate; - @Parameter(name = ApiConstants.POSITION, type = CommandType.INTEGER, description = "Position in the execution sequence for tariffs of the same type", since = "4.19.0.0") + @Parameter(name = ApiConstants.POSITION, type = CommandType.INTEGER, description = "Position in the execution sequence for tariffs of the same type", since = "4.20.0.0") private Integer position; @Override diff --git a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffUpdateCmd.java b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffUpdateCmd.java index af0270191ea8..1db1e0db8e9c 100644 --- a/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffUpdateCmd.java +++ b/plugins/database/quota/src/main/java/org/apache/cloudstack/api/command/QuotaTariffUpdateCmd.java @@ -67,7 +67,7 @@ public class QuotaTariffUpdateCmd extends BaseCmd { "value will be applied. Inform empty to remove the activation rule.", length = 65535, since = "4.18.0.0") private String activationRule; - @Parameter(name = ApiConstants.POSITION, type = CommandType.INTEGER, description = "Position in the execution sequence for tariffs of the same type", since = "4.19.0.0") + @Parameter(name = ApiConstants.POSITION, type = CommandType.INTEGER, description = "Position in the execution sequence for tariffs of the same type", since = "4.20.0.0") private Integer position; public Integer getUsageType() { diff --git a/test/integration/plugins/test_quota_tariff_order.py b/test/integration/plugins/test_quota_tariff_order.py index 6ef0f0a64495..33ae2c0d419d 100644 --- a/test/integration/plugins/test_quota_tariff_order.py +++ b/test/integration/plugins/test_quota_tariff_order.py @@ -159,20 +159,16 @@ def test_01_quota_tariff_order(self): start = datetime.datetime.now() + datetime.timedelta(seconds=1) end = datetime.datetime.now() + datetime.timedelta(hours=1) - self.debug("INSERT INTO cloud_usage.cloud_usage (zone_id,account_id,domain_id,description,usage_display," - "usage_type,raw_usage,vm_instance_id,vm_name,offering_id,template_id,usage_id,`type`,`size`," - "network_id,start_date,end_date,virtual_size,cpu_speed,cpu_cores,memory,quota_calculated," - "is_hidden,state) VALUES ('{}','{}','{}','Test','1 Hrs',22,1,NULL,NULL,NULL,NULL,NULL," - "'VirtualMachine',NULL,NULL,'{}','{}',NULL,NULL,NULL,NULL,0,0,NULL);" - .format(zone_id, account_id, domain_id, start, end)) + query = "INSERT INTO cloud_usage.cloud_usage (zone_id,account_id,domain_id,description,usage_display," + "usage_type,raw_usage,vm_instance_id,vm_name,offering_id,template_id,usage_id,`type`,`size`," + "network_id,start_date,end_date,virtual_size,cpu_speed,cpu_cores,memory,quota_calculated," + "is_hidden,state) VALUES ('{}','{}','{}','Test','1 Hrs',22,1,NULL,NULL,NULL,NULL,NULL," + "'VirtualMachine',NULL,NULL,'{}','{}',NULL,NULL,NULL,NULL,0,0,NULL);".format(zone_id, account_id, domain_id, start, end) + + self.debug(query) self.dbclient.execute( - "INSERT INTO cloud_usage.cloud_usage (zone_id,account_id,domain_id,description,usage_display,usage_type," - "raw_usage,vm_instance_id,vm_name,offering_id,template_id,usage_id,`type`,`size`,network_id," - "start_date,end_date,virtual_size,cpu_speed,cpu_cores,memory,quota_calculated,is_hidden," - "state) VALUES ('{}','{}','{}','Test','1 Hrs',22,1,NULL,NULL,NULL,NULL,NULL,'VirtualMachine'," - "NULL,NULL,'{}','{}',NULL,NULL,NULL,NULL,0,0,NULL);" - .format(zone_id, account_id, domain_id, start, end)) + query) cmd = quotaUpdate.quotaUpdateCmd() self.api_client.quotaUpdate(cmd) From 38b1f007f8adf3b983d22ff6f82d30e272e46304 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Wed, 20 Mar 2024 10:13:48 -0300 Subject: [PATCH 5/6] Address reviews --- .../plugins/test_quota_tariff_order.py | 20 +++++-------------- 1 file changed, 5 insertions(+), 15 deletions(-) diff --git a/test/integration/plugins/test_quota_tariff_order.py b/test/integration/plugins/test_quota_tariff_order.py index 33ae2c0d419d..3236c6c3c18f 100644 --- a/test/integration/plugins/test_quota_tariff_order.py +++ b/test/integration/plugins/test_quota_tariff_order.py @@ -42,15 +42,14 @@ def setUpClass(cls): cls.domain = get_domain(cls.api_client) cls.zone = get_zone(cls.api_client, cls.testClient.getZoneForTests()) + cls._cleanup = [] # Create Account cls.account = Account.create( cls.api_client, cls.services["account"], domainid=cls.domain.id ) - cls._cleanup = [ - cls.account, - ] + cls._cleanup.append(cls.account) cls.services["account"] = cls.account.name @@ -58,12 +57,7 @@ def setUpClass(cls): @classmethod def tearDownClass(cls): - try: - # Cleanup resources used - cleanup_resources(cls.api_client, cls._cleanup) - except Exception as e: - raise Exception("Warning: Exception during cleanup : %s" % e) - return + super(TestQuotaTariffOrder, cls).tearDownClass() def setUp(self): self.apiclient = self.testClient.getApiClient() @@ -73,12 +67,8 @@ def setUp(self): return def tearDown(self): - try: - cleanup_resources(self.api_client, self.cleanup) - self.delete_tariffs() - except Exception as e: - raise Exception("Warning: Exception during cleanup : %s" % e) - return + self.delete_tariffs() + super(TestQuotaTariffOrder, self).tearDown() def delete_tariffs(self): for tariff in self.tariffs: From ff7fa7c07e96434fc82f22443a58b65fdc12d504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Jandre?= <48719461+JoaoJandre@users.noreply.github.com> Date: Tue, 16 Apr 2024 14:17:55 -0300 Subject: [PATCH 6/6] use idempotent_add_column --- .../src/main/resources/META-INF/db/schema-41900to42000.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/schema/src/main/resources/META-INF/db/schema-41900to42000.sql b/engine/schema/src/main/resources/META-INF/db/schema-41900to42000.sql index 5d2099267ba5..639b48f20c5c 100644 --- a/engine/schema/src/main/resources/META-INF/db/schema-41900to42000.sql +++ b/engine/schema/src/main/resources/META-INF/db/schema-41900to42000.sql @@ -82,4 +82,4 @@ CREATE TABLE IF NOT EXISTS `cloud_usage`.`quota_email_configuration`( -- Quota inject tariff result into subsequent ones -ALTER TABLE `cloud_usage`.`quota_tariff` ADD COLUMN `position` bigint(20) NOT NULL DEFAULT 1 COMMENT 'Position in the execution sequence for tariffs of the same type' ; +CALL `cloud_usage`.`IDEMPOTENT_ADD_COLUMN`('cloud_usage.quota_tariff', 'position', 'bigint(20) NOT NULL DEFAULT 1 COMMENT "Position in the execution sequence for tariffs of the same type"');