cloudstack-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dar...@apache.org
Subject git commit: updated refs/heads/master to f3e968b
Date Wed, 23 Oct 2013 23:37:05 GMT
Updated Branches:
  refs/heads/master b74754959 -> f3e968b98


Update StorageStrategyFactory to not sort, but just iterate and find the first


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/f3e968b9
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/f3e968b9
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/f3e968b9

Branch: refs/heads/master
Commit: f3e968b9830a667cb3b55477f1fe8259e56ceffb
Parents: b747549
Author: Darren Shepherd <darren.s.shepherd@gmail.com>
Authored: Wed Oct 23 16:36:31 2013 -0700
Committer: Darren Shepherd <darren.s.shepherd@gmail.com>
Committed: Wed Oct 23 16:36:31 2013 -0700

----------------------------------------------------------------------
 .../api/storage/StorageStrategyFactory.java     |  9 --
 .../helper/StorageStrategyFactoryImpl.java      | 57 ++++--------
 .../api/storage/StrategyPriorityTest.java       | 93 ++++++++++++++------
 3 files changed, 82 insertions(+), 77 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f3e968b9/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java
----------------------------------------------------------------------
diff --git a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java
b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java
index 3a73cd0..ac1e311 100644
--- a/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java
+++ b/engine/api/src/org/apache/cloudstack/engine/subsystem/api/storage/StorageStrategyFactory.java
@@ -18,7 +18,6 @@
  */
 package org.apache.cloudstack.engine.subsystem.api.storage;
 
-import java.util.Collection;
 import java.util.Map;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
@@ -28,18 +27,10 @@ import com.cloud.storage.Snapshot;
 
 public interface StorageStrategyFactory {
 
-    Collection<DataMotionStrategy> getDataMotionStrategies(DataObject srcData, DataObject
destData);
-
     DataMotionStrategy getDataMotionStrategy(DataObject srcData, DataObject destData);
 
-
-    Collection<DataMotionStrategy> getDataMotionStrategies(Map<VolumeInfo, DataStore>
volumeMap, Host srcHost, Host destHost);
-
     DataMotionStrategy getDataMotionStrategy(Map<VolumeInfo, DataStore> volumeMap,
Host srcHost, Host destHost);
 
-
-    Collection<SnapshotStrategy> getSnapshotStrategies(Snapshot snapshot, SnapshotOperation
op);
-
     SnapshotStrategy getSnapshotStrategy(Snapshot snapshot, SnapshotOperation op); 
 
 }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f3e968b9/engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java
----------------------------------------------------------------------
diff --git a/engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java
b/engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java
index 0bbd28b..30812bf 100644
--- a/engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java
+++ b/engine/storage/src/org/apache/cloudstack/storage/helper/StorageStrategyFactoryImpl.java
@@ -19,10 +19,8 @@
 package org.apache.cloudstack.storage.helper;
 
 import java.util.Collection;
-import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
-import java.util.TreeSet;
 
 import javax.inject.Inject;
 
@@ -30,10 +28,10 @@ import org.apache.cloudstack.engine.subsystem.api.storage.DataMotionStrategy;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataObject;
 import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy;
+import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
 import org.apache.cloudstack.engine.subsystem.api.storage.StorageStrategyFactory;
 import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
 import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
-import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
 
 import com.cloud.host.Host;
 import com.cloud.storage.Snapshot;
@@ -44,23 +42,8 @@ public class StorageStrategyFactoryImpl implements StorageStrategyFactory
{
     List<DataMotionStrategy> dataMotionStrategies;
 
     @Override
-    public DataMotionStrategy getDataMotionStrategy(DataObject srcData, DataObject destData)
{
-        return first(getDataMotionStrategies(srcData, destData));
-    }
-
-    @Override
-    public DataMotionStrategy getDataMotionStrategy(Map<VolumeInfo, DataStore> volumeMap,
Host srcHost, Host destHost) {
-        return first(getDataMotionStrategies(volumeMap, srcHost, destHost));
-    }
-
-    @Override
-    public SnapshotStrategy getSnapshotStrategy(Snapshot snapshot, SnapshotOperation op)
{
-        return first(getSnapshotStrategies(snapshot, op));
-    }
-
-    @Override
-    public Collection<DataMotionStrategy> getDataMotionStrategies(final DataObject
srcData, final DataObject destData) {
-        return sort(dataMotionStrategies, new CanHandle<DataMotionStrategy>() {
+    public DataMotionStrategy getDataMotionStrategy(final DataObject srcData, final DataObject
destData) {
+        return bestMatch(dataMotionStrategies, new CanHandle<DataMotionStrategy>()
{
             @Override
             public StrategyPriority canHandle(DataMotionStrategy strategy) {
                 return strategy.canHandle(srcData, destData);
@@ -69,8 +52,8 @@ public class StorageStrategyFactoryImpl implements StorageStrategyFactory
{
     }
 
     @Override
-    public Collection<DataMotionStrategy> getDataMotionStrategies(final Map<VolumeInfo,
DataStore> volumeMap, final Host srcHost, final Host destHost) {
-        return sort(dataMotionStrategies, new CanHandle<DataMotionStrategy>() {
+    public DataMotionStrategy getDataMotionStrategy(final Map<VolumeInfo, DataStore>
volumeMap, final Host srcHost, final Host destHost) {
+        return bestMatch(dataMotionStrategies, new CanHandle<DataMotionStrategy>()
{
             @Override
             public StrategyPriority canHandle(DataMotionStrategy strategy) {
                 return strategy.canHandle(volumeMap, srcHost, destHost);
@@ -79,8 +62,8 @@ public class StorageStrategyFactoryImpl implements StorageStrategyFactory
{
     }
 
     @Override
-    public Collection<SnapshotStrategy> getSnapshotStrategies(final Snapshot snapshot,
final SnapshotOperation op) {
-        return sort(snapshotStrategies, new CanHandle<SnapshotStrategy>() {
+    public SnapshotStrategy getSnapshotStrategy(final Snapshot snapshot, final SnapshotOperation
op) {
+        return bestMatch(snapshotStrategies, new CanHandle<SnapshotStrategy>() {
             @Override
             public StrategyPriority canHandle(SnapshotStrategy strategy) {
                 return strategy.canHandle(snapshot, op);
@@ -88,30 +71,22 @@ public class StorageStrategyFactoryImpl implements StorageStrategyFactory
{
         });
     }
 
-    private static <T> Collection<T> sort(Collection<T> collection, final
CanHandle<T> canHandle) {
+    private static <T> T bestMatch(Collection<T> collection, final CanHandle<T>
canHandle) {
         if (collection.size() == 0)
             return null;
 
-        TreeSet<T> resultSet = new TreeSet<T>(new Comparator<T>() {
-            @Override
-            public int compare(T o1, T o2) {
-                int i1 = canHandle.canHandle(o1).ordinal();
-                int i2 = canHandle.canHandle(o2).ordinal();
-                return new Integer(i2).compareTo(new Integer(i1));
-            }
-        });
+        StrategyPriority highestPriority = StrategyPriority.CANT_HANDLE;
 
-        for ( T test : collection ) {
-            if ( canHandle.canHandle(test) != StrategyPriority.CANT_HANDLE ) {
-                resultSet.add(test);
+        T strategyToUse = null;
+        for (T strategy : collection) {
+            StrategyPriority priority = canHandle.canHandle(strategy);
+            if (priority.ordinal() > highestPriority.ordinal()) {
+                highestPriority = priority;
+                strategyToUse = strategy;
             }
         }
 
-        return resultSet;
-    }
-    
-    private static <T> T first(Collection<T> resultSet) {
-        return resultSet.size() == 0 ? null : resultSet.iterator().next();
+        return strategyToUse;
     }
     
     private static interface CanHandle<T> {

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/f3e968b9/engine/storage/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java
----------------------------------------------------------------------
diff --git a/engine/storage/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java
b/engine/storage/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java
index 8cfa955..1c3ceb6 100644
--- a/engine/storage/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java
+++ b/engine/storage/test/org/apache/cloudstack/engine/subsystem/api/storage/StrategyPriorityTest.java
@@ -16,23 +16,23 @@
 // under the License.
 package org.apache.cloudstack.engine.subsystem.api.storage;
 
-import static org.junit.Assert.*;
-import static org.mockito.Matchers.*;
-import static org.mockito.Mockito.*;
-
 import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
 import org.apache.cloudstack.engine.subsystem.api.storage.SnapshotStrategy.SnapshotOperation;
+import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
 import org.apache.cloudstack.storage.helper.StorageStrategyFactoryImpl;
 import org.junit.Test;
 
 import com.cloud.host.Host;
 import com.cloud.storage.Snapshot;
 
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+
 public class StrategyPriorityTest {
 
     @Test
@@ -50,17 +50,30 @@ public class StrategyPriorityTest {
         doReturn(StrategyPriority.HIGHEST).when(highestStrategy).canHandle(any(Snapshot.class),
any(SnapshotOperation.class));
 
         List<SnapshotStrategy> strategies = new ArrayList<SnapshotStrategy>(5);
-        strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy,
highestStrategy));
+        SnapshotStrategy strategy = null;
 
         StorageStrategyFactoryImpl factory = new StorageStrategyFactoryImpl();
         factory.setSnapshotStrategies(strategies);
-        Iterator<SnapshotStrategy> iter = factory.getSnapshotStrategies(mock(Snapshot.class),
SnapshotOperation.TAKE).iterator();
 
-        assertEquals("Highest was not 1st.", highestStrategy, iter.next());
-        assertEquals("Plugin was not 2nd.", pluginStrategy, iter.next());
-        assertEquals("Hypervisor was not 3rd.", hyperStrategy, iter.next());
-        assertEquals("Default was not 4th.", defaultStrategy, iter.next());
-        assertTrue("Can't Handle was not 5th.", !iter.hasNext());
+        strategies.add(cantHandleStrategy);
+        strategy = factory.getSnapshotStrategy(mock(Snapshot.class), SnapshotOperation.TAKE);
+        assertEquals("A strategy was found when it shouldn't have been.", null, strategy);
+
+        strategies.add(defaultStrategy);
+        strategy = factory.getSnapshotStrategy(mock(Snapshot.class), SnapshotOperation.TAKE);
+        assertEquals("Default strategy was not picked.", defaultStrategy, strategy);
+
+        strategies.add(hyperStrategy);
+        strategy = factory.getSnapshotStrategy(mock(Snapshot.class), SnapshotOperation.TAKE);
+        assertEquals("Hypervisor strategy was not picked.", hyperStrategy, strategy);
+
+        strategies.add(pluginStrategy);
+        strategy = factory.getSnapshotStrategy(mock(Snapshot.class), SnapshotOperation.TAKE);
+        assertEquals("Plugin strategy was not picked.", pluginStrategy, strategy);
+
+        strategies.add(highestStrategy);
+        strategy = factory.getSnapshotStrategy(mock(Snapshot.class), SnapshotOperation.TAKE);
+        assertEquals("Highest strategy was not picked.", highestStrategy, strategy);
     }
 
     @Test
@@ -78,17 +91,30 @@ public class StrategyPriorityTest {
         doReturn(StrategyPriority.HIGHEST).when(highestStrategy).canHandle(any(DataObject.class),
any(DataObject.class));
 
         List<DataMotionStrategy> strategies = new ArrayList<DataMotionStrategy>(5);
-        strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy,
highestStrategy));
+        DataMotionStrategy strategy = null;
 
         StorageStrategyFactoryImpl factory = new StorageStrategyFactoryImpl();
         factory.setDataMotionStrategies(strategies);
-        Iterator<DataMotionStrategy> iter = factory.getDataMotionStrategies(mock(DataObject.class),
mock(DataObject.class)).iterator();
 
-        assertEquals("Highest was not 1st.", highestStrategy, iter.next());
-        assertEquals("Plugin was not 2nd.", pluginStrategy, iter.next());
-        assertEquals("Hypervisor was not 3rd.", hyperStrategy, iter.next());
-        assertEquals("Default was not 4th.", defaultStrategy, iter.next());
-        assertTrue("Can't Handle was not 5th.", !iter.hasNext());
+        strategies.add(cantHandleStrategy);
+        strategy = factory.getDataMotionStrategy(mock(DataObject.class), mock(DataObject.class));
+        assertEquals("A strategy was found when it shouldn't have been.", null, strategy);
+
+        strategies.add(defaultStrategy);
+        strategy = factory.getDataMotionStrategy(mock(DataObject.class), mock(DataObject.class));
+        assertEquals("Default strategy was not picked.", defaultStrategy, strategy);
+
+        strategies.add(hyperStrategy);
+        strategy = factory.getDataMotionStrategy(mock(DataObject.class), mock(DataObject.class));
+        assertEquals("Hypervisor strategy was not picked.", hyperStrategy, strategy);
+
+        strategies.add(pluginStrategy);
+        strategy = factory.getDataMotionStrategy(mock(DataObject.class), mock(DataObject.class));
+        assertEquals("Plugin strategy was not picked.", pluginStrategy, strategy);
+
+        strategies.add(highestStrategy);
+        strategy = factory.getDataMotionStrategy(mock(DataObject.class), mock(DataObject.class));
+        assertEquals("Highest strategy was not picked.", highestStrategy, strategy);
     }
 
     @Test
@@ -107,16 +133,29 @@ public class StrategyPriorityTest {
         doReturn(StrategyPriority.HIGHEST).when(highestStrategy).canHandle(any(Map.class),
any(Host.class), any(Host.class));
 
         List<DataMotionStrategy> strategies = new ArrayList<DataMotionStrategy>(5);
-        strategies.addAll(Arrays.asList(defaultStrategy, pluginStrategy, hyperStrategy, cantHandleStrategy,
highestStrategy));
+        DataMotionStrategy strategy = null;
 
         StorageStrategyFactoryImpl factory = new StorageStrategyFactoryImpl();
         factory.setDataMotionStrategies(strategies);
-        Iterator<DataMotionStrategy> iter = factory.getDataMotionStrategies(mock(Map.class),
mock(Host.class), mock(Host.class)).iterator();
 
-        assertEquals("Highest was not 1st.", highestStrategy, iter.next());
-        assertEquals("Plugin was not 2nd.", pluginStrategy, iter.next());
-        assertEquals("Hypervisor was not 3rd.", hyperStrategy, iter.next());
-        assertEquals("Default was not 4th.", defaultStrategy, iter.next());
-        assertTrue("Can't Handle was not 5th.", !iter.hasNext());
+        strategies.add(cantHandleStrategy);
+        strategy = factory.getDataMotionStrategy(mock(Map.class), mock(Host.class), mock(Host.class));
+        assertEquals("A strategy was found when it shouldn't have been.", null, strategy);
+
+        strategies.add(defaultStrategy);
+        strategy = factory.getDataMotionStrategy(mock(Map.class), mock(Host.class), mock(Host.class));
+        assertEquals("Default strategy was not picked.", defaultStrategy, strategy);
+
+        strategies.add(hyperStrategy);
+        strategy = factory.getDataMotionStrategy(mock(Map.class), mock(Host.class), mock(Host.class));
+        assertEquals("Hypervisor strategy was not picked.", hyperStrategy, strategy);
+
+        strategies.add(pluginStrategy);
+        strategy = factory.getDataMotionStrategy(mock(Map.class), mock(Host.class), mock(Host.class));
+        assertEquals("Plugin strategy was not picked.", pluginStrategy, strategy);
+
+        strategies.add(highestStrategy);
+        strategy = factory.getDataMotionStrategy(mock(Map.class), mock(Host.class), mock(Host.class));
+        assertEquals("Highest strategy was not picked.", highestStrategy, strategy);
     }
 }


Mime
View raw message