Skip to content

Commit cbb920a

Browse files
olavloitegcf-owl-bot[bot]
olavloite
and
gcf-owl-bot[bot]
authored
fix: add option to disable client-side pk generation (#930)
* test: add mockserver tests * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix: add option to disable client-side pk generation Add an option to disable client-side generation of primary key values. This makes it possible to use the Spanner Django dialect in combination with other Django dialects, without the Spanner Django PK generation interfering with the other dialect(s). Fixes #783 * chore: introduce configuration option for id generation Adds a configuration option for enabling/disabling client-side ID generation using a random UUID. This allows users to better control ID generation when using non-Spanner databases in combination with Spanner. This option can in the future also be used for using IDENTITY columns for ID generation. --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
1 parent 8dc5f4d commit cbb920a

5 files changed

Lines changed: 140 additions & 13 deletions

File tree

‎packages/django-google-spanner/django_spanner/__init__.py‎

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@
1212
# do that.
1313
from uuid import uuid4
1414

15+
RANDOM_ID_GENERATION_ENABLED_SETTING = "RANDOM_ID_GENERATION_ENABLED"
16+
1517
import pkg_resources
18+
from django.conf.global_settings import DATABASES
19+
from django.db import DEFAULT_DB_ALIAS
1620
from google.cloud.spanner_v1 import JsonObject
1721
from django.db.models.fields import (
1822
NOT_PROVIDED,
@@ -64,11 +68,25 @@ def autofield_init(self, *args, **kwargs):
6468
kwargs["blank"] = True
6569
Field.__init__(self, *args, **kwargs)
6670

67-
if (
68-
django.db.connection.settings_dict["ENGINE"] == "django_spanner"
69-
and self.default == NOT_PROVIDED
70-
):
71-
self.default = gen_rand_int64
71+
# The following behavior is chosen to prevent breaking changes with the original behavior.
72+
# 1. We use a client-side randomly generated int64 value for autofields if Spanner is the
73+
# default database, and DISABLE_RANDOM_ID_GENERATION has not been set.
74+
# 2. If Spanner is one of the non-default databases, and no value at all has been set for
75+
# DISABLE_RANDOM_ID_GENERATION, then we do not enable it. If there is a value for this
76+
# configuration option, then we use that value.
77+
databases = django.db.connections.databases
78+
for db, config in databases.items():
79+
default_enabled = str(db == DEFAULT_DB_ALIAS)
80+
if (
81+
config["ENGINE"] == "django_spanner"
82+
and self.default == NOT_PROVIDED
83+
and config.get(
84+
RANDOM_ID_GENERATION_ENABLED_SETTING, default_enabled
85+
).lower()
86+
== "true"
87+
):
88+
self.default = gen_rand_int64
89+
break
7290

7391

7492
AutoField.__init__ = autofield_init

‎packages/django-google-spanner/tests/mockserver_tests/mock_server_test_base.py‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
import os
1616
import unittest
1717

18-
from django.db import connection
18+
from django.db import connection, connections
1919
from google.cloud.spanner_dbapi.parsed_statement import AutocommitDmlMode
2020
import google.cloud.spanner_v1.types.type as spanner_type
2121
import google.cloud.spanner_v1.types.result_set as result_set
@@ -37,6 +37,7 @@
3737
start_mock_server,
3838
)
3939
from tests.mockserver_tests.mock_database_admin import DatabaseAdminServicer
40+
from tests.settings import DATABASES
4041

4142

4243
def add_result(sql: str, result: ResultSet):
@@ -178,11 +179,17 @@ def teardown_class(cls):
178179
MockServerTestBase.server = None
179180

180181
def setup_method(self, test_method):
181-
connection.settings_dict["OPTIONS"]["client"] = self.client
182-
connection.settings_dict["OPTIONS"]["pool"] = self.pool
182+
for db, config in DATABASES.items():
183+
if config["ENGINE"] == "django_spanner":
184+
connections[db].settings_dict["OPTIONS"][
185+
"client"
186+
] = self.client
187+
connections[db].settings_dict["OPTIONS"]["pool"] = self.pool
183188

184189
def teardown_method(self, test_method):
185-
connection.close()
190+
for db, config in DATABASES.items():
191+
if config["ENGINE"] == "django_spanner":
192+
connections[db].close()
186193
MockServerTestBase.spanner_service.clear_requests()
187194
MockServerTestBase.database_admin_service.clear_requests()
188195
self._client = None

‎packages/django-google-spanner/tests/mockserver_tests/test_basics.py‎

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,21 @@
1111
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
14-
1514
from google.cloud.spanner_v1 import (
1615
BatchCreateSessionsRequest,
1716
ExecuteSqlRequest,
17+
CommitRequest,
1818
)
1919
from tests.mockserver_tests.mock_server_test_base import (
2020
MockServerTestBase,
2121
add_select1_result,
2222
add_singer_query_result,
23+
add_update_count,
2324
)
24-
from django.db import connection
25+
from django.db import connection, models
2526

2627
from tests.mockserver_tests.models import Singer
28+
from tests.settings import DATABASES
2729

2830

2931
class TestBasics(MockServerTestBase):
@@ -60,3 +62,69 @@ def test_django_select_singer(self):
6062
self.assertEqual(len(requests), 2)
6163
self.assertIsInstance(requests[0], BatchCreateSessionsRequest)
6264
self.assertIsInstance(requests[1], ExecuteSqlRequest)
65+
66+
def test_django_select_singer_using_other_db(self):
67+
add_singer_query_result(
68+
"SELECT tests_singer.id, tests_singer.first_name, tests_singer.last_name FROM tests_singer"
69+
)
70+
singers = Singer.objects.using("secondary").all()
71+
self.assertEqual(len(singers), 2)
72+
requests = self.spanner_service.requests
73+
self.assertEqual(len(requests), 2)
74+
self.assertIsInstance(requests[0], BatchCreateSessionsRequest)
75+
self.assertIsInstance(requests[1], ExecuteSqlRequest)
76+
77+
def test_insert_singer(self):
78+
add_update_count(
79+
"INSERT INTO tests_singer "
80+
"(id, first_name, last_name) "
81+
"VALUES (@a0, @a1, @a2)",
82+
1,
83+
)
84+
singer = Singer(first_name="test", last_name="test")
85+
singer.save()
86+
requests = self.spanner_service.requests
87+
self.assertEqual(len(requests), 3)
88+
self.assertIsInstance(requests[0], BatchCreateSessionsRequest)
89+
self.assertIsInstance(requests[1], ExecuteSqlRequest)
90+
self.assertIsInstance(requests[2], CommitRequest)
91+
# The ExecuteSqlRequest should have 3 parameters:
92+
# 1. first_name
93+
# 2. last_name
94+
# 3. client-side auto-generated primary key
95+
self.assertEqual(len(requests[1].params), 3)
96+
97+
def test_insert_singer_with_disabled_random_primary_key(self):
98+
for db, config in DATABASES.items():
99+
if config["ENGINE"] == "django_spanner":
100+
config["RANDOM_ID_GENERATION_ENABLED"] = "false"
101+
102+
# Define a class locally in this test method to ensure that
103+
# it is initialized after disabling random ID generation.
104+
class LocalSinger(models.Model):
105+
first_name = models.CharField(max_length=200)
106+
last_name = models.CharField(max_length=200)
107+
108+
try:
109+
add_update_count(
110+
"INSERT INTO tests_localsinger "
111+
"(first_name, last_name) "
112+
"VALUES (@a0, @a1)",
113+
1,
114+
)
115+
singer = LocalSinger(first_name="test", last_name="test")
116+
singer.save()
117+
requests = self.spanner_service.requests
118+
self.assertEqual(len(requests), 3)
119+
self.assertIsInstance(requests[0], BatchCreateSessionsRequest)
120+
self.assertIsInstance(requests[1], ExecuteSqlRequest)
121+
self.assertIsInstance(requests[2], CommitRequest)
122+
# The ExecuteSqlRequest should have 2 parameters:
123+
# 1. first_name
124+
# 2. last_name
125+
# There should be no client-side auto-generated primary key.
126+
self.assertEqual(len(requests[1].params), 2)
127+
finally:
128+
for db, config in DATABASES.items():
129+
if config["ENGINE"] == "django_spanner":
130+
config.pop("DISABLE_RANDOM_ID_GENERATION", None)

‎packages/django-google-spanner/tests/settings.py‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,17 @@
3838
"INSTANCE": INSTANCE_ID,
3939
"NAME": DATABASE_NAME,
4040
"TEST": {"NAME": DATABASE_NAME},
41-
}
41+
},
42+
"secondary": {
43+
"ENGINE": "django_spanner",
44+
"PROJECT": PROJECT_ID,
45+
"INSTANCE": INSTANCE_ID,
46+
"NAME": DATABASE_NAME,
47+
"TEST": {"NAME": DATABASE_NAME},
48+
},
49+
"other": {
50+
"ENGINE": "django.db.backends.sqlite3",
51+
},
4252
}
4353

4454
SECRET_KEY = "spanner env secret key"

‎packages/django-google-spanner/tests/unit/django_spanner/test_schema.py‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77

88
from .models import Author
9-
from django.db import NotSupportedError, connection
9+
from django.db import NotSupportedError, connection, connections
1010
from django.db.models import Index
1111
from django.db.models.fields import AutoField, IntegerField
1212
from django_spanner import gen_rand_int64
@@ -433,3 +433,27 @@ def test_autofield_not_spanner_w_default(self):
433433
assert gen_rand_int64 != field.default
434434
assert mock_func == field.default
435435
connection.settings_dict["ENGINE"] = "django_spanner"
436+
437+
def test_autofield_spanner_as_non_default_db_random_generation_enabled(
438+
self,
439+
):
440+
"""Not Spanner as the default db, default for field not provided."""
441+
connections.settings["default"]["ENGINE"] = "another_db"
442+
connections.settings["secondary"]["ENGINE"] = "django_spanner"
443+
connections.settings["secondary"][
444+
"RANDOM_ID_GENERATION_ENABLED"
445+
] = "true"
446+
field = AutoField(name="field_name")
447+
assert gen_rand_int64 == field.default
448+
connections.settings["default"]["ENGINE"] = "django_spanner"
449+
connections.settings["secondary"]["ENGINE"] = "django_spanner"
450+
del connections.settings["secondary"]["RANDOM_ID_GENERATION_ENABLED"]
451+
452+
def test_autofield_random_generation_disabled(self):
453+
"""Spanner, default is not provided."""
454+
connections.settings["default"][
455+
"RANDOM_ID_GENERATION_ENABLED"
456+
] = "false"
457+
field = AutoField(name="field_name")
458+
assert gen_rand_int64 != field.default
459+
del connections.settings["default"]["RANDOM_ID_GENERATION_ENABLED"]

0 commit comments

Comments
 (0)