Refactor booking list data to use typed model instead of raw array round-trip

src/Model/BookingListEntry.php

@@ -0,0 +1,197 @@
+<?php
+
+namespace CommonsBooking\Model;
+
+use ArrayAccess;
+use ArrayIterator;
+use Countable;
+use IteratorAggregate;
+use JsonSerializable;
+use Traversable;
+
+/**
+ * One row of the booking list (@see \CommonsBooking\View\Booking::getBookingListData()), pairing
+ * the typed Booking model with its rendered row data.
+ *
+ * The known/fixed row fields (see KNOWN_FIELD_DEFAULTS) are real typed properties, so internal code
+ * can use e.g. $entry->postID directly. commonsbooking_booking_filter callbacks are free to add
+ * arbitrary extra keys though (that's part of the hook's contract), so anything outside the known
+ * schema is kept in an untyped overflow bag instead of being rejected.
+ *
+ * Implements ArrayAccess/IteratorAggregate/Countable so that existing filter callbacks written
+ * against the old plain assoc array (e.g. $rowData['postID'], foreach/count over it) keep working
+ * unchanged against this object. Callbacks that pass the row into a function that requires a literal
+ * array (array_diff_key, preg_grep, etc.) need to call toArray() first, since PHP's array_*
+ * functions don't accept ArrayAccess objects.
+ *
+ * This object is only reduced to a plain array at the webservice boundary, via JsonSerializable,
+ * when wp_json_encode() serializes the AJAX response in Booking::getTemplateData(). Everywhere else
+ * it stays typed.
+ */
+class BookingListEntry implements ArrayAccess, IteratorAggregate, Countable, JsonSerializable {
+
+	public Booking $booking;
+
+	public int $postID;
+
+	public int $startDate;
+
+	/**
+	 * @var int|false Mirrors \CommonsBooking\Model\Booking::getEndDate(), which can return false.
+	 */
+	public $endDate;
+
+	public string $startDateFormatted;
+
+	public string $endDateFormatted;
+
+	public string $item;
+
+	public string $location;
+
+	public string $locationAddr;
+
+	public mixed $locationLat;
+
+	public mixed $locationLong;
+
+	public string $bookingDate;
+
+	public string $user;
+
+	public string $status;
+
+	public mixed $fullDay;
+
+	public string $calendarLink;
+
+	public array $content;
+
+	public ?array $bookingCode;
+
+	public string $actions;
+
+	/**
+	 * Overflow bag for keys outside the known schema above -- e.g. custom fields added by a
+	 * commonsbooking_booking_filter callback. Kept untyped on purpose, since the hook explicitly
+	 * allows arbitrary extension.
+	 *
+	 * @var array
+	 */
+	private array $extra = [];
+
+	/**
+	 * Default values for the known/fixed row fields, also used as the authoritative list of which
+	 * keys are known (as opposed to landing in the untyped overflow bag).
+	 */
+	private const KNOWN_FIELD_DEFAULTS = [
+		'postID'             => 0,
+		'startDate'          => 0,
+		'endDate'            => 0,
+		'startDateFormatted' => '',
+		'endDateFormatted'   => '',
+		'item'               => '',
+		'location'           => '',
+		'locationAddr'       => '',
+		'locationLat'        => 0,
+		'locationLong'       => 0,
+		'bookingDate'        => '',
+		'user'               => '',
+		'status'             => '',
+		'fullDay'            => null,
+		'calendarLink'       => '',
+		'content'            => [],
+		'bookingCode'        => null,
+		'actions'            => '',
+	];
+
+	public function __construct( Booking $booking, array $rowData ) {
+		$this->booking = $booking;
+		foreach ( self::KNOWN_FIELD_DEFAULTS as $field => $default ) {
+			$this->$field = $default;
+		}
+		foreach ( $rowData as $key => $value ) {
+			$this->offsetSet( $key, $value );
+		}
+	}
+
+	/**
+	 * Returns the row data as a plain array. Use this when passing the row into code that requires
+	 * a literal array (e.g. array_diff_key(), preg_grep()).
+	 *
+	 * NOTE: 'bookingCode' is omitted entirely (not included as null) when a booking has no code,
+	 * matching the original conditional array_key_exists()/isset() behavior for that key.
+	 *
+	 * @return array
+	 */
+	public function toArray(): array {
+		$data = [];
+		foreach ( array_keys( self::KNOWN_FIELD_DEFAULTS ) as $field ) {
+			if ( $field === 'bookingCode' && $this->bookingCode === null ) {
+				continue;
+			}
+			$data[ $field ] = $this->$field;
+		}
+
+		return array_merge( $data, $this->extra );
+	}
+
+	public function offsetExists( $offset ): bool {
+		if ( array_key_exists( $offset, self::KNOWN_FIELD_DEFAULTS ) ) {
+			return $this->$offset !== null;
+		}
+
+		return array_key_exists( $offset, $this->extra );
+	}
+
+	public function offsetGet( $offset ): mixed {
+		if ( array_key_exists( $offset, self::KNOWN_FIELD_DEFAULTS ) ) {
+			return $this->$offset;
+		}
+
+		return $this->extra[ $offset ] ?? null;
+	}
+
+	public function offsetSet( $offset, $value ): void {
+		if ( $offset === null ) {
+			$this->extra[] = $value;
+			return;
+		}
+
+		if ( array_key_exists( $offset, self::KNOWN_FIELD_DEFAULTS ) ) {
+			$this->$offset = $value;
+			return;
+		}
+
+		$this->extra[ $offset ] = $value;
+	}
+
+	public function offsetUnset( $offset ): void {
+		if ( array_key_exists( $offset, self::KNOWN_FIELD_DEFAULTS ) ) {
+			// Known fields are real typed properties and structurally always present; "unsetting"
+			// one resets it to its schema default rather than truly removing it.
+			$this->$offset = self::KNOWN_FIELD_DEFAULTS[ $offset ];
+			return;
+		}
+
+		unset( $this->extra[ $offset ] );
+	}
+
+	public function getIterator(): Traversable {
+		return new ArrayIterator( $this->toArray() );
+	}
+
+	public function count(): int {
+		return count( $this->toArray() );
+	}
+
+	/**
+	 * Called by wp_json_encode()/json_encode() at the webservice boundary. Not meant to be called
+	 * directly elsewhere -- use toArray() for that.
+	 *
+	 * @return array
+	 */
+	public function jsonSerialize(): array {
+		return $this->toArray();
+	}
+}

src/View/Booking.php

@@ -7,6 +7,7 @@
 use Exception;
 
 use CommonsBooking\Helper\Wordpress;
+use CommonsBooking\Model\BookingListEntry;
 use CommonsBooking\Plugin;
 use CommonsBooking\Settings\Settings;
 use CommonsBooking\Service\iCalendar;
@@ -28,11 +29,29 @@ class Booking extends View {
 	 */
 	public static function getTemplateData(): void {
 		header( 'Content-Type: application/json' );
+
+		// This is the webservice boundary: wp_json_encode() reduces the BookingListEntry objects
+		// returned by getBookingListData() to plain arrays right here, via BookingListEntry::jsonSerialize().
+		// Nowhere earlier in the codebase are they turned into arrays.
 		echo wp_json_encode( self::getBookingListData() );
 		wp_die(); // All ajax handlers die when finished
 	}
 
 	/**
+	 * Returns paginated/filtered/sorted booking list data.
+	 *
+	 * NOTE: The 'data' entry of the returned array holds a list of {@see BookingListEntry} objects,
+	 * pairing each row's typed Booking model with its rendered row data. Known row fields (postID,
+	 * startDate, item, ...) are real typed properties on the entry (e.g. $entry->postID); fields
+	 * added by a commonsbooking_booking_filter callback outside that schema land in an untyped
+	 * overflow bag instead of being rejected. Entries stay typed objects throughout -- they behave
+	 * like the old plain assoc array (ArrayAccess/Countable/foreach) for compatibility with existing
+	 * filter callbacks, but are only actually turned into arrays where required: at the webservice
+	 * boundary when JSON-encoded ({@see self::getTemplateData()}), or explicitly via
+	 * {@see BookingListEntry::toArray()}. Internal consumers (e.g. {@see self::getBookingListiCal()})
+	 * should use the Booking model directly via the entry's public $booking property instead of
+	 * re-fetching it.
+	 *
 	 * @param int           $postsPerPage
 	 * @param \WP_User|null $user
 	 *
@@ -172,9 +191,9 @@ public static function getBookingListData( int $postsPerPage = 6, ?\WP_User $use
 				$location      = $booking->getLocation();
 				$locationTitle = $location ? $booking->getLocation()->post_title : commonsbooking_sanitizeHTML( __( 'Not available', 'commonsbooking' ) );
 
-				// Prepare row data
-				// FIXME This untyped structure is exposed via the filter commonsbooking_booking_filter below, but the set of keys of the assoc array must not be changed. This is not ideal and should be either replace by a dedicated object type or removed entirely.
-				// If not, why not expose this as own type?
+				// Prepare row data. Its set of keys is public API (exposed via commonsbooking_booking_filter
+				// and the AJAX JSON response below), so it must not change. It gets paired with the typed
+				// $booking model into a BookingListEntry before being passed to the filter.
 				$rowData = [
 					'postID'             => $booking->ID,
 					'startDate'          => $booking->getStartDate(),
@@ -239,31 +258,39 @@ public static function getBookingListData( int $postsPerPage = 6, ?\WP_User $use
 					$rowData['actions'] = $actions;
 
 					/**
-					 * Default assoc array of row data and the booking object, which gets added to the booking list data result.
+					 * Row data for one booking, which gets added to the booking list data result.
 					 *
-					 * NOTE: Upon using this filter hook, the schema of associative array keys needs to be adhered to in order to not break the booking list.
-					 * See $rowData in this function, for the valid keys.
+					 * NOTE: $row behaves like the plain assoc array this filter used to receive
+					 * (ArrayAccess/Countable/foreach all still work, e.g. $row['postID']), so the
+					 * schema of keys still needs to be adhered to in order to not break the booking
+					 * list. See $rowData in this function, for the valid keys. A plain array can
+					 * still be returned instead for backwards compatibility. Return null to drop the
+					 * row from the list.
 					 *
-					 * @param array|mixed|null              $rowData assoc array of one row booking data
+					 * @param BookingListEntry|array|null   $row     row data for one booking
 					 * @param \CommonsBooking\Model\Booking $booking booking model of one row booking data
 					 *
 					 *@since 2.7.3
 					 */
-					$filteredRowData = apply_filters( 'commonsbooking_booking_filter', $rowData, $booking );
-
-					// Only includes non-null array row data objects
-					if ( isset( $filteredRowData ) && is_array( $filteredRowData ) ) {
-						if ( WP_DEBUG ) {
-							// Logs absent keys, relative to the original row data keys, could cause problems
-							$absentKeys = array_diff_key( $filteredRowData, $rowData );
-							if ( count( $absentKeys ) > 0 ) {
-								error_log( 'After commonsbooking_booking_filter: Filtered rows have missing keys: ' . join( ',', array_keys( $absentKeys ) ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log
-							}
-						}
-					} else {
+					$filteredRow = apply_filters( 'commonsbooking_booking_filter', new BookingListEntry( $booking, $rowData ), $booking );
+
+					// Only includes non-null row data, normalized back to a BookingListEntry for
+					// callbacks that still return a plain array.
+					if ( is_array( $filteredRow ) ) {
+						$filteredRow = new BookingListEntry( $booking, $filteredRow );
+					} elseif ( ! $filteredRow instanceof BookingListEntry ) {
 						continue;
 					}
-					$bookingDataArray['data'][] = $filteredRowData;
+
+					if ( WP_DEBUG ) {
+						// Logs absent keys, relative to the original row data keys, could cause problems
+						$absentKeys = array_diff_key( $filteredRow->toArray(), $rowData );
+						if ( count( $absentKeys ) > 0 ) {
+							error_log( 'After commonsbooking_booking_filter: Filtered rows have missing keys: ' . join( ',', array_keys( $absentKeys ) ) ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log
+						}
+					}
+
+					$bookingDataArray['data'][] = $filteredRow;
 				}
 			}
 
@@ -287,7 +314,7 @@ public static function getBookingListData( int $postsPerPage = 6, ?\WP_User $use
 
 				// Init function to pass sort and order param to sorting callback
 				$sorter = function ( $sort, $order ) {
-					return function ( $a, $b ) use ( $sort, $order ) {
+					return function ( BookingListEntry $a, BookingListEntry $b ) use ( $sort, $order ) {
 						if ( $order == 'asc' ) {
 							return strcasecmp( $a[ $sort ], $b[ $sort ] );
 						} else {
@@ -527,8 +554,9 @@ public static function getBookingListiCal( $user = null ) {
 
 		$calendar = new iCalendar();
 
-		foreach ( $bookingList['data'] as $bookingData ) {
-			$booking = \CommonsBooking\Repository\Booking::getPostById( $bookingData['postID'] );
+		/** @var BookingListEntry $entry */
+		foreach ( $bookingList['data'] as $entry ) {
+			$booking = $entry->booking;
 			if ( ! $booking->isConfirmed() ) {
 				continue;
 			}