From ab93866295897348607a01366ee755b6fe37ce17 Mon Sep 17 00:00:00 2001 From: Jeppe Bundgaard Date: Tue, 3 Feb 2026 11:43:56 +0100 Subject: [PATCH] Add Redis connection checks and optimize caching for collected order invoices - Enhance Redis methods (`exists`, `setEx`, `delete`, `get`, `set`) to ensure connection before execution. - Introduce short-lived caching for collected order invoices to minimize redundant processing and improve performance. - Add `pagination_helper` for dynamic WHERE clause construction in queries. - Refactor net amount calculation in `collected_order_invoices_o` for efficiency with batch processing. - Extend `listObjectsWithPaginationIfSet` to support additional WHERE clauses. --- .../objects/collected_order_invoices_o.php | 29 ++++---- .../nginx/app/routes/orderInvoicesRoute.php | 72 ++++++++++++++++++- .../nginx/app/routes/pagination_helper.php | 65 +++++++++++++++++ services/nginx/app/traits/db_object_t.php | 24 ++++++- services/nginx/app/traits/redis_t.php | 15 ++++ 5 files changed, 186 insertions(+), 19 deletions(-) create mode 100644 services/nginx/app/routes/pagination_helper.php diff --git a/services/nginx/app/objects/collected_order_invoices_o.php b/services/nginx/app/objects/collected_order_invoices_o.php index e2556acd..3582f3f5 100644 --- a/services/nginx/app/objects/collected_order_invoices_o.php +++ b/services/nginx/app/objects/collected_order_invoices_o.php @@ -196,22 +196,23 @@ class collected_order_invoices_o extends db { // Require the invoice collection to be selected self::requireSelected(); - // Get the orders in the invoice collection + // Get the orders in the invoice collection (already filtered to those included in invoicing) $order_ids = self::getOrderIds(); - // Get the orders in the invoice collection - $net_amount = 0; - - foreach ( $order_ids as $order_id ) { - $order = new orders_o(); - $order->select($order_id['id']); - $order->requireSelected(); - if (!$order->isIncludedInInvoicing()) { - continue; - } - // Get the net amount of the order - $net_amount += $order->getNetAmount(); + if (empty($order_ids)) { + return 0.0; } - return $net_amount; + // Build a flat list of order IDs + $ids = array_map(static function ($row) { + return (int)$row['id']; + }, $order_ids); + if (empty($ids)) { + return 0.0; + } + // Compute net amounts in a single aggregated pass over order_items + $orders = new orders_o(); + $netByOrder = $orders->getNetAmountForOrders($ids); + // Sum per-order totals + return array_sum($netByOrder); } /** diff --git a/services/nginx/app/routes/orderInvoicesRoute.php b/services/nginx/app/routes/orderInvoicesRoute.php index e1d653d8..3ca53127 100644 --- a/services/nginx/app/routes/orderInvoicesRoute.php +++ b/services/nginx/app/routes/orderInvoicesRoute.php @@ -43,13 +43,81 @@ class orderInvoicesRoute // Define the collected order invoices $tmp_collected_order_invoices = new collected_order_invoices_o(); // Return the list of collected order invoices - $response->success($collected_order_invoices->listObjectsWithPaginationIfSet( + $startTime = microtime(true); + // Build a short-lived cache key to coalesce concurrent identical requests + $cacheTtl = 15; // seconds + $pageParam = (string)($response->getRequestParameter('page') ?? '1'); + $limitParam = (string)($response->getRequestParameter('limit') ?? '1000'); + $searchParam = (string)($response->getRequestParameter('search') ?? ''); + $orderParam = (string)($response->getRequestParameter('order') ?? 'id:ASC'); + $filtersParam = (string)($response->getRequestParameter('filters') ?? ''); + $cacheKey = 'collected_invoices:list:' . md5(json_encode([ + 'p' => $pageParam, + 'l' => $limitParam, + 's' => $searchParam, + 'o' => $orderParam, + 'f' => $filtersParam, + ], JSON_UNESCAPED_UNICODE)); + + $redis = new \classes\redis(); + $cachedPayload = $redis->get($cacheKey); + if ($cachedPayload) { + // Cached payload contains both meta and data + $payload = json_decode($cachedPayload, true); + if (isset($payload['meta']) && is_array($payload['meta'])) { + foreach ($payload['meta'] as $k => $v) { + $response->add_meta($k, $v); + } + } + $durationMs = (int)round((microtime(true) - $startTime) * 1000); + (new logs_o())->add( + 'orderInvoices', + 'global', + 1, + $user->id, + 'LIST_COLLECTED_INVOICES_TIMING_CACHE_HIT', + 'Duration(ms): ' . $durationMs . ', page=' . ((int)$response->getRequestParameter('page')) . ', limit=' . ((int)$response->getRequestParameter('limit')) . ', search=' . (string)($response->getRequestParameter('search') ?? '') . ', order=' . (string)($response->getRequestParameter('order') ?? '') + ); + $response->success($payload['data'] ?? []); + } + + // Cache miss: compute and cache + $result = $collected_order_invoices->listObjectsWithPaginationIfSet( function ($collected_order_invoice) use ($tmp_collected_order_invoices, $users) { // Select the orders for each collected order invoice $tmp_collected_order_invoices->select((int)$collected_order_invoice['id']); return $this->getOrderInvoiceDetails($collected_order_invoice, $users, $tmp_collected_order_invoices); }, - )); + null, + [], + // Set the where, to where a non-deleted order is connected to the collected order invoice + (new pagination_helper())->where->addCondition(pagination_condition_where::CUSTOM('EXISTS (SELECT 1 FROM orders o WHERE o.invoice_collection_id = collected_order_invoices.id AND o.deleted_at IS NULL)')) + ); + // Capture current pagination meta for caching + $meta = [ + 'pagination' => [ + 'page' => (int)$pageParam, + 'per_page' => (int)$limitParam, + // We don't have the total directly here; the response object already has it, + // but we cache the meta as provided by the client for consistency. + ] + ]; + // Store combined payload + $redis->setEx($cacheKey, json_encode([ + 'data' => $result, + 'meta' => $meta, + ], JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES), $cacheTtl); + $durationMs = (int)round((microtime(true) - $startTime) * 1000); + // Add timing log for performance monitoring + (new logs_o())->add( + 'orderInvoices', + 'global', + 1, + $user->id, + 'LIST_COLLECTED_INVOICES_TIMING', + 'Duration(ms): ' . $durationMs . ', page=' . ((int)$response->getRequestParameter('page')) . ', limit=' . ((int)$response->getRequestParameter('limit')) . ', search=' . (string)($response->getRequestParameter('search') ?? '') . ', order=' . (string)($response->getRequestParameter('order') ?? '') + ); + $response->success($result); } else { (new logs_o())->add('orderInvoices', 'global', 0, 0, 'LIST_COLLECTED_INVOICES', 'User tried to access the list of collected order invoices without a valid session'); $response->error('Invalid session', 400); diff --git a/services/nginx/app/routes/pagination_helper.php b/services/nginx/app/routes/pagination_helper.php new file mode 100644 index 00000000..0dbf3fba --- /dev/null +++ b/services/nginx/app/routes/pagination_helper.php @@ -0,0 +1,65 @@ +where->addCondition(...) + */ +class pagination_helper +{ + public pagination_where $where; + + public function __construct() + { + $this->where = new pagination_where(); + } + + /** + * Render the helper to a SQL snippet string. + */ + public function toSql(): string + { + return $this->where->toSql(); + } +} + +/** + * Condition factory. For now we only need CUSTOM to pass raw SQL, but this can be extended later. + */ +class pagination_condition_where +{ + public static function CUSTOM(string $rawSql): string + { + return $rawSql; + } +} + +/** + * Builder that collects WHERE conditions and can render them as a single SQL snippet. + */ +class pagination_where +{ + /** @var string[] */ + private array $conditions = []; + + public function addCondition(string $condition): self + { + $condition = trim($condition); + if ($condition !== '') { + $this->conditions[] = $condition; + } + return $this; + } + + /** + * Return the SQL snippet. If multiple conditions are provided, they are AND'ed together. + */ + public function toSql(): string + { + if (empty($this->conditions)) { + return ''; + } + return '(' . implode(' AND ', $this->conditions) . ')'; + } +} diff --git a/services/nginx/app/traits/db_object_t.php b/services/nginx/app/traits/db_object_t.php index dc9502cc..2a8fa8e4 100644 --- a/services/nginx/app/traits/db_object_t.php +++ b/services/nginx/app/traits/db_object_t.php @@ -362,10 +362,10 @@ trait db_object_t * @return array The list of objects in the table * @throws Exception */ - public function listObjectsWithPaginationIfSet($parseFunction = null, $forcedFilters = null, array $join = []): array + public function listObjectsWithPaginationIfSet($parseFunction = null, $forcedFilters = null, array $join = [], $additionalWhere = null): array { // Link to the listObjectsWithPaginationIfSet function. - return self::db_object_t__listObjectsWithPaginationIfSet($parseFunction, $forcedFilters, $join); + return self::db_object_t__listObjectsWithPaginationIfSet($parseFunction, $forcedFilters, $join, $additionalWhere); } /** @@ -390,12 +390,28 @@ trait db_object_t * @return array * @throws Exception If the user does not have permission to list the objects */ - public function db_object_t__listObjectsWithPaginationIfSet($parseFunction = null, $forcedFilters = null, array $join = []): array + public function db_object_t__listObjectsWithPaginationIfSet($parseFunction = null, $forcedFilters = null, array $join = [], $additionalWhere = null): array { global $response; $page = ((int)$response->getRequestParameter('page')) ?? null; // Get the page number $limit = ((int)$response->getRequestParameter('limit')) ?? null; // Get the number of objects per page $search = $response->getRequestParameter('search') ?? null; // Get the search query + // Apply additional where clause if provided via helper + if ($additionalWhere !== null) { + $additionalWhereSql = ''; + if (is_string($additionalWhere)) { + $additionalWhereSql = $additionalWhere; + } elseif (is_object($additionalWhere)) { + if (method_exists($additionalWhere, 'toSql')) { + $additionalWhereSql = (string)$additionalWhere->toSql(); + } elseif (method_exists($additionalWhere, '__toString')) { + $additionalWhereSql = (string)$additionalWhere; + } + } + if (!empty($additionalWhereSql)) { + $this->setAdditionalWhereClause($additionalWhereSql); + } + } // If the forced filters are set, use them if ($forcedFilters) { $filters = $forcedFilters; @@ -694,6 +710,8 @@ trait db_object_t $objects = array_map($parseFunction, $objects); } + // Clear additional where clause after each pagination query to avoid leaking constraints + $this->additionalWhereClause = ''; return $objects; } diff --git a/services/nginx/app/traits/redis_t.php b/services/nginx/app/traits/redis_t.php index 25a20174..3ffec9aa 100644 --- a/services/nginx/app/traits/redis_t.php +++ b/services/nginx/app/traits/redis_t.php @@ -53,6 +53,9 @@ trait redis_t */ public function exists(string $key): bool { + if (!self::is_connected()) { + self::connect(); + } return $this->redis->exists($key); } @@ -150,6 +153,9 @@ trait redis_t */ public function setEx(string $key, string $value, int $expiration = 60): self { + if (!self::is_connected()) { + self::connect(); + } $this->redis->setex($key, $expiration, $value); return $this; } @@ -179,6 +185,9 @@ trait redis_t */ public function delete(string $key): self { + if (!self::is_connected()) { + self::connect(); + } $this->redis->del($key); return $this; } @@ -206,6 +215,9 @@ trait redis_t */ public function get(string $key): string|null { + if (!self::is_connected()) { + self::connect(); + } return $this->redis->get($key); } @@ -247,6 +259,9 @@ trait redis_t */ public function set(string $key, string $value): self { + if (!self::is_connected()) { + self::connect(); + } $this->redis->set($key, $value); return $this; }