PHPC-2412: Deprecate CursorId class (#1616)

* PHPC-2412: Deprecate CursorId class * Use zend_bool instead of bool * Remove fixed length assertion in test * Correctly set default parameter value * Remove int cast to avoid warnings on 32-bit platforms * Fix wrong expectation for CursorId debug format * Use new cursor ID logic in tests where possible * Remove deprecation comment from CursorId stub * Restore cursorid serialisation test * Restore original class initialisation order
mongodb · Sep 5, 2024 · 0cfecc2 · 0cfecc2
1 parent 17624af
commit 0cfecc2
Show file tree

Hide file tree

Showing 14 changed files with 127 additions and 29 deletions.
diff --git a/src/MongoDB/Cursor.c b/src/MongoDB/Cursor.c
@@ -143,12 +143,22 @@ static PHP_METHOD(MongoDB_Driver_Cursor, toArray)
 static PHP_METHOD(MongoDB_Driver_Cursor, getId)
 {
 	php_phongo_cursor_t* intern;
+	zend_bool            asInt64 = false;
 
 	intern = Z_CURSOR_OBJ_P(getThis());
 
-	PHONGO_PARSE_PARAMETERS_NONE();
+	PHONGO_PARSE_PARAMETERS_START(0, 1)
+	Z_PARAM_OPTIONAL
+	Z_PARAM_BOOL(asInt64)
+	PHONGO_PARSE_PARAMETERS_END();
 
-	php_phongo_cursor_id_new_from_id(return_value, mongoc_cursor_get_id(intern->cursor));
+	if (asInt64) {
+		phongo_int64_new(return_value, mongoc_cursor_get_id(intern->cursor));
+	} else {
+		php_error_docref(NULL, E_DEPRECATED, "The method \"MongoDB\\Driver\\Cursor::getId\" will no longer return a \"MongoDB\\Driver\\CursorId\" instance in the future. Pass \"true\" as argument to change to the new behavior and receive a \"MongoDB\\BSON\\Int64\" instance instead.");
+
+		php_phongo_cursor_id_new_from_id(return_value, mongoc_cursor_get_id(intern->cursor));
+	}
 }
 
 /* Returns the Server object to which this cursor is attached */

diff --git a/src/MongoDB/Cursor.stub.php b/src/MongoDB/Cursor.stub.php
@@ -18,7 +18,13 @@ public function current(): array|object|null {}
     public function current() {}
 #endif
 
-    final public function getId(): CursorId {}
+#if PHP_VERSION_ID >= 80000
+    /** @tentative-return-type */
+    final public function getId(bool $asInt64 = false): CursorId|\MongoDB\BSON\Int64 {}
+#else
+    /** @return CursorId|\MongoDB\BSON\Int64 */
+    final public function getId(bool $asInt64 = false) {}
+#endif
 
     final public function getServer(): Server {}
 

diff --git a/src/MongoDB/CursorInterface.stub.php b/src/MongoDB/CursorInterface.stub.php
@@ -9,8 +9,13 @@
 
 interface CursorInterface extends \Traversable
 {
+#if PHP_VERSION_ID >= 80000
     /** @tentative-return-type */
-    public function getId(): CursorId;
+    public function getId(): CursorId|\MongoDB\BSON\Int64;
+#else
+    /** @return CursorId|\MongoDB\BSON\Int64 */
+    public function getId();
+#endif
 
     /** @tentative-return-type */
     public function getServer(): Server;

diff --git a/src/MongoDB/CursorInterface_arginfo.h b/src/MongoDB/CursorInterface_arginfo.h
diff --git a/src/MongoDB/Cursor_arginfo.h b/src/MongoDB/Cursor_arginfo.h
diff --git a/tests/cursor/cursor-session-001.phpt b/tests/cursor/cursor-session-001.phpt
@@ -26,7 +26,7 @@ $iterator = new IteratorIterator($cursor);
 $iterator->rewind();
 $iterator->next();
 
-printf("Cursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no');
+printf("Cursor ID is zero: %s\n", $cursor->getId(true) == 0 ? 'yes' : 'no');
 var_dump($cursor);
 
 $iterator->next();
@@ -35,7 +35,7 @@ $iterator->next();
  * is exhausted. While this is primarily done to ensure implicit sessions for
  * command cursors are returned to the pool ASAP, it also applies to explicit
  * sessions. */
-printf("\nCursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no');
+printf("\nCursor ID is zero: %s\n", $cursor->getId(true) == 0 ? 'yes' : 'no');
 var_dump($cursor);
 
 ?>

diff --git a/tests/cursor/cursor-session-002.phpt b/tests/cursor/cursor-session-002.phpt
@@ -28,12 +28,12 @@ $iterator->next();
 /* Implicit sessions for query cursors are never exposed to PHPC, as they are
  * handled internally by libmongoc. Cursor debug ouput should never report such
  * sessions. */
-printf("Cursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no');
+printf("Cursor ID is zero: %s\n", $cursor->getId(true) == 0 ? 'yes' : 'no');
 var_dump($cursor);
 
 $iterator->next();
 
-printf("\nCursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no');
+printf("\nCursor ID is zero: %s\n", $cursor->getId(true) == 0 ? 'yes' : 'no');
 var_dump($cursor);
 
 ?>

diff --git a/tests/cursor/cursor-session-003.phpt b/tests/cursor/cursor-session-003.phpt
@@ -30,7 +30,7 @@ $iterator = new IteratorIterator($cursor);
 $iterator->rewind();
 $iterator->next();
 
-printf("Cursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no');
+printf("Cursor ID is zero: %s\n", $cursor->getId(true) == 0 ? 'yes' : 'no');
 var_dump($cursor);
 
 $iterator->next();
@@ -39,7 +39,7 @@ $iterator->next();
  * is exhausted. While this is primarily done to ensure implicit sessions for
  * command cursors are returned to the pool ASAP, it also applies to explicit
  * sessions. */
-printf("\nCursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no');
+printf("\nCursor ID is zero: %s\n", $cursor->getId(true) == 0 ? 'yes' : 'no');
 var_dump($cursor);
 
 ?>

diff --git a/tests/cursor/cursor-session-004.phpt b/tests/cursor/cursor-session-004.phpt
@@ -29,7 +29,7 @@ $iterator = new IteratorIterator($cursor);
 $iterator->rewind();
 $iterator->next();
 
-printf("Cursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no');
+printf("Cursor ID is zero: %s\n", $cursor->getId(true) == 0 ? 'yes' : 'no');
 var_dump($cursor);
 
 $iterator->next();
@@ -38,7 +38,7 @@ $iterator->next();
  * libmongoc, PHPC-1152 emulates its own implicit sessions for command cursors
  * in order to ensure that command cursors always share the same session as the
  * originating command. */
-printf("\nCursor ID is zero: %s\n", (string) $cursor->getId() === '0' ? 'yes' : 'no');
+printf("\nCursor ID is zero: %s\n", $cursor->getId(true) == 0 ? 'yes' : 'no');
 var_dump($cursor);
 
 ?>

diff --git a/tests/cursor/cursor-tailable_error-002.phpt b/tests/cursor/cursor-tailable_error-002.phpt
@@ -57,7 +57,7 @@ echo throws(function() use ($manager) {
         if ($numAwaitAttempts === 5) {
             $cursor->getServer()->executeCommand(DATABASE_NAME, new MongoDB\Driver\Command([
                 'killCursors' => COLLECTION_NAME,
-                'cursors' => [ $cursor->getId() ],
+                'cursors' => [ $cursor->getId(true) ],
             ]));
         }
 

diff --git a/tests/cursor/cursorid-getId-001.phpt b/tests/cursor/cursorid-getId-001.phpt
@@ -0,0 +1,45 @@
+--TEST--
+MongoDB\Driver\Cursor::getId
+--SKIPIF--
+<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
+<?php skip_if_not_live(); ?>
+<?php skip_if_not_clean(); ?>
+--FILE--
+<?php
+
+require_once __DIR__ . "/../utils/basic.inc";
+
+$manager = create_test_manager();
+
+$bulk = new MongoDB\Driver\BulkWrite();
+$bulk->insert(['_id' => 1]);
+$bulk->insert(['_id' => 2]);
+$bulk->insert(['_id' => 3]);
+$manager->executeBulkWrite(NS, $bulk);
+
+$cursor = $manager->executeQuery(NS, new MongoDB\Driver\Query([], ['batchSize' => 2]));
+
+var_dump($cursor->getId());
+var_dump($cursor->getId(false));
+var_dump($cursor->getId(true));
+
+?>
+===DONE===
+<?php exit(0); ?>
+--EXPECTF--
+Deprecated: MongoDB\Driver\Cursor::getId(): The method "MongoDB\Driver\Cursor::getId" will no longer return a "MongoDB\Driver\CursorId" instance in the future. Pass "true" as argument to change to the new behavior and receive a "MongoDB\BSON\Int64" instance instead. in %s
+object(MongoDB\Driver\CursorId)#%d (%d) {
+  ["id"]=>
+  %rint\(%d\)|string\(%d\) "%d"%r
+}
+
+Deprecated: MongoDB\Driver\Cursor::getId(): The method "MongoDB\Driver\Cursor::getId" will no longer return a "MongoDB\Driver\CursorId" instance in the future. Pass "true" as argument to change to the new behavior and receive a "MongoDB\BSON\Int64" instance instead. in %s
+object(MongoDB\Driver\CursorId)#%d (%d) {
+  ["id"]=>
+  %rint\(%d\)|string\(%d\) "%d"%r
+}
+object(MongoDB\BSON\Int64)#%d (%d) {
+  ["integer"]=>
+  string(%d) "%d"
+}
+===DONE===
diff --git a/tests/cursorid/cursorid-001.phpt b/tests/cursorid/cursorid-001.phpt
@@ -26,6 +26,7 @@ hex_dump(fromPHP(['cid' => $cursorId]));
 ===DONE===
 <?php exit(0); ?>
 --EXPECTF--
+Deprecated: MongoDB\Driver\Cursor::getId(): The method "MongoDB\Driver\Cursor::getId" will no longer return a "MongoDB\Driver\CursorId" instance in the future. Pass "true" as argument to change to the new behavior and receive a "MongoDB\BSON\Int64" instance instead. in %s
      0 : 12 00 00 00 12 63 69 64 00 %x %x %x %x %x %x %x  [.....cid.%s]
     10 : %x 00                                            [%s.]
 ===DONE===
diff --git a/tests/cursorid/cursorid-002.phpt b/tests/cursorid/cursorid-002.phpt
@@ -37,7 +37,8 @@ printf("Killed expected cursor: %s\n", (string) $cursorId === (string) $result->
 ?>
 ===DONE===
 <?php exit(0); ?>
---EXPECT--
+--EXPECTF--
+Deprecated: MongoDB\Driver\Cursor::getId(): The method "MongoDB\Driver\Cursor::getId" will no longer return a "MongoDB\Driver\CursorId" instance in the future. Pass "true" as argument to change to the new behavior and receive a "MongoDB\BSON\Int64" instance instead. in %s
 Killed 1 cursor(s)
 Killed expected cursor: yes
 ===DONE===
diff --git a/tests/functional/cursorid-001.phpt b/tests/functional/cursorid-001.phpt
@@ -20,22 +20,17 @@ $query = new MongoDB\Driver\Query(array(), array(
 
 $cursor = $manager->executeQuery(NS, $query);
 
-$cursorid = $cursor->getId();
-$s1 = (string)$cursorid;
-var_dump(
-    $cursorid,
-    $s1
-);
-var_dump($s1 > 0); 
+$cursorid = $cursor->getId(true);
+var_dump($cursorid);
+var_dump($cursorid != 0);
 
 ?>
 ===DONE===
 <?php exit(0); ?>
 --EXPECTF--
-object(MongoDB\Driver\CursorId)#%d (%d) {
-  ["id"]=>
-  %rint\(\d+\)|string\(\d+\) "\d+"%r
+object(MongoDB\BSON\Int64)#%d (%d) {
+  ["integer"]=>
+  string(%d) "%d"
 }
-string(%d) "%d"
 bool(true)
 ===DONE===