diff --git a/CHANGELOG.md b/CHANGELOG.md index 29d29a751b9..37a8f5f3b6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Features - Make `ISpan.startChild` overloads with `SpanOptions` public ([#5927](https://github.com/getsentry/sentry-java/pull/5927)) +- Add `Sentry.feedback().enableOnShake()`, `Sentry.feedback().disableOnShake()`, and `Sentry.feedback().isOnShakeEnabled()` to toggle and query shake-to-report at runtime ([#5827](https://github.com/getsentry/sentry-java/pull/5827)) ### Fixes @@ -16,7 +17,9 @@ ### Performance +- Read the clock once per performance collection round instead of once per in-flight transaction ([#5934](https://github.com/getsentry/sentry-java/pull/5934)) - Reduce allocations while collecting cpu usage during transactions by reading the process cpu time via `Process.getElapsedCpuTime()` instead of parsing `/proc/self/stat` (33.6kB to 16 bytes per sample on a Pixel 3) ([#5926](https://github.com/getsentry/sentry-java/pull/5926)) +- Store performance measurements as primitives, removing a boxed allocation per measurement per performance sample ([#5935](https://github.com/getsentry/sentry-java/pull/5935)) ### Dependencies diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index da80a74e32c..65bf072f0a0 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -293,9 +293,12 @@ public abstract class io/sentry/android/core/EnvelopeFileObserverIntegration : i public final fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V } -public final class io/sentry/android/core/FeedbackShakeIntegration : android/app/Application$ActivityLifecycleCallbacks, io/sentry/Integration, java/io/Closeable { +public final class io/sentry/android/core/FeedbackShakeIntegration : android/app/Application$ActivityLifecycleCallbacks, io/sentry/Integration, io/sentry/SentryFeedbackOptions$IShakeController, java/io/Closeable { public fun (Landroid/app/Application;)V public fun close ()V + public fun disableOnShake ()V + public fun enableOnShake ()V + public fun isOnShakeEnabled ()Z public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V public fun onActivityDestroyed (Landroid/app/Activity;)V public fun onActivityPaused (Landroid/app/Activity;)V @@ -573,7 +576,9 @@ public abstract interface class io/sentry/android/core/SentryUserFeedbackDialog$ public class io/sentry/android/core/SentryUserFeedbackForm : android/app/AlertDialog { protected fun onCreate (Landroid/os/Bundle;)V + public fun onDetachedFromWindow ()V protected fun onStart ()V + protected fun onStop ()V public fun setCancelable (Z)V public fun setOnDismissListener (Landroid/content/DialogInterface$OnDismissListener;)V public fun show ()V diff --git a/sentry-android-core/build.gradle.kts b/sentry-android-core/build.gradle.kts index 23248d6dae4..0e3708a89bf 100644 --- a/sentry-android-core/build.gradle.kts +++ b/sentry-android-core/build.gradle.kts @@ -115,6 +115,7 @@ dependencies { testImplementation(libs.androidx.test.ext.junit) testImplementation(libs.androidx.test.runner) testImplementation(libs.awaitility.kotlin) + testImplementation(libs.google.truth) testImplementation(libs.mockito.kotlin) testImplementation(libs.mockito.inline) testImplementation(projects.sentryTestSupport) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java index b059b0104da..4405cd19309 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java @@ -4,31 +4,54 @@ import android.app.Activity; import android.app.Application; +import android.app.Dialog; import android.os.Bundle; import io.sentry.IScopes; import io.sentry.Integration; +import io.sentry.SentryFeedbackOptions; import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.util.Objects; import java.io.Closeable; import java.io.IOException; import java.lang.ref.WeakReference; +import java.util.concurrent.CopyOnWriteArrayList; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** - * Detects shake gestures and shows the user feedback dialog when a shake is detected. Only active - * when {@link io.sentry.SentryFeedbackOptions#isUseShakeGesture()} returns {@code true}. + * Detects shake gestures and shows the user feedback dialog when a shake is detected. {@link + * io.sentry.SentryFeedbackOptions#isUseShakeGesture()} determines the initial state; it can be + * toggled at runtime via {@code Sentry.feedback().enableOnShake()} and {@code + * Sentry.feedback().disableOnShake()}. + * + *

Shake detection is scoped to the resumed activity: a dialog belongs to the window of the + * activity that created it, so it can only ever be visible while that activity is resumed. Dialogs + * report themselves via {@link #onDialogVisible(Activity, Dialog)} / {@link #onDialogGone(Dialog)} + * and detection is then suppressed for the activity hosting them, which keeps a shake from stacking + * a second dialog on top of a visible one without letting a dialog on a backgrounded activity + * suppress detection elsewhere. */ public final class FeedbackShakeIntegration - implements Integration, Closeable, Application.ActivityLifecycleCallbacks { + implements Integration, + Closeable, + Application.ActivityLifecycleCallbacks, + SentryFeedbackOptions.IShakeController { private final @NotNull Application application; private final @NotNull SentryShakeDetector shakeDetector; private @Nullable SentryAndroidOptions options; + private volatile boolean enabled = false; private volatile @Nullable WeakReference currentActivityRef; - private volatile boolean isDialogShowing = false; - private volatile @Nullable Runnable previousOnFormClose; + + /** + * The feedback dialogs that are currently visible, together with the activity hosting them. More + * than one can be visible at a time, e.g. when the app calls {@code Sentry.feedback().show()} + * while another dialog is already showing. + */ + private final @NotNull CopyOnWriteArrayList visibleDialogs = + new CopyOnWriteArrayList<>(); public FeedbackShakeIntegration(final @NotNull Application application) { this.application = Objects.requireNonNull(application, "Application is required"); @@ -46,13 +69,25 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions final @NotNull SentryAndroidOptions options = this.options; - if (!options.getFeedbackOptions().isUseShakeGesture()) { + // Always expose the runtime toggle, even when the option starts out disabled. + options.getFeedbackOptions().setShakeController(this); + + if (options.getFeedbackOptions().isUseShakeGesture()) { + enableOnShake(); + } + } + + @Override + public synchronized void enableOnShake() { + final @Nullable SentryAndroidOptions options = this.options; + if (enabled || options == null) { return; } + enabled = true; - // Re-arm the detector in case this integration is being re-registered after a previous close() - // (e.g. a second Sentry.init reusing the same options), otherwise the closed latch would keep - // shake detection off permanently. + // Re-arm the detector in case it was closed before, either by disableOnShake() or by a previous + // close() (e.g. a second Sentry.init reusing the same options), otherwise the closed latch + // would keep shake detection off permanently. shakeDetector.reopen(); // Resolving the accelerometer is the most expensive part of init (the first SensorManager @@ -72,7 +107,7 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions application.registerActivityLifecycleCallbacks(this); options.getLogger().log(SentryLevel.DEBUG, "FeedbackShakeIntegration installed."); - // In case of a deferred init, hook into any already-resumed activity + // In case of a deferred init or runtime enable, hook into any already-resumed activity final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity(); if (activity != null) { currentActivityRef = new WeakReference<>(activity); @@ -81,34 +116,111 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions } @Override - public void close() throws IOException { + public synchronized void disableOnShake() { + if (!enabled) { + return; + } + enabled = false; + application.unregisterActivityLifecycleCallbacks(this); shakeDetector.close(); - // Restore onFormClose if a dialog is still showing, since lifecycle callbacks - // are now unregistered and onActivityDestroyed cleanup won't fire. - if (isDialogShowing) { - isDialogShowing = false; - if (options != null) { - options.getFeedbackOptions().setOnFormClose(previousOnFormClose); - } - previousOnFormClose = null; - } currentActivityRef = null; } @Override - public void onActivityResumed(final @NotNull Activity activity) { - // If a dialog is showing on a different activity (e.g. user navigated via notification), - // clean up since the dialog's host activity is going away and onActivityDestroyed - // won't match currentActivity anymore. - final @Nullable Activity current = currentActivityRef != null ? currentActivityRef.get() : null; - if (isDialogShowing && current != null && current != activity) { - isDialogShowing = false; - if (options != null) { - options.getFeedbackOptions().setOnFormClose(previousOnFormClose); + public boolean isOnShakeEnabled() { + return enabled; + } + + /** + * Reports a feedback dialog as visible on {@code host}. Shake detection is suppressed for that + * activity until the dialog reports back via {@link #onDialogGone(Dialog)}, so a shake can never + * stack a second dialog on top of a visible one — no matter how the visible one was opened. + */ + void onDialogVisible(final @NotNull Activity host, final @NotNull Dialog dialog) { + visibleDialogs.add(new VisibleDialog(host, dialog)); + stopShakeDetection(); + } + + /** Reports a feedback dialog as no longer visible. Safe to call more than once per dialog. */ + void onDialogGone(final @NotNull Dialog dialog) { + if (!removeDialog(dialog)) { + return; + } + final @Nullable WeakReference currentRef = currentActivityRef; + final @Nullable Activity current = currentRef == null ? null : currentRef.get(); + if (enabled && current != null) { + startShakeDetection(current); + } + } + + private boolean removeDialog(final @NotNull Dialog dialog) { + boolean removed = false; + for (final @NotNull VisibleDialog visibleDialog : visibleDialogs) { + // Drop entries whose dialog was collected without reporting back, so they can't suppress + // detection forever. + final @Nullable Dialog trackedDialog = visibleDialog.dialogRef.get(); + if (trackedDialog == dialog) { + removed = visibleDialogs.remove(visibleDialog) || removed; + } else if (trackedDialog == null) { + visibleDialogs.remove(visibleDialog); } - previousOnFormClose = null; } + return removed; + } + + private boolean hasDialogOn(final @NotNull Activity activity) { + for (final @NotNull VisibleDialog visibleDialog : visibleDialogs) { + if (visibleDialog.dialogRef.get() != null && visibleDialog.activityRef.get() == activity) { + return true; + } + } + return false; + } + + @TestOnly + @Nullable + Activity getDialogActivity() { + for (final @NotNull VisibleDialog visibleDialog : visibleDialogs) { + if (visibleDialog.dialogRef.get() != null) { + return visibleDialog.activityRef.get(); + } + } + return null; + } + + /** Creates the dialog shown on shake. Replaceable in tests to simulate a failing show(). */ + interface DialogFactory { + @NotNull + Dialog create(final @NotNull Activity activity); + } + + private @NotNull DialogFactory dialogFactory = + activity -> new SentryUserFeedbackForm.Builder(activity).create(); + + @TestOnly + void setDialogFactory(final @NotNull DialogFactory dialogFactory) { + this.dialogFactory = dialogFactory; + } + + private static final class VisibleDialog { + private final @NotNull WeakReference activityRef; + private final @NotNull WeakReference

dialogRef; + + VisibleDialog(final @NotNull Activity activity, final @NotNull Dialog dialog) { + this.activityRef = new WeakReference<>(activity); + this.dialogRef = new WeakReference<>(dialog); + } + } + + @Override + public void close() throws IOException { + disableOnShake(); + visibleDialogs.clear(); + } + + @Override + public void onActivityResumed(final @NotNull Activity activity) { currentActivityRef = new WeakReference<>(activity); startShakeDetection(activity); } @@ -118,16 +230,11 @@ public void onActivityPaused(final @NotNull Activity activity) { // Only stop if this is the activity we're tracking. When transitioning between // activities, B.onResume may fire before A.onPause — stopping unconditionally // would kill shake detection for the new activity. - final @Nullable Activity current = currentActivityRef != null ? currentActivityRef.get() : null; + final @Nullable WeakReference currentRef = currentActivityRef; + final @Nullable Activity current = currentRef != null ? currentRef.get() : null; if (activity == current) { stopShakeDetection(); - // Keep currentActivityRef set when a dialog is showing so onActivityDestroyed - // can still match and clean up. Otherwise the cleanup condition - // (activity == current) would always be false since onPause fires - // before onDestroy. - if (!isDialogShowing) { - currentActivityRef = null; - } + currentActivityRef = null; } } @@ -146,19 +253,7 @@ public void onActivitySaveInstanceState( final @NotNull Activity activity, final @NotNull Bundle outState) {} @Override - public void onActivityDestroyed(final @NotNull Activity activity) { - // Only reset if this is the activity that hosts the dialog — the dialog cannot - // outlive its host activity being destroyed. - final @Nullable Activity current = currentActivityRef != null ? currentActivityRef.get() : null; - if (isDialogShowing && activity == current) { - isDialogShowing = false; - currentActivityRef = null; - if (options != null) { - options.getFeedbackOptions().setOnFormClose(previousOnFormClose); - } - previousOnFormClose = null; - } - } + public void onActivityDestroyed(final @NotNull Activity activity) {} private void startShakeDetection(final @NotNull Activity activity) { if (options == null) { @@ -166,47 +261,50 @@ private void startShakeDetection(final @NotNull Activity activity) { } // Stop any existing detection (e.g. when transitioning between activities) stopShakeDetection(); + // A dialog is already visible here, so a shake could only stack a second one on top of it. + // The dialog has no detector of its own in this case: SentryUserFeedbackForm only starts one + // while shake-to-report is globally disabled, which is exactly when this integration is not + // detecting either. + if (hasDialogOn(activity)) { + return; + } shakeDetector.start( activity, () -> { final @Nullable WeakReference ref = currentActivityRef; final Activity active = ref != null ? ref.get() : null; final Boolean inBackground = AppState.getInstance().isInBackground(); - if (active != null - && options != null - && !isDialogShowing - && !Boolean.TRUE.equals(inBackground)) { - active.runOnUiThread( - () -> { - if (isDialogShowing || active.isFinishing() || active.isDestroyed()) { - return; - } - try { - isDialogShowing = true; - final Runnable captured = options.getFeedbackOptions().getOnFormClose(); - previousOnFormClose = captured; - options - .getFeedbackOptions() - .setOnFormClose( - () -> { - isDialogShowing = false; - options.getFeedbackOptions().setOnFormClose(captured); - if (captured != null) { - captured.run(); - } - previousOnFormClose = null; - }); - new SentryUserFeedbackForm.Builder(active).create().show(); - } catch (Throwable e) { - isDialogShowing = false; - options.getFeedbackOptions().setOnFormClose(previousOnFormClose); - previousOnFormClose = null; - options - .getLogger() - .log(SentryLevel.ERROR, "Failed to show feedback dialog on shake.", e); - } - }); + if (active == null + || options == null + || !enabled + || hasDialogOn(active) + || Boolean.TRUE.equals(inBackground)) { + return; } + active.runOnUiThread( + () -> { + // Re-check on the main thread: shake-to-report may have been disabled, or an + // earlier queued shake may have shown a dialog in the meantime (the dialog reports + // itself synchronously in onStart). + if (!enabled + || hasDialogOn(active) + || active.isFinishing() + || active.isDestroyed()) { + return; + } + @Nullable Dialog dialog = null; + try { + dialog = dialogFactory.create(active); + dialog.show(); + } catch (Throwable e) { + if (dialog != null) { + onDialogGone(dialog); + } + options + .getLogger() + .log(SentryLevel.ERROR, "Failed to show feedback dialog on shake.", e); + } + }); }); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java index 43500d50ebc..01d4546d877 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java @@ -60,9 +60,12 @@ public class SentryUserFeedbackForm extends AlertDialog { } private void maybeStartShakeDetection(final @NotNull Context context) { + // Only start shake detection if it's enabled within the options, + // and not already running globally final @NotNull SentryFeedbackOptions globalFeedbackOptions = Sentry.getCurrentScopes().getOptions().getFeedbackOptions(); - if (!resolvedFeedbackOptions.isUseShakeGesture() || globalFeedbackOptions.isUseShakeGesture()) { + if (!resolvedFeedbackOptions.isUseShakeGesture() + || globalFeedbackOptions.getShakeController().isOnShakeEnabled()) { return; } final @Nullable Activity activity = getActivity(context); @@ -95,6 +98,15 @@ private void stopShakeDetection() { private @NotNull SentryShakeDetector.Listener shakeListener( final @NotNull WeakReference activityRef) { return () -> { + // If shake-to-report got enabled globally in the meantime, FeedbackShakeIntegration + // reacts to the same shake — don't show a second dialog for it. + if (Sentry.getCurrentScopes() + .getOptions() + .getFeedbackOptions() + .getShakeController() + .isOnShakeEnabled()) { + return; + } final @Nullable Activity active = activityRef.get(); if (active != null && !active.isFinishing() && !active.isDestroyed()) { active.runOnUiThread( @@ -284,13 +296,27 @@ protected void onCreate(Bundle savedInstanceState) { final @Nullable SentryFeedbackOptions.SentryFeedbackCallback onSubmitSuccess = feedbackOptions.getOnSubmitSuccess(); if (onSubmitSuccess != null) { - onSubmitSuccess.call(feedback); + try { + onSubmitSuccess.call(feedback); + } catch (Exception e) { + Sentry.getCurrentScopes() + .getOptions() + .getLogger() + .log(SentryLevel.ERROR, "onSubmitSuccess callback threw an exception.", e); + } } } else { final @Nullable SentryFeedbackOptions.SentryFeedbackCallback onSubmitError = feedbackOptions.getOnSubmitError(); if (onSubmitError != null) { - onSubmitError.call(feedback); + try { + onSubmitError.call(feedback); + } catch (Exception e) { + Sentry.getCurrentScopes() + .getOptions() + .getLogger() + .log(SentryLevel.ERROR, "onSubmitError callback threw an exception.", e); + } } } cancel(); @@ -310,7 +336,15 @@ public void setOnDismissListener(final @Nullable OnDismissListener listener) { if (onFormClose != null) { super.setOnDismissListener( dialog -> { - onFormClose.run(); + // User-provided callback: a crash in it must not take down the app or skip the + // cleanup and the user's own dismiss listener below + try { + onFormClose.run(); + } catch (Exception e) { + options + .getLogger() + .log(SentryLevel.ERROR, "onFormClose callback threw an exception.", e); + } currentReplayId = null; if (delegate != null) { delegate.onDismiss(dialog); @@ -324,7 +358,7 @@ public void setOnDismissListener(final @Nullable OnDismissListener listener) { @Override protected void onStart() { super.onStart(); - // Clear the message field so subsequent show() calls start with a fresh form + // Clear the message field so subsequent show() calls start with a fresh dialog final @NotNull EditText edtMessage = findViewById(R.id.sentry_dialog_user_feedback_edt_description); edtMessage.getText().clear(); @@ -332,14 +366,58 @@ protected void onStart() { final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions(); final @NotNull SentryFeedbackOptions feedbackOptions = options.getFeedbackOptions(); + // Pause shake-to-report on this dialog's activity while it is visible, so a shake can't stack + // a second dialog on top of it + final @Nullable FeedbackShakeIntegration integration = getFeedbackShakeIntegration(); + final @Nullable Activity activity = getActivity(getContext()); + if (integration != null && activity != null) { + integration.onDialogVisible(activity, this); + } final @Nullable Runnable onFormOpen = feedbackOptions.getOnFormOpen(); if (onFormOpen != null) { - onFormOpen.run(); + try { + onFormOpen.run(); + } catch (Exception e) { + options.getLogger().log(SentryLevel.ERROR, "onFormOpen callback threw an exception.", e); + } } options.getReplayController().captureReplay(false); currentReplayId = options.getReplayController().getReplayId(); } + @Override + protected void onStop() { + super.onStop(); + final @Nullable FeedbackShakeIntegration integration = getFeedbackShakeIntegration(); + if (integration != null) { + integration.onDialogGone(this); + } + } + + @Override + public void onDetachedFromWindow() { + super.onDetachedFromWindow(); + // Runs on every teardown: on dismiss the decor view is removed before onStop(), and when the + // host activity is destroyed with the dialog still showing this is the only callback that + // fires. onDialogGone is idempotent, so reporting from both here and onStop() is safe. + final @Nullable FeedbackShakeIntegration integration = getFeedbackShakeIntegration(); + if (integration != null) { + integration.onDialogGone(this); + } + } + + /** + * The shake integration to report this dialog's visibility to, or null when shake-to-report isn't + * available (non-Android controller, or the integration was never installed). + */ + private @Nullable FeedbackShakeIntegration getFeedbackShakeIntegration() { + final @NotNull SentryFeedbackOptions.IShakeController controller = + Sentry.getCurrentScopes().getOptions().getFeedbackOptions().getShakeController(); + return controller instanceof FeedbackShakeIntegration + ? (FeedbackShakeIntegration) controller + : null; + } + @Override public void show() { // If Sentry is disabled, don't show the dialog, but log a warning diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt index 170211abf55..f2118fb76de 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt @@ -2,8 +2,16 @@ package io.sentry.android.core import android.app.Activity import android.app.Application +import android.app.Dialog import android.content.Context +import android.hardware.Sensor +import android.hardware.SensorEvent +import android.hardware.SensorEventListener +import android.hardware.SensorManager +import android.os.Handler +import android.view.WindowManager import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat import io.sentry.Scopes import io.sentry.SentryFeedbackOptions import io.sentry.test.DeferredExecutorService @@ -12,10 +20,14 @@ import kotlin.test.BeforeTest import kotlin.test.Test import org.junit.runner.RunWith import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.atLeastOnce +import org.mockito.kotlin.doAnswer import org.mockito.kotlin.eq +import org.mockito.kotlin.isA import org.mockito.kotlin.mock import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -165,4 +177,316 @@ class FeedbackShakeIntegrationTest { val sut = fixture.getSut() sut.close() } + + @Test + fun `register sets itself as shake controller even when useShakeGesture is disabled`() { + val sut = fixture.getSut(useShakeGesture = false) + sut.register(fixture.scopes, fixture.options) + + assertThat(fixture.options.feedbackOptions.shakeController).isSameInstanceAs(sut) + assertThat(sut.isOnShakeEnabled).isFalse() + } + + @Test + fun `enable after register starts shake detection at runtime`() { + CurrentActivityHolder.getInstance().setActivity(fixture.activity) + whenever(fixture.activity.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = false) + sut.register(fixture.scopes, fixture.options) + verify(fixture.application, never()).registerActivityLifecycleCallbacks(any()) + + sut.enableOnShake() + + assertThat(sut.isOnShakeEnabled).isTrue() + verify(fixture.application).registerActivityLifecycleCallbacks(any()) + // Hooks into the already-resumed activity + verify(fixture.activity).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `enable is idempotent`() { + val sut = fixture.getSut(useShakeGesture = false) + sut.register(fixture.scopes, fixture.options) + + sut.enableOnShake() + sut.enableOnShake() + + verify(fixture.application, times(1)).registerActivityLifecycleCallbacks(any()) + } + + @Test + fun `disable stops shake detection at runtime`() { + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + sut.disableOnShake() + + assertThat(sut.isOnShakeEnabled).isFalse() + verify(fixture.application).unregisterActivityLifecycleCallbacks(any()) + } + + @Test + fun `disable is idempotent`() { + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + sut.disableOnShake() + sut.disableOnShake() + + verify(fixture.application, times(1)).unregisterActivityLifecycleCallbacks(any()) + } + + @Test + fun `disable when never enabled does not unregister callbacks`() { + val sut = fixture.getSut(useShakeGesture = false) + sut.register(fixture.scopes, fixture.options) + + sut.disableOnShake() + + verify(fixture.application, never()).unregisterActivityLifecycleCallbacks(any()) + } + + @Test + fun `enable before register is a no-op`() { + val sut = fixture.getSut(useShakeGesture = false) + + sut.enableOnShake() + + assertThat(sut.isOnShakeEnabled).isFalse() + verify(fixture.application, never()).registerActivityLifecycleCallbacks(any()) + } + + @Test + fun `re-enable after disable re-arms shake detection`() { + val deferredExecutor = DeferredExecutorService() + fixture.options.executorService = deferredExecutor + whenever(fixture.application.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + sut.disableOnShake() + sut.enableOnShake() + + deferredExecutor.runAll() + + assertThat(sut.isOnShakeEnabled).isTrue() + verify(fixture.application, atLeastOnce()).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `close disables shake detection`() { + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + sut.close() + + assertThat(sut.isOnShakeEnabled).isFalse() + } + + @Test + fun `a visible dialog does not tear down the detection machinery`() { + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + val dialog = mock() + sut.onDialogVisible(fixture.activity, dialog) + sut.onDialogGone(dialog) + + verify(fixture.application, never()).unregisterActivityLifecycleCallbacks(any()) + assertThat(sut.isOnShakeEnabled).isTrue() + } + + @Test + fun `a dialog suppresses detection on the activity it belongs to`() { + whenever(fixture.activity.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + sut.onDialogVisible(fixture.activity, mock()) + assertThat(sut.dialogActivity).isSameInstanceAs(fixture.activity) + + // Coming back to the activity the dialog is on (e.g. screen off/on) must not re-arm detection, + // otherwise a shake would stack a second dialog on top of the visible one. + sut.onActivityResumed(fixture.activity) + + verify(fixture.activity, never()).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `a dialog on a backgrounded activity does not suppress detection on the next one`() { + // A dialog lives in the window of the activity that created it, so once that activity is no + // longer resumed the dialog cannot be seen - it must not keep detection off on the activity + // now in front. Android's order is A.onPause() -> B.onResume(), so exercise exactly that. + val otherActivity = mock() + whenever(fixture.activity.getSystemService(any())).thenReturn(null) + whenever(otherActivity.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + CurrentActivityHolder.getInstance().setActivity(fixture.activity) + sut.onActivityResumed(fixture.activity) + sut.onDialogVisible(fixture.activity, mock()) + + sut.onActivityPaused(fixture.activity) + sut.onActivityResumed(otherActivity) + + verify(otherActivity).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `a dialog reports the activity it is showing on, not the current one`() { + // The dialog's host activity is what a stacked dialog would land on, so a mid-transition + // CurrentActivityHolder must not decide which activity detection is suppressed for. + val otherActivity = mock() + whenever(fixture.activity.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + CurrentActivityHolder.getInstance().setActivity(otherActivity) + sut.onDialogVisible(fixture.activity, mock()) + + assertThat(sut.dialogActivity).isSameInstanceAs(fixture.activity) + + sut.onActivityResumed(fixture.activity) + + verify(fixture.activity, never()).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `dismissing a dialog re-arms detection on the current activity`() { + whenever(fixture.activity.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + CurrentActivityHolder.getInstance().setActivity(fixture.activity) + sut.onActivityResumed(fixture.activity) + val dialog = mock() + sut.onDialogVisible(fixture.activity, dialog) + sut.onDialogGone(dialog) + + assertThat(sut.dialogActivity).isNull() + verify(fixture.activity, atLeastOnce()).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `dismissing one of two visible dialogs keeps detection suppressed`() { + // Two dialogs can be visible at once, e.g. when the app calls showForm() while a dialog is + // already up. The first one going away must not re-arm detection under the second. + whenever(fixture.activity.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + CurrentActivityHolder.getInstance().setActivity(fixture.activity) + sut.onActivityResumed(fixture.activity) + val first = mock() + val second = mock() + sut.onDialogVisible(fixture.activity, first) + sut.onDialogVisible(fixture.activity, second) + + sut.onDialogGone(first) + assertThat(sut.dialogActivity).isSameInstanceAs(fixture.activity) + verify(fixture.activity, times(1)).getSystemService(eq(Context.SENSOR_SERVICE)) + + sut.onDialogGone(second) + assertThat(sut.dialogActivity).isNull() + verify(fixture.activity, times(2)).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `reporting the same dialog gone twice re-arms detection only once`() { + // A dismissed dialog reports back from both onStop() and onDetachedFromWindow(). + whenever(fixture.activity.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + CurrentActivityHolder.getInstance().setActivity(fixture.activity) + sut.onActivityResumed(fixture.activity) + val dialog = mock() + sut.onDialogVisible(fixture.activity, dialog) + sut.onDialogGone(dialog) + sut.onDialogGone(dialog) + + // Once for the resume, once for the single re-arm - the second report is a no-op. + verify(fixture.activity, times(2)).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `a dialog that fails to show does not leave detection suppressed`() { + // Dialog.show() runs onStart() - which reports the dialog as visible and stops detection - + // before the window is added, so an addView() failure hits with the dialog already tracked + // and no lifecycle callback left to report it gone. + val sensorManager = mock() + val accelerometer = mock() + whenever(fixture.activity.getSystemService(Context.SENSOR_SERVICE)).thenReturn(sensorManager) + whenever(sensorManager.getDefaultSensor(Sensor.TYPE_ACCELEROMETER, false)) + .thenReturn(accelerometer) + whenever(fixture.activity.runOnUiThread(any())).thenAnswer { + (it.arguments[0] as Runnable).run() + null + } + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + CurrentActivityHolder.getInstance().setActivity(fixture.activity) + sut.onActivityResumed(fixture.activity) + + val dialog = mock() + doAnswer { + sut.onDialogVisible(fixture.activity, dialog) + throw WindowManager.BadTokenException("Unable to add window") + } + .whenever(dialog) + .show() + sut.setDialogFactory { dialog } + + val listener = argumentCaptor() + verify(sensorManager) + .registerListener( + listener.capture(), + eq(accelerometer), + eq(SensorManager.SENSOR_DELAY_NORMAL), + isA(), + ) + shake(listener.lastValue) + + verify(dialog).show() + assertThat(sut.dialogActivity).isNull() + verify(sensorManager, times(2)) + .registerListener( + any(), + eq(accelerometer), + eq(SensorManager.SENSOR_DELAY_NORMAL), + isA(), + ) + } + + private fun shake(listener: SensorEventListener) { + val baseTimestamp = 1_000_000_000L + val intervalNs = 20_000_000L + for (i in 0 until 20) { + listener.onSensorChanged( + createSensorEvent(floatArrayOf(20f, 0f, 0f), baseTimestamp + i * intervalNs) + ) + } + } + + private fun createSensorEvent(values: FloatArray, timestamp: Long): SensorEvent { + val sensor = mock() + whenever(sensor.type).thenReturn(Sensor.TYPE_ACCELEROMETER) + + val constructor = SensorEvent::class.java.getDeclaredConstructor(Int::class.javaPrimitiveType) + constructor.isAccessible = true + val event = constructor.newInstance(values.size) + values.copyInto(event.values) + SensorEvent::class.java.getField("sensor").set(event, sensor) + SensorEvent::class.java.getField("timestamp").set(event, timestamp) + return event + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryUserFeedbackFormTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryUserFeedbackFormTest.kt index 9df2a16d72e..852b4e7e8f0 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryUserFeedbackFormTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryUserFeedbackFormTest.kt @@ -1,6 +1,9 @@ package io.sentry.android.core +import android.app.Activity +import android.app.Application import android.content.Context +import android.os.Looper import android.view.WindowManager import android.widget.TextView import androidx.test.core.app.ApplicationProvider @@ -19,13 +22,18 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotEquals import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertTrue import org.junit.runner.RunWith import org.mockito.Mockito.mockStatic +import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever +import org.robolectric.Robolectric +import org.robolectric.Shadows.shadowOf @RunWith(AndroidJUnit4::class) class SentryUserFeedbackFormTest { @@ -143,4 +151,66 @@ class SentryUserFeedbackFormTest { val flags = window.attributes.flags assertEquals(0, flags and WindowManager.LayoutParams.FLAG_ALT_FOCUSABLE_IM) } + + @Test + fun `a crashing onFormClose callback does not crash the app when the dialog is closed`() { + fixture.options.isEnabled = true + fixture.options.feedbackOptions.onFormClose = Runnable { throw RuntimeException("user bug") } + val sut = fixture.getSut() + sut.show() + + sut.dismiss() + // The dismiss listener is dispatched via a Handler message + shadowOf(Looper.getMainLooper()).idle() + + verify(fixture.mockLogger) + .log(eq(SentryLevel.ERROR), eq("onFormClose callback threw an exception."), any()) + } + + @Test + fun `a crashing onFormClose callback still runs the user's dismiss listener`() { + fixture.options.isEnabled = true + fixture.options.feedbackOptions.onFormClose = Runnable { throw RuntimeException("user bug") } + val sut = fixture.getSut() + var dismissed = false + sut.setOnDismissListener { dismissed = true } + sut.show() + + sut.dismiss() + shadowOf(Looper.getMainLooper()).idle() + + assertTrue(dismissed) + } + + @Test + fun `a crashing onFormOpen callback does not crash the app when the dialog is shown`() { + fixture.options.isEnabled = true + fixture.options.feedbackOptions.onFormOpen = Runnable { throw RuntimeException("user bug") } + val sut = fixture.getSut() + + sut.show() + + verify(fixture.mockLogger) + .log(eq(SentryLevel.ERROR), eq("onFormOpen callback threw an exception."), any()) + // The form open must still complete its own work after the callback crash + verify(fixture.mockReplayController).captureReplay(eq(false)) + } + + @Test + fun `dialog reports its own host activity to the shake integration while visible`() { + fixture.options.isEnabled = true + val integration = FeedbackShakeIntegration(fixture.application as Application) + fixture.options.feedbackOptions.setShakeController(integration) + val activity = Robolectric.buildActivity(Activity::class.java).setup().get() + + val sut = SentryUserFeedbackForm(activity, 0, null, null, null) + sut.show() + + assertEquals(activity, integration.dialogActivity) + + sut.dismiss() + shadowOf(Looper.getMainLooper()).idle() + + assertNull(integration.dialogActivity) + } } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.kt index 90e75feee71..b38f17ed64c 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.kt @@ -805,21 +805,23 @@ fun UserFeedbackScreen() { } } - // Enable shake-to-show for a specific form instance + // Toggle shake-to-show at runtime using the global Sentry.feedback() API item(span = { GridItemSpan(maxLineSpan) }) { + var shakeEnabled by remember { mutableStateOf(Sentry.feedback().isOnShakeEnabled) } Button( modifier = Modifier, onClick = { - SentryUserFeedbackForm.Builder(activity) - .configurator { options -> - options.isUseShakeGesture = true - options.formTitle = "Shake Feedback" - } - .create() - Toast.makeText(activity, "Shake your device to open the form!", Toast.LENGTH_SHORT).show() + if (shakeEnabled) { + Sentry.feedback().disableOnShake() + } else { + Sentry.feedback().enableOnShake() + Toast.makeText(activity, "Shake your device to open the form!", Toast.LENGTH_SHORT) + .show() + } + shakeEnabled = Sentry.feedback().isOnShakeEnabled }, ) { - Text(text = "Enable Shake-to-Show") + Text(text = if (shakeEnabled) "Disable Shake-to-Show" else "Enable Shake-to-Show") } } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 3f8176de977..b02815a73e3 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -854,6 +854,9 @@ public abstract interface class io/sentry/IFeedbackApi { public abstract fun capture (Lio/sentry/protocol/Feedback;)Lio/sentry/protocol/SentryId; public abstract fun capture (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public abstract fun capture (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public abstract fun disableOnShake ()V + public abstract fun enableOnShake ()V + public abstract fun isOnShakeEnabled ()Z public abstract fun show ()V public abstract fun show (Lio/sentry/SentryFeedbackOptions$OptionsConfigurator;)V public abstract fun show (Lio/sentry/protocol/SentryId;Lio/sentry/SentryFeedbackOptions$OptionsConfigurator;)V @@ -1605,7 +1608,10 @@ public final class io/sentry/NoOpFeedbackApi : io/sentry/IFeedbackApi { public fun capture (Lio/sentry/protocol/Feedback;)Lio/sentry/protocol/SentryId; public fun capture (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun capture (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public fun disableOnShake ()V + public fun enableOnShake ()V public static fun getInstance ()Lio/sentry/NoOpFeedbackApi; + public fun isOnShakeEnabled ()Z public fun show ()V public fun show (Lio/sentry/SentryFeedbackOptions$OptionsConfigurator;)V public fun show (Lio/sentry/protocol/SentryId;Lio/sentry/SentryFeedbackOptions$OptionsConfigurator;)V @@ -3232,6 +3238,7 @@ public final class io/sentry/SentryFeedbackOptions { public fun getOnFormOpen ()Ljava/lang/Runnable; public fun getOnSubmitError ()Lio/sentry/SentryFeedbackOptions$SentryFeedbackCallback; public fun getOnSubmitSuccess ()Lio/sentry/SentryFeedbackOptions$SentryFeedbackCallback; + public fun getShakeController ()Lio/sentry/SentryFeedbackOptions$IShakeController; public fun getSubmitButtonLabel ()Ljava/lang/CharSequence; public fun getSuccessMessageText ()Ljava/lang/CharSequence; public fun isEmailRequired ()Z @@ -3257,6 +3264,7 @@ public final class io/sentry/SentryFeedbackOptions { public fun setOnFormOpen (Ljava/lang/Runnable;)V public fun setOnSubmitError (Lio/sentry/SentryFeedbackOptions$SentryFeedbackCallback;)V public fun setOnSubmitSuccess (Lio/sentry/SentryFeedbackOptions$SentryFeedbackCallback;)V + public fun setShakeController (Lio/sentry/SentryFeedbackOptions$IShakeController;)V public fun setShowBranding (Z)V public fun setShowEmail (Z)V public fun setShowName (Z)V @@ -3271,6 +3279,12 @@ public abstract interface class io/sentry/SentryFeedbackOptions$IFormHandler { public abstract fun showForm (Lio/sentry/protocol/SentryId;Lio/sentry/SentryFeedbackOptions$OptionsConfigurator;)V } +public abstract interface class io/sentry/SentryFeedbackOptions$IShakeController { + public abstract fun disableOnShake ()V + public abstract fun enableOnShake ()V + public abstract fun isOnShakeEnabled ()Z +} + public abstract interface class io/sentry/SentryFeedbackOptions$OptionsConfigurator { public abstract fun configure (Lio/sentry/SentryFeedbackOptions;)V } diff --git a/sentry/src/main/java/io/sentry/FeedbackApi.java b/sentry/src/main/java/io/sentry/FeedbackApi.java index b8b8a3c9b9a..3822c6fd7c7 100644 --- a/sentry/src/main/java/io/sentry/FeedbackApi.java +++ b/sentry/src/main/java/io/sentry/FeedbackApi.java @@ -31,6 +31,21 @@ public void show( options.getFeedbackOptions().getFormHandler().showForm(associatedEventId, configurator); } + @Override + public void enableOnShake() { + scopes.getOptions().getFeedbackOptions().getShakeController().enableOnShake(); + } + + @Override + public void disableOnShake() { + scopes.getOptions().getFeedbackOptions().getShakeController().disableOnShake(); + } + + @Override + public boolean isOnShakeEnabled() { + return scopes.getOptions().getFeedbackOptions().getShakeController().isOnShakeEnabled(); + } + @Override public @NotNull SentryId capture(final @NotNull Feedback feedback) { return scopes.captureFeedback(feedback); diff --git a/sentry/src/main/java/io/sentry/IFeedbackApi.java b/sentry/src/main/java/io/sentry/IFeedbackApi.java index 5bab630fa8b..b0915bc43c2 100644 --- a/sentry/src/main/java/io/sentry/IFeedbackApi.java +++ b/sentry/src/main/java/io/sentry/IFeedbackApi.java @@ -15,6 +15,33 @@ void show( final @Nullable SentryId associatedEventId, final @Nullable SentryFeedbackOptions.OptionsConfigurator configurator); + /** + * Enables showing the feedback form when a shake gesture is detected, overriding {@link + * SentryFeedbackOptions#isUseShakeGesture()}. Only supported on Android; no-op on other + * platforms. + */ + void enableOnShake(); + + /** + * Disables showing the feedback form when a shake gesture is detected, overriding {@link + * SentryFeedbackOptions#isUseShakeGesture()}. Only supported on Android; no-op on other + * platforms. + * + *

This turns off the SDK-wide shake detection only. A feedback form whose own options enable + * the shake gesture still runs its own detection while it is showing and while its host activity + * is alive, so shaking can re-open that form. + */ + void disableOnShake(); + + /** + * Whether showing the feedback form on a shake gesture is currently enabled. Always {@code false} + * on non-Android platforms. Reflects the SDK-wide shake detection only; a form running its own + * shake detection is not covered by this. + * + * @return true if the feedback form is shown when a shake gesture is detected + */ + boolean isOnShakeEnabled(); + @NotNull SentryId capture(final @NotNull Feedback feedback); diff --git a/sentry/src/main/java/io/sentry/NoOpFeedbackApi.java b/sentry/src/main/java/io/sentry/NoOpFeedbackApi.java index bdef5d37590..23ea5cb47eb 100644 --- a/sentry/src/main/java/io/sentry/NoOpFeedbackApi.java +++ b/sentry/src/main/java/io/sentry/NoOpFeedbackApi.java @@ -26,6 +26,17 @@ public void show( final @Nullable SentryId associatedEventId, final @Nullable SentryFeedbackOptions.OptionsConfigurator configurator) {} + @Override + public void enableOnShake() {} + + @Override + public void disableOnShake() {} + + @Override + public boolean isOnShakeEnabled() { + return false; + } + @Override public @NotNull SentryId capture(final @NotNull Feedback feedback) { return SentryId.EMPTY_ID; diff --git a/sentry/src/main/java/io/sentry/SentryFeedbackOptions.java b/sentry/src/main/java/io/sentry/SentryFeedbackOptions.java index a72b352317e..69b16b87ac1 100644 --- a/sentry/src/main/java/io/sentry/SentryFeedbackOptions.java +++ b/sentry/src/main/java/io/sentry/SentryFeedbackOptions.java @@ -93,8 +93,12 @@ public final class SentryFeedbackOptions { private @NotNull IFormHandler iFormHandler; - SentryFeedbackOptions(@NotNull IFormHandler iFormHandler) { + private @NotNull IShakeController shakeController; + + SentryFeedbackOptions( + final @NotNull IFormHandler iFormHandler, final @NotNull IShakeController shakeController) { this.iFormHandler = iFormHandler; + this.shakeController = shakeController; } /** Creates a copy of the passed {@link SentryFeedbackOptions}. */ @@ -122,6 +126,7 @@ public SentryFeedbackOptions(final @NotNull SentryFeedbackOptions other) { this.onSubmitSuccess = other.onSubmitSuccess; this.onSubmitError = other.onSubmitError; this.iFormHandler = other.iFormHandler; + this.shakeController = other.shakeController; } /** @@ -554,6 +559,26 @@ public void setFormHandler(final @NotNull IFormHandler iFormHandler) { return iFormHandler; } + /** + * Sets the controller to be used to enable/disable shake-to-report at runtime. + * + * @param shakeController the controller to be used to enable/disable shake-to-report at runtime + */ + @ApiStatus.Internal + public void setShakeController(final @NotNull IShakeController shakeController) { + this.shakeController = shakeController; + } + + /** + * Gets the controller to be used to enable/disable shake-to-report at runtime. + * + * @return the controller to be used to enable/disable shake-to-report at runtime + */ + @ApiStatus.Internal + public @NotNull IShakeController getShakeController() { + return shakeController; + } + @Override public String toString() { return "SentryFeedbackOptions{" @@ -615,6 +640,16 @@ void showForm( final @Nullable SentryFeedbackOptions.OptionsConfigurator configurator); } + /** Controls shake-to-report at runtime, overriding {@link #isUseShakeGesture()}. */ + @ApiStatus.Internal + public interface IShakeController { + void enableOnShake(); + + void disableOnShake(); + + boolean isOnShakeEnabled(); + } + /** Configuration callback for feedback options. */ public interface OptionsConfigurator { diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index dc9d9521dbb..49b1d0deaf2 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -3538,7 +3538,23 @@ private SentryOptions(final boolean empty) { feedbackOptions = new SentryFeedbackOptions( (associatedEventId, configurator) -> - logger.log(SentryLevel.WARNING, "showForm() can only be called in Android.")); + logger.log(SentryLevel.WARNING, "showForm() can only be called in Android."), + new SentryFeedbackOptions.IShakeController() { + @Override + public void enableOnShake() { + logger.log(SentryLevel.WARNING, "Shake to report is only supported on Android."); + } + + @Override + public void disableOnShake() { + logger.log(SentryLevel.WARNING, "Shake to report is only supported on Android."); + } + + @Override + public boolean isOnShakeEnabled() { + return false; + } + }); if (!empty) { setSpanFactory(SpanFactoryFactory.create(new LoadClass(), NoOpLogger.getInstance())); diff --git a/sentry/src/test/java/io/sentry/FeedbackApiTest.kt b/sentry/src/test/java/io/sentry/FeedbackApiTest.kt new file mode 100644 index 00000000000..762e5241b4e --- /dev/null +++ b/sentry/src/test/java/io/sentry/FeedbackApiTest.kt @@ -0,0 +1,47 @@ +package io.sentry + +import com.google.common.truth.Truth.assertThat +import kotlin.test.Test +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +class FeedbackApiTest { + + private class Fixture { + val shakeController = mock() + val options = SentryOptions().apply { feedbackOptions.setShakeController(shakeController) } + val scopes = mock().also { whenever(it.options).thenReturn(options) } + + fun getSut(): FeedbackApi = FeedbackApi(scopes) + } + + private val fixture = Fixture() + + @Test + fun `enableOnShake delegates to the shake controller`() { + fixture.getSut().enableOnShake() + + verify(fixture.shakeController).enableOnShake() + } + + @Test + fun `disableOnShake delegates to the shake controller`() { + fixture.getSut().disableOnShake() + + verify(fixture.shakeController).disableOnShake() + } + + @Test + fun `isOnShakeEnabled delegates to the shake controller`() { + whenever(fixture.shakeController.isOnShakeEnabled).thenReturn(true) + + assertThat(fixture.getSut().isOnShakeEnabled).isTrue() + verify(fixture.shakeController).isOnShakeEnabled + } + + @Test + fun `default shake controller is disabled`() { + assertThat(SentryOptions().feedbackOptions.shakeController.isOnShakeEnabled).isFalse() + } +} diff --git a/sentry/src/test/java/io/sentry/SentryFeedbackOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryFeedbackOptionsTest.kt index e4b96cb17d0..c3e0dbd7303 100644 --- a/sentry/src/test/java/io/sentry/SentryFeedbackOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryFeedbackOptionsTest.kt @@ -1,6 +1,7 @@ package io.sentry import io.sentry.SentryFeedbackOptions.IFormHandler +import io.sentry.SentryFeedbackOptions.IShakeController import kotlin.test.Test import kotlin.test.assertEquals import org.mockito.kotlin.mock @@ -8,7 +9,7 @@ import org.mockito.kotlin.mock class SentryFeedbackOptionsTest { @Test fun `feedback options is initialized with default values`() { - val options = SentryFeedbackOptions(mock()) + val options = SentryFeedbackOptions(mock(), mock()) assertEquals(false, options.isNameRequired) assertEquals(true, options.isShowName) assertEquals(false, options.isEmailRequired) @@ -35,7 +36,7 @@ class SentryFeedbackOptionsTest { @Test fun `feedback options copy constructor`() { val options = - SentryFeedbackOptions(mock()).apply { + SentryFeedbackOptions(mock(), mock()).apply { isNameRequired = true isShowName = false isEmailRequired = true @@ -81,5 +82,6 @@ class SentryFeedbackOptionsTest { assertEquals(options.onSubmitSuccess, optionsCopy.onSubmitSuccess) assertEquals(options.onSubmitError, optionsCopy.onSubmitError) assertEquals(options.formHandler, optionsCopy.formHandler) + assertEquals(options.shakeController, optionsCopy.shakeController) } } diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index f1ee97459a1..64482b5d5d0 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -947,6 +947,18 @@ class SentryOptionsTest { verify(logger).log(eq(SentryLevel.WARNING), eq("showForm() can only be called in Android.")) } + @Test + fun `default shake controller logs a warning`() { + val logger = mock() + val options = + SentryOptions.empty().apply { + setLogger(logger) + isDebug = true + } + options.feedbackOptions.shakeController.enableOnShake() + verify(logger).log(eq(SentryLevel.WARNING), eq("Shake to report is only supported on Android.")) + } + @Test fun `autoTransactionDeadlineTimeoutMillis option defaults to 30000`() { val options = SentryOptions.empty()