nacx requested changes on this pull request.
Thanks @alibazlamit!
> + @SerializedNames({"resources", "ports", "process"})
+ public static Alerts create(Resources resources, Ports ports, Process
process) {
+ return new AutoValue_MonitoringCenter_Alerts(resources, ports,
process);
+ }
+ }
+
+ @AutoValue
+ public abstract static class Agent {
+
+ public abstract boolean agentinstalled();
+
+ public abstract boolean monitoringNeedsAgent();
+
+ public abstract boolean missingAgentAlert();
+
+ @SerializedNames({"agent_installed", "monitoring_needs_agent",
"missingAgentAlert"})
Last parameter should be `missing_agent_alert` according tot he docs.
> +
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.util.Date;
+import java.util.TimeZone;
+import org.apache.jclouds.oneandone.rest.domain.Types.CustomPeriodType;
+import org.apache.jclouds.oneandone.rest.domain.Types.PeriodType;
+import org.jclouds.http.options.BaseHttpRequestOptions;
+
+public class GenericDateQueryOptions extends BaseHttpRequestOptions {
+
+ public static final String PERIOD = "period";
+ public static final String STARTDATE = "start_date";
+ public static final String ENDDATE = "end_date";
+
+ public GenericDateQueryOptions CustomPeriod(Date startDate, Date endDate) {
Method names should start lowercase
> + queryParameters.put(ENDDATE, FormatDateToISO8601(endDate));
+ return this;
+ }
+
+ public GenericDateQueryOptions FixedPeriods(PeriodType period) {
+
+ queryParameters.put(PERIOD, period.toString());
+ return this;
+ }
+
+ private String FormatDateToISO8601(Date date) {
+ TimeZone tz = TimeZone.getTimeZone("UTC");
+ DateFormat df = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss");
+ df.setTimeZone(tz);
+ return df.format(date).concat("Z");
+ }
It would be better to use the jclouds
[DateService](https://github.com/jclouds/jclouds/blob/master/core/src/main/java/org/jclouds/date/DateService.java)
to format the dates.
> +import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+@Test(groups = "live", testName = "MonitoringCenterApiLiveTest")
+public class MonitoringCenterApiLiveTest extends BaseOneAndOneLiveTest {
+
+ private Server currentServer;
+
+ private MonitoringCenterApi monitoringCenterApi() {
+
+ return api.monitoringCenterApi();
+ }
+
+ @BeforeClass
+ public void setupTest() throws InterruptedException {
+ currentServer = GetRandomServerWithMonitoringPolicy();
This method can return null. It would be better to throw a `SkipException` here
to avoid running the tests in this class if no server is available.
Would it make more sense to just create one server with the needed info, and
runt he tests with that one?
> + private MonitoringCenterApi monitoringCenterApi() {
+
+ return api.monitoringCenterApi();
+ }
+
+ @BeforeClass
+ public void setupTest() throws InterruptedException {
+ currentServer = GetRandomServerWithMonitoringPolicy();
+ assertNodeAvailable(currentServer);
+ }
+
+ @Test
+ public void testList() {
+ List<MonitoringCenter> result = monitoringCenterApi().list();
+
+ assertNotNull(result);
Verify that at least returns one element?
> + }
+
+ @Test
+ public void testList() {
+ List<MonitoringCenter> result = monitoringCenterApi().list();
+
+ assertNotNull(result);
+ }
+
+ @Test
+ public void testListWithOption() {
+ GenericQueryOptions options = new GenericQueryOptions();
+ options.options(1, 1, null, null, null);
+ List<MonitoringCenter> resultWithQuery =
monitoringCenterApi().list(options);
+
+ assertNotNull(resultWithQuery);
Same here, verify that at least one element is returned?
> @@ -202,4 +202,20 @@ public static VPNType fromValue(String v) {
return Enums.getIfPresent(VPNType.class, v).or(UNRECOGNIZED);
}
}
+
+ public enum PeriodType {
+ LAST_HOUR, LAST_24H, LAST_7D, LAST_30D, LAST_365D, UNRECOGNIZED;
+
+ public static PeriodType fromValue(String v) {
+ return Enums.getIfPresent(PeriodType.class, v).or(UNRECOGNIZED);
+ }
+ }
+
+ public enum CustomPeriodType {
+ CUSTOM, UNRECOGNIZED;
Wouldn't it make more sense to have the `CUSTOM` value being part of the
`PeriodType` enum, and remove this one?
> + public void testGetCustomPeriod() throws InterruptedException,
> ParseException {
+ server.enqueue(
+ new
MockResponse().setBody(stringFromResource("/monitoringcenters/get.json"))
+ );
+ GenericDateQueryOptions options = new GenericDateQueryOptions();
+ String startStr = "11-11-2012";
+ String endStr = "11-11-2013";
+ DateFormat dateFormat = new SimpleDateFormat("dd-MM-yyyy");
+ Date end = dateFormat.parse(endStr);
+ Date start = dateFormat.parse(startStr);
+ options.CustomPeriod(start, end);
+ MonitoringCenter result = monitoringCenterApi().get("serverId", options);
+
+ assertNotNull(result);
+ assertEquals(server.getRequestCount(), 1);
+ assertSent(server, "GET",
"/monitoring_center/serverId?period=CUSTOM&start_date=" +
URLEncoder.encode(FormatDateToISO8601(start)) + "&end_date=" +
URLEncoder.encode(FormatDateToISO8601(end)));
Put the expected date as a string literal, to make sure we are serializing the
date as expected.
> +
+ public void testGetCustomPeriod404() throws InterruptedException,
ParseException {
+ server.enqueue(
+ new MockResponse().setResponseCode(404));
+ GenericDateQueryOptions options = new GenericDateQueryOptions();
+ String startStr = "11-11-2012";
+ String endStr = "11-11-2013";
+ DateFormat dateFormat = new SimpleDateFormat("dd-MM-yyyy");
+ Date end = dateFormat.parse(endStr);
+ Date start = dateFormat.parse(startStr);
+ options.CustomPeriod(start, end);
+ MonitoringCenter result = monitoringCenterApi().get("serverId", options);
+
+ assertEquals(result, null);
+ assertEquals(server.getRequestCount(), 1);
+ assertSent(server, "GET",
"/monitoring_center/serverId?period=CUSTOM&start_date=" +
URLEncoder.encode(FormatDateToISO8601(start)) + "&end_date=" +
URLEncoder.encode(FormatDateToISO8601(end)));
Same here about expected dates.
> @@ -107,4 +111,11 @@ protected RecordedRequest assertSent(MockWebServer
> server, String method, String
assertEquals(parser.parse(new String(request.getBody(),
Charsets.UTF_8)), parser.parse(json));
return request;
}
+
+ protected String FormatDateToISO8601(Date date) {
+ TimeZone tz = TimeZone.getTimeZone("UTC");
+ DateFormat df = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss");
+ df.setTimeZone(tz);
+ return df.format(date).concat("Z");
+ }
Same as before, better use the DateService. You can get it from the Guice
injector.
> @@ -143,4 +143,17 @@ protected Server turnOnServer(String serverId) {
protected Server turnOFFServer(String serverId) {
return api.serverApi().updateStatus(serverId,
Server.UpdateStatus.create(Types.ServerAction.POWER_OFF,
Types.ServerActionMethod.SOFTWARE));
}
+
+ protected Server GetRandomServerWithMonitoringPolicy() throws
InterruptedException {
+ List<Server> result = api.serverApi().list();
+ for (Server server : result) {
+ Thread.sleep(2000);
Is this sleep really needed?
> @@ -143,4 +143,17 @@ protected Server turnOnServer(String serverId) {
protected Server turnOFFServer(String serverId) {
return api.serverApi().updateStatus(serverId,
Server.UpdateStatus.create(Types.ServerAction.POWER_OFF,
Types.ServerActionMethod.SOFTWARE));
}
+
+ protected Server GetRandomServerWithMonitoringPolicy() throws
InterruptedException {
+ List<Server> result = api.serverApi().list();
+ for (Server server : result) {
+ Thread.sleep(2000);
+ server = api.serverApi().get(server.id());
+ if (server.monitoringPolicy() != null) {
+ return server;
+ }
+ }
+
+ return null;
If no server can be found, just create one for this testing? Or change the test
class to always create one for 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/322#pullrequestreview-3286571
