Skip to content

Commit 4d6cd26

Browse files
chemelnucfindhermes
chemelnucfin
authored and
dhermes
committed
Spanner: Make sure **exactly** one of start_*/end_* are passed to KeyRange (googleapis#4618)
Also making sure to send the `start_*` / `end_*` protobuf values even if they are an empty list.
1 parent b11b3a1 commit 4d6cd26

File tree

2 files changed

+104
-67
lines changed

2 files changed

+104
-67
lines changed

‎spanner/google/cloud/spanner_v1/keyset.py‎

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,28 +24,40 @@
2424
class KeyRange(object):
2525
"""Identify range of table rows via start / end points.
2626
27-
:type start_open: list of scalars
28-
:param start_open: keys identifying start of range (this key excluded)
29-
30-
:type start_closed: list of scalars
31-
:param start_closed: keys identifying start of range (this key included)
32-
33-
:type end_open: list of scalars
34-
:param end_open: keys identifying end of range (this key excluded)
35-
36-
:type end_closed: list of scalars
37-
:param end_closed: keys identifying end of range (this key included)
27+
.. note::
28+
29+
Exactly one of ``start_open`` and ``start_closed`` must be
30+
passed and exactly one of ``end_open`` and ``end_closed`` must be.
31+
To "start at the beginning" (i.e. specify no start for the range)
32+
pass ``start_closed=[]``. To "go to the end" (i.e. specify no end
33+
for the range) pass ``end_closed=[]``.
34+
35+
Args:
36+
start_open (List): Keys identifying start of range (this key
37+
excluded).
38+
start_closed (List): Keys identifying start of range (this key
39+
included).
40+
end_open (List): Keys identifying end of range (this key
41+
excluded).
42+
end_closed (List): Keys identifying end of range (this key
43+
included).
44+
45+
Raises:
46+
ValueError: If **neither** ``start_open`` or ``start_closed`` is
47+
passed.
48+
ValueError: If **both** ``start_open`` and ``start_closed`` are passed.
49+
ValueError: If **neither** ``end_open`` or ``end_closed`` is passed.
50+
ValueError: If **both** ``end_open`` and ``end_closed`` are passed.
3851
"""
3952
def __init__(self, start_open=None, start_closed=None,
4053
end_open=None, end_closed=None):
41-
if not any([start_open, start_closed, end_open, end_closed]):
42-
raise ValueError("Must specify at least a start or end row.")
43-
44-
if start_open and start_closed:
45-
raise ValueError("Specify one of 'start_open' / 'start_closed'.")
54+
if ((start_open is None and start_closed is None)
55+
or (start_open is not None and start_closed is not None)):
56+
raise ValueError('Specify exactly one of start_closed or start_open')
4657

47-
if end_open and end_closed:
48-
raise ValueError("Specify one of 'end_open' / 'end_closed'.")
58+
if ((end_open is None and end_closed is None)
59+
or (end_open is not None and end_closed is not None)):
60+
raise ValueError('Specify exactly one of end_closed or end_open')
4961

5062
self.start_open = start_open
5163
self.start_closed = start_closed
@@ -60,16 +72,16 @@ def to_pb(self):
6072
"""
6173
kwargs = {}
6274

63-
if self.start_open:
75+
if self.start_open is not None:
6476
kwargs['start_open'] = _make_list_value_pb(self.start_open)
6577

66-
if self.start_closed:
78+
if self.start_closed is not None:
6779
kwargs['start_closed'] = _make_list_value_pb(self.start_closed)
6880

69-
if self.end_open:
81+
if self.end_open is not None:
7082
kwargs['end_open'] = _make_list_value_pb(self.end_open)
7183

72-
if self.end_closed:
84+
if self.end_closed is not None:
7385
kwargs['end_closed'] = _make_list_value_pb(self.end_closed)
7486

7587
return KeyRangePB(**kwargs)

‎spanner/tests/unit/test_keyset.py‎

Lines changed: 70 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -30,49 +30,47 @@ def test_ctor_no_start_no_end(self):
3030
with self.assertRaises(ValueError):
3131
self._make_one()
3232

33-
def test_ctor_w_start_open_and_start_closed(self):
33+
def test_ctor_start_open_and_start_closed(self):
3434
KEY_1 = [u'key_1']
3535
KEY_2 = [u'key_2']
3636
with self.assertRaises(ValueError):
3737
self._make_one(start_open=KEY_1, start_closed=KEY_2)
3838

39-
def test_ctor_w_end_open_and_end_closed(self):
39+
def test_ctor_end_open_and_end_closed(self):
4040
KEY_1 = [u'key_1']
4141
KEY_2 = [u'key_2']
4242
with self.assertRaises(ValueError):
4343
self._make_one(end_open=KEY_1, end_closed=KEY_2)
4444

45-
def test_ctor_w_only_start_open(self):
45+
def test_ctor_conflicting_start(self):
4646
KEY_1 = [u'key_1']
47-
krange = self._make_one(start_open=KEY_1)
48-
self.assertEqual(krange.start_open, KEY_1)
49-
self.assertEqual(krange.start_closed, None)
50-
self.assertEqual(krange.end_open, None)
51-
self.assertEqual(krange.end_closed, None)
47+
with self.assertRaises(ValueError):
48+
self._make_one(start_open=[], start_closed=[], end_closed=KEY_1)
5249

53-
def test_ctor_w_only_start_closed(self):
50+
def test_ctor_conflicting_end(self):
5451
KEY_1 = [u'key_1']
55-
krange = self._make_one(start_closed=KEY_1)
56-
self.assertEqual(krange.start_open, None)
57-
self.assertEqual(krange.start_closed, KEY_1)
58-
self.assertEqual(krange.end_open, None)
59-
self.assertEqual(krange.end_closed, None)
52+
with self.assertRaises(ValueError):
53+
self._make_one(start_open=KEY_1, end_open=[], end_closed=[])
6054

61-
def test_ctor_w_only_end_open(self):
55+
def test_ctor_single_key_start_closed(self):
6256
KEY_1 = [u'key_1']
63-
krange = self._make_one(end_open=KEY_1)
64-
self.assertEqual(krange.start_open, None)
65-
self.assertEqual(krange.start_closed, None)
66-
self.assertEqual(krange.end_open, KEY_1)
67-
self.assertEqual(krange.end_closed, None)
57+
with self.assertRaises(ValueError):
58+
self._make_one(start_closed=KEY_1)
6859

69-
def test_ctor_w_only_end_closed(self):
60+
def test_ctor_single_key_start_open(self):
7061
KEY_1 = [u'key_1']
71-
krange = self._make_one(end_closed=KEY_1)
72-
self.assertEqual(krange.start_open, None)
73-
self.assertEqual(krange.start_closed, None)
74-
self.assertEqual(krange.end_open, None)
75-
self.assertEqual(krange.end_closed, KEY_1)
62+
with self.assertRaises(ValueError):
63+
self._make_one(start_open=KEY_1)
64+
65+
def test_ctor_single_key_end_closed(self):
66+
KEY_1 = [u'key_1']
67+
with self.assertRaises(ValueError):
68+
self._make_one(end_closed=KEY_1)
69+
70+
def test_ctor_single_key_end_open(self):
71+
KEY_1 = [u'key_1']
72+
with self.assertRaises(ValueError):
73+
self._make_one(end_open=KEY_1)
7674

7775
def test_ctor_w_start_open_and_end_closed(self):
7876
KEY_1 = [u'key_1']
@@ -93,31 +91,58 @@ def test_ctor_w_start_closed_and_end_open(self):
9391
self.assertEqual(krange.end_closed, None)
9492

9593
def test_to_pb_w_start_closed_and_end_open(self):
94+
from google.protobuf.struct_pb2 import ListValue
95+
from google.protobuf.struct_pb2 import Value
9696
from google.cloud.spanner_v1.proto.keys_pb2 import KeyRange
9797

98-
KEY_1 = [u'key_1']
99-
KEY_2 = [u'key_2']
100-
krange = self._make_one(start_closed=KEY_1, end_open=KEY_2)
101-
krange_pb = krange.to_pb()
102-
self.assertIsInstance(krange_pb, KeyRange)
103-
self.assertEqual(len(krange_pb.start_closed), 1)
104-
self.assertEqual(krange_pb.start_closed.values[0].string_value,
105-
KEY_1[0])
106-
self.assertEqual(len(krange_pb.end_open), 1)
107-
self.assertEqual(krange_pb.end_open.values[0].string_value, KEY_2[0])
98+
key1 = u'key_1'
99+
key2 = u'key_2'
100+
key_range = self._make_one(start_closed=[key1], end_open=[key2])
101+
key_range_pb = key_range.to_pb()
102+
expected = KeyRange(
103+
start_closed=ListValue(values=[
104+
Value(string_value=key1)
105+
]),
106+
end_open=ListValue(values=[
107+
Value(string_value=key2)
108+
]),
109+
)
110+
self.assertEqual(key_range_pb, expected)
108111

109112
def test_to_pb_w_start_open_and_end_closed(self):
113+
from google.protobuf.struct_pb2 import ListValue
114+
from google.protobuf.struct_pb2 import Value
110115
from google.cloud.spanner_v1.proto.keys_pb2 import KeyRange
111116

112-
KEY_1 = [u'key_1']
113-
KEY_2 = [u'key_2']
114-
krange = self._make_one(start_open=KEY_1, end_closed=KEY_2)
115-
krange_pb = krange.to_pb()
116-
self.assertIsInstance(krange_pb, KeyRange)
117-
self.assertEqual(len(krange_pb.start_open), 1)
118-
self.assertEqual(krange_pb.start_open.values[0].string_value, KEY_1[0])
119-
self.assertEqual(len(krange_pb.end_closed), 1)
120-
self.assertEqual(krange_pb.end_closed.values[0].string_value, KEY_2[0])
117+
key1 = u'key_1'
118+
key2 = u'key_2'
119+
key_range = self._make_one(start_open=[key1], end_closed=[key2])
120+
key_range_pb = key_range.to_pb()
121+
expected = KeyRange(
122+
start_open=ListValue(values=[
123+
Value(string_value=key1)
124+
]),
125+
end_closed=ListValue(values=[
126+
Value(string_value=key2)
127+
]),
128+
)
129+
self.assertEqual(key_range_pb, expected)
130+
131+
def test_to_pb_w_empty_list(self):
132+
from google.protobuf.struct_pb2 import ListValue
133+
from google.protobuf.struct_pb2 import Value
134+
from google.cloud.spanner_v1.proto.keys_pb2 import KeyRange
135+
136+
key = u'key'
137+
key_range = self._make_one(start_closed=[], end_closed=[key])
138+
key_range_pb = key_range.to_pb()
139+
expected = KeyRange(
140+
start_closed=ListValue(values=[]),
141+
end_closed=ListValue(values=[
142+
Value(string_value=key)
143+
]),
144+
)
145+
self.assertEqual(key_range_pb, expected)
121146

122147

123148
class TestKeySet(unittest.TestCase):

0 commit comments

Comments
 (0)