Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Typo in variable for ExtMongoDbResourceManager adapter makes passing db a no-op #181

Closed
1 task done
TysonAndre opened this issue Mar 24, 2019 · 3 comments · Fixed by #182
Closed
1 task done

Typo in variable for ExtMongoDbResourceManager adapter makes passing db a no-op #181

TysonAndre opened this issue Mar 24, 2019 · 3 comments · Fixed by #182
Labels
Milestone

Comments

@TysonAndre
Copy link
Contributor

  • I was not able to find an open or closed issue matching what I'm seeing.

The latest commit on master uses isset($resouce), which is a typo (and an undeclared variable). So selectCollection is always passed 'zend' as the first argument (noticed when running codespell)

Attempting to fix this may break existing cache installations - I'm not sure what the impact is because I'm not familiar with use cases.

--- a/src/Storage/Adapter/ExtMongoDbResourceManager.php
+++ b/src/Storage/Adapter/ExtMongoDbResourceManager.php
@@ -98,7 +98,7 @@ class ExtMongoDbResourceManager
                 }
 
                 $collection = $resource['client_instance']->selectCollection(
-                    isset($resouce['db']) ? $resource['db'] : 'zend',
+                    isset($resource['db']) ? $resource['db'] : 'zend',
                     isset($resource['collection']) ? $resource['collection'] : 'cache'
                 );

Code to reproduce the issue

Creating resources with a non-default db (e.g. 'db1', 'db2') and managing them with this cache would instead get created in the 'zend' db when using ExtMongoDbResourceManager but not MongoDbResourceManager and unexpectedly affect what was intended to be a different db?

(I'm not familiar enough with this library to be sure of that)

Expected results

Actual results

@TysonAndre
Copy link
Contributor Author

@weierophinney - Any ideas on what the impact of fixing the typo would be (e.g. would applications using this library lose data)?

@weierophinney
Copy link
Member

They wouldn't lose data - but if they had configured the database name, they'd now be hitting a different database. If they want to use the original, they'd have to either copy records to the correct database, or change their configuration to point to the "zend" database.

Personally, I consider this a bugfix. We'd simply need to address the issue in the CHANGELOG with the above options when we release.

Could you submit a PR, please?

@glennschmidt
Copy link

Looking forward to this fix being pulled

@michalbundyra michalbundyra added this to the 2.8.3 milestone Aug 14, 2019
michalbundyra added a commit that referenced this issue Aug 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants