Merge pull request #454 from aledsage/issue-830-vapptemplate-tests-2

Issue #830: vappTemplate live tests improvements

labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/VCloudDirectorMediaType.java

@@ -191,17 +191,10 @@ public class VCloudDirectorMediaType {
    
    public static final String ADMIN_VDC = "application/vnd.vmware.admin.vdc+xml";
    
-   public static final String OVF_TEXT_XML = "text/xml";
+   public static final String TEXT_XML = "text/xml";
 
    /**
-    * All acceptable OVF-specific media types.
+    * All acceptable media types.
-    */
-   public static final List<String> ALL_OVF = ImmutableList.of(
-            OVF_TEXT_XML
-         );
-
-   /**
-    * All acceptable vCloud-specific media types.
     */
    public static final List<String> ALL = ImmutableList.of(
          SESSION, ERROR, ORG_LIST, METADATA, METADATA_ENTRY, METADATA_VALUE,
@@ -222,7 +215,9 @@ public class VCloudDirectorMediaType {
          ORG_GENERAL_SETTINGS, ORG_EMAIL_SETTINGS, ORG_SETTINGS, ADMIN_NETWORK,
          ADMIN_ORG_NETWORK, USER, ROLE, DEPLOY_VAPP_PARAMS, RECOMPOSE_VAPP_PARAMS,
          RELOCATE_VM_PARAMS, UNDEPLOY_VAPP_PARAMS, ADMIN_VDC, MEDIA_PARAMS,
-         RUNTIME_INFO_SECTION, SCREEN_TICKET, VAPP_NETWORK
+         RUNTIME_INFO_SECTION, SCREEN_TICKET, VAPP_NETWORK,
+         
+         TEXT_XML
       );
 
    // NOTE These lists must be updated whenever a new media type constant is added.

labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/CloneVAppTemplateParams.java

@@ -22,6 +22,7 @@ package org.jclouds.vcloud.director.v1_5.domain;
 import static com.google.common.base.Objects.equal;
 
 import javax.xml.bind.annotation.XmlElement;
+import javax.xml.bind.annotation.XmlRootElement;
 import javax.xml.bind.annotation.XmlType;
 
 import com.google.common.base.Objects;
@@ -54,6 +55,7 @@ import com.google.common.base.Objects;
       "source",
       "isSourceDelete"
 })
+@XmlRootElement(name = "CloneVAppTemplateParams")
 public class CloneVAppTemplateParams extends ParamsType {
    public static Builder<?> builder() {
       return new ConcreteBuilder();

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/domain/Checks.java

@@ -168,12 +168,8 @@ public class Checks {
       assertTrue(Link.Rel.ALL.contains(link.getRel()), String.format(REQUIRED_VALUE_OBJECT_FMT, "Rel", "Link", link.getRel(), Iterables.toString(Link.Rel.ALL)));
 
       // Check parent type
-      if (link.getRel().equals(Link.Rel.OVF)) {
-         checkReferenceType(link, VCloudDirectorMediaType.ALL_OVF);
-      } else {
       checkReferenceType(link);
    }
-   }
 
    public static void checkTask(Task task) {
       // Check required fields

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/features/VAppTemplateClientLiveTest.java

@@ -20,16 +20,18 @@ package org.jclouds.vcloud.director.v1_5.features;
 
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertTrue;
 import static org.testng.Assert.fail;
 
 import java.net.URI;
 import java.util.Map;
-import java.util.NoSuchElementException;
 import java.util.Random;
 import java.util.Set;
 
+import org.jclouds.vcloud.director.v1_5.VCloudDirectorException;
 import org.jclouds.vcloud.director.v1_5.domain.Checks;
+import org.jclouds.vcloud.director.v1_5.domain.CloneVAppTemplateParams;
 import org.jclouds.vcloud.director.v1_5.domain.CustomizationSection;
 import org.jclouds.vcloud.director.v1_5.domain.GuestCustomizationSection;
 import org.jclouds.vcloud.director.v1_5.domain.LeaseSettingsSection;
@@ -70,16 +72,13 @@ public class VAppTemplateClientLiveTest extends BaseVCloudDirectorClientLiveTest
 
    private final Random random = new Random();
    private VAppTemplateClient vappTemplateClient;
+   private VdcClient vdcClient;
    
    @BeforeClass(inheritGroups = true)
    @Override
-   public void setupRequiredClients() {
+   public void setupRequiredClients() throws Exception {
-      try {
       vappTemplateClient = context.getApi().getVAppTemplateClient();
-      } catch (Exception e) {
+      vdcClient = context.getApi().getVdcClient();
-         // TODO Declare super as throws Exception?
-         throw Throwables.propagate(e);
-      }
    }
 
    @Test
@@ -175,14 +174,10 @@ public class VAppTemplateClientLiveTest extends BaseVCloudDirectorClientLiveTest
 
    @Test
    public void testEditVAppTemplate() {
-      // TODO Should we be using the orig vappTemplate? Or a blank one with only the name/description?
-      
-      VAppTemplate oldVAppTemplate = vappTemplateClient.getVAppTemplate(vAppTemplateURI);
-      
       String uid = ""+random.nextInt();
       String name = "myname-"+uid;
       String description = "mydescr-"+uid;
-      VAppTemplate template = oldVAppTemplate.toBuilder()
+      VAppTemplate template = VAppTemplate.builder()
                .name(name)
                .description(description)
                .build();
@@ -344,21 +339,29 @@ public class VAppTemplateClientLiveTest extends BaseVCloudDirectorClientLiveTest
       assertEquals(newNetworkConnectionSection.getInfo(), info);
    }
    
-   @Test // FIXME Need to clone a new template so have something to delete
+   @Test // FIXME cloneVAppTemplate is giving back 500 error
    public void testDeleteVAppTemplate() throws Exception {
-      // FIXME fix case of VappTemplate -> VAppTemplate
+      CloneVAppTemplateParams cloneVAppTemplateParams = CloneVAppTemplateParams.builder()
-      String newVAppTemplateId = "something";
+               .source(Reference.builder().href(vAppTemplateURI).build())
-      URI newVAppTemplateURI = URI.create(endpoint + "/vAppTemplate/" + newVAppTemplateId);
+               .build();
+      VAppTemplate clonedVappTemplate = vdcClient.cloneVAppTemplate(vdcURI, cloneVAppTemplateParams);
+      Task cloneTask = Iterables.getFirst(clonedVappTemplate.getTasks(), null);
+      assertNotNull(cloneTask, "vdcClient.cloneVAppTemplate returned VAppTemplate that did not contain any tasks");
+      retryTaskSuccess.apply(cloneTask);
 
-      final Task task = vappTemplateClient.deleteVappTemplate(newVAppTemplateURI);
+      // Confirm that "get" works pre-delete
+      VAppTemplate vAppTemplatePreDelete = vappTemplateClient.getVAppTemplate(clonedVappTemplate.getHref());
+      Checks.checkVAppTemplate(vAppTemplatePreDelete);
+      
+      // Delete the template
+      final Task task = vappTemplateClient.deleteVappTemplate(clonedVappTemplate.getHref());
       retryTaskSuccess.apply(task);
 
-      // TODO What is the nice way to find out if a template now exists? It's certainly not the code below!
+      // Confirm that can't access post-delete, i.e. template has been deleted
-      // But this gives an idea of what I'm trying to do.
       try {
-         vappTemplateClient.getVAppTemplate(newVAppTemplateURI);
+         vappTemplateClient.getVAppTemplate(clonedVappTemplate.getHref());
-      } catch (NoSuchElementException e) {
+      } catch (VCloudDirectorException e) {
-         // success
+         // success; should get a 403 because vAppTemplate no longer exists
       }
    }
 
@@ -366,10 +369,14 @@ public class VAppTemplateClientLiveTest extends BaseVCloudDirectorClientLiveTest
    public void testDisableVAppTemplateDownload() throws Exception {
       vappTemplateClient.disableDownloadVappTemplate(vAppTemplateURI);
       
-      // Assert that "download" link is now not offered
+      // TODO Check that it really is disabled. The only thing I can see for determining this 
-      VAppTemplate vAppTemplate = vappTemplateClient.getVAppTemplate(vAppTemplateURI);
+      // is the undocumented "download" link in the VAppTemplate. But that is brittle and we
-      Set<Link> links = vAppTemplate.getLinks();
+      // don't know what timing guarantees there are for adding/removing the link.
-      assertFalse(hasLinkMatchingRel(links, "download.*"), "Should not offer download link after disabling download: "+vAppTemplate);
+      //
+      // For example:
+      //    VAppTemplate vAppTemplate = vappTemplateClient.getVAppTemplate(vAppTemplateURI);
+      //    Set<Link> links = vAppTemplate.getLinks();
+      //    assertFalse(hasLinkMatchingRel(links, "download.*"), "Should not offer download link after disabling download: "+vAppTemplate);
    }
    
    @Test
@@ -379,12 +386,17 @@ public class VAppTemplateClientLiveTest extends BaseVCloudDirectorClientLiveTest
       final Task task = vappTemplateClient.enableDownloadVappTemplate(vAppTemplateURI);
       retryTaskSuccess.apply(task);
       
-      // Assert that "download" link is now offered
+      // TODO Check that it really is enabled. The only thing I can see for determining this 
-      VAppTemplate vAppTemplate = vappTemplateClient.getVAppTemplate(vAppTemplateURI);
+      // is the undocumented "download" link in the VAppTemplate. But that is brittle and we
-      Set<Link> links = vAppTemplate.getLinks();
+      // don't know what timing guarantees there are for adding/removing the link.
-      assertTrue(hasLinkMatchingRel(links, "download.*"), "Should offer download link after enabling download: "+vAppTemplate);
+      //
+      // For example:
+      //    VAppTemplate vAppTemplate = vappTemplateClient.getVAppTemplate(vAppTemplateURI);
+      //    Set<Link> links = vAppTemplate.getLinks();
+      //    assertTrue(hasLinkMatchingRel(links, "download.*"), "Should offer download link after enabling download: "+vAppTemplate);
    }
    
+   @SuppressWarnings("unused")
    private boolean hasLinkMatchingRel(Set<Link> links, String regex) {
       for (Link link : links) {
          if (link.getRel() != null && link.getRel().matches(regex)) {
@@ -413,7 +425,9 @@ public class VAppTemplateClientLiveTest extends BaseVCloudDirectorClientLiveTest
       retryTaskSuccess.apply(task);
    }
    
-   @Test
+   // This failed previously, but is passing now. 
+   // However, it's not part of the official API so not necessary to assert it.
+   @Test(enabled = false) 
    public void testCompletedTaskNotIncludedInVAppTemplate() throws Exception {
       // Kick off a task, and wait for it to complete
       vappTemplateClient.disableDownloadVappTemplate(vAppTemplateURI);

labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/internal/BaseVCloudDirectorClientLiveTest.java

@@ -70,7 +70,7 @@ public abstract class BaseVCloudDirectorClientLiveTest extends BaseVersionedServ
    }
 
    // NOTE Implement as required to populate xxxClient fields, or NOP
-   public abstract void setupRequiredClients();
+   public abstract void setupRequiredClients() throws Exception;
 
    public Predicate<Task> retryTaskSuccess;
    public Predicate<Task> retryTaskSuccessLong;
@@ -89,7 +89,7 @@ public abstract class BaseVCloudDirectorClientLiveTest extends BaseVersionedServ
    protected Session session;
 
    @BeforeClass(groups = { "live" })
-   public void setupContext() {
+   public void setupContext() throws Exception {
       setupCredentials();
       Properties overrides = setupProperties();