PHPLIB-1505 Add driver option `builderEncoder` by GromNaN · Pull Request #1382 · mongodb/mongo-php-library

GromNaN · 2024-09-09T20:59:51Z

The new option builderEncoder contains a MongoDB\Codec\Encoder object that will be used to encode queries and pipelines. When not set, it defaults to a new instance of MongoDB\Builder\BuilderEncoder.

GromNaN · 2024-09-10T07:26:41Z

tests/ClientTest.php

-        $client = new Client(static::getUri(), $uriOptions);
+        $driverOptions = [
+            'builderEncoder' => $builderEncoder = $this->createMock(Encoder::class),
+            'typeMap' => ['root' => 'array'],


typeMap option was not tested here.

GromNaN · 2024-09-10T07:28:03Z

tests/TestCase.php

    protected function getInvalidArrayValues(bool $includeNull = false): array
    {
-        return array_merge([123, 3.14, 'foo', true, new stdClass()], $includeNull ? [null] : []);
+        return [123, 3.14, 'foo', true, new stdClass(), ...($includeNull ? [null] : [])];


Reformatting data provider using array unpacking.

I feel bad for forgetting PHP has this feature now 😅

src/Client.php

alcaeus

I'd suggest making the encoder property readonly, but LGTM other than that.

alcaeus · 2024-09-10T11:32:17Z

src/Client.php

     *
     * Supported driver-specific options:
     *
+     *  * builderEncoder (MongoDB\Builder\Encoder): Encoder for query builder


Suggestion to call out there's a default value being used:

Suggested change

* * builderEncoder (MongoDB\Builder\Encoder): Encoder for query builder

* * builderEncoder (MongoDB\Builder\Encoder): Encoder for query and aggregation builders. If not given, the default encoder will be used.

If you accept this, it would make sense to apply this to the Database and Collection classes as well.

alcaeus · 2024-09-10T11:34:41Z

src/Client.php


    private array $typeMap;

+    private Encoder $builderEncoder;


PHP 8.1 allows us to make this readonly:

Suggested change

private Encoder $builderEncoder;

private readonly Encoder $builderEncoder;

Also, just thought of this after seeing the Psalm errors in the follow-up PR, but specifying a template for the Encoder might be helpful:

Suggested change

private Encoder $builderEncoder;

/** @psalm-var Encoder<array|stdClass|Document|PackedArray, mixed> */

private readonly Encoder $builderEncoder;

I had to check the Encoder interface template types. Seems correct.

src/Collection.php

src/Database.php

alcaeus · 2024-09-10T11:36:37Z

tests/TestCase.php

    protected function getInvalidArrayValues(bool $includeNull = false): array
    {
-        return array_merge([123, 3.14, 'foo', true, new stdClass()], $includeNull ? [null] : []);
+        return [123, 3.14, 'foo', true, new stdClass(), ...($includeNull ? [null] : [])];


I feel bad for forgetting PHP has this feature now 😅

src/Client.php

        $driverOptions['driver'] = $this->mergeDriverInfo($driverOptions['driver'] ?? []);

        $this->uri = $uri ?? self::DEFAULT_URI;
+        $this->builderEncoder = $driverOptions['builderEncoder'] ?? new BuilderEncoder();


src/Collection.php

            throw InvalidArgumentException::invalidType('"writeConcern" option', $options['writeConcern'], WriteConcern::class);
        }

+        $this->builderEncoder = $options['builderEncoder'] ?? new BuilderEncoder();


src/Database.php

            throw InvalidArgumentException::invalidType('"writeConcern" option', $options['writeConcern'], WriteConcern::class);
        }

+        $this->builderEncoder = $options['builderEncoder'] ?? new BuilderEncoder();


* v1.x: (87 commits) Performance: Keep collections and indexes between GridFS tests (#1421) Add final annotations to non-internal Operation classes (#1410) Fix types accepted by $round (#1401) Replace arrayHasKey with assertArrayHasKey in tests (#1403) PHPLIB-1514 Make data providers static (#1404) PHPLIB-1515 Replace assertObjectHasAttribute with assertObjectHasProperty (#1405) Restore Prose22_RangeExplicitEncryptionTest (#1400) Remove Prose22_RangeExplicitEncryptionTest that requires ext-mongodb < 1.20 (#1394) Use `match` instead of `switch` when a simple value is returned (#1393) Remove PHPUnit functions polyfill (#1395) Update branch names for GHA workflows (#1390) PHPLIB-1419 Encode Agg builder objects in Collection methods (#1383) PHPLIB-1420 Integrate query builder for non-aggregation APIs (#1385) Fix optional parameter declared before required parameter (#1384) PHPLIB-1505 Add driver option "builderEncoder" (#1382) Exclude rector.php from the artifact Check generated files are up-to-date Replace composer package mongodb/builder Skip Pedentry method sort for generated files Remove composer constraints already imposed by the main package ...

* v1.x: (90 commits) Merge v1.20 into v1.x (#1447) PHPLIB-1525 Removes dependency to Symfony PHPUnit bridge (#1413) Change deprecated assertObjectHasAttribute to assertObjectHasProperty (#1432) Performance: Keep collections and indexes between GridFS tests (#1421) Add final annotations to non-internal Operation classes (#1410) Fix types accepted by $round (#1401) Replace arrayHasKey with assertArrayHasKey in tests (#1403) PHPLIB-1514 Make data providers static (#1404) PHPLIB-1515 Replace assertObjectHasAttribute with assertObjectHasProperty (#1405) Restore Prose22_RangeExplicitEncryptionTest (#1400) Remove Prose22_RangeExplicitEncryptionTest that requires ext-mongodb < 1.20 (#1394) Use `match` instead of `switch` when a simple value is returned (#1393) Remove PHPUnit functions polyfill (#1395) Update branch names for GHA workflows (#1390) PHPLIB-1419 Encode Agg builder objects in Collection methods (#1383) PHPLIB-1420 Integrate query builder for non-aggregation APIs (#1385) Fix optional parameter declared before required parameter (#1384) PHPLIB-1505 Add driver option "builderEncoder" (#1382) Exclude rector.php from the artifact Check generated files are up-to-date ...

GromNaN commented Sep 10, 2024

View reviewed changes

src/Client.php Show resolved Hide resolved

GromNaN commented Sep 10, 2024

View reviewed changes

src/Client.php Outdated Show resolved Hide resolved

GromNaN marked this pull request as ready for review September 10, 2024 07:31

GromNaN requested a review from a team as a code owner September 10, 2024 07:31

GromNaN requested a review from alcaeus September 10, 2024 07:31

GromNaN force-pushed the PHPLIB-1505 branch from 55bbda4 to fe5e143 Compare September 10, 2024 08:51

GromNaN mentioned this pull request Sep 10, 2024

PHPLIB-1419 Encode Agg builder objects in Collection methods #1383

Merged

GromNaN force-pushed the PHPLIB-1505 branch from fe5e143 to 43d6c74 Compare September 10, 2024 09:12

alcaeus requested changes Sep 10, 2024

View reviewed changes

GromNaN force-pushed the PHPLIB-1505 branch from 43d6c74 to c8571a7 Compare September 10, 2024 13:16

GromNaN requested a review from alcaeus September 10, 2024 13:18

alcaeus approved these changes Sep 10, 2024

View reviewed changes

PHPLIB-1505 Add driver option "builderEncoder"

c767ffe

GromNaN force-pushed the PHPLIB-1505 branch from c8571a7 to c767ffe Compare September 10, 2024 15:08

GromNaN enabled auto-merge (squash) September 10, 2024 15:08

github-advanced-security bot found potential problems Sep 10, 2024

View reviewed changes

GromNaN merged commit be2df01 into mongodb:master Sep 10, 2024

GromNaN deleted the PHPLIB-1505 branch September 11, 2024 06:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PHPLIB-1505 Add driver option `builderEncoder`#1382

PHPLIB-1505 Add driver option `builderEncoder`#1382
GromNaN merged 1 commit intomongodb:masterfrom
GromNaN:PHPLIB-1505

GromNaN commented Sep 9, 2024

Uh oh!

GromNaN Sep 10, 2024

Uh oh!

GromNaN Sep 10, 2024 •

edited

Loading

Uh oh!

alcaeus Sep 10, 2024

Uh oh!

Uh oh!

Uh oh!

alcaeus left a comment

Uh oh!

alcaeus Sep 10, 2024

Uh oh!

alcaeus Sep 10, 2024

Uh oh!

alcaeus Sep 10, 2024

Uh oh!

GromNaN Sep 10, 2024

Uh oh!

Uh oh!

Uh oh!

alcaeus Sep 10, 2024

Uh oh!

Check notice

Check notice

Check notice

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	* * builderEncoder (MongoDB\Builder\Encoder): Encoder for query builder
	* * builderEncoder (MongoDB\Builder\Encoder): Encoder for query and aggregation builders. If not given, the default encoder will be used.

	private Encoder $builderEncoder;
	private readonly Encoder $builderEncoder;

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

Conversation

GromNaN commented Sep 9, 2024

Uh oh!

GromNaN Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

GromNaN Sep 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alcaeus Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alcaeus left a comment

Choose a reason for hiding this comment

Uh oh!

alcaeus Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

alcaeus Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

alcaeus Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

GromNaN Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

alcaeus Sep 10, 2024

Choose a reason for hiding this comment

Uh oh!

Check notice

Check notice

Check notice

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GromNaN Sep 10, 2024 •

edited

Loading