Problem/Motivation

#2044435: Convert pager.inc to a service introduced the DeprecatedArray class to allow throwing deprecation error when accessing the legacy pager global variables.

However, this class only implements \ArrayAccess, the array eleemnts can not be traversed with foreach, nor casted to 'real' arrays.

Unfortunately this is going to break the Pagerer module in contrib, that is inspecting the $pager* array variables, and no longer can.

Proposed resolution

Implement some sort of iteration to the DeprecatedArray class (?)

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#10 3087602_10.patch3.31 KBGhost of Drupal Past
#6 3087602_6.patch2.67 KBGhost of Drupal Past
#3 3087602-3.patch1.78 KBkim.pepper
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mondrake created an issue. See original summary.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Iterator aggregate would be simplest

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper at PreviousNext commented
Status: Active » Needs review
FileSize
3087602-3.patch1.78 KB

Updated so that DeprecatedArray implements \ArrayAccess, \IteratorAggregate

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented
Status: Needs review » Reviewed & tested by the community

Thanks a lot!!

Ghost of Drupal Past’s picture

Ghost of Drupal Past
he/him
CreditAttribution: Ghost of Drupal Past at Smartsheet commented

This works but wouldn't https://www.php.net/arrayobject be better?

Ghost of Drupal Past’s picture

Ghost of Drupal Past
he/him
CreditAttribution: Ghost of Drupal Past at Smartsheet commented
Status: Reviewed & tested by the community » Needs review
FileSize
3087602_6.patch2.67 KB

Like this. Significantly simpler, isn't it? (I hope it works, too.)

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
CreditAttribution: kim.pepper at PreviousNext commented

I like your version better @chx 🙂

mondrake’s picture

mondrake
Location 🇮🇹
CreditAttribution: mondrake commented

Wow!

Would this also work with array functions? For instance, array_keys? ATM doing array_keys($pager*) fails because the variable is no longer strictly an array. Also, would it work while asserting equality in tests, e.g. assertEquals([5, 3], $pager_total)?

Ghost of Drupal Past’s picture

Ghost of Drupal Past
he/him
CreditAttribution: Ghost of Drupal Past at Smartsheet commented

Would this also work with array functions?

Nope. An ArrayObject is, well, an ArrayObject and those functions require an array. It's sad.

Ghost of Drupal Past’s picture

Ghost of Drupal Past
he/him
CreditAttribution: Ghost of Drupal Past at Smartsheet commented
FileSize
3087602_10.patch3.31 KB

I realize AO now implements four interfaces (ArrayAccess,IteratorAggregate,Serializeable,Countable) which all should throw deprecation notices. The patch is longer but equally boring.

Arguably it's a bug in core that offsetSet has a return value when the signature requires a void. I fixed this (and offsetUnset and added unserialize w/o a return).

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Priority: Critical » Major
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Acro Commerce, Thunder commented
Status: Reviewed & tested by the community » Fixed

Committed and pushed 31314fca7d to 9.0.x and e4fe714f5f to 8.9.x and d371e9cf36 to 8.8.x. Thanks!

diff --git a/core/lib/Drupal/Component/Utility/DeprecatedArray.php b/core/lib/Drupal/Component/Utility/DeprecatedArray.php
index da39284052..d8483ea686 100644
--- a/core/lib/Drupal/Component/Utility/DeprecatedArray.php
+++ b/core/lib/Drupal/Component/Utility/DeprecatedArray.php
@@ -91,5 +91,4 @@ public function count() {
     return parent::count();
   }
 
-
 }

Fixed coding standards on commit.

  • alexpott committed 31314fc on 9.0.x 
    Issue #3087602 by Charlie ChX Negyesi, kim.pepper, mondrake, larowlan:...

  • alexpott committed e4fe714 on 8.9.x 
    Issue #3087602 by Charlie ChX Negyesi, kim.pepper, mondrake, larowlan:...

  • alexpott committed d371e9c on 8.8.x 
    Issue #3087602 by Charlie ChX Negyesi, kim.pepper, mondrake, larowlan:...

Status: Fixed » Closed (fixed)

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