From ddb70df2d9754c185999c1fd82944a6e1bf58776 Mon Sep 17 00:00:00 2001 From: Andrew Donald Kennedy Date: Wed, 14 Mar 2012 03:50:21 +0000 Subject: [PATCH] Incorporate code review comments from @jclouds --- .../v1_5/VCloudDirectorException.java | 2 +- .../v1_5/VCloudDirectorRuntimeException.java | 32 ---------------- .../vcloud/director/v1_5/domain/Error.java | 18 ++++++--- .../director/v1_5/domain/FilesList.java | 34 ++++++++--------- .../domain/InstantiateVAppParamsType.java | 5 ++- .../v1_5/domain/NetworkConnection.java | 37 ++++++++++++++----- .../director/v1_5/domain/ReferenceType.java | 7 +++- .../v1_5/domain/ResourceEntityType.java | 14 ++++--- .../director/v1_5/domain/ResourceType.java | 4 ++ .../vcloud/director/v1_5/domain/Checks.java | 4 +- 10 files changed, 81 insertions(+), 76 deletions(-) delete mode 100644 labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/VCloudDirectorRuntimeException.java diff --git a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/VCloudDirectorException.java b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/VCloudDirectorException.java index c79aa42824..3da73d6d2e 100644 --- a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/VCloudDirectorException.java +++ b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/VCloudDirectorException.java @@ -24,7 +24,7 @@ import org.jclouds.vcloud.director.v1_5.domain.Task; /** * @author grkvlt@apache.org */ -public class VCloudDirectorException extends VCloudDirectorRuntimeException { +public class VCloudDirectorException extends RuntimeException { /** The serialVersionUID. */ private static final long serialVersionUID = -5292516858598372960L; diff --git a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/VCloudDirectorRuntimeException.java b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/VCloudDirectorRuntimeException.java deleted file mode 100644 index 3008e9986a..0000000000 --- a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/VCloudDirectorRuntimeException.java +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Licensed to jclouds, Inc. (jclouds) under one or more - * contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. jclouds licenses this file - * to you 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 org.jclouds.vcloud.director.v1_5; - -/** - * @author grkvlt@apache.org - */ -public class VCloudDirectorRuntimeException extends RuntimeException { - - /** The serialVersionUID. */ - private static final long serialVersionUID = -8590262859549695447L; - - public VCloudDirectorRuntimeException(String message) { - super(message); - } -} diff --git a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/Error.java b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/Error.java index 9040368e99..4f01522555 100644 --- a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/Error.java +++ b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/Error.java @@ -18,16 +18,17 @@ */ package org.jclouds.vcloud.director.v1_5.domain; -import static com.google.common.base.Objects.*; -import static com.google.common.base.Preconditions.*; +import static com.google.common.base.Objects.equal; +import static com.google.common.base.Preconditions.checkNotNull; import java.util.Arrays; +import javax.annotation.Resource; import javax.xml.bind.annotation.XmlAttribute; import javax.xml.bind.annotation.XmlRootElement; +import org.jclouds.logging.Logger; import org.jclouds.vcloud.director.v1_5.VCloudDirectorMediaType; -import org.jclouds.vcloud.director.v1_5.VCloudDirectorRuntimeException; import com.google.common.base.Objects; import com.google.common.base.Optional; @@ -46,6 +47,9 @@ import com.google.common.collect.Iterables; @XmlRootElement(name = "Error") public class Error { + @Resource + protected static Logger logger = Logger.NULL; + public static enum Code { OK(200), @@ -60,10 +64,11 @@ public class Error { NOT_ALLOWED(405), INTERNAL_ERROR(500), NOT_IMPLEMENTED(501), - UNAVAILABLE(503); + UNAVAILABLE(503), + UNRECOGNIZED(-1); private Integer majorErrorCode; - + private Code(Integer majorErrorCode) { this.majorErrorCode = majorErrorCode; } @@ -82,7 +87,8 @@ public class Error { if (found.isPresent()) { return found.get(); } else { - throw new VCloudDirectorRuntimeException(String.format("Illegal major error code '%d'", majorErrorCode)); + logger.warn("Unrecognized major error code '%d'", majorErrorCode); + return UNRECOGNIZED; } } } diff --git a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/FilesList.java b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/FilesList.java index b7ef5c60d1..edc56d3ee6 100644 --- a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/FilesList.java +++ b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/FilesList.java @@ -23,16 +23,15 @@ import static com.google.common.base.Objects.equal; import static com.google.common.base.Preconditions.checkNotNull; import java.util.Collections; -import java.util.List; +import java.util.Set; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlType; import com.google.common.base.Objects; -import com.google.common.collect.ForwardingList; -import com.google.common.collect.ImmutableList; +import com.google.common.collect.ForwardingSet; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Lists; +import com.google.common.collect.Sets; /** @@ -60,7 +59,9 @@ import com.google.common.collect.Lists; @XmlType(name = "FilesList", propOrder = { "files" }) -public class FilesList extends ForwardingList { +public class FilesList extends ForwardingSet { + + // TODO Investigate using the same wrapper (e.g. see Tasks); can we eliminate this class? public static Builder builder() { return new Builder(); @@ -72,13 +73,13 @@ public class FilesList extends ForwardingList { public static class Builder { - private List files = Lists.newLinkedList(); + private Set files = Sets.newLinkedHashSet(); /** * @see FilesList#getFiles() */ - public Builder files(List files) { - this.files = Lists.newLinkedList(checkNotNull(files, "files")); + public Builder files(Iterable files) { + this.files = Sets.newLinkedHashSet(checkNotNull(files, "files")); return this; } @@ -100,23 +101,22 @@ public class FilesList extends ForwardingList { } } + @XmlElement(name = "File", required = true) + private Set files = Sets.newLinkedHashSet(); + private FilesList() { // for JAXB } - private FilesList(List files) { - this.files = ImmutableList.copyOf(files); + private FilesList(Iterable files) { + this.files = ImmutableSet.copyOf(files); } - - @XmlElement(name = "File", required = true) - protected List files = Lists.newLinkedList(); - /** * Gets the value of the file property. */ - public List getFiles() { - return Collections.unmodifiableList(this.files); + public Set getFiles() { + return Collections.unmodifiableSet(this.files); } @Override @@ -141,7 +141,7 @@ public class FilesList extends ForwardingList { } @Override - protected List delegate() { + protected Set delegate() { return getFiles(); } } diff --git a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/InstantiateVAppParamsType.java b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/InstantiateVAppParamsType.java index dfe38c361a..3b45289f78 100644 --- a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/InstantiateVAppParamsType.java +++ b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/InstantiateVAppParamsType.java @@ -18,14 +18,14 @@ */ package org.jclouds.vcloud.director.v1_5.domain; -import static com.google.common.base.Objects.*; +import static com.google.common.base.Objects.equal; import javax.xml.bind.annotation.XmlAttribute; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlType; - import com.google.common.base.Objects; +import com.google.common.base.Objects.ToStringHelper; /** * Represents vApp instantiation parameters. @@ -205,6 +205,7 @@ public class InstantiateVAppParamsType> e } @Override + @SuppressWarnings("unchecked") public Builder fromVAppCreationParamsType(VAppCreationParamsType in) { return Builder.class.cast(super.fromVAppCreationParamsType(in)); } diff --git a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/NetworkConnection.java b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/NetworkConnection.java index 8a9c346049..f0982f92e7 100644 --- a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/NetworkConnection.java +++ b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/NetworkConnection.java @@ -20,8 +20,8 @@ package org.jclouds.vcloud.director.v1_5.domain; import static com.google.common.base.Objects.equal; +import static com.google.common.base.Preconditions.checkNotNull; -import java.util.Arrays; import java.util.List; import javax.xml.bind.annotation.XmlAttribute; @@ -29,6 +29,7 @@ import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlType; import com.google.common.base.Objects; +import com.google.common.collect.ImmutableList; /** @@ -69,15 +70,31 @@ import com.google.common.base.Objects; }) public class NetworkConnection { - public static class IpAddressAllocationMode { - public static final String POOL = "pool"; - public static final String DHCP = "dhcp"; - public static final String MANUAL = "manual"; - public static final String NONE = "none"; - - public static final List ALL = Arrays.asList( - POOL, DHCP, MANUAL, NONE - ); + public static enum IpAddressAllocationMode { + POOL("pool"), + DHCP("dhcp"), + MANUAL("manual"), + NONE("none"), + UNRECOGNIZED("unrecognized"); + + public static final List ALL = ImmutableList.of(POOL, DHCP, MANUAL, NONE); + + private final String label; + private IpAddressAllocationMode(String label) { + this.label = label; + } + + public String getLabel() { + return label; + } + + public static IpAddressAllocationMode fromValue(String value) { + try { + return valueOf(checkNotNull(value, "value").toUpperCase()); + } catch (IllegalArgumentException e) { + return UNRECOGNIZED; + } + } } public static Builder builder() { diff --git a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ReferenceType.java b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ReferenceType.java index 03eb135968..894953a9b5 100644 --- a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ReferenceType.java +++ b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ReferenceType.java @@ -18,7 +18,7 @@ */ package org.jclouds.vcloud.director.v1_5.domain; -import static com.google.common.base.Objects.*; +import static com.google.common.base.Objects.equal; import java.net.URI; import java.util.Map; @@ -27,6 +27,8 @@ import javax.xml.bind.annotation.XmlAccessType; import javax.xml.bind.annotation.XmlAccessorType; import javax.xml.bind.annotation.XmlAttribute; +import org.jclouds.logging.Logger; + import com.google.common.base.Objects; import com.google.common.base.Objects.ToStringHelper; @@ -44,6 +46,9 @@ import com.google.common.base.Objects.ToStringHelper; @XmlAccessorType(XmlAccessType.FIELD) public class ReferenceType> { + @javax.annotation.Resource + protected static Logger logger = Logger.NULL; + public static > Builder builder() { return new Builder(); } diff --git a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ResourceEntityType.java b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ResourceEntityType.java index 75ec826934..4cf1613840 100644 --- a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ResourceEntityType.java +++ b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ResourceEntityType.java @@ -19,8 +19,8 @@ package org.jclouds.vcloud.director.v1_5.domain; -import static com.google.common.base.Objects.*; -import static com.google.common.base.Preconditions.*; +import static com.google.common.base.Objects.equal; +import static com.google.common.base.Preconditions.checkNotNull; import java.net.URI; import java.util.Arrays; @@ -30,8 +30,6 @@ import javax.xml.bind.annotation.XmlAttribute; import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlType; -import org.jclouds.vcloud.director.v1_5.VCloudDirectorRuntimeException; - import com.google.common.base.Objects; import com.google.common.base.Objects.ToStringHelper; import com.google.common.base.Optional; @@ -71,7 +69,10 @@ public abstract class ResourceEntityType> extend UPLOAD_COPYING(12, "Upload initiated, copying contents.", true, false, false), UPLOAD_DISK_PENDING(13, "Upload initiated , disk contents pending.", true, false, false), UPLOAD_QUARANTINED(14, "Upload has been quarantined.", true, false, false), - UPLOAD_QUARANTINE_EXPIRED(15, "Upload quarantine period has expired.", true, false, false); + UPLOAD_QUARANTINE_EXPIRED(15, "Upload quarantine period has expired.", true, false, false), + + // Convention is "UNRECOGNIZED", but that is already a valid state name! so using UNRECOGNIZED_VALUE + UNRECOGNIZED_VALUE(404, "Unrecognized", false, false, false); private Integer value; private String description; @@ -117,7 +118,8 @@ public abstract class ResourceEntityType> extend if (found.isPresent()) { return found.get(); } else { - throw new VCloudDirectorRuntimeException(String.format("Illegal status value '%d'", value)); + logger.warn("Illegal status value '%d'", value); + return UNRECOGNIZED_VALUE; } } } diff --git a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ResourceType.java b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ResourceType.java index 6df6be3653..37e96b212d 100644 --- a/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ResourceType.java +++ b/labs/vcloud-director/src/main/java/org/jclouds/vcloud/director/v1_5/domain/ResourceType.java @@ -30,6 +30,7 @@ import javax.xml.bind.annotation.XmlElement; import javax.xml.bind.annotation.XmlType; import org.jclouds.javax.annotation.Nullable; +import org.jclouds.logging.Logger; import com.google.common.base.Objects; import com.google.common.base.Objects.ToStringHelper; @@ -51,6 +52,9 @@ import com.google.common.collect.Sets; */ @XmlType(name = "ResourceType") public class ResourceType> { + + @javax.annotation.Resource + protected static Logger logger = Logger.NULL; public NewBuilder toNewBuilder() { throw new UnsupportedOperationException("New builder not yet implemented for this class"); diff --git a/labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/domain/Checks.java b/labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/domain/Checks.java index b32ec28df0..9be9037b27 100644 --- a/labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/domain/Checks.java +++ b/labs/vcloud-director/src/test/java/org/jclouds/vcloud/director/v1_5/domain/Checks.java @@ -28,6 +28,7 @@ import java.util.UUID; import org.jclouds.vcloud.director.v1_5.VCloudDirectorMediaType; import org.jclouds.vcloud.director.v1_5.domain.CustomOrgLdapSettings.AuthenticationMechanism; import org.jclouds.vcloud.director.v1_5.domain.CustomOrgLdapSettings.ConnectorType; +import org.jclouds.vcloud.director.v1_5.domain.NetworkConnection.IpAddressAllocationMode; import org.jclouds.vcloud.director.v1_5.domain.OrgLdapSettings.LdapMode; import org.jclouds.vcloud.director.v1_5.domain.cim.ResourceAllocationSettingData; import org.jclouds.vcloud.director.v1_5.domain.cim.VirtualSystemSettingData; @@ -1052,7 +1053,8 @@ public class Checks { // Check required fields assertNotNull(val.getNetwork(), String.format(NOT_NULL_OBJECT_FMT, "Network", "NetworkConnection")); assertNotNull(val.getIpAddressAllocationMode(), String.format(NOT_NULL_OBJECT_FMT, "IpAddressAllocationMode", "NetworkConnection")); - assertTrue(NetworkConnection.IpAddressAllocationMode.ALL.contains(val.getIpAddressAllocationMode()), + IpAddressAllocationMode mode = NetworkConnection.IpAddressAllocationMode.valueOf(val.getIpAddressAllocationMode()); + assertTrue(NetworkConnection.IpAddressAllocationMode.ALL.contains(mode), String.format(REQUIRED_VALUE_OBJECT_FMT, "IpAddressAllocationMode", "NetworkConnection", val.getIpAddressAllocationMode(), Iterables.toString(NetworkConnection.IpAddressAllocationMode.ALL))); // Check optional fields