From f713de9b75bbda412ac6ba14b0934eb30adbfe1c Mon Sep 17 00:00:00 2001 From: Muzahidul Islam Date: Thu, 22 Aug 2024 21:49:27 +0600 Subject: [PATCH 1/2] Rollback to 4.0.0 --- .../ab/android/odp/ODPSegmentClientTest.kt | 82 +++++++++---------- .../ab/android/odp/ODPSegmentClient.kt | 6 +- 2 files changed, 43 insertions(+), 45 deletions(-) diff --git a/odp/src/androidTest/java/com/optimizely/ab/android/odp/ODPSegmentClientTest.kt b/odp/src/androidTest/java/com/optimizely/ab/android/odp/ODPSegmentClientTest.kt index ec18f126..c3027843 100644 --- a/odp/src/androidTest/java/com/optimizely/ab/android/odp/ODPSegmentClientTest.kt +++ b/odp/src/androidTest/java/com/optimizely/ab/android/odp/ODPSegmentClientTest.kt @@ -68,45 +68,45 @@ class ODPSegmentClientTest { verify(urlConnection).disconnect() } -// @Test -// fun fetchQualifiedSegments_400() { -// `when`(urlConnection.responseCode).thenReturn(400) -// -// segmentClient.fetchQualifiedSegments(apiKey, apiEndpoint, payload) -// -// verify(client).execute(captor.capture(), eq(0), eq(0)) -// val received = captor.value.execute() -// -// assertNull(received) -// verify(logger).error("Unexpected response from ODP segment endpoint, status: 400") -// verify(urlConnection).disconnect() -// } - -// @Test -// fun fetchQualifiedSegments_500() { -// `when`(urlConnection.responseCode).thenReturn(500) -// -// segmentClient.fetchQualifiedSegments(apiKey, apiEndpoint, payload) -// -// verify(client).execute(captor.capture(), eq(0), eq(0)) -// val received = captor.value.execute() -// -// assertNull(received) -// verify(logger).error("Unexpected response from ODP segment endpoint, status: 500") -// verify(urlConnection).disconnect() -// } - -// @Test -// fun fetchQualifiedSegments_connectionFailed() { -// `when`(urlConnection.responseCode).thenReturn(200) -// -// apiEndpoint = "invalid-url" -// segmentClient.fetchQualifiedSegments(apiKey, apiEndpoint, payload) -// -// verify(client).execute(captor.capture(), eq(0), eq(0)) -// val received = captor.value.execute() -// -// assertNull(received) -// verify(logger).error(contains("Error making ODP segment request"), any()) -// } + @Test + fun fetchQualifiedSegments_400() { + `when`(urlConnection.responseCode).thenReturn(400) + + segmentClient.fetchQualifiedSegments(apiKey, apiEndpoint, payload) + + verify(client).execute(captor.capture(), eq(0), eq(0)) + val received = captor.value.execute() + + assertNull(received) + verify(logger).error("Unexpected response from ODP segment endpoint, status: 400") + verify(urlConnection).disconnect() + } + + @Test + fun fetchQualifiedSegments_500() { + `when`(urlConnection.responseCode).thenReturn(500) + + segmentClient.fetchQualifiedSegments(apiKey, apiEndpoint, payload) + + verify(client).execute(captor.capture(), eq(0), eq(0)) + val received = captor.value.execute() + + assertNull(received) + verify(logger).error("Unexpected response from ODP segment endpoint, status: 500") + verify(urlConnection).disconnect() + } + + @Test + fun fetchQualifiedSegments_connectionFailed() { + `when`(urlConnection.responseCode).thenReturn(200) + + apiEndpoint = "invalid-url" + segmentClient.fetchQualifiedSegments(apiKey, apiEndpoint, payload) + + verify(client).execute(captor.capture(), eq(0), eq(0)) + val received = captor.value.execute() + + assertNull(received) + verify(logger).error(contains("Error making ODP segment request"), any()) + } } diff --git a/odp/src/main/java/com/optimizely/ab/android/odp/ODPSegmentClient.kt b/odp/src/main/java/com/optimizely/ab/android/odp/ODPSegmentClient.kt index 1d8d7496..c8875047 100644 --- a/odp/src/main/java/com/optimizely/ab/android/odp/ODPSegmentClient.kt +++ b/odp/src/main/java/com/optimizely/ab/android/odp/ODPSegmentClient.kt @@ -62,13 +62,11 @@ open class ODPSegmentClient(private val client: ClientForODPOnly, private val lo } else { var errMsg = "Unexpected response from ODP segment endpoint, status: $status" logger.error(errMsg) -// return@Request null - throw Exception(errMsg) + return@Request null } } catch (e: Exception) { logger.error("Error making ODP segment request", e) - // return@Request null - throw e + return@Request null } finally { if (urlConnection != null) { try { From f985a7946670349b0334363044509319f6d2c443 Mon Sep 17 00:00:00 2001 From: Muzahidul Islam Date: Fri, 23 Aug 2024 16:31:01 +0600 Subject: [PATCH 2/2] ODP for client only removed --- .../ab/android/odp/ODPSegmentClientTest.kt | 6 +- .../ab/android/odp/DefaultODPApiManager.kt | 4 +- .../ab/android/odp/ODPSegmentClient.kt | 6 +- .../ab/android/shared/ClientForODPOnly.java | 189 ------------------ 4 files changed, 8 insertions(+), 197 deletions(-) delete mode 100644 shared/src/main/java/com/optimizely/ab/android/shared/ClientForODPOnly.java diff --git a/odp/src/androidTest/java/com/optimizely/ab/android/odp/ODPSegmentClientTest.kt b/odp/src/androidTest/java/com/optimizely/ab/android/odp/ODPSegmentClientTest.kt index c3027843..c7a2a82e 100644 --- a/odp/src/androidTest/java/com/optimizely/ab/android/odp/ODPSegmentClientTest.kt +++ b/odp/src/androidTest/java/com/optimizely/ab/android/odp/ODPSegmentClientTest.kt @@ -15,7 +15,7 @@ package com.optimizely.ab.android.odp import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.optimizely.ab.android.shared.ClientForODPOnly +import com.optimizely.ab.android.shared.Client import java.io.OutputStream import java.net.HttpURLConnection import org.junit.Assert.assertNull @@ -34,9 +34,9 @@ import org.slf4j.Logger @RunWith(AndroidJUnit4::class) class ODPSegmentClientTest { private val logger = mock(Logger::class.java) - private val client = mock(ClientForODPOnly::class.java) + private val client = mock(Client::class.java) private val urlConnection = mock(HttpURLConnection::class.java) - private val captor = ArgumentCaptor.forClass(ClientForODPOnly.Request::class.java) + private val captor = ArgumentCaptor.forClass(Client.Request::class.java) private lateinit var segmentClient: ODPSegmentClient private val apiKey = "valid-key" diff --git a/odp/src/main/java/com/optimizely/ab/android/odp/DefaultODPApiManager.kt b/odp/src/main/java/com/optimizely/ab/android/odp/DefaultODPApiManager.kt index 4f6c4439..c243f3cb 100644 --- a/odp/src/main/java/com/optimizely/ab/android/odp/DefaultODPApiManager.kt +++ b/odp/src/main/java/com/optimizely/ab/android/odp/DefaultODPApiManager.kt @@ -16,7 +16,7 @@ package com.optimizely.ab.android.odp import android.content.Context import androidx.annotation.VisibleForTesting -import com.optimizely.ab.android.shared.ClientForODPOnly +import com.optimizely.ab.android.shared.Client import com.optimizely.ab.android.shared.OptlyStorage import com.optimizely.ab.android.shared.WorkerScheduler import com.optimizely.ab.odp.ODPApiManager @@ -33,7 +33,7 @@ open class DefaultODPApiManager(private val context: Context, timeoutForSegmentF @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) var segmentClient = ODPSegmentClient( - ClientForODPOnly(OptlyStorage(context), LoggerFactory.getLogger(ClientForODPOnly::class.java)), + Client(OptlyStorage(context), LoggerFactory.getLogger(Client::class.java)), LoggerFactory.getLogger(ODPSegmentClient::class.java) ) private val logger = LoggerFactory.getLogger(DefaultODPApiManager::class.java) diff --git a/odp/src/main/java/com/optimizely/ab/android/odp/ODPSegmentClient.kt b/odp/src/main/java/com/optimizely/ab/android/odp/ODPSegmentClient.kt index c8875047..27287841 100644 --- a/odp/src/main/java/com/optimizely/ab/android/odp/ODPSegmentClient.kt +++ b/odp/src/main/java/com/optimizely/ab/android/odp/ODPSegmentClient.kt @@ -15,7 +15,7 @@ package com.optimizely.ab.android.odp import androidx.annotation.VisibleForTesting -import com.optimizely.ab.android.shared.ClientForODPOnly +import com.optimizely.ab.android.shared.Client import com.optimizely.ab.odp.parser.ResponseJsonParser import com.optimizely.ab.odp.parser.ResponseJsonParserFactory import org.slf4j.Logger @@ -23,7 +23,7 @@ import java.net.HttpURLConnection import java.net.URL @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) -open class ODPSegmentClient(private val client: ClientForODPOnly, private val logger: Logger) { +open class ODPSegmentClient(private val client: Client, private val logger: Logger) { @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) open fun fetchQualifiedSegments( @@ -32,7 +32,7 @@ open class ODPSegmentClient(private val client: ClientForODPOnly, private val lo payload: String ): List? { - val request: ClientForODPOnly.Request = ClientForODPOnly.Request { + val request: Client.Request = Client.Request { var urlConnection: HttpURLConnection? = null try { val url = URL(apiEndpoint) diff --git a/shared/src/main/java/com/optimizely/ab/android/shared/ClientForODPOnly.java b/shared/src/main/java/com/optimizely/ab/android/shared/ClientForODPOnly.java deleted file mode 100644 index 82b30844..00000000 --- a/shared/src/main/java/com/optimizely/ab/android/shared/ClientForODPOnly.java +++ /dev/null @@ -1,189 +0,0 @@ -/**************************************************************************** - * Copyright 2016-2017,2021, Optimizely, Inc. and contributors * - * * - * 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.optimizely.ab.android.shared; - -import android.os.Build; - -import androidx.annotation.NonNull; -import androidx.annotation.Nullable; - -import org.slf4j.Logger; - -import java.io.BufferedInputStream; -import java.io.InputStream; -import java.net.HttpURLConnection; -import java.net.URL; -import java.net.URLConnection; -import java.util.Scanner; -import java.util.concurrent.TimeUnit; - -import javax.net.ssl.HttpsURLConnection; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLSocketFactory; - -/** - * Functionality common to all clients using http connections - */ -public class ClientForODPOnly { - - static final int MAX_BACKOFF_TIMEOUT = (int) Math.pow(2, 5); - - @NonNull private final OptlyStorage optlyStorage; - @NonNull private final Logger logger; - - /** - * Constructs a new Client instance - * - * @param optlyStorage an instance of {@link OptlyStorage} - * @param logger an instance of {@link Logger} - */ - public ClientForODPOnly(@NonNull OptlyStorage optlyStorage, @NonNull Logger logger) { - this.optlyStorage = optlyStorage; - this.logger = logger; - } - - /** - * Opens {@link HttpURLConnection} from a {@link URL} - * - * @param url a {@link URL} instance - * @return an open {@link HttpURLConnection} - */ - @Nullable - public HttpURLConnection openConnection(URL url) { - try { - // API 21 (LOLLIPOP)+ supposed to use TLS1.2 as default, but some API-21 devices still fail, so include it here. - if (Build.VERSION.SDK_INT <= Build.VERSION_CODES.LOLLIPOP) { - SSLContext sslContext = SSLContext.getInstance("TLSv1.2"); - sslContext.init(null, null, null); - SSLSocketFactory sslSocketFactory = new TLSSocketFactory(); - HttpsURLConnection.setDefaultSSLSocketFactory(sslSocketFactory); - } - - return (HttpURLConnection) url.openConnection(); - } catch (Exception e) { - logger.warn("Error making request to {}.", url); - } - return null; - } - - /** - * Adds a if-modified-since header to the open {@link URLConnection} if this value is - * stored in {@link OptlyStorage}. - * @param urlConnection an open {@link URLConnection} - */ - public void setIfModifiedSince(@NonNull URLConnection urlConnection) { - if (urlConnection == null || urlConnection.getURL() == null) { - logger.error("Invalid connection"); - return; - } - - long lastModified = optlyStorage.getLong(urlConnection.getURL().toString(), 0); - if (lastModified > 0) { - urlConnection.setIfModifiedSince(lastModified); - } - } - - /** - * Retrieves the last-modified head from a {@link URLConnection} and saves it - * in {@link OptlyStorage}. - * @param urlConnection a {@link URLConnection} instance - */ - public void saveLastModified(@NonNull URLConnection urlConnection) { - if (urlConnection == null || urlConnection.getURL() == null) { - logger.error("Invalid connection"); - return; - } - - long lastModified = urlConnection.getLastModified(); - if (lastModified > 0) { - optlyStorage.saveLong(urlConnection.getURL().toString(), urlConnection.getLastModified()); - } else { - logger.warn("CDN response didn't have a last modified header"); - } - } - - @Nullable - public String readStream(@NonNull URLConnection urlConnection) { - Scanner scanner = null; - try { - InputStream in = new BufferedInputStream(urlConnection.getInputStream()); - scanner = new Scanner(in).useDelimiter("\\A"); - return scanner.hasNext() ? scanner.next() : ""; - } catch (Exception e) { - logger.warn("Error reading urlConnection stream.", e); - return null; - } - finally { - if (scanner != null) { - // We assume that closing the scanner will close the associated input stream. - try { - scanner.close(); - } - catch (Exception e) { - logger.error("Problem with closing the scanner on a input stream" , e); - } - } - } - } - - /** - * Executes a request with exponential backoff - * @param request the request executable, would be a lambda on Java 8 - * @param timeout the numerical base for the exponential backoff - * @param power the number of retries - * @param the response type of the request - * @return the response - */ - public T execute(Request request, int timeout, int power) { - int baseTimeout = timeout; - int maxTimeout = (int) Math.pow(baseTimeout, power); - T response = null; - while(timeout <= maxTimeout) { - try { - response = request.execute(); - } catch (Exception e) { - logger.error("(ClientForODPOnly) Request failed with error: ", e); - throw e; - } - - if (response == null || response == Boolean.FALSE) { - // retry is disabled when timeout set to 0 - if (timeout == 0) break; - - try { - logger.info("Request failed, waiting {} seconds to try again", timeout); - Thread.sleep(TimeUnit.MILLISECONDS.convert(timeout, TimeUnit.SECONDS)); - } catch (InterruptedException e) { - logger.warn("Exponential backoff failed", e); - break; - } - timeout = timeout * baseTimeout; - } else { - break; - } - } - return response; - } - - /** - * Bundles up a request allowing it's execution to be deferred - * @param The response type of the request - */ - public interface Request { - T execute(); - } -}