Skip to content

PHPLIB-1680: Remove pre-4.2 wire version checks and test code #1801

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jmikola wants to merge 5 commits into
base: v2.x
Choose a base branch
from
Open

PHPLIB-1680: Remove pre-4.2 wire version checks and test code #1801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1b4ef2a
Remove obsolete wire version checks for MongoDB 4.2
alcaeus May 20, 2025
4288925
Remove tests for pre-4.2 servers
jmikola Nov 26, 2025
22e3cc3
Transactions are supported on 4.2+ sharded clusters
jmikola Nov 26, 2025
657f785
phpcbf fixes
jmikola Dec 15, 2025
911fabc
Don't check storageEngine on mongos
jmikola Dec 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions psalm-baseline.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -813,9 +813,6 @@
</PossiblyInvalidArgument>
</file>
<file src="src/Operation/Update.php">
<MixedArgument>
<code><![CDATA[$this->options['writeConcern']]]></code>
</MixedArgument>
<MixedAssignment>
<code><![CDATA[$cmd['bypassDocumentValidation']]]></code>
<code><![CDATA[$options[$option]]]></code>
Expand Down
19 changes: 5 additions & 14 deletions src/Collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ class Collection implements Stringable
'root' => BSONDocument::class,
];

private const WIRE_VERSION_FOR_READ_CONCERN_WITH_WRITE_STAGE = 8;

/** @psalm-var Encoder<array|stdClass|Document|PackedArray, mixed> */
private readonly Encoder $builderEncoder;

Expand Down Expand Up @@ -236,24 +234,17 @@ public function aggregate(array|Pipeline $pipeline, array $options = []): Cursor
$hasWriteStage = is_last_pipeline_operator_write($pipeline);

$options = $this->inheritReadPreference($options);

$server = $hasWriteStage
? select_server_for_aggregate_write_stage($this->manager, $options)
: select_server($this->manager, $options);

/* MongoDB 4.2 and later supports a read concern when an $out stage is
* being used, but earlier versions do not.
*/
if (! $hasWriteStage || server_supports_feature($server, self::WIRE_VERSION_FOR_READ_CONCERN_WITH_WRITE_STAGE)) {
$options = $this->inheritReadConcern($options);
}

$options = $this->inheritReadConcern($options);
$options = $this->inheritCodecOrTypeMap($options);

if ($hasWriteStage) {
$options = $this->inheritWriteOptions($options);
}

$server = $hasWriteStage
? select_server_for_aggregate_write_stage($this->manager, $options)
: select_server($this->manager, $options);

$operation = new Aggregate($this->databaseName, $this->collectionName, $pipeline, $options);

return $operation->execute($server);
Expand Down
5 changes: 1 addition & 4 deletions src/Database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ class Database implements Stringable
'root' => BSONDocument::class,
];

private const WIRE_VERSION_FOR_READ_CONCERN_WITH_WRITE_STAGE = 8;

/** @psalm-var Encoder<array|stdClass|Document|PackedArray, mixed> */
private readonly Encoder $builderEncoder;

Expand Down Expand Up @@ -229,8 +227,7 @@ public function aggregate(array|Pipeline $pipeline, array $options = []): Cursor
*/
if (
! isset($options['readConcern']) &&
! is_in_transaction($options) &&
( ! $hasWriteStage || server_supports_feature($server, self::WIRE_VERSION_FOR_READ_CONCERN_WITH_WRITE_STAGE))
! is_in_transaction($options)
) {
$options['readConcern'] = $this->readConcern;
}
Expand Down
8 changes: 0 additions & 8 deletions src/Operation/FindAndModify.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ final class FindAndModify implements Explainable
{
private const WIRE_VERSION_FOR_HINT = 9;

private const WIRE_VERSION_FOR_UNSUPPORTED_OPTION_SERVER_SIDE_ERROR = 8;

private array $options;

/**
Expand Down Expand Up @@ -227,12 +225,6 @@ public function __construct(private string $databaseName, private string $collec
*/
public function execute(Server $server): array|object|null
{
/* Server versions >= 4.2.0 raise errors for unsupported update options.
* For previous versions, the CRUD spec requires a client-side error. */
if (isset($this->options['hint']) && ! server_supports_feature($server, self::WIRE_VERSION_FOR_UNSUPPORTED_OPTION_SERVER_SIDE_ERROR)) {
throw UnsupportedException::hintNotSupported();
Copy link
Member

@alcaeus alcaeus Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Is this method still used after we remove everything? Not sure if we consider this public API, but we can remove the method if we consider it private and it's no longer used.

Copy link
Member Author

@jmikola jmikola Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still used by the Delete operation: https://github.com/mongodb/mongo-php-library/blob/2.1.2/src/Operation/Delete.php#L126-L133

I don't feel strongly about removing it after we raise the minimum server version to 4.4+, but I'm amenable to leaving it around and deprecated since it's technically public.

}

/* CRUD spec requires a client-side error when using "hint" with an
* unacknowledged write concern on an unsupported server. */
if (
Expand Down
13 changes: 0 additions & 13 deletions src/Operation/Update.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@
use function MongoDB\is_document;
use function MongoDB\is_first_key_operator;
use function MongoDB\is_pipeline;
use function MongoDB\is_write_concern_acknowledged;
use function MongoDB\server_supports_feature;

/**
* Operation for the update command.
Expand All @@ -46,8 +44,6 @@
*/
final class Update implements Explainable
{
private const WIRE_VERSION_FOR_HINT = 8;

private array $options;

/**
Expand Down Expand Up @@ -176,15 +172,6 @@ public function __construct(private string $databaseName, private string $collec
*/
public function execute(Server $server): UpdateResult
{
/* CRUD spec requires a client-side error when using "hint" with an
* unacknowledged write concern on an unsupported server. */
if (
isset($this->options['writeConcern']) && ! is_write_concern_acknowledged($this->options['writeConcern']) &&
isset($this->options['hint']) && ! server_supports_feature($server, self::WIRE_VERSION_FOR_HINT)
) {
throw UnsupportedException::hintNotSupported();
}

$inTransaction = isset($this->options['session']) && $this->options['session']->isInTransaction();
if ($inTransaction && isset($this->options['writeConcern'])) {
throw UnsupportedException::writeConcernNotSupportedInTransaction();
Expand Down
4 changes: 0 additions & 4 deletions tests/Collection/BuilderCollectionFunctionalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,6 @@ public function testUpdateOne(): void

public function testUpdateWithPipeline(): void
{
$this->skipIfServerVersion('<', '4.2.0', 'Pipeline-style updates are not supported');

$result = $this->collection->updateOne(
Query::query(x: Query::lt(2)),
new Pipeline(
Expand All @@ -259,8 +257,6 @@ public function testUpdateMany(): void

public function testUpdateManyWithPipeline(): void
{
$this->skipIfServerVersion('<', '4.2.0', 'Pipeline-style updates are not supported');

$result = $this->collection->updateMany(
Query::query(x: Query::gt(1)),
new Pipeline(
Expand Down
22 changes: 10 additions & 12 deletions tests/FunctionalTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,10 +469,6 @@ protected function skipIfCausalConsistencyIsNotSupported(): void

protected function skipIfClientSideEncryptionIsNotSupported(): void
{
if (version_compare($this->getFeatureCompatibilityVersion(), '4.2', '<')) {
$this->markTestSkipped('Client Side Encryption only supported on FCV 4.2 or higher');
}

if (static::getModuleInfo('libmongocrypt') === 'disabled') {
$this->markTestSkipped('Client Side Encryption is not enabled in the MongoDB extension');
}
Expand All @@ -491,16 +487,18 @@ protected function skipIfGeoHaystackIndexIsNotSupported(): void

protected function skipIfTransactionsAreNotSupported(): void
{
if ($this->getPrimaryServer()->getType() === Server::TYPE_STANDALONE) {
$this->markTestSkipped('Transactions are not supported on standalone servers');
}
switch ($this->getPrimaryServer()->getType()) {
case Server::TYPE_STANDALONE:
$this->markTestSkipped('Transactions are not supported on standalone servers');
break;

if ($this->isShardedCluster()) {
$this->markTestSkipped('Transactions are only supported on FCV 4.2 or higher');
}
case Server::TYPE_RS_PRIMARY:
Copy link
Member

@GromNaN GromNaN Dec 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition on $this->getPrimaryServer()->getType() === Server::TYPE_RS_PRIMARY. I read the following comment, but I don't get the reason of this update.

Copy link
Member

@alcaeus alcaeus Dec 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous behaviour was to skip on a standalone server, on sharded clusters (not sure why this was unconditional, it should've only skipped on sharded clusters < 4.2), and on replica sets with a storage engine other than WiredTiger. Since we're now always on 4.2 and higher, we can skip on standalones, check for WiredTiger on a primary, and always support sharded clusters, hence the change.

Copy link
Member Author

@jmikola jmikola Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why this was unconditional, it should've only skipped on sharded clusters < 4.2

That stood out as an error to me (i.e. never running these tests on sharded clusters), but it went away with this change so I didn't call it out.

The new logic assumes WiredTiger on sharded clusters since we can't ask mongos directly (similar to what was already done for casual consistency checks). We have logic elsewhere (or did at one point) to punch through to individual servers in the sharded cluster configuration but that seems like overkill given we know exactly what environments we test on (and the only options going forward are in-memory and WT).

// Note: mongos does not report storage engine information
if ($this->getServerStorageEngine() !== 'wiredTiger') {
$this->markTestSkipped('Transactions require WiredTiger storage engine');
}

if ($this->getServerStorageEngine() !== 'wiredTiger') {
$this->markTestSkipped('Transactions require WiredTiger storage engine');
break;
}
}

Expand Down
8 changes: 0 additions & 8 deletions tests/Operation/BulkWriteFunctionalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@
use PHPUnit\Framework\Attributes\Depends;
use stdClass;

use function is_array;

class BulkWriteFunctionalTest extends FunctionalTestCase
{
private Collection $collection;
Expand Down Expand Up @@ -198,10 +196,6 @@ function (array $event) use ($expectedReplacement): void {
#[DataProvider('provideUpdatePipelines')]
public function testUpdateDocuments($update, $expectedUpdate): void
{
if (is_array($expectedUpdate)) {
$this->skipIfServerVersion('<', '4.2.0', 'Pipeline-style updates are not supported');
}

(new CommandObserver())->observe(
function () use ($update): void {
$operation = new BulkWrite(
Expand Down Expand Up @@ -423,8 +417,6 @@ function (array $event): void {

public function testBulkWriteWithPipelineUpdates(): void
{
$this->skipIfServerVersion('<', '4.2.0', 'Pipeline-style updates are not supported');

$this->createFixtures(4);

$ops = [
Expand Down
2 changes: 0 additions & 2 deletions tests/Operation/ExplainFunctionalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,6 @@ public function testAggregate(): void
#[DataProvider('provideVerbosityInformation')]
public function testAggregateOptimizedToQuery($verbosity, $executionStatsExpected, $allPlansExecutionExpected): void
{
$this->skipIfServerVersion('<', '4.2.0', 'MongoDB < 4.2 does not optimize simple aggregation pipelines');

$this->createFixtures(3);

$pipeline = [['$match' => ['_id' => ['$ne' => 2]]]];
Expand Down
16 changes: 0 additions & 16 deletions tests/Operation/FindAndModifyFunctionalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,22 +126,6 @@ function (array $event): void {
);
}

public function testHintOptionUnsupportedClientSideError(): void
{
$this->skipIfServerVersion('>=', '4.2.0', 'server reports error for unsupported findAndModify options');

$operation = new FindAndModify(
$this->getDatabaseName(),
$this->getCollectionName(),
['remove' => true, 'hint' => '_id_'],
);

$this->expectException(UnsupportedException::class);
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');

$operation->execute($this->getPrimaryServer());
}

public function testHintOptionAndUnacknowledgedWriteConcernUnsupportedClientSideError(): void
{
$this->skipIfServerVersion('>=', '4.4.0', 'hint is supported');
Expand Down
25 changes: 0 additions & 25 deletions tests/Operation/UpdateFunctionalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,13 @@
use MongoDB\Driver\BulkWrite;
use MongoDB\Driver\Exception\LogicException;
use MongoDB\Driver\WriteConcern;
use MongoDB\Exception\UnsupportedException;
use MongoDB\Operation\Update;
use MongoDB\Tests\CommandObserver;
use MongoDB\UpdateResult;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Depends;
use stdClass;

use function is_array;

class UpdateFunctionalTest extends FunctionalTestCase
{
private Collection $collection;
Expand Down Expand Up @@ -54,10 +51,6 @@ function (array $event) use ($expectedFilter): void {
#[DataProvider('provideReplacementDocumentLikePipeline')]
public function testUpdateDocuments($update, $expectedUpdate): void
{
if (is_array($expectedUpdate)) {
$this->skipIfServerVersion('<', '4.2.0', 'Pipeline-style updates are not supported');
}

(new CommandObserver())->observe(
function () use ($update): void {
$operation = new Update(
Expand Down Expand Up @@ -148,24 +141,6 @@ function (array $event): void {
);
}

public function testHintOptionAndUnacknowledgedWriteConcernUnsupportedClientSideError(): void
{
$this->skipIfServerVersion('>=', '4.2.0', 'hint is supported');

$operation = new Update(
$this->getDatabaseName(),
$this->getCollectionName(),
['_id' => 1],
['$inc' => ['x' => 1]],
['hint' => '_id_', 'writeConcern' => new WriteConcern(0)],
);

$this->expectException(UnsupportedException::class);
$this->expectExceptionMessage('Hint is not supported by the server executing this operation');

$operation->execute($this->getPrimaryServer());
}

public function testUpdateOne(): void
{
$this->createFixtures(3);
Expand Down
37 changes: 0 additions & 37 deletions tests/SpecTests/PrimaryStepDownSpecTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,43 +72,6 @@ public function testNotPrimaryKeepsConnectionPool(): void
$this->assertSame($totalConnectionsCreated, $this->getTotalConnectionsCreated());
}

/** @see https://github.com/mongodb/specifications/tree/master/source/connections-survive-step-down/tests#not-primary-reset-connection-pool */
public function testNotPrimaryResetConnectionPool(): void
{
$runOn = [(object) ['minServerVersion' => '4.0.0', 'maxServerVersion' => '4.0.999', 'topology' => [self::TOPOLOGY_REPLICASET]]];
$this->checkServerRequirements($runOn);

// Set a fail point
$this->configureFailPoint([
'configureFailPoint' => 'failCommand',
'mode' => ['times' => 1],
'data' => [
'failCommands' => ['insert'],
'errorCode' => self::NOT_PRIMARY,
],
]);

$totalConnectionsCreated = $this->getTotalConnectionsCreated();

// Execute an insert into the test collection of a {test: 1} document.
try {
$this->insertDocuments(1);
} catch (BulkWriteException $e) {
// Verify that the insert failed with an operation failure with 10107 code.
$this->assertSame(self::NOT_PRIMARY, $e->getCode());
}

/* Verify that the connection pool has been cleared and that a new
* connection has been created. Use ">=" to allow for the possibility
* that the server created additional connections unrelated to this
* test. */
$this->assertGreaterThanOrEqual($totalConnectionsCreated + 1, $this->getTotalConnectionsCreated());

// Execute an insert into the test collection of a {test: 1} document and verify that it succeeds.
$result = $this->insertDocuments(1);
$this->assertSame(1, $result->getInsertedCount());
}

/** @see https://github.com/mongodb/specifications/tree/master/source/connections-survive-step-down/tests#shutdown-in-progress-reset-connection-pool */
public function testShutdownResetConnectionPool(): void
{
Expand Down
6 changes: 0 additions & 6 deletions tests/UnifiedSpecTests/UnifiedTestRunner.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,12 +337,6 @@ private function isAuthenticated(): bool
*/
private function isClientSideEncryptionSupported(): bool
{
/* CSFLE technically requires FCV 4.2+ but this is sufficient since we
* do not test on mixed-version clusters. */
if (version_compare($this->getServerVersion(), '4.2', '<')) {
return false;
}

if (FunctionalTestCase::getModuleInfo('libmongocrypt') === 'disabled') {
return false;
}
Expand Down