diff --git a/CodenameOne/src/com/codename1/payment/Purchase.java b/CodenameOne/src/com/codename1/payment/Purchase.java index c5057ffb97..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,31 +475,83 @@ 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); + } } } - /// Removes a receipt from pending purchases. + /// 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); + } + } + } + + /// 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 +564,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 +632,30 @@ 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); + // 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 + // 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..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]); @@ -243,6 +245,116 @@ 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()); + } + + @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 + // 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 +413,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();