Standardize StringUtils usages in NetworkModelImpl

[winterhazel]

Description

The CloudStack coding conventions specify org.apache.commons.lang3.StringUtils as the default alternative for string operations, and com.cloud.utils.StringUtils for operations not covered by the former.

Since org.apache.commons.lang3.StringUtils is seen as the default alternative and is used more often in NetworkModelImpl.java, this PR inverts how these two classes are referenced in this file in order to standardize it.

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

Feature/Enhancement Scale

• Major
• Minor

Screenshots (if appropriate):

How Has This Been Tested?

[DaanHoogland]

@winterhazel , I think the default should remain com.cloud.utils.StringUtils or be named org.apache.cloudstack.utils.StringUtils . We should maintain a proxy to all external libraries to reduce maintenance risk and ease migration.
org.apache.commons.lang3.StringUtils is not a default but just the most convenient one. We should not put all our egs in that basket.

[codecov]

Codecov Report

Merging #7980 (0bfe04d) into main (0375714) will increase coverage by 0.39%.
The diff coverage is 50.00%.

@@             Coverage Diff              @@
##               main    #7980      +/-   ##
============================================
+ Coverage     28.79%   29.18%   +0.39%     
- Complexity    29993    30421     +428     
============================================
  Files          5100     5100              
  Lines        358320   358320              
  Branches      52308    52308              
============================================
+ Hits         103165   104587    +1422     
+ Misses       240925   239362    -1563     
- Partials      14230    14371     +141     

Flag	Coverage Δ
simulator-marvin-tests	25.21% <5.55%> (+0.49%)	⬆️
uitests	4.87% <ø> (ø)
unit-tests	14.42% <50.00%> (ø)

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

Files Changed	Coverage Δ
.../main/java/com/cloud/network/NetworkModelImpl.java	48.82% <50.00%> (+0.95%)	⬆️

... and 121 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[shwstppr]

Code LGTM

[GutoVeronezi]

@winterhazel , I think the default should remain com.cloud.utils.StringUtils or be named org.apache.cloudstack.utils.StringUtils . We should maintain a proxy to all external libraries to reduce maintenance risk and ease migration. org.apache.commons.lang3.StringUtils is not a default but just the most convenient one. We should not put all our egs in that basket.

@DaanHoogland, by the discussion and vote threads we had in the dev mailing list (which led to the creation of session Libraries in our coding conventions), com.cloud.utils.StringUtils should only be used when org.apache.commons.lang3.StringUtils does not cover the operation.

Anyways, this PR is not changing the libraries that are being used, the changes were already made by PR #5386. 😄
This PR is just inverting the references in class NetworkModelImpl, as we have more calls to org.apache.commons.lang3.StringUtils than com.cloud.utils.StringUtils. This makes the code more readable, IMHO.

[DaanHoogland]

@GutoVeronezi I am not against making it standard. I am against not proxying it. In a codebase as big as this one we need to be aware of the maintenance costs of 3rd library updates. We should proxy all important libraries, even if only passing through. This is also why I think the log4j change is half done. It is a best practice for using 3rd party libraries that we only half implement in our project (i.e. hypervisor plugins) and should be more vigilent about. Strings are not the most complex 3rd party library but still...
anyway, not -1íng just warning for a breaking with best practices here.

[JoaoJandre]

CLGTM

[shwstppr]

@blueorangutan package

[blueorangutan]

@shwstppr 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 7429

[shwstppr]

@blueorangutan package

[blueorangutan]

@shwstppr 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 7431

[shwstppr]

@blueorangutan test

[blueorangutan]

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

[shwstppr]

@blueorangutan package

[blueorangutan]

@shwstppr 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 7577

[shwstppr]

@blueorangutan test

[blueorangutan]

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

[blueorangutan]

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

Test	Result	Time (s)	Test File
test_02_upgrade_kubernetes_cluster	Failure	607.60	test_kubernetes_clusters.py

[GutoVeronezi]

CLGTM