Skip to content

Serve push notification test image from MockWebServer #1097

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.

Sign up for GitHub

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

Open
franco-zalamena-iterable wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Serve push notification test image from MockWebServer #1097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
49f637c
Serve push notification test image from MockWebServer
franco-zalamena-iterable Sep 23, 2026
080825a
Stop IterableApiResponseTest retries leaking across tests
franco-zalamena-iterable Sep 23, 2026
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
58 changes: 46 additions & 12 deletions iterableapi/src/androidTest/java/com/iterable/iterableapi/IterableApiResponseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,12 @@
import org.junit.runner.RunWith;

import java.io.IOException;
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import okhttp3.mockwebserver.Dispatcher;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
Expand All @@ -36,21 +39,45 @@ public class IterableApiResponseTest {

private MockWebServer server;

private final Object scriptLock = new Object();
private final Deque<MockResponse> scriptedResponses = new ArrayDeque<>();
private MockResponse lastServedResponse;

@Before
public void setUp() throws IOException {
server = new MockWebServer();
// Explicitly start the server to ensure it's ready
try {
server.start();
} catch (IllegalStateException e) {
// Server may already be started by url() call below, which is fine
}
// IterableRequestTask resolves the endpoint from a static override on every attempt,
// and schedules retries on a static main-looper Handler up to
// RETRY_DELAY_MS * MAX_RETRY_COUNT in the future. A retry left pending when a test
// finishes therefore re-resolves to the *next* test's server. With the default queue
// dispatcher that stray request consumed the response the next test was waiting for,
// leaving it blocked until its latch timed out. Serving from a dispatcher that repeats
// the last scripted response makes a stray request harmless instead of destructive.
server.setDispatcher(new Dispatcher() {
@Override
public MockResponse dispatch(RecordedRequest request) {
synchronized (scriptLock) {
MockResponse next = scriptedResponses.poll();
if (next != null) {
lastServedResponse = next;
} else if (lastServedResponse == null) {
lastServedResponse = new MockResponse().setResponseCode(200).setBody("{}");
}
return lastServedResponse;
}
Comment on lines +56 to +67

@jferrao-itrbl jferrao-itrbl Oct 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

also ~L309–323: the 500 chain can still outlive the test, and a stray request still consumes the next script

What the code does: testResponseCode500 stubs 500, 500, then 200, then returns after the second takeRequest. The dispatcher poll()s a scripted response first and, only when the deque is empty, returns the same lastServedResponse instance. tearDown does not drain the main looper.

What the spec says: Commit 080825af says to end the chain with a success so it cannot outlive the test, and to repeat the last response so a stray request is harmless.

Why it conflicts: Attempt 3 is the 200, scheduled with delay 0 (retryCount > 2 is false for the second 500) and never awaited, so it can run after overrideURLEndpointPath points at the next server. If that test has already called stubResponse, poll() takes its scripted response. Repeating only happens when the deque is empty. OkHttp MockWebServer 4.9 writes that body with throttledTransfer, which reads the Buffer until it is empty, so a second use of the same MockResponse has an empty body.

Suggested action: On the rebase, wait until the terminal success has been received before the test returns, and have the dispatcher return a new copy of the fallback response on every call so it neither steals a scripted response nor serves a drained body.

}
});
server.start();
IterableApi.overrideURLEndpointPath(server.url("").toString());
createIterableApi();
}

@After
public void tearDown() throws IOException {
synchronized (scriptLock) {
scriptedResponses.clear();
lastServedResponse = null;
}
server.shutdown();
server = null;
}
Expand All @@ -67,7 +94,13 @@ private void stubAnyRequestReturningStatusCode(int statusCode, String body) {
if (body != null) {
response.setBody(body);
}
server.enqueue(response);
stubResponse(response);
}

private void stubResponse(MockResponse response) {
synchronized (scriptLock) {
scriptedResponses.add(response);
}
}

@Test
Expand Down Expand Up @@ -274,9 +307,11 @@ public void testMaxRetriesOnMultipleInvalidJwtPayloads() throws Exception {

@Test
public void testResponseCode500() throws Exception {
for (int i = 0; i < 5; i++) {
stubAnyRequestReturningStatusCode(500, "{}");
}
// Ends the retry chain with a success so it cannot outlive the test and fire against
// the next test's server.
stubAnyRequestReturningStatusCode(500, "{}");
stubAnyRequestReturningStatusCode(500, "{}");
stubAnyRequestReturningStatusCode(200, "{}");

IterableApiRequest request = new IterableApiRequest("fake_key", "", new JSONObject(), IterableApiRequest.POST, null, null, null);
IterableRequestTask task = new IterableRequestTask();
Expand Down Expand Up @@ -310,8 +345,7 @@ public void onFailure(@NonNull String reason, @Nullable JSONObject data) {
public void testConnectionError() throws Exception {
final CountDownLatch signal = new CountDownLatch(1);

MockResponse response = new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_DURING_REQUEST_BODY);
server.enqueue(response);
stubResponse(new MockResponse().setSocketPolicy(SocketPolicy.DISCONNECT_DURING_REQUEST_BODY));

IterableApiRequest request = new IterableApiRequest("fake_key", "", new JSONObject(), IterableApiRequest.POST, null, null, new IterableHelper.FailureHandler() {
@Override
Expand Down
36 changes: 34 additions & 2 deletions iterableapi/src/test/java/com/iterable/iterableapi/IterableNotificationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,18 @@
import org.json.JSONArray;
import org.json.JSONObject;

import okhttp3.mockwebserver.Dispatcher;
import okhttp3.mockwebserver.MockResponse;
import okhttp3.mockwebserver.MockWebServer;
import okhttp3.mockwebserver.RecordedRequest;
import okio.Buffer;

@RunWith(RobolectricTestRunner.class)
@Config(sdk = {Build.VERSION_CODES.P})
public class IterableNotificationTest {
Context appContext;
NotificationManager mNotificationManager;
private MockWebServer imageServer;

//EX: itbl_notif = "{\"itbl\":{\"templateId\":28,\"campaignId\":17,\"messageId\":\"85fbb5663ada4f40b5ae0d437e040fa6\",\"isGhostPush\":false},\"body\":\"Pushy\",\"badge\":2,\"sound\":\"me.wav\"}}";

Expand All @@ -51,7 +58,7 @@ public class IterableNotificationTest {
String itbl_ghost = "{\"templateId\":1,\"campaignId\":1,\"messageId\":\"11111111111111111111111111111111\",\"isGhostPush\":true}";
String itbl1 = "{\"templateId\":1,\"campaignId\":1,\"messageId\":\"11111111111111111111111111111111\",\"isGhostPush\":false}";
String itbl2 = "{\"templateId\":2,\"campaignId\":2,\"messageId\":\"22222222222222222222222222222222\",\"isGhostPush\":false}}";
String itbl_image = "{\"templateId\":1,\"campaignId\":1,\"messageId\":\"11111111111111111111111111111111\",\"isGhostPush\":false,\"attachment-url\":\"https://assets.iterable.com/assets/images/logos/itbl-logo-full-gray-800x300.png\"}";
String itbl_image;

private Context getContext() {
return getApplicationContext();
Expand Down Expand Up @@ -80,17 +87,42 @@ private String getResourceString(String fileName) throws IOException {
return stringBuilder.toString();
}

private byte[] getResourceBytes(String fileName) throws IOException {
InputStream inputStream = getClass().getClassLoader().getResourceAsStream(fileName);
Buffer buffer = new Buffer();
buffer.readFrom(inputStream);
inputStream.close();
return buffer.readByteArray();
}

@Before
public void setUp() throws Exception {
appContext = getContext().getApplicationContext();
mNotificationManager = (NotificationManager)
getContext().getSystemService(Context.NOTIFICATION_SERVICE);
mNotificationManager.cancelAll();

final byte[] imageBytes = getResourceBytes("350x150.png");
imageServer = new MockWebServer();
// Every notification build() re-downloads the attachment, so serve the image on
// every request instead of enqueuing a fixed number of responses.
imageServer.setDispatcher(new Dispatcher() {
@Override
public MockResponse dispatch(RecordedRequest request) {
return new MockResponse()
.setHeader("Content-Type", "image/png")
.setBody(new Buffer().write(imageBytes));
}
});
imageServer.start();

itbl_image = "{\"templateId\":1,\"campaignId\":1,\"messageId\":\"11111111111111111111111111111111\",\"isGhostPush\":false,\"attachment-url\":\"" + imageServer.url("/push-image.png") + "\"}";
Comment on lines +105 to +119

@jferrao-itrbl jferrao-itrbl Oct 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also iterableapi/src/androidTest/java/com/iterable/iterableapi/IterableApiResponseTest.java ~L120: this branch does not apply onto current master

What the code does: The branch serves the push image from a MockWebServer started in every IterableNotificationTest setUp, and IterableApiResponseTest still calls new IterableRequestTask().execute(request) (for example ~L120 and ~L318). git merge-tree against master (79444469) reports both files changed on both sides.

What the spec says: Commit 49f637c6 says to stop depending on the CDN and serve 350x150.png locally. Commit 080825af says a 500 retry left on the static main-looper Handler must not hit the next test’s server.

Why it conflicts: master already points testNotificationImage at that PNG via ClassLoader.getResource in #1093 (IterableNotificationTest.java ~L116–122). #1096 removed IterableRequestTask.execute; the class is a Runnable, and the instrumentation test calls dispatchRequest → IterableRequestDispatcher.online().execute. master’s testResponseCode500 (~L275–285) still enqueues five 500s and returns after the second takeRequest, while retryRequestWithDelay (~L419–421) still schedules later attempts. The leak this PR describes is still on master, and this patch does not compile against that API.

Suggested action: Rebase onto master. Keep #1093’s classpath image URL. Reapply only the retry fix on top of dispatchRequest.

}

@After
public void tearDown() {
public void tearDown() throws Exception {
mNotificationManager.cancelAll();
imageServer.shutdown();
}

@Test
Expand Down
Loading