core: Report names rather than ids in affinity conflicts

When an affinity conflict occurs, the hosts or VMs participating in
the conflict are reported by their ids.  This is accurate but not very
user friendly, especially in the web UI dialog.  Let’s report the
conflicting entities by their names instead.

We must retrieve the names from the corresponding DAOs.  In theory,
AffinityGroup’s should contain entity names corresponding to the ids,
but this doesn’t hold and the names are often missing there.

Due to the DAO access, AffinityValidator.checkAffinityGroupConflicts
can no longer be static.  And because AffinityValidator is a
singleton, it cannot be used in tests as it is (AFAICT).  That means
we have to mock AffinityValidator.checkAffinityGroupConflicts in some
tests and give up on testing that method there.

Bug-Url: https://bugzilla.redhat.com/1991622

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/scheduling/commands/AffinityGroupCRUDCommand.java

@@ -47,6 +47,8 @@ public abstract class AffinityGroupCRUDCommand <T extends AffinityGroupCRUDParam
     private AffinityGroupDao affinityGroupDao;
     @Inject
     private LabelDao labelDao;
+    @Inject
+    private AffinityValidator affinityValidator;
 
     AffinityGroup affinityGroup = null;
 
@@ -179,7 +181,7 @@ private boolean affinityGroupsWithoutConflict(AffinityGroup affinityGroup) {
         affinityGroups.removeIf(g -> g.getId().equals(affinityGroupCopy.getId()));
         affinityGroups.add(affinityGroupCopy);
 
-        AffinityValidator.Result result = AffinityValidator.checkAffinityGroupConflicts(affinityGroups);
+        AffinityValidator.Result result = affinityValidator.checkAffinityGroupConflicts(affinityGroups);
         if (result.getValidationResult().isValid()) {
             result.getLoggingMethod().accept(this, auditLogDirector);
         }

backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/AffinityValidator.java

@@ -20,13 +20,17 @@
 import org.ovirt.engine.core.bll.scheduling.arem.AffinityRulesUtils.AffinityGroupConflicts;
 import org.ovirt.engine.core.common.businessentities.BusinessEntity;
 import org.ovirt.engine.core.common.businessentities.Label;
+import org.ovirt.engine.core.common.businessentities.VdsStatic;
+import org.ovirt.engine.core.common.businessentities.VmStatic;
 import org.ovirt.engine.core.common.errors.EngineMessage;
 import org.ovirt.engine.core.common.scheduling.AffinityGroup;
 import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector;
 import org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogable;
 import org.ovirt.engine.core.dao.LabelDao;
+import org.ovirt.engine.core.dao.VdsStaticDao;
+import org.ovirt.engine.core.dao.VmStaticDao;
 import org.ovirt.engine.core.dao.scheduling.AffinityGroupDao;
 
 @Singleton
@@ -64,6 +68,10 @@ public static Result newFailed(ValidationResult validationResult) {
     private AffinityGroupDao affinityGroupDao;
     @Inject
     private LabelDao labelDao;
+    @Inject
+    private VdsStaticDao vdsStaticDao;
+    @Inject
+    private VmStaticDao vmStaticDao;
 
     public Result validateAffinityUpdateForVm(Guid clusterId, Guid vmId, Collection<AffinityGroup> affinityGroups, Collection<Label> labels) {
         return validateAffinityUpdate(clusterId, vmId, affinityGroups, labels, AffinityGroup::getVmIds, Label::getVms);
@@ -193,7 +201,7 @@ public static Set<Guid> unpackAffinityGroupHostsFromLabels(AffinityGroup group,
         return hostIds;
     }
 
-    public static Result checkAffinityGroupConflicts(List<AffinityGroup> groups) {
+    public Result checkAffinityGroupConflicts(List<AffinityGroup> groups) {
         if (groups.isEmpty()) {
             return Result.VALID;
         }
@@ -206,12 +214,12 @@ public static Result checkAffinityGroupConflicts(List<AffinityGroup> groups) {
                     .map(AffinityGroup::getName)
                     .collect(Collectors.joining(","));
 
-            String hosts = conflict.getHosts().stream()
-                    .map(Guid::toString)
+            String hosts = vdsStaticDao.getByIds(new ArrayList<>(conflict.getHosts())).stream()
+                    .map(VdsStatic::getName)
                     .collect(Collectors.joining(","));
 
-            String vms = conflict.getVms().stream()
-                    .map(Guid::toString)
+            String vms = vmStaticDao.getByIds(new ArrayList<>(conflict.getVms())).stream()
+                    .map(VmStatic::getName)
                     .collect(Collectors.joining(","));
 
             if (conflict.getType().canBeSaved()) {
@@ -228,9 +236,10 @@ public static Result checkAffinityGroupConflicts(List<AffinityGroup> groups) {
                 return Result.newFailed(new ValidationResult(EngineMessage.ACTION_TYPE_FAILED_AFFINITY_RULES_COLLISION,
                         String.format("$UnifiedAffinityGroups %1$s", vms),
                         String.format("$negativeAR %1$s", affinityGroupsNames),
-                        String.format("$Vms %1$s", conflict.getNegativeVms().stream()
-                                .map(Guid::toString)
-                                .collect(Collectors.joining(",")))));
+                        String.format("$Vms %1$s",
+                                vmStaticDao.getByIds(new ArrayList<>(conflict.getNegativeVms())).stream()
+                                        .map(VmStatic::getName)
+                                        .collect(Collectors.joining(",")))));
             } else {
                 return Result.newFailed(new ValidationResult(
                         Arrays.asList(EngineMessage.ACTION_TYPE_FAILED_AFFINITY_HOSTS_RULES_COLLISION,

backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/scheduling/commands/AddAffinityGroupCommandTest.java

@@ -17,6 +17,7 @@
 import org.mockito.quality.Strictness;
 import org.ovirt.engine.core.bll.BaseCommandTest;
 import org.ovirt.engine.core.bll.ValidateTestUtils;
+import org.ovirt.engine.core.bll.validator.AffinityValidator;
 import org.ovirt.engine.core.common.AuditLogType;
 import org.ovirt.engine.core.common.businessentities.Cluster;
 import org.ovirt.engine.core.common.businessentities.Label;
@@ -49,6 +50,9 @@ public class AddAffinityGroupCommandTest extends BaseCommandTest {
     @Mock
     private VdsStaticDao vdsStaticDao;
 
+    @Mock
+    AffinityValidator affinityValidator;
+
     AffinityGroupCRUDParameters parameters = new AffinityGroupCRUDParameters(null, createAffinityGroup());
 
     @Spy
@@ -69,6 +73,8 @@ public void setup() {
         Label vmLabel = new LabelBuilder().id(vmLabelId).build();
         Label hostLabel = new LabelBuilder().id(hostLabelId).build();
         doReturn(Arrays.asList(vmLabel, hostLabel)).when(labelDao).getAllByIds(any());
+
+        doReturn(AffinityValidator.Result.VALID).when(affinityValidator).checkAffinityGroupConflicts(any());
     }
 
     @Test

backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/scheduling/commands/EditAffinityGroupCommandTest.java

@@ -18,6 +18,7 @@
 import org.mockito.quality.Strictness;
 import org.ovirt.engine.core.bll.BaseCommandTest;
 import org.ovirt.engine.core.bll.ValidateTestUtils;
+import org.ovirt.engine.core.bll.validator.AffinityValidator;
 import org.ovirt.engine.core.common.AuditLogType;
 import org.ovirt.engine.core.common.businessentities.Cluster;
 import org.ovirt.engine.core.common.businessentities.VmStatic;
@@ -45,6 +46,9 @@ public class EditAffinityGroupCommandTest extends BaseCommandTest {
     @Mock
     private VdsStaticDao vdsStaticDao;
 
+    @Mock
+    AffinityValidator affinityValidator;
+
     AffinityGroupCRUDParameters parameters = new AffinityGroupCRUDParameters(null, createAffinityGroup());
 
     @Spy
@@ -64,6 +68,7 @@ public void setup() {
         vmStatic.setClusterId(clusterId);
         doReturn(Collections.singletonList(vmStatic)).when(vmStaticDao).getByIds(any());
         doReturn(clusterId).when(command).getClusterId();
+        doReturn(AffinityValidator.Result.VALID).when(affinityValidator).checkAffinityGroupConflicts(any());
     }
 
     @Test