Skip to content

Commit

Permalink
3rd review changes
Browse files Browse the repository at this point in the history
  • Loading branch information
chemelnucfin committed Mar 6, 2018
1 parent 1843740 commit ed6e22e
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 9 deletions.
11 changes: 6 additions & 5 deletions firestore/google/cloud/firestore_v1beta1/_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,9 @@ class FieldPath(object):

def __init__(self, *parts):
for part in parts:
if not isinstance(part, six.string_types):
raise ValueError("One or more components is not a string.")
if not isinstance(part, six.string_types) or not part:
error = 'One or more components is not a string or is empty.'
raise ValueError(error)
self.parts = tuple(parts)

@staticmethod
Expand All @@ -146,10 +147,10 @@ def from_string(string):
as arguments to `FieldPath`.
"""
invalid_characters = '~*/[]'
for invalid_character in invalid_characters:
if invalid_character in string:
raise ValueError('Invalid characters in string.')
string = string.split('.')
for part in string:
if not part or part in invalid_characters:
raise ValueError("No string or invalid characters in string.")
return FieldPath(*string)

def to_api_repr(self):
Expand Down
12 changes: 8 additions & 4 deletions firestore/tests/unit/test__helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,10 @@ def test_none_fails(self):
with self.assertRaises(ValueError):
field_path = self._make_one('a', None, 'b')

def test_empty_string_in_part_fails(self):
with self.assertRaises(ValueError):
field_path = self._make_one('a', '', 'b')

def test_integer_fails(self):
with self.assertRaises(ValueError):
field_path = self._make_one('a', 3, 'b')
Expand Down Expand Up @@ -210,15 +214,15 @@ def test_tuple_fails(self):
field_path = self._make_one(parts)

def test_equality(self):
field_path = self._make_one("a", "b")
string_path = self._get_target_class().from_string("a.b")
field_path = self._make_one('a', 'b')
string_path = self._get_target_class().from_string('a.b')
self.assertEqual(field_path, string_path)

def test_non_equal_types(self):
import mock
mock = mock.Mock()
mock.parts = "a", "b"
field_path = self._make_one("a", "b")
mock.parts = 'a', 'b'
field_path = self._make_one('a', 'b')
self.assertNotEqual(field_path, mock)

def test_key(self):
Expand Down

0 comments on commit ed6e22e

Please sign in to comment.