Problem/Motivation

Deprecate Drupal\Core\Database\StatementEmpty as it only is used in testing and therefore does not belong in the main namespace.

Proposed resolution

Deprecate Drupal\Core\Database\StatementEmpty and replace where it is used. For pure testing, StatementInterface could be mocked to return empty values.

Remaining tasks

TBD

User interface changes

None

API changes

The class Drupal\Core\Database\StatementEmpty will be deprecated.

Data model changes

None

Release notes snippet

TBD

Issue fork drupal-3186795

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

daffie created an issue. See original summary.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Active » Postponed

Postponed on #3177660: Remove public properties from StatementInterface implementations.

daffie’s picture

daffie CreditAttribution: daffie commented
Title: Remove Drupal\Core\Database\StatementEmpty as it only is used in testing and replace it with mocks » Deprecate Drupal\Core\Database\StatementEmpty as it only is used in testing and replace it with mocks
Issue summary: View changes
donquixote’s picture

donquixote CreditAttribution: donquixote as a volunteer commented

Interesting.
#3195524: $query->execute() should throw exception on failure, not return NULL
Here I suggest the opposite: Use StatementEmpty as a return value for select queries where we can determine the result to be empty before running the actual query.

(thanks @mondrake for linking)

mondrake made their first commit to this issue’s fork.

mondrake opened merge request !377

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Status: Postponed » Needs review

IMHO it's not necessary to wait on #3177660: Remove public properties from StatementInterface implementations.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented

Rerolled

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Title: Deprecate Drupal\Core\Database\StatementEmpty as it only is used in testing and replace it with mocks » Deprecate Drupal\Core\Database\StatementEmpty
Issue summary: View changes

Maybe we can just return NULL from ::queryRange of test drivers, that makes the entire class dead code.

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake as a volunteer commented
Priority: Minor » Normal

I do not think this is minor.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Reviewed & tested by the community

All the code changes llok good to me.
The class StatementEmpty has a deprecation and it is tested.
I have updated the CR.
For me it is RTBC.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Nice clean-up.

Committed c72c953 and pushed to 9.2.x. Thanks!

  • catch committed c72c953 on 9.2.x 
    Issue #3186795 by mondrake, daffie: Deprecate Drupal\Core\Database\...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

mondrake closed merge request !377

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.