PHPLIB-475: Use ReadPreference string constant consistently in the code and the doc

[GromNaN]

Fix PHPLIB-475
Caused by PHPC-1489
Block PHPC-1021

The ReadPreference::RP_* constants are deprecated and replaced by the string constants for readability.

• Replace int constants by their equivalent string constants
• Use $readPreference->getModeString() instead of $readPreference->getMode() to compare string values.

I used rector to automate the refactoring. Library users can do the same by copying the rule in rector.php. I have an issue to change the constructor argument from the string "primary" to the constant MongoDB\Driver\ReadPreference::PRIMARY rectorphp/rector#7956. More rector automation expected in PHPLIB-1141.

<?php // rector.php

use Rector\Config\RectorConfig;
use Rector\Renaming\Rector\ClassConstFetch\RenameClassConstFetchRector;
use Rector\Renaming\ValueObject\RenameClassConstFetch;

return static function (RectorConfig $rectorConfig): void {
    $rectorConfig->paths([
        __DIR__ . '/examples',
        __DIR__ . '/src',
        __DIR__ . '/tests',
        __DIR__ . '/tools',
    ]);

    $rectorConfig->ruleWithConfiguration(RenameClassConstFetchRector::class, [
        new RenameClassConstFetch(MongoDB\Driver\ReadPreference::class, 'RP_PRIMARY', 'PRIMARY'),
        new RenameClassConstFetch(MongoDB\Driver\ReadPreference::class, 'RP_PRIMARY_PREFERRED', 'PRIMARY_PREFERRED'),
        new RenameClassConstFetch(MongoDB\Driver\ReadPreference::class, 'RP_SECONDARY', 'SECONDARY'),
        new RenameClassConstFetch(MongoDB\Driver\ReadPreference::class, 'RP_SECONDARY_PREFERRED', 'SECONDARY_PREFERRED'),
        new RenameClassConstFetch(MongoDB\Driver\ReadPreference::class, 'RP_NEAREST', 'NEAREST'),
    ]);
};

[jmikola]

This doesn't work like you'd expect. Consider:

$a = new MongoDB\Driver\ReadPreference('primary');
$b = new MongoDB\Driver\ReadPreference('secondary');
$c = new MongoDB\Driver\ReadPreference('secondary', [[]]);

var_dump($a);
var_dump($b);
var_dump($c);

var_dump($a == $b);
var_dump($b == $c);
var_dump($c == $a);

Running that yields:

object(MongoDB\Driver\ReadPreference)#2 (1) {
  ["mode"]=>
  string(7) "primary"
}
object(MongoDB\Driver\ReadPreference)#4 (1) {
  ["mode"]=>
  string(9) "secondary"
}
object(MongoDB\Driver\ReadPreference)#5 (2) {
  ["mode"]=>
  string(9) "secondary"
  ["tags"]=>
  array(1) {
    [0]=>
    object(stdClass)#6 (0) {
    }
  }
}
bool(true)
bool(true)
bool(true)

This is because zend_std_compare_objects() defaults to comparing the properties HashTable of the zend_object struct, which we don't use in the extension. Each PHPC class has its own struct (e.g. php_phongo_readpreference_t) which contains internal properties (a mongoc_read_prefs_t* in this case) followed by an embedded zend_object struct. There is a separate properties HashTable in our struct, but it's only used by our own get_properties object handlers. The distinction is important, as zend_object.properties is what gets modified if you were to assign a dynamic property on a ReadPreference object in userland. That's one reason we chose to keep everything separate.

In any event, using assertEquals() here would require the ReadPreference, ReadConcern, and WriteConcern classes to implement explicit compare_objects handlers like we currently have for BSON classes.

If that sounds like something you're interested in (would make for a good PHPC onboarding task down the line), feel free to create two tickets:

• One PHPC ticket to implement the compare_objects handlers on those classes
• A second PHPLIB ticket to propose this refactoring, which should depend on the PHPC issue

In the meantime, please revert the assertions back to separate instanceof and field-level checks.

[GromNaN]

🤯 I didn't suspect it for a second. Indeed, it's preferable to implement compare_objects to prevent other developers from making the same mistake without having the chance to be corrected by @jmikola.

• PHPC-2234
• PHPLIB-1150

[jmikola]

One question about whether we should remove the rector rule. I defer to your decision there.

Reverting the RST docs to use string literals SGTM.

Provisional LGTM with those two items addressed.