Refactor SnapshotDataStoreDaoImpl by GutoVeronezi · Pull Request #6751 · apache/cloudstack

GutoVeronezi · 2022-09-19T21:23:09Z

Description

PR #5909 is moving class SnapshotDataStoreDaoImpl to another package. Sonarcloud identified it as changes in every line of the file, reporting several code smells. As for the PR context it is only necessary to move the class to another package, I created this PR to address the code smells and also refactor some methods of the class.

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

How Has This Been Tested?

I created unit tests for the methods.

codecov · 2022-09-19T23:06:03Z

Codecov Report

Merging #6751 (e9a76a8) into main (bbc1260) will increase coverage by 0.10%.
The diff coverage is 5.03%.

@@             Coverage Diff              @@
##               main    #6751      +/-   ##
============================================
+ Coverage     10.42%   10.52%   +0.10%     
- Complexity     6701     6787      +86     
============================================
  Files          2458     2464       +6     
  Lines        243246   244089     +843     
  Branches      38067    38203     +136     
============================================
+ Hits          25358    25701     +343     
- Misses       214714   215154     +440     
- Partials       3174     3234      +60

Impacted Files	Coverage Δ
...ack/storage/image/db/SnapshotDataStoreDaoImpl.java	`5.75% <5.03%> (+1.48%)`	⬆️
...rce/wrapper/LibvirtResizeVolumeCommandWrapper.java	`49.50% <0.00%> (-27.17%)`	⬇️
.../cloud/hypervisor/kvm/storage/KVMPhysicalDisk.java	`70.27% <0.00%> (-10.98%)`	⬇️
...pper/LibvirtPrepareForMigrationCommandWrapper.java	`43.10% <0.00%> (-10.09%)`	⬇️
...loud/hypervisor/kvm/resource/LibvirtSecretDef.java	`60.00% <0.00%> (-3.16%)`	⬇️
.../hypervisor/kvm/storage/ScaleIOStorageAdaptor.java	`10.48% <0.00%> (-2.63%)`	⬇️
...apache/cloudstack/storage/volume/VolumeObject.java	`35.75% <0.00%> (-2.61%)`	⬇️
...vm/resource/wrapper/LibvirtStopCommandWrapper.java	`42.66% <0.00%> (-1.78%)`	⬇️
.../main/java/com/cloud/vm/dao/VMInstanceDaoImpl.java	`27.34% <0.00%> (-0.74%)`	⬇️
...torage/allocator/AbstractStoragePoolAllocator.java	`9.03% <0.00%> (-0.66%)`	⬇️
... and 73 more

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

GutoVeronezi · 2022-09-20T11:58:17Z

@blueorangutan package

blueorangutan · 2022-09-20T11:59: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 · 2022-09-20T12:53:26Z

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

DaanHoogland · 2022-09-20T14:00:50Z

@blueorangutan test

blueorangutan · 2022-09-20T14:02:05Z

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

blueorangutan · 2022-09-20T14:18:41Z

Trillian Build Failed (tid-4936)

blueorangutan · 2022-09-20T14:40:26Z

Trillian Build Failed (tid-4937)

Co-authored-by: dahn <daan.hoogland@gmail.com>

sonarqubecloud · 2022-09-20T17:53:43Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

6.7% Coverage
0.0% Duplication

DaanHoogland · 2022-09-21T09:35:00Z

@blueorangutan package

blueorangutan · 2022-09-21T09:36:03Z

@DaanHoogland 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 · 2022-09-21T10:27:53Z

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

DaanHoogland · 2022-09-21T10:35:05Z

@blueorangutan test matrix

blueorangutan · 2022-09-21T10:36:03Z

@DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

blueorangutan · 2022-09-21T10:53:27Z

Trillian Build Failed (tid-4950)

blueorangutan · 2022-09-21T10:53:56Z

Trillian Build Failed (tid-4951)

blueorangutan · 2022-09-21T10:54:43Z

Trillian Build Failed (tid-4952)

acs-robot · 2022-09-30T23:27:54Z

Found UI changes, kicking a new UI QA build
@blueorangutan ui

blueorangutan · 2022-09-30T23:29:03Z

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

acs-robot · 2022-09-30T23:30:20Z

Found UI changes, kicking a new UI QA build
@blueorangutan ui

blueorangutan · 2022-09-30T23:31:02Z

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

acs-robot · 2022-09-30T23:32:01Z

Found UI changes, kicking a new UI QA build
@blueorangutan ui

blueorangutan · 2022-09-30T23:33:03Z

@acs-robot a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

blueorangutan · 2022-09-30T23:34:46Z

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6751 (SL-JID-2451)

blueorangutan · 2022-09-30T23:36:30Z

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6751 (SL-JID-2452)

blueorangutan · 2022-09-30T23:38:15Z

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/6751 (SL-JID-2453)

DaanHoogland · 2022-10-02T15:05:38Z

@blueorangutan package

blueorangutan · 2022-10-02T15:06:02Z

@DaanHoogland 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 · 2022-10-02T15:55:08Z

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 4331

DaanHoogland · 2022-10-03T12:34:58Z

@blueorangutan package

blueorangutan · 2022-10-03T12:36:03Z

@DaanHoogland 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 · 2022-10-03T14:04:51Z

Packaging result: ✖️ el7 ✖️ el8 ✖️ debian ✖️ suse15. SL-JID 4343

GutoVeronezi · 2022-10-03T16:03:15Z

@blueorangutan package

blueorangutan · 2022-10-03T16:04: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 · 2022-10-03T16:57:48Z

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

RodrigoDLopez

clgtm

DaanHoogland · 2022-10-07T21:53:03Z

@blueorangutan test

blueorangutan · 2022-10-07T21:54:02Z

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

blueorangutan · 2022-10-08T11:35:22Z

Trillian test result (tid-5081)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 47902 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6751-t5081-kvm-centos7.zip
Smoke tests completed. 102 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`	482.17	test_kubernetes_clusters.py
test_08_upgrade_kubernetes_ha_cluster	`Failure`	804.75	test_kubernetes_clusters.py

DaanHoogland · 2022-10-11T11:51:07Z

+    private SearchBuilder<SnapshotDataStoreVO> searchFilteringStoreIdEqStoreRoleEqStateNeqRefCntNeq;
+    protected SearchBuilder<SnapshotDataStoreVO> searchFilteringStoreIdEqStateEqStoreRoleEqIdEqUpdateCountEqSnapshotIdEqVolumeIdEq;


impresive names ;)

DaanHoogland

clgtm

Refactor SnapshotDataStoreDaoImpl and add unit tests

a171984

boring-cyborg Bot added component:database component:storage labels Sep 19, 2022

GutoVeronezi mentioned this pull request Sep 19, 2022

Quota custom tariffs #5909

Merged

7 tasks

Create constants for duplicated literals

2ff4792

DaanHoogland reviewed Sep 20, 2022

View reviewed changes

Comment thread framework/db/src/test/java/com/cloud/utils/db/GenericSearchBuilderTest.java Outdated

GutoVeronezi and others added 2 commits September 20, 2022 12:57

Add missing license to file

ec1c413

Co-authored-by: dahn <daan.hoogland@gmail.com>

Remove duplicated license header

acec300

GutoVeronezi added 2 commits September 30, 2022 20:06

Undo unnecessary changes

e285a77

Refactor search builders

626addb

GutoVeronezi force-pushed the resolve-code-smells-in-SnapshotDataStoreDaoImpl branch from bc9cf00 to 7972f3f Compare September 30, 2022 23:31

DaanHoogland reviewed Oct 3, 2022

View reviewed changes

Comment thread ...e/storage/src/main/java/org/apache/cloudstack/storage/image/db/SnapshotDataStoreDaoImpl.java Outdated

Add missing comma

be63780

GutoVeronezi marked this pull request as draft October 3, 2022 14:24

Fixes

e9a76a8

GutoVeronezi marked this pull request as ready for review October 3, 2022 16:03

GutoVeronezi requested a review from DaanHoogland October 5, 2022 16:15

RodrigoDLopez approved these changes Oct 6, 2022

View reviewed changes

DaanHoogland reviewed Oct 11, 2022

View reviewed changes

DaanHoogland approved these changes Oct 11, 2022

View reviewed changes

DaanHoogland merged commit f7b2985 into apache:main Oct 11, 2022

DaanHoogland added this to the 4.18.0.0 milestone Oct 11, 2022

		private SearchBuilder<SnapshotDataStoreVO> searchFilteringStoreIdEqStoreRoleEqStateNeqRefCntNeq;
		protected SearchBuilder<SnapshotDataStoreVO> searchFilteringStoreIdEqStateEqStoreRoleEqIdEqUpdateCountEqSnapshotIdEqVolumeIdEq;

Conversation

GutoVeronezi commented Sep 19, 2022 • edited by DaanHoogland Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

How Has This Been Tested?

Uh oh!

codecov Bot commented Sep 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

GutoVeronezi commented Sep 20, 2022

Uh oh!

blueorangutan commented Sep 20, 2022

Uh oh!

blueorangutan commented Sep 20, 2022

Uh oh!

Uh oh!

DaanHoogland commented Sep 20, 2022

Uh oh!

blueorangutan commented Sep 20, 2022

Uh oh!

blueorangutan commented Sep 20, 2022

Uh oh!

blueorangutan commented Sep 20, 2022

Uh oh!

sonarqubecloud Bot commented Sep 20, 2022

Uh oh!

DaanHoogland commented Sep 21, 2022

Uh oh!

blueorangutan commented Sep 21, 2022

Uh oh!

blueorangutan commented Sep 21, 2022

Uh oh!

DaanHoogland commented Sep 21, 2022

Uh oh!

blueorangutan commented Sep 21, 2022

Uh oh!

blueorangutan commented Sep 21, 2022

Uh oh!

blueorangutan commented Sep 21, 2022

Uh oh!

blueorangutan commented Sep 21, 2022

Uh oh!

acs-robot commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

acs-robot commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

acs-robot commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

blueorangutan commented Sep 30, 2022

Uh oh!

DaanHoogland commented Oct 2, 2022

Uh oh!

blueorangutan commented Oct 2, 2022

Uh oh!

blueorangutan commented Oct 2, 2022

Uh oh!

Uh oh!

DaanHoogland commented Oct 3, 2022

Uh oh!

blueorangutan commented Oct 3, 2022

Uh oh!

blueorangutan commented Oct 3, 2022

Uh oh!

GutoVeronezi commented Oct 3, 2022

Uh oh!

blueorangutan commented Oct 3, 2022

GutoVeronezi commented Sep 19, 2022 •

edited by DaanHoogland

Loading

codecov Bot commented Sep 19, 2022 •

edited

Loading