Minor improvements and refactoring in PageArray and WireArray, plus this should also fix processwire/processwire-issues#1416

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
+			// if this (maybe unreachable) point is reached, then index needs to 
-			// because either item is no longer here, or has moved
+			// 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");
+			$page = $this->wire()->pages->get("id=$page");
-			if($page->id) {
+			if($page->id) parent::add($page); 
-				parent::add($page); 
-				$this->numTotal++;
-			}
 		}
 		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,8 +711,13 @@ class PageArray extends PaginatedArray implements WirePaginatable {
 	 */
 	protected function trackRemove($item, $key) {
 		parent::trackRemove($item, $key);
+		if(!$item instanceof Page) return;
+		/** @var Page $item */
+		if(isset($this->keyIndex[$item->id])) {
+			if($this->numTotal) $this->numTotal--;
 			unset($this->keyIndex[$item->id]);
 		}
 	}
+}
 
 

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->isValidItem($value)) {
-		if(!$this->isValidKey($key)) throw new WireException("Key '$key' is not an allowed key for " . get_class($this));
+			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); 
 	}
 
@@ -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); 
+		if($limit) {
-			else $slice = array_slice($this->data, $start); 
+			$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; 
 	}