PHPLIB-1518 WriteResult::get*Count() always return an int on acknow…

…ledged result (#1454)
mongodb · Sep 26, 2024 · ea818c8 · ea818c8
1 parent f584124
commit ea818c8
Show file tree

Hide file tree

Showing 15 changed files with 116 additions and 168 deletions.
diff --git a/psalm.xml.dist b/psalm.xml.dist
@@ -23,6 +23,7 @@
         <file name="stubs/BSON/PackedArray.stub.php"/>
         <file name="stubs/Driver/Cursor.stub.php"/>
         <file name="stubs/Driver/CursorInterface.stub.php"/>
+        <file name="stubs/Driver/WriteResult.stub.php"/>
     </stubs>
 
     <issueHandlers>

diff --git a/src/BulkWriteResult.php b/src/BulkWriteResult.php
@@ -17,19 +17,16 @@
 
 namespace MongoDB;
 
+use MongoDB\Driver\Exception\LogicException;
 use MongoDB\Driver\WriteResult;
-use MongoDB\Exception\BadMethodCallException;
 
 /**
  * Result class for a bulk write operation.
  */
 class BulkWriteResult
 {
-    private bool $isAcknowledged;
-
     public function __construct(private WriteResult $writeResult, private array $insertedIds)
     {
-        $this->isAcknowledged = $writeResult->isAcknowledged();
     }
 
     /**
@@ -38,15 +35,11 @@ public function __construct(private WriteResult $writeResult, private array $ins
      * This method should only be called if the write was acknowledged.
      *
      * @see BulkWriteResult::isAcknowledged()
-     * @throws BadMethodCallException if the write result is unacknowledged
+     * @throws LogicException if the write result is unacknowledged
      */
-    public function getDeletedCount(): ?int
+    public function getDeletedCount(): int
     {
-        if ($this->isAcknowledged) {
-            return $this->writeResult->getDeletedCount();
-        }
-
-        throw BadMethodCallException::unacknowledgedWriteResultAccess(__METHOD__);
+        return $this->writeResult->getDeletedCount();
     }
 
     /**
@@ -55,15 +48,11 @@ public function getDeletedCount(): ?int
      * This method should only be called if the write was acknowledged.
      *
      * @see BulkWriteResult::isAcknowledged()
-     * @throws BadMethodCallException if the write result is unacknowledged
+     * @throws LogicException if the write result is unacknowledged
      */
-    public function getInsertedCount(): ?int
+    public function getInsertedCount(): int
     {
-        if ($this->isAcknowledged) {
-            return $this->writeResult->getInsertedCount();
-        }
-
-        throw BadMethodCallException::unacknowledgedWriteResultAccess(__METHOD__);
+        return $this->writeResult->getInsertedCount();
     }
 
     /**
@@ -86,15 +75,11 @@ public function getInsertedIds(): array
      * This method should only be called if the write was acknowledged.
      *
      * @see BulkWriteResult::isAcknowledged()
-     * @throws BadMethodCallException if the write result is unacknowledged
+     * @throws LogicException if the write result is unacknowledged
      */
-    public function getMatchedCount(): ?int
+    public function getMatchedCount(): int
     {
-        if ($this->isAcknowledged) {
-            return $this->writeResult->getMatchedCount();
-        }
-
-        throw BadMethodCallException::unacknowledgedWriteResultAccess(__METHOD__);
+        return $this->writeResult->getMatchedCount();
     }
 
     /**
@@ -106,15 +91,11 @@ public function getMatchedCount(): ?int
      * This method should only be called if the write was acknowledged.
      *
      * @see BulkWriteResult::isAcknowledged()
-     * @throws BadMethodCallException if the write result is unacknowledged
+     * @throws LogicException if the write result is unacknowledged
      */
-    public function getModifiedCount(): ?int
+    public function getModifiedCount(): int
     {
-        if ($this->isAcknowledged) {
-            return $this->writeResult->getModifiedCount();
-        }
-
-        throw BadMethodCallException::unacknowledgedWriteResultAccess(__METHOD__);
+        return $this->writeResult->getModifiedCount();
     }
 
     /**
@@ -123,15 +104,11 @@ public function getModifiedCount(): ?int
      * This method should only be called if the write was acknowledged.
      *
      * @see BulkWriteResult::isAcknowledged()
-     * @throws BadMethodCallException if the write result is unacknowledged
+     * @throws LogicException if the write result is unacknowledged
      */
-    public function getUpsertedCount(): ?int
+    public function getUpsertedCount(): int
     {
-        if ($this->isAcknowledged) {
-            return $this->writeResult->getUpsertedCount();
-        }
-
-        throw BadMethodCallException::unacknowledgedWriteResultAccess(__METHOD__);
+        return $this->writeResult->getUpsertedCount();
     }
 
     /**
@@ -145,15 +122,11 @@ public function getUpsertedCount(): ?int
      * This method should only be called if the write was acknowledged.
      *
      * @see BulkWriteResult::isAcknowledged()
-     * @throws BadMethodCallException if the write result is unacknowledged
+     * @throws LogicException if the write result is unacknowledged
      */
     public function getUpsertedIds(): array
     {
-        if ($this->isAcknowledged) {
-            return $this->writeResult->getUpsertedIds();
-        }
-
-        throw BadMethodCallException::unacknowledgedWriteResultAccess(__METHOD__);
+        return $this->writeResult->getUpsertedIds();
     }
 
     /**
@@ -164,6 +137,6 @@ public function getUpsertedIds(): array
      */
     public function isAcknowledged(): bool
     {
-        return $this->isAcknowledged;
+        return $this->writeResult->isAcknowledged();
     }
 }
diff --git a/src/DeleteResult.php b/src/DeleteResult.php
@@ -17,19 +17,16 @@
 
 namespace MongoDB;
 
+use MongoDB\Driver\Exception\LogicException;
 use MongoDB\Driver\WriteResult;
-use MongoDB\Exception\BadMethodCallException;
 
 /**
  * Result class for a delete operation.
  */
 class DeleteResult
 {
-    private bool $isAcknowledged;
-
     public function __construct(private WriteResult $writeResult)
     {
-        $this->isAcknowledged = $writeResult->isAcknowledged();
     }
 
     /**
@@ -38,15 +35,11 @@ public function __construct(private WriteResult $writeResult)
      * This method should only be called if the write was acknowledged.
      *
      * @see DeleteResult::isAcknowledged()
-     * @throws BadMethodCallException if the write result is unacknowledged
+     * @throws LogicException if the write result is unacknowledged
      */
-    public function getDeletedCount(): ?int
+    public function getDeletedCount(): int
     {
-        if ($this->isAcknowledged) {
-            return $this->writeResult->getDeletedCount();
-        }
-
-        throw BadMethodCallException::unacknowledgedWriteResultAccess(__METHOD__);
+        return $this->writeResult->getDeletedCount();
     }
 
     /**
@@ -57,6 +50,6 @@ public function getDeletedCount(): ?int
      */
     public function isAcknowledged(): bool
     {
-        return $this->isAcknowledged;
+        return $this->writeResult->isAcknowledged();
     }
 }
diff --git a/src/Exception/BadMethodCallException.php b/src/Exception/BadMethodCallException.php
@@ -33,15 +33,4 @@ public static function classIsImmutable(string $class): self
     {
         return new self(sprintf('%s is immutable', $class));
     }
-
-    /**
-     * Thrown when accessing a result field on an unacknowledged write result.
-     *
-     * @param string $method Method name
-     * @internal
-     */
-    public static function unacknowledgedWriteResultAccess(string $method): self
-    {
-        return new self(sprintf('%s should not be called for an unacknowledged write result', $method));
-    }
 }
diff --git a/src/GridFS/Bucket.php b/src/GridFS/Bucket.php
@@ -583,16 +583,9 @@ public function rename(mixed $id, string $newFilename): void
             return;
         }
 
-        /* If the update resulted in no modification, it's possible that the
-         * file did not exist, in which case we must raise an error. Checking
-         * the write result's matched count will be most efficient, but fall
-         * back to a findOne operation if necessary (i.e. legacy writes).
-         */
-        $found = $updateResult->getMatchedCount() !== null
-            ? $updateResult->getMatchedCount() === 1
-            : $this->collectionWrapper->findFileById($id) !== null;
-
-        if (! $found) {
+        // If the update resulted in no modification, it's possible that the
+        // file did not exist, in which case we must raise an error.
+        if ($updateResult->getMatchedCount() !== 1) {
             throw FileNotFoundException::byId($id, $this->getFilesNamespace());
         }
     }

diff --git a/src/GridFS/CollectionWrapper.php b/src/GridFS/CollectionWrapper.php
@@ -73,7 +73,7 @@ public function deleteChunksByFilesId(mixed $id): void
     /**
      * Delete all GridFS files and chunks for a given filename.
      */
-    public function deleteFileAndChunksByFilename(string $filename): ?int
+    public function deleteFileAndChunksByFilename(string $filename): int
     {
         /** @var iterable<array{_id: mixed}> $files */
         $files = $this->findFiles(['filename' => $filename], [
@@ -262,7 +262,7 @@ public function insertFile(array|object $file): void
     /**
      * Updates the filename field in the file document for all the files with a given filename.
      */
-    public function updateFilenameForFilename(string $filename, string $newFilename): ?int
+    public function updateFilenameForFilename(string $filename, string $newFilename): int
     {
         return $this->filesCollection->updateMany(
             ['filename' => $filename],

diff --git a/src/InsertManyResult.php b/src/InsertManyResult.php
@@ -17,19 +17,16 @@
 
 namespace MongoDB;
 
+use MongoDB\Driver\Exception\LogicException;
 use MongoDB\Driver\WriteResult;
-use MongoDB\Exception\BadMethodCallException;
 
 /**
  * Result class for a multi-document insert operation.
  */
 class InsertManyResult
 {
-    private bool $isAcknowledged;
-
     public function __construct(private WriteResult $writeResult, private array $insertedIds)
     {
-        $this->isAcknowledged = $writeResult->isAcknowledged();
     }
 
     /**
@@ -38,15 +35,11 @@ public function __construct(private WriteResult $writeResult, private array $ins
      * This method should only be called if the write was acknowledged.
      *
      * @see InsertManyResult::isAcknowledged()
-     * @throws BadMethodCallException if the write result is unacknowledged
+     * @throws LogicException if the write result is unacknowledged
      */
-    public function getInsertedCount(): ?int
+    public function getInsertedCount(): int
     {
-        if ($this->isAcknowledged) {
-            return $this->writeResult->getInsertedCount();
-        }
-
-        throw BadMethodCallException::unacknowledgedWriteResultAccess(__METHOD__);
+        return $this->writeResult->getInsertedCount();
     }
 
     /**

diff --git a/src/InsertOneResult.php b/src/InsertOneResult.php
@@ -17,19 +17,16 @@
 
 namespace MongoDB;
 
+use MongoDB\Driver\Exception\LogicException;
 use MongoDB\Driver\WriteResult;
-use MongoDB\Exception\BadMethodCallException;
 
 /**
  * Result class for a single-document insert operation.
  */
 class InsertOneResult
 {
-    private bool $isAcknowledged;
-
     public function __construct(private WriteResult $writeResult, private mixed $insertedId)
     {
-        $this->isAcknowledged = $writeResult->isAcknowledged();
     }
 
     /**
@@ -38,15 +35,11 @@ public function __construct(private WriteResult $writeResult, private mixed $ins
      * This method should only be called if the write was acknowledged.
      *
      * @see InsertOneResult::isAcknowledged()
-     * @throws BadMethodCallException if the write result is unacknowledged
+     * @throws LogicException if the write result is unacknowledged
      */
-    public function getInsertedCount(): ?int
+    public function getInsertedCount(): int
     {
-        if ($this->isAcknowledged) {
-            return $this->writeResult->getInsertedCount();
-        }
-
-        throw BadMethodCallException::unacknowledgedWriteResultAccess(__METHOD__);
+        return $this->writeResult->getInsertedCount();
     }
 
     /**