Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review2 #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 21 additions & 10 deletions src/main/java/com/coscale/sdk/client/ApiClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public class ApiClient {
private final String APPID;

/** API_PATH, API_VERSION for request URL. */
private final String API_PATH = "api";
private static final String API_PATH = "api";
private String API_VERSION = "v1";

/** URL path separator */
Expand Down Expand Up @@ -199,19 +199,20 @@ public int getDeserializationExceptions() {
* used by request.
* @param uri
* of the API call.
* @param data
* in string format to pass to the request.
* @param payload
* data in string format to pass to the request.
* @param authenticate
* True if the call requires authenticate, false if not.
* @return String response for the request.
* @throws IOException
*/
private String doHttpRequest(String method, String uri, String payload, boolean authenticate)
throws IOException {
URL url;
HttpURLConnection conn = null;
int responseCode = -1;

try {
url = new URL(uri);
URL url = new URL(uri);
conn = (HttpURLConnection) url.openConnection();

// Set connection timeout.
Expand Down Expand Up @@ -288,8 +289,7 @@ private void login() throws IOException {
String uri = credentials.usesToken() ? getAppRequestURL("/login/")
: getGlobalRequestURL("/users/login/");
Credentials.TokenHelper data = call("POST", uri, credentials,
new TypeReference<Credentials.TokenHelper>() {
}, false);
new TokenHelperTypeReference(), false);

token = data.token;
}
Expand Down Expand Up @@ -328,6 +328,7 @@ public <T> T callWithAuth(String method, String endpoint, Object obj, TypeRefere
} catch (CoscaleApiException e) {
if (e.statusCode == HttpURLConnection.HTTP_UNAUTHORIZED) {
this.token = null; // will trigger new login
responseCode = 401; // 401 means login failed: https://httpstatuses.com/401
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It used to be that the responseCode remained 0, which requires users to read the actual error message before knowing what's wrong.

} else {
throw e;
}
Expand All @@ -349,6 +350,8 @@ public <T> T callWithAuth(String method, String endpoint, Object obj, TypeRefere
* object with data for the request. This parameter can be null.
* @param valueType
* is the type expected.
* @param auth
* true if the call requires authentication, false if not.
* @return The Object received as a result for the request.
* @throws IOException
*/
Expand Down Expand Up @@ -410,11 +413,11 @@ public String getGlobalRequestURL(String endpoint) {
public String getRequesUrl(String endpoint, Options options, boolean globalApi) {

StringBuilder sb = new StringBuilder();
sb.append(getBaseURL());
sb.append(API_BASE_URL);
sb.append(PATH_DIV);
sb.append(getApiPath());
sb.append(API_PATH);
sb.append(PATH_DIV);
sb.append(getApiVersion());
sb.append(API_VERSION);
if (!globalApi) {
sb.append(PATH_DIV);
sb.append("app");
Expand Down Expand Up @@ -486,4 +489,12 @@ private <T> String objectToJson(T obj) throws IOException {
}
}
}

/**
* Copied from the Intellij code analysis:
* A static inner class does not keep an implicit reference to its enclosing instance.
* This prevents a common cause of memory leaks and uses less memory per instance of the class.
*/
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently a potential source of memory leaks.

private static class TokenHelperTypeReference extends TypeReference<Credentials.TokenHelper> {
}
}
11 changes: 9 additions & 2 deletions src/main/java/com/coscale/sdk/client/data/DataApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,14 @@ public DataApi(ApiClient api) {
* @throws IOException
*/
public Msg insert(String subjectId, DataInsert data) throws IOException {
return api.callWithAuth("POST", "/data/" + subjectId + '/', data, new TypeReference<Msg>() {
});
return api.callWithAuth("POST", "/data/" + subjectId + '/', data, new MsgTypeReference());
}

/**
* Copied from the Intellij code analysis:
* A static inner class does not keep an implicit reference to its enclosing instance.
* This prevents a common cause of memory leaks and uses less memory per instance of the class.
*/
private static class MsgTypeReference extends TypeReference<Msg> {
}
}
46 changes: 28 additions & 18 deletions src/main/java/com/coscale/sdk/client/events/EventsApi.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ public EventsApi(ApiClient api) {
* @throws IOException
*/
public List<Event> all() throws IOException {
return api.callWithAuth("GET", "/events/", null, new TypeReference<List<Event>>() {
});
return api.callWithAuth("GET", "/events/", null, new EventListTypeReference());
}

/**
Expand All @@ -54,8 +53,7 @@ public List<Event> all() throws IOException {
public List<Event> all(Options options) throws IOException {
String url = "/events/";
url += (options.hasQuery() ? "?" : "&") + options.query();
return api.callWithAuth("GET", url, null, new TypeReference<List<Event>>() {
});
return api.callWithAuth("GET", url, null, new EventListTypeReference());
}

/**
Expand All @@ -67,8 +65,7 @@ public List<Event> all(Options options) throws IOException {
* @throws IOException
*/
public Event get(long id) throws IOException {
return api.callWithAuth("GET", "/events/" + id + '/', null, new TypeReference<Event>() {
});
return api.callWithAuth("GET", "/events/" + id + '/', null, new EventTypeReference());
}

/**
Expand All @@ -80,8 +77,7 @@ public Event get(long id) throws IOException {
* @throws IOException
*/
public Event insert(EventInsert event) throws IOException {
return api.callWithAuth("POST", "/events/", event, new TypeReference<Event>() {
});
return api.callWithAuth("POST", "/events/", event, new EventTypeReference());
}

/**
Expand All @@ -93,8 +89,7 @@ public Event insert(EventInsert event) throws IOException {
* @throws IOException
*/
public Msg delete(long id) throws IOException {
return api.callWithAuth("DELETE", "/events/" + id + '/', null, new TypeReference<Msg>() {
});
return api.callWithAuth("DELETE", "/events/" + id + '/', null, new MsgTypeReference());
}

/** Events data end point calls. */
Expand Down Expand Up @@ -127,8 +122,7 @@ public List<EventData> allData(Long eventId, @Nullable Long start, @Nullable Lon
first = false;
}

return api.callWithAuth("GET", endpoint, null, new TypeReference<List<EventData>>() {
});
return api.callWithAuth("GET", endpoint, null, new EventDataListTypeReference());
}

/**
Expand All @@ -143,8 +137,7 @@ public List<EventData> allData(Long eventId, @Nullable Long start, @Nullable Lon
*/
public EventData getData(Long eventId, Long dataId) throws IOException {
return api.callWithAuth("GET", "/events/" + eventId + "/data/get/" + dataId + '/', null,
new TypeReference<EventData>() {
});
new EventDataTypeReference());
}

/**
Expand All @@ -159,8 +152,7 @@ public EventData getData(Long eventId, Long dataId) throws IOException {
*/
public EventData insertData(Long eventId, EventDataInsert data) throws IOException {
return api.callWithAuth("POST", "/events/" + eventId + "/data/", data,
new TypeReference<EventData>() {
});
new EventDataTypeReference());
}

/**
Expand All @@ -175,8 +167,26 @@ public EventData insertData(Long eventId, EventDataInsert data) throws IOExcepti
*/
public Msg deleteData(Long eventId, Long dataId) throws IOException {
return api.callWithAuth("DELETE", "/events/" + eventId + "/data/" + dataId + '/', null,
new TypeReference<Msg>() {
});
new MsgTypeReference());
}

/**
* Copied from the Intellij code analysis:
* A static inner class does not keep an implicit reference to its enclosing instance.
* This prevents a common cause of memory leaks and uses less memory per instance of the class.
*/
private static class EventListTypeReference extends TypeReference<List<Event>> {
}

private static class EventTypeReference extends TypeReference<Event> {
}

private static class MsgTypeReference extends TypeReference<Msg> {
}

private static class EventDataListTypeReference extends TypeReference<List<EventData>> {
}

private static class EventDataTypeReference extends TypeReference<EventData> {
}
}
7 changes: 4 additions & 3 deletions src/main/java/com/coscale/sdk/client/metrics/MetricGroup.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
import com.google.common.collect.Lists;

public class MetricGroup {

Expand Down Expand Up @@ -48,13 +49,13 @@ public MetricGroup(Long id, String source, java.util.List<Metric> metrics, Subje
java.util.List<Long> metricIds, String type, Long version) {
this.id = id;
this.source = source;
this.metrics = metrics;
this.metrics = Lists.newArrayList(metrics); // Copy the contents, don't keep reference.
this.subject = subject;
this.description = description;
this.name = name;
this.state = state;
this.metricgroups = metricgroups;
this.metricIds = metricIds;
this.metricgroups = Lists.newArrayList(metricgroups);
this.metricIds = Lists.newArrayList(metricIds);
this.type = type;
this.version = version;
}
Expand Down
Loading