From 1890b68505d0499d6e8745d7246bce4e5c3560b1 Mon Sep 17 00:00:00 2001 From: Ryan Cramer Date: Wed, 28 Jul 2021 09:55:24 -0400 Subject: [PATCH] Minor improvements and refactoring in PageArray and WireArray, plus this should also fix processwire/processwire-issues#1416 --- wire/core/PageArray.php | 132 ++++++++-------------------------------- wire/core/WireArray.php | 43 +++++++------ 2 files changed, 51 insertions(+), 124 deletions(-) diff --git a/wire/core/PageArray.php b/wire/core/PageArray.php index ecdddb59..046e68c4 100644 --- a/wire/core/PageArray.php +++ b/wire/core/PageArray.php @@ -28,7 +28,7 @@ * ~~~~~ * #pw-body * - * ProcessWire 3.x, Copyright 2019 by Ryan Cramer + * ProcessWire 3.x, Copyright 2021 by Ryan Cramer * https://processwire.com * * @method string getMarkup($key = null) Render a simple/default markup value for each item #pw-internal @@ -123,14 +123,14 @@ class PageArray extends PaginatedArray implements WirePaginatable { // given item exists in this PageArray (or at least has) $key = $this->keyIndex[$id]; if(isset($this->data[$key])) { - $page = $this->data[$key]; + $page = $this->data[$key]; /** @var Page $page */ if($page->id === $id) { // found it return $key; } } - // if this point is reached, then index needs to be rebuilt - // because either item is no longer here, or has moved + // if this (maybe unreachable) point is reached, then index needs to + // be rebuilt because either item is no longer here, or has moved $this->keyIndex = array(); foreach($this->data as $key => $page) { $this->keyIndex[$page->id] = $key; @@ -163,7 +163,7 @@ class PageArray extends PaginatedArray implements WirePaginatable { * */ public function makeBlankItem() { - return $this->wire('pages')->newPage(); + return $this->wire()->pages->newPage(); } /** @@ -197,19 +197,13 @@ class PageArray extends PaginatedArray implements WirePaginatable { if(!self::iterable($pages)) return $this; foreach($pages as $page) $this->add($page); if($pages instanceof PageArray) { - if(count($pages) < $pages->getTotal()) $this->setTotal($this->getTotal() + ($pages->getTotal() - count($pages))); + if(count($pages) < $pages->getTotal()) { + $this->setTotal($this->getTotal() + ($pages->getTotal() - count($pages))); + } } return $this; } - /* - public function get($key) { - if(ctype_digit("$key")) return parent::get($key); - @todo check if selector, then call findOne(). If it returns null, return a NullPage instead. - return null; - } - */ - /** * Does this PageArray contain the given index or Page? * @@ -252,106 +246,17 @@ class PageArray extends PaginatedArray implements WirePaginatable { if($this->isValidItem($page)) { parent::add($page); - $this->numTotal++; } else if($page instanceof PageArray || is_array($page)) { return $this->import($page); } else if(ctype_digit("$page")) { - $page = $this->wire('pages')->get("id=$page"); - if($page->id) { - parent::add($page); - $this->numTotal++; - } + $page = $this->wire()->pages->get("id=$page"); + if($page->id) parent::add($page); } return $this; } - - /** - * Sets an index in the PageArray. - * - * #pw-internal - * - * @param int $key Key of item to set. - * @param Page $value Value of item. - * @return $this - * - */ - public function set($key, $value) { - $has = $this->has($key); - parent::set($key, $value); - if(!$has) $this->numTotal++; - return $this; - } - - /** - * Prepend a Page to the beginning of the PageArray. - * - * #pw-internal - * - * @param Page|PageArray $item - * @return WireArray This instance. - * - */ - public function prepend($item) { - parent::prepend($item); - // note that WireArray::prepend does a recursive call to prepend with each item, - // so it's only necessary to increase numTotal if the given item is Page (vs. PageArray) - if($item instanceof Page) $this->numTotal++; - return $this; - } - - - /** - * Remove the given Page or key from the PageArray. - * - * #pw-internal - * - * @param int|Page $key - * @return $this This PageArray instance - * - */ - public function remove($key) { - - // if a Page object has been passed, determine its key - if($this->isValidItem($key)) { - $key = $this->getItemKey($key); - } - if($this->has($key)) { - parent::remove($key); - $this->numTotal--; - } - - return $this; - } - - /** - * Shift the first Page off of the PageArray and return it. - * - * #pw-internal - * - * @return Page|NULL - * - */ - public function shift() { - if($this->numTotal) $this->numTotal--; - return parent::shift(); - } - - /** - * Pop the last page off of the PageArray and return it. - * - * #pw-internal - * - * @return Page|NULL - * - */ - public function pop() { - if($this->numTotal) $this->numTotal--; - return parent::pop(); - } - /** * Get one or more random pages from this PageArray. * @@ -641,7 +546,12 @@ class PageArray extends PaginatedArray implements WirePaginatable { * */ protected function filterDataSelectors(Selectors $selectors) { - // @todo make it remove references to include= statements since not applicable in-memory + $disallowed = array('include', 'check_access', 'checkAccess'); + foreach($selectors as $selector) { + if(in_array($selector->field(), $disallowed)) { + $selectors->remove($selector); + } + } parent::filterDataSelectors($selectors); } @@ -786,6 +696,9 @@ class PageArray extends PaginatedArray implements WirePaginatable { */ protected function trackAdd($item, $key) { parent::trackAdd($item, $key); + if(!$item instanceof Page) return; + /** @var Page $item */ + if(!isset($this->keyIndex[$item->id])) $this->numTotal++; $this->keyIndex[$item->id] = $key; } @@ -798,7 +711,12 @@ class PageArray extends PaginatedArray implements WirePaginatable { */ protected function trackRemove($item, $key) { parent::trackRemove($item, $key); - unset($this->keyIndex[$item->id]); + if(!$item instanceof Page) return; + /** @var Page $item */ + if(isset($this->keyIndex[$item->id])) { + if($this->numTotal) $this->numTotal--; + unset($this->keyIndex[$item->id]); + } } } diff --git a/wire/core/WireArray.php b/wire/core/WireArray.php index 0b215569..5ab710f9 100644 --- a/wire/core/WireArray.php +++ b/wire/core/WireArray.php @@ -10,9 +10,8 @@ * WireArray is the base of the PageArray (subclass) which is the most used instance. * * @todo can we implement next() and prev() like on Page, as alias to getNext() and getPrev()? - * @todo narrow down to one method of addition and removal, especially for removal, i.e. make shift() run through remove() * - * ProcessWire 3.x, Copyright 2016 by Ryan Cramer + * ProcessWire 3.x, Copyright 2021 by Ryan Cramer * https://processwire.com * * @method WireArray and($item) @@ -405,7 +404,7 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count $a = $this->get($itemA); $b = $this->get($itemB); if($a && $b) { - // swap a and b + // swap a and b, both already present in this WireArray $data = $this->data; foreach($data as $key => $value) { $k = null; @@ -455,8 +454,12 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count */ public function set($key, $value) { - if(!$this->isValidItem($value)) throw new WireException("Item '$key' set to " . get_class($this) . " is not an allowed type"); - if(!$this->isValidKey($key)) throw new WireException("Key '$key' is not an allowed key for " . get_class($this)); + if(!$this->isValidItem($value)) { + throw new WireException("Item '$key' set to " . get_class($this) . " is not an allowed type"); + } + if(!$this->isValidKey($key)) { + throw new WireException("Key '$key' is not an allowed key for " . get_class($this)); + } $this->trackChange($key, isset($this->data[$key]) ? $this->data[$key] : null, $value); $this->data[$key] = $value; @@ -476,7 +479,9 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count * */ public function __set($property, $value) { - if($this->getProperty($property)) throw new WireException("Property '$property' is a reserved word and may not be set by direct reference."); + if($this->getProperty($property)) { + throw new WireException("Property '$property' is a reserved word and may not be set by direct reference."); + } $this->set($property, $value); } @@ -666,7 +671,7 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count 'first' => 'first', 'keys' => 'getKeys', 'values' => 'getValues', - ); + ); if(!in_array($property, $properties)) return $this->data($property); $func = $properties[$property]; @@ -941,8 +946,11 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count * */ public function slice($start, $limit = 0) { - if($limit) $slice = array_slice($this->data, $start, $limit); - else $slice = array_slice($this->data, $start); + if($limit) { + $slice = array_slice($this->data, $start, $limit); + } else { + $slice = array_slice($this->data, $start); + } $items = $this->makeNew(); $items->import($slice); $items->setTrackChanges(true); @@ -987,7 +995,6 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count reset($this->data); $key = key($this->data); } - //if($item instanceof Wire) $item->setTrackChanges(); $this->trackChange('prepend', null, $item); $this->trackAdd($item, $key); return $this; @@ -1206,12 +1213,11 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count $key = $this->getItemKey($key); } - if($this->has($key)) { + if(array_key_exists($key, $this->data)) { $item = $this->data[$key]; unset($this->data[$key]); $this->trackChange("remove", $item, null); $this->trackRemove($item, $key); - } return $this; @@ -1475,7 +1481,7 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count protected function filterData($selectors, $not = false) { if(is_object($selectors) && $selectors instanceof Selectors) { - // fantastic + $selectors = clone $selectors; } else { if(!is_array($selectors) && ctype_digit("$selectors")) $selectors = "id=$selectors"; $selector = $selectors; @@ -1485,6 +1491,7 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count $this->filterDataSelectors($selectors); + $fields = $this->wire()->fields; $sort = array(); $start = 0; $limit = null; @@ -1509,7 +1516,7 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count // use only the last limit selector $limit = (int) $selector->value; - } else if(($field === 'index' || $field == 'eq') && !$this->wire('fields')->get($field)) { + } else if(($field === 'index' || $field == 'eq') && !$fields->get($field)) { // eq or index properties switch($selector->value) { case 'first': $eq = 0; break; @@ -1533,7 +1540,9 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count $qty++; if(is_array($selector->field)) { $value = array(); - foreach($selector->field as $field) $value[] = (string) $this->getItemPropertyValue($item, $field); + foreach($selector->field as $field) { + $value[] = (string) $this->getItemPropertyValue($item, $field); + } } else { $value = (string) $this->getItemPropertyValue($item, $selector->field); } @@ -1588,7 +1597,7 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count $this->data = array_slice($this->data, $start, $limit, true); } - $this->trackChange("filterData:$selectors"); + if($this->trackChanges()) $this->trackChange("filterData:$selectors"); return $this; } @@ -2069,7 +2078,7 @@ class WireArray extends Wire implements \IteratorAggregate, \ArrayAccess, \Count 'skipEmpty' => true, 'prepend' => '', 'append' => '' - ); + ); if(!count($this->data)) return '';