core: validate transfer phase

Do not update the phase if the incoming phase is invalid. This only affects external phase updates, when the user attempts to cancel or finalize a transfer. Bug-Url: bugzilla.redhat.com/2092816
oVirt · Sep 11, 2022 · 6c0fd3d · 6c0fd3d
1 parent 153e96f
commit 6c0fd3d
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 6 deletions.
diff --git a/.../bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/image/ImageTransferUpdater.java b/.../bll/src/main/java/org/ovirt/engine/core/bll/storage/disk/image/ImageTransferUpdater.java
@@ -10,6 +10,7 @@
 
 import org.ovirt.engine.core.bll.LockMessagesMatchUtil;
 import org.ovirt.engine.core.common.businessentities.storage.ImageTransfer;
+import org.ovirt.engine.core.common.businessentities.storage.ImageTransferPhase;
 import org.ovirt.engine.core.common.errors.EngineMessage;
 import org.ovirt.engine.core.common.locks.LockingGroup;
 import org.ovirt.engine.core.compat.Guid;
@@ -50,12 +51,18 @@ public ImageTransfer updateEntity(ImageTransfer updates, Guid commandId, boolean
                     entity.setId(updates.getId());
                 }
                 if (updates.getPhase() != null) {
-                    // TODO: Validate that phase change is valid. For now just log.
-                    log.info("Updating image transfer '{}' phase from '{}' to '{}'",
-                            commandId,
-                            entity.getPhase(),
-                            updates.getPhase());
-                    entity.setPhase(updates.getPhase());
+                    if (!ImageTransferPhase.isValidTransition(entity.getPhase(), updates.getPhase())) {
+                        log.error("Image transfer '{}' transition from phase '{}' to '{}' is invalid",
+                                commandId,
+                                entity.getPhase(),
+                                updates.getPhase());
+                    } else {
+                        log.info("Updating image transfer '{}' phase from '{}' to '{}'",
+                                commandId,
+                                entity.getPhase(),
+                                updates.getPhase());
+                        entity.setPhase(updates.getPhase());
+                    }
                 }
                 if (updates.getType() != null) {
                     entity.setType(updates.getType());

diff --git a/...c/main/java/org/ovirt/engine/core/common/businessentities/storage/ImageTransferPhase.java b/...c/main/java/org/ovirt/engine/core/common/businessentities/storage/ImageTransferPhase.java
@@ -26,17 +26,39 @@ public enum ImageTransferPhase implements Identifiable {
     private String description;
     private static final Map<Integer, ImageTransferPhase> valueToPhase = new HashMap<>();
 
+    // TODO: revisit this in the future to validate all transition
+    // private static final EnumMap<ImageTransferPhase, EnumSet<ImageTransferPhase>> validTransitions = new EnumMap<>(ImageTransferPhase.class);
+
     static {
         for (ImageTransferPhase phase : values()) {
             valueToPhase.put(phase.getValue(), phase);
         }
+
+        // TODO: revisit this in the future to validate all transition
+        //        validTransitions.put(INITIALIZING, EnumSet.of(TRANSFERRING, PAUSED_SYSTEM, CANCELLED_SYSTEM));
+        //        validTransitions.put(TRANSFERRING, EnumSet.of(FINALIZING_SUCCESS, PAUSED_SYSTEM, CANCELLED_SYSTEM, CANCELLED_USER, PAUSED_USER));
+        //        validTransitions.put(RESUMING, EnumSet.of(TRANSFERRING, PAUSED_SYSTEM, CANCELLED_SYSTEM));
+        //        validTransitions.put(PAUSED_SYSTEM, EnumSet.of(RESUMING, CANCELLED_USER, CANCELLED_SYSTEM));
+        //        validTransitions.put(PAUSED_USER, EnumSet.of(RESUMING, CANCELLED_USER, CANCELLED_SYSTEM));
+        //        validTransitions.put(CANCELLED_USER, EnumSet.of(FINALIZING_CLEANUP));
+        //        validTransitions.put(CANCELLED_SYSTEM, EnumSet.of(FINALIZING_FAILURE));
+        //        validTransitions.put(FINALIZING_SUCCESS, EnumSet.of(FINALIZING_FAILURE, FINISHED_SUCCESS));
+        //        validTransitions.put(FINALIZING_FAILURE, EnumSet.of(FINISHED_FAILURE));
+        //        validTransitions.put(FINALIZING_CLEANUP, EnumSet.of(FINISHED_CLEANUP));
+        //        validTransitions.put(FINISHED_FAILURE, EnumSet.of(FINISHED_FAILURE));
+        //        validTransitions.put(FINISHED_SUCCESS, EnumSet.of(FINISHED_SUCCESS));
+
     }
 
     ImageTransferPhase(int value, String description) {
         this.value = value;
         this.description = description;
     }
 
+    public static boolean isValidTransition(ImageTransferPhase from, ImageTransferPhase to) {
+        return !from.isFinished();
+    }
+
     @Override
     public int getValue() {
         return value;