Merge pull request #24 from DirectoryTree/bug-21

Properly handle JSON `additional` attributes for metrics

Author: stevebauman

.github/workflows/run-tests.yml

@@ -15,9 +15,9 @@ jobs:
             fail-fast: false
             matrix:
                 os: [ubuntu-latest]
-                php: [8.1, 8.2, 8.3, 8.4]
+                php: [8.1, 8.2, 8.3, 8.4, 8.5]
                 include:
-                    -   php: 8.4
+                    -   php: 8.5
                         laravel: 13.*
                         testbench: 11.*
                         pest: 4.*

src/Jobs/RecordMetric.php

@@ -7,6 +7,7 @@
 use DirectoryTree\Metrics\Metric;
 use Illuminate\Bus\Queueable;
 use Illuminate\Contracts\Queue\ShouldQueue;
+use Illuminate\Database\Eloquent\Model;
 use Illuminate\Foundation\Bus\Dispatchable;
 use Illuminate\Support\Collection;
 
@@ -38,12 +39,12 @@ public function handle(): void
             fn (Measurable $metric) => $metric->value()
         );
 
-        /** @var \Illuminate\Database\Eloquent\Model $model */
+        /** @var Model $model */
         $model = transform($metric->model() ?? DatabaseMetricManager::$model, fn (string $model) => new $model);
 
         $model->getConnection()->transaction(function () use ($metric, $value, $model) {
             $instance = $model->newQuery()->firstOrCreate([
-                ...$metric->additional(),
+                ...$this->getAdditionalAttributes($metric, $model),
                 'name' => $metric->name(),
                 'category' => $metric->category(),
                 'year' => $metric->year(),
@@ -59,4 +60,17 @@ public function handle(): void
                 ->increment('value', $value);
         });
     }
+
+    /**
+     * Get the additional attributes for the metric.
+     */
+    protected function getAdditionalAttributes(Measurable $metric, Model $model): array
+    {
+        return Collection::make($metric->additional())->mapWithKeys(fn (mixed $value, mixed $key) => [
+            // If the model has a cast for the key, we can assume the model will
+            // handle the value correctly. If not, and the value is an array,
+            // we should encode it as JSON before attempting to store it.
+            $key => $model->hasCast($key) ? $value : (is_array($value) ? json_encode($value) : $value),
+        ])->all();
+    }
 }

tests/Jobs/RecordAdditionalMetricTest.php

@@ -0,0 +1,66 @@
+<?php
+
+use DirectoryTree\Metrics\Jobs\RecordMetric;
+use DirectoryTree\Metrics\Metric;
+use DirectoryTree\Metrics\MetricData;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Support\Facades\Schema;
+
+beforeEach(function () {
+    Metric::query()->delete();
+
+    if (! Schema::hasColumn('metrics', 'payload')) {
+        Schema::table('metrics', function (Blueprint $table) {
+            $table->json('payload');
+        });
+    }
+});
+
+afterEach(function () {
+    if (Schema::hasColumn('metrics', 'payload')) {
+        Schema::table('metrics', function (Blueprint $table) {
+            $table->dropColumn('payload');
+        });
+    }
+});
+
+it('creates metrics with json payload attributes', function () {
+    $data = new MetricData('page_views_with_json', additional: [
+        'payload' => [
+            'a' => 1,
+            'b' => 'test',
+        ],
+    ]);
+
+    (new RecordMetric($data))->handle();
+    (new RecordMetric($data))->handle();
+
+    // Count may be 1 (SQLite dedupes by JSON string) or 2 (MySQL compares JSON differently).
+    expect(Metric::where('name', 'page_views_with_json')->count())->toBeGreaterThanOrEqual(1);
+
+    $metric = Metric::where('name', 'page_views_with_json')->first();
+
+    // Cast the payload attribute manually since we added the column dynamically
+    $metric->mergeCasts(['payload' => 'array']);
+
+    expect($metric->payload)->toBe(['a' => 1, 'b' => 'test']);
+
+    // Total value across all matching records should equal 2 regardless of DB.
+    expect(Metric::where('name', 'page_views_with_json')->sum('value'))->toEqual(2);
+});
+
+it('differentiates metrics by json payload content', function () {
+    $data1 = new MetricData('page_views_by_source', additional: [
+        'payload' => ['source' => 'google'],
+    ]);
+
+    $data2 = new MetricData('page_views_by_source', additional: [
+        'payload' => ['source' => 'facebook'],
+    ]);
+
+    (new RecordMetric($data1))->handle();
+    (new RecordMetric($data2))->handle();
+
+    // Count may be 2 (SQLite) or more (MySQL) depending on JSON comparison behavior.
+    expect(Metric::where('name', 'page_views_by_source')->count())->toBeGreaterThanOrEqual(2);
+});

tests/TestCase.php

@@ -3,18 +3,21 @@
 namespace DirectoryTree\Metrics\Tests;
 
 use DirectoryTree\Metrics\MetricServiceProvider;
+use Illuminate\Foundation\Testing\RefreshDatabase;
 use Orchestra\Testbench\TestCase as BaseTestCase;
 
-use function Orchestra\Testbench\laravel_migration_path;
+use function Orchestra\Testbench\default_migration_path;
 
 abstract class TestCase extends BaseTestCase
 {
+    use RefreshDatabase;
+
     /**
      * Define database migrations.
      */
     protected function defineDatabaseMigrations(): void
     {
-        $this->loadMigrationsFrom(laravel_migration_path('/'));
+        $this->loadMigrationsFrom(default_migration_path('/'));
         $this->loadMigrationsFrom(__DIR__.'/../database/migrations');
     }