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

Make PendingResult#await throw specific Exception #238

Merged
merged 6 commits into from
Mar 22, 2017
Merged
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
29 changes: 13 additions & 16 deletions src/main/java/com/google/maps/GeoApiContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import com.google.maps.errors.OverQueryLimitException;
import com.google.maps.internal.ApiConfig;
import com.google.maps.internal.ApiResponse;
import com.google.maps.internal.ExceptionResult;
import com.google.maps.internal.ExceptionsAllowedToRetry;
import com.google.maps.internal.UrlSigner;

Expand All @@ -30,6 +29,8 @@
import java.io.UnsupportedEncodingException;
import java.net.Proxy;
import java.net.URLEncoder;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
import java.util.Map;
import java.util.concurrent.TimeUnit;

Expand Down Expand Up @@ -115,7 +116,8 @@ <T, R extends ApiResponse<T>> PendingResult<T> get(ApiConfig config, Class<? ext
try {
query.append(URLEncoder.encode(param.getValue(), "UTF-8"));
} catch (UnsupportedEncodingException e) {
return new ExceptionResult<T>(e);
// This should never happen. UTF-8 support is required for every Java implementation.
throw new IllegalStateException(e);
}
}

Expand Down Expand Up @@ -143,7 +145,8 @@ <T, R extends ApiResponse<T>> PendingResult<T> get(ApiConfig config, Class<? ext
try {
query.append(URLEncoder.encode(params[i], "UTF-8"));
} catch (UnsupportedEncodingException e) {
return new ExceptionResult<T>(e);
// This should never happen. UTF-8 support is required for every Java implementation.
throw new IllegalStateException(e);
}
}

Expand All @@ -170,12 +173,7 @@ <T, R extends ApiResponse<T>> PendingResult<T> post(ApiConfig config,
}

if (config.supportsClientId && clientId != null) {
try {
String signature = urlSigner.getSignature(url.toString());
url.append("&signature=").append(signature);
} catch (Exception e) {
return new ExceptionResult<T>(e);
}
url.append("&signature=").append(urlSigner.getSignature(url.toString()));
}

String hostName = config.hostName;
Expand Down Expand Up @@ -213,12 +211,7 @@ private <T, R extends ApiResponse<T>> PendingResult<T> getWithPath(Class<R> claz
url.append(encodedPath);

if (canUseClientId && clientId != null) {
try {
String signature = urlSigner.getSignature(url.toString());
url.append("&signature=").append(signature);
} catch (Exception e) {
return new ExceptionResult<T>(e);
}
url.append("&signature=").append(urlSigner.getSignature(url.toString()));
}

if (baseUrlOverride != null) {
Expand Down Expand Up @@ -258,7 +251,11 @@ public GeoApiContext setApiKey(String apiKey) {

public GeoApiContext setEnterpriseCredentials(String clientId, String cryptographicSecret) {
this.clientId = clientId;
this.urlSigner = new UrlSigner(cryptographicSecret);
try {
this.urlSigner = new UrlSigner(cryptographicSecret);
} catch (NoSuchAlgorithmException | InvalidKeyException e) {
throw new IllegalStateException(e);
}
return this;
}

Expand Down
6 changes: 5 additions & 1 deletion src/main/java/com/google/maps/PendingResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@

package com.google.maps;

import com.google.maps.errors.ApiException;

import java.io.IOException;

/**
* Represents a pending result from an API call.
*
Expand All @@ -33,7 +37,7 @@ public interface PendingResult<T> {
*
* @return The result.
*/
T await() throws Exception;
T await() throws ApiException, InterruptedException, IOException;

/**
* Performs the request synchronously, ignoring exceptions while performing the request and errors
Expand Down
17 changes: 11 additions & 6 deletions src/main/java/com/google/maps/PendingResultBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@

package com.google.maps;

import com.google.maps.errors.ApiException;
import com.google.maps.internal.ApiConfig;
import com.google.maps.internal.ApiResponse;
import com.google.maps.internal.StringJoin.UrlValue;

import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -51,7 +53,7 @@ public final void setCallback(Callback<T> callback) {
}

@Override
public final T await() throws Exception {
public final T await() throws ApiException, InterruptedException, IOException {
PendingResult<T> request = makeRequest();
return request.await();
}
Expand All @@ -75,12 +77,15 @@ private PendingResult<T> makeRequest() {
"'await', 'awaitIgnoreError' or 'setCallback' was already called.");
}
validateRequest();
if(config.requestVerb == "GET") {
delegate = context.get(config, responseClass, params);
} else if (config.requestVerb == "POST") {
delegate = context.post(config, responseClass, params);
switch (config.requestVerb) {
case "GET":
return delegate = context.get(config, responseClass, params);
case "POST":
return delegate = context.post(config, responseClass, params);
default:
throw new IllegalStateException(
String.format("Unexpected request method '%s'", config.requestVerb));
}
return delegate;
}

protected abstract void validateRequest();
Expand Down
48 changes: 0 additions & 48 deletions src/main/java/com/google/maps/internal/ExceptionResult.java

This file was deleted.

26 changes: 17 additions & 9 deletions src/main/java/com/google/maps/internal/GaePendingResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.maps.PendingResult;
import com.google.maps.PhotoRequest;
import com.google.maps.errors.ApiException;
import com.google.maps.errors.UnknownErrorException;
import com.google.maps.model.AddressComponentType;
import com.google.maps.model.AddressType;
import com.google.maps.model.Distance;
Expand All @@ -49,6 +50,7 @@
import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future;


Expand All @@ -68,7 +70,6 @@ public class GaePendingResult<T, R extends ApiResponse<T>>
private final Integer maxRetries;
private final ExceptionsAllowedToRetry exceptionsAllowedToRetry;

private Callback<T> callback;
private long errorTimeOut;
private int retryCounter = 0;
private long cumulativeSleepTime = 0;
Expand Down Expand Up @@ -105,12 +106,18 @@ public void setCallback(Callback<T> callback) {
}

@Override
public T await() throws Exception {
HTTPResponse response = call.get();
if (response != null) {
return parseResponse(this, response);
} else {
throw new Exception(response.getResponseCode() + " " + new String(response.getContent()));
public T await() throws ApiException, IOException, InterruptedException {
try {
return parseResponse(this, call.get());
} catch (ExecutionException e) {
if (e.getCause() instanceof IOException) {
throw (IOException) e.getCause();
} else {
// According to
// https://cloud.google.com/appengine/docs/standard/java/javadoc/com/google/appengine/api/urlfetch/URLFetchService
// all exceptions should be subclass of IOException so this should not happen.
throw new UnknownErrorException("Unexpected exception from " + e.getMessage());
}
}
}

Expand All @@ -130,7 +137,8 @@ public void cancel() {


@SuppressWarnings("unchecked")
private T parseResponse(GaePendingResult<T, R> request, HTTPResponse response) throws Exception {
private T parseResponse(GaePendingResult<T, R> request, HTTPResponse response)
throws IOException, ApiException, InterruptedException {
if (shouldRetry(response)) {
// Retry is a blocking method, but that's OK. If we're here, we're either in an await()
// call, which is blocking anyway, or we're handling a callback in a separate thread.
Expand Down Expand Up @@ -212,7 +220,7 @@ private T parseResponse(GaePendingResult<T, R> request, HTTPResponse response) t
}
}

private T retry() throws Exception {
private T retry() throws IOException, ApiException, InterruptedException {
retryCounter++;
LOG.info("Retrying request. Retry #{}",retryCounter);
this.call = client.fetchAsync(request);
Expand Down
11 changes: 6 additions & 5 deletions src/main/java/com/google/maps/internal/OkHttpPendingResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -115,23 +115,23 @@ public void setCallback(Callback<T> callback) {
private class QueuedResponse {
private final OkHttpPendingResult<T, R> request;
private final Response response;
private final Exception e;
private final IOException e;

public QueuedResponse(OkHttpPendingResult<T, R> request, Response response) {
this.request = request;
this.response = response;
this.e = null;
}

public QueuedResponse(OkHttpPendingResult<T, R> request, Exception e) {
public QueuedResponse(OkHttpPendingResult<T, R> request, IOException e) {
this.request = request;
this.response = null;
this.e = e;
}
}

@Override
public T await() throws Exception {
public T await() throws ApiException, IOException, InterruptedException {
// Handle sleeping for retried requests
if (retryCounter > 0) {
// 0.5 * (1.5 ^ i) represents an increased sleep time of 1.5x per iteration,
Expand Down Expand Up @@ -210,7 +210,8 @@ public void onResponse(Response response) throws IOException {
}

@SuppressWarnings("unchecked")
private T parseResponse(OkHttpPendingResult<T, R> request, Response response) throws Exception {
private T parseResponse(OkHttpPendingResult<T, R> request, Response response)
throws ApiException, InterruptedException, IOException {
if (shouldRetry(response)) {
// Retry is a blocking method, but that's OK. If we're here, we're either in an await()
// call, which is blocking anyway, or we're handling a callback in a separate thread.
Expand Down Expand Up @@ -284,7 +285,7 @@ private T parseResponse(OkHttpPendingResult<T, R> request, Response response) th
}
}

private T retry() throws Exception {
private T retry() throws ApiException, InterruptedException, IOException {
retryCounter++;
LOG.info("Retrying request. Retry #" + retryCounter);
this.call = client.newCall(request);
Expand Down
16 changes: 8 additions & 8 deletions src/main/java/com/google/maps/internal/UrlSigner.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@
* a client ID</a> for more detail on this protocol.
*/
public class UrlSigner {
private final SecretKeySpec key;
private static final String ALGORITHM_HMAC_SHA1 = "HmacSHA1";
private final Mac mac;

public UrlSigner(final String keyString) {
public UrlSigner(final String keyString) throws NoSuchAlgorithmException, InvalidKeyException {
// Convert from URL-safe base64 to regular base64.
String base64 = keyString.replace('-', '+').replace('_', '/');

Expand All @@ -41,17 +42,16 @@ public UrlSigner(final String keyString) {
// NOTE: don't log the exception, in case some of the private key leaks to an end-user.
throw new IllegalArgumentException("Private key is invalid.");
}
this.key = new SecretKeySpec(decodedKey.toByteArray(), "HmacSHA1");

// TODO(macd): add test
mac = Mac.getInstance(ALGORITHM_HMAC_SHA1);
mac.init(new SecretKeySpec(decodedKey.toByteArray(), ALGORITHM_HMAC_SHA1));
}

/**
* Generate url safe HmacSHA1 of a path.
*/
public String getSignature(String path)
throws NoSuchAlgorithmException, InvalidKeyException {
// TODO(macd): add test
Mac mac = Mac.getInstance("HmacSHA1");
mac.init(key);
public String getSignature(String path) {
byte[] digest = mac.doFinal(path.getBytes());
return ByteString.of(digest).base64().replace('+', '-').replace('/', '_');
}
Expand Down