Allow configuring the maximum number off breadcrumbs

CHANGELOG.md

@@ -1,6 +1,16 @@
 Changelog
 =========
 
+## TBD
+
+### Enhancements
+
+* The maximum number of breadcrumbs can now be configured between 0-100 (inclusive)
+  [#652](https://github.com/bugsnag/bugsnag-php/pull/652)
+
+* Raised the default maximum number of breadcrumbs to 50
+  [#652](https://github.com/bugsnag/bugsnag-php/pull/652)
+
 ## 3.28.0 (2022-05-18)
 
 ### Enhancements

src/Breadcrumbs/Recorder.php

@@ -15,72 +15,88 @@ class Recorder implements Countable, Iterator
      *
      * @var int
      */
-    const MAX_ITEMS = 25;
+    private $maxBreadcrumbs = 50;
 
     /**
      * The recorded breadcrumbs.
      *
      * @var \Bugsnag\Breadcrumbs\Breadcrumb[]
      */
-    protected $breadcrumbs = [];
+    private $breadcrumbs = [];
 
     /**
-     * The head position.
+     * The iteration position.
      *
      * @var int
      */
-    protected $head = 0;
+    private $position = 0;
 
     /**
-     * The pointer position.
+     * Record a breadcrumb.
      *
-     * @var int
+     * @param \Bugsnag\Breadcrumbs\Breadcrumb $breadcrumb
+     *
+     * @return void
      */
-    protected $pointer = 0;
+    public function record(Breadcrumb $breadcrumb)
+    {
+        $this->breadcrumbs[] = $breadcrumb;
+
+        // drop the oldest breadcrumb if we're over the max
+        if ($this->count() > $this->maxBreadcrumbs) {
+            array_shift($this->breadcrumbs);
+        }
+    }
 
     /**
-     * The iteration position.
+     * Clear all recorded breadcrumbs.
      *
-     * @var int
+     * @return void
      */
-    protected $position = 0;
+    public function clear()
+    {
+        $this->position = 0;
+        $this->breadcrumbs = [];
+    }
 
     /**
-     * Record a breadcrumb.
+     * Set the maximum number of breadcrumbs that are allowed to be stored.
      *
-     * We're recording a maximum of 25 breadcrumbs. Once we've recorded 25, we
-     * start wrapping back around and replacing the earlier ones. In order to
-     * indicate the start of the list, we advance a head pointer.
+     * This must be an integer between 0 and 100 (inclusive).
      *
-     * @param \Bugsnag\Breadcrumbs\Breadcrumb $breadcrumb
+     * @param int $maxBreadcrumbs
      *
      * @return void
      */
-    public function record(Breadcrumb $breadcrumb)
+    public function setMaxBreadcrumbs($maxBreadcrumbs)
     {
-        // advance the head by one if we've caught up
-        if ($this->breadcrumbs && $this->pointer === $this->head) {
-            $this->head = ($this->head + 1) % static::MAX_ITEMS;
+        if (!is_int($maxBreadcrumbs) || $maxBreadcrumbs < 0 || $maxBreadcrumbs > 100) {
+            error_log(
+                'Bugsnag Warning: maxBreadcrumbs should be an integer between 0 and 100 (inclusive)'
+            );
+
+            return;
         }
 
-        // record the new breadcrumb
-        $this->breadcrumbs[$this->pointer] = $breadcrumb;
+        $this->maxBreadcrumbs = $maxBreadcrumbs;
 
-        // advance the pointer so we set the next breadcrumb in the next slot
-        $this->pointer = ($this->pointer + 1) % static::MAX_ITEMS;
+        // drop the oldest breadcrumbs if we're over the max
+        if ($this->count() > $this->maxBreadcrumbs) {
+            $this->breadcrumbs = array_slice(
+                $this->breadcrumbs,
+                $this->count() - $this->maxBreadcrumbs
+            );
+        }
     }
 
     /**
-     * Clear all recorded breadcrumbs.
+     * Get the maximum number of breadcrumbs that are allowed to be stored.
      *
-     * @return void
+     * @return int
      */
-    public function clear()
+    public function getMaxBreadcrumbs()
     {
-        $this->head = 0;
-        $this->pointer = 0;
-        $this->position = 0;
-        $this->breadcrumbs = [];
+        return $this->maxBreadcrumbs;
     }
 
     /**
@@ -102,7 +118,7 @@ public function count()
     #[\ReturnTypeWillChange]
     public function current()
     {
-        return $this->breadcrumbs[($this->head + $this->position) % static::MAX_ITEMS];
+        return $this->breadcrumbs[$this->position];
     }
 
     /**

src/Client.php

@@ -1066,4 +1066,24 @@ public function getRedactedKeys()
     {
         return $this->config->getRedactedKeys();
     }
+
+    /**
+     * @param int $maxBreadcrumbs
+     *
+     * @return $this
+     */
+    public function setMaxBreadcrumbs($maxBreadcrumbs)
+    {
+        $this->recorder->setMaxBreadcrumbs($maxBreadcrumbs);
+
+        return $this;
+    }
+
+    /**
+     * @return int
+     */
+    public function getMaxBreadcrumbs()
+    {
+        return $this->recorder->getMaxBreadcrumbs();
+    }
 }

tests/Breadcrumbs/RecorderTest.php

@@ -5,17 +5,40 @@
 use Bugsnag\Breadcrumbs\Breadcrumb;
 use Bugsnag\Breadcrumbs\Recorder;
 use Bugsnag\Tests\TestCase;
+use Countable;
 use Iterator;
+use stdClass;
 
 class RecorderTest extends TestCase
 {
-    public function testIterable()
+    public function testItImplementsIteratorAndCountable()
     {
         $recorder = new Recorder();
 
         $this->assertInstanceOf(Iterator::class, $recorder);
+        $this->assertInstanceOf(Countable::class, $recorder);
+    }
 
-        $this->assertSame([], iterator_to_array($recorder));
+    public function testItCanBeIterated()
+    {
+        $breadcrumbs = [
+            new Breadcrumb('one', 'error'),
+            new Breadcrumb('two', 'user'),
+            new Breadcrumb('three', 'user'),
+            new Breadcrumb('four', 'user'),
+        ];
+
+        $recorder = new Recorder();
+        $recorder->record($breadcrumbs[0]);
+        $recorder->record($breadcrumbs[1]);
+        $recorder->record($breadcrumbs[2]);
+        $recorder->record($breadcrumbs[3]);
+
+        foreach ($recorder as $i => $breadcrumb) {
+            $this->assertSame($breadcrumbs[$i], $breadcrumb);
+        }
+
+        $this->assertSame($breadcrumbs, iterator_to_array($recorder));
     }
 
     public function testNoneRecorded()
@@ -68,6 +91,8 @@ public function testTwoRecorded()
     public function testManyRecorded()
     {
         $recorder = new Recorder();
+        $recorder->setMaxBreadcrumbs(25);
+
         $one = new Breadcrumb('Foo', 'error');
         $two = new Breadcrumb('Bar', 'user');
         $three = new Breadcrumb('Baz', 'request');
@@ -90,4 +115,122 @@ public function testManyRecorded()
         $this->assertCount(0, $recorder);
         $this->assertSame([], iterator_to_array($recorder));
     }
+
+    public function testItCanGrow()
+    {
+        $recorder = new Recorder();
+        $recorder->setMaxBreadcrumbs(2);
+
+        $one = new Breadcrumb('one', 'error');
+        $two = new Breadcrumb('two', 'user');
+        $three = new Breadcrumb('three', 'user');
+        $four = new Breadcrumb('four', 'user');
+
+        $recorder->record($one);
+        $recorder->record($two);
+        $recorder->record($three);
+        $recorder->record($four);
+
+        $this->assertSame([$three, $four], iterator_to_array($recorder));
+
+        $recorder->setMaxBreadcrumbs(4);
+
+        $recorder->record($one);
+        $recorder->record($two);
+
+        $this->assertSame([$three, $four, $one, $two], iterator_to_array($recorder));
+
+        $recorder->record($three);
+        $recorder->record($four);
+
+        $this->assertSame([$one, $two, $three, $four], iterator_to_array($recorder));
+    }
+
+    public function testItCanShrink()
+    {
+        $recorder = new Recorder();
+        $recorder->setMaxBreadcrumbs(4);
+
+        $one = new Breadcrumb('one', 'error');
+        $two = new Breadcrumb('two', 'user');
+
+        $recorder->record($one);
+        $recorder->record($two);
+        $recorder->record($one);
+        $recorder->record($two);
+
+        $this->assertSame([$one, $two, $one, $two], iterator_to_array($recorder));
+
+        $recorder->setMaxBreadcrumbs(2);
+
+        $this->assertSame([$one, $two], iterator_to_array($recorder));
+
+        $recorder->record($two);
+        $recorder->record($one);
+
+        $this->assertSame([$two, $one], iterator_to_array($recorder));
+
+        $recorder->setMaxBreadcrumbs(0);
+
+        $this->assertSame([], iterator_to_array($recorder));
+
+        $recorder->record($two);
+
+        $this->assertSame([], iterator_to_array($recorder));
+    }
+
+    public function testItDoesNotAllowNegativeMaxes()
+    {
+        $log = $this->getFunctionMock('Bugsnag\Breadcrumbs', 'error_log');
+        $log->expects($this->once())
+            ->with($this->equalTo(
+                'Bugsnag Warning: maxBreadcrumbs should be an integer between 0 and 100 (inclusive)'
+            ));
+
+        $recorder = new Recorder();
+
+        $previousMax = $recorder->getMaxBreadcrumbs();
+
+        $recorder->setMaxBreadcrumbs(-1);
+
+        $this->assertNotSame(-1, $recorder->getMaxBreadcrumbs());
+        $this->assertSame($previousMax, $recorder->getMaxBreadcrumbs());
+    }
+
+    public function testItDoesNotAllowMaxesGreaterThan100()
+    {
+        $log = $this->getFunctionMock('Bugsnag\Breadcrumbs', 'error_log');
+        $log->expects($this->once())
+            ->with($this->equalTo(
+                'Bugsnag Warning: maxBreadcrumbs should be an integer between 0 and 100 (inclusive)'
+            ));
+
+        $recorder = new Recorder();
+
+        $previousMax = $recorder->getMaxBreadcrumbs();
+        $recorder->setMaxBreadcrumbs(101);
+
+        $this->assertNotSame(101, $recorder->getMaxBreadcrumbs());
+        $this->assertSame($previousMax, $recorder->getMaxBreadcrumbs());
+    }
+
+    public function testItDoesNotAllowNonIntegerMaxes()
+    {
+        $log = $this->getFunctionMock('Bugsnag\Breadcrumbs', 'error_log');
+        $log->expects($this->exactly(4))
+            ->with($this->equalTo(
+                'Bugsnag Warning: maxBreadcrumbs should be an integer between 0 and 100 (inclusive)'
+            ));
+
+        $recorder = new Recorder();
+
+        $previousMax = $recorder->getMaxBreadcrumbs();
+
+        $recorder->setMaxBreadcrumbs(10.1);
+        $recorder->setMaxBreadcrumbs(new stdClass());
+        $recorder->setMaxBreadcrumbs([1, 2, 3]);
+        $recorder->setMaxBreadcrumbs(null);
+
+        $this->assertSame($previousMax, $recorder->getMaxBreadcrumbs());
+    }
 }

tests/Middleware/BreadcrumbsDataTest.php

@@ -111,6 +111,7 @@ public function testManyBreadcrumbs()
     {
         $breadcrumbs = null;
         $middleware = new BreadcrumbData($this->recorder);
+        $this->recorder->setMaxBreadcrumbs(25);
 
         $this->recorder->record(new Breadcrumb('Foo', 'error', ['foo' => 'bar']));