Skip to content
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

Jump to bottom

Deprecate functionality to be removed #1441

Open
wants to merge 2 commits into
base: v1.20
Choose a base branch
from
+39 −7
Open

Deprecate functionality to be removed #1441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
84a94de
PHPLIB-1532: Emit deprecation notice for modifiers and oplogReplay op…
alcaeus Sep 23, 2024
8c6988e
PHPLIB-1533: Mark Collection::mapReduce as deprecated
alcaeus Sep 23, 2024
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
6 changes: 6 additions & 0 deletions src/Collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,11 @@
use function array_key_exists;
use function current;
use function is_array;
use function sprintf;
use function strlen;
use function trigger_error;

use const E_USER_DEPRECATED;

class Collection
{
Expand Down Expand Up @@ -927,6 +931,8 @@ public function listSearchIndexes(array $options = []): Iterator
*/
public function mapReduce(JavascriptInterface $map, JavascriptInterface $reduce, $out, array $options = [])
{
@trigger_error(sprintf('The %s method is deprecated and will be removed in a future release.', __METHOD__), E_USER_DEPRECATED);
Copy link
Member

Choose a reason for hiding this comment

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

Will you remove it in version 2.0? In that case, you can mention the version here.

Suggested change
@trigger_error(sprintf('The %s method is deprecated and will be removed in a future release.', __METHOD__), E_USER_DEPRECATED);
@trigger_error(sprintf('The %s method is deprecated and will be removed in version 2.0.', __METHOD__), E_USER_DEPRECATED);


$hasOutputCollection = ! is_mapreduce_output_inline($out);

// Check if the out option is inline because we will want to coerce a primary read preference if not
Expand Down
16 changes: 12 additions & 4 deletions src/Operation/Find.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,20 @@ public function __construct(string $databaseName, string $collectionName, $filte
unset($options['readConcern']);
}

if (isset($options['snapshot'])) {
trigger_error('The "snapshot" option is deprecated and will be removed in a future release', E_USER_DEPRECATED);
if (isset($options['maxScan'])) {
@trigger_error('The "maxScan" option is deprecated and will be removed in a future release', E_USER_DEPRECATED);
}

if (isset($options['maxScan'])) {
trigger_error('The "maxScan" option is deprecated and will be removed in a future release', E_USER_DEPRECATED);
if (isset($options['modifiers'])) {
@trigger_error('The "modifiers" option is deprecated and will be removed in a future release', E_USER_DEPRECATED);
}

if (isset($options['oplogReplay'])) {
@trigger_error('The "oplogReplay" option is deprecated and will be removed in a future release', E_USER_DEPRECATED);
}
Copy link
Member

@jmikola jmikola Sep 23, 2024 •
edited by alcaeus
Loading

Choose a reason for hiding this comment

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

Excluding modifiers, PHPC raises its own E_DEPRECATED notice these options in the Query constructor. Does that mean users would see two deprecation notices?

As for modifiers, that seems like something we should add in PHPC 1.x before we remove the option entirely in 2.0 (PHPC-2440). I created PHPC-2452 and opened mongodb/mongo-php-driver#1680 to address that.

Copy link
Member Author

@alcaeus alcaeus Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, that would cause duplicate deprecation notices, so we probably should remove the extra notices thrown in PHPLIB. I'll comment on mongodb/mongo-php-driver#1680 regarding the modifier deprecations, but I'll remove the query option deprecations entirely in this PR.


if (isset($options['snapshot'])) {
@trigger_error('The "snapshot" option is deprecated and will be removed in a future release', E_USER_DEPRECATED);
}

if (isset($options['codec']) && isset($options['typeMap'])) {
Expand Down
4 changes: 3 additions & 1 deletion tests/Collection/CollectionFunctionalTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,9 @@ public function testMapReduce(): void
$reduce = new Javascript('function(key, values) { return Array.sum(values); }');
$out = ['inline' => 1];

$result = $this->collection->mapReduce($map, $reduce, $out);
$result = $this->assertDeprecated(
fn () => $this->collection->mapReduce($map, $reduce, $out),
);

$this->assertInstanceOf(MapReduceResult::class, $result);
$expected = [
Expand Down
14 changes: 14 additions & 0 deletions tests/Operation/FindTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,20 @@ public function testMaxScanOptionIsDeprecated(): void
});
}

public function testModifiersOptionIsDeprecated(): void
{
$this->assertDeprecated(function (): void {
new Find($this->getDatabaseName(), $this->getCollectionName(), [], ['modifiers' => []]);
});
}

public function testOplogReplayOptionIsDeprecated(): void
{
$this->assertDeprecated(function (): void {
new Find($this->getDatabaseName(), $this->getCollectionName(), [], ['oplogReplay' => true]);
});
}

/** @dataProvider provideInvalidConstructorCursorTypeOptions */
public function testConstructorCursorTypeOption($cursorType): void
{
Expand Down
6 changes: 4 additions & 2 deletions tests/TestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ public function provideInvalidStringValues(): array
return $this->wrapValuesForDataProvider($this->getInvalidStringValues());
}

protected function assertDeprecated(callable $execution): void
protected function assertDeprecated(callable $execution)
{
$errors = [];

Expand All @@ -174,12 +174,14 @@ protected function assertDeprecated(callable $execution): void
}, E_USER_DEPRECATED);

try {
call_user_func($execution);
$result = call_user_func($execution);
} finally {
restore_error_handler();
}

$this->assertCount(1, $errors);

return $result;
}

protected function createOptionDataProvider(array $options): array
Expand Down