Opened 2 years ago

Closed 2 years ago

#20348 closed Bug (fixed)

Consistently handle `Promise` objects assigned to model fields.

Reported by: mrmachine Owned by: nobody
Component: Database layer (models, ORM) Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently, all Promise objects are passed to force_text() deep in ORM query code. Not only does this make it difficult or impossible for developers to prevent or alter this behaviour, but it is also wrong for non-text fields.

Field.get_prep_value() seems like a better place to handle Promise objects, and _proxy____cast() seems like a better way to do it than passing them through force_text(). All Field subclasses should call get_prep_value() on their super class to ensure they have a real value to work with.

This change would also facilitate the creation of custom fields like PickleField, which *can* store Promise objects, to override this behaviour.

Change History (4)

comment:1 Changed 2 years ago by mrmachine

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

Refs #10498.

comment:3 Changed 2 years ago by akaariai

  • Triage Stage changed from Unreviewed to Ready for checkin

I like this. This means that lazy values are always evaluated at the time they are needed by the DB and one can customize the way the promise object is evaluated per field.

Ill mark this as ready for checkin. The issues keeping me from committing this straight ahead:

  • Is this too risky to commit in alpha stage?
  • Could this lead to incompatibilities for custom fields?
  • It seems get_prep_lookup() doesn't always use get_prep_value(). I wonder if this could be fixed...

But, like the "ready for checkin" triage stage indicates I don't see any of the above as blockers for commit. Waiting a bit to see if any other opinions arise.

comment:4 Changed 2 years ago by Anssi Kääriäinen <akaariai@…>

  • Resolution set to fixed
  • Status changed from new to closed

In 31e6d58d46894ca35080b4eab7967e4c6aae82d4:

Fixed #20348 -- Consistently handle Promise objects in model fields.

All Promise objects were passed to force_text() deep in ORM query code.
Not only does this make it difficult or impossible for developers to
prevent or alter this behaviour, but it is also wrong for non-text
fields.

This commit changes Field.get_prep_value() from a no-op to one that
resolved Promise objects. All subclasses now call super() method first
to ensure that they have a real value to work with.

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