Opened 104 minutes ago
Last modified 100 minutes ago
#37024 assigned Cleanup/optimization
Integer SITE_ID check incorrect when a different primary key type is used
| Reported by: | Tim Graham | Owned by: | Tim Graham |
|---|---|---|---|
| Component: | contrib.sites | Version: | dev |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Unreviewed | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
To catch a programmer mistake where the SITE_ID setting is defined with an incorrect type (e.g. SITE_ID = "1"), #31802 added a system check that only allows SITE_ID to be an int or None. This forces third-party databases with non-integer primary keys like MongoDB (which uses ObjectId) to silence this check.
It would be more appropriate if this check performed the validation based on Site._meta.pk rather than with hardcoded types.
I thought of a few options:
- Use
Site._meta.pk.to_python(settings.SITE_ID)to validate theSITE_ID, relying on it to raiseValidationErrorfor unexpected types. This won't work becauseAutoField.to_python()accepts strings that coerce to integer which are values the original fix was intended to reject.
- Use
Site._meta.pk.to_python(settings.SITE_ID)but also check ifsettings.SITE_ID != Site._meta.pk.to_python(settings.SITE_ID). This would catch invalid values thatto_python()coerces to the correct type.
- Add
Field.expected_typewhich could beint,ObjectId, or whatever. The implementation of the check could continue to useisinstance(). The downside is that adding a new attribute to theFieldAPI would be non-trivial.
I've implemented option #2.
PR