PHPLIB-1702: Always consult server encryptedFieldsMap when dropping collections with autoEncryption enabled
#1745
Conversation
jmikola
force-pushed
the
1.21-phplib-1702
branch
from
55ec999 to
97284ec
Compare
Necessary to fix PyMongo compat with MongoDB 4.0
| ); | ||
|
|
||
| // createEncryptedCollection should create three collections | ||
| $this->assertCount($originalNumCollections + 3, $this->database->listCollectionNames()); |
There was a problem hiding this comment.
If the collection already exists before running the test, this assertion fails.
There was a problem hiding this comment.
Is this only a concern when aborting the test locally (or retrying after a failure)?
No objections to dropping it before this first assertion.
|
|
||
| private WriteConcern $writeConcern; | ||
|
|
||
| private bool $autoEncryptionEnabled; |
There was a problem hiding this comment.
Created PHPC-2615 to allow us to fetch this from the Manager directly.
| $this->uri = $uri ?? self::DEFAULT_URI; | ||
| $this->builderEncoder = $driverOptions['builderEncoder'] ?? new BuilderEncoder(); | ||
| $this->typeMap = $driverOptions['typeMap']; | ||
| $this->autoEncryptionEnabled = isset($driverOptions['autoEncryption']['keyVaultNamespace']); |
There was a problem hiding this comment.
Noted that keyVaultNamespace is a required autoEncryption option.
phongo_manager_set_auto_encryption_opts doesn't seem to check about keyVaultNamespace being unset, so I assume we'd rely on libmongoc(rypt) raising an error since it's documented as required. In any event, it's sensible to check for it directly instead of just seeing if autoEncryption is set.
| function get_encrypted_fields_from_server(string $databaseName, string $collectionName, Server $server) | ||
| { | ||
| // No-op if the encryptedFieldsMap autoEncryption driver option was omitted | ||
| if ($manager->getEncryptedFieldsMap() === null) { |
There was a problem hiding this comment.
Ideally, it seems like we'd want the Manager to be able to report whether autoEncryption is enabled.
GromNaN
changed the title
autoEncryption enabled
Necessary to fix change stream tests for server 8.2+
https://jira.mongodb.org/browse/PHPLIB-1702
This is a rebase of #1742 on v1.21 with additional modifications from @GromNaN's code review.