Change in ovirt-engine[master]: backend: make commands' constructors accessible from Command...

[masayag at redhat.com]

Moti Asayag has submitted this change and it was merged.

Change subject: backend: make commands' constructors accessible from CommandsFactory
......................................................................


backend: make commands' constructors accessible from CommandsFactory

Since CommandsFactory is the point where commands' constructors are
called, it should be able to access them.
Thus, each constructor's modifier of a command that is located:
- outside of CommandsFactory's package - should be public.
- in the same package as CommandsFactory - should not be private.

This patch adds a test to enforce this constraint, alongside changing
the modifiers of constructors that don't fulfill it.

Note that constructors which are used as a part of the compensation
process on startup (those that get the command's guid as a parameter)
are not exceptional, and should also fulfill this constraint.
Today, we call setAccessible(true) on these constructors before
accessing them. This concept is wrong for a few reasons:
1. It slows down the command's creation.
2. It's Java Security configuration dependant - a user/costumer can
   accidently break it.
3. It's not thread safe - today, the compensation process that runs on
   startup is serial. But if one day we would make it parallel,
   createCommand(className, commandId) will break.
   For example: thread A enters the "if" statement and sets the
   constructor's accessability to true. Then thread B doesn't enter the
   "if", thread A ends its excecution (after setting the constructor's
   accessability to false) and thread B throws NoSuchMethodException
   when trying to call getDeclaredConstructor.
4. There's no point in setting these constructors modifiers as
   protected, when we know they always should have public access.

Actually, there is no advantage in this approach, and thus this patch
removes the block of code that deals with the constructor's
accessability, counting on the test that it's already accessible.

Change-Id: Ic4598946f37f25b7883ee24719a99a75b3eb60b1
Signed-off-by: Idan Shaby <ishaby at redhat.com>
---
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandsFactory.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/aaa/RemoveGroupCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/aaa/RemoveUserCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdeploy/AddVdsCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdeploy/InstallVdsCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdeploy/UpdateVdsCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/hostdeploy/UpgradeOvirtNodeCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/AttachNetworkToClusterInternalCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/pm/RestartVdsCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/CreateAllSnapshotsFromVmCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/CreateSnapshotCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/RestoreAllSnapshotsCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/snapshots/TryBackToAllSnapshotsOfVmCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/FenceVdsManualyCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/AddDiskCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/image/AddImageFromScratchCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/image/RegisterDiskCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/ActivateStorageDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/AddExistingBlockStorageDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/AddExistingFileStorageDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/AddGlusterFsStorageDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/AddLocalStorageDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/AddNFSStorageDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/AddPosixFsStorageDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/AddSANStorageDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/AttachStorageDomainToPoolCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/DeactivateStorageDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/DeactivateStorageDomainWithOvfUpdateCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/DetachStorageDomainFromPoolCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/domain/ExtendSANStorageDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/export/ExportVmCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/export/ExportVmTemplateCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/export/MoveOrCopyTemplateCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/lsm/LiveMigrateVmDisksCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ovfstore/ProcessOvfUpdateForStorageDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/pool/AddStoragePoolWithStoragesCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/pool/ReconstructMasterDomainCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/pool/RecoveryStoragePoolCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/pool/RemoveStoragePoolCommand.java
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/pool/UpdateStoragePoolCommand.java
M backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/CommandCtorsTest.java
M backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ReflectionUtils.java
42 files changed, 159 insertions(+), 52 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Allon Mureinik: Looks good to me, but someone else must approve
  Moti Asayag: Looks good to me, approved
  Idan Shaby: Verified



-- 
To view, visit https://gerrit.ovirt.org/51605
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic4598946f37f25b7883ee24719a99a75b3eb60b1
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkaplan at redhat.com>
Gerrit-Reviewer: Arik Hadas <ahadas at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <laravot at redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipchuk at redhat.com>
Gerrit-Reviewer: Moti Asayag <masayag at redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourfali at redhat.com>
Gerrit-Reviewer: Roy Golan <rgolan at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>