nacx requested changes on this pull request.
Thanks @andreaturli! This is taking shape! :)
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jclouds.packet.compute.utils;
+
+import java.net.URI;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+public class URIs {
+
+ public static String toId(URI uri) {
+ checkNotNull(uri, "uri");
+ String path = uri.getPath();
+ return path.substring(path.lastIndexOf('/') + 1);
What about URIs Just like `http://packet.net`? Let's better validate the input
to propagate an appropriate error and add a unit test class for this.
> + @Override
+ protected IterableWithMarker<Device>
fetchPageUsingOptions(ListOptions options, Optional<Object> arg0) {
+ return api.deviceApi((String) arg0.get()).list(options);
+ }
+ }
+ }
+
+ @Named("device:create")
+ @POST
+ @Produces(MediaType.APPLICATION_JSON)
+ @MapBinder(BindToJsonPayload.class)
+ @ResponseParser(URIParser.class)
+ URI create(@PayloadParam("hostname") String hostname,
@PayloadParam("plan") String plan,
+ @PayloadParam("billing_cycle") String billingCycle,
@PayloadParam("facility") String facility,
+ @PayloadParam("features") Map<String, String> features,
@PayloadParam("operating_system") String operatingSystem,
+ @PayloadParam("locked") boolean locked,
@PayloadParam("userdata") String userdata, @PayloadParam("tags") Set<String>
tags);
For methods with such an amount of parameters, consider creating an object
instead.
> + @ConstructorProperties({ "devices", "meta" })
+ public Devices(List<Device> items, Meta meta) {
+ super(items, meta);
+ }
+ }
+
+ private static class ToPagedIterable extends
BaseToPagedIterable<Device, ListOptions> {
+
+ @Inject ToPagedIterable(PacketApi api, Function<Href, ListOptions>
linkToOptions) {
+ super(api, linkToOptions);
+ }
+
+ @Override
+ protected List<Object> getArgs(GeneratedHttpRequest request) {
+ return request.getCaller().get().getArgs();
+ }
Do we need to override this?
> +import org.jclouds.rest.annotations.PayloadParam;
+import org.jclouds.rest.annotations.RequestFilters;
+import org.jclouds.rest.annotations.ResponseParser;
+import org.jclouds.rest.annotations.Transform;
+import org.jclouds.rest.binders.BindToJsonPayload;
+
+import com.google.common.base.Function;
+import com.google.common.base.Optional;
+import com.google.inject.TypeLiteral;
+
+@Path("/ssh-keys")
+@Consumes(MediaType.APPLICATION_JSON)
+@RequestFilters({ AddXAuthTokenToRequest.class, AddApiVersionToRequest.class} )
+public interface SshKeyApi {
+
+ @Named("sshKey:list")
Use lowercase naming like in the other apis.
> + protected List<Object> getArgs(GeneratedHttpRequest request) {
+ return request.getCaller().get().getArgs();
+ }
+
+ @Override
+ protected IterableWithMarker<Device>
fetchPageUsingOptions(ListOptions options, Optional<Object> arg0) {
+ return api.deviceApi((String) arg0.get()).list(options);
+ }
+ }
+ }
+
+ @Named("device:create")
+ @POST
+ @Produces(MediaType.APPLICATION_JSON)
+ @MapBinder(BindToJsonPayload.class)
+ @ResponseParser(URIParser.class)
Use the existing
[ParseURIFromListOrLocationHeaderIf20x](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/http/functions/ParseURIFromListOrLocationHeaderIf20x.java)
instead?
> + assertEquals(size(devices), 5);
+ assertEquals(server.getRequestCount(), 1);
+
+ assertSent(server, "GET",
"/projects/93907f48-adfe-43ed-ad89-0e6e83721a54/devices?page=1&per_page=5");
+ }
+
+ public void testListDevicesWithOptionsReturns404() throws
InterruptedException {
+ server.enqueue(response404());
+
+ Iterable<Device> actions =
api.deviceApi("93907f48-adfe-43ed-ad89-0e6e83721a54").list(page(1).perPage(5));
+
+ assertTrue(isEmpty(actions));
+
+ assertEquals(server.getRequestCount(), 1);
+ assertSent(server, "GET",
"/projects/93907f48-adfe-43ed-ad89-0e6e83721a54/devices?page=1&per_page=5");
+ }
Add the mock tests for the remaining methods in the DeviceApi.
> +import static org.testng.util.Strings.isNullOrEmpty;
+
+@Test(groups = "live", testName = "SshKeyApiLiveTest")
+public class SshKeyApiLiveTest extends BasePacketApiLiveTest {
+
+ public void testListSshKeys() {
+ final AtomicInteger found = new AtomicInteger(0);
+ assertTrue(Iterables.all(api().list().concat(), new Predicate<SshKey>() {
+ @Override
+ public boolean apply(SshKey input) {
+ found.incrementAndGet();
+ return !isNullOrEmpty(input.id());
+ }
+ }), "All ssh keys must have the 'id' field populated");
+ assertTrue(found.get() > 0, "Expected some ssh keys to be returned");
+ }
Add the live tests for the create, get and delete operations.
> + assertEquals(size(actions), 5);
+ assertEquals(server.getRequestCount(), 1);
+
+ assertSent(server, "GET", "/ssh-keys?page=1&per_page=5");
+ }
+
+ public void testListSshKeysWithOptionsReturns404() throws
InterruptedException {
+ server.enqueue(response404());
+
+ Iterable<SshKey> actions = api.sshKeyApi().list(page(1).perPage(5));
+
+ assertTrue(isEmpty(actions));
+
+ assertEquals(server.getRequestCount(), 1);
+ assertSent(server, "GET", "/ssh-keys?page=1&per_page=5");
+ }
Add the tests for the remaining methods in 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/345#pullrequestreview-17313562
