Skip to content
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
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
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ language: python
services:
- mongodb

install: python -m pip install https://github.com/mongodb/mongo-python-driver/archive/3.11.0rc0.tar.gz

python:
- 3.5
- 3.6
Expand Down
110 changes: 82 additions & 28 deletions pymongoexplain/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,20 @@

from bson.son import SON
from pymongo.collection import Collection
from pymongo.helpers import _index_document
from pymongo.collation import validate_collation_or_none
from .utils import convert_to_camelcase


Document = Union[dict, SON]


class BaseCommand():
def __init__(self, collection):
def __init__(self, collection, collation):
self.command_document = {}
collation = validate_collation_or_none(collation)
if collation is not None:
self.command_document["collation"] = collation
self.collection = collection

@property
Expand All @@ -44,16 +49,40 @@ def get_SON(self):

class UpdateCommand(BaseCommand):
def __init__(self, collection: Collection, filter, update,
kwargs):
super().__init__(collection.name)
return_document = {"updates":[{"q": filter, "u": update}]}
for key in kwargs:
value = kwargs[key]
if key == "bypass_document_validation":
return_document[key] = value
else:
return_document["updates"][0][key] = value
self.command_document = convert_to_camelcase(return_document)
upsert=None, multi=None, collation=None, array_filters=None,
hint=None, ordered=None, write_concern=None,
bypass_document_validation=None, comment=None):
super().__init__(collection.name, collation)
self.command_document.update({
"updates":[{"q": filter, "u": update}]
})
if upsert is not None:
self.command_document["updates"][0]["upsert"] = upsert
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest refactoring this to:

update_doc = {"q": filter, "u": update}
self.command_document["updates"] = [update_doc]
if upsert is not None:
   update_doc["upsert"] = upsert
...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see this change. Did it get lost?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I refactored it incorrectly, it is done now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I still don't think this is complete. We can replace self.command_document["updates"][0] with update_doc now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


if multi is not None:
self.command_document["updates"][0]["multi"] = multi

if array_filters is not None:
self.command_document["updates"][0]["array_filters"] = array_filters

if hint is not None:
self.command_document["updates"][0]["hint"] = hint if \
isinstance(hint, str) else _index_document(hint)

if ordered is not None:
self.command_document["ordered"] = ordered

if write_concern is not None:
self.command_document["write_concern"] = write_concern

if bypass_document_validation is not None and \
bypass_document_validation is not False:
self.command_document["bypass_document_validation"] = bypass_document_validation

if comment is not None:
self.command_document["comment"] = comment

self.command_document = convert_to_camelcase(self.command_document)

@property
def command_name(self):
Expand All @@ -63,10 +92,11 @@ def command_name(self):
class DistinctCommand(BaseCommand):
def __init__(self, collection: Collection, key, filter, session,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do some of these classes accept a session but some don't? I think they should all be consistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

kwargs):
super().__init__(collection.name)
self.command_document = {"key": key, "query": filter}
for key, value in kwargs.items():
self.command_document[key] = value
super().__init__(collection.name, kwargs.get("collation", None))
self.command_document.update({"key": key, "query": filter})
if kwargs.get("read_concern", None) is not None:
self.command_document["read_concern"] = kwargs["read_concern"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

read_concern is not an option for distinct so we can remove this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.


self.command_document = convert_to_camelcase(self.command_document)

@property
Expand All @@ -77,24 +107,33 @@ def command_name(self):
class AggregateCommand(BaseCommand):
def __init__(self, collection: Collection, pipeline, session,
cursor_options,
kwargs, exclude_keys = []):
super().__init__(collection.name)
self.command_document = {"pipeline": pipeline, "cursor": cursor_options}
kwargs):

super().__init__(collection.name, kwargs.get("collation", None))
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should be using kwargs.pop("collation", None) instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

self.command_document.update({"pipeline": pipeline, "cursor":
cursor_options})

for key, value in kwargs.items():
self.command_document[key] = value
if key == "batchSize":
if value == 0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to check for None too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None values are already removed in the converting to snakecase step

continue
self.command_document["cursor"]["batchSize"] = value
else:
self.command_document[key] = value

self.command_document = convert_to_camelcase(
self.command_document, exclude_keys=exclude_keys)
self.command_document)

@property
def command_name(self):
return "aggregate"


class CountCommand(BaseCommand):
def __init__(self, collection: Collection, filter,
kwargs):
super().__init__(collection.name)
self.command_document = {"query": filter}
super().__init__(collection.name, kwargs.get("collation", None))
self.command_document.update({"query": filter})
for key, value in kwargs.items():
self.command_document[key] = value
self.command_document = convert_to_camelcase(self.command_document)
Expand All @@ -107,7 +146,7 @@ def command_name(self):
class FindCommand(BaseCommand):
def __init__(self, collection: Collection,
kwargs):
super().__init__(collection.name)
super().__init__(collection.name, kwargs.get("collation", None))
for key, value in kwargs.items():
self.command_document[key] = value
self.command_document = convert_to_camelcase(self.command_document)
Expand All @@ -116,12 +155,21 @@ def __init__(self, collection: Collection,
def command_name(self):
return "find"


class FindAndModifyCommand(BaseCommand):
def __init__(self, collection: Collection,
kwargs):
super().__init__(collection.name)
super().__init__(collection.name, kwargs.get("collation", None))
for key, value in kwargs.items():
self.command_document[key] = value
if key == "update" and kwargs.get("replacement", None) is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you can remove all the "replacement" references here because we never pass "replacement" through kwargs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replacement is passed through kwargs for find_one_and_replace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind, I was confused. Fixed it now.

continue
if key == "hint":
self.command_document["hint"] = value if \
isinstance(value, str) else _index_document(value)
elif key == "replacement":
self.command_document["update"] = value
else:
self.command_document[key] = value
self.command_document = convert_to_camelcase(self.command_document)

@property
Expand All @@ -132,10 +180,16 @@ def command_name(self):
class DeleteCommand(BaseCommand):
def __init__(self, collection: Collection, filter,
limit, collation, kwargs):
super().__init__(collection.name)
self.command_document = {"deletes": [SON({"q": filter, "limit": limit})]}
super().__init__(collection.name, kwargs.get("collation", None))
self.command_document.update({"deletes": [SON({"q": filter, "limit":
limit})]})
for key, value in kwargs.items():
self.command_document[key] = value
if key == "hint":
self.command_document["deletes"][0]["hint"] = value if \
isinstance(value, str) else _index_document(value)
else:
self.command_document[key] = value

self.command_document = convert_to_camelcase(self.command_document)

@property
Expand Down
56 changes: 26 additions & 30 deletions pymongoexplain/explainable_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@
from typing import Union, List, Dict

import pymongo
from pymongo.collection import Collection
from bson.son import SON

from .commands import AggregateCommand, FindCommand, CountCommand, \
UpdateCommand, DistinctCommand, DeleteCommand, FindAndModifyCommand

Document = Union[dict, SON]


class ExplainCollection():
def __init__(self, collection):
self.collection = collection
Expand All @@ -39,22 +41,19 @@ def update_one(self, filter, update, upsert=False,
bypass_document_validation=False,
collation=None, array_filters=None, hint=None,
session=None, **kwargs):
kwargs.update(locals())
del kwargs["self"], kwargs["kwargs"], kwargs["filter"], kwargs["update"]
kwargs["multi"] = False
if bypass_document_validation == False:
del kwargs["bypass_document_validation"]
command = UpdateCommand(self.collection, filter, update, kwargs)
command = UpdateCommand(self.collection, filter, update,
bypass_document_validation=
bypass_document_validation,
array_filters=array_filters,
collation=collation, hint=hint,
upsert=upsert, multi=False)
return self._explain_command(command)

def update_many(self, filter: Document, update: Document, upsert=False,
array_filters=None, bypass_document_validation=False, collation=None, session=None, **kwargs):
kwargs.update(locals())
del kwargs["self"], kwargs["kwargs"], kwargs["filter"], kwargs["update"]
kwargs["multi"] = True
if bypass_document_validation == False:
del kwargs["bypass_document_validation"]
command = UpdateCommand(self.collection, filter, update, kwargs)
array_filters=None, bypass_document_validation=False,
collation=None, hint=None, session=None, **kwargs):
command = UpdateCommand(self.collection, filter, update, multi=True,
bypass_document_validation=bypass_document_validation, upsert=upsert, collation=collation, array_filters=array_filters, hint=hint)
return self._explain_command(command)

def distinct(self, key: str, filter: Document=None, session=None, **kwargs):
Expand All @@ -63,7 +62,7 @@ def distinct(self, key: str, filter: Document=None, session=None, **kwargs):

def aggregate(self, pipeline: List[Document], session=None, **kwargs):
command = AggregateCommand(self.collection, pipeline, session,
{},kwargs)
{}, kwargs)
return self._explain_command(command)

def estimated_document_count(self,
Expand All @@ -76,7 +75,7 @@ def count_documents(self, filter: Document, session=None,
**kwargs):

command = AggregateCommand(self.collection, [{'$match': filter},
{'$group': {'n': {'$sum': 1}, '_id': 1}}],
{'$group': {'n': {'$sum': 1}, '_id': 1}}],
session, {}, kwargs,
exclude_keys=filter.keys())
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is exclude_keys?

return self._explain_command(command)
Expand All @@ -95,7 +94,7 @@ def delete_many(self, filter: Document, collation=None,
limit = 0
kwargs["session"] = session
command = DeleteCommand(self.collection, filter, limit, collation,
kwargs)
kwargs)
return self._explain_command(command)

def watch(self, pipeline: Document = None, full_document: Document = None,
Expand Down Expand Up @@ -124,7 +123,8 @@ def find(self, filter: Document = None,
kwargs.update(locals())
del kwargs["self"], kwargs["kwargs"]
command = FindCommand(self.collection,
kwargs)
kwargs)

return self._explain_command(command)

def find_one(self, filter: Document = None, **kwargs: Dict[str,
Expand All @@ -149,29 +149,30 @@ def find_one_and_delete(self, filter: Document, projection: list = None,
kwargs)
return self._explain_command(command)

def find_one_and_replace(self, filter: Document, replacement: Document,
def find_one_and_replace(self, filter: Document, update:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This argument should still be called "replacement", not "update". It needs to match Pymongo's find_one_and_replace

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Document={},
projection: list = None, sort=None,
return_document=pymongo.ReturnDocument.BEFORE,
session=None, **kwargs):
kwargs["query"] = filter
kwargs["fields"] = projection
kwargs["sort"] = sort
kwargs["new"] = False
kwargs["update"] = replacement
kwargs["update"] = update
kwargs["session"] = session
command = FindAndModifyCommand(self.collection,
kwargs)
return self._explain_command(command)

def find_one_and_update(self, filter: Document, replacement: Document,
def find_one_and_update(self, filter: Document, update: Document,
projection: list = None, sort=None,
return_document=pymongo.ReturnDocument.BEFORE,
session=None, **kwargs):
kwargs["query"] = filter
kwargs["fields"] = projection
kwargs["sort"] = sort
kwargs["upsert"] = False
kwargs["update"] = replacement
kwargs["update"] = update
kwargs["session"] = session

command = FindAndModifyCommand(self.collection,
Expand All @@ -180,15 +181,10 @@ def find_one_and_update(self, filter: Document, replacement: Document,

def replace_one(self, filter: Document, replacement: Document,
upsert=False, bypass_document_validation=False,
collation=None, session=None, **kwargs):
kwargs.update(locals())
del kwargs["self"], kwargs["kwargs"], kwargs["filter"], kwargs[
"replacement"]
kwargs["multi"] = False
if not bypass_document_validation:
del kwargs["bypass_document_validation"]
update = replacement
command = UpdateCommand(self.collection, filter, update, kwargs)
collation=None, hint=None, session=None, **kwargs):
command = UpdateCommand(self.collection, filter, update=replacement,
bypass_document_validation=bypass_document_validation,
hint=hint, collation=collation, multi=False, upsert=upsert)

return self._explain_command(command)

Expand Down
4 changes: 2 additions & 2 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@
'console_scripts': [
'pymongoexplain=pymongoexplain.cli_explain:cli_explain'],
},
tests_require=["pymongo==3.10.1"],
install_requires=['pymongo==3.10.1'],
tests_require=["pymongo==3.11.0rc0"],
install_requires=['pymongo==3.11.0rc0'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be pymongo>=3.11

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

python_requires='>=3.5',
license="Apache License, Version 2.0",
classifiers=[
Expand Down
Loading