You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by alibazlamit <no...@github.com> on 2016/05/27 14:51:59 UTC
[jclouds/jclouds-labs] oneandone-server-api (#275)
@baldwinSPC
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/275
-- Commit Summary --
* oneandone-server-api
-- File Changes --
A oneandone/pom.xml (142)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/OneAndOneApi.java (29)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/OneAndOneApiMetadata.java (72)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/OneAndOneProviderMetadata.java (79)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/BaseGenericDeleteRequestBinder.java (80)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/BaseOneAndOneRequestBinder.java (89)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/GenericDeleteRequestBinder.java (38)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/GenericUpdateRequestBinder.java (38)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/AddHddRequestBinder.java (63)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/AddIpFirewallPolicyRequestBinder.java (51)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/AddIpLoadBalancerRequestBinder.java (50)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/AddIpRequestBinder.java (50)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/AssignPrivateNetworkRequestBinder.java (49)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/CreateCloneRequestBinder.java (51)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/CreateFixedInstanceRequestBinder.java (78)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/CreateServerRequestBinder.java (99)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/CreateSnapshotRequestBinder.java (48)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/LoadDvdRequestBinder.java (48)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/UpdateHardwareRequestBinder.java (51)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/UpdateHddRequestBinder.java (50)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/UpdateImageRequestBinder.java (51)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/UpdateServerRequestBinder.java (53)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/binder/server/UpdateStatusRequestBinder.java (51)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/config/Authentication.java (31)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/config/OneAndOneHttpApiModule.java (45)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/config/OneAndOneProperties.java (23)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Alert.java (37)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/CriticalAlert.java (35)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Dvd.java (64)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/FixedInstanceHardware.java (31)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/GenericRequest.java (66)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Hardware.java (119)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/HardwareFlavour.java (72)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Hdd.java (127)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Image.java (85)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Licenses.java (25)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Server.java (401)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/ServerFirewallPolicy.java (67)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/ServerIp.java (79)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/ServerLoadBalancer.java (68)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/ServerMonitoringPolicy.java (33)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/ServerPrivateNetwork.java (63)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Servers.java (33)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Snapshot.java (61)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Status.java (34)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Types.java (470)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/WarningAlert.java (35)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/options/GenericQueryOptions.java (49)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/features/ServerApi.java (580)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/filters/AuthenticateRequest.java (53)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/handlers/OneAndOneHttpErrorHandler.java (67)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/refrence/AuthHeaders.java (26)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/util/ArrayAdapter.java (149)
A oneandone/src/main/java/org/apache/jclouds/oneandone/rest/util/ArrayAdapterFactory.java (43)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/OneAndOneProviderMetadataTest.java (28)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/BinderTestBase.java (95)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/AddHddRequestBinderTest.java (69)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/AddIpFirewallPolicyRequestBinderTest.java (62)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/AddIpLoadBalancerRequestBinderTest.java (63)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/AddIpRequestBinderTest.java (62)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/AssignPrivateNetworkRequestBinderTest.java (61)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/CreateCloneRequestBinderTest.java (62)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/CreateFixedInstanceRequestBinderTest.java (79)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/CreateServerRequestBinderTest.java (79)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/CreateSnapshotRequestBinderTest.java (60)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/LoadDvdRequestBinderTest.java (62)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/UpdateHardwareRequestBinderTest.java (63)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/UpdateHddRequestBinderTest.java (62)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/UpdateImageRequestBinderTest.java (62)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/UpdateServerRequestBinderTest.java (62)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/binder/server/UpdateStatusRequestBinderTest.java (63)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/features/ServerApiLiveTest.java (300)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/features/ServerApiMockTest.java (660)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/features/ServerNetworkApiLiveTest.java (155)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/features/ServerOperationsApiLiveTest.java (183)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/internal/BaseOneAndOneApiMockTest.java (121)
A oneandone/src/test/java/org/apache/jclouds/oneandone/rest/internal/BaseOneAndOneLiveTest.java (102)
A oneandone/src/test/resources/server/add.hdds.json (54)
A oneandone/src/test/resources/server/delete.json (51)
A oneandone/src/test/resources/server/get.dvd.json (4)
A oneandone/src/test/resources/server/get.flavour.json (15)
A oneandone/src/test/resources/server/get.hardware.json (13)
A oneandone/src/test/resources/server/get.hdd.json (5)
A oneandone/src/test/resources/server/get.image.json (4)
A oneandone/src/test/resources/server/get.json (48)
A oneandone/src/test/resources/server/get.privatenetwork.json (4)
A oneandone/src/test/resources/server/get.status.json (4)
A oneandone/src/test/resources/server/list.flavours.json (113)
A oneandone/src/test/resources/server/list.hardware.hdds.json (7)
A oneandone/src/test/resources/server/list.ip.firewallPolicies.json (8)
A oneandone/src/test/resources/server/list.ip.json (10)
A oneandone/src/test/resources/server/list.ip.loadBalancers.json (6)
A oneandone/src/test/resources/server/list.json (341)
A oneandone/src/test/resources/server/list.options-query-test.json (309)
A oneandone/src/test/resources/server/list.privatenetwork.json (6)
A oneandone/src/test/resources/server/list.snapshot.json (7)
A oneandone/src/test/resources/server/update.json (51)
M pom.xml (8)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/275.patch
https://github.com/jclouds/jclouds-labs/pull/275.diff
---
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/275
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + @MapBinder(BindToJsonPayload.class)
> + @ResponseParser(ServerApi.ServerParser.class)
> + Server addIp(@PathParam("serverId") String serverId, @PayloadParam("type") Types.IPType type);
> +
> + @Named("servers:ip:get")
> + @GET
> + @Path("/{serverId}/ips/{ipId}")
> + @ResponseParser(ServerApi.ServerIpParser.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + ServerIp getIp(@PathParam("serverId") String serverId, @PathParam("ipId") String ipId);
> +
> + @Named("server:ip:delete")
> + @DELETE
> + @Path("/{serverId}/ips/{ipId}")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @MapBinder(BindToJsonPayload.class)
Remove binder.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68669655
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
> +import com.google.common.reflect.TypeToken;
> +import com.google.gson.Gson;
> +import com.google.gson.TypeAdapter;
> +import com.google.gson.stream.JsonReader;
> +import com.google.gson.stream.JsonToken;
> +import com.google.gson.stream.JsonWriter;
> +import com.google.inject.TypeLiteral;
> +import java.io.IOException;
> +import java.lang.reflect.Type;
> +import java.util.ArrayList;
> +import java.util.List;
> +import java.util.Map;
> +import org.apache.jclouds.oneandone.rest.domain.ServerFirewallPolicy;
> +import org.apache.jclouds.oneandone.rest.domain.Snapshot;
> +
> +public class ArrayAdapter<T> extends TypeAdapter<List<T>> {
The SnapshotListParser uses this, the API return a single object if there is only one item, if there are many snapshots it would return a list so this Adapter solved this confusion by the API
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68857420
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + assertNotNull(fixedInstanceServer);
> + assertNotNull(fixedInstanceServer.id());
> + assertEquals(currentFlavour.hardware().vcore(), fixedInstanceServer.hardware().vcore());
> + assertEquals(currentFlavour.hardware().coresPerProcessor(), fixedInstanceServer.hardware().coresPerProcessor());
> + assertEquals(currentFlavour.hardware().ram(), fixedInstanceServer.hardware().ram());
> +
> + }
> +
> + @Test
> + public void testList() {
> +
> + servers = serverApi().getList();
> +
> + assertNotNull(servers);
> + assertFalse(servers.isEmpty());
Better verify that the server the test created exists. Otherwise this could lead to false positives if running the tests on an account with existing servers.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68670862
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @Test
> + public void testListWithOption() throws InterruptedException {
> + server.enqueue(
> + new MockResponse().setBody(stringFromResource("/server/list.options-query-test.json"))
> + );
> + GenericQueryOptions options = new GenericQueryOptions();
> + options.options(0, 0, null, "test", null);
> + List<Server> servers = serverApi().getList(options);
> +
> + assertNotNull(servers);
> + assertEquals(servers.size(), 9);
> +
> + assertEquals(server.getRequestCount(), 1);
> + assertSent(server, "GET", "/servers?q=test");
> + }
Added test for fallback in the list with options method.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68671179
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + public abstract List<Hdd> hdds();
> +
> + @SerializedNames({"fixed_instance_size_id", "vcore", "cores_per_processor", "ram", "hdds"})
> + public static Hardware create(String fixedInstanceSizeId, double vcore, double coresPerProcessor, double ram, List<Hdd> hdds) {
> + return new AutoValue_HardwareFlavour_Hardware(fixedInstanceSizeId, vcore, coresPerProcessor, ram, hdds);
> + }
> +
> + @AutoValue
> + public abstract static class Hdd {
> +
> + public abstract String unit();
> +
> + public abstract int size();
> +
> + public abstract Boolean isMain();
Apply the same criteria to all Boolean object values.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68666616
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@alibazlamit pushed 1 commit.
2aeb10a Removed DEV values
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/275/files/d4dd4d6f976d5fb524e0b443ed49a4ab29acd749..2aeb10a4b8128cd34a9a16ade5119e7b68c1be24
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + public static OSFamliyType fromValue(String v) {
> + return Enums.getIfPresent(OSFamliyType.class, v).or(UNRECOGNIZED);
> + }
> +
> + // the value which is used for matching
> + // the json node value with this enum
> + private final String value;
> +
> + OSFamliyType(final String type) {
> + value = type;
> + }
> +
> + @Override
> + public String toString() {
> + return value;
Do we need to actually "produce" the empty string as the enum value, when it is UNRECOGNIZED? If not, you can get rid of the value variable, the constructor and this method, as the default enum implementation will use the `name()` method.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68667914
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @Named("servers:create")
> + @POST
> + @ResponseParser(ServerApi.ServerParser.class)
> + Server create(@BinderParam(BindToJsonPayload.class) CreateServer server);
> +
> + @Named("servers:create")
> + @POST
> + @ResponseParser(ServerApi.ServerParser.class)
> + Server createFixedInstanceServer(@BinderParam(BindToJsonPayload.class) CreateFixedInstanceServer server);
> +
> + @Named("server:update")
> + @PUT
> + @Path("/{serverId}")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Produces("application/json")
Add this annotation to all methods that produce a body?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68669064
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
They can be inner classes. Each one must have the annotated factory method, though.
Also consider if the create/update builder pattern makes sense here. If update operations are simple ones, such as changing the names and descriptions, then it makes sense to have the api method receive those two parameters instead of an update object, and define a `@Payload` annotation to generate the body directly.
Let's look for simplicity. Builders are great for complex objects, but let's try to have a simple model matching the provider api and simple api methods.
---
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/275#issuecomment-227604932
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
Changes Updated
---
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/275#issuecomment-229993339
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @Nullable
> + public abstract String serverId();
> +
> + @Nullable
> + public abstract ImageFrequency frequency();
> +
> + public abstract int numImages();
> +
> + @Nullable
> +
> + public abstract String creationDate();
> +
> + @SerializedNames({"id", "name", "os_family", "os", "os_version", "availableSites", "architecture", "os_image_type", "type", "min_hdd_size", "licenses", "state", "description", "hdds", "server_id", "frequency", "numImages", "creation_date"})
> + public static Image create(String id, String name, OSFamliyType osFamily, OSType os, String osVersion, List<String> availableSites, int architecture, String osImageType, ImageType type, int minHddSize, List<Licenses> licenses, String state, String description, List<Hdd> hdds, String serverId, ImageFrequency frequency, int numImages, String creationDate) {
> + return new AutoValue_Image(id, name, osFamily, os, osVersion, availableSites, architecture, osImageType, type, minHddSize, licenses, state, description, hdds, serverId, frequency, numImages, creationDate);
In general it is a best practice to avoid having null collections, and helps a lot avoiding NPEs.
There is no problem in returning empty ones by default if the api does not return a value for them. But when sending requests, some apis require the values to be skipped from the json instead of sending an empty array. If empty arrays are ok in this api, then we'd better avoid having null collections and always initialize them to empty in the factory methods when the value is null.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68702235
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Produces("application/json")
> + Server update(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.UpdateServer server);
> +
> + @Named("server:Status:update")
> + @PUT
> + @Path("/{serverId}/status/action")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Produces("application/json")
> + Server updateStatus(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.UpdateStatus server);
> +
> + @Named("server:delete")
> + @DELETE
> + @Path("/{serverId}")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @MapBinder(BindToJsonPayload.class)
Oh, ok, then leave delete methods this way.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r69216023
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @Named("servers:snapshot:delete")
> + @DELETE
> + @Path("/{serverId}/snapshots/{snapshotId}")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @MapBinder(BindToJsonPayload.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + Server deleteSnapshot(@PathParam("serverId") String serverId, @PathParam("snapshotId") String snapshotId);
> +
> + @Named("servers:clone:create")
> + @POST
> + @Path("/{serverId}/clone")
> + @ResponseParser(ServerApi.ServerParser.class)
> + Server clone(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.Clone clone);
> +
> + static final class ServerListParser extends ParseJson<List<Server>> {
jclouds should be able to deserialize these objects out of the box. Can all the parsers in this class be removed, and the corresponding response parser annotations in the api methods?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68669933
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@nacx Removed all binders and did a few changes to the Mock tests as well, please take a look and let me know.
---
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/275#issuecomment-228615989
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + public abstract String id();
> +
> + public abstract String name();
> +
> + public abstract Hardware hardware();
> +
> + @SerializedNames({"id", "name", "hardware"})
> + public static HardwareFlavour create(String id, String name, Hardware hardware) {
> + return new AutoValue_HardwareFlavour(id, name, hardware);
> + }
> +
> + @AutoValue
> + public abstract static class Hardware {
> +
> + @Nullable
> + public abstract String fixedInstanceSizeId();
Oh, right. Let's keep it as-is then!
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68701865
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + @MapBinder(BindToJsonPayload.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + Server deleteHdd(@PathParam("serverId") String serverId, @PathParam("hddId") String hddId);
> +
> + @Named("servers:image:get")
> + @GET
> + @Path("/{serverId}/image")
> + @ResponseParser(ServerApi.ImageParser.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + Image getImage(@PathParam("serverId") String serverId);
> +
> + @Named("server:image:update")
> + @PUT
> + @Path("/{serverId}/image")
> + @ResponseParser(ServerApi.UpdateServerParser.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
Remove fallback.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68669618
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@alibazlamit pushed 1 commit.
d4dd4d6 PR review changes commited
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/275/files/0767d011a984011133c9e34b3389b449ca0756cd..d4dd4d6f976d5fb524e0b443ed49a4ab29acd749
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@nacx How can i keep the java standards for fields for example in the `CreateServerRequestBinder `we have `"firewallPolicyId"` that maps to `"firewall_policy_id"` from the API is there a way to make the AutoValue map it from Java standards to API property name?
---
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/275#issuecomment-227592147
Re: [jclouds/jclouds-labs] oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
Reopened #275.
---
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/275#event-674639717
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @Nullable
> + public abstract List<ServerIp> ips();
> +
> + @Nullable
> + public abstract List<Alert> alerts();
> +
> + @Nullable
> + public abstract ServerMonitoringPolicy monitoringPolicy();
> +
> + @Nullable
> + public abstract List<ServerPrivateNetwork> privateNetworks();
> +
> + @SerializedNames({"id", "name", "creation_date", "first_password", "description", "status", "hardware", "image", "dvd", "snapshot", "datacenter", "ips", "alerts", "monitoring_policy", "private_networks"})
> + public static Server create(String id, String name, Date creationDate, String firstPassword, String description, Status status, Hardware hardware, Image image, Dvd dvd, Snapshot snapshot, DataCenter datacenter, List<ServerIp> ips, List<Alert> alerts, ServerMonitoringPolicy policy, List<ServerPrivateNetwork> privateNetworks) {
> + return new AutoValue_Server(id, name, creationDate, firstPassword, description, status, hardware, image, dvd, snapshot, datacenter, ips, alerts, policy, privateNetworks);
Same comment about nullable lists.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68666906
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + @Inject
> + public AuthenticateRequest(@Provider Supplier<Credentials> splr) {
> + authToken = splr.get();
> + }
> +
> + public static String TokenBased() {
> + return authToken.identity;
> + }
> +
> + public static String TokenBased(String auth_token) {
> + return auth_token;
> + }
> +
> + @Override
> + public HttpRequest filter(HttpRequest request) throws HttpException {
> + checkNotNull(authToken.identity, "credential returned null");
Move this to the constructor.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68670117
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + assertEquals(request.getPath(), path);
> + assertEquals(request.getHeader("X-TOKEN"), AUTH_HEADER);
> + return request;
> + }
> +
> + protected RecordedRequest assertSent(MockWebServer server, String method, String path, String json)
> + throws InterruptedException {
> + RecordedRequest request = assertSent(server, method, path);
> +
> + String expectedContentType = "application/json";
> +
> + assertEquals(request.getHeader("Content-Type"), expectedContentType);
> + assertEquals(parser.parse(new String(request.getBody(), Charsets.UTF_8)), parser.parse(json));
> + return request;
> + }
> +}
Remove unused methods.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68671485
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Produces("application/json")
> + Server update(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.UpdateServer server);
> +
> + @Named("server:Status:update")
> + @PUT
> + @Path("/{serverId}/status/action")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Produces("application/json")
> + Server updateStatus(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.UpdateStatus server);
> +
> + @Named("server:delete")
> + @DELETE
> + @Path("/{serverId}")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @MapBinder(BindToJsonPayload.class)
The API asks for a content-type:applicationjson for delete methods and this does the job
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68676041
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @alibazlamit! It is now a pretty clean api! This is a really nice start. Great job!
---
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/275#issuecomment-228901632
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + public abstract List<Hdd> hdds();
> +
> + @SerializedNames({"fixed_instance_size_id", "vcore", "cores_per_processor", "ram", "hdds"})
> + public static Hardware create(String fixedInstanceSizeId, double vcore, double coresPerProcessor, double ram, List<Hdd> hdds) {
> + return new AutoValue_HardwareFlavour_Hardware(fixedInstanceSizeId, vcore, coresPerProcessor, ram, hdds);
> + }
> +
> + @AutoValue
> + public abstract static class Hdd {
> +
> + public abstract String unit();
> +
> + public abstract int size();
> +
> + public abstract Boolean isMain();
Use a primitive value if this can never be null, otherwise annotate as `@Nullable`.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68666552
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
Closed #275.
---
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/275#event-719697550
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import com.google.common.base.Supplier;
> +import com.google.inject.Inject;
> +import javax.inject.Singleton;
> +import org.apache.jclouds.oneandone.rest.refrence.AuthHeaders;
> +import org.jclouds.http.HttpException;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.http.HttpRequestFilter;
> +import org.jclouds.location.Provider;
> +import static com.google.common.base.Preconditions.checkNotNull;
> +import org.jclouds.domain.Credentials;
> +
> +@Singleton
> +public class AuthenticateRequest implements HttpRequestFilter {
> +
> + private static Credentials authToken;
Remove the static modifier and make it final.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68670056
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + public abstract double coresPerProcessor();
> +
> + public abstract double ram();
> +
> + public abstract List<Hdd> hdds();
> +
> + @SerializedNames({"vcore", "cores_per_processor", "ram", "hdds"})
> + public static Hardware create(double vcore, double coresPerProcessor, double ram, List<Hdd> hdds) {
> + return new AutoValue_Hardware(vcore, coresPerProcessor, ram, hdds);
> + }
> +
> + @AutoValue
> + public abstract static class CreateHardware {
> +
> + public abstract double vCore();
Just `vcore`, like in the `Hardware` object? Use a consistent naming.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68666310
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
I really think we should get rid of *all* binders. Looking at the remaining ones, most of them seem to have mostly direct mappings from the Json objects to the Java ones. Instead of having binders to map that, we should have a model that matches the API. It is OK if some properties are not used; if the domain model properly reflects the API, then using it will be easier for users and it will
be easier to contribute and maintain the provider too.
---
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/275#issuecomment-227589711
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
> +import org.apache.jclouds.oneandone.rest.filters.AuthenticateRequest;
> +import org.apache.jclouds.oneandone.rest.util.ArrayAdapterFactory;
> +import org.jclouds.Fallbacks;
> +import org.jclouds.http.functions.ParseJson;
> +import org.jclouds.json.Json;
> +import org.jclouds.rest.annotations.BinderParam;
> +import org.jclouds.rest.annotations.Fallback;
> +import org.jclouds.rest.annotations.MapBinder;
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.RequestFilters;
> +import org.jclouds.rest.annotations.ResponseParser;
> +import org.jclouds.rest.binders.BindToJsonPayload;
> +import org.jclouds.util.Strings2;
> +
> +@Path("servers")
> +@RequestFilters(AuthenticateRequest.class)
I did not have the need for that my guess is that the Response parser does a similar job
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68675705
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.jclouds.oneandone.rest.config;
> +
> +import java.lang.annotation.ElementType;
> +import java.lang.annotation.Retention;
> +import java.lang.annotation.RetentionPolicy;
> +import java.lang.annotation.Target;
> +import javax.inject.Qualifier;
> +
> +@Retention(value = RetentionPolicy.RUNTIME)
> +@Target(value = {ElementType.TYPE, ElementType.FIELD, ElementType.PARAMETER, ElementType.METHOD})
> +@Qualifier
> +public @interface Authentication {
> +}
Is this class actually used? If not, please remove it.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68665804
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +public class AuthenticateRequest implements HttpRequestFilter {
> +
> + private static Credentials authToken;
> +
> + @Inject
> + public AuthenticateRequest(@Provider Supplier<Credentials> splr) {
> + authToken = splr.get();
> + }
> +
> + public static String TokenBased() {
> + return authToken.identity;
> + }
> +
> + public static String TokenBased(String auth_token) {
> + return auth_token;
> + }
Remove these methods
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68670095
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
This looks great @alibazlamit! Just a couple minors left and a comment about the adapter!
---
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/275#issuecomment-229800238
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
Pushed to master as [7df28d25](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/7df28d25). Thanks for the continued effort @alibazlamit!
---
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/275#issuecomment-231949780
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + Injector injector = newBuilder().modules(modules).overrides(props).buildInjector();
> + constants = injector.getInstance(OneAndOneConstants.class);
> + Predicate<Server> serverAvailableCheck = new Predicate<Server>() {
> + @Override
> + public boolean apply(Server currentServer) {
> + Server server = api.serverApi().get(currentServer.id());
> +
> + if ((server.status().state() != Types.ServerState.POWERED_OFF
> + && server.status().state() != Types.ServerState.POWERED_ON)
> + || server.status().percent() != 0) {
> + return false;
> + } else {
> + return true;
> + }
> + }
> + };
No worries, it is OK to have it in the HttpApi module instead of in the compute one.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r69009015
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
OK Thanks ill do the change.
---
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/275#issuecomment-227606828
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
Apologies for the lack of feedback; I've been quite busy these last days and haven't had cycles to review PRs. I'll review it later today!
---
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/275#issuecomment-225551849
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Produces("application/json")
> + Server update(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.UpdateServer server);
> +
> + @Named("server:Status:update")
> + @PUT
> + @Path("/{serverId}/status/action")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Produces("application/json")
> + Server updateStatus(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.UpdateStatus server);
> +
> + @Named("server:delete")
> + @DELETE
> + @Path("/{serverId}")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @MapBinder(BindToJsonPayload.class)
Then replace it by a `@Produces` annotation. It is the way to explicitly configure the value of the content-type header.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68702439
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + assertNotNull(fixedInstanceServer);
> + assertNotNull(fixedInstanceServer.id());
> + assertEquals(currentFlavour.hardware().vcore(), fixedInstanceServer.hardware().vcore());
> + assertEquals(currentFlavour.hardware().coresPerProcessor(), fixedInstanceServer.hardware().coresPerProcessor());
> + assertEquals(currentFlavour.hardware().ram(), fixedInstanceServer.hardware().ram());
> +
> + }
> +
> + @Test
> + public void testList() {
> +
> + servers = serverApi().getList();
> +
> + assertNotNull(servers);
> + assertFalse(servers.isEmpty());
> + Assert.assertTrue(servers.size() > 0);
This is redundant.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68670799
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
Yes. The order in which fields are declared in the class should match the order in which the parameters are declared in the static factory method that creates the auto-value class. That method can be annotated with the `@SerializedNames` annotation, to provide the list, in the same order than the method parameters, of the json property names. jclouds uses that to translate between java and json.
---
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/275#issuecomment-227594874
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@nacx Removed all of the unneeded binders, i had to keep a few binders where the Json/Objects are more complex, let me know if its looks ok now.
---
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/275#issuecomment-224762334
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@alibazlamit pushed 1 commit.
5a3cf2d Minor changes
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/275/files/c9d547f1cb264ecd82a4004149a0e1f9ca72daea..5a3cf2d891aab7f4fc419e949e5acb0c46492fb5
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + IMAGES("IMAGES"), MY_IMAGE("MY_IMAGE"), PERSONAL("PERSONAL"), UNRECOGNIZED("");
> +
> + public static ImageType fromValue(String v) {
> + return Enums.getIfPresent(ImageType.class, v).or(UNRECOGNIZED);
> + }
> + // the value which is used for matching
> + // the json node value with this enum
> + private final String value;
> +
> + ImageType(final String type) {
> + value = type;
> + }
> +
> + @Override
> + public String toString() {
> + return value;
Same comment than above.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68668065
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@nacx no problem at all, I thought i needed to trigger some alert or somthing, thanks for the response.
---
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/275#issuecomment-225552682
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +@Path("servers")
> +@RequestFilters(AuthenticateRequest.class)
> +public interface ServerApi extends Closeable {
> +
> + @Named("servers:list")
> + @GET
> + @ResponseParser(ServerApi.ServerListParser.class)
> + @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
> + List<Server> getList();
> +
> + @Named("servers:list")
> + @GET
> + @ResponseParser(ServerApi.ServerListParser.class)
> + @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
> + List<Server> getList(GenericQueryOptions options);
Rename these method to just `list`.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68668711
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + Hdd getHdd(@PathParam("serverId") String serverId, @PathParam("hddId") String hddId);
> +
> + @Named("server:update")
> + @PUT
> + @Path("/{serverId}/hardware/hdds/{hddId}")
> + @MapBinder(BindToJsonPayload.class)
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + Server updateHdd(@PathParam("serverId") String serverId, @PathParam("hddId") String hddId, @PayloadParam("size") double size);
> +
> + @Named("server:hdds/delete")
> + @DELETE
> + @Path("/{serverId}/hardware/hdds/{hddId}")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @MapBinder(BindToJsonPayload.class)
There is no payload in this method. Remove this.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68669468
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @Nullable
> + public abstract String serverId();
> +
> + @Nullable
> + public abstract ImageFrequency frequency();
> +
> + public abstract int numImages();
> +
> + @Nullable
> +
> + public abstract String creationDate();
> +
> + @SerializedNames({"id", "name", "os_family", "os", "os_version", "availableSites", "architecture", "os_image_type", "type", "min_hdd_size", "licenses", "state", "description", "hdds", "server_id", "frequency", "numImages", "creation_date"})
> + public static Image create(String id, String name, OSFamliyType osFamily, OSType os, String osVersion, List<String> availableSites, int architecture, String osImageType, ImageType type, int minHddSize, List<Licenses> licenses, String state, String description, List<Hdd> hdds, String serverId, ImageFrequency frequency, int numImages, String creationDate) {
> + return new AutoValue_Image(id, name, osFamily, os, osVersion, availableSites, architecture, osImageType, type, minHddSize, licenses, state, description, hdds, serverId, frequency, numImages, creationDate);
Can we make lists non-nullable, by initialising them to an immutable empty list, if the input parameter is null? Or does the api expect that you do not send empty lists?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68666778
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Produces("application/json")
> + Server update(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.UpdateServer server);
> +
> + @Named("server:Status:update")
> + @PUT
> + @Path("/{serverId}/status/action")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Produces("application/json")
> + Server updateStatus(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.UpdateStatus server);
> +
> + @Named("server:delete")
> + @DELETE
> + @Path("/{serverId}")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @MapBinder(BindToJsonPayload.class)
This did work for the DELETE method it didn't generate he content-type header for me
My code example
```
@Named("servers:privatenetwork:delete")
@DELETE
@Path("/{serverId}/private_networks/{privateNetworkId}")
@Fallback(Fallbacks.NullOnNotFoundOr404.class)
@Produces("application/json")
Server deletePrivateNetwork(@PathParam("serverId") String serverId, @PathParam("privateNetworkId") String privateNetworkId);
```
this is what the header looks like :
`User-Agent: jclouds-okhttp/2.0.0-SNAPSHOT java/1.8.0_73
Accept: application/json
X-TOKEN: 4f34bcc24bf7bbf6af2fac5e35e600d8
Content-Length: 0
Host: cloudpanel-api.1and1.com
Connection: Keep-Alive
Accept-Encoding: gzip
`
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r69044773
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@alibazlamit pushed 2 commits.
9a4734e Changed Create Server request from Binders to Model
ddfdc4a Removed All binders
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/275/files/895f63962376a82e96b98db209cbb64f3b272b2e..ddfdc4a683cc61c3dbd089f8b9819f53cb895b16
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
Squashed Thanks @nacx moving on to the next branch.
---
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/275#issuecomment-231881099
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + Server clone(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.Clone clone);
> +
> + static final class FirewallPolicyListParser extends ParseJson<List<ServerFirewallPolicy>> {
> +
> + static final TypeLiteral<List<ServerFirewallPolicy>> list = new TypeLiteral<List<ServerFirewallPolicy>>() {
> + };
> +
> + @Inject
> + FirewallPolicyListParser(Json json) {
> + super(json, list);
> + }
> +
> + @Override
> + public <V> V apply(InputStream stream, Type type) throws IOException {
> + try {
> + Gson gson = new GsonBuilder().registerTypeAdapterFactory(new ArrayAdapterFactory()).create();
Since the ArrayAdapterFactory is now only used for these two concrete lists, would it make sense to just have two adapters, one specific for each list and remove the generic logic?
---
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/275/files/2aeb10a4b8128cd34a9a16ade5119e7b68c1be24#r69217410
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + @ResponseParser(ServerApi.ServerParser.class)
> + Server addHdd(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Hdd.CreateHddList hdds);
> +
> + @Named("servers:hardware:hdds:get")
> + @GET
> + @Path("/{serverId}/hardware/hdds/{hddId}")
> + @ResponseParser(ServerApi.HddParser.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + Hdd getHdd(@PathParam("serverId") String serverId, @PathParam("hddId") String hddId);
> +
> + @Named("server:update")
> + @PUT
> + @Path("/{serverId}/hardware/hdds/{hddId}")
> + @MapBinder(BindToJsonPayload.class)
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
Don't add 404 fallbacks to post/put methods.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68669433
Re: [jclouds/jclouds-labs] oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
fixing the issues reported
---
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/275#issuecomment-222197120
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + * 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.apache.jclouds.oneandone.rest.domain;
> +
> +import com.google.auto.value.AutoValue;
> +
> +@AutoValue
> +public abstract class Licenses {
> +
> + public abstract String name();
Add the factory method.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68666850
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + //delete fixed instance server once ready
> + assertNodeAvailable(fixedInstanceServer);
> + deleteServer(fixedInstanceServer.id());
> + }
> + if (currentServer != null) {
> + //delete currentserver once ready
> + assertNodeAvailable(currentServer);
> + deleteServer(currentServer.id());
> + }
> + }
> +
> + @Test(dependsOnMethods = "testListHardwareFlavours")
> + public void testCreateFixedInstanceServer() {
> + fixedInstanceServer = serverApi().createFixedInstanceServer(Server.CreateFixedInstanceServer.create(
> + "java test fixed instance 2", "testing with jclouds", FixedInstanceHardware.create(currentFlavour.id()), "7C5FA1D21B98DE39D7516333AAB7DA54",
> + null, "Test123!", null, Boolean.TRUE, null, null, null, null));
For constructors taking so many values, consider adding an auto-value builder to the model class.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68670769
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + Injector injector = newBuilder().modules(modules).overrides(props).buildInjector();
> + constants = injector.getInstance(OneAndOneConstants.class);
> + Predicate<Server> serverAvailableCheck = new Predicate<Server>() {
> + @Override
> + public boolean apply(Server currentServer) {
> + Server server = api.serverApi().get(currentServer.id());
> +
> + if ((server.status().state() != Types.ServerState.POWERED_OFF
> + && server.status().state() != Types.ServerState.POWERED_ON)
> + || server.status().percent() != 0) {
> + return false;
> + } else {
> + return true;
> + }
> + }
> + };
Configure this in a reusable predicate in the HttpApiModule. Take [this](https://github.com/jclouds/jclouds/blob/master/providers/digitalocean2/src/main/java/org/jclouds/digitalocean2/compute/config/DigitalOcean2ComputeServiceContextModule.java#L100-L106) as an example.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68671611
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +@RequestFilters(AuthenticateRequest.class)
> +public interface ServerApi extends Closeable {
> +
> + @Named("servers:list")
> + @GET
> + @ResponseParser(ServerApi.ServerListParser.class)
> + @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
> + List<Server> getList();
> +
> + @Named("servers:list")
> + @GET
> + @ResponseParser(ServerApi.ServerListParser.class)
> + @Fallback(Fallbacks.EmptyListOnNotFoundOr404.class)
> + List<Server> getList(GenericQueryOptions options);
> +
> + @Named("servers:Flavours:list")
Why Flavours with a capital letter? Follow the same naming in all methods.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68669532
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@alibazlamit pushed 1 commit.
bb33f99 Adapters Change
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/275/files/2aeb10a4b8128cd34a9a16ade5119e7b68c1be24..bb33f99b413728c43507730539130207dd7a9a4a
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + @Named("servers:hardware:hdds:get")
> + @GET
> + @Path("/{serverId}/hardware/hdds/{hddId}")
> + @ResponseParser(ServerApi.HddParser.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + Hdd getHdd(@PathParam("serverId") String serverId, @PathParam("hddId") String hddId);
> +
> + @Named("server:update")
> + @PUT
> + @Path("/{serverId}/hardware/hdds/{hddId}")
> + @MapBinder(BindToJsonPayload.class)
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + Server updateHdd(@PathParam("serverId") String serverId, @PathParam("hddId") String hddId, @PayloadParam("size") double size);
> +
> + @Named("server:hdds/delete")
Use the same naming convention in all methods.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68669598
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + exception = new IllegalStateException(response.getMessage(), exception);
> + break;
> + case 404:
> + case 410:
> + if (!command.getCurrentRequest().getMethod().equals("DELETE"))
> + exception = new ResourceNotFoundException(response.getMessage(), exception);
> + break;
> + case 413:
> + case 503:
> + // if nothing (default message was OK) was parsed from command executor, assume it was an 503 (Maintenance) html response.
> + if (response.getMessage().equals("OK"))
> + exception = new HttpResponseException("The OneAndOne team is currently carrying out maintenance.", command, response);
> + else
> + exception = new InsufficientResourcesException(response.getMessage(), exception);
> + break;
> + }
An exception must be set y default, otherwise we could be setting a null one if none of the codes above is returned. Let's better cover that case.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68670326
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.apache.jclouds.oneandone.rest.filters.AuthenticateRequest;
> +import org.apache.jclouds.oneandone.rest.util.ArrayAdapterFactory;
> +import org.jclouds.Fallbacks;
> +import org.jclouds.http.functions.ParseJson;
> +import org.jclouds.json.Json;
> +import org.jclouds.rest.annotations.BinderParam;
> +import org.jclouds.rest.annotations.Fallback;
> +import org.jclouds.rest.annotations.MapBinder;
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.RequestFilters;
> +import org.jclouds.rest.annotations.ResponseParser;
> +import org.jclouds.rest.binders.BindToJsonPayload;
> +import org.jclouds.util.Strings2;
> +
> +@Path("servers")
> +@RequestFilters(AuthenticateRequest.class)
And that would also allow you to remove all the response parsers, except the array one.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r69009139
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + public enum ArchitectureType {
> + Bits32(32),
> + Bits64(64);
> +
> + private int id; // Could be other data type besides int
> +
> + private ArchitectureType(int id) {
> + this.id = id;
> + }
> +
> + public int getId() {
> + return this.id;
> + }
> +
> + public static Map<Integer, ArchitectureType> buildMap() {
Is this method actually used?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68668032
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.apache.jclouds.oneandone.rest.filters.AuthenticateRequest;
> +import org.apache.jclouds.oneandone.rest.util.ArrayAdapterFactory;
> +import org.jclouds.Fallbacks;
> +import org.jclouds.http.functions.ParseJson;
> +import org.jclouds.json.Json;
> +import org.jclouds.rest.annotations.BinderParam;
> +import org.jclouds.rest.annotations.Fallback;
> +import org.jclouds.rest.annotations.MapBinder;
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.RequestFilters;
> +import org.jclouds.rest.annotations.ResponseParser;
> +import org.jclouds.rest.binders.BindToJsonPayload;
> +import org.jclouds.util.Strings2;
> +
> +@Path("servers")
> +@RequestFilters(AuthenticateRequest.class)
Should a `@Consumes(APPLICATION_JSON)` annotation be added here, to send the Accept header by default?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68669131
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +import com.google.inject.Inject;
> +import javax.inject.Singleton;
> +import org.apache.jclouds.oneandone.rest.refrence.AuthHeaders;
> +import org.jclouds.domain.Credentials;
> +import org.jclouds.http.HttpException;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.http.HttpRequestFilter;
> +import org.jclouds.location.Provider;
> +
> +@Singleton
> +public class AuthenticateRequest implements HttpRequestFilter {
> +
> + private final Credentials authToken;
> +
> + @Inject
> + private AuthenticateRequest(@Provider Supplier<Credentials> splr) {
Package private visibility [is preferred](https://github.com/google/guice/wiki/KeepConstructorsHidden) for injection constructors. Change it to just:
```java
@Inject
AuthenticateRequest(@Provider Supplier<Credentials> splr) {
```
---
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/275/files/2aeb10a4b8128cd34a9a16ade5119e7b68c1be24#r69217043
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@alibazlamit pushed 1 commit.
0767d01 fixed checkstyle errors
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/275/files/ddfdc4a683cc61c3dbd089f8b9819f53cb895b16..0767d011a984011133c9e34b3389b449ca0756cd
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
> + public abstract String id();
> +
> + public abstract String name();
> +
> + public abstract Hardware hardware();
> +
> + @SerializedNames({"id", "name", "hardware"})
> + public static HardwareFlavour create(String id, String name, Hardware hardware) {
> + return new AutoValue_HardwareFlavour(id, name, hardware);
> + }
> +
> + @AutoValue
> + public abstract static class Hardware {
> +
> + @Nullable
> + public abstract String fixedInstanceSizeId();
The Hdd object is different too
```
public abstract static class Hdd {
public abstract String unit();
public abstract int size();
public abstract Boolean isMain();
```
And at the Hardware class
```
public abstract class Hdd {
public abstract String id();
public abstract double size();
public abstract Boolean isMain();
```
I can mix them into one but i think this is neater, what do you say?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68673027
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@nacx [Example](https://github.com/StackPointCloud/jclouds-labs/blob/oneandone-server-api/oneandone/src/main/java/org/apache/jclouds/oneandone/rest/domain/Server.java#L182-L389) Just to make sure i understand you correctly you are saying that i need to get rid of all these Classes inside the Request class in the example and make each one of them a separate class with the static factory method like this `@SerializedNames({"id", "country_code", "location"})
public static DataCenter create(String id, String countryCode, String location) {
return new AutoValue_Server_DataCenter(id, countryCode, location);
}` to provide Models for POST and PUT requests.
---
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/275#issuecomment-227603053
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + public enum IPType {
> + IPV4("IPV4"),
> + IPV6("IPV6");
> + // the value which is used for matching
> + // the json node value with this enum
> + private final String value;
> +
> + IPType(final String type) {
> + value = type;
> + }
> +
> + @Override
> + public String toString() {
> + return value;
> + }
> + }
Consider adding the `fromValue` to all enums and removing the unnecessary value when it is exactly the same as the enum name.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68668257
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + + " \"method\": \"SOFTWARE\"\n"
> + + "}"
> + );
> + }
> +
> + @Test
> + public void testDeleteServer() throws InterruptedException {
> + server.enqueue(
> + new MockResponse().setBody(stringFromResource("/server/delete.json"))
> + );
> + Server response = serverApi().delete("serverId");
> +
> + assertNotNull(response);
> + assertEquals(server.getRequestCount(), 1);
> + assertSent(server, "DELETE", "/servers/serverId");
> + }
Add missing mock tests (specially the fallback ones).
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68671288
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
Thanks for the change @alibazlamit! One last thing: can you format the code to use 3 space indentation, according to our [style guide](https://cwiki.apache.org/confluence/display/JCLOUDS/Coding+Standards)? No need for another review round, if it works for you, just squash all the commits into a single one and apply the formatting change directly so we can merge it without delaying it more :)
A side note about the adapters (feel free to apply the change or not): would it make sense to create the Gson instance in the constructor, so it hasn't to be created every time an objects needs to be deserialised?
---
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/275#issuecomment-231851055
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Zack Shoylev <no...@github.com>.
> I really think we should get rid of all binders.
I second that. Binders are sometimes (very rarely) needed but should be always considered suspicious.
---
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/275#issuecomment-227602129
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
> +
> + @Nullable
> + public abstract String serverId();
> +
> + @Nullable
> + public abstract ImageFrequency frequency();
> +
> + public abstract int numImages();
> +
> + @Nullable
> +
> + public abstract String creationDate();
> +
> + @SerializedNames({"id", "name", "os_family", "os", "os_version", "availableSites", "architecture", "os_image_type", "type", "min_hdd_size", "licenses", "state", "description", "hdds", "server_id", "frequency", "numImages", "creation_date"})
> + public static Image create(String id, String name, OSFamliyType osFamily, OSType os, String osVersion, List<String> availableSites, int architecture, String osImageType, ImageType type, int minHddSize, List<Licenses> licenses, String state, String description, List<Hdd> hdds, String serverId, ImageFrequency frequency, int numImages, String creationDate) {
> + return new AutoValue_Image(id, name, osFamily, os, osVersion, availableSites, architecture, osImageType, type, minHddSize, licenses, state, description, hdds, serverId, frequency, numImages, creationDate);
The API does not require empty lists do i need to make this change?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68673484
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
> + Injector injector = newBuilder().modules(modules).overrides(props).buildInjector();
> + constants = injector.getInstance(OneAndOneConstants.class);
> + Predicate<Server> serverAvailableCheck = new Predicate<Server>() {
> + @Override
> + public boolean apply(Server currentServer) {
> + Server server = api.serverApi().get(currentServer.id());
> +
> + if ((server.status().state() != Types.ServerState.POWERED_OFF
> + && server.status().state() != Types.ServerState.POWERED_ON)
> + || server.status().percent() != 0) {
> + return false;
> + } else {
> + return true;
> + }
> + }
> + };
Is there an example for this in the HttpApiModule this is in the ComputeContextModule, maybe leave it here until we get to the compute and then added it to the ContextModule?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68864765
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + private final OneAndOneApi api;
> +
> + public PrivateNetworkReadyPredicate(OneAndOneApi api) {
> + this.api = checkNotNull(api, "api must not be null");
> + }
> +
> + @Override
> + public boolean apply(ServerPrivateNetworkRef networkRef) {
> + checkNotNull(networkRef, "ServerPrivateNetworkRef");
> + PrivateNetwork server = api.serverApi().getPrivateNetwork(networkRef.serverId(), networkRef.privateNetworkId());
> +
> + if (server.state() != Types.GenericState.ACTIVE) {
> + return false;
> + } else {
> + return true;
> + }
Just `return server.state() == Types.GenericState.ACTIVE`? Apply this to the other similar conditionals.
---
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/275/files/2aeb10a4b8128cd34a9a16ade5119e7b68c1be24#r69216777
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
I have pushed all the changes Could you please take a look at the OneAndOneHttpApiModule and let me know if that's how its supposed to be.
Thanks
---
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/275#issuecomment-229707651
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@alibazlamit pushed 1 commit.
895f639 Minor changes
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/275/files/5a3cf2d891aab7f4fc419e949e5acb0c46492fb5..895f63962376a82e96b98db209cbb64f3b272b2e
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.apache.jclouds.oneandone.rest.filters.AuthenticateRequest;
> +import org.apache.jclouds.oneandone.rest.util.ArrayAdapterFactory;
> +import org.jclouds.Fallbacks;
> +import org.jclouds.http.functions.ParseJson;
> +import org.jclouds.json.Json;
> +import org.jclouds.rest.annotations.BinderParam;
> +import org.jclouds.rest.annotations.Fallback;
> +import org.jclouds.rest.annotations.MapBinder;
> +import org.jclouds.rest.annotations.PayloadParam;
> +import org.jclouds.rest.annotations.RequestFilters;
> +import org.jclouds.rest.annotations.ResponseParser;
> +import org.jclouds.rest.binders.BindToJsonPayload;
> +import org.jclouds.util.Strings2;
> +
> +@Path("servers")
> +@RequestFilters(AuthenticateRequest.class)
If you add this annotation, jclouds will automatically use its built-in json parser to parse responses.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r69008780
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@nacx Do i need to open another ticket or change something after pushing the code in order for the changes to be reviewed?
---
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/275#issuecomment-225546798
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + }
> + }
> + };
> + serverReady = Predicates2.retry(serverAvailableCheck, constants.pollTimeout(), constants.pollPeriod(), constants.pollMaxPeriod(), TimeUnit.SECONDS);
> + return injector.getInstance(OneAndOneApi.class);
> + }
> +
> + protected Server createServer(String serverName) {
> +
> + List<Hdd.CreateHdd> hdds = new ArrayList<Hdd.CreateHdd>();
> + Hdd.CreateHdd hdd = Hdd.CreateHdd.create(30, Boolean.TRUE);
> + hdds.add(hdd);
> + Hardware.CreateHardware hardware = Hardware.CreateHardware.create(4.0, 1.0, 2.0, hdds);
> +
> + return api.serverApi().create(Server.CreateServer.create(serverName, "testing with jclouds", hardware, "81504C620D98BCEBAA5202D145203B4B", "908DC2072407C94C8054610AD5A53B8C",
> + "Test123!", null, Boolean.TRUE, null, null, null, null)
Consider using a builder here.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68671687
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Produces("application/json")
> + Server update(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.UpdateServer server);
> +
> + @Named("server:Status:update")
> + @PUT
> + @Path("/{serverId}/status/action")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @Produces("application/json")
> + Server updateStatus(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.UpdateStatus server);
> +
> + @Named("server:delete")
> + @DELETE
> + @Path("/{serverId}")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @MapBinder(BindToJsonPayload.class)
Remove this? There is no payload in this method.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68669207
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
> +
> + @Named("servers:snapshot:delete")
> + @DELETE
> + @Path("/{serverId}/snapshots/{snapshotId}")
> + @ResponseParser(ServerApi.ServerParser.class)
> + @MapBinder(BindToJsonPayload.class)
> + @Fallback(Fallbacks.NullOnNotFoundOr404.class)
> + Server deleteSnapshot(@PathParam("serverId") String serverId, @PathParam("snapshotId") String snapshotId);
> +
> + @Named("servers:clone:create")
> + @POST
> + @Path("/{serverId}/clone")
> + @ResponseParser(ServerApi.ServerParser.class)
> + Server clone(@PathParam("serverId") String serverId, @BinderParam(BindToJsonPayload.class) Server.Clone clone);
> +
> + static final class ServerListParser extends ParseJson<List<Server>> {
I get this error when removing the response parser and run the tests
`You must specify a ResponseParser annotation`
any other annotation i can use that would do the job
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68856534
Re: [jclouds/jclouds-labs] oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
Closed #275.
---
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/275#event-674638685
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +import javax.inject.Singleton;
> +import org.apache.jclouds.oneandone.rest.refrence.AuthHeaders;
> +import org.jclouds.http.HttpException;
> +import org.jclouds.http.HttpRequest;
> +import org.jclouds.http.HttpRequestFilter;
> +import org.jclouds.location.Provider;
> +import static com.google.common.base.Preconditions.checkNotNull;
> +import org.jclouds.domain.Credentials;
> +
> +@Singleton
> +public class AuthenticateRequest implements HttpRequestFilter {
> +
> + private static Credentials authToken;
> +
> + @Inject
> + public AuthenticateRequest(@Provider Supplier<Credentials> splr) {
Remove the public modifier. Injection constructors should be package private.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68669990
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + servers = serverApi().getList();
> +
> + assertNotNull(servers);
> + assertFalse(servers.isEmpty());
> + Assert.assertTrue(servers.size() > 0);
> + }
> +
> + @Test(dependsOnMethods = "testList")
> + public void testListWithOption() {
> + GenericQueryOptions options = new GenericQueryOptions();
> + options.options(0, 0, null, "test", null);
> + List<Server> serversWithQuery = serverApi().getList(options);
> +
> + assertNotNull(serversWithQuery);
> + assertFalse(serversWithQuery.isEmpty());
> + Assert.assertTrue(serversWithQuery.size() > 0);
Remove this redundant assertion and all similar ones.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68670944
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + public abstract String id();
> +
> + public abstract String name();
> +
> + public abstract Hardware hardware();
> +
> + @SerializedNames({"id", "name", "hardware"})
> + public static HardwareFlavour create(String id, String name, Hardware hardware) {
> + return new AutoValue_HardwareFlavour(id, name, hardware);
> + }
> +
> + @AutoValue
> + public abstract static class Hardware {
> +
> + @Nullable
> + public abstract String fixedInstanceSizeId();
If this is nullable, could we add it to the existing `Hardware` class and reuse it instead of having this inner one?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68666467
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + private ServerPrivateNetwork currentPrivateNetwork;
> +
> + private ServerApi serverApi() {
> +
> + return api.serverApi();
> + }
> +
> + @BeforeClass
> + public void setupTest() {
> + currentServer = createServer("jclouds operations test");
> + }
> +
> + @AfterClass(alwaysRun = true)
> + public void teardownTest() throws InterruptedException {
> + //give time for operations to finish
> + Thread.sleep(10000);
Is it possible to get rid of this?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68671371
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by alibazlamit <no...@github.com>.
@alibazlamit pushed 2 commits.
b0d58f5 Merge branch 'master' of https://github.com/jclouds/jclouds-labs into oneandone-server-api
8ca2a76 Changed indention and adapter Gson init
---
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/275/files/bb33f99b413728c43507730539130207dd7a9a4a..8ca2a76e5622e8bf3e79213c253cd0396b3bbcfb
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +import com.google.common.reflect.TypeToken;
> +import com.google.gson.Gson;
> +import com.google.gson.TypeAdapter;
> +import com.google.gson.stream.JsonReader;
> +import com.google.gson.stream.JsonToken;
> +import com.google.gson.stream.JsonWriter;
> +import com.google.inject.TypeLiteral;
> +import java.io.IOException;
> +import java.lang.reflect.Type;
> +import java.util.ArrayList;
> +import java.util.List;
> +import java.util.Map;
> +import org.apache.jclouds.oneandone.rest.domain.ServerFirewallPolicy;
> +import org.apache.jclouds.oneandone.rest.domain.Snapshot;
> +
> +public class ArrayAdapter<T> extends TypeAdapter<List<T>> {
Caan you give an example of what this is trying to deserialize? Is this class really 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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68670629
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + }
> +
> + @Test(dependsOnMethods = "testUpdateStaus")
> + public void testUpdateHardware() throws InterruptedException {
> + assertNodeAvailable(currentServer);
> +
> + Server updateResult = serverApi().updateHardware(currentServer.id(), Hardware.UpdateHardware.create(4, 2, 6));
> +
> + assertNotNull(updateResult);
> + }
> +
> + @Test(dependsOnMethods = "testAddHdds")
> + public void testListHardwareHdds() throws InterruptedException {
> + assertNodeAvailable(currentServer);
> + //give time for harddisk to be added
> + Thread.sleep(60000);
Is there a smarter way to do this?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68671040
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.jclouds.oneandone.rest.domain;
> +
> +import com.google.auto.value.AutoValue;
> +import org.jclouds.json.SerializedNames;
> +
> +@AutoValue
> +public abstract class WarningAlert {
> +
> + public abstract String type();
> +
> + public abstract String description();
> +
> + public abstract String date();
Change type to Date?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68668292
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + private void deleteIp() throws InterruptedException {
> + assertNodeAvailable(currentServer);
> +
> + Server response = serverApi().deleteIp(currentServer.id(), currentIP.id());
> +
> + assertNotNull(response);
> + }
> +
> + @Test(dependsOnMethods = "testAddIp")
> + public void testListips() throws InterruptedException {
> + List<ServerIp> ips = serverApi().listIps(currentServer.id());
> + currentIP = ips.get(0);
> + assertNotNull(ips);
> + assertFalse(ips.isEmpty());
> + Assert.assertTrue(ips.size() > 0);
Remove redundant assertions.
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68671325
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +package org.apache.jclouds.oneandone.rest.domain;
> +
> +import com.google.auto.value.AutoValue;
> +import org.jclouds.json.SerializedNames;
> +
> +@AutoValue
> +public abstract class Snapshot {
> +
> + public abstract String id();
> +
> + public abstract String creation_date();
> +
> + public abstract String deletion_date();
Change to Date and camel case?
---
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/275/files/0767d011a984011133c9e34b3389b449ca0756cd#r68667063
Re: [jclouds/jclouds-labs] JCLOUDS-1124 oneandone-server-api (#275)
Posted by Ignasi Barrera <no...@github.com>.
Thanks for the contribution @alibazlamit!
Before we start a detailed review of the code, I think we should try to address some design. Having a quick look, it seems this PR is inspired by the ProfitBricks rest one, but I'm not sure that is the best approach given the nature of the one and one api.
Most of the code in this pull request are just request binders, but as far as I see the 1&1 api, its model classes are pretty straightforward and we could easily map all them to plain AutoValue model classes, and get the jclouds default json binders just serialize and deserialize everything without the need for custom binders. Is there an special need to justify all the amount of code (and maintenance overhead) that having a specific binder for each request would introduce?
It is really important to have the simpler implementation possible, and I'd strongly suggest you refactor the model to uso AutoValue to directly map the 1&1 json model, and let jclouds default binders take care of serialization.
---
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/275#issuecomment-224406735