refactor: changed types and tests

leroy-merlin-br · Oct 2, 2024 · 4f07e60 · 4f07e60
1 parent eb7ab11
commit 4f07e60
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 80 deletions.
diff --git a/src/Query/Builder.php b/src/Query/Builder.php
@@ -1,6 +1,7 @@
 <?php
 namespace Mongolid\Query;
 
+use Illuminate\Contracts\Container\BindingResolutionException;
 use InvalidArgumentException;
 use Mongolid\Connection\Connection;
 use Mongolid\Container\Container;
@@ -10,6 +11,7 @@
 use Mongolid\Cursor\EagerLoadingCursor;
 use Mongolid\Event\EventTriggerService;
 use Mongolid\Model\Exception\ModelNotFoundException;
+use Mongolid\Model\Exception\NoCollectionNameException;
 use Mongolid\Model\ModelInterface;
 
 /**
@@ -20,23 +22,17 @@ class Builder
 {
     private bool $ignoreSoftDelete = false;
 
-    /**
-     * Connection that is going to be used to interact with the database.
-     *
-     * @var Connection
-     */
-    protected $connection;
-
     /**
      * In order to dispatch events when necessary.
-     *
-     * @var EventTriggerService
      */
-    protected $eventService;
-
-    public function __construct(Connection $connection)
-    {
-        $this->connection = $connection;
+    protected ?EventTriggerService $eventService = null;
+
+    public function __construct(
+        /**
+         * Connection that is going to be used to interact with the database.
+         */
+        protected Connection $connection
+    ){
     }
 
     /**
@@ -46,8 +42,10 @@ public function __construct(Connection $connection)
      * Notice: Saves with Unacknowledged WriteConcern will not fire `saved` event.
      * Return is always false if write concern is Unacknowledged.
      *
-     * @param ModelInterface $model   the model used in the operation
-     * @param array          $options possible options to send to mongo driver
+     * @param ModelInterface $model the model used in the operation
+     * @param array $options possible options to send to mongo driver
+     * @throws BindingResolutionException
+     * @throws NoCollectionNameException
      */
     public function save(ModelInterface $model, array $options = []): bool
     {
@@ -86,9 +84,11 @@ public function save(ModelInterface $model, array $options = []): bool
      * Notice: Inserts with Unacknowledged WriteConcern will not fire `inserted` event.
      * Return is always false if write concern is Unacknowledged.
      *
-     * @param ModelInterface $model      the model used in the operation
-     * @param array          $options    possible options to send to mongo driver
-     * @param bool           $fireEvents whether events should be fired
+     * @param ModelInterface $model the model used in the operation
+     * @param array $options possible options to send to mongo driver
+     * @param bool $fireEvents whether events should be fired
+     * @throws BindingResolutionException
+     * @throws NoCollectionNameException
      */
     public function insert(ModelInterface $model, array $options = [], bool $fireEvents = true): bool
     {
@@ -122,8 +122,10 @@ public function insert(ModelInterface $model, array $options = [], bool $fireEve
      * Notice: Updates with Unacknowledged WriteConcern will not fire `updated` event.
      * Return is always false if write concern is Unacknowledged.
      *
-     * @param ModelInterface $model   the model used in the operation
-     * @param array          $options possible options to send to mongo driver
+     * @param ModelInterface $model the model used in the operation
+     * @param array $options possible options to send to mongo driver
+     * @throws BindingResolutionException
+     * @throws NoCollectionNameException
      */
     public function update(ModelInterface $model, array $options = []): bool
     {
@@ -166,8 +168,10 @@ public function update(ModelInterface $model, array $options = []): bool
      * Notice: Deletes with Unacknowledged WriteConcern will not fire `deleted` event.
      * Return is always false if write concern is Unacknowledged.
      *
-     * @param ModelInterface $model   the model used in the operation
-     * @param array          $options possible options to send to mongo driver
+     * @param ModelInterface $model the model used in the operation
+     * @param array $options possible options to send to mongo driver
+     * @throws BindingResolutionException
+     * @throws NoCollectionNameException
      */
     public function delete(ModelInterface $model, array $options = []): bool
     {
@@ -198,8 +202,9 @@ public function delete(ModelInterface $model, array $options = []): bool
      * @param mixed          $query      MongoDB query to retrieve documents
      * @param array          $projection fields to project in MongoDB query
      * @param bool           $useCache   retrieves a CacheableCursor instead
+     * @throws NoCollectionNameException
      */
-    public function where(ModelInterface $model, $query = [], array $projection = [], bool $useCache = false): CursorInterface
+    public function where(ModelInterface $model, mixed $query = [], array $projection = [], bool $useCache = false): CursorInterface
     {
         $cursor = $useCache ? CacheableCursor::class : Cursor::class;
 
@@ -240,8 +245,9 @@ public function all(ModelInterface $model): CursorInterface
      * @param boolean        $useCache   retrieves the first through a CacheableCursor
      *
      * @return ModelInterface|array|null
+     * @throws NoCollectionNameException
      */
-    public function first(ModelInterface $model, $query = [], array $projection = [], bool $useCache = false)
+    public function first(ModelInterface $model, mixed $query = [], array $projection = [], bool $useCache = false): mixed
     {
         if (null === $query) {
             return null;
@@ -275,7 +281,7 @@ public function first(ModelInterface $model, $query = [], array $projection = []
      *
      * @return ModelInterface|null
      */
-    public function firstOrFail(ModelInterface $model, $query = [], array $projection = [], bool $useCache = false)
+    public function firstOrFail(ModelInterface $model, mixed $query = [], array $projection = [], bool $useCache = false): mixed
     {
         if ($result = $this->first($model, $query, $projection, $useCache)) {
             return $result;
@@ -295,14 +301,15 @@ public function withoutSoftDelete(): self
      * Triggers an event. May return if that event had success.
      *
      * @param string $event identification of the event
-     * @param mixed  $model event payload
-     * @param bool   $halt  true if the return of the event handler will be used in a conditional
+     * @param ModelInterface $model event payload
+     * @param bool $halt true if the return of the event handler will be used in a conditional
      *
      * @return mixed event handler return
+     * @throws BindingResolutionException
      */
-    protected function fireEvent(string $event, ModelInterface $model, bool $halt = false)
+    protected function fireEvent(string $event, ModelInterface $model, bool $halt = false): mixed
     {
-        $event = "mongolid.{$event}: ".get_class($model);
+        $event = "mongolid.{$event}: ".$model::class;
 
         $this->eventService ?: $this->eventService = Container::make(EventTriggerService::class);
 
@@ -351,7 +358,7 @@ protected function prepareProjection(array $fields): array
 
             if (is_int($key) && is_string($value)) {
                 $key = $value;
-                if (0 === strpos($value, '-')) {
+                if (str_starts_with($value, '-')) {
                     $key = substr($key, 1);
                     $value = false;
                 } else {
@@ -404,7 +411,6 @@ private function calculateChanges(array &$changes, array $newData, array $oldDat
         foreach ($oldData as $k => $v) { // data that used to exist, but now doesn't
             if (!isset($newData[$k])) { // removed field
                 $changes['$unset']["{$keyfix}{$k}"] = '';
-                continue;
             }
         }
     }
@@ -430,7 +436,7 @@ private function afterSuccess(ModelInterface $model): void
         $model->syncOriginalDocumentAttributes();
     }
 
-    private function getUpdateData($model, array $data): array
+    private function getUpdateData(ModelInterface $model, array $data): array
     {
         $changes = [];
         $this->calculateChanges($changes, $data, $model->getOriginalDocumentAttributes());

diff --git a/src/Query/BulkWrite.php b/src/Query/BulkWrite.php
@@ -4,6 +4,7 @@
 use MongoDB\BSON\ObjectId;
 use MongoDB\BulkWriteResult;
 use MongoDB\Driver\WriteConcern;
+use Mongolid\Model\Exception\NoCollectionNameException;
 use Mongolid\Model\ModelInterface;
 
 /**
@@ -14,21 +15,14 @@
  */
 class BulkWrite
 {
-    /**
-     * @var ModelInterface
-     */
-    private $model;
-
     /**
      * Hold bulk write operations to run.
-     *
-     * @var array
      */
-    private $operations = [];
+    private array $operations = [];
 
-    public function __construct(ModelInterface $model)
-    {
-        $this->model = $model;
+    public function __construct(
+        private ModelInterface $model
+    ){
     }
 
     public function isEmpty(): bool
@@ -43,12 +37,9 @@ public function isEmpty(): bool
      *
      * @see https://docs.mongodb.com/manual/reference/operator/update/set/#set-top-level-fields
      *
-     * @param ObjectId|string|array $filter
-     * @param array                 $dataToSet
-     * @param array                 $options
      */
     public function updateOne(
-        $filter,
+        ObjectId|string|array $filter,
         array $dataToSet,
         array $options = ['upsert' => true],
         string $operator = '$set'
@@ -64,7 +55,7 @@ public function updateOne(
      * Execute the BulkWrite, using connection.
      * The collection is inferred from model's collection name.
      *
-     * @throws \Mongolid\Model\Exception\NoCollectionNameException
+     * @throws NoCollectionNameException
      */
     public function execute(int $writeConcern = 1): BulkWriteResult
     {

diff --git a/src/Query/EagerLoader/Cache.php b/src/Query/EagerLoader/Cache.php
@@ -9,14 +9,10 @@ class Cache
 {
     use CacheKeyGeneratorTrait;
 
-    /**
-     * @var CacheComponentInterface
-     */
-    private $cacheComponent;
-
-    public function __construct(CacheComponentInterface $cacheComponent)
-    {
-        $this->cacheComponent = $cacheComponent;
+    public function __construct(
+        private
+        CacheComponentInterface $cacheComponent
+    ){
     }
 
     public function cache(array $eagerLoadedModel): void

diff --git a/src/Query/EagerLoader/Extractor.php b/src/Query/EagerLoader/Extractor.php
@@ -12,24 +12,21 @@
  */
 class Extractor
 {
-    /**
-     * This array will handle all related models extracted
-     * from the models passed by.
-     *
-     * @var array
-     */
-    private $relatedModels;
-
-    public function __construct(array $relatedModels)
-    {
-        $this->relatedModels = $relatedModels;
+    public function __construct(
+        /**
+         * This array will handle all related models extracted
+         * from the models passed by.
+         */
+        private array $relatedModels
+    ){
     }
 
     /**
      * The model received here may be either an array from mongodb
      * or a cached model as a serialized array.
      * That's why we always will force an array to be used here.
      * It will ensure that we are only working with model arrays.
+     * @throws EagerLoaderException
      */
     public function extractFrom(array $model): array
     {
@@ -55,7 +52,7 @@ public function getRelatedModels(): array
         return $this->relatedModels;
     }
 
-    private function addIdFor(string $eagerLoadKey, $id): void
+    private function addIdFor(string $eagerLoadKey, mixed $id): void
     {
         $this->relatedModels[$eagerLoadKey]['ids'][$id] = $id;
     }
@@ -67,7 +64,7 @@ private function addIdFor(string $eagerLoadKey, $id): void
      *
      * @example 'skus.shop_id' => I want all shop ids from embedded skus.
      */
-    private function keyHasDots($key): bool
+    private function keyHasDots(string $key): bool
     {
         return str_contains($key, '.');
     }
@@ -76,10 +73,11 @@ private function keyHasDots($key): bool
      * Using dot notations, the user can specify what ids on embedded
      * models he wants to extract. So, our job is to loop on every
      * embedded model to get the id specified on the key.
+     * @throws EagerLoaderException
      */
     private function extractFromEmbeddedModel(string $eagerLoadKey, array $model, string $key): void
     {
-        list($method, $attribute) = explode('.', $key);
+        [$method, $attribute] = explode('.', $key);
 
         // As we are working with models as array, this give us
         // flexibility to use it as array instead of calling methods.
@@ -88,6 +86,9 @@ private function extractFromEmbeddedModel(string $eagerLoadKey, array $model, st
         }
     }
 
+    /**
+     * @throws EagerLoaderException
+     */
     private function extractFromModel(string $eagerLoadKey, array $model, string $key): void
     {
         if (!$id = $model[$key] ?? false) {

diff --git a/src/Query/ModelMapper.php b/src/Query/ModelMapper.php
@@ -76,7 +76,7 @@ private function manageTimestamps(ModelInterface $model, bool $timestamps): void
         }
     }
 
-    private function manageId(ModelInterface $model)
+    private function manageId(ModelInterface $model): void
     {
         $value = $model->_id;
 

diff --git a/tests/Unit/Query/BuilderTest.php b/tests/Unit/Query/BuilderTest.php
@@ -148,7 +148,7 @@ public function testShouldInsertWithoutFiringEvents(
         int $writeConcern,
         bool $shouldFireEventAfter,
         bool $expected
-    ) {
+    ): void {
         // Set
         $connection = m::mock(Connection::class);
         $builder = new Builder($connection);
@@ -359,7 +359,7 @@ public function testUpdateShouldCallInsertWhenObjectHasNoId(
         int $writeConcern,
         bool $shouldFireEventAfter,
         bool $expected
-    ) {
+    ):void {
         // Set
         $connection = m::mock(Connection::class);
         $builder = new Builder($connection);
@@ -462,7 +462,7 @@ public function testDatabaseOperationsShouldBailOutIfTheEventHandlerReturnsFalse
         string $operation,
         string $dbOperation,
         string $eventName
-    ) {
+    ): void {
         // Set
         $connection = m::mock(Connection::class);
         $builder = m::mock(Builder::class.'[getCollection]', [$connection]);
@@ -552,9 +552,6 @@ public function testShouldGetWithWhereQueryEagerLoadingModels(): void
             ],
         ];
 
-        // Expectations
-
-
         // Actions
         $result = $builder->where($model, $query, $projection);
         $collectionResult = $this->getProtected($result, 'collection');

diff --git a/tests/Unit/Query/EagerLoader/ExtractorTest.php b/tests/Unit/Query/EagerLoader/ExtractorTest.php
@@ -34,14 +34,14 @@ public function testShouldExtractIdFromModel(): void
         $expected = [
             'price' => [
                 'key' => '_id',
-                'model' => 'Mongolid\Tests\Stubs\Price',
+                'model' => \Mongolid\Tests\Stubs\Price::class,
                 'ids' => [
                     123 => 123,
                 ],
             ],
             'shop' => [
                 'key' => 'skus.shop_id',
-                'model' => 'Mongolid\Tests\Stubs\Legacy\Shop',
+                'model' => \Mongolid\Tests\Stubs\Legacy\Shop::class,
                 'ids' => [
                     12345 => 12345,
                 ],