re-index re-worked

Author: pdphilip

resources/stubs/ElasticModel.php.stub

@@ -3,7 +3,6 @@
 namespace {{ namespace }};
 
 use Carbon\Carbon;
-use PDPhilip\Elasticsearch\Eloquent\HasMappingDefinition;
 use PDPhilip\Elasticsearch\Eloquent\Model as Eloquent;
 use PDPhilip\Elasticsearch\Schema\Blueprint;
 
@@ -15,7 +14,7 @@ use PDPhilip\Elasticsearch\Schema\Blueprint;
  * @property Carbon $updated_at
  * @property Carbon $created_at
  */
-class {{ class }} extends Eloquent implements HasMappingDefinition
+class {{ class }} extends Eloquent
 {
     protected $connection = 'elasticsearch';
 

src/Commands/ReIndexCommand.php

@@ -10,7 +10,6 @@
 use Illuminate\Support\Facades\DB;
 use OmniTerm\HasOmniTerm;
 use PDPhilip\Elasticsearch\Connection;
-use PDPhilip\Elasticsearch\Eloquent\HasMappingDefinition;
 use PDPhilip\Elasticsearch\Eloquent\Model;
 use PDPhilip\Elasticsearch\Schema\Blueprint;
 use PDPhilip\Elasticsearch\Schema\Builder as SchemaBuilder;
@@ -174,21 +173,17 @@ private function resolveModel(): ?Model
             return null;
         }
 
-        if (! is_subclass_of($class, HasMappingDefinition::class)) {
+        if (! $class::hasMappingDefinition()) {
             $this->newLine();
             $this->omni->statusError('Missing mapping definition', $class, [
-                'Your model must implement HasMappingDefinition:',
+                'Your model must override mappingDefinition():',
                 '',
-                'use PDPhilip\Elasticsearch\Eloquent\HasMappingDefinition;',
                 'use PDPhilip\Elasticsearch\Schema\Blueprint;',
                 '',
-                'class YourModel extends Eloquent implements HasMappingDefinition',
+                'public static function mappingDefinition(Blueprint $index): void',
                 '{',
-                '    public static function mappingDefinition(Blueprint $index): void',
-                '    {',
-                '        $index->keyword(\'status\');',
-                '        $index->geoPoint(\'location\');',
-                '    }',
+                '    $index->keyword(\'status\');',
+                '    $index->geoPoint(\'location\');',
                 '}',
             ]);
             $this->newLine();
@@ -250,7 +245,6 @@ private function setupFromModel(Model $model): void
         $this->tempIndexName = $this->indexName.'_temp';
         $this->connectionName = $model->getConnectionName();
 
-        /** @var Connection $connection */
         $this->connection = DB::connection($this->connectionName);
         $this->schema = $this->connection->getSchemaBuilder();
 
@@ -603,14 +597,14 @@ private function summary(): void
     {
         $duration = round(microtime(true) - $this->startTime, 2);
 
-        $this->omni->hr();
-        $this->omni->titleBar('Re-Index Complete', 'emerald');
-        $this->omni->tableHeader('Metric', 'Value');
-        $this->omni->tableRow('Index', $this->indexName);
-        $this->omni->tableRow('Original count', number_format($this->originalCount));
-        $this->omni->tableRow('Final count', number_format($this->finalCount));
-        $this->omni->tableRow('Duration', $duration.'s');
-        $this->omni->hr();
+        $data = [
+            'Index' => $this->indexName,
+            'Original count' => number_format($this->originalCount),
+            'Final count' => number_format($this->finalCount),
+            'Duration' => $duration.'s',
+
+        ];
+        $this->omni->dataList($data, 'Re-Index Complete', 'text-emerald-500');
         $this->newLine();
     }
 
@@ -671,7 +665,7 @@ private function mappingMismatches(): array
 
         $blueprint = Helpers::getLaravelCompatabilityVersion() >= 12
             ? new Blueprint($this->connection, $this->indexName)
-            : new Blueprint($this->indexName);
+            : new Blueprint($this->indexName); // @phpstan-ignore arguments.count
         ($this->mappingDefinition)($blueprint);
 
         $mismatches = [];

src/Eloquent/ElasticsearchModel.php

@@ -130,7 +130,7 @@ protected function newBaseQueryBuilder()
             Model::OPTION_STORE_IDS => $this->storeIdInDocument,
         ]);
 
-        if ($this instanceof HasMappingDefinition) {
+        if (static::hasMappingDefinition()) {
             $connection->registerMappingDefinition(
                 $connection->getTablePrefix().$this->getTable(),
                 fn ($index) => static::mappingDefinition($index)

src/Eloquent/HasMappingDefinition.php

@@ -6,6 +6,9 @@
 
 use PDPhilip\Elasticsearch\Schema\Blueprint;
 
+/**
+ * @deprecated Override Model::mappingDefinition() directly instead.
+ */
 interface HasMappingDefinition
 {
     public static function mappingDefinition(Blueprint $index): void;

src/Eloquent/Model.php

@@ -6,6 +6,8 @@
 
 use Illuminate\Database\Eloquent\Model as BaseModel;
 use PDPhilip\Elasticsearch\Data\ModelMeta;
+use PDPhilip\Elasticsearch\Schema\Blueprint;
+use ReflectionMethod;
 
 /**
  * @property object $searchHighlights
@@ -82,4 +84,23 @@ final public static function isElasticsearchModel(string|object $class): bool
         // Document models must use the DocumentModel trait.
         return self::$documentModelClasses[$class] = array_key_exists(ElasticsearchModel::class, class_uses_recursive($class));
     }
+
+    /**
+     * Define the index mapping for this model.
+     * Override this method to specify field types for your Elasticsearch index.
+     */
+    public static function mappingDefinition(Blueprint $index): void
+    {
+        //
+    }
+
+    /**
+     * Check if this model has a mapping definition (i.e. overrides the base method).
+     */
+    final public static function hasMappingDefinition(): bool
+    {
+        $ref = new ReflectionMethod(static::class, 'mappingDefinition');
+
+        return $ref->getDeclaringClass()->getName() !== self::class;
+    }
 }

src/Query/Concerns/BuildsNestedQueries.php

@@ -28,9 +28,11 @@ public function whereNestedObject($column, $query, $filterInnerHits = false, $op
         }
         $options = $options->toArray();
 
-        $this->options()->add('parentField', $column);
         if (! is_string($query) && is_callable($query)) {
-            call_user_func($query, $query = $this->newQuery($from));
+            $callback = $query;
+            $query = $this->newQuery($from);
+            $query->options()->add('parentField', $column);
+            call_user_func($callback, $query);
         }
 
         $this->wheres[] = compact('column', 'query', 'type', 'boolean', 'not', 'options');
@@ -72,9 +74,11 @@ public function filterNested($column, $query, $options = [])
         $options = $this->setOptions($options, 'nested');
         $options = $options->toArray();
 
-        $this->options()->add('parentField', $column);
         if (! is_string($query) && is_callable($query)) {
-            call_user_func($query, $query = $this->newQuery($from));
+            $callback = $query;
+            $query = $this->newQuery($from);
+            $query->options()->add('parentField', $column);
+            call_user_func($callback, $query);
         }
 
         $this->wheres[] = compact('column', 'query', 'type', 'boolean', 'not', 'options');

tests/MakeCommandTest.php

@@ -68,7 +68,7 @@
 
     $content = File::get(app_path('Models/Event.php'));
     expect($content)
-        ->toContain('implements HasMappingDefinition')
+        ->not->toContain('implements HasMappingDefinition')
         ->toContain('public static function mappingDefinition(Blueprint $index): void')
         ->toContain("\$index->date('created_at');")
         ->toContain("\$index->date('updated_at');");

tests/Models/ReIndexTarget.php

@@ -4,12 +4,11 @@
 
 namespace PDPhilip\Elasticsearch\Tests\Models;
 
-use PDPhilip\Elasticsearch\Eloquent\HasMappingDefinition;
 use PDPhilip\Elasticsearch\Eloquent\Model;
 use PDPhilip\Elasticsearch\Schema\Blueprint;
 use PDPhilip\Elasticsearch\Tests\Concerns\TestsWithIdStrategies;
 
-class ReIndexTarget extends Model implements HasMappingDefinition
+class ReIndexTarget extends Model
 {
     use TestsWithIdStrategies;
 

tests/QueryElasticsearchSpecificTest.php

@@ -128,7 +128,14 @@
 });
 
 it('counts nested queries', function () {
-    $count = Post::where('status',2)->whereNestedObject('comments', function (Builder $query) {
+    // Only "Getting Started with Laravel" has status=2 AND a USA comment
+    $count = Post::where('status', 2)->whereNestedObject('comments', function (Builder $query) {
+        $query->where('country', 'USA');
+    })->count();
+    expect($count)->toBe(1);
+
+    // Both USA-comment posts (status 1 and 2)
+    $count = Post::whereNestedObject('comments', function (Builder $query) {
         $query->where('country', 'USA');
     })->count();
     expect($count)->toBe(2);

tests/ReIndexCommandTest.php

@@ -55,7 +55,7 @@
     ])->assertFailed();
 });
 
-it('fails when model lacks HasMappingDefinition', function () {
+it('fails when model lacks mappingDefinition override', function () {
     $this->artisan('elastic:re-index', [
         'model' => \PDPhilip\Elasticsearch\Tests\Models\Product::class,
         '--force' => true,