Opened 5 years ago

Closed 5 years ago

#29979 closed Cleanup/optimization (fixed)

Refactor AutoField logic into a mixin, implement checks and validators.

Reported by: Nick Pope Owned by: Nick Pope
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: autofield, validators
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently AutoField inherits from Field and BigAutoField from AutoField. In effect they largely redefine IntegerField and BigIntegerField respectively, but add in the auto field "behaviour". As a result they do not perform some of the system checks, e.g. max_length warning, nor the validation checks, e.g. range checks, that the integer fields do.

The proposal is to move all the auto field "behaviour" into a new AutoFieldMixin and fix AutoField and BigAutoField to inherit from this new mixin and IntegerField and BigIntegerField respectively.

Many attributes and methods would be nicely inherited from the correct parent field type without requiring redefinition:

  • description
  • empty_strings_allowed
  • default_error_messages
  • get_prep_value()
  • to_python()

AutoField and BigAutoField could also inherit the following checks from IntegerField:

  • IntegerField._check_max_length_warning()

AutoField and BigAutoField could also perform minimum and maximum value validation checks inherited from IntegerField.

This should be backwards compatible and potentially will make it easier to define new types of auto fields based on other fields in the future.

Change History (5)

comment:1 by Nick Pope, 5 years ago

Has patch: set

comment:2 by Carlton Gibson, 5 years ago

Triage Stage: UnreviewedAccepted

Conversation on the PR is already on-going. I’ll provisionally accept this pending an implementation everyone is happy with.
(Seems reasonable in principle.)

comment:3 by Tim Graham, 5 years ago

Patch needs improvement: set

comment:4 by Nick Pope, 5 years ago

Patch needs improvement: unset

PR updated. It seems that this also partially addresses #17337, but that will further depend on #470 for default values for non-integer fields as well as #24042 for replacing isinstance(..., AutoField) checks.

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 21e5594:

Fixed #29979, Refs #17337 -- Extracted AutoField field logic into a mixin and refactored AutoFields.

This reduces duplication by allowing AutoField, BigAutoField and
SmallAutoField to inherit from IntegerField, BigIntegerField and
SmallIntegerField respectively. Doing so also allows for enabling the
max_length warning check and minimum/maximum value validation for auto
fields, as well as providing a mixin that can be used for other possible
future auto field types such as a theoretical UUIDAutoField.

Note: See TracTickets for help on using tickets.
Back to Top