Add access modifier to VolumeVO

[FelipeM525]

Description

The class VolumeVO lacks access modifiers in its fields. This PR aims to improve adherence to object-oriented programming by adding private access modifiers to all fields in the class mentioned above, along with their respective getters and setters. I've also added a getter to an occurrence in VolumeApiServiceImpl where a field belonging to VolumeVO was being accessed directly.

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

Feature/Enhancement Scale

• Major
• Minor

How Has This Been Tested?

I ran all the tests related to VolumeVO and VolumeApiServiceImpl, checked all usages of both classes, and made sure nothing was broken.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 15.81%. Comparing base (15e25bc) to head (67dbfe3).

Files with missing lines	Patch %	Lines
...hema/src/main/java/com/cloud/storage/VolumeVO.java	0.00%	2 Missing ⚠️
...n/java/com/cloud/storage/VolumeApiServiceImpl.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #9394      +/-   ##
============================================
- Coverage     15.81%   15.81%   -0.01%     
+ Complexity    12554    12553       -1     
============================================
  Files          5629     5629              
  Lines        492023   492023              
  Branches      62519    63929    +1410     
============================================
- Hits          77813    77812       -1     
  Misses       405887   405887              
- Partials       8323     8324       +1     

Flag	Coverage Δ
uitests	4.48% <ø> (ø)
unittests	16.60% <0.00%> (-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.

[DaanHoogland]

good code @FelipeM525, but is there a cause for this? I mean is there a plan to exploit this ?

[weizhouapache]

good code @FelipeM525, but is there a cause for this? I mean is there a plan to exploit this ?

I have same questions

[FelipeM525]

good code @FelipeM525, but is there a cause for this? I mean is there a plan to exploit this ?

Hello @DaanHoogland, Given the scope of Cloudstack and the number of people that work on this project, I believe it's important to emphasize keeping the code base clean by applying clean code principles and respecting the main concept of Java, which is OOP. Therefore, we should make sure classes have private access modifiers and that fields aren't accessed directly without getters so as to adhere to encapsulation.

[GaOrtiga]

CLGTM

[JoaoJandre]

CLGTM

[JoaoJandre]

@blueorangutan package

[blueorangutan]

@JoaoJandre a [SL] 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]

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10397

[weizhouapache]

@blueorangutan test

[blueorangutan]

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

[blueorangutan]

[SF] Trillian Build Failed (tid-10909)

[FelipeM525]

@weizhouapache could you run the test again?

[weizhouapache]

@blueorangutan test rocky8 kvm-rocky8

[blueorangutan]

@weizhouapache a [SL] Trillian-Jenkins test job (rocky8 mgmt + kvm-rocky8) has been kicked to run smoke tests

[blueorangutan]

[SF] Trillian test result (tid-11008)
Environment: kvm-rocky8 (x2), Advanced Networking with Mgmt server r8
Total time taken: 49272 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr9394-t11008-kvm-rocky8.zip
Smoke tests completed. 136 look OK, 1 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test	Result	Time (s)	Test File
test_06_purge_expunged_vm_background_task	Failure	332.10	test_purge_expunged_vms.py

[weizhouapache]

@FelipeM525

• hostip should be renamed to hostIp
• getHostip and setHostip are not needed. there are already getHostIp and setHostIp

[weizhouapache]

@blueorangutan package

[JoaoJandre]

@blueorangutan package

[blueorangutan]

@JoaoJandre a [SL] 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]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10673

[FelipeM525]

@weizhouapache could you run tests?

[DaanHoogland]

@blueorangutan LLtest

[blueorangutan]

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

[github-actions]

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

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@DaanHoogland a [SL] 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]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✖️ debian ✔️ suse15. SL-JID 11094

[DaanHoogland]

for the debian build I see

10:02:05 [ERROR] Failed to execute goal on project cloud-server: Could not resolve dependencies for project org.apache.cloudstack:cloud-server:jar:4.20.0.0-shapeblue11941: Could not transfer artifact org.slf4j:slf4j-api:jar:1.7.32 from/to central (https://repo.maven.apache.org/maven2): transfer failed for https://repo.maven.apache.org/maven2/org/slf4j/slf4j-api/1.7.32/slf4j-api-1.7.32.jar: Connection reset -> [Help 1]

I don't think we still have a slf4j dependency as it was removed and logging was consolidated to be all log4j2???

[weizhouapache]

for the debian build I see

10:02:05 [ERROR] Failed to execute goal on project cloud-server: Could not resolve dependencies for project org.apache.cloudstack:cloud-server:jar:4.20.0.0-shapeblue11941: Could not transfer artifact org.slf4j:slf4j-api:jar:1.7.32 from/to central (https://repo.maven.apache.org/maven2): transfer failed for https://repo.maven.apache.org/maven2/org/slf4j/slf4j-api/1.7.32/slf4j-api-1.7.32.jar: Connection reset -> [Help 1]

I don't think we still have a slf4j dependency as it was removed and logging was consolidated to be all log4j2???

maybe a intermittent failure

@blueorangutan package

[blueorangutan]

@weizhouapache a [SL] 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]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 11097