From 029fca481d7a0e7a9ee6a05c02a7ed5de62ef7f0 Mon Sep 17 00:00:00 2001 From: Tersoo Date: Mon, 5 Aug 2019 03:04:51 -0400 Subject: [PATCH 1/2] added more tests to get up to 84% test coverage --- firestore/__init__.py | 2 ++ firestore/containers/document.py | 38 ++++++++++------------ firestore/datatypes/base.py | 1 + firestore/datatypes/map.py | 56 +++++++++++++++++++++++++++++--- firestore/datatypes/string.py | 9 +++++ firestore/db/__init__.py | 4 +++ firestore/db/connection.py | 21 ++++++++++-- tests/datatypes/map_test.py | 51 ++++++++++++++++++++++++++--- tests/datatypes/string_test.py | 7 ++++ 9 files changed, 156 insertions(+), 33 deletions(-) diff --git a/firestore/__init__.py b/firestore/__init__.py index dd50820..f8e128c 100755 --- a/firestore/__init__.py +++ b/firestore/__init__.py @@ -30,6 +30,7 @@ String, Timestamp, ) +from .db import Connection __all__ = [ @@ -37,6 +38,7 @@ "Boolean", "Byte", "Collection", + "Connection", "Datatype", "Document", "Float", diff --git a/firestore/containers/document.py b/firestore/containers/document.py index f7c01c5..e9e1275 100755 --- a/firestore/containers/document.py +++ b/firestore/containers/document.py @@ -47,16 +47,11 @@ class Document(object): i.e. setting and saving, querying, and updating document instances. """ - @staticmethod - def __constructor__(self, *args, **kwargs): - """custom method to load document constraints""" - # Document constraints are the constraints found on - # fields that pertain to the entire document and not - # just the field. - # For instance required, unique, pk etc... These fields - # do not have any meaning without the larger document, and or - # Collection in the picture. - pass + @classmethod + def __autospector__(cls, *args, **kwargs): + return { + k: v for k, v in cls.__dict__.items() if k not in ["__module__", "__doc__"] + } def __deref__(self, doc_ref): """ @@ -85,17 +80,16 @@ def __init__(self, *args, **kwargs): # that exist on this document class. # Useful for pk, unique, required and other document # level validation. - self.fields_cache = { - k: v - for k, v in type(self).__dict__.items() - if k not in ["__module__", "__doc__"] - } + self.fields_cache = self.__autospector__() for k in kwargs: - if k not in self.fields_cache.keys(): # on the fly access to obviate the need for gc + if ( + k not in self.fields_cache.keys() + ): # on the fly access to obviate the need for gc raise UnknownFieldError( f"Key {k} not found in document {type(self).__name__}" ) + self._data.add(k, kwargs.get(k)) def add_field(self, field, value): @@ -110,7 +104,7 @@ def get_field(self, field): Get a field form the internal _data store of field values """ return self._data.get(field._name) - + def _presave(self): """ Validates inputs and ensures all required fields and other @@ -125,11 +119,13 @@ def _presave(self): if not v: if f.default: - self._data.add(k, v.default) - if callable(v.default): - self._data.add(k, v.default()) + self._data.add(k, f.default) + if callable(f.default): + self._data.add(k, f.default()) elif f.required: - raise ValidationError(f'{f._name} is a required field of {type(self).__name__}') + raise ValidationError( + f"{f._name} is a required field of {type(self).__name__}" + ) def save(self): """ diff --git a/firestore/datatypes/base.py b/firestore/datatypes/base.py index a624952..4f01ba4 100644 --- a/firestore/datatypes/base.py +++ b/firestore/datatypes/base.py @@ -14,6 +14,7 @@ def __get__(self, instance, metadata): def __set__(self, instance, value): self.validate(value) + self.value = value instance.add_field(self, value) def __set_name__(self, cls, name): diff --git a/firestore/datatypes/map.py b/firestore/datatypes/map.py index 18a4b0d..6432364 100755 --- a/firestore/datatypes/map.py +++ b/firestore/datatypes/map.py @@ -1,15 +1,19 @@ -from .datatype import Datatype +from firestore.containers.document import Document +from firestore.datatypes.base import Base +from firestore.errors import ValidationError -class MapSchema(object): + +class MapSchema(Document): """ A map schema defines a helper by which maps can be populated so there is no need to use default python dicts""" - pass + def __init__(self, *args, **kwargs): + super(MapSchema, self).__init__(*args, **kwargs) -class Map(object): +class Map(Base): """Maps as defined by firestore represent an object saved within a document. In python speak - A map is akin to a dictionary. @@ -19,4 +23,46 @@ class Map(object): """ def __init__(self, *args, **kwargs): - pass + try: + self.map_ref = args[0] + except IndexError: + self.map_ref = None + super(Map, self).__init__(*args, **kwargs) + + def __set__(self, instance, value): + self.validate(value) + + # if a mapschema was passed in to the document + # field map descriptor field then it is expected + # that a mapping or dict will be the input value. + # If the input is a dict then convert it to a map schema + # and save otherwise save the map schema. + # If no mapschema was used then save the dict as is. + if self.map_ref: + value = self.map_ref(**value) if isinstance(value, dict) else value + self.value = value + instance.add_field(self, value) + + def validate(self, value): + # If the map descriptor field has any + # children at all then it should be + # a MapSchema instance of keys and values + # expected in the map. + # Maps unfortunately can not be marked as pk, as + # a required field, or as having a default value. + # You can however map the keys in the + # map schema instance as required. + if self.map_ref: + if not isinstance(value, (MapSchema, dict)): + raise ValueError() + if isinstance(value, dict): + _schema = self.map_ref.__autospector__() + + for k in _schema: + # get a local copy of the field instance + f = _schema.get(k) + v = value.get(k) + f.validate(v) + else: + # If mapschema then validation happens at map schema level + value._presave() diff --git a/firestore/datatypes/string.py b/firestore/datatypes/string.py index 2e2bb1b..22328a1 100755 --- a/firestore/datatypes/string.py +++ b/firestore/datatypes/string.py @@ -13,6 +13,15 @@ def __init__(self, *args, **kwargs): super(String, self).__init__(*args, **kwargs) def validate(self, value): + if not isinstance(value, str): + if not self.coerce: + raise ValueError( + f"Can not assign type {type(value)} to str and coerce is disabled" + ) + value = str(value) + + # Value is either a string after this point or has + # been coerced to a string max_msg = f"{self._name} must have a maximum len of {self.maximum}, found {len(value)}" min_msg = ( f"{self._name} must have minimum len of {self.minimum}, found {len(value)}" diff --git a/firestore/db/__init__.py b/firestore/db/__init__.py index e69de29..251024b 100755 --- a/firestore/db/__init__.py +++ b/firestore/db/__init__.py @@ -0,0 +1,4 @@ +from firestore.db.connection import Connection + + +__all__ = ["Connection"] diff --git a/firestore/db/connection.py b/firestore/db/connection.py index 4fcd3ef..68c805b 100755 --- a/firestore/db/connection.py +++ b/firestore/db/connection.py @@ -1,4 +1,9 @@ -from google.cloud.client import Client +import os + +import firebase_admin + +from firebase_admin import credentials +from firebase_admin import firestore # we know that these guys will not be imported with import * as they begin with an underscore _dbs = {} @@ -13,4 +18,16 @@ class Connection(object): :param connection_string {str}: """ - pass + def __init__(self, certificate): + _client = _connections.get("client") + if _client: + self._db = _client + else: + if certificate: + self.certificate = credentials.Certificate(certificate) + firebase_admin.initialize_app(self.certificate) + self._db = firestore.client() + _connections["client"] = self._db + + def push(self): + pass diff --git a/tests/datatypes/map_test.py b/tests/datatypes/map_test.py index 4545170..218ae46 100644 --- a/tests/datatypes/map_test.py +++ b/tests/datatypes/map_test.py @@ -1,30 +1,71 @@ from unittest import TestCase +from pytest import mark from firestore import Document, Integer, Map, String from firestore.datatypes.map import MapSchema +from firestore.errors import ValidationError + class Mapping(MapSchema): - name = String(required=True) + name = String(required=True, default="Tiza") age = Integer(minimum=4) +class AltMapping(MapSchema): + name = String(required=True) class MapDocument(Document): - map = Map(required=True) + map = Map() + +class MapDocumentMapping(Document): + map = Map(Mapping) class MapTest(TestCase): def setUp(self): self.md = MapDocument() + self.mdm = MapDocumentMapping() def tearDown(self): pass - def test_map_schema(self): + def test_map_schema_validation(self): """Tests that the map schema object correctly transforms to a python dict in the document instance and vis a vis""" - pass + with self.assertRaises(ValidationError): + self.mdm.map = {"name": "Peter", "age": 2} + + def test_map_schema_value_error(self): + with self.assertRaises(ValueError): + self.mdm.map = {"name": "Peter", "age": "two"} + + def test_document_presave_with_map(self): + mapping = Mapping() + self.mdm.map = mapping + self.assertEqual(self.mdm.map.name, "Tiza") + + def test_mapschema_assignment_validation(self): + """ + Test that validations in mapschema are invoked if + a MapSchema object is used for assignment as opposed + to a dict loaded into a mapschema + """ + mapping = AltMapping() + with self.assertRaises(ValidationError): + self.mdm.map = mapping + + def test_map_schema_assignment(self): + mapping = Mapping() + mapping.name = "Tiza" + self.mdm.map = mapping + self.assertEqual(self.mdm.map.name, "Tiza") def test_map_in_document_instance(self): """Test when we create a map it get's loaded into the parent doc instance""" - self.md.map = {} + _map = {"name": "peter", "age": 5} + self.md.map = _map + + # this is not testing self.md_data but testing that the map + # descriptor field has pushed it's value to the + # documents _data cache + self.assertEqual(self.md._data.get('map'), _map) diff --git a/tests/datatypes/string_test.py b/tests/datatypes/string_test.py index c80db45..e3c1739 100644 --- a/tests/datatypes/string_test.py +++ b/tests/datatypes/string_test.py @@ -7,6 +7,7 @@ class StringDocument(Document): name = String(required=True, minimum=5, maximum=10) + email = String(coerce=False) class StringTest(TestCase): @@ -25,6 +26,12 @@ def test_string_minimum(self): self.sd.name = "me" with self.assertRaises(ValidationError): self.sd.name = "very very very very long name" + with self.assertRaises(ValidationError): + self.sd.name = 5 + + def test_string_coerce(self): + with self.assertRaises(ValueError): + self.sd.email = 5 def test_string_in_document(self): self.sd.name = "Whosand" From 8f8ac77cd11f1bff68c797e037675e414e4c160a Mon Sep 17 00:00:00 2001 From: Tersoo Date: Mon, 5 Aug 2019 03:05:14 -0400 Subject: [PATCH 2/2] added tests for persistence to firestore run --- tests/db/__init__.py | 0 tests/db/connection_test.py | 15 +++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 tests/db/__init__.py create mode 100644 tests/db/connection_test.py diff --git a/tests/db/__init__.py b/tests/db/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/db/connection_test.py b/tests/db/connection_test.py new file mode 100644 index 0000000..99011b2 --- /dev/null +++ b/tests/db/connection_test.py @@ -0,0 +1,15 @@ +from unittest import TestCase + +from firestore import Connection + + +class TestConnection(TestCase): + + def setUp(self): + pass + + def tearDown(self): + pass + + def test_firebase_connection(self): + pass