From 91f39a151f12f1807ffdd79ba37cc255736fe552 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 20 May 2026 04:14:31 +0300 Subject: [PATCH 1/2] Fix Purchase.synchronizeReceipts re-submitting same receipt and firing callback N times Three closely-related bugs surfaced from a forum report of submitReceipt being invoked repeatedly: 1. removePendingPurchase matched only on transactionId and silently no-op'd when the receipt's transactionId was null. The pending queue then still contained the receipt, the recursion at the end of synchronizeReceipts pulled it again, and the same receipt was re-submitted forever. removePendingPurchase now takes the Receipt itself and falls back to matching on (sku, storeCode, purchaseDate, orderData) when transactionId is null on either side. 2. The recursive synchronizeReceipts(0, callback) re-registered the caller's SuccessCallback on every iteration, so a queue of N pending receipts caused the user's callback to fire N times. The recursive call now passes null since the original callback is already in synchronizeReceiptsCallbacks. 3. syncInProgress was reset to false after removePendingPurchase, so a throw from removePendingPurchase (or any user-supplied submitReceipt implementation) permanently wedged the sync state. syncInProgress is now reset at the top of onSucess. Added three regression tests in PurchaseTest: - testSynchronizeReceiptsSyncDrainsMultiplePendingReceipts: each pending receipt is submitted exactly once. - testSynchronizeReceiptsCallbackFiresOnceWhenDrainingMultiplePendingReceipts - testSynchronizeReceiptsDoesNotInfinitelyResubmitReceiptWithNullTransactionId (uses a CountingReceiptStore that throws after a cap so a regression fails the test instead of hanging it). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/com/codename1/payment/Purchase.java | 55 ++++++++--- .../com/codename1/payment/PurchaseTest.java | 96 +++++++++++++++++++ 2 files changed, 137 insertions(+), 14 deletions(-) diff --git a/CodenameOne/src/com/codename1/payment/Purchase.java b/CodenameOne/src/com/codename1/payment/Purchase.java index c5057ffb97..e9e6c17f8c 100644 --- a/CodenameOne/src/com/codename1/payment/Purchase.java +++ b/CodenameOne/src/com/codename1/payment/Purchase.java @@ -472,25 +472,28 @@ private void addPendingPurchase(Receipt receipt) { } } - /// Removes a receipt from pending purchases. + /// Removes a receipt from pending purchases. Receipts with a non-null + /// `transactionId` are matched on `transactionId`; receipts whose + /// `transactionId` is null fall back to matching on `sku`, `storeCode`, + /// `purchaseDate` and `orderData` so that the receipt is still removed + /// from the queue and `synchronizeReceipts` doesn't re-submit it forever. /// /// #### Parameters /// - /// - `transactionId`: the transaction id + /// - `target`: the receipt to remove /// /// #### Returns /// - /// the removed receipt - private Receipt removePendingPurchase(String transactionId) { + /// the removed receipt, or null if no matching receipt was found + private Receipt removePendingPurchase(Receipt target) { synchronized (PENDING_PURCHASE_LOCK) { Storage s = Storage.getInstance(); List pendingPurchases = getPendingPurchases(); Receipt found = null; for (Receipt r : pendingPurchases) { - if (r.getTransactionId() != null && r.getTransactionId().equals(transactionId)) { + if (receiptsMatch(r, target)) { found = r; break; - } } if (found != null) { @@ -503,6 +506,25 @@ private Receipt removePendingPurchase(String transactionId) { } } + private static boolean receiptsMatch(Receipt a, Receipt b) { + String aTx = a.getTransactionId(); + String bTx = b.getTransactionId(); + if (aTx != null && bTx != null) { + return aTx.equals(bTx); + } + if (aTx != null || bTx != null) { + return false; + } + return nullSafeEquals(a.getSku(), b.getSku()) + && nullSafeEquals(a.getStoreCode(), b.getStoreCode()) + && nullSafeEquals(a.getPurchaseDate(), b.getPurchaseDate()) + && nullSafeEquals(a.getOrderData(), b.getOrderData()); + } + + private static boolean nullSafeEquals(Object a, Object b) { + return a == null ? b == null : a.equals(b); + } + public final void synchronizeReceipts() { if (syncInProgress) { return; @@ -552,19 +574,24 @@ public final void synchronizeReceipts(final long ifOlderThanMs, final SuccessCal if (!pending.isEmpty() && receiptStore != null) { final Receipt receipt = pending.get(0); - receiptStore.submitReceipt(pending.get(0), new SuccessCallback() { + receiptStore.submitReceipt(receipt, new SuccessCallback() { @Override public void onSucess(Boolean submitSucceeded) { + // Reset syncInProgress before doing any work that can + // throw so that a failure here doesn't permanently + // wedge synchronizeReceipts in the "in progress" + // state for the rest of the app's lifetime. + syncInProgress = false; if (submitSucceeded) { - removePendingPurchase(receipt.getTransactionId()); - syncInProgress = false; - - // If the submit succeeded we need to refetch - // so we set this to zero here. - synchronizeReceipts(0, callback); + removePendingPurchase(receipt); + // Continue draining the queue. The original + // callback is already registered in + // synchronizeReceiptsCallbacks; passing null here + // avoids registering it again and firing it once + // per drained receipt. + synchronizeReceipts(0, null); } else { - syncInProgress = false; fireSynchronizeReceiptsCallbacks(false); } } diff --git a/maven/core-unittests/src/test/java/com/codename1/payment/PurchaseTest.java b/maven/core-unittests/src/test/java/com/codename1/payment/PurchaseTest.java index d297568096..5acef06b7b 100644 --- a/maven/core-unittests/src/test/java/com/codename1/payment/PurchaseTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/payment/PurchaseTest.java @@ -243,6 +243,73 @@ void testSynchronizeReceiptsSyncReturnsFalseWhenSubmitFails() { assertEquals("silver", pending.get(0).getSku()); } + @EdtTest + void testSynchronizeReceiptsSyncDrainsMultiplePendingReceipts() { + List pending = new ArrayList(); + pending.add(createReceipt("gold", new Date(1000L), new Date(5000L))); + pending.add(createReceipt("silver", new Date(2000L), new Date(6000L))); + pending.add(createReceipt("bronze", new Date(3000L), new Date(7000L))); + Storage.getInstance().writeObject(PENDING_STORAGE, pending); + + TestReceiptStore store = new TestReceiptStore(); + purchase.setReceiptStore(store); + + boolean success = purchase.synchronizeReceiptsSync(0); + assertTrue(success); + assertEquals(3, store.getSubmittedReceipts().size(), + "Each pending receipt should be submitted exactly once"); + assertTrue(purchase.getPendingPurchases().isEmpty()); + } + + @EdtTest + void testSynchronizeReceiptsCallbackFiresOnceWhenDrainingMultiplePendingReceipts() { + List pending = new ArrayList(); + pending.add(createReceipt("gold", new Date(1000L), new Date(5000L))); + pending.add(createReceipt("silver", new Date(2000L), new Date(6000L))); + pending.add(createReceipt("bronze", new Date(3000L), new Date(7000L))); + Storage.getInstance().writeObject(PENDING_STORAGE, pending); + + TestReceiptStore store = new TestReceiptStore(); + purchase.setReceiptStore(store); + + final int[] callCount = new int[1]; + final boolean[] result = new boolean[1]; + purchase.synchronizeReceipts(0, new SuccessCallback() { + public void onSucess(Boolean value) { + callCount[0]++; + result[0] = value; + } + }); + flushSerialCalls(); + + assertEquals(1, callCount[0], + "Synchronize callback must fire exactly once, not once per drained receipt"); + assertTrue(result[0]); + assertEquals(3, store.getSubmittedReceipts().size()); + } + + @EdtTest + void testSynchronizeReceiptsDoesNotInfinitelyResubmitReceiptWithNullTransactionId() { + // A receipt with a null transactionId must still be removable from the + // pending queue. Otherwise synchronizeReceipts recurses forever, + // resubmitting the same receipt to the ReceiptStore on every iteration. + Receipt nullTxReceipt = createReceipt("orphan", new Date(1000L), new Date(5000L)); + nullTxReceipt.setTransactionId(null); + List pending = new ArrayList(); + pending.add(nullTxReceipt); + Storage.getInstance().writeObject(PENDING_STORAGE, pending); + + CountingReceiptStore store = new CountingReceiptStore(5); + purchase.setReceiptStore(store); + + boolean success = purchase.synchronizeReceiptsSync(0); + assertTrue(success); + assertEquals(1, store.getSubmittedReceipts().size(), + "Receipt with null transactionId should be submitted exactly once, not in a loop"); + assertTrue(purchase.getPendingPurchases().isEmpty(), + "Receipt with null transactionId should be removed from pending queue after successful submit"); + } + @Test void testSynchronizeReceiptsSyncWaitsForAsyncFetch() { final Receipt asyncReceipt = createReceipt("async", new Date(1000L), new Date(5000L)); @@ -301,6 +368,35 @@ public void submitReceipt(Receipt receipt, SuccessCallback callback) { } } + /// Receipt store that refuses further submissions after a hard cap is hit + /// so a regression of the null-transactionId loop bug fails the test + /// instead of hanging it. + private static class CountingReceiptStore implements ReceiptStore { + private final List submitted = new ArrayList(); + private final int maxSubmits; + + CountingReceiptStore(int maxSubmits) { + this.maxSubmits = maxSubmits; + } + + List getSubmittedReceipts() { + return new ArrayList(submitted); + } + + public void fetchReceipts(SuccessCallback callback) { + callback.onSucess(new Receipt[0]); + } + + public void submitReceipt(Receipt receipt, SuccessCallback callback) { + submitted.add(receipt); + if (submitted.size() > maxSubmits) { + throw new AssertionError("submitReceipt invoked more than " + maxSubmits + + " times; pending receipt is being resubmitted in a loop"); + } + callback.onSucess(Boolean.TRUE); + } + } + private static class TestPurchase extends Purchase { private final List recordedPurchases = new ArrayList(); private final List recordedPromotionalPurchases = new ArrayList(); From 6983a955e6694a44b154e19811828ca83ae0bc31 Mon Sep 17 00:00:00 2001 From: Shai Almog <67850168+shai-almog@users.noreply.github.com> Date: Wed, 20 May 2026 06:50:38 +0300 Subject: [PATCH 2/2] Dedupe submitReceipt on transactionId so it's invoked at most once per id MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Until now the framework's contract was effectively "submitReceipt fires once per pending-queue entry," which leaks platform-level behavior to the user: iOS StoreKit can redeliver an unfinished transaction across app sessions, and sandbox subscription renewals fire repeatedly — each delivery hits postReceipt and produces another submitReceipt call with the same transactionId. The user-facing contract was inconsistent across platforms. Add a persistent List of processed transactionIds in CN1 Storage (key ProcessedPurchases.dat). Two checks close the gap: 1. addPendingPurchase drops a receipt whose transactionId is already in the processed set, or already sitting in the pending queue. So duplicate postReceipt calls — from iOS cross-session redelivery, in-session double-fire, or anything else — never reach the queue. 2. The success branch of synchronizeReceipts records the transactionId in the processed set before removing the receipt from pending. Doing it in that order means a parallel re-enqueue racing the remove is also dropped. Receipts with a null transactionId can't be tracked in the set; they fall back to the existing receiptsMatch-based in-pending dedup. Two new tests: - testPostReceiptSkipsReceiptThatWasAlreadySuccessfullySubmitted (cross-session redelivery — verified the test fails 1 vs 2 without the fix) - testPostReceiptSkipsDuplicateTransactionIdAlreadyPending (same-session double-fire before sync runs) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/com/codename1/payment/Purchase.java | 72 +++++++++++++++++-- .../com/codename1/payment/PurchaseTest.java | 45 ++++++++++++ 2 files changed, 113 insertions(+), 4 deletions(-) diff --git a/CodenameOne/src/com/codename1/payment/Purchase.java b/CodenameOne/src/com/codename1/payment/Purchase.java index e9e6c17f8c..c99910ab6a 100644 --- a/CodenameOne/src/com/codename1/payment/Purchase.java +++ b/CodenameOne/src/com/codename1/payment/Purchase.java @@ -52,6 +52,7 @@ public abstract class Purchase { private static final String RECEIPTS_KEY = "CN1SubscriptionsData.dat"; private static final String RECEIPTS_REFRESH_TIME_KEY = "CN1SubscriptionsDataRefreshTime.dat"; private static final String PENDING_PURCHASE_KEY = "PendingPurchases.dat"; + private static final String PROCESSED_PURCHASE_KEY = "ProcessedPurchases.dat"; private static final Object PENDING_PURCHASE_LOCK = new Object(); private static final Object synchronizationLock = new Object(); private static final Object receiptsLock = new Object(); @@ -458,7 +459,15 @@ public List getPendingPurchases() { } } - /// Adds a receipt to be pushed to the server. + /// Adds a receipt to be pushed to the server. Receipts whose + /// `transactionId` has already been successfully submitted to the + /// `ReceiptStore` (recorded in the persistent processed set) or is + /// already sitting in the pending queue are silently dropped. This + /// closes the abstraction so the user's `ReceiptStore.submitReceipt` + /// is invoked at most once per `transactionId` across the lifetime of + /// the install, regardless of platform-level redelivery quirks + /// (e.g. iOS StoreKit redelivering an unfinished transaction across + /// app sessions, or sandbox subscription renewals). /// /// #### Parameters /// @@ -466,9 +475,58 @@ public List getPendingPurchases() { private void addPendingPurchase(Receipt receipt) { synchronized (PENDING_PURCHASE_LOCK) { Storage s = Storage.getInstance(); - List pendingPurchases = getPendingPurchases(); - pendingPurchases.add(receipt); - s.writeObject(PENDING_PURCHASE_KEY, pendingPurchases); + String txId = receipt.getTransactionId(); + if (txId != null) { + if (getProcessedTransactionIds().contains(txId)) { + return; + } + List pendingPurchases = getPendingPurchases(); + for (Receipt r : pendingPurchases) { + if (txId.equals(r.getTransactionId())) { + return; + } + } + pendingPurchases.add(receipt); + s.writeObject(PENDING_PURCHASE_KEY, pendingPurchases); + } else { + // Receipts without a transactionId can't be tracked in the + // processed set; fall back to enqueueing and let the + // synchronize path drain them. receiptsMatch handles + // removal correctly when transactionId is null on both + // sides. + List pendingPurchases = getPendingPurchases(); + pendingPurchases.add(receipt); + s.writeObject(PENDING_PURCHASE_KEY, pendingPurchases); + } + } + } + + /// Returns the persistent list of transactionIds that have already + /// been successfully submitted to the `ReceiptStore`. The caller + /// must hold `PENDING_PURCHASE_LOCK`. + @SuppressWarnings("unchecked") + private List getProcessedTransactionIds() { + Storage s = Storage.getInstance(); + if (s.exists(PROCESSED_PURCHASE_KEY)) { + return (List) s.readObject(PROCESSED_PURCHASE_KEY); + } + return new ArrayList(); + } + + /// Records that the receipt with the given transactionId has been + /// successfully submitted to the `ReceiptStore`, so future + /// `addPendingPurchase` calls with the same transactionId skip the + /// enqueue. + private void recordProcessedTransactionId(String txId) { + if (txId == null) { + return; + } + synchronized (PENDING_PURCHASE_LOCK) { + List processed = getProcessedTransactionIds(); + if (!processed.contains(txId)) { + processed.add(txId); + Storage.getInstance().writeObject(PROCESSED_PURCHASE_KEY, processed); + } } } @@ -584,6 +642,12 @@ public void onSucess(Boolean submitSucceeded) { // state for the rest of the app's lifetime. syncInProgress = false; if (submitSucceeded) { + // Record the transactionId before removing the + // receipt from pending so a parallel + // postReceipt re-enqueue (e.g. iOS redelivery) + // is dropped by addPendingPurchase rather than + // sneaking back into the queue. + recordProcessedTransactionId(receipt.getTransactionId()); removePendingPurchase(receipt); // Continue draining the queue. The original // callback is already registered in diff --git a/maven/core-unittests/src/test/java/com/codename1/payment/PurchaseTest.java b/maven/core-unittests/src/test/java/com/codename1/payment/PurchaseTest.java index 5acef06b7b..19a1b1d387 100644 --- a/maven/core-unittests/src/test/java/com/codename1/payment/PurchaseTest.java +++ b/maven/core-unittests/src/test/java/com/codename1/payment/PurchaseTest.java @@ -24,6 +24,7 @@ class PurchaseTest extends UITestBase { private static final String RECEIPTS_STORAGE = "CN1SubscriptionsData.dat"; private static final String RECEIPTS_REFRESH_STORAGE = "CN1SubscriptionsDataRefreshTime.dat"; private static final String PENDING_STORAGE = "PendingPurchases.dat"; + private static final String PROCESSED_STORAGE = "ProcessedPurchases.dat"; private TestPurchase purchase; @@ -56,6 +57,7 @@ private void resetPurchaseState() { storage.deleteStorageFile(RECEIPTS_STORAGE); storage.deleteStorageFile(RECEIPTS_REFRESH_STORAGE); storage.deleteStorageFile(PENDING_STORAGE); + storage.deleteStorageFile(PROCESSED_STORAGE); ReceiptStore clearingStore = new ReceiptStore() { public void fetchReceipts(SuccessCallback callback) { callback.onSucess(new Receipt[0]); @@ -288,6 +290,49 @@ public void onSucess(Boolean value) { assertEquals(3, store.getSubmittedReceipts().size()); } + @FormTest + void testPostReceiptSkipsReceiptThatWasAlreadySuccessfullySubmitted() { + // Simulate iOS StoreKit redelivering a transaction across app + // sessions: the same transactionId arrives via postReceipt after + // it was already successfully submitted in a prior cycle. + TestReceiptStore store = new TestReceiptStore(); + purchase.setReceiptStore(store); + + long purchaseTime = System.currentTimeMillis(); + Purchase.postReceipt(Receipt.STORE_CODE_ITUNES, "pro", "tx-redelivery", purchaseTime, "order-1"); + flushSerialCalls(); + + assertEquals(1, store.getSubmittedReceipts().size()); + assertTrue(purchase.getPendingPurchases().isEmpty()); + + // Same transactionId arrives again (e.g. iOS redelivery on the + // next app launch). Framework should drop it before it reaches + // the pending queue, so submitReceipt is not invoked a second + // time. + Purchase.postReceipt(Receipt.STORE_CODE_ITUNES, "pro", "tx-redelivery", purchaseTime, "order-1"); + flushSerialCalls(); + + assertEquals(1, store.getSubmittedReceipts().size(), + "Receipt with already-processed transactionId must not be re-submitted"); + assertTrue(purchase.getPendingPurchases().isEmpty()); + } + + @FormTest + void testPostReceiptSkipsDuplicateTransactionIdAlreadyPending() { + // Same transactionId queued twice before any sync happens (e.g. + // native layer fires postReceipt twice for one transaction). + // The second add must be silently dropped so the receipt is only + // submitted once when sync runs. + long purchaseTime = System.currentTimeMillis(); + Purchase.postReceipt(Receipt.STORE_CODE_ITUNES, "pro", "tx-dupe", purchaseTime, "order-1"); + Purchase.postReceipt(Receipt.STORE_CODE_ITUNES, "pro", "tx-dupe", purchaseTime, "order-1"); + flushSerialCalls(); + + List pending = purchase.getPendingPurchases(); + assertEquals(1, pending.size(), + "Duplicate transactionId enqueued before sync should be dropped at addPendingPurchase"); + } + @EdtTest void testSynchronizeReceiptsDoesNotInfinitelyResubmitReceiptWithNullTransactionId() { // A receipt with a null transactionId must still be removable from the