fixed CachedData and scraper service classes

app/Anime.php

@@ -133,6 +133,16 @@ class Anime extends JikanApiSearchableModel
         ];
     }
 
+    public function getThemesAttribute()
+    {
+        $result = [];
+        if (array_key_exists("themes", $this->attributes)) {
+            $result = $this->attributes["themes"];
+        }
+
+        return $result;
+    }
+
     /** @noinspection PhpUnused */
     public function filterByType(\Laravel\Scout\Builder|\Illuminate\Database\Eloquent\Builder $query, AnimeTypeEnum $value): \Laravel\Scout\Builder|\Illuminate\Database\Eloquent\Builder
     {

app/Contracts/Repository.php

@@ -20,7 +20,7 @@ interface Repository extends RepositoryQuery
     /**
      * @return ?T
      */
-    public function getByMalId(int $id);
+    public function getByMalId(int $id): JikanApiModel|array|null;
 
     public function getAllByMalId(int $id): Collection;
 

app/Macros/ResponseJikanCacheFlags.php

@@ -22,7 +22,7 @@ final class ResponseJikanCacheFlags
                 ->header("X-Request-Fingerprint", $cacheKey)
                 ->setTtl(app(CacheOptions::class)->ttl())
                 ->setExpires(Carbon::createFromTimestamp($scraperResults->expiry()))
-                ->setLastModified(Carbon::createFromTimestamp($scraperResults->lastModified()));
+                ->setLastModified(Carbon::createFromTimestamp($scraperResults->lastModified() ?? 0));
         };
     }
 }

app/Repositories/DatabaseRepository.php

@@ -3,6 +3,7 @@
 namespace App\Repositories;
 
 use App\Contracts\Repository;
+use App\JikanApiModel;
 use App\Support\RepositoryQuery;
 use Illuminate\Contracts\Database\Query\Builder;
 use Illuminate\Support\Collection;
@@ -17,7 +18,7 @@ class DatabaseRepository extends RepositoryQuery implements Repository
         return $this->queryable()->newModelInstance();
     }
 
-    public function getByMalId(int $id)
+    public function getByMalId(int $id): JikanApiModel|array|null
     {
         $results = $this->getAllByMalId($id);
 

app/Services/DefaultCachedScraperService.php

@@ -5,6 +5,7 @@ namespace App\Services;
 use App\Contracts\CachedScraperService;
 use App\Contracts\Repository;
 use App\Http\HttpHelper;
+use App\JikanApiModel;
 use App\Support\CachedData;
 use Illuminate\Contracts\Database\Query\Builder;
 use Illuminate\Support\Carbon;
@@ -60,7 +61,8 @@ final class DefaultCachedScraperService implements CachedScraperService
      */
     public function find(int $id, string $cacheKey): CachedData
     {
-        $results = CachedData::from($this->repository->getAllByMalId($id));
+        $dbResults = $this->repository->getAllByMalId($id);
+        $results = $this->dbResultSetToCachedData($dbResults);
 
         if ($results->isEmpty() || $results->isExpired()) {
             $response = $this->repository->scrape($id);
@@ -77,7 +79,8 @@ final class DefaultCachedScraperService implements CachedScraperService
 
     public function findByKey(string $key, mixed $val, string $cacheKey): CachedData
     {
-        $results = CachedData::from($this->repository->where($key, $val)->get());
+        $dbResults = $this->repository->where($key, $val)->get();
+        $results = $this->dbResultSetToCachedData($dbResults);
 
         if ($results->isEmpty() || $results->isExpired()) {
             $scraperResponse = $this->repository->scrape($key);
@@ -85,17 +88,18 @@ final class DefaultCachedScraperService implements CachedScraperService
             $this->raiseNotFoundIfErrors($scraperResponse);
 
             $response = $this->prepareScraperResponse($cacheKey, $results->isEmpty(), $scraperResponse);
-            $response->offsetSet($key, $val);
+            $response[$key] = $val;
 
             if ($results->isEmpty()) {
-                $this->repository->insert($response->toArray());
+                $this->repository->insert($response);
             }
 
             if ($results->isExpired()) {
-                $this->repository->where($key, $val)->update($response->toArray());
+                $this->repository->where($key, $val)->update($response);
             }
 
-            $results = CachedData::from($this->repository->where($key, $val)->get());
+            $dbResults = $this->repository->where($key, $val)->get();
+            $results = $this->dbResultSetToCachedData($dbResults);
         }
 
         $this->raiseNotFoundIfEmpty($results);
@@ -105,7 +109,30 @@ final class DefaultCachedScraperService implements CachedScraperService
 
     public function get(string $cacheKey): CachedData
     {
-        return CachedData::from($this->getByCacheKey($cacheKey));
+        $dbResults = $this->getByCacheKey($cacheKey);
+        return $this->dbResultSetToCachedData($dbResults);
+    }
+
+    private function dbResultSetToCachedData(Collection $dbResults): CachedData
+    {
+        if (!$dbResults->isEmpty()) {
+            $item = $dbResults->first();
+            return $this->dbRecordToCachedData($item);
+        }
+        else {
+            $item = collect();
+        }
+
+        return CachedData::from($item);
+    }
+
+    private function dbRecordToCachedData(JikanApiModel|array $item): CachedData
+    {
+        if ($item instanceof JikanApiModel) {
+            return CachedData::fromModel($item);
+        }
+
+        return CachedData::fromArray($item);
     }
 
     private function raiseNotFoundIfEmpty(CachedData $results)
@@ -127,14 +154,19 @@ final class DefaultCachedScraperService implements CachedScraperService
         $response = $this->prepareScraperResponse($cacheKey, $results->isEmpty(), $scraperResponse);
 
         if ($results->isEmpty()) {
-            $this->repository->insert($response->toArray());
+            $this->repository->insert($response);
         }
 
         if ($results->isExpired()) {
-            $this->repository->queryByMalId($id)->update($response->toArray());
+            $this->repository->queryByMalId($id)->update($response);
         }
 
-        return CachedData::from(collect($this->repository->getAllByMalId($id)));
+        $dbResult = $this->repository->getByMalId($id);
+        if ($dbResult === null) {
+            return CachedData::from(collect());
+        }
+
+        return $this->dbRecordToCachedData($dbResult);
     }
 
     private function updateCacheByKey(string $cacheKey, CachedData $results, array $scraperResponse): CachedData
@@ -143,15 +175,15 @@ final class DefaultCachedScraperService implements CachedScraperService
 
         // insert cache if resource doesn't exist
         if ($results->isEmpty()) {
-            $this->repository->insert($response->toArray());
+            $this->repository->insert($response);
         } else if ($results->isExpired()) {
-            $this->getQueryableByCacheKey($cacheKey)->update($response->toArray());
+            $this->getQueryableByCacheKey($cacheKey)->update($response);
         }
 
-        return CachedData::from($this->getByCacheKey($cacheKey));
+        return $this->get($cacheKey);
     }
 
-    private function prepareScraperResponse(string $cacheKey, bool $resultsEmpty, array $scraperResponse): CachedData
+    private function prepareScraperResponse(string $cacheKey, bool $resultsEmpty, array $scraperResponse): array
     {
         $meta = [];
         if ($resultsEmpty) {
@@ -167,7 +199,7 @@ final class DefaultCachedScraperService implements CachedScraperService
         $meta['modifiedAt'] = new UTCDateTime(Carbon::now()->getPreciseTimestamp(3));
 
         // join meta data with response
-        return CachedData::from(collect($meta + $scraperResponse));
+        return $meta + $scraperResponse;
     }
 
     private function getByCacheKey(string $cacheKey): Collection

app/Support/CachedData.php

@@ -26,6 +26,16 @@ final class CachedData
         return new self($scraperResult, app(CacheOptions::class)->ttl());
     }
 
+    public static function fromArray(array $scraperResult): self
+    {
+        return self::from(collect($scraperResult));
+    }
+
+    public static function fromModel(JikanApiModel $model): self
+    {
+        return self::fromArray($model->toArray());
+    }
+
     public function collect(): Collection
     {
         return $this->scraperResult;
@@ -82,20 +92,12 @@ final class CachedData
             return null;
         }
 
-        $result = $this->scraperResult->first();
+        $result = $this->scraperResult;
 
-        if ($result instanceof JikanApiModel && null != $modifiedAt = $result->getAttributeValue("modifiedAt")) {
+        if (null !== $modifiedAt = $result->get("modifiedAt")) {
             return $this->mixedToTimestamp($modifiedAt);
         }
 
-        if (is_array($result) && array_key_exists("modifiedAt", $result)) {
-            return $this->mixedToTimestamp($result["modifiedAt"]);
-        }
-
-        if (is_object($result) && property_exists($result, "modifiedAt")) {
-            return $this->mixedToTimestamp($result->modifiedAt);
-        }
-
         return null;
     }
 

tests/Unit/DefaultCachedScraperServiceTest.php

@@ -48,11 +48,18 @@ final class DefaultCachedScraperServiceTest extends TestCase
     public function testIfFindListReturnsNotExpiredItems()
     {
         $testRequestHash = $this->requestHash();
+        $now = Carbon::now();
+        Carbon::setTestNow($now);
         // the cached data in the database
-        $dummyResults = collect([
+        // this should be an array of arrays as builder->get() returns multiple items
-            ["dummy" => "dummy1", "modifiedAt" => new UTCDateTime()],
+        $dummyResults = collect([[
-            ["dummy" => "dummy2", "modifiedAt" => new UTCDateTime()]
+            "request_hash" => $testRequestHash,
-        ]);
+            "modifiedAt" => new UTCDateTime($now->getPreciseTimestamp(3)),
+            "results" => [
+                ["dummy" => "dummy1"],
+                ["dummy" => "dummy2"]
+            ]
+        ]]);
         [$queryBuilderMock, $repositoryMock, $serializerMock] = $this->makeCtorArgMocks();
         $queryBuilderMock->expects()->get()->once()->andReturn($dummyResults);
 
@@ -60,7 +67,7 @@ final class DefaultCachedScraperServiceTest extends TestCase
 
         $result = $target->findList($testRequestHash, fn() => []);
 
-        $this->assertEquals($dummyResults->toArray(), $result->toArray());
+        $this->assertEquals($dummyResults->first(), $result->toArray());
     }
 
     public function testIfFindListUpdatesCacheIfItemsExpired()
@@ -70,10 +77,15 @@ final class DefaultCachedScraperServiceTest extends TestCase
         Carbon::setTestNow($now);
 
         // the cached data in the database
-        $dummyResults = collect([
+        // this should be an array of arrays as builder->get() returns multiple items
-            ["dummy" => "dummy1", "modifiedAt" => new UTCDateTime($now->sub("2 days")->getPreciseTimestamp(3))],
+        $dummyResults = collect([[
-            ["dummy" => "dummy2", "modifiedAt" => new UTCDateTime($now->sub("2 days")->getPreciseTimestamp(3))]
+            "request_hash" => $testRequestHash,
-        ]);
+            "modifiedAt" => new UTCDateTime($now->sub("2 days")->getPreciseTimestamp(3)),
+            "results" => [
+                ["dummy" => "dummy1"],
+                ["dummy" => "dummy2"]
+            ]
+        ]]);
 
         // the data returned by the scraper
         $scraperData = [
@@ -83,8 +95,16 @@ final class DefaultCachedScraperServiceTest extends TestCase
             ]
         ];
         [$queryBuilderMock, $repositoryMock, $serializerMock] = $this->makeCtorArgMocks(3);
-        $queryBuilderMock->expects()->get()->twice()->andReturn($dummyResults);
+        $queryBuilderMock->expects()->update(Mockery::capture($updatedData))->once()->andReturn(1);
-        $queryBuilderMock->expects()->update(Mockery::any())->once()->andReturn(1);
+        $queryBuilderMock->shouldReceive("get")->twice()->andReturnUsing(
+            fn () => $dummyResults,
+            function () use (&$updatedData) {
+                // builder->get() returns multiple items
+                return collect([
+                    $updatedData
+                ]);
+            }
+        );
 
         $serializerMock->allows([
             "toArray" => $scraperData
@@ -94,8 +114,11 @@ final class DefaultCachedScraperServiceTest extends TestCase
         $result = $target->findList($testRequestHash, fn() => []);
 
         $this->assertEquals([
-            ["dummy" => "dummy1", "modifiedAt" => $dummyResults->toArray()[0]["modifiedAt"]],
+            "modifiedAt" => new UTCDateTime($now->getPreciseTimestamp(3)),
-            ["dummy" => "dummy2", "modifiedAt" => $dummyResults->toArray()[1]["modifiedAt"]]
+            "results" => [
+                ["dummy" => "dummy1"],
+                ["dummy" => "dummy2"]
+            ]
         ], $result->toArray());
     }
 
@@ -113,17 +136,27 @@ final class DefaultCachedScraperServiceTest extends TestCase
             ]
         ];
 
-        $cacheData = [
+        // the cached data in the database
-            ["dummy" => "dummy1", "modifiedAt" => new UTCDateTime($now->getPreciseTimestamp(3))],
+        // this should be an array of arrays as builder->get() returns multiple items
-            ["dummy" => "dummy2", "modifiedAt" => new UTCDateTime($now->getPreciseTimestamp(3))]
+        $cacheData = collect([[
-        ];
+            "request_hash" => $testRequestHash,
+            "modifiedAt" => new UTCDateTime($now->getPreciseTimestamp(3)),
+            "createdAt" => new UTCDateTime($now->getPreciseTimestamp(3)),
+            "results" => [
+                ["dummy" => "dummy1"],
+                ["dummy" => "dummy2"]
+            ]
+        ]]);;
 
         [$queryBuilderMock, $repositoryMock, $serializerMock] = $this->makeCtorArgMocks(2);
-        // at first the cache is empty
+        $repositoryMock->expects()->insert(Mockery::capture($insertedData))->once()->andReturn(true);
-        $queryBuilderMock->expects()->get()->once()->andReturn(collect());
+        // at first the cache is empty, then it gets "inserted" into
-        // then it gets "inserted" into
+        // so, we change the return value of builder->get accordingly
-        $queryBuilderMock->expects()->get()->once()->andReturn(collect($cacheData));
+        $queryBuilderMock->expects()->get()->twice()->andReturnUsing(fn() => collect(), function () use (&$insertedData) {
-        $repositoryMock->expects()->insert(Mockery::any())->once()->andReturn(true);
+            return collect([
+                $insertedData
+            ]);
+        });
 
         $serializerMock->allows([
             "toArray" => $scraperData
@@ -132,11 +165,6 @@ final class DefaultCachedScraperServiceTest extends TestCase
         $target = new DefaultCachedScraperService($repositoryMock, new MalClient(), $serializerMock);
         $result = $target->findList($testRequestHash, fn() => []);
 
-        $this->assertEquals($cacheData, $result->toArray());
+        $this->assertEquals($cacheData->first(), $result->toArray());
-    }
-
-    public function testIfModifiedAtValueSetCorrectlyDuringCacheUpdate()
-    {
-
     }
 }