diff --git a/packages/react-native/android/src/main/java/com/bugsee/reactnative/BugseeModule.java b/packages/react-native/android/src/main/java/com/bugsee/reactnative/BugseeModule.java index 8508bcd0..2f5c6f39 100644 --- a/packages/react-native/android/src/main/java/com/bugsee/reactnative/BugseeModule.java +++ b/packages/react-native/android/src/main/java/com/bugsee/reactnative/BugseeModule.java @@ -27,13 +27,14 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.Executors; /** * The Android half of the `Bugsee` TurboModule. * *

Thin on purpose. Everything that translates between the JS wire shape and * the SDK's types lives in {@link BugseeTokens}, {@link BugseeStatusMapper}, - * {@link ReportOps} and {@link ReportHandlerBridge}, which are plain Java and + * {@link ReportOps}, {@link ReportHandlerBridge} and {@link CreatedReports}, which are plain Java and * unit-tested without React Native or a device; what is left here is the part * that can only be exercised by running the app, and is covered by the example * app's e2e instead. @@ -67,6 +68,7 @@ public void logUnhandledException( private static final String E_REPORT_HANDLE_DEAD = "E_REPORT_HANDLE_DEAD"; private static final String E_REPORT_ATTACHMENT_REJECTED = "E_REPORT_ATTACHMENT_REJECTED"; private static final String E_REPORT_BAD_ARGUMENT = "E_REPORT_BAD_ARGUMENT"; + private static final String E_REPORT_CREATE_BUSY = "E_REPORT_CREATE_BUSY"; // The stable code of src/attributes/errors.ts's AttributeErrorCode.Rejected. private static final String E_ATTRIBUTE_REJECTED = "E_ATTRIBUTE_REJECTED"; @@ -118,6 +120,9 @@ public void invalidate() { // next runtime cannot know them, so the reports must not wait out // their deadlines. ReportHandlerBridge.shared().detach(this); + // The next runtime cannot know a created-report handle. Drop it and + // free the one slot before that runtime reserves its own. + CreatedReports.shared().clear(); // Same for any outstanding vh request, and disables the view tree: // the next runtime's anchor has not mounted yet. DataRequestBridge.shared().detach(dataRequestSink); @@ -763,29 +768,99 @@ public void reportAddDataAttachment( } } - // Task 8.2b. No registry yet, so this does not call the SDK: the SDK made none. + /** + * One created report at a time. The slot is reserved before the SDK is + * asked, because {@code onCreated} arrives later, on the main thread. + * A throw from that call ends the reservation; otherwise the listener does, + * with {@code cr-} or null. + */ @Override public void createReport(final Promise promise) { - promise.resolve(null); + final CreatedReports registry = CreatedReports.shared(); + final int stamp = registry.tryReserve(); + if (stamp == 0) { + Log.i(TAG, "created report - busy"); + promise.reject( + E_REPORT_CREATE_BUSY, + "A created report is already outstanding."); + return; + } + try { + Bugsee.createReport(created -> deliverCreatedReport(registry, stamp, created, promise)); + } catch (final Throwable thrown) { + // The SDK threw before the listener ran. The stamp no-ops once + // this reservation has been cleared and another one opened. + registry.fulfil(stamp, null); + promise.reject(thrown); + } + } + + /** + * Runs on the SDK's listener thread. A throw here would escape onto that + * thread, so it is caught, the reservation is ended, and the promise rejects. + * The stamp is the one captured at reserve time: a listener that arrives + * after {@code invalidate} cleared that reservation cannot end a newer one. + */ + private static void deliverCreatedReport( + final CreatedReports registry, + final int stamp, + @Nullable final Report created, + final Promise promise + ) { + String handle = null; + try { + handle = registry.fulfil(stamp, created); + if (handle == null) { + Log.i(TAG, "created report - none"); + } else { + Log.i(TAG, "created report " + handle + " created"); + } + promise.resolve(handle); + } catch (final Throwable thrown) { + if (handle != null) { + registry.take(handle); + } else { + registry.fulfil(stamp, null); + } + promise.reject(thrown); + } } - // Task 8.2b. No created-report handle exists until the registry lands. @Override public void createdReportRead(final String handleId, final Promise promise) { - rejectHandleDead(promise); + final Report report = CreatedReports.shared().get(handleId); + if (report == null) { + rejectHandleDead(promise); + return; + } + try { + promise.resolve(snapshotToWritableMap(ReportOps.read(report))); + } catch (final Throwable e) { + promise.reject(e); + } } - // Task 8.2b. No created-report handle exists until the registry lands. @Override public void createdReportUpdate( final String handleId, final String patchJson, final Promise promise ) { - rejectHandleDead(promise); + final Report report = CreatedReports.shared().get(handleId); + if (report == null) { + rejectHandleDead(promise); + return; + } + try { + ReportOps.applyJson(report, patchJson); + promise.resolve(null); + } catch (final ReportOps.BadArgument e) { + promise.reject(E_REPORT_BAD_ARGUMENT, e.getMessage()); + } catch (final Throwable e) { + promise.reject(e); + } } - // Task 8.2b. No created-report handle exists until the registry lands. @Override public void createdReportAddDataAttachment( final String handleId, @@ -794,10 +869,27 @@ public void createdReportAddDataAttachment( @Nullable final String mimeType, final Promise promise ) { - rejectHandleDead(promise); + final Report report = CreatedReports.shared().get(handleId); + if (report == null) { + rejectHandleDead(promise); + return; + } + final byte[] data; + try { + data = Base64.decode(base64, Base64.NO_WRAP); + } catch (final IllegalArgumentException e) { + promise.reject(E_REPORT_BAD_ARGUMENT, "data must be base64-encoded"); + return; + } + try { + settleCreatedAttachment( + handleId, ReportOps.addData(report, data, name, mimeType), promise); + } catch (final Throwable e) { + promise.reject(e); + } } - // Task 8.2b. No created-report handle exists until the registry lands. No move argument. + /** Copied, not moved: a created report has no {@code move} argument. */ @Override public void createdReportAddFileAttachment( final String handleId, @@ -806,13 +898,44 @@ public void createdReportAddFileAttachment( @Nullable final String mimeType, final Promise promise ) { - rejectHandleDead(promise); + final Report report = CreatedReports.shared().get(handleId); + if (report == null) { + rejectHandleDead(promise); + return; + } + try { + settleCreatedAttachment( + handleId, + ReportOps.addFile(report, path, name, mimeType, false), + promise); + } catch (final Throwable e) { + promise.reject(e); + } } - // Task 8.2b. No created-report handle exists until the registry lands. + /** + * Takes the report first, so the slot is free whether or not the upload + * succeeds. The handle is dead afterwards. + */ @Override public void createdReportUpload(final String handleId, final Promise promise) { - rejectHandleDead(promise); + final Report report = CreatedReports.shared().take(handleId); + if (report == null) { + rejectHandleDead(promise); + return; + } + try { + Bugsee.upload(report, ok -> { + try { + Log.i(TAG, "created report " + handleId + " uploaded ok=" + ok); + promise.resolve(ok); + } catch (final Throwable thrown) { + promise.reject(thrown); + } + }); + } catch (final Throwable thrown) { + promise.reject(thrown); + } } /** @@ -834,6 +957,21 @@ private static void settleAttachment( } } + /** Same decision as {@link #settleAttachment}, against the created-report registry. */ + private static void settleCreatedAttachment( + final String handleId, + final boolean added, + final Promise promise + ) { + if (added) { + promise.resolve(null); + } else if (CreatedReports.shared().get(handleId) == null) { + rejectHandleDead(promise); + } else { + promise.reject(E_REPORT_ATTACHMENT_REJECTED, "The SDK declined the attachment"); + } + } + private static void rejectHandleDead(final Promise promise) { promise.reject( E_REPORT_HANDLE_DEAD, diff --git a/packages/react-native/android/src/main/java/com/bugsee/reactnative/CreatedReports.java b/packages/react-native/android/src/main/java/com/bugsee/reactnative/CreatedReports.java new file mode 100644 index 00000000..56f49a3a --- /dev/null +++ b/packages/react-native/android/src/main/java/com/bugsee/reactnative/CreatedReports.java @@ -0,0 +1,127 @@ +package com.bugsee.reactnative; + +import androidx.annotation.Nullable; + +import com.bugsee.library.contracts.reporting.Report; + +/** + * The one created report this process may have outstanding. + * + *

A slot is taken by {@link #reserve()} or {@link #tryReserve()} and held + * until {@link #fulfil} stores a report, {@link #fulfil} is given null, + * {@link #take} removes the report, or {@link #clear} drops whatever is + * there. A second reserve while the slot is taken returns false. Handles are + * {@code cr-}, numbered from 1 for the life of the process, and are not + * reused after a take or a clear. + * + *

{@link #clear()} does not cancel an SDK listener that is already in + * flight. Each reservation carries a stamp, and {@link #fulfil} ignores any + * stamp that is not the current one, so that listener cannot store its report + * into a reservation opened afterwards or free that reservation. + */ +final class CreatedReports { + + private static final CreatedReports SHARED = new CreatedReports(); + + /** True from a successful reserve until that reservation is ended. */ + private boolean creating; + + /** + * Stamp of the open reservation. {@code 0} is never a live stamp: it is + * what {@link #tryReserve()} returns when busy, and what {@link #clear()} + * and a finished {@link #fulfil} leave behind so every earlier stamp misses. + */ + private int currentStamp; + + /** Next stamp to issue. Skips {@code 0}, which means "not admitted". */ + private int nextStamp = 1; + + @Nullable + private Report report; + + @Nullable + private String handle; + + /** Next suffix. Never reset: a handle is fresh for the process. */ + private int next = 1; + + static CreatedReports shared() { + return SHARED; + } + + /** Package-private so a test can build an instance that is not {@link #shared()}. */ + CreatedReports() { + } + + /** False while a created report is outstanding or being created. */ + synchronized boolean reserve() { + return tryReserve() != 0; + } + + /** + * {@code 0} when a report is outstanding or being created. Otherwise the + * stamp {@link #fulfil} must present to end this reservation. + */ + synchronized int tryReserve() { + if (creating || report != null) { + return 0; + } + creating = true; + currentStamp = nextStamp; + nextStamp += 1; + if (nextStamp == 0) { + nextStamp = 1; + } + return currentStamp; + } + + /** + * Ends the reservation {@code stamp} names. A report gets {@code cr-} + * and holds the slot. Null frees the slot and mints nothing. A stamp that + * is not the current one, including every stamp issued before + * {@link #clear()}, changes nothing and returns null. + */ + @Nullable + synchronized String fulfil(final int stamp, @Nullable final Report created) { + if (!creating || stamp == 0 || stamp != currentStamp) { + return null; + } + creating = false; + currentStamp = 0; + if (created == null) { + return null; + } + handle = "cr-" + next; + next += 1; + report = created; + return handle; + } + + @Nullable + synchronized Report get(final String handleId) { + if (report != null && handleId != null && handleId.equals(handle)) { + return report; + } + return null; + } + + /** Removes the report and frees the slot. An unknown handle removes nothing. */ + @Nullable + synchronized Report take(final String handleId) { + final Report created = get(handleId); + if (created == null) { + return null; + } + report = null; + handle = null; + return created; + } + + /** Drops the slot and makes every stamp issued so far fail {@link #fulfil}. */ + synchronized void clear() { + creating = false; + report = null; + handle = null; + currentStamp = 0; + } +} diff --git a/packages/react-native/android/src/test/java/com/bugsee/reactnative/CreatedReportsTest.java b/packages/react-native/android/src/test/java/com/bugsee/reactnative/CreatedReportsTest.java new file mode 100644 index 00000000..1239a300 --- /dev/null +++ b/packages/react-native/android/src/test/java/com/bugsee/reactnative/CreatedReportsTest.java @@ -0,0 +1,181 @@ +package com.bugsee.reactnative; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +import com.bugsee.library.contracts.reporting.Report; + +import org.junit.Test; + +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +/** + * One created report is outstanding at a time, from the moment creation is + * reserved until the report is taken for upload, discarded as null, or + * cleared. Handles are {@code cr-}, fresh for the life of the registry. + */ +public class CreatedReportsTest { + + private static Report report() { + return FakeReports.create(new FakeReports.State()); + } + + /** A reservation, and a fulfilled report, each refuse a second reserve. */ + @Test + public void oneOutstandingAtATime() { + final CreatedReports reports = new CreatedReports(); + + final int stamp = reports.tryReserve(); + assertTrue(stamp != 0); + assertFalse(reports.reserve()); + + reports.fulfil(stamp, report()); + assertFalse(reports.reserve()); + } + + /** The SDK made none: the slot opens again and no handle is minted. */ + @Test + public void aNullReportFreesTheSlot() { + final CreatedReports reports = new CreatedReports(); + + final int stamp = reports.tryReserve(); + assertTrue(stamp != 0); + assertNull(reports.fulfil(stamp, null)); + assertTrue(reports.reserve()); + } + + /** Upload takes the report and leaves the slot free. */ + @Test + public void takeFreesTheSlot() { + final CreatedReports reports = new CreatedReports(); + final Report created = report(); + + final int stamp = reports.tryReserve(); + assertTrue(stamp != 0); + final String handle = reports.fulfil(stamp, created); + assertFalse(reports.reserve()); + + assertSame(created, reports.take(handle)); + assertTrue(reports.reserve()); + } + + /** + * The first report is {@code cr-1}, the next {@code cr-2}. Taking the + * first does not hand its handle out again. + */ + @Test + public void handlesAreFreshAndPrefixed() { + final CreatedReports reports = new CreatedReports(); + final Report first = report(); + final Report second = report(); + + final int firstStamp = reports.tryReserve(); + assertTrue(firstStamp != 0); + assertEquals("cr-1", reports.fulfil(firstStamp, first)); + assertSame(first, reports.take("cr-1")); + + final int secondStamp = reports.tryReserve(); + assertTrue(secondStamp != 0); + assertEquals("cr-2", reports.fulfil(secondStamp, second)); + assertNull(reports.get("cr-1")); + assertSame(second, reports.get("cr-2")); + } + + /** After take, get and take both miss that handle. */ + @Test + public void aTakenHandleIsGone() { + final CreatedReports reports = new CreatedReports(); + final Report created = report(); + + final int stamp = reports.tryReserve(); + assertTrue(stamp != 0); + final String handle = reports.fulfil(stamp, created); + assertSame(created, reports.take(handle)); + + assertNull(reports.get(handle)); + assertNull(reports.take(handle)); + } + + /** A held report and a bare reservation both disappear, and the slot opens. */ + @Test + public void clearFreesEverything() { + final CreatedReports reports = new CreatedReports(); + + final int stamp = reports.tryReserve(); + assertTrue(stamp != 0); + final String handle = reports.fulfil(stamp, report()); + reports.clear(); + assertNull(reports.get(handle)); + assertNull(reports.take(handle)); + assertTrue(reports.reserve()); + + reports.clear(); + assertTrue(reports.reserve()); + } + + /** Eight threads leave the barrier together; one reservation is admitted. */ + @Test + public void concurrentReservationsAdmitOne() throws Exception { + final CreatedReports reports = new CreatedReports(); + final int threads = 8; + final CyclicBarrier start = new CyclicBarrier(threads); + final AtomicInteger admitted = new AtomicInteger(); + final AtomicReference failure = new AtomicReference<>(); + final Thread[] workers = new Thread[threads]; + + for (int i = 0; i < threads; i++) { + workers[i] = new Thread(() -> { + try { + start.await(5, TimeUnit.SECONDS); + if (reports.reserve()) { + admitted.incrementAndGet(); + } + } catch (final Throwable t) { + failure.compareAndSet(null, t); + } + }); + workers[i].start(); + } + for (final Thread worker : workers) { + worker.join(5000); + assertFalse(worker.isAlive()); + } + + assertNull(failure.get()); + assertEquals(1, admitted.get()); + } + + /** + * {@code clear} ends a reservation without cancelling the SDK listener. + * That listener's stamp must not store its report into, or free, the + * reservation a later runtime has already opened. A null fulfil with the + * same stale stamp (the listener's failure path) must not either. + */ + @Test + public void aStaleStampDoesNotConsumeANewerReservation() { + final CreatedReports reports = new CreatedReports(); + final int stale = reports.tryReserve(); + assertTrue(stale != 0); + reports.clear(); + + final int live = reports.tryReserve(); + assertTrue(live != 0); + assertTrue(live != stale); + + final Report abandoned = report(); + final Report created = report(); + assertNull(reports.fulfil(stale, abandoned)); + assertNull(reports.fulfil(stale, null)); + assertFalse(reports.reserve()); + + assertEquals("cr-1", reports.fulfil(live, created)); + assertSame(created, reports.get("cr-1")); + assertFalse(reports.reserve()); + } +}