nacx requested changes on this pull request.
Thanks @code4clouds!
Final comments added. Mostly related to the general comments I made about the
use of the nullable annotation and tags field. I pointed out every item that
still needs to be addressed, and added a comment about the mock tests.
> }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder singlePlacementGroup(Boolean
singlePlacementGroup);
+ public abstract Builder overProvision(Boolean overProvision);
+ public abstract Builder
upgradePolicy(VirtualMachineScaleSetUpgradePolicy upgradePolicy);
+ public abstract Builder
virtualMachineProfile(VirtualMachineScaleSetVirtualMachineProfile
virtualMachineProfile);
+
+ abstract VirtualMachineScaleSetProperties autoBuild();
This is only needed if you are going to change how the "build" method works.
You can remove it and leave just the `public abstract
VirtualMachineScaleSetProperties build();`.
> +
+ private Subnet createDefaultSubnet() {
+ server.enqueue(jsonResponse("/getonesubnet.json").setResponseCode(200));
+ final SubnetApi subnetApi = api.getSubnetApi("azurearmtesting",
"myvirtualnetwork");
+ Subnet subnet = subnetApi.get("mysubnet");
+ String path =
String.format("/subscriptions/%s/resourcegroups/%s/providers/Microsoft.Network/virtualNetworks/%s/subnets/%s?%s",
subscriptionid, resourcegroup, virtualNetwork, subnetName, apiVersion);
+ try {
+ assertSent(server, "GET", path);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ return subnet;
+ }
+
+
+}
Mock tests are intended to verify that the API methods produce the expected
requests. We test the API against a mock webserver that records the requests it
receives, and then we assert that the expected ones were generated.
The tests in this class must exercise all the methods in the API (list,
createOrUpdate, get, delete), and then validate that all produced requests are
the expected ones.
You can use the
[DiskApiMockTest](https://github.com/jclouds/jclouds-labs/blob/master/azurecompute-arm/src/test/java/org/jclouds/azurecompute/arm/features/DiskApiMockTest.java)
as a model.
> @@ -43,7 +43,7 @@
@SerializedNames({ "id", "name", "location", "tags", "properties", "etag" })
public static NetworkSecurityGroup create(final String id, final String
name, final String location,
final Map<String, String> tags, final NetworkSecurityGroupProperties
properties, final String etag) {
- return new AutoValue_NetworkSecurityGroup(id, name, location, (tags ==
null) ? null : ImmutableMap.copyOf(tags),
+ return new AutoValue_NetworkSecurityGroup(id, name, location, (tags ==
null) ? ImmutableMap.<String, String>of() : ImmutableMap.copyOf(tags),
Don't initialize to empty when null.
> +package org.jclouds.azurecompute.arm.domain;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
+import org.jclouds.javax.annotation.Nullable;
+import org.jclouds.json.SerializedNames;
+
+import java.util.List;
+
+@AutoValue
+public abstract class ExtensionProfileSettings {
+
+ /**
+ * The fileUris reference of the extension profile settings
+ */
+ @Nullable
You are enforcing presence and immutability. Remove this annotation.
> + public abstract String typeHandlerVersion();
+
+ /**
+ * The autoUpgradeMinorVersion reference of the extension properties
+ */
+ public abstract Boolean autoUpgradeMinorVersion();
+
+ /**
+ * The ExtensionProfileSettings of the extension properties
+ */
+ public abstract ExtensionProfileSettings settings();
+
+ /**
+ * The list of the protectedSettings of the extension properties
+ */
+ @Nullable
You are enforcing presence and immutability. Remove this annotation.
> public static NetworkInterfaceCardProperties create(final String
> provisioningState, final String resourceGuid,
final Boolean enableIPForwarding, final List<IpConfiguration>
ipConfigurations,
final IdReference networkSecurityGroup) {
NetworkInterfaceCardProperties.Builder builder =
NetworkInterfaceCardProperties.builder()
.provisioningState(provisioningState)
.resourceGuid(resourceGuid)
.enableIPForwarding(enableIPForwarding)
- .ipConfigurations(ipConfigurations == null ? null :
ImmutableList.copyOf(ipConfigurations))
+ .ipConfigurations(ipConfigurations == null ?
+ ImmutableList.<IpConfiguration>of() :
ImmutableList.copyOf(ipConfigurations))
Don't change the behavior from null to an empty list. Leave it null, as it is
not related to the changes in this PR.
> + /**
+ * The networkSecurityGroup of the NetworkInterfaceConfigurationProperties
+ */
+ @Nullable
+ public abstract VirtualMachineScaleSetNetworkSecurityGroup
networkSecurityGroup();
+
+ /**
+ * The dnsSettings of the NetworkInterfaceConfigurationProperties
+ */
+ @Nullable
+ public abstract VirtualMachineScaleSetDNSSettings dnsSettings();
+
+ /**
+ * The ipConfigurations of the NetworkInterfaceConfigurationProperties
+ */
+ @Nullable
You are enforcing presence and immutability. Remove this annotation.
> +import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
+import org.jclouds.javax.annotation.Nullable;
+import org.jclouds.json.SerializedNames;
+
+import java.util.List;
+
+/**
+ * VirtualMachineScaleSetDNSSettings
+ */
+@AutoValue
+public abstract class VirtualMachineScaleSetDNSSettings {
+ /**
+ * The list of DNS servers of the Virtual Machine Scale Set DNS Settings
+ */
+ @Nullable
You are enforcing presence and immutability. Remove this annotation.
> +
+ /**
+ * The subnet of the Virtual Machine Scale Set Ip Configuration Properties
+ */
+ public abstract Subnet subnet();
+
+ /**
+ * The private IP address version of the Virtual Machine Scale Set Ip
Configuration Properties
+ */
+ @Nullable
+ public abstract String privateIPAddressVersion();
+
+ /**
+ * The load balancer backend address pools of the Virtual Machine Scale Set
Ip Configuration Properties
+ */
+ @Nullable
You are enforcing presence and immutability. Remove this annotation.
> + /**
+ * The private IP address version of the Virtual Machine Scale Set Ip
Configuration Properties
+ */
+ @Nullable
+ public abstract String privateIPAddressVersion();
+
+ /**
+ * The load balancer backend address pools of the Virtual Machine Scale Set
Ip Configuration Properties
+ */
+ @Nullable
+ public abstract List<IdReference> loadBalancerBackendAddressPools();
+
+ /**
+ * The load balancer inbound nat pools of the Virtual Machine Scale Set Ip
Configuration Properties
+ */
+ @Nullable
You are enforcing presence and immutability. Remove this annotation.
> +
+ /**
+ * The provision VM Agent true of false.
+ */
+ public abstract boolean provisionVMAgent();
+
+ /**
+ * winRM
+ */
+ @Nullable
+ public abstract WinRM winRM();
+
+ /**
+ * unattend content
+ */
+ @Nullable
You are enforcing presence and immutability. Remove this annotation.
> + /**
+ * The linux configuration of the VM
+ */
+ @Nullable
+ public abstract LinuxConfiguration linuxConfiguration();
+
+ /**
+ * The windows configuration of the VM
+ */
+ @Nullable
+ public abstract WindowsConfiguration windowsConfiguration();
+
+ /**
+ * The Secrets configuration of the VM
+ */
+ @Nullable
You are enforcing presence and immutability. Remove this annotation.
> + */
+ @Nullable
+ public abstract List<Secrets> secrets();
+
+ @SerializedNames({"computerNamePrefix", "adminUsername", "adminPassword",
"linuxConfiguration",
+ "windowsConfiguration", "secrets"})
+ public static VirtualMachineScaleSetOSProfile create(final String
computerNamePrefix, final String adminUsername,
+ final String
adminPassword, final LinuxConfiguration linuxConfiguration,
+ final
WindowsConfiguration windowsConfiguration, final List<Secrets> secrets) {
+ return builder()
+ .computerNamePrefix(computerNamePrefix)
+ .adminUsername(adminUsername)
+ .adminPassword(adminPassword)
+ .linuxConfiguration(linuxConfiguration)
+ .windowsConfiguration(windowsConfiguration)
+ .secrets(secrets == null ? ImmutableList.<Secrets>of() :
ImmutableList.copyOf(secrets) )
Move this logic to the builder itself.
> + }
+
+ public abstract Builder toBuilder();
+
+ public static Builder builder() {
+ return new
AutoValue_VirtualMachineScaleSetPublicIPAddressConfiguration.Builder();
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ public abstract Builder name(String name);
+
+ public abstract Builder
properties(VirtualMachineScaleSetPublicIPAddressProperties properties);
+
+ abstract VirtualMachineScaleSetPublicIPAddressConfiguration autoBuild();
Same here; this method is not needed.
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/412#pullrequestreview-71755450
