diff --git a/bson/codec_options.py b/bson/codec_options.py index 2c6260373..6ea5708c1 100644 --- a/bson/codec_options.py +++ b/bson/codec_options.py @@ -43,10 +43,10 @@ class CodecOptions(_options_base): def __new__(cls, as_class=dict, tz_aware=False, uuid_representation=PYTHON_LEGACY): if not issubclass(as_class, MutableMapping): - raise TypeError("document_class must be a " - "subclass of MutableMapping") + raise TypeError("document_class must be dict, bson.son.SON, or " + "another subclass of collections.MutableMapping") if not isinstance(tz_aware, bool): - raise TypeError("tz_aware must be a boolean") + raise TypeError("tz_aware must be True or False") if uuid_representation not in ALL_UUID_REPRESENTATIONS: raise ValueError("uuid_representation must be a value " "from bson.binary.ALL_UUID_REPRESENTATIONS") diff --git a/gridfs/__init__.py b/gridfs/__init__.py index ff37636a7..5a11213eb 100644 --- a/gridfs/__init__.py +++ b/gridfs/__init__.py @@ -20,6 +20,8 @@ The :mod:`gridfs` package is an implementation of GridFS on top of .. mongodoc:: gridfs """ +from collections import Mapping + from gridfs.errors import NoFile from gridfs.grid_file import (GridIn, GridOut, @@ -275,7 +277,7 @@ class GridFS(object): - `**kwargs` (optional): any additional keyword arguments are the same as the arguments to :meth:`find`. """ - if filter is not None and not isinstance(filter, dict): + if filter is not None and not isinstance(filter, Mapping): filter = {"_id": filter} for f in self.find(filter, *args, **kwargs): diff --git a/pymongo/bulk.py b/pymongo/bulk.py index 74476458f..2820008aa 100644 --- a/pymongo/bulk.py +++ b/pymongo/bulk.py @@ -19,11 +19,12 @@ from __future__ import unicode_literals -import collections - from bson.objectid import ObjectId from bson.son import SON -from pymongo import helpers +from pymongo.common import (validate_is_mapping, + validate_is_mutable_mapping, + validate_ok_for_replace, + validate_ok_for_update) from pymongo.errors import (BulkWriteError, DocumentTooLarge, InvalidOperation, @@ -200,8 +201,7 @@ class _Bulk(object): def add_insert(self, document): """Add an insert document to the list of ops. """ - if not isinstance(document, collections.MutableMapping): - raise TypeError('document must be a mapping type.') + validate_is_mutable_mapping("document", document) # Generate ObjectId client side. if '_id' not in document: document['_id'] = ObjectId() @@ -210,7 +210,7 @@ class _Bulk(object): def add_update(self, selector, update, multi=False, upsert=False): """Create an update document and add it to the list of ops. """ - helpers._check_ok_for_update(update) + validate_ok_for_update(update) cmd = SON([('q', selector), ('u', update), ('multi', multi), ('upsert', upsert)]) self.ops.append((_UPDATE, cmd)) @@ -218,7 +218,7 @@ class _Bulk(object): def add_replace(self, selector, replacement, upsert=False): """Create a replace document and add it to the list of ops. """ - helpers._check_ok_for_replace(replacement) + validate_ok_for_replace(replacement) cmd = SON([('q', selector), ('u', replacement), ('multi', False), ('upsert', upsert)]) self.ops.append((_UPDATE, cmd)) @@ -559,8 +559,7 @@ class BulkOperationBuilder(object): - A :class:`BulkWriteOperation` instance, used to add update and remove operations to this bulk operation. """ - if not isinstance(selector, collections.Mapping): - raise TypeError('selector must be a mapping type.') + validate_is_mapping("selector", selector) return BulkWriteOperation(selector, self.__bulk) def insert(self, document): @@ -578,7 +577,6 @@ class BulkOperationBuilder(object): - write_concern (optional): the write concern for this bulk execution. """ - if (write_concern and not - isinstance(write_concern, collections.Mapping)): - raise TypeError('write_concern must be a mapping type.') + if write_concern is not None: + validate_is_mapping("write_concern", write_concern) return self.__bulk.execute(write_concern) diff --git a/pymongo/client_options.py b/pymongo/client_options.py index d5de2b3d1..1b9e7e0c8 100644 --- a/pymongo/client_options.py +++ b/pymongo/client_options.py @@ -17,7 +17,7 @@ from bson.codec_options import _parse_codec_options from bson.py3compat import iteritems from pymongo.auth import _build_credentials_tuple -from pymongo.common import validate +from pymongo.common import validate, validate_boolean from pymongo.errors import ConfigurationError from pymongo.pool import PoolOptions from pymongo.read_preferences import make_read_preference @@ -57,8 +57,8 @@ def _parse_write_concern(options): def _parse_ssl_options(options): """Parse ssl options.""" use_ssl = options.get('ssl') - if use_ssl is not None and not isinstance(use_ssl, bool): - raise TypeError("ssl must be a boolean") + if use_ssl is not None: + validate_boolean('ssl', use_ssl) certfile = options.get('ssl_certfile') keyfile = options.get('ssl_keyfile') diff --git a/pymongo/collection.py b/pymongo/collection.py index d80aa909e..4b55283c7 100644 --- a/pymongo/collection.py +++ b/pymongo/collection.py @@ -395,8 +395,7 @@ class Collection(common.BaseObject): :Returns: - An instance of :class:`~pymongo.results.InsertOneResult`. """ - if not isinstance(document, collections.MutableMapping): - raise TypeError("document must be a mutable mapping type") + common.validate_is_mutable_mapping("document", document) if "_id" not in document: document["_id"] = ObjectId() return InsertOneResult(self._insert(document), @@ -422,9 +421,7 @@ class Collection(common.BaseObject): def gen(): """A generator that validates documents and handles _ids.""" for document in documents: - if not isinstance(document, collections.MutableMapping): - raise TypeError("document must be a dict or other " - "subclass of collections.MutableMapping") + common.validate_is_mutable_mapping("document", document) if "_id" not in document: document["_id"] = ObjectId() inserted_ids.append(document["_id"]) @@ -438,10 +435,8 @@ class Collection(common.BaseObject): def _update(self, filter, document, upsert=False, check_keys=True, multi=False, manipulate=False, write_concern=None): """Internal update / replace helper.""" - if not isinstance(filter, collections.Mapping): - raise TypeError("filter must be a mapping type") - if not isinstance(upsert, bool): - raise TypeError("upsert must be an instance of bool") + common.validate_is_mapping("filter", filter) + common.validate_boolean("upsert", upsert) if manipulate: document = self.__database._fix_incoming(document, self) @@ -495,7 +490,7 @@ class Collection(common.BaseObject): :Returns: - An instance of :class:`~pymongo.results.UpdateResult`. """ - helpers._check_ok_for_replace(replacement) + common.validate_ok_for_replace(replacement) result = self._update(filter, replacement, upsert) return UpdateResult(result, self.write_concern.acknowledged) @@ -511,7 +506,7 @@ class Collection(common.BaseObject): :Returns: - An instance of :class:`~pymongo.results.UpdateResult`. """ - helpers._check_ok_for_update(update) + common.validate_ok_for_update(update) result = self._update(filter, update, upsert, False) return UpdateResult(result, self.write_concern.acknowledged) @@ -527,7 +522,7 @@ class Collection(common.BaseObject): :Returns: - An instance of :class:`~pymongo.results.UpdateResult`. """ - helpers._check_ok_for_update(update) + common.validate_ok_for_update(update) result = self._update(filter, update, upsert, False, True) return UpdateResult(result, self.write_concern.acknowledged) @@ -543,8 +538,7 @@ class Collection(common.BaseObject): def _delete(self, filter, multi, write_concern=None): """Internal delete helper.""" - if not isinstance(filter, collections.Mapping): - raise TypeError("filter must be a mapping type") + common.validate_is_mapping("filter", filter) concern = (write_concern or self.write_concern).document safe = concern.get("w") != 0 @@ -1284,8 +1278,7 @@ class Collection(common.BaseObject): use_cursor = kwargs.pop( "useCursor", ("cursor" in kwargs or client._writable_max_wire_version() > 0)) - if not isinstance(use_cursor, bool): - raise ValueError("useCursor must be a bool") + common.validate_boolean("useCursor", use_cursor) if batch_size is not None and not use_cursor: raise ConfigurationError("batchSize requires the use of a cursor") @@ -1536,8 +1529,7 @@ class Collection(common.BaseObject): def __find_and_modify(self, filter, projection, sort, upsert=None, return_document=ReturnDocument.Before, **kwargs): """Internal findAndModify helper.""" - if not isinstance(filter, collections.Mapping): - raise TypeError("filter must be a mapping type") + common.validate_is_mapping("filter", filter) if not isinstance(return_document, bool): raise ValueError("return_document must be " "ReturnDocument.Before or ReturnDocument.After") @@ -1551,8 +1543,7 @@ class Collection(common.BaseObject): if sort is not None: cmd["sort"] = helpers._index_document(sort) if upsert is not None: - if not isinstance(upsert, bool): - raise ValueError("upsert must be True or False") + common.validate_boolean("upsert", upsert) cmd["upsert"] = upsert out = self._command(cmd, ReadPreference.PRIMARY, @@ -1609,7 +1600,7 @@ class Collection(common.BaseObject): as keyword arguments (for example maxTimeMS can be used with recent server versions). """ - helpers._check_ok_for_replace(replacement) + common.validate_ok_for_replace(replacement) kwargs['update'] = replacement return self.__find_and_modify(filter, projection, sort, upsert, return_document, **kwargs) @@ -1643,7 +1634,7 @@ class Collection(common.BaseObject): as keyword arguments (for example maxTimeMS can be used with recent server versions). """ - helpers._check_ok_for_update(update) + common.validate_ok_for_update(update) kwargs['update'] = update return self.__find_and_modify(filter, projection, sort, upsert, return_document, **kwargs) @@ -1658,8 +1649,7 @@ class Collection(common.BaseObject): """ warnings.warn("save is deprecated. Use insert_one or replace_one " "instead", DeprecationWarning, stacklevel=2) - if not isinstance(to_save, collections.MutableMapping): - raise TypeError("cannot save object of type %s" % type(to_save)) + common.validate_is_mutable_mapping("to_save", to_save) write_concern = None if kwargs: @@ -1701,10 +1691,8 @@ class Collection(common.BaseObject): """ warnings.warn("update is deprecated. Use replace_one, update_one or " "update_many instead.", DeprecationWarning, stacklevel=2) - if not isinstance(spec, collections.Mapping): - raise TypeError("spec must be a mapping type") - if not isinstance(document, collections.Mapping): - raise TypeError("document must be a mapping type") + common.validate_is_mapping("spec", spec) + common.validate_is_mapping("document", document) if document: # If a top level key begins with '$' this is a modify operation # and we should skip key validation. It doesn't matter which key diff --git a/pymongo/common.py b/pymongo/common.py index eb03de208..8fc99d296 100644 --- a/pymongo/common.py +++ b/pymongo/common.py @@ -101,16 +101,20 @@ _UUID_REPRESENTATIONS = { def validate_boolean(option, value): - """Validates that 'value' is 'true' or 'false'. - """ + """Validates that 'value' is True or False.""" if isinstance(value, bool): return value - elif isinstance(value, string_type): + raise TypeError("%s must be True or False" % (option,)) + + +def validate_boolean_or_string(option, value): + """Validates that value is True, False, 'true', or 'false'.""" + if isinstance(value, string_type): if value not in ('true', 'false'): raise ConfigurationError("The value of %s must be " "'true' or 'false'" % (option,)) return value == 'true' - raise TypeError("Wrong type for %s, value must be a boolean" % (option,)) + return validate_boolean(option, value) def validate_integer(option, value): @@ -300,11 +304,49 @@ def validate_auth_mechanism_properties(option, value): def validate_document_class(option, value): """Validate the document_class option.""" if not issubclass(value, collections.MutableMapping): - raise ConfigurationError("%s must be a sublass of " + raise ConfigurationError("%s must be dict, bson.son.SON, or another " + "sublass of " "collections.MutableMapping" % (option,)) return value +def validate_is_mapping(option, value): + """Validate the type of method arguments that expect a document.""" + if not isinstance(value, collections.Mapping): + raise TypeError("%s must be an instance of dict, bson.son.SON, or " + "other type that inherits from " + "collections.Mapping" % (option,)) + + +def validate_is_mutable_mapping(option, value): + """Validate the type of method arguments that expect a mutable document.""" + if not isinstance(value, collections.MutableMapping): + raise TypeError("%s must be an instance of dict, bson.son.SON, or " + "other type that inherits from " + "collections.MutableMapping" % (option,)) + + +def validate_ok_for_replace(replacement): + """Validate a replacement document.""" + validate_is_mapping("replacement", replacement) + # Replacement can be {} + if replacement: + first = next(iter(replacement)) + if first.startswith('$'): + raise ValueError('replacement can not include $ operators') + + +def validate_ok_for_update(update): + """Validate an update document.""" + validate_is_mapping("update", update) + # Update can not be {} + if not update: + raise ValueError('update only works with $ operators') + first = next(iter(update)) + if not first.startswith('$'): + raise ValueError('update only works with $ operators') + + # journal is an alias for j, # wtimeoutms is an alias for wtimeout, VALIDATORS = { @@ -312,16 +354,16 @@ VALIDATORS = { 'w': validate_int_or_basestring, 'wtimeout': validate_integer, 'wtimeoutms': validate_integer, - 'fsync': validate_boolean, - 'j': validate_boolean, - 'journal': validate_boolean, + 'fsync': validate_boolean_or_string, + 'j': validate_boolean_or_string, + 'journal': validate_boolean_or_string, 'connecttimeoutms': validate_timeout_or_none, 'max_pool_size': validate_positive_integer_or_none, - 'socketkeepalive': validate_boolean, + 'socketkeepalive': validate_boolean_or_string, 'sockettimeoutms': validate_timeout_or_none, 'waitqueuetimeoutms': validate_timeout_or_none, 'waitqueuemultiple': validate_positive_integer_or_none, - 'ssl': validate_boolean, + 'ssl': validate_boolean_or_string, 'ssl_keyfile': validate_readable, 'ssl_certfile': validate_readable, 'ssl_cert_reqs': validate_cert_reqs, @@ -334,7 +376,7 @@ VALIDATORS = { 'authsource': validate_string, 'authmechanismproperties': validate_auth_mechanism_properties, 'document_class': validate_document_class, - 'tz_aware': validate_boolean, + 'tz_aware': validate_boolean_or_string, 'uuidrepresentation': validate_uuid_representation, } diff --git a/pymongo/cursor.py b/pymongo/cursor.py index 817d34b62..ee3ed04b4 100644 --- a/pymongo/cursor.py +++ b/pymongo/cursor.py @@ -17,7 +17,7 @@ import copy import socket -from collections import deque, Mapping +from collections import deque from bson import RE_TYPE from bson.code import Code @@ -26,11 +26,12 @@ from bson.py3compat import (iteritems, string_type) from bson.son import SON from pymongo import helpers, message -from pymongo.read_preferences import ReadPreference +from pymongo.common import validate_boolean, validate_is_mapping from pymongo.errors import (AutoReconnect, InvalidOperation, NotMasterError, OperationFailure) +from pymongo.read_preferences import ReadPreference _QUERY_OPTIONS = { "tailable_cursor": 2, @@ -113,24 +114,19 @@ class Cursor(object): if spec is None: spec = {} - if not isinstance(spec, Mapping): - raise TypeError("filter must be a mapping type.") + validate_is_mapping("filter", spec) if not isinstance(skip, int): raise TypeError("skip must be an instance of int") if not isinstance(limit, int): raise TypeError("limit must be an instance of int") - if not isinstance(no_cursor_timeout, bool): - raise TypeError("no_cursor_timeout must be an instance of bool") + validate_boolean("no_cursor_timeout", no_cursor_timeout) if cursor_type not in (NON_TAILABLE, TAILABLE, TAILABLE_AWAIT, EXHAUST): raise ValueError("not a valid value for cursor_type") - if not isinstance(allow_partial_results, bool): - raise TypeError("allow_partial_results " - "must be an instance of bool") - if not isinstance(oplog_replay, bool): - raise TypeError("oplog_replay must be an instance of bool") - if modifiers is not None and not isinstance(modifiers, Mapping): - raise TypeError("modifiers must be a mapping type.") + validate_boolean("allow_partial_results", allow_partial_results) + validate_boolean("oplog_replay", oplog_replay) + if modifiers is not None: + validate_is_mapping("modifiers", modifiers) if projection is not None: if not projection: @@ -666,8 +662,7 @@ class Cursor(object): .. versionchanged:: 2.8 The :meth:`~count` method now supports :meth:`~hint`. """ - if not isinstance(with_limit_and_skip, bool): - raise TypeError("with_limit_and_skip must be an instance of bool") + validate_boolean("with_limit_and_skip", with_limit_and_skip) options = {"query": self.__spec} if self.__max_time_ms is not None: options["maxTimeMS"] = self.__max_time_ms diff --git a/pymongo/helpers.py b/pymongo/helpers.py index 26ffc5ec8..7675e8565 100644 --- a/pymongo/helpers.py +++ b/pymongo/helpers.py @@ -33,29 +33,6 @@ from pymongo.errors import (CursorNotFound, from pymongo.message import query -def _check_ok_for_replace(replacement): - """Validate a replacement document.""" - if not isinstance(replacement, collections.Mapping): - raise TypeError('replacement must be a mapping type.') - # Replacement can be {} - if replacement: - first = next(iter(replacement)) - if first.startswith('$'): - raise ValueError('replacement can not include $ operators') - - -def _check_ok_for_update(update): - """Validate an update document.""" - if not isinstance(update, collections.Mapping): - raise TypeError('update must be a mapping type.') - # Update can not be {} - if not update: - raise ValueError('update only works with $ operators') - first = next(iter(update)) - if not first.startswith('$'): - raise ValueError('update only works with $ operators') - - def _index_list(key_or_list, direction=None): """Helper to generate a list of (key, direction) pairs. diff --git a/pymongo/options.py b/pymongo/options.py index ddccb917e..97885abad 100644 --- a/pymongo/options.py +++ b/pymongo/options.py @@ -14,8 +14,7 @@ """Option class definitions.""" -import collections - +from pymongo.common import validate_boolean, validate_is_mapping class ReturnDocument(object): """An enum used with @@ -26,7 +25,7 @@ class ReturnDocument(object): """Return the original document before it was updated/replaced, or ``None`` if no document matches the query. """ - After = True + After = True """Return the updated/replaced or inserted document.""" @@ -36,10 +35,10 @@ class _WriteOp(object): __slots__ = ("_filter", "_doc", "_upsert") def __init__(self, filter=None, doc=None, upsert=None): - if filter is not None and not isinstance(filter, collections.Mapping): - raise TypeError("filter must be a mapping type.") - if upsert is not None and not isinstance(upsert, bool): - raise TypeError("upsert must be True or False") + if filter is not None: + validate_is_mapping("filter", filter) + if upsert is not None: + validate_boolean("upsert", upsert) self._filter = filter self._doc = doc self._upsert = upsert diff --git a/pymongo/read_preferences.py b/pymongo/read_preferences.py index a0a3e9bb2..245f3ed94 100644 --- a/pymongo/read_preferences.py +++ b/pymongo/read_preferences.py @@ -55,7 +55,9 @@ def _validate_tag_sets(tag_sets): for tags in tag_sets: if not isinstance(tags, Mapping): raise ConfigurationError( - "Tag set %r invalid, must be a mapping." % (tags,)) + "Tag set %r invalid, must be an instance of dict, " + "bson.son.SON or other type that inherits from " + "collection.Mapping" % (tags,)) return tag_sets