From bea7e5697b3c42dc8ad1e090e2d47507bff98e87 Mon Sep 17 00:00:00 2001 From: Jeppe Bundgaard Date: Mon, 11 May 2026 04:36:11 +0200 Subject: [PATCH] Handle empty inputs in Redis and database operations, improve safety seal validation, and enhance related tests - Return empty arrays for empty inputs in Redis `mget`, `db_object_t`, and `users_o` operations. - Refactor safety seal validation logic to handle numeric strings and improve clarity. - Add unit and API tests to verify handling of empty inputs and numeric safety seal strings. --- services/nginx/app/classes/redis.php | 4 +++ services/nginx/app/objects/users_o.php | 4 +++ .../nginx/app/routes/orderBookingRoute.php | 29 ++++++++++------ .../Api/OrderBookingsCompletionApiTest.php | 34 +++++++++++++++++++ .../Unit/Redis/RedisAtomicReservationTest.php | 29 ++++++++++++++++ .../Users/UsersCustomerNamesCacheTest.php | 14 ++++++++ services/nginx/app/traits/db_object_t.php | 8 +++++ 7 files changed, 112 insertions(+), 10 deletions(-) diff --git a/services/nginx/app/classes/redis.php b/services/nginx/app/classes/redis.php index cc826ec4..5c4372ac 100644 --- a/services/nginx/app/classes/redis.php +++ b/services/nginx/app/classes/redis.php @@ -471,6 +471,10 @@ class redis implements redis_i public function mget(array $array_map): array { + if (empty($array_map)) { + return []; + } + // Get multiple keys from Redis return $this->redis->mget($array_map); } diff --git a/services/nginx/app/objects/users_o.php b/services/nginx/app/objects/users_o.php index 2f57100b..d17fcc51 100644 --- a/services/nginx/app/objects/users_o.php +++ b/services/nginx/app/objects/users_o.php @@ -1534,6 +1534,10 @@ class users_o extends db { global $db; $customer_numbers = array_map('intval', $customer_numbers); + if (empty($customer_numbers)) { + return []; + } + // Look in the cache first $customer_numbers_to_fetch = []; $customer_names_cached = self::getCachedForMultipleObjects('economic_customer_name', $customer_numbers); diff --git a/services/nginx/app/routes/orderBookingRoute.php b/services/nginx/app/routes/orderBookingRoute.php index fafa4acc..89c1de08 100644 --- a/services/nginx/app/routes/orderBookingRoute.php +++ b/services/nginx/app/routes/orderBookingRoute.php @@ -369,7 +369,7 @@ class orderBookingRoute * Parameters */ $object = self::getTargetObject(); - $safetySeal = self::getSafetySeal(false); // Int | Null + $safetySeal = self::getSafetySeal(false); /** * Authentication */ @@ -505,16 +505,25 @@ class orderBookingRoute $error = 'Invalid safety seal'; if (!$required && !$this->isParametersSet([$parameter])) return null; self::requireParameters([$parameter]); - if ($required) { - self::requireType(self::getParameter($parameter), self::type_int()); - } else { - self::requireTypeIn(self::getParameter($parameter), [self::type_int(), self::type_null()]); - // Check if the value is null - if ($this->getParameter($parameter) === null) return null; + $rawValue = self::getParameter($parameter); + if (!$required && $rawValue === null) return null; + if (is_string($rawValue)) { + $rawValue = trim($rawValue); + if (!$required && $rawValue === '') return null; + if (!ctype_digit($rawValue)) $response->error($error, 400); + } elseif (!is_int($rawValue)) { + if ($required) { + self::requireType($rawValue, self::type_int()); + } else { + self::requireTypeIn($rawValue, [self::type_int(), self::type_null()]); + } } - self::requireMinLength($parameter, 1); - self::requireMaxLength($parameter, 9); - $value = (int)self::getParameter($parameter); + if ($required || $rawValue !== null) { + $valueLength = strlen((string)$rawValue); + if ($valueLength < 1) $response->error('Parameter ' . $parameter . ' must be at least 1 characters long', 400); + if ($valueLength > 9) $response->error('Parameter ' . $parameter . ' must be at most 9 characters long', 400); + } + $value = (int)$rawValue; self::requireMinValue($value, 1); self::requireMaxValue($value, 999999999); return $value; diff --git a/services/nginx/app/tests/Api/OrderBookingsCompletionApiTest.php b/services/nginx/app/tests/Api/OrderBookingsCompletionApiTest.php index 8b64cf61..bf7fb24b 100644 --- a/services/nginx/app/tests/Api/OrderBookingsCompletionApiTest.php +++ b/services/nginx/app/tests/Api/OrderBookingsCompletionApiTest.php @@ -85,6 +85,40 @@ it('allows linked POS order booking completion for mobile POS compatibility', fu expect((int)($row['order_id'] ?? 0))->toBe($order['id']); }); +it('accepts numeric safety seal strings when completing a linked POS order booking', function (): void { + $customer = api_fixtures()->createUser(['display_name' => 'Linked Booking String Seal Customer']); + $department = api_fixtures()->createDepartment(); + $cashier = api_fixtures()->createUser(['display_name' => 'Linked Booking String Seal Cashier']); + $order = api_fixtures()->createOrder([ + 'customer_id' => $customer['customer_number'], + 'cashier_id' => $cashier['id'], + 'department_id' => $department['id'], + 'safety_seal' => null, + ]); + $booking = api_fixtures()->createOrderBooking([ + 'customer_number' => $customer['customer_number'], + 'department' => $department['id'], + 'order_id' => $order['id'], + ]); + $session = api_fixtures()->createUserSession([ + 'complete_bookings', + 'department_access_' . $department['id'], + ]); + + $response = api_client()->post('/order-bookings/complete', [ + 'id' => $booking['id'], + 'safety_seal' => '123456', + ], $session['headers']); + + $response + ->assertStatus(200) + ->assertEnvelope() + ->assertSuccess(); + + expect($response->data()['id'] ?? null)->toBe($booking['id']); + expect($response->data()['order_id'] ?? null)->toBe($order['id']); +}); + it('disables the legacy complete wash without certificate route', function (): void { $response = api_client()->post('/admin/bookings/completeWashWithoutWashCertificate', [ 'id' => 123, diff --git a/services/nginx/app/tests/Unit/Redis/RedisAtomicReservationTest.php b/services/nginx/app/tests/Unit/Redis/RedisAtomicReservationTest.php index 500ccf79..dc493977 100644 --- a/services/nginx/app/tests/Unit/Redis/RedisAtomicReservationTest.php +++ b/services/nginx/app/tests/Unit/Redis/RedisAtomicReservationTest.php @@ -13,6 +13,12 @@ class RedisAtomicReservationTestClient extends PredisClient { } + public function mget(array $keys): array + { + $this->calls[] = ['mget', $keys]; + return $this->returnValue; + } + public function set(...$arguments): mixed { $this->calls[] = $arguments; @@ -71,3 +77,26 @@ it('returns false when the slot is already claimed and clamps ttl to one second' ['goal_alert_sent:22:2026-03-17', '1', 'EX', 1, 'NX'], ]); }); + +it('does not send empty mget commands to redis', function (): void { + global $REDIS_CONFIG; + + $REDIS_CONFIG = [ + 'host' => 'redis', + 'database' => 0, + 'password' => '', + ]; + + $client = new RedisAtomicReservationTestClient(['cached-value']); + + $redis = new redis(); + redis_test_inject_client($redis, $client); + + expect($redis->mget([]))->toBe([]); + expect($client->calls)->toBe([]); + + expect($redis->mget(['cache-key']))->toBe(['cached-value']); + expect($client->calls)->toBe([ + ['mget', ['cache-key']], + ]); +}); diff --git a/services/nginx/app/tests/Unit/Users/UsersCustomerNamesCacheTest.php b/services/nginx/app/tests/Unit/Users/UsersCustomerNamesCacheTest.php index aae7ba21..01c9440b 100644 --- a/services/nginx/app/tests/Unit/Users/UsersCustomerNamesCacheTest.php +++ b/services/nginx/app/tests/Unit/Users/UsersCustomerNamesCacheTest.php @@ -1,8 +1,10 @@ toContain('if ($cache_payload !== null) {'); expect($content)->toContain("\$this->cache('economic_customer_name', \$cache_payload, \$customer_number);"); }); + +it('returns an empty customer name map without touching cache for empty input', function (): void { + $users = new users_o(); + + expect($users->getCustomerNames([]))->toBe([]); +}); + +it('returns no rows for empty array field filters', function (): void { + $users = new users_o(); + + expect($users->getFieldsWhere(['id' => []], ['customer_number']))->toBe([]); +}); diff --git a/services/nginx/app/traits/db_object_t.php b/services/nginx/app/traits/db_object_t.php index a782d103..f2b3de52 100644 --- a/services/nginx/app/traits/db_object_t.php +++ b/services/nginx/app/traits/db_object_t.php @@ -173,6 +173,10 @@ trait db_object_t // If the value is "!null", add a where clause to check if the field is not null $where[] = "$field IS NOT NULL"; } elseif (is_array($value)) { + if (empty($value)) { + return []; + } + // If the value is an array, add a where clause to check if the field is in the array $in = implode(',', array_map(function ($v) { // Escape the value to prevent SQL injection @@ -1052,6 +1056,10 @@ trait db_object_t */ public function getCachedForMultipleObjects(string $key, array $objectIds): array { + if (empty($objectIds)) { + return []; + } + return redis->mget(array_map(function($objectId) use ($key) { return $this->table . '_' . $objectId . '_' . $key; }, $objectIds));