Skip to content

Commit

Permalink
prohibit int in ObjectIdAutoField
Browse files Browse the repository at this point in the history
  • Loading branch information
timgraham committed Feb 14, 2025
1 parent f5dbacb commit 5e98655
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 21 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/test-python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
uses: actions/checkout@v4
with:
repository: 'mongodb-forks/django'
ref: 'mongodb-5.1.x'
ref: 'objectid-no-int'
path: 'django_repo'
persist-credentials: false
- name: Install system packages for Django's Python test dependencies
Expand Down
5 changes: 5 additions & 0 deletions django_mongodb_backend/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ def django_test_expected_failures(self):
"lookup.tests.LookupTests.test_in_ignore_none_with_unhashable_items",
"m2m_through_regress.tests.ThroughLoadDataTestCase.test_sequence_creation",
"many_to_many.tests.ManyToManyTests.test_add_remove_invalid_type",
"many_to_one.tests.ManyToOneTests.test_fk_to_smallautofield",
"many_to_one.tests.ManyToOneTests.test_fk_to_bigautofield",
"migrations.test_operations.OperationTests.test_autofield__bigautofield_foreignfield_growth",
"migrations.test_operations.OperationTests.test_model_with_bigautofield",
"migrations.test_operations.OperationTests.test_smallfield_autofield_foreignfield_growth",
Expand All @@ -213,6 +215,8 @@ def django_test_expected_failures(self):
"model_fields.test_autofield.BigAutoFieldTests",
"model_fields.test_autofield.SmallAutoFieldTests",
"queries.tests.TestInvalidValuesRelation.test_invalid_values",
"schema.tests.SchemaTests.test_alter_autofield_pk_to_bigautofield_pk",
"schema.tests.SchemaTests.test_alter_autofield_pk_to_smallautofield_pk",
},
"Converters aren't run on returning fields from insert.": {
# Unsure this is needed for this backend. Can implement by request.
Expand All @@ -231,6 +235,7 @@ def django_test_expected_failures(self):
"queries.test_qs_combinators.QuerySetSetOperationTests.test_order_raises_on_non_selected_column",
"queries.tests.RelatedLookupTypeTests.test_values_queryset_lookup",
"queries.tests.ValuesSubqueryTests.test_values_in_subquery",
"sites_tests.tests.CreateDefaultSiteTests.test_no_site_id",
},
"Cannot use QuerySet.delete() when querying across multiple collections on MongoDB.": {
"admin_changelist.tests.ChangeListTests.test_distinct_for_many_to_many_at_second_level_in_search_fields",
Expand Down
23 changes: 6 additions & 17 deletions django_mongodb_backend/fields/auto.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,11 @@ def deconstruct(self):
return name, path, args, kwargs

def get_prep_value(self, value):
if value is None:
return None
# Accept int for compatibility with Django's test suite which has many
# instances of manually assigned integer IDs, as well as for things
# like settings.SITE_ID which has a system check requiring an integer.
if isinstance(value, (ObjectId | int)):
if value is None or isinstance(value, ObjectId):
return value
try:
return ObjectId(value)
except errors.InvalidId as e:
# A manually assigned integer ID?
if isinstance(value, str) and value.isdigit():
return int(value)
raise ValueError(f"Field '{self.name}' expected an ObjectId but got {value!r}.") from e

def get_internal_type(self):
Expand All @@ -46,14 +38,11 @@ def to_python(self, value):
try:
return ObjectId(value)
except errors.InvalidId:
try:
return int(value)
except ValueError:
raise exceptions.ValidationError(
self.error_messages["invalid"],
code="invalid",
params={"value": value},
) from None
raise exceptions.ValidationError(
self.error_messages["invalid"],
code="invalid",
params={"value": value},
) from None

@cached_property
def validators(self):
Expand Down
4 changes: 2 additions & 2 deletions tests/indexes_/test_condition.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def test_negated_not_supported(self):
Index(
name="test",
fields=["headline"],
condition=~Q(pk=True),
condition=~Q(pk__isnull=True),
)._get_condition_mql(Article, schema_editor=editor)

def test_xor_not_supported(self):
Expand All @@ -43,7 +43,7 @@ def test_xor_not_supported(self):
Index(
name="test",
fields=["headline"],
condition=Q(pk=True) ^ Q(pk=False),
condition=Q(pk__isnull=True) ^ Q(pk__isnull=False),
)._get_condition_mql(Article, schema_editor=editor)

def test_operations(self):
Expand Down
5 changes: 4 additions & 1 deletion tests/model_fields_/test_autofield.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from django.core.exceptions import ValidationError
from django.test import SimpleTestCase

from django_mongodb_backend.fields import ObjectIdAutoField
Expand All @@ -17,4 +18,6 @@ def test_get_internal_type(self):

def test_to_python(self):
f = ObjectIdAutoField()
self.assertEqual(f.to_python("1"), 1)
msg = "“1” is not a valid Object Id."
with self.assertRaisesMessage(ValidationError, msg):
f.to_python("1")

0 comments on commit 5e98655

Please sign in to comment.