Code

#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.

Attachments (0)

Change History (4)

comment:1 Changed 14 months ago by mrmachine

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

Refs #10498.

comment:3 Changed 13 months 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 11 months 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.