From db1b73644686b5f2b93be12eb5b4f33e4950119d Mon Sep 17 00:00:00 2001 From: Jatin Matani Date: Mon, 29 Sep 2014 17:16:30 -0700 Subject: [PATCH] Changes related Sync Engine library that communicates to cloud server This change updates some of the code that the library uses to communicate with the cloud server Specifically : - Update BlockingHttpClient to use template ResponseProcessor - Update HttpUrlConnectionBuilder to setAuthToken for HttpUrlConnection Bug: 17464069 Bug: 17464805 Change-Id: I7d7b58dff594b920162f799d102e8582ff9fe6a4 --- .../latin/network/AuthException.java | 35 +++++++++ .../latin/network/BlockingHttpClient.java | 34 ++++---- .../latin/network/HttpException.java | 46 +++++++++++ .../network/HttpUrlConnectionBuilder.java | 34 +++++--- .../network/BlockingHttpClientTests.java | 78 +++++++++---------- .../HttpUrlConnectionBuilderTests.java | 9 +++ 6 files changed, 170 insertions(+), 66 deletions(-) create mode 100644 java/src/com/android/inputmethod/latin/network/AuthException.java create mode 100644 java/src/com/android/inputmethod/latin/network/HttpException.java diff --git a/java/src/com/android/inputmethod/latin/network/AuthException.java b/java/src/com/android/inputmethod/latin/network/AuthException.java new file mode 100644 index 000000000..1bce4c156 --- /dev/null +++ b/java/src/com/android/inputmethod/latin/network/AuthException.java @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * 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 com.android.inputmethod.latin.network; + +/** + * Authentication exception. When this exception is thrown, the client may + * try to refresh the authentication token and try again. + */ +public class AuthException extends Exception { + public AuthException() { + super(); + } + + public AuthException(Throwable throwable) { + super(throwable); + } + + public AuthException(String detailMessage) { + super(detailMessage); + } +} \ No newline at end of file diff --git a/java/src/com/android/inputmethod/latin/network/BlockingHttpClient.java b/java/src/com/android/inputmethod/latin/network/BlockingHttpClient.java index 0d0cbe169..5e86d94cd 100644 --- a/java/src/com/android/inputmethod/latin/network/BlockingHttpClient.java +++ b/java/src/com/android/inputmethod/latin/network/BlockingHttpClient.java @@ -16,6 +16,8 @@ package com.android.inputmethod.latin.network; +import android.util.Log; + import com.android.inputmethod.annotations.UsedForTesting; import java.io.BufferedOutputStream; @@ -35,20 +37,15 @@ import javax.annotation.Nullable; */ @UsedForTesting public class BlockingHttpClient { + private static final boolean DEBUG = false; + private static final String TAG = BlockingHttpClient.class.getSimpleName(); + private final HttpURLConnection mConnection; /** * Interface that handles processing the response for a request. */ - public interface ResponseProcessor { - /** - * Called when the HTTP request fails with an error. - * - * @param httpStatusCode The status code of the HTTP response. - * @param message The HTTP response message, if any, or null. - */ - void onError(int httpStatusCode, @Nullable String message); - + public interface ResponseProcessor { /** * Called when the HTTP request finishes successfully. * The {@link InputStream} is closed by the client after the method finishes, @@ -56,7 +53,7 @@ public class BlockingHttpClient { * * @param response An input stream that can be used to read the HTTP response. */ - void onSuccess(InputStream response); + T onSuccess(InputStream response) throws IOException; } /** @@ -73,11 +70,11 @@ public class BlockingHttpClient { * TODO: Remove @UsedForTesting after this is actually used. * * @param request The request payload, if any, or null. - * @param responeProcessor A processor for the HTTP response. + * @param responseProcessor A processor for the HTTP response. */ @UsedForTesting - public void execute(@Nullable byte[] request, @Nonnull ResponseProcessor responseProcessor) - throws IOException { + public T execute(@Nullable byte[] request, @Nonnull ResponseProcessor responseProcessor) + throws IOException, AuthException, HttpException { try { if (request != null) { OutputStream out = new BufferedOutputStream(mConnection.getOutputStream()); @@ -88,9 +85,16 @@ public class BlockingHttpClient { final int responseCode = mConnection.getResponseCode(); if (responseCode != HttpURLConnection.HTTP_OK) { - responseProcessor.onError(responseCode, mConnection.getResponseMessage()); + if (DEBUG) { + Log.d(TAG, "Response error: " + responseCode + ", Message: " + + mConnection.getResponseMessage()); + } + if (responseCode == HttpURLConnection.HTTP_UNAUTHORIZED) { + throw new AuthException(mConnection.getResponseMessage()); + } + throw new HttpException(responseCode); } else { - responseProcessor.onSuccess(mConnection.getInputStream()); + return responseProcessor.onSuccess(mConnection.getInputStream()); } } finally { mConnection.disconnect(); diff --git a/java/src/com/android/inputmethod/latin/network/HttpException.java b/java/src/com/android/inputmethod/latin/network/HttpException.java new file mode 100644 index 000000000..b9d8b6372 --- /dev/null +++ b/java/src/com/android/inputmethod/latin/network/HttpException.java @@ -0,0 +1,46 @@ +/* + * Copyright (C) 2014 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * 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 com.android.inputmethod.latin.network; + +import com.android.inputmethod.annotations.UsedForTesting; + +/** + * The HttpException exception represents a XML/HTTP fault with a HTTP status code. + */ +public class HttpException extends Exception { + + /** + * The HTTP status code. + */ + private final int mStatusCode; + + /** + * @param statusCode int HTTP status code. + */ + public HttpException(int statusCode) { + super("Response Code: " + statusCode); + mStatusCode = statusCode; + } + + /** + * @return the HTTP status code related to this exception. + */ + @UsedForTesting + public int getHttpStatusCode() { + return mStatusCode; + } +} \ No newline at end of file diff --git a/java/src/com/android/inputmethod/latin/network/HttpUrlConnectionBuilder.java b/java/src/com/android/inputmethod/latin/network/HttpUrlConnectionBuilder.java index 35b65be56..502f72f17 100644 --- a/java/src/com/android/inputmethod/latin/network/HttpUrlConnectionBuilder.java +++ b/java/src/com/android/inputmethod/latin/network/HttpUrlConnectionBuilder.java @@ -36,6 +36,11 @@ import java.util.Map.Entry; public class HttpUrlConnectionBuilder { private static final int DEFAULT_TIMEOUT_MILLIS = 5 * 1000; + /** + * Request header key for authentication. + */ + public static final String HTTP_HEADER_AUTHORIZATION = "Authorization"; + /** * Request header key for cache control. */ @@ -78,7 +83,7 @@ public class HttpUrlConnectionBuilder { * Sets the URL that'll be used for the request. * This *must* be set before calling {@link #build()} * - * TODO: Remove @UsedForTesting after this is actually used. + * TODO: Remove @UsedForTesting after this method is actually used. */ @UsedForTesting public HttpUrlConnectionBuilder setUrl(String url) throws MalformedURLException { @@ -92,7 +97,7 @@ public class HttpUrlConnectionBuilder { /** * Sets the connect timeout. Defaults to {@value #DEFAULT_TIMEOUT} milliseconds. * - * TODO: Remove @UsedForTesting after this is actually used. + * TODO: Remove @UsedForTesting after this method is actually used. */ @UsedForTesting public HttpUrlConnectionBuilder setConnectTimeout(int timeoutMillis) { @@ -107,7 +112,7 @@ public class HttpUrlConnectionBuilder { /** * Sets the read timeout. Defaults to {@value #DEFAULT_TIMEOUT} milliseconds. * - * TODO: Remove @UsedForTesting after this is actually used. + * TODO: Remove @UsedForTesting after this method is actually used. */ @UsedForTesting public HttpUrlConnectionBuilder setReadTimeout(int timeoutMillis) { @@ -122,7 +127,7 @@ public class HttpUrlConnectionBuilder { /** * Adds an entry to the request header. * - * TODO: Remove @UsedForTesting after this is actually used. + * TODO: Remove @UsedForTesting after this method is actually used. */ @UsedForTesting public HttpUrlConnectionBuilder addHeader(String key, String value) { @@ -130,11 +135,22 @@ public class HttpUrlConnectionBuilder { return this; } + /** + * Sets an authentication token. + * + * TODO: Remove @UsedForTesting after this method is actually used. + */ + @UsedForTesting + public HttpUrlConnectionBuilder setAuthToken(String value) { + mHeaderMap.put(HTTP_HEADER_AUTHORIZATION, value); + return this; + } + /** * Sets the request to be executed such that the input is not buffered. * This may be set when the request size is known beforehand. * - * TODO: Remove @UsedForTesting after this is actually used. + * TODO: Remove @UsedForTesting after this method is actually used. */ @UsedForTesting public HttpUrlConnectionBuilder setFixedLengthForStreaming(int length) { @@ -145,7 +161,7 @@ public class HttpUrlConnectionBuilder { /** * Indicates if the request can use cached responses or not. * - * TODO: Remove @UsedForTesting after this is actually used. + * TODO: Remove @UsedForTesting after this method is actually used. */ @UsedForTesting public HttpUrlConnectionBuilder setUseCache(boolean useCache) { @@ -161,7 +177,7 @@ public class HttpUrlConnectionBuilder { * @see #MODE_DOWNLOAD_ONLY * @see #MODE_BI_DIRECTIONAL * - * TODO: Remove @UsedForTesting after this is actually used. + * TODO: Remove @UsedForTesting after this method is actually used */ @UsedForTesting public HttpUrlConnectionBuilder setMode(int mode) { @@ -177,7 +193,7 @@ public class HttpUrlConnectionBuilder { /** * Builds the {@link HttpURLConnection} instance that can be used to execute the request. * - * TODO: Remove @UsedForTesting after this is actually used. + * TODO: Remove @UsedForTesting after this method is actually used. */ @UsedForTesting public HttpURLConnection build() throws IOException { @@ -210,4 +226,4 @@ public class HttpUrlConnectionBuilder { } return connection; } -} +} \ No newline at end of file diff --git a/tests/src/com/android/inputmethod/latin/network/BlockingHttpClientTests.java b/tests/src/com/android/inputmethod/latin/network/BlockingHttpClientTests.java index d151732aa..fed8be920 100644 --- a/tests/src/com/android/inputmethod/latin/network/BlockingHttpClientTests.java +++ b/tests/src/com/android/inputmethod/latin/network/BlockingHttpClientTests.java @@ -16,8 +16,8 @@ package com.android.inputmethod.latin.network; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.eq; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -53,41 +53,52 @@ public class BlockingHttpClientTests extends AndroidTestCase { MockitoAnnotations.initMocks(this); } - public void testError_badGateway() throws IOException { + public void testError_badGateway() throws IOException, AuthException { when(mMockHttpConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_BAD_GATEWAY); final BlockingHttpClient client = new BlockingHttpClient(mMockHttpConnection); - final FakeErrorResponseProcessor processor = - new FakeErrorResponseProcessor(HttpURLConnection.HTTP_BAD_GATEWAY); + final FakeErrorResponseProcessor processor = new FakeErrorResponseProcessor(); - client.execute(null /* empty request */, processor); - assertTrue("ResponseProcessor was not invoked", processor.mInvoked); + try { + client.execute(null /* empty request */, processor); + fail("Expecting an HttpException"); + } catch (HttpException e) { + // expected HttpException + assertEquals(HttpURLConnection.HTTP_BAD_GATEWAY, e.getHttpStatusCode()); + } } - public void testError_clientTimeout() throws IOException { + public void testError_clientTimeout() throws Exception { when(mMockHttpConnection.getResponseCode()).thenReturn( HttpURLConnection.HTTP_CLIENT_TIMEOUT); final BlockingHttpClient client = new BlockingHttpClient(mMockHttpConnection); - final FakeErrorResponseProcessor processor = - new FakeErrorResponseProcessor(HttpURLConnection.HTTP_CLIENT_TIMEOUT); + final FakeErrorResponseProcessor processor = new FakeErrorResponseProcessor(); - client.execute(null /* empty request */, processor); - assertTrue("ResponseProcessor was not invoked", processor.mInvoked); + try { + client.execute(null /* empty request */, processor); + fail("Expecting an HttpException"); + } catch (HttpException e) { + // expected HttpException + assertEquals(HttpURLConnection.HTTP_CLIENT_TIMEOUT, e.getHttpStatusCode()); + } } - public void testError_forbiddenWithRequest() throws IOException { + public void testError_forbiddenWithRequest() throws Exception { final OutputStream mockOutputStream = Mockito.mock(OutputStream.class); when(mMockHttpConnection.getResponseCode()).thenReturn(HttpURLConnection.HTTP_FORBIDDEN); when(mMockHttpConnection.getOutputStream()).thenReturn(mockOutputStream); final BlockingHttpClient client = new BlockingHttpClient(mMockHttpConnection); - final FakeErrorResponseProcessor processor = - new FakeErrorResponseProcessor(HttpURLConnection.HTTP_FORBIDDEN); + final FakeErrorResponseProcessor processor = new FakeErrorResponseProcessor(); - client.execute(new byte[100], processor); + try { + client.execute(new byte[100], processor); + fail("Expecting an HttpException"); + } catch (HttpException e) { + assertEquals(HttpURLConnection.HTTP_FORBIDDEN, e.getHttpStatusCode()); + } verify(mockOutputStream).write(any(byte[].class), eq(0), eq(100)); - assertTrue("ResponseProcessor was not invoked", processor.mInvoked); } - public void testSuccess_emptyRequest() throws IOException { + public void testSuccess_emptyRequest() throws Exception { final Random rand = new Random(); byte[] response = new byte[100]; rand.nextBytes(response); @@ -101,7 +112,7 @@ public class BlockingHttpClientTests extends AndroidTestCase { assertTrue("ResponseProcessor was not invoked", processor.mInvoked); } - public void testSuccess() throws IOException { + public void testSuccess() throws Exception { final OutputStream mockOutputStream = Mockito.mock(OutputStream.class); final Random rand = new Random(); byte[] response = new byte[100]; @@ -117,28 +128,15 @@ public class BlockingHttpClientTests extends AndroidTestCase { assertTrue("ResponseProcessor was not invoked", processor.mInvoked); } - private static class FakeErrorResponseProcessor implements ResponseProcessor { - private final int mExpectedStatusCode; - - boolean mInvoked; - - FakeErrorResponseProcessor(int expectedStatusCode) { - mExpectedStatusCode = expectedStatusCode; - } - + private static class FakeErrorResponseProcessor implements ResponseProcessor { @Override - public void onError(int httpStatusCode, String message) { - mInvoked = true; - assertEquals("onError:", mExpectedStatusCode, httpStatusCode); - } - - @Override - public void onSuccess(InputStream response) { + public Void onSuccess(InputStream response) { fail("Expected an error but received success"); + return null; } } - private static class FakeSuccessResponseProcessor implements ResponseProcessor { + private static class FakeSuccessResponseProcessor implements ResponseProcessor { private final byte[] mExpectedResponse; boolean mInvoked; @@ -148,12 +146,7 @@ public class BlockingHttpClientTests extends AndroidTestCase { } @Override - public void onError(int httpStatusCode, String message) { - fail("Expected a response but received an error"); - } - - @Override - public void onSuccess(InputStream response) { + public Void onSuccess(InputStream response) { try { mInvoked = true; BufferedInputStream in = new BufferedInputStream(response); @@ -169,6 +162,7 @@ public class BlockingHttpClientTests extends AndroidTestCase { } catch (IOException ex) { fail("IOException in onSuccess"); } + return null; } } } diff --git a/tests/src/com/android/inputmethod/latin/network/HttpUrlConnectionBuilderTests.java b/tests/src/com/android/inputmethod/latin/network/HttpUrlConnectionBuilderTests.java index 2b43d5b14..5b3e78eaf 100644 --- a/tests/src/com/android/inputmethod/latin/network/HttpUrlConnectionBuilderTests.java +++ b/tests/src/com/android/inputmethod/latin/network/HttpUrlConnectionBuilderTests.java @@ -142,4 +142,13 @@ public class HttpUrlConnectionBuilderTests extends AndroidTestCase { assertTrue(connection.getDoInput()); assertTrue(connection.getDoOutput()); } + + public void testSetAuthToken() throws IOException { + HttpUrlConnectionBuilder builder = new HttpUrlConnectionBuilder(); + builder.setUrl("https://www.example.com"); + builder.setAuthToken("some-random-auth-token"); + HttpURLConnection connection = builder.build(); + assertEquals("some-random-auth-token", + connection.getRequestProperty(HttpUrlConnectionBuilder.HTTP_HEADER_AUTHORIZATION)); + } }