Add ForeignKey support to REQUIRED_FIELDS

[Chris Jerdonek]

If instance=None is passed to _route_db in db.utils, then an exception like the following is raised:

  ...
  File ".../django/core/management/base.py", line 285, in execute
    output = self.handle(*args, **options)
  File ".../django/contrib/auth/management/commands/createsuperuser.py", line 96, in handle
    username = self.username_field.clean(raw_value, None)
  File ".../django/db/models/fields/__init__.py", line 255, in clean
    self.validate(value, model_instance)
  File ".../django/db/models/fields/related.py", line 1199, in validate
    using = router.db_for_read(model_instance.__class__, instance=model_instance)
  File ".../django/db/utils.py", line 250, in _route_db
    return hints['instance']._state.db or DEFAULT_DB_ALIAS
AttributeError: 'NoneType' object has no attribute '_state'

The code is ​here:

try:
    return hints['instance']._state.db or DEFAULT_DB_ALIAS
except KeyError:
    return DEFAULT_DB_ALIAS

It would be better if the None case were handled in the same way that KeyError is being caught.

[Baptiste Mispelon]

Hi,

It's not clear to me whether you've managed to trigger this error while using the ORM or if you have a use-case where you'd like to be able to pass instance=None as a hint to the db router.

If it's the former, do you have a bit of code that reproduces the isssue and if it's the latter, could you explain why you need to pass instance=None rather than not passing an instance at all?

Thanks.

[Chris Jerdonek]

Hi, I've been meaning to elaborate on this because I'm not sure if this should be changed as I've described.

This error occurred when setting the USERNAME_FIELD to a foreign key field and running the createsuperuser command. The base field class's validate() method ​doesn't use the model_instance argument, but the ForeignKey field version's ​does.

Obviously, I needed to change the createsuperuser command in other ways to get createsuperuser to work in my use case above, but this error I encountered in the process seemed unnecessarily obscure.

The possible resolutions of this issue I can think of are:

• pass a valid model_instance instead of None in the default implementation of createsuperuser, even though it is not needed in the out-of-the-box case,
• allow None to be passed in the foreign key field version of validate() (for example by catching the AttributeError in db.utils _route_db() as I described in my original suggestion),
• raising a simpler-to-decipher error earlier in the process (e.g. by raising in createsuperuser or elsewhere that foreign key fields aren't allowed), or
• leaving things as is.

Thanks for considering this.

[Chris Jerdonek]

By the way, this ticket isn't the place to resolve the following issue in general, but my preference would be for Django to make it easier to support the following two use cases out of the box: (1) USERNAME_FIELD being a foreign key, and (2) USERNAME_FIELD being a tuple of fields. I may open tickets for these two issues at some point if they haven't already been suggested. If the resolution of this ticket can reduce one roadblock towards use case (1), that would be good. The current issue is simply one of the first places where things went wrong.

[Baptiste Mispelon]

OK, I'm starting to understand a bit better (thanks for clarifying).

I couldn't find any official documentation for models.Field.validate() and in particular, I don't think we've documented what's acceptable to pass as model_instance or not.

Typically, Field.validate is called as part of model validation (Model.clean_fields) which always passes a valid model_instance) so there's no issue. In this case though, the problem arises because createsuperuser calls field validation directly, in which case it doesn't make much sense to pass a model_instance.

I think your option 2 is a valid approach, though I would use something like this rather than catching AttributeError (I feel that it's not specific enough):

instance = hints.get('instance')
if instance is not None and instance._state.db:
    return instance._state.db
return DEFAULT_DB_ALIAS

Regarding using a ForeignKey as a USERNAME_FIELD, while it doesn't seem like a very common use-case, it's not explicitly forbidden by the documentation [1] which only requires the field to be unique.
I suggest opening a ticket regarding this feature so that we can either support it or at least document that it's not possible.

Thanks.

[1] ​https://docs.djangoproject.com/en/1.6/topics/auth/customizing/#django.contrib.auth.models.CustomUser.USERNAME_FIELD

[Chris Jerdonek]

I suggest opening a ticket regarding this feature so that we can either support it or at least document that it's not possible.

Thanks for your thoughts. Okay, I created #21832 for this.

[davidc]

I am running into the same problem for a foreign key in the REQUIRED_FIELDS of my custom user model. It does not have to be the USERNAME_FIELD. I think this would be much more common. In my case I need all users to have a department.

[Chris Jerdonek]

@davidc, for future reference, can you include the stack trace you are getting? The stack trace I included in my original post references the username field (in the line, username = self.username_field.clean(raw_value, None)), so it looks like there is another place in Django's code base that is passing None as the model instance.

[dcreech]

@cjerdonek, Here is the stack trace:

...
  File "C:\Python27\lib\site-packages\django\core\management\base.py", line 285, in execute
    output = self.handle(*args, **options)
  File "C:\Python27\lib\site-packages\django\contrib\auth\management\commands\createsuperuser.py", line 116, in handle
    user_data[field_name] = field.clean(raw_value, None)
  File "C:\Python27\lib\site-packages\django\db\models\fields\__init__.py", line 255, in clean
    self.validate(value, model_instance)
  File "C:\Python27\lib\site-packages\django\db\models\fields\related.py", line 1199, in validate
    using = router.db_for_read(model_instance.__class__, instance=model_instance)
  File "C:\Python27\lib\site-packages\django\db\utils.py", line 250, in _route_db
    return hints['instance']._state.db or DEFAULT_DB_ALIAS
AttributeError: 'NoneType' object has no attribute '_state'

[ANUBHAV JOSHI]

Replying to davidc:

I am running into the same problem for a foreign key in the REQUIRED_FIELDS of my custom user model. It does not have to be the USERNAME_FIELD. I think this would be much more common. In my case I need all users to have a department.

I think we cannot use ForeignKey here. See:​https://docs.djangoproject.com/en/1.6/topics/auth/customizing/#django.contrib.auth.models.CustomUser.REQUIRED_FIELDS

[ANUBHAV JOSHI]

​https://github.com/django/django/pull/2866

[ANUBHAV JOSHI]

For now, I have skipped the call to clean in case of FK and left the validation part of Descriptors.
I tried to implement what @bmispelon commented, but there seemed to a be problem with that. In validate() in related.py, after we get the correct qs, qs.exists() somehow retriggers call to validate() and we get empty qs and error is raised. Can anyone explain this?

EDITS:
I think that problem was due to some changes that I made myself.
Updated the patch with new approach,

[Tim Graham <timograham@…>]

In 9bc2d766a0c31701a4c0e20a8b04164bb22cf6d5:

Fixed #21755 -- Added ForeignKey support to REQUIRED_FIELDS.

This allows specifying ForeignKeys in REQUIRED_FIELDS when using a
custom User model.

Thanks cjerdonek and bmispelon for suggestion and timgraham for review.

[Mariusz Felisiak <felisiak.mariusz@…>]

In 4ff500f2:

Refs #21755 -- Fixed createsuperuser crash for required foreign keys passed in options in interactive mode.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@…>

[Mariusz Felisiak <felisiak.mariusz@…>]

In b55df4c7:

[4.0.x] Refs #21755 -- Fixed createsuperuser crash for required foreign keys passed in options in interactive mode.

Co-authored-by: Mariusz Felisiak <felisiak.mariusz@…>

Backport of 4ff500f2948bfc332b3f4159021cad06e91943d3 from main