Adjusts/fixes in quota tariff APIs by GutoVeronezi · Pull Request #7146 · apache/cloudstack

GutoVeronezi · 2023-01-31T02:17:55Z

Description

PR #5909 was created during the release 4.17.0.0 and the parameter since in APIs was written as 4.17.0.0; however, #5909 was really introduced only in 4.18.0.0.

This PR intends to adjust the since in the introduced APIs and parameters and also:

improve the parameters documentation;
adjust parameters length, according to the field in the database;
rename parameter UUID of quotaTariffDelete to ID, to follow the pattern we use along other APIs;
cleanup some unused/unnecessary code in the Cmd classes;

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

How Has This Been Tested?

I retested the whole tariff CRUD, as follows:

Creating a tariff

(lab)  > quota tariffcreate name='test-aadjusts' usagetype=12 value=0 description="a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose"
{
  "quotatariff": {
    "currency": "$",
    "description": "a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose",
    "effectiveDate": "2023-01-31T01:50:38+0000",
    "name": "test-adjusts",
    "tariffValue": 0,
    "usageDiscriminator": "None",
    "usageName": "PORT_FORWARDING_RULE",
    "usageType": 12,
    "usageTypeDescription": "Port Forwarding Usage",
    "usageUnit": "Policy*Month",
    "uuid": "c30cacf7-a378-4e43-8ec0-6c5ad2d0d9e9"
  }
}

Listing the tariffs after creating

(lab)  > quota tarifflist name=test-adjusts listall=true
{
  "count": 1,
  "quotatariff": [
    {
      "currency": "$",
      "description": "a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose",
      "effectiveDate": "2023-01-31T01:50:38+0000",
      "name": "test-adjusts",
      "tariffValue": 0,
      "usageDiscriminator": "None",
      "usageName": "PORT_FORWARDING_RULE",
      "usageType": 12,
      "usageTypeDescription": "Port Forwarding Usage",
      "usageUnit": "Policy*Month",
      "uuid": "c30cacf7-a378-4e43-8ec0-6c5ad2d0d9e9"
    }
  ]
}

Updating the tariff

(lab)  > quota tariffupdate name=test-adjusts activationrule="a rule" description="a simple description"
{
  "quotatariff": {
    "activationRule": "a rule",
    "currency": "$",
    "description": "a simple description",
    "effectiveDate": "2023-01-31T01:50:38+0000",
    "name": "test-adjusts",
    "tariffValue": 0,
    "usageDiscriminator": "None",
    "usageName": "PORT_FORWARDING_RULE",
    "usageType": 12,
    "usageTypeDescription": "Port Forwarding Usage",
    "usageUnit": "Policy*Month",
    "uuid": "2d9afee9-960d-4b32-9a68-6a4939e59b6b"
  }
}

Listing the tariffs after updating

(lab)  > quota tarifflist name=test-adjusts listall=true
{
  "count": 2,
  "quotatariff": [
    {
      "currency": "$",
      "description": "a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose",
      "effectiveDate": "2023-01-31T01:50:38+0000",
      "name": "test-adjusts",
      "removed": "2023-01-31T01:51:58+0000",
      "tariffValue": 0,
      "usageDiscriminator": "None",
      "usageName": "PORT_FORWARDING_RULE",
      "usageType": 12,
      "usageTypeDescription": "Port Forwarding Usage",
      "usageUnit": "Policy*Month",
      "uuid": "c30cacf7-a378-4e43-8ec0-6c5ad2d0d9e9"
    },
    {
      "activationRule": "a rule",
      "currency": "$",
      "description": "a simple description",
      "effectiveDate": "2023-01-31T01:50:38+0000",
      "name": "test-adjusts",
      "tariffValue": 0,
      "usageDiscriminator": "None",
      "usageName": "PORT_FORWARDING_RULE",
      "usageType": 12,
      "usageTypeDescription": "Port Forwarding Usage",
      "usageUnit": "Policy*Month",
      "uuid": "2d9afee9-960d-4b32-9a68-6a4939e59b6b"
    }
  ]
}

Removing the tariff

(lab)  > quota tariffdelete id=2d9afee9-960d-4b32-9a68-6a4939e59b6b
{
  "success": true
}

Listing the tariffs after removing

(lab)  > quota tarifflist name=test-adjusts listall=true
{
  "count": 2,
  "quotatariff": [
    {
      "currency": "$",
      "description": "a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose | a long description that will describe the tariff's purpose",
      "effectiveDate": "2023-01-31T01:50:38+0000",
      "name": "test-adjusts",
      "removed": "2023-01-31T01:51:58+0000",
      "tariffValue": 0,
      "usageDiscriminator": "None",
      "usageName": "PORT_FORWARDING_RULE",
      "usageType": 12,
      "usageTypeDescription": "Port Forwarding Usage",
      "usageUnit": "Policy*Month",
      "uuid": "c30cacf7-a378-4e43-8ec0-6c5ad2d0d9e9"
    },
    {
      "activationRule": "a rule",
      "currency": "$",
      "description": "a simple description",
      "effectiveDate": "2023-01-31T01:50:38+0000",
      "name": "test-adjusts",
      "removed": "2023-01-31T01:52:34+0000",
      "tariffValue": 0,
      "usageDiscriminator": "None",
      "usageName": "PORT_FORWARDING_RULE",
      "usageType": 12,
      "usageTypeDescription": "Port Forwarding Usage",
      "usageUnit": "Policy*Month",
      "uuid": "2d9afee9-960d-4b32-9a68-6a4939e59b6b"
    }
  ]
}

GutoVeronezi · 2023-01-31T02:20:18Z

@DaanHoogland, can we add these adjusts/fixes to 4.18.0.0?

@blueorangutan package

blueorangutan · 2023-01-31T02:21:03Z

@GutoVeronezi a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2023-01-31T03:15:31Z

Packaging result: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 5448

sonarqubecloud · 2023-01-31T03:19:37Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

33.3% Coverage
0.0% Duplication

DaanHoogland · 2023-01-31T08:14:01Z

@blueorangutan test

blueorangutan · 2023-01-31T08:15:03Z

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

DaanHoogland

That was surprisingly hard to review for such a simple fix, but CLGTM ;)

DaanHoogland · 2023-01-31T08:25:08Z

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

    QuotaResponseBuilder _responseBuilder;

-    @Parameter(name = ApiConstants.USAGE_TYPE, type = CommandType.INTEGER, required = false, description = "Usage type of the resource")
+    @Parameter(name = ApiConstants.USAGE_TYPE, type = CommandType.INTEGER, description = "Usage type of the resource")


these have note become required, but the default is just removed !!?

The default value of required is false; therefore, there is no need to inform it as false:

cloudstack/api/src/main/java/org/apache/cloudstack/api/Parameter.java

Line 35 in d8c7e34

boolean required() default false;

GutoVeronezi · 2023-01-31T11:29:21Z

Thanks, @DaanHoogland

blueorangutan · 2023-01-31T20:12:46Z

Trillian test result (tid-6038)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41626 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr7146-t6038-kvm-centos7.zip
Smoke tests completed. 107 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File

nvazquez

Code LGTM

GutoVeronezi added 5 commits January 30, 2023 09:39

Fix since

d249b4f

Fix parameters' length and documentation

9380fdb

Rename parameter UUID to ID in quotaTariffDelete

f62dd1d

Clean-up

502c846

Undo tab

7024318

boring-cyborg bot added the component:quota label Jan 31, 2023

GutoVeronezi requested a review from DaanHoogland January 31, 2023 02:18

GutoVeronezi changed the title ~~Adjusts in quota tariff APIs~~ Adjusts/fixes in quota tariff APIs Jan 31, 2023

DaanHoogland approved these changes Jan 31, 2023

View reviewed changes

DaanHoogland added this to the 4.18.0.0 milestone Jan 31, 2023

nvazquez approved these changes Feb 1, 2023

View reviewed changes

DaanHoogland merged commit 9c4b3a6 into apache:main Feb 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adjusts/fixes in quota tariff APIs#7146

Adjusts/fixes in quota tariff APIs#7146
DaanHoogland merged 5 commits intoapache:mainfrom
GutoVeronezi:cleanup-and-adjusts-in-quota-tariff-apis

GutoVeronezi commented Jan 31, 2023

Uh oh!

GutoVeronezi commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

sonarqubecloud bot commented Jan 31, 2023 •

edited

Loading

Uh oh!

DaanHoogland commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

DaanHoogland left a comment

Uh oh!

DaanHoogland Jan 31, 2023

Uh oh!

GutoVeronezi Jan 31, 2023

Uh oh!

GutoVeronezi commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

nvazquez left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

GutoVeronezi commented Jan 31, 2023

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

How Has This Been Tested?

Uh oh!

GutoVeronezi commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

sonarqubecloud bot commented Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DaanHoogland commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland Jan 31, 2023

Choose a reason for hiding this comment

Uh oh!

GutoVeronezi Jan 31, 2023

Choose a reason for hiding this comment

Uh oh!

GutoVeronezi commented Jan 31, 2023

Uh oh!

blueorangutan commented Jan 31, 2023

Uh oh!

nvazquez left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sonarqubecloud bot commented Jan 31, 2023 •

edited

Loading