Merge branch 'feature/sensitive-attributes' into 9.x

hareku · Jul 7, 2021 · 60e8206 · 60e8206
2 parents 2ab9e6c + c680c74
commit 60e8206
Show file tree

Hide file tree

Showing 7 changed files with 273 additions and 10 deletions.
diff --git a/src/ModelObserver.php b/src/ModelObserver.php
@@ -2,6 +2,7 @@
 
 namespace Laravel\Scout;
 
+use Closure;
 use Illuminate\Database\Eloquent\SoftDeletes;
 use Illuminate\Support\Facades\Config;
 
@@ -14,6 +15,20 @@ class ModelObserver
      */
     public $afterCommit;
 
+    /**
+     * Indicates if Scout will keep soft deleted records in the search indexes.
+     *
+     * @var bool
+     */
+    protected $usingSoftDeletes;
+
+    /**
+     * Indicates if the model is currently force saving.
+     *
+     * @var bool
+     */
+    protected $forceSaving = false;
+
     /**
      * The class names that syncing is disabled for.
      *
@@ -29,6 +44,7 @@ class ModelObserver
     public function __construct()
     {
         $this->afterCommit = Config::get('scout.after_commit', false);
+        $this->usingSoftDeletes = Config::get('scout.soft_delete', false);
     }
 
     /**
@@ -78,6 +94,10 @@ public function saved($model)
             return;
         }
 
+        if (! $this->forceSaving && ! $model->searchIndexShouldBeUpdated()) {
+            return;
+        }
+
         if (! $model->shouldBeSearchable()) {
             if ($model->wasSearchableBeforeUpdate()) {
                 $model->unsearchable();
@@ -105,8 +125,10 @@ public function deleted($model)
             return;
         }
 
-        if ($this->usesSoftDelete($model) && config('scout.soft_delete', false)) {
-            $this->saved($model);
+        if ($this->usingSoftDeletes && $this->usesSoftDelete($model)) {
+            $this->whileForcingUpdate(function () use ($model) {
+                $this->saved($model);
+            });
         } else {
             $model->unsearchable();
         }
@@ -135,7 +157,26 @@ public function forceDeleted($model)
      */
     public function restored($model)
     {
-        $this->saved($model);
+        $this->whileForcingUpdate(function () use ($model) {
+            $this->saved($model);
+        });
+    }
+
+    /**
+     * Execute the given callback while forcing updates.
+     *
+     * @param  \Closure  $callback
+     * @return mixed
+     */
+    protected function whileForcingUpdate(Closure $callback)
+    {
+        $this->forceSaving = true;
+
+        $result = $callback();
+
+        $this->forceSaving = false;
+
+        return $result;
     }
 
     /**

diff --git a/src/Searchable.php b/src/Searchable.php
@@ -98,6 +98,16 @@ public function shouldBeSearchable()
         return true;
     }
 
+    /**
+     * When updating a model, this method determines if we should update the search index.
+     *
+     * @return bool
+     */
+    public function searchIndexShouldBeUpdated()
+    {
+        return true;
+    }
+
     /**
      * Perform a search against the model's indexed data.
      *

diff --git a/tests/Fixtures/SearchableModelWithSensitiveAttributes.php b/tests/Fixtures/SearchableModelWithSensitiveAttributes.php
@@ -0,0 +1,35 @@
+<?php
+
+namespace Laravel\Scout\Tests\Fixtures;
+
+use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\SoftDeletes;
+use Laravel\Scout\Searchable;
+
+class SearchableModelWithSensitiveAttributes extends Model
+{
+    use Searchable;
+    use SoftDeletes;
+
+    /**
+     * The attributes that are mass assignable.
+     *
+     * @var array
+     */
+    protected $fillable = ['first_name', 'last_name', 'remember_token', 'password'];
+
+    /**
+     * When updating a model, this method determines if we
+     * should perform a search engine update or not.
+     *
+     * @return bool
+     */
+    public function searchShouldUpdate(): bool
+    {
+        $sensitiveAttributeKeys = ['first_name', 'last_name'];
+
+        return collect($this->getDirty())->keys()
+            ->intersect($sensitiveAttributeKeys)
+            ->isNotEmpty();
+    }
+}
diff --git a/tests/Unit/AlgoliaEngineTest.php b/tests/Unit/AlgoliaEngineTest.php
@@ -20,6 +20,7 @@ class AlgoliaEngineTest extends TestCase
     protected function setUp(): void
     {
         Config::shouldReceive('get')->with('scout.after_commit', m::any())->andReturn(false);
+        Config::shouldReceive('get')->with('scout.soft_delete', m::any())->andReturn(false);
     }
 
     protected function tearDown(): void

diff --git a/tests/Unit/ModelObserverTest.php b/tests/Unit/ModelObserverTest.php
@@ -5,14 +5,18 @@
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Support\Facades\Config;
 use Laravel\Scout\ModelObserver;
+use Laravel\Scout\Tests\Fixtures\SearchableModelWithSensitiveAttributes;
+use Laravel\Scout\Tests\Fixtures\SearchableModelWithSoftDeletes;
 use Mockery as m;
 use PHPUnit\Framework\TestCase;
 
 class ModelObserverTest extends TestCase
 {
     protected function setUp(): void
     {
+        Config::clearResolvedInstances();
         Config::shouldReceive('get')->with('scout.after_commit', m::any())->andReturn(false);
+        Config::shouldReceive('get')->with('scout.soft_delete', m::any())->andReturn(false);
     }
 
     protected function tearDown(): void
@@ -24,11 +28,22 @@ public function test_saved_handler_makes_model_searchable()
     {
         $observer = new ModelObserver;
         $model = m::mock();
+        $model->shouldReceive('searchShouldUpdate')->andReturn(true);
         $model->shouldReceive('shouldBeSearchable')->andReturn(true);
         $model->shouldReceive('searchable')->once();
         $observer->saved($model);
     }
 
+    public function test_saved_handler_doesnt_make_model_searchable_when_search_shouldnt_update()
+    {
+        $observer = new ModelObserver;
+        $model = m::mock();
+        $model->shouldReceive('searchShouldUpdate')->andReturn(false);
+        $model->shouldReceive('shouldBeSearchable')->andReturn(true);
+        $model->shouldReceive('searchable')->never();
+        $observer->saved($model);
+    }
+
     public function test_saved_handler_doesnt_make_model_searchable_when_disabled()
     {
         $observer = new ModelObserver;
@@ -43,6 +58,7 @@ public function test_saved_handler_makes_model_unsearchable_when_disabled_per_mo
     {
         $observer = new ModelObserver;
         $model = m::mock();
+        $model->shouldReceive('searchShouldUpdate')->andReturn(true);
         $model->shouldReceive('shouldBeSearchable')->andReturn(false);
         $model->shouldReceive('wasSearchableBeforeUpdate')->andReturn(true);
         $model->shouldReceive('searchable')->never();
@@ -53,7 +69,8 @@ public function test_saved_handler_makes_model_unsearchable_when_disabled_per_mo
     public function test_saved_handler_doesnt_make_model_unsearchable_when_disabled_per_model_rule_and_already_unsearchable()
     {
         $observer = new ModelObserver;
-        $model = m::mock(Model::class)->makePartial();
+        $model = m::mock(Model::class);
+        $model->shouldReceive('searchShouldUpdate')->andReturn(true);
         $model->shouldReceive('shouldBeSearchable')->andReturn(false);
         $model->shouldReceive('wasSearchableBeforeUpdate')->andReturn(false);
         $model->shouldReceive('searchable')->never();
@@ -75,16 +92,91 @@ public function test_deleted_handler_makes_model_unsearchable()
         $observer = new ModelObserver;
         $model = m::mock();
         $model->shouldReceive('wasSearchableBeforeDelete')->andReturn(true);
-        $model->shouldReceive('unsearchable');
+        $model->shouldReceive('unsearchable')->once();
         $observer->deleted($model);
     }
 
-    public function test_restored_handler_makes_model_searchable()
+    public function test_deleted_handler_on_soft_delete_model_makes_model_unsearchable()
     {
         $observer = new ModelObserver;
-        $model = m::mock();
-        $model->shouldReceive('shouldBeSearchable')->andReturn(true);
-        $model->shouldReceive('searchable');
-        $observer->restored($model);
+        $model = m::mock(SearchableModelWithSoftDeletes::class);
+        $model->shouldReceive('wasSearchableBeforeDelete')->andReturn(true);
+        $model->shouldReceive('searchable')->never();
+        $model->shouldReceive('unsearchable')->once();
+        $observer->deleted($model);
+    }
+
+    public function test_update_on_sensitive_attributes_triggers_search()
+    {
+        $model = m::mock(
+            new SearchableModelWithSensitiveAttributes([
+                'first_name' => 'taylor',
+                'last_name' => 'Otwell',
+                'remember_token' => 123,
+                'password' => 'secret',
+            ])
+        )->makePartial();
+
+        // Let's pretend it's in sync with the database.
+        $model->syncOriginal();
+
+        // Update
+        $model->password = 'extremelySecurePassword';
+        $model->first_name = 'Taylor';
+
+        // Assertions
+        $model->shouldReceive('searchable')->once();
+        $model->shouldReceive('unsearchable')->never();
+
+        $observer = new ModelObserver;
+        $observer->saved($model);
+    }
+
+    public function test_update_on_non_sensitive_attributes_doesnt_trigger_search()
+    {
+        $model = m::mock(
+            new SearchableModelWithSensitiveAttributes([
+                'first_name' => 'taylor',
+                'last_name' => 'Otwell',
+                'remember_token' => 123,
+                'password' => 'secret',
+            ])
+        )->makePartial();
+
+        // Let's pretend it's in sync with the database.
+        $model->syncOriginal();
+
+        // Update
+        $model->password = 'extremelySecurePassword';
+        $model->remember_token = 456;
+
+        // Assertions
+        $model->shouldReceive('searchable')->never();
+        $model->shouldReceive('unsearchable')->never();
+
+        $observer = new ModelObserver;
+        $observer->saved($model);
+    }
+
+    public function test_unsearchable_should_be_called_when_deleting()
+    {
+        $model = m::mock(
+            new SearchableModelWithSensitiveAttributes([
+                'first_name' => 'taylor',
+                'last_name' => 'Otwell',
+                'remember_token' => 123,
+                'password' => 'secret',
+            ])
+        )->makePartial();
+
+        // Let's pretend it's in sync with the database.
+        $model->syncOriginal();
+
+        // Assertions
+        $model->shouldReceive('searchable')->never();
+        $model->shouldReceive('unsearchable')->once();
+
+        $observer = new ModelObserver;
+        $observer->deleted($model);
     }
 }
diff --git a/tests/Unit/ModelObserverWithSoftDeletesTest.php b/tests/Unit/ModelObserverWithSoftDeletesTest.php
@@ -0,0 +1,83 @@
+<?php
+
+namespace Laravel\Scout\Tests\Unit;
+
+use Illuminate\Support\Facades\Config;
+use Laravel\Scout\ModelObserver;
+use Laravel\Scout\Tests\Fixtures\SearchableModelWithSensitiveAttributes;
+use Laravel\Scout\Tests\Fixtures\SearchableModelWithSoftDeletes;
+use Mockery as m;
+use PHPUnit\Framework\TestCase;
+
+class ModelObserverWithSoftDeletesTest extends TestCase
+{
+    protected function setUp(): void
+    {
+        Config::clearResolvedInstances();
+        Config::shouldReceive('get')->with('scout.after_commit', m::any())->andReturn(false);
+        Config::shouldReceive('get')->with('scout.soft_delete', m::any())->andReturn(true);
+    }
+
+    protected function tearDown(): void
+    {
+        m::close();
+    }
+
+    public function test_deleted_handler_makes_model_unsearchable_when_it_should_not_be_searchable()
+    {
+        $observer = new ModelObserver;
+        $model = m::mock(SearchableModelWithSoftDeletes::class);
+        $model->shouldReceive('searchShouldUpdate')->never(); // The saved event is forced
+        $model->shouldReceive('shouldBeSearchable')->andReturn(false); // Should not be searchable
+        $model->shouldReceive('wasSearchableBeforeDelete')->andReturn(true);
+        $model->shouldReceive('wasSearchableBeforeUpdate')->andReturn(true);
+        $model->shouldReceive('searchable')->never();
+        $model->shouldReceive('unsearchable')->once();
+        $observer->deleted($model);
+    }
+
+    public function test_deleted_handler_makes_model_searchable_when_it_should_be_searchable()
+    {
+        $observer = new ModelObserver;
+        $model = m::mock(SearchableModelWithSoftDeletes::class);
+        $model->shouldReceive('searchShouldUpdate')->never(); // The saved event is forced
+        $model->shouldReceive('shouldBeSearchable')->andReturn(true); // Should be searchable
+        $model->shouldReceive('wasSearchableBeforeDelete')->andReturn(true);
+        $model->shouldReceive('searchable')->once();
+        $model->shouldReceive('unsearchable')->never();
+        $observer->deleted($model);
+    }
+
+    public function test_restored_handler_makes_model_searchable()
+    {
+        $observer = new ModelObserver;
+        $model = m::mock(SearchableModelWithSoftDeletes::class);
+        $model->shouldReceive('searchShouldUpdate')->never();
+        $model->shouldReceive('shouldBeSearchable')->andReturn(true);
+        $model->shouldReceive('searchable')->once();
+        $model->shouldReceive('unsearchable')->never();
+        $observer->restored($model);
+    }
+
+    public function test_unsearchable_should_be_called_when_deleting()
+    {
+        $model = m::mock(
+            new SearchableModelWithSensitiveAttributes([
+                'first_name' => 'taylor',
+                'last_name' => 'Otwell',
+                'remember_token' => 123,
+                'password' => 'secret',
+            ])
+        )->makePartial();
+
+        // Let's pretend it's in sync with the database.
+        $model->syncOriginal();
+
+        // Assertions
+        $model->shouldReceive('searchable')->once();
+        $model->shouldReceive('unsearchable')->never();
+
+        $observer = new ModelObserver;
+        $observer->deleted($model);
+    }
+}
diff --git a/tests/Unit/RemoveFromSearchTest.php b/tests/Unit/RemoveFromSearchTest.php
@@ -16,6 +16,7 @@ class RemoveFromSearchTest extends TestCase
     protected function setUp(): void
     {
         Config::shouldReceive('get')->with('scout.after_commit', m::any())->andReturn(false);
+        Config::shouldReceive('get')->with('scout.soft_delete', m::any())->andReturn(false);
     }
 
     protected function tearDown(): void