refactoring breadcrumb callback (#239)

sentry-core/src/main/java/io/sentry/core/Hub.java

@@ -5,9 +5,7 @@
 import io.sentry.core.util.Objects;
 import java.io.Closeable;
 import java.util.Deque;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.concurrent.LinkedBlockingDeque;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -62,7 +60,7 @@ private static void validateOptions(@NotNull SentryOptions options) {
 
   private static StackItem createRootStackItem(@NotNull SentryOptions options) {
     validateOptions(options);
-    Scope scope = new Scope(options.getMaxBreadcrumbs(), options.getBeforeBreadcrumb());
+    Scope scope = new Scope(options);
     ISentryClient client = new SentryClient(options);
     return new StackItem(client, scope);
   }
@@ -198,15 +196,7 @@ public void addBreadcrumb(@NotNull Breadcrumb breadcrumb, @Nullable Object hint)
     } else {
       StackItem item = stack.peek();
       if (item != null) {
-        SentryOptions.BeforeBreadcrumbCallback callback = options.getBeforeBreadcrumb();
-        if (callback != null) {
-          breadcrumb = executeBeforeBreadcrumb(callback, breadcrumb, hint);
-        }
-        if (breadcrumb != null) {
-          item.scope.addBreadcrumb(breadcrumb, false);
-        } else {
-          options.getLogger().log(SentryLevel.INFO, "Breadcrumb was dropped by beforeBreadcrumb");
-        }
+        item.scope.addBreadcrumb(breadcrumb, hint);
       } else {
         options.getLogger().log(SentryLevel.FATAL, "Stack peek was null when addBreadcrumb");
       }
@@ -337,30 +327,6 @@ public void setExtra(@NotNull String key, @NotNull String value) {
     }
   }
 
-  private @Nullable Breadcrumb executeBeforeBreadcrumb(
-      @NotNull SentryOptions.BeforeBreadcrumbCallback callback,
-      @NotNull Breadcrumb breadcrumb,
-      @Nullable Object hint) {
-    try {
-      breadcrumb = callback.execute(breadcrumb, hint);
-    } catch (Exception e) {
-      options
-          .getLogger()
-          .log(
-              SentryLevel.ERROR,
-              "The BeforeBreadcrumbCallback callback threw an exception. It will be added as breadcrumb and continue.",
-              e);
-
-      Map<String, String> data = breadcrumb.getData();
-      if (breadcrumb.getData() == null) {
-        data = new HashMap<>();
-      }
-      data.put("sentry:message", e.getMessage());
-      breadcrumb.setData(data);
-    }
-    return breadcrumb;
-  }
-
   @Override
   public @NotNull SentryId getLastEventId() {
     return lastEventId;
@@ -511,7 +477,7 @@ public void flush(long timeoutMills) {
       } catch (CloneNotSupportedException e) {
         // TODO: Why do we need to deal with this? We must guarantee clone is possible here!
         options.getLogger().log(SentryLevel.ERROR, "Clone not supported");
-        clonedScope = new Scope(options.getMaxBreadcrumbs(), options.getBeforeBreadcrumb());
+        clonedScope = new Scope(options);
       }
       StackItem cloneItem = new StackItem(item.client, clonedScope);
       clone.stack.push(cloneItem);

sentry-core/src/main/java/io/sentry/core/Scope.java

@@ -19,20 +19,12 @@ public final class Scope implements Cloneable {
   private @NotNull Queue<Breadcrumb> breadcrumbs;
   private @NotNull Map<String, String> tags = new ConcurrentHashMap<>();
   private @NotNull Map<String, Object> extra = new ConcurrentHashMap<>();
-  private final int maxBreadcrumb;
-  private @Nullable final SentryOptions.BeforeBreadcrumbCallback beforeBreadcrumbCallback;
   private @NotNull List<EventProcessor> eventProcessors = new ArrayList<>();
+  private final @NotNull SentryOptions options;
 
-  public Scope(
-      int maxBreadcrumb,
-      @Nullable final SentryOptions.BeforeBreadcrumbCallback beforeBreadcrumbCallback) {
-    this.maxBreadcrumb = maxBreadcrumb;
-    this.beforeBreadcrumbCallback = beforeBreadcrumbCallback;
-    this.breadcrumbs = createBreadcrumbsList(this.maxBreadcrumb);
-  }
-
-  public Scope(int maxBreadcrumb) {
-    this(maxBreadcrumb, null);
+  public Scope(final @NotNull SentryOptions options) {
+    this.options = options;
+    this.breadcrumbs = createBreadcrumbsList(options.getMaxBreadcrumbs());
   }
 
   public @Nullable SentryLevel getLevel() {
@@ -73,34 +65,48 @@ Queue<Breadcrumb> getBreadcrumbs() {
     return breadcrumbs;
   }
 
-  public void addBreadcrumb(@NotNull Breadcrumb breadcrumb) {
-    addBreadcrumb(breadcrumb, true);
+  private @Nullable Breadcrumb executeBeforeBreadcrumb(
+      final @NotNull SentryOptions.BeforeBreadcrumbCallback callback,
+      @NotNull Breadcrumb breadcrumb,
+      final @Nullable Object hint) {
+    try {
+      breadcrumb = callback.execute(breadcrumb, hint);
+    } catch (Exception e) {
+      options
+          .getLogger()
+          .log(
+              SentryLevel.ERROR,
+              "The BeforeBreadcrumbCallback callback threw an exception. It will be added as breadcrumb and continue.",
+              e);
+
+      Map<String, String> data = breadcrumb.getData();
+      if (breadcrumb.getData() == null) {
+        data = new HashMap<>();
+      }
+      data.put("sentry:message", e.getMessage());
+      breadcrumb.setData(data);
+    }
+    return breadcrumb;
   }
 
-  void addBreadcrumb(@NotNull Breadcrumb breadcrumb, boolean executeBeforeBreadcrumb) {
-    if (executeBeforeBreadcrumb && beforeBreadcrumbCallback != null) {
-      try {
-        breadcrumb =
-            beforeBreadcrumbCallback.execute(breadcrumb, null); // TODO: whats about hint here?
-      } catch (Exception e) {
-        // TODO: log it
-
-        Map<String, String> data = breadcrumb.getData();
-        if (breadcrumb.getData() == null) {
-          data = new HashMap<>();
-        }
-        data.put("sentry:message", e.getMessage());
-        breadcrumb.setData(data);
-      }
+  public void addBreadcrumb(@NotNull Breadcrumb breadcrumb, final @Nullable Object hint) {
+    if (breadcrumb == null) {
+      return;
+    }
 
-      if (breadcrumb == null) {
-        //        options.getLogger().log(SentryLevel.INFO, "Breadcrumb was dropped by scope
-        // beforeBreadcrumb");
-        return;
-      }
+    SentryOptions.BeforeBreadcrumbCallback callback = options.getBeforeBreadcrumb();
+    if (callback != null) {
+      breadcrumb = executeBeforeBreadcrumb(callback, breadcrumb, hint);
     }
+    if (breadcrumb != null) {
+      this.breadcrumbs.add(breadcrumb);
+    } else {
+      options.getLogger().log(SentryLevel.INFO, "Breadcrumb was dropped by beforeBreadcrumb");
+    }
+  }
 
-    this.breadcrumbs.add(breadcrumb);
+  public void addBreadcrumb(@NotNull Breadcrumb breadcrumb) {
+    addBreadcrumb(breadcrumb, null);
   }
 
   public void clearBreadcrumbs() {
@@ -157,7 +163,7 @@ public Scope clone() throws CloneNotSupportedException {
     clone.eventProcessors = eventProcessors != null ? new ArrayList<>(eventProcessors) : null;
 
     if (breadcrumbs != null) {
-      Queue<Breadcrumb> breadcrumbsClone = createBreadcrumbsList(maxBreadcrumb);
+      Queue<Breadcrumb> breadcrumbsClone = createBreadcrumbsList(options.getMaxBreadcrumbs());
 
       for (Breadcrumb item : breadcrumbs) {
         Breadcrumb breadcrumbClone = item.clone();

sentry-core/src/test/java/io/sentry/core/ScopeTest.kt

@@ -10,7 +10,7 @@ import kotlin.test.assertNotSame
 class ScopeTest {
     @Test
     fun `cloning scope wont have the same references`() {
-        val scope = Scope(1)
+        val scope = Scope(SentryOptions())
         val level = SentryLevel.DEBUG
         scope.level = level
 
@@ -60,7 +60,7 @@ class ScopeTest {
 
     @Test
     fun `cloning scope will have the same values`() {
-        val scope = Scope(1)
+        val scope = Scope(SentryOptions())
         val level = SentryLevel.DEBUG
         scope.level = level
 
@@ -97,7 +97,7 @@ class ScopeTest {
 
     @Test
     fun `cloning scope and changing the original values wont change the clone values`() {
-        val scope = Scope(1)
+        val scope = Scope(SentryOptions())
         val level = SentryLevel.DEBUG
         scope.level = level
 
@@ -164,7 +164,7 @@ class ScopeTest {
             setBeforeBreadcrumb { breadcrumb, _ -> breadcrumb }
         }
 
-        val scope = Scope(1, options.beforeBreadcrumb)
+        val scope = Scope(options)
         scope.addBreadcrumb(Breadcrumb())
         assertEquals(1, scope.breadcrumbs.count())
     }
@@ -175,7 +175,7 @@ class ScopeTest {
             setBeforeBreadcrumb { _, _ -> null }
         }
 
-        val scope = Scope(1, options.beforeBreadcrumb)
+        val scope = Scope(options)
         scope.addBreadcrumb(Breadcrumb())
         assertEquals(0, scope.breadcrumbs.count())
     }
@@ -188,7 +188,7 @@ class ScopeTest {
             setBeforeBreadcrumb { _, _ -> throw exception }
         }
 
-        val scope = Scope(1, options.beforeBreadcrumb)
+        val scope = Scope(options)
         val actual = Breadcrumb()
         scope.addBreadcrumb(actual)
 
@@ -199,15 +199,15 @@ class ScopeTest {
     fun `when adding breadcrumb, executeBreadcrumb wont be executed as its not set, but it will be added`() {
         val options = SentryOptions()
 
-        val scope = Scope(1, options.beforeBreadcrumb)
+        val scope = Scope(options)
         scope.addBreadcrumb(Breadcrumb())
         assertEquals(1, scope.breadcrumbs.count())
     }
 
     @Test
     fun `when adding eventProcessor, eventProcessor should be in the list`() {
         val processor = CustomEventProcessor()
-        val scope = Scope(1)
+        val scope = Scope(SentryOptions())
         scope.addEventProcessor(processor)
         assertEquals(processor, scope.eventProcessors.first())
     }

sentry-core/src/test/java/io/sentry/core/SentryClientTest.kt

@@ -300,7 +300,7 @@ class SentryClientTest {
         val sut = fixture.getSut()
 
         val event = SentryEvent()
-        val scope = Scope(10)
+        val scope = Scope(SentryOptions())
         scope.level = SentryLevel.FATAL
         sut.captureEvent(event, scope, mock<Cached>())
 
@@ -312,7 +312,7 @@ class SentryClientTest {
         val sut = fixture.getSut()
 
         val event = SentryEvent()
-        val scope = Scope(10)
+        val scope = Scope(SentryOptions())
         scope.level = SentryLevel.FATAL
         sut.captureEvent(event, scope, Object())
 
@@ -391,7 +391,7 @@ class SentryClientTest {
     }
 
     private fun createScope(): Scope {
-        return Scope(fixture.sentryOptions.maxBreadcrumbs).apply {
+        return Scope(SentryOptions()).apply {
             addBreadcrumb(Breadcrumb().apply {
                 message = "message"
             })

sentry-native-sample/build.gradle.kts

@@ -70,7 +70,7 @@ android {
 dependencies {
     implementation(fileTree(mapOf("dir" to "libs", "include" to listOf("*.jar"))))
 
-    implementation("io.sentry:sentry-android:2.0.0-beta02")
+    implementation("io.sentry:sentry-android:2.0.0-rc01")
 
     implementation(Config.Libs.appCompat)
 

sentry-sample/src/main/java/io/sentry/sample/MainActivity.java

@@ -2,7 +2,6 @@
 
 import android.os.Bundle;
 import androidx.appcompat.app.AppCompatActivity;
-import io.sentry.core.Breadcrumb;
 import io.sentry.core.Sentry;
 import io.sentry.core.SentryLevel;
 import io.sentry.core.protocol.User;
@@ -25,35 +24,26 @@ protected void onCreate(Bundle savedInstanceState) {
             });
 
     findViewById(R.id.send_message)
-        .setOnClickListener(
-            view -> {
-              Sentry.captureMessage("Some message.");
-            });
+        .setOnClickListener(view -> Sentry.captureMessage("Some message."));
 
     findViewById(R.id.capture_exception)
         .setOnClickListener(
-            view -> {
-              Sentry.captureException(
-                  new Exception(new Exception(new Exception("Some exception."))));
-            });
+            view ->
+                Sentry.captureException(
+                    new Exception(new Exception(new Exception("Some exception.")))));
 
     findViewById(R.id.breadcrumb)
         .setOnClickListener(
             view -> {
-              Sentry.configureScope(
-                  scope -> {
-                    Breadcrumb breadcrumb = new Breadcrumb();
-                    breadcrumb.setMessage("Breadcrumb");
-                    scope.addBreadcrumb(breadcrumb);
-                    scope.setExtra("extra", "extra");
-                    scope.setFingerprint(Collections.singletonList("fingerprint"));
-                    scope.setLevel(SentryLevel.INFO);
-                    scope.setTransaction("transaction");
-                    User user = new User();
-                    user.setUsername("username");
-                    scope.setUser(user);
-                    scope.setTag("tag", "tag");
-                  });
+              Sentry.addBreadcrumb("Breadcrumb");
+              Sentry.setExtra("extra", "extra");
+              Sentry.setFingerprint(Collections.singletonList("fingerprint"));
+              Sentry.setLevel(SentryLevel.INFO);
+              Sentry.setTransaction("transaction");
+              User user = new User();
+              user.setUsername("username");
+              Sentry.setUser(user);
+              Sentry.setTag("tag", "tag");
               Sentry.captureException(new Exception("Some exception with scope."));
             });