diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index cf84f84a..90669fe5 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -81,7 +81,18 @@ jobs: strategy: fail-fast: false matrix: - api-level: [21, 25, 26, 29] + # 21 is too old (not properly supported by github) + # mockito version is not working in 30+ + include: + - api-level: 25 + arch: x86 + target: google_apis + - api-level: 27 + arch: x86 + target: google_apis + - api-level: 29 + arch: x86_64 + target: default steps: - name: checkout uses: actions/checkout@v4 @@ -113,14 +124,15 @@ jobs: ~/.android/avd/* ~/.android/adb* ~/.android/debug.keystore - key: avd-${{ matrix.api-level }} + key: avd-${{ matrix.api-level }}-${{ matrix.arch }}-${{ matrix.target }} - name: create AVD and generate snapshot for caching if: steps.avd-cache.outputs.cache-hit != 'true' uses: reactivecircus/android-emulator-runner@v2 with: api-level: ${{ matrix.api-level }} - # arch: arm64-v8a # Specify ARM architecture + arch: ${{ matrix.arch }} + target: ${{ matrix.target }} force-avd-creation: false emulator-options: -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none disable-animations: false @@ -130,6 +142,8 @@ jobs: uses: reactivecircus/android-emulator-runner@v2 with: api-level: ${{ matrix.api-level }} + arch: ${{ matrix.arch }} + target: ${{ matrix.target }} force-avd-creation: false emulator-options: -no-snapshot-save -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none disable-animations: true diff --git a/.gitignore b/.gitignore index a3c8f936..b251bf76 100644 --- a/.gitignore +++ b/.gitignore @@ -10,7 +10,9 @@ /build /captures libs/ +fsc-test-libs/ values.gradle jacoco.exec .vscode/ +.settings/ diff --git a/android-sdk/build.gradle b/android-sdk/build.gradle index c049276f..2cbb1dfd 100644 --- a/android-sdk/build.gradle +++ b/android-sdk/build.gradle @@ -73,8 +73,8 @@ dependencies { api project(':user-profile') api project(':odp') // Enable using a local core-api jar for testing when the 'useLocalJars' property is specified - if (project.hasProperty('useLocalJars') && file('../libs/core-api.jar').exists()) { - api files('../libs/core-api.jar') + if (project.hasProperty('useLocalJars') && file('../fsc-test-libs/core-api.jar').exists()) { + api files('../fsc-test-libs/core-api.jar') } else { api ("com.optimizely.ab:core-api:$java_core_ver") { exclude group: 'com.google.code.findbugs' diff --git a/android-sdk/src/main/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClient.kt b/android-sdk/src/main/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClient.kt index d76ef733..c094dc6e 100644 --- a/android-sdk/src/main/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClient.kt +++ b/android-sdk/src/main/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClient.kt @@ -107,6 +107,12 @@ open class DefaultCmabClient : CmabClient { logger.error(errorMessage) throw CmabFetchException(errorMessage) } + } catch (e: CmabInvalidResponseException) { + // Propagate validation exceptions as-is + throw e + } catch (e: CmabFetchException) { + // Propagate fetch exceptions as-is + throw e } catch (e: Exception) { logger.debug("Failed to fetch CMAB decision for ruleId={} and userId={}", ruleId, userId); val errorMessage: String = @@ -123,7 +129,17 @@ open class DefaultCmabClient : CmabClient { } } } - return client.execute(request, REQUEST_BACKOFF_TIMEOUT, REQUEST_RETRIES_POWER) + val result = client.execute(request, REQUEST_BACKOFF_TIMEOUT, REQUEST_RETRIES_POWER) + + // Required to throw exception on any error so CmabService can return cmab error decision. + // Null result means CMAB fetch failed - throw exception. + if (result == null) { + val errorMessage = String.format(cmabClientHelper.cmabFetchFailed, "Client returned null - request failed") + logger.error(errorMessage) + throw CmabFetchException(errorMessage) + } + + return result } companion object { diff --git a/android-sdk/src/test/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClientTest.java b/android-sdk/src/test/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClientTest.java index 8ca06d6f..c29f3773 100644 --- a/android-sdk/src/test/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClientTest.java +++ b/android-sdk/src/test/java/com/optimizely/ab/android/sdk/cmab/DefaultCmabClientTest.java @@ -15,6 +15,8 @@ package com.optimizely.ab.android.sdk.cmab; import com.optimizely.ab.android.shared.Client; +import com.optimizely.ab.cmab.client.CmabFetchException; +import com.optimizely.ab.cmab.client.CmabInvalidResponseException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -88,15 +90,20 @@ public void testConstructorWithContextAndHelper() { } @Test - public void testFetchDecisionSuccess() throws Exception { + public void testFetchDecisionSuccess() { HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class); ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class); - String mockResponseJson = "{\"variation_id\":\"variation_1\",\"status\":\"success\"}"; - when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection); - when(mockUrlConnection.getResponseCode()).thenReturn(200); - when(mockClient.readStream(mockUrlConnection)).thenReturn(mockResponseJson); - when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream); + try { + String mockResponseJson = "{\"variation_id\":\"variation_1\",\"status\":\"success\"}"; + when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection); + when(mockUrlConnection.getResponseCode()).thenReturn(200); + when(mockClient.readStream(mockUrlConnection)).thenReturn(mockResponseJson); + when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream); + } catch (IOException e) { + // Never happens with mocked connection + } + when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> { Client.Request request = invocation.getArgument(0); return request.execute(); @@ -120,13 +127,18 @@ public void testFetchDecisionSuccess() throws Exception { verify(mockUrlConnection).setConnectTimeout(10*1000); verify(mockUrlConnection).setReadTimeout(60*1000); - verify(mockUrlConnection).setRequestMethod("POST"); + try { + verify(mockUrlConnection).setRequestMethod("POST"); + } catch (Exception e) { + // Never happens - verify() doesn't actually invoke the method + } verify(mockUrlConnection).setRequestProperty("content-type", "application/json"); verify(mockUrlConnection).setDoOutput(true); } - @Test - public void testFetchDecisionConnectionFailure() throws Exception { + @Test(expected = CmabFetchException.class) + public void testFetchDecisionConnectionFailure() { + // When openConnection returns null, should throw CmabFetchException when(mockClient.openConnection(any(URL.class))).thenReturn(null); when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> { Client.Request request = invocation.getArgument(0); @@ -135,20 +147,164 @@ public void testFetchDecisionConnectionFailure() throws Exception { mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper); - String result = mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid); - assertNull(result); + // Should throw CmabFetchException when connection fails to open + mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid); + } + + @Test(expected = CmabFetchException.class) + public void testFetchDecisionThrowsExceptionOn500Error() { + HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class); + ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class); + + try { + when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection); + when(mockUrlConnection.getResponseCode()).thenReturn(500); + when(mockUrlConnection.getResponseMessage()).thenReturn("Internal Server Error"); + when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream); + } catch (IOException e) { + // Never happens with mocked connection + } + + when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> { + Client.Request request = invocation.getArgument(0); + return request.execute(); + }); + + doReturn("{\"user_id\":\"test-user-456\"}") + .when(mockCmabClientHelper) + .buildRequestJson(any(), any(), any(), any()); + + mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper); + + // Should throw CmabFetchException for 500 error + mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid); + } + + @Test(expected = CmabFetchException.class) + public void testFetchDecisionThrowsExceptionOn400Error() { + HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class); + ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class); + + try { + when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection); + when(mockUrlConnection.getResponseCode()).thenReturn(400); + when(mockUrlConnection.getResponseMessage()).thenReturn("Bad Request"); + when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream); + } catch (IOException e) { + // Never happens with mocked connection + } + + when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> { + Client.Request request = invocation.getArgument(0); + return request.execute(); + }); + + doReturn("{\"user_id\":\"test-user-456\"}") + .when(mockCmabClientHelper) + .buildRequestJson(any(), any(), any(), any()); + + mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper); + + // Should throw CmabFetchException for 400 error + mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid); + } + + @Test(expected = CmabFetchException.class) + public void testFetchDecisionThrowsExceptionOnNetworkError() { + HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class); + ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class); + + try { + when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection); + when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream); + doThrow(new IOException("Network error")).when(mockUrlConnection).getResponseCode(); + } catch (IOException e) { + // Never happens with mocked connection + } + + when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> { + Client.Request request = invocation.getArgument(0); + return request.execute(); + }); + + doReturn("{\"user_id\":\"test-user-456\"}") + .when(mockCmabClientHelper) + .buildRequestJson(any(), any(), any(), any()); + + mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper); + + // Should throw CmabFetchException when network IOException occurs + mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid); + } + + @Test(expected = CmabInvalidResponseException.class) + public void testFetchDecisionThrowsExceptionOnInvalidJson() { + HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class); + ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class); + + try { + String invalidResponseJson = "{\"invalid\":\"response\"}"; + when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection); + when(mockUrlConnection.getResponseCode()).thenReturn(200); + when(mockClient.readStream(mockUrlConnection)).thenReturn(invalidResponseJson); + when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream); + } catch (IOException e) { + // Never happens with mocked connection + } + + when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> { + Client.Request request = invocation.getArgument(0); + return request.execute(); + }); + + doReturn("{\"user_id\":\"test-user-456\"}") + .when(mockCmabClientHelper) + .buildRequestJson(any(), any(), any(), any()); + doReturn(false) // Invalid response + .when(mockCmabClientHelper) + .validateResponse(any()); + + mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper); + + // Should throw CmabInvalidResponseException when response validation fails + mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid); } @Test - public void testRetryOnFailureWithRetryBackoff() throws Exception { - when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenReturn(null); + public void testRetryConfigurationPassedToClient() { + HttpURLConnection mockUrlConnection = mock(HttpURLConnection.class); + ByteArrayOutputStream mockOutputStream = mock(ByteArrayOutputStream.class); + + try { + String mockResponseJson = "{\"variation_id\":\"variation_1\",\"status\":\"success\"}"; + when(mockClient.openConnection(any(URL.class))).thenReturn(mockUrlConnection); + when(mockUrlConnection.getResponseCode()).thenReturn(200); + when(mockClient.readStream(mockUrlConnection)).thenReturn(mockResponseJson); + when(mockUrlConnection.getOutputStream()).thenReturn(mockOutputStream); + } catch (IOException e) { + // Never happens with mocked connection + } + + when(mockClient.execute(any(Client.Request.class), anyInt(), anyInt())).thenAnswer(invocation -> { + Client.Request request = invocation.getArgument(0); + return request.execute(); + }); + + doReturn("{\"user_id\":\"test-user-456\"}") + .when(mockCmabClientHelper) + .buildRequestJson(any(), any(), any(), any()); + doReturn(true) + .when(mockCmabClientHelper) + .validateResponse(any()); + doReturn("variation_1") + .when(mockCmabClientHelper) + .parseVariationId(any()); mockCmabClient = new DefaultCmabClient(mockClient, mockCmabClientHelper); - String result = mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid); - assertNull(result); + mockCmabClient.fetchDecision(testRuleId, testUserId, testAttributes, testCmabUuid); - // Verify the retry configuration matches our constants + // Verify the retry configuration is passed to client.execute() verify(mockClient).execute(any(Client.Request.class), eq(DefaultCmabClient.REQUEST_BACKOFF_TIMEOUT), eq(DefaultCmabClient.REQUEST_RETRIES_POWER)); assertEquals("REQUEST_BACKOFF_TIMEOUT should be 1", 1, DefaultCmabClient.REQUEST_BACKOFF_TIMEOUT); assertEquals("REQUEST_RETRIES_POWER should be 1", 1, DefaultCmabClient.REQUEST_RETRIES_POWER); diff --git a/shared/build.gradle b/shared/build.gradle index 50103bea..fb48d149 100644 --- a/shared/build.gradle +++ b/shared/build.gradle @@ -46,8 +46,8 @@ android { dependencies { // Enable using a local core-api jar for testing when the 'useLocalJars' property is specified - if (project.hasProperty('useLocalJars') && file('../libs/core-api.jar').exists()) { - api files('../libs/core-api.jar') + if (project.hasProperty('useLocalJars') && file('../fsc-test-libs/core-api.jar').exists()) { + api files('../fsc-test-libs/core-api.jar') } else { api ("com.optimizely.ab:core-api:$java_core_ver") { exclude group: 'com.google.code.findbugs'