Removed duplicate code in CitrixResourceBase.getPatchFiles

@cristofolini comments:
Removed unnecessary duplicated code by having the body of the
getPatchFiles method only in the
CitrixResourceBase superclass. Given that all of its implementations
consisted of the same code
except for the path which contains the necessary file for that
implementation. An abstract method
getPatchFilePath was created in the CitrixResourceBase superclass so
that each implementation may
return the path containing the specific file needed by that
implementation.

Test cases were created for each implementation, simple as they may be.
One assert is made to verify
that the path returned by each implementation corresponds to the path
that was previously specified
on each getPatchFiles implementation.

removed trailing whitespace

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java

@@ -124,6 +124,7 @@ import com.cloud.utils.StringUtils;
 import com.cloud.utils.Ternary;
 import com.cloud.utils.exception.CloudRuntimeException;
 import com.cloud.utils.net.NetUtils;
+import com.cloud.utils.script.Script;
 import com.cloud.utils.ssh.SSHCmdHelper;
 import com.cloud.utils.ssh.SshHelper;
 import com.cloud.vm.VirtualMachine.PowerState;
@@ -2867,10 +2868,19 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe
         }
     }
 
-    protected List<File> getPatchFiles() {
+    private List<File> getPatchFiles() {
-        return null;
+        String patch = getPatchFilePath();
+        String patchfilePath = Script.findScript("", patch);
+        if (patchfilePath == null) {
+            throw new CloudRuntimeException("Unable to find patch file "+patch);
+        }
+        List<File> files = new ArrayList<File>();
+        files.add(new File(patchfilePath));
+        return files;
     }
 
+    protected abstract String getPatchFilePath();
+
     public String getPerfMon(final Connection conn, final Map<String, String> params, final int wait) {
         String result = null;
         try {

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XcpOssResource.java

@@ -17,17 +17,11 @@
 
 package com.cloud.hypervisor.xenserver.resource;
 
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
-
 import javax.ejb.Local;
 
 import org.apache.xmlrpc.XmlRpcException;
 
 import com.cloud.resource.ServerResource;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.script.Script;
 import com.xensource.xenapi.Connection;
 import com.xensource.xenapi.Types.XenAPIException;
 import com.xensource.xenapi.VM;
@@ -38,17 +32,8 @@ public class XcpOssResource extends CitrixResourceBase {
     private static final long mem_32m = 33554432L;
 
     @Override
-    protected List<File> getPatchFiles() {
+    protected String getPatchFilePath() {
-        final List<File> files = new ArrayList<File>();
+        return "scripts/vm/hypervisor/xenserver/xcposs/patch";
-        final String patch = "scripts/vm/hypervisor/xenserver/xcposs/patch";
-        final String patchfilePath = Script.findScript("", patch);
-        if (patchfilePath == null) {
-            throw new CloudRuntimeException("Unable to find patch file "
-                    + patch);
-        }
-        final File file = new File(patchfilePath);
-        files.add(file);
-        return files;
     }
 
     @Override

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XcpServerResource.java

@@ -16,18 +16,12 @@
 // under the License.
 package com.cloud.hypervisor.xenserver.resource;
 
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
-
 import javax.ejb.Local;
 
 import org.apache.log4j.Logger;
 import org.apache.xmlrpc.XmlRpcException;
 
 import com.cloud.resource.ServerResource;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.script.Script;
 import com.xensource.xenapi.Connection;
 import com.xensource.xenapi.Host;
 import com.xensource.xenapi.Types.XenAPIException;
@@ -40,16 +34,8 @@ public class XcpServerResource extends CitrixResourceBase {
     private final static long mem_32m = 33554432L;
 
     @Override
-    protected List<File> getPatchFiles() {
+    protected String getPatchFilePath() {
-        final List<File> files = new ArrayList<File>();
+        return "scripts/vm/hypervisor/xenserver/xcpserver/patch";
-        final String patch = "scripts/vm/hypervisor/xenserver/xcpserver/patch";
-        final String patchfilePath = Script.findScript("", patch);
-        if (patchfilePath == null) {
-            throw new CloudRuntimeException("Unable to find patch file " + patch);
-        }
-        final File file = new File(patchfilePath);
-        files.add(file);
-        return files;
     }
 
     /**

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer56FP1Resource.java

@@ -16,9 +16,6 @@
 // under the License.
 package com.cloud.hypervisor.xenserver.resource;
 
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
 import java.util.Map;
 
 import javax.ejb.Local;
@@ -26,8 +23,6 @@ import javax.ejb.Local;
 import org.apache.xmlrpc.XmlRpcException;
 
 import com.cloud.resource.ServerResource;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.script.Script;
 import com.xensource.xenapi.Connection;
 import com.xensource.xenapi.Host;
 import com.xensource.xenapi.Types.XenAPIException;
@@ -36,16 +31,8 @@ import com.xensource.xenapi.Types.XenAPIException;
 public class XenServer56FP1Resource extends XenServer56Resource {
 
     @Override
-    protected List<File> getPatchFiles() {
+    protected String getPatchFilePath() {
-        final List<File> files = new ArrayList<File>();
+        return "scripts/vm/hypervisor/xenserver/xenserver56fp1/patch";
-        final String patch = "scripts/vm/hypervisor/xenserver/xenserver56fp1/patch";
-        final String patchfilePath = Script.findScript("", patch);
-        if (patchfilePath == null) {
-            throw new CloudRuntimeException("Unable to find patch file " + patch);
-        }
-        final File file = new File(patchfilePath);
-        files.add(file);
-        return files;
     }
 
     /**

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer56Resource.java

@@ -16,14 +16,14 @@
 // under the License.
 package com.cloud.hypervisor.xenserver.resource;
 
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
-
 import javax.ejb.Local;
 
 import org.apache.log4j.Logger;
 
+import com.cloud.agent.api.StartupCommand;
+import com.cloud.resource.ServerResource;
+import com.cloud.utils.exception.CloudRuntimeException;
+import com.cloud.utils.ssh.SSHCmdHelper;
 import com.xensource.xenapi.Connection;
 import com.xensource.xenapi.Host;
 import com.xensource.xenapi.Network;
@@ -31,28 +31,13 @@ import com.xensource.xenapi.PIF;
 import com.xensource.xenapi.Types.XenAPIException;
 import com.xensource.xenapi.VLAN;
 
-import com.cloud.agent.api.StartupCommand;
-import com.cloud.resource.ServerResource;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.script.Script;
-import com.cloud.utils.ssh.SSHCmdHelper;
-
 @Local(value = ServerResource.class)
 public class XenServer56Resource extends CitrixResourceBase {
     private final static Logger s_logger = Logger.getLogger(XenServer56Resource.class);
 
     @Override
-    protected List<File> getPatchFiles() {
+    protected String getPatchFilePath() {
-        final List<File> files = new ArrayList<File>();
+        return "scripts/vm/hypervisor/xenserver/xenserver56/patch";
-        final String patch = "scripts/vm/hypervisor/xenserver/xenserver56/patch";
-        final String patchfilePath = Script.findScript("", patch);
-        if (patchfilePath == null) {
-            throw new CloudRuntimeException("Unable to find patch file " + patch);
-        }
-        final File file = new File(patchfilePath);
-        files.add(file);
-
-        return files;
     }
 
     @Override

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer56SP2Resource.java

@@ -16,15 +16,9 @@
 // under the License.
 package com.cloud.hypervisor.xenserver.resource;
 
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
-
 import javax.ejb.Local;
 
 import com.cloud.resource.ServerResource;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.script.Script;
 
 @Local(value = ServerResource.class)
 public class XenServer56SP2Resource extends XenServer56FP1Resource {
@@ -34,17 +28,4 @@ public class XenServer56SP2Resource extends XenServer56FP1Resource {
         _xsMemoryUsed = 128 * 1024 * 1024L;
         _xsVirtualizationFactor = 62.0 / 64.0;
     }
-
-    @Override
-    protected List<File> getPatchFiles() {
-        final List<File> files = new ArrayList<File>();
-        final String patch = "scripts/vm/hypervisor/xenserver/xenserver56fp1/patch";
-        final String patchfilePath = Script.findScript("", patch);
-        if (patchfilePath == null) {
-            throw new CloudRuntimeException("Unable to find patch file " + patch);
-        }
-        final File file = new File(patchfilePath);
-        files.add(file);
-        return files;
-    }
 }

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer600Resource.java

@@ -16,30 +16,15 @@
 // under the License.
 package com.cloud.hypervisor.xenserver.resource;
 
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
-
 import javax.ejb.Local;
 
 import com.cloud.resource.ServerResource;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.script.Script;
 
 @Local(value = ServerResource.class)
 public class XenServer600Resource extends XenServer56SP2Resource {
 
     @Override
-    protected List<File> getPatchFiles() {
+    protected String getPatchFilePath() {
-        final List<File> files = new ArrayList<File>();
+        return "scripts/vm/hypervisor/xenserver/xenserver60/patch";
-        final String patch = "scripts/vm/hypervisor/xenserver/xenserver60/patch";
-        final String patchfilePath = Script.findScript("", patch);
-        if (patchfilePath == null) {
-            throw new CloudRuntimeException("Unable to find patch file " + patch);
-        }
-        final File file = new File(patchfilePath);
-        files.add(file);
-        return files;
     }
-
 }

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServer650Resource.java

@@ -18,29 +18,15 @@
  */
 package com.cloud.hypervisor.xenserver.resource;
 
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
-
 import javax.ejb.Local;
 
 import com.cloud.resource.ServerResource;
-import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.script.Script;
 
 @Local(value=ServerResource.class)
 public class XenServer650Resource extends Xenserver625Resource {
 
     @Override
-    protected List<File> getPatchFiles() {
+    protected String getPatchFilePath() {
-        final List<File> files = new ArrayList<File>();
+        return "scripts/vm/hypervisor/xenserver/xenserver65/patch";
-        final String patch = "scripts/vm/hypervisor/xenserver/xenserver65/patch";
-        final String patchfilePath = Script.findScript("", patch);
-        if (patchfilePath == null) {
-            throw new CloudRuntimeException("Unable to find patch file " + patch);
-        }
-        final File file = new File(patchfilePath);
-        files.add(file);
-        return files;
     }
 }

plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625Resource.java

@@ -18,10 +18,6 @@
  */
 package com.cloud.hypervisor.xenserver.resource;
 
-import java.io.File;
-import java.util.ArrayList;
-import java.util.List;
-
 import javax.ejb.Local;
 
 import org.apache.cloudstack.hypervisor.xenserver.XenServerResourceNewBase;
@@ -32,7 +28,6 @@ import com.cloud.resource.ServerResource;
 import com.cloud.storage.resource.StorageSubsystemCommandHandler;
 import com.cloud.storage.resource.StorageSubsystemCommandHandlerBase;
 import com.cloud.utils.exception.CloudRuntimeException;
-import com.cloud.utils.script.Script;
 import com.cloud.utils.ssh.SSHCmdHelper;
 import com.xensource.xenapi.Connection;
 import com.xensource.xenapi.Host;
@@ -45,27 +40,18 @@ public class Xenserver625Resource extends XenServerResourceNewBase {
     private static final Logger s_logger = Logger.getLogger(Xenserver625Resource.class);
 
     @Override
-    protected List<File> getPatchFiles() {
+    protected String getPatchFilePath() {
-        final List<File> files = new ArrayList<File>();
+        return "scripts/vm/hypervisor/xenserver/xenserver62/patch";
-        final String patch = "scripts/vm/hypervisor/xenserver/xenserver62/patch";
-        final String patchfilePath = Script.findScript("", patch);
-        if (patchfilePath == null) {
-            throw new CloudRuntimeException("Unable to find patch file " + patch);
-        }
-        final File file = new File(patchfilePath);
-        files.add(file);
-        return files;
     }
 
     @Override
     protected StorageSubsystemCommandHandler buildStorageHandler() {
-        final XenServerStorageProcessor processor = new Xenserver625StorageProcessor(this);
+        XenServerStorageProcessor processor = new Xenserver625StorageProcessor(this);
         return new StorageSubsystemCommandHandlerBase(processor);
     }
 
     @Override
-    public void umountSnapshotDir(final Connection conn, final Long dcId) {
+    public void umountSnapshotDir(final Connection conn, final Long dcId) {}
-    }
 
     @Override
     public boolean setupServer(final Connection conn,final Host host) {

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XcpOssResourcePathTest.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2015 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.cloud.hypervisor.xenserver.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class XcpOssResourcePathTest {
+
+    private XcpOssResource xcpOssResource = new XcpOssResource();
+
+    @Test
+    public void testPatchFilePath() {
+        String patchFilePath = xcpOssResource.getPatchFilePath();
+        String patch = "scripts/vm/hypervisor/xenserver/xcposs/patch";
+
+        Assert.assertEquals(patch, patchFilePath);
+    }
+}

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XcpServerResourcePathTest.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2015 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.cloud.hypervisor.xenserver.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class XcpServerResourcePathTest {
+
+    private XcpServerResource xcpServerResource = new XcpServerResource();
+
+    @Test
+    public void testPatchFilePath() {
+        String patchFilePath = xcpServerResource.getPatchFilePath();
+        String patch = "scripts/vm/hypervisor/xenserver/xcpserver/patch";
+
+        Assert.assertEquals(patch, patchFilePath);
+    }
+}

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer56FP1ResourcePathTest.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2015 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.cloud.hypervisor.xenserver.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class XenServer56FP1ResourcePathTest {
+
+    private XenServer56FP1Resource xenServer56FP1Resource = new XenServer56FP1Resource();
+
+    @Test
+    public void testPatchFilePath() {
+        String patchFilePath = xenServer56FP1Resource.getPatchFilePath();
+        String patch = "scripts/vm/hypervisor/xenserver/xenserver56fp1/patch";
+
+        Assert.assertEquals(patch, patchFilePath);
+    }
+}

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer56ResourcePathTest.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2015 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.cloud.hypervisor.xenserver.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class XenServer56ResourcePathTest {
+
+    private XenServer56Resource xenServer56Resource = new XenServer56Resource();
+
+    @Test
+    public void testPatchFilePath() {
+        String patchFilePath = xenServer56Resource.getPatchFilePath();
+        String patch = "scripts/vm/hypervisor/xenserver/xenserver56/patch";
+
+        Assert.assertEquals(patch, patchFilePath);
+    }
+}

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer56SP2ResourcePathTest.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2015 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.cloud.hypervisor.xenserver.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class XenServer56SP2ResourcePathTest {
+
+    private XenServer56SP2Resource xenServer56SP2Resource = new XenServer56SP2Resource();
+
+    @Test
+    public void testPatchFilePath() {
+        String patchFilePath = xenServer56SP2Resource.getPatchFilePath();
+        String patch = "scripts/vm/hypervisor/xenserver/xenserver56fp1/patch";
+
+        Assert.assertEquals(patch, patchFilePath);
+    }
+}

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer600ResourcePathTest.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2015 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.cloud.hypervisor.xenserver.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class XenServer600ResourcePathTest {
+
+    private XenServer600Resource xenServer600Resource = new XenServer600Resource();
+
+    @Test
+    public void testPatchFilePath() {
+        String patchFilePath = xenServer600Resource.getPatchFilePath();
+        String patch = "scripts/vm/hypervisor/xenserver/xenserver60/patch";
+
+        Assert.assertEquals(patch, patchFilePath);
+    }
+}

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer625ResourcePathTest.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2015 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.cloud.hypervisor.xenserver.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class XenServer625ResourcePathTest {
+
+    private Xenserver625Resource xenServer625Resource = new Xenserver625Resource();
+
+    @Test
+    public void testPatchFilePath() {
+        String patchFilePath = xenServer625Resource.getPatchFilePath();
+        String patch = "scripts/vm/hypervisor/xenserver/xenserver62/patch";
+
+        Assert.assertEquals(patch, patchFilePath);
+    }
+}

plugins/hypervisors/xenserver/test/com/cloud/hypervisor/xenserver/resource/XenServer650ResourcePathTest.java

@@ -0,0 +1,32 @@
+/*
+ * Copyright 2015 The Apache Software Foundation.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.cloud.hypervisor.xenserver.resource;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class XenServer650ResourcePathTest {
+
+    private XenServer650Resource xenServer650Resource = new XenServer650Resource();
+
+    @Test
+    public void testPatchFilePath() {
+        String patchFilePath = xenServer650Resource.getPatchFilePath();
+        String patch = "scripts/vm/hypervisor/xenserver/xenserver65/patch";
+
+        Assert.assertEquals(patch, patchFilePath);
+    }
+}