Create new generic method for resource UUID obtention in event's descriptions

[erikbocks]

Description

Currently, API parameters that have UUID as their type either accept the resource's UUID, or their internal ID directly. If the resource UUID was provided, a workflow responsible for processing the parameters of the APIs automatically translates it to the resource's internal ID before it reaches the API level. However, some APIs events descriptions are created after the translation process. If the command wishes to present which resource is being interacted with, the translation process causes API events descriptions to expose the resource's internal ID to the final user.

As a workaround, it is possible to use the UuidMgr.getUuid(Class<T> entityType, Long customId) method to get the resource's UUID. Even though this method can be used, it is very verbose and makes an additional call to the database. Thus, in order to solve this issue, the getResourceUuid(String parameterName) method was created.

This method belongs to the BaseCmd class, allowing all current and future API classes to inherit it. To store the resources UUIDs, the apiResourcesUuids HashMap was added to the CallContext class. This map is populated during the parameter processing, that occurs at the ParamProcessWorker#translateUuidToInternalId(final String uuid, final Parameter annotation) method, leveraging the UUID translation process to eliminate the additional database call. For each UUID type parameter sent, the map is populated with the parameter's name as the map key, and the resource's UUID as its value. If the provided key is not found in the map, or the value found is not a UUID, the method returns a null value.

This PR also refactors events descriptions, aiming to enhance them to be more descriptive or only to implement the new getResourceUuid() method. The method calling was standardized by using the ApiConstants attributes as the parameterName value.

Fixes: #11859

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)
• Build/CI
• Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Screenshots (if appropriate):

How Has This Been Tested?

After applying the updated packages to my local environment, I called the disable account API, informing the id parameter as the account's UUID, and validated that the created event contained the resource UUID instead of the database ID. The same process was repeated informing the resource's database ID as the parameter value, and the correct behavior was observed. By using a debugger, I also validated that the parameter processing flow as executing the correct validations and storing the values correctly in the HashMap.

The codebase tests were also successfully executed.

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[bernardodemarco]

@erikbocks, it seems that there are some checkstyle errors:

Error:  Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.1.0:check (cloudstack-checkstyle) on project cloud-api: Failed during checkstyle execution: There is 1 error reported by Checkstyle 8.18 with cloud-style.xml ruleset. -> [Help 1]

[codecov]

Codecov Report

❌ Patch coverage is 9.34579% with 291 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.85%. Comparing base (420bf6d) to head (3b14b87).
⚠️ Report is 10 commits behind head on main.

Files with missing lines	Patch %	Lines
...ack/api/command/user/template/CopyTemplateCmd.java	0.00%	8 Missing ⚠️
...stack/api/command/user/volume/CreateVolumeCmd.java	0.00%	6 Missing ⚠️
...stack/api/command/user/volume/DetachVolumeCmd.java	0.00%	6 Missing ⚠️
...ava/com/cloud/api/dispatch/ParamProcessWorker.java	0.00%	6 Missing ⚠️
...k/api/command/admin/account/DisableAccountCmd.java	0.00%	5 Missing ⚠️
.../cloudstack/api/command/admin/vm/MigrateVMCmd.java	0.00%	4 Missing ⚠️
...cloudstack/api/command/user/iso/ExtractIsoCmd.java	0.00%	4 Missing ⚠️
...command/user/network/ReplaceNetworkACLListCmd.java	0.00%	4 Missing ⚠️
.../api/command/user/template/ExtractTemplateCmd.java	0.00%	4 Missing ⚠️
...he/cloudstack/api/commands/DedicateClusterCmd.java	0.00%	4 Missing ⚠️
... and 207 more

Additional details and impacted files

@@            Coverage Diff            @@
##               main   #12502   +/-   ##
=========================================
  Coverage     17.85%   17.85%           
- Complexity    15993    15995    +2     
=========================================
  Files          5929     5929           
  Lines        531155   531176   +21     
  Branches      64921    64929    +8     
=========================================
+ Hits          94830    94839    +9     
- Misses       425709   425720   +11     
- Partials      10616    10617    +1     

Flag	Coverage Δ
uitests	3.58% <ø> (ø)
unittests	18.95% <9.34%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[DaanHoogland]

Thanks for the hard work and description. It seems like a good solution. I will try to get through all the small changes.

[DaanHoogland]

small remarks, clgtm generally, needs a lot of trivial testing.

[erikbocks]

Hello, @DaanHoogland and @sureshanaparti

Firstly, thank you for the reviews and for the suggestions. Regarding some the questions brought by @sureshanaparti:

What event description is logged for create/add calls (UUID might not exist)?

This behavior will depend on how the API handles its events' creation. The majority of APIs uses the ActionEvent annotation, which stores information as the description and the event type. This information is gathered by some intercept... methods present at the ActionEventInterception class. These methods gather the information defined at the method's annotation, and publish the event. Some APIs, as the asynchronous ones, have some intermediate events informing whether the operation was Scheduled or Started. The description of the Scheduled events are based on the getEventDescription() method, which is the main focus of this PR. Events with other states, as Completed and Created ones, use the CallContext.eventDetails attribute. This attribute is set during the API processing, and is used together with the ActionEvent description.

The majority of APIs for creation or addition of resources does not have any events informing the resource's UUID; thus, the change in this PR does not affect them. However, there are some APIs that have an event description that display a UUID. These APIs are the ones that inherit from the BaseAsyncCreateCmd class, and thus, have the create() method, which is a method that is executed prior to the BaseCmd's execute(). This means that, when the API processing flow reaches the event's description obtention step, the resource was already created.

can add to Map here, instead of querying db again.

The reason for not inserting the UUID obtention logic there is because these lines are only executed when the parameter has the @ACL annotation. In order to address both parameters that has these annotation, as the ones that does not, the UUID obtention logic was added to the translateUuidToInternalId() method. This method is executed inside the setFieldValue() method, which is executed at line 210, prior to the referenced snippets (lines 264 and 287). In addition to that, is important to note that the second snippet refers to the processing of a UUID list parameter. As changing the processing of List type parameters would require changing the current logic, in order to allow the storing of the resources' UUIDs, I choose not to address these cases in this PR.

---

@DaanHoogland, after analysing @sureshanaparti's reviews, I noticed that some events details set in the CallContext class were not being addressed. Therefore, I went through these occurrences and made changes to them, in order to use the new UUID obtention method. Could you please review the files again?