Skip to content

Commit 21bf5a0

Browse files
committed
Add evalWithShaCache method to RedisConnection for robust Lua script execution
Extracts the evalSha-with-NOSCRIPT-fallback pattern from cache operations into a reusable method on RedisConnection. Unlike naive implementations that treat any false return as NOSCRIPT, this properly distinguishes NOSCRIPT from other errors (syntax, OOM, WRONGTYPE) and handles legitimate nil returns correctly. - Add evalWithShaCache() method with separate $keys and $args parameters - Add LuaScriptException for Lua script execution failures - Update 8 cache operations to use new method - Add unit and integration tests for new method
1 parent eeebf22 commit 21bf5a0

23 files changed

+529
-337
lines changed

src/cache/src/Redis/Operations/AnyTag/Add.php

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -116,12 +116,14 @@ private function executeCluster(string $key, mixed $value, int $seconds, array $
116116
private function executeUsingLua(string $key, mixed $value, int $seconds, array $tags): bool
117117
{
118118
return $this->context->withConnection(function (RedisConnection $conn) use ($key, $value, $seconds, $tags) {
119-
$client = $conn->client();
120119
$prefix = $this->context->prefix();
121120

121+
$keys = [
122+
$prefix . $key, // KEYS[1]
123+
$this->context->reverseIndexKey($key), // KEYS[2]
124+
];
125+
122126
$args = [
123-
$prefix . $key, // KEYS[1]
124-
$this->context->reverseIndexKey($key), // KEYS[2]
125127
$this->serialization->serializeForLua($conn, $value), // ARGV[1]
126128
max(1, $seconds), // ARGV[2]
127129
$this->context->fullTagPrefix(), // ARGV[3]
@@ -132,14 +134,7 @@ private function executeUsingLua(string $key, mixed $value, int $seconds, array
132134
...$tags, // ARGV[8...]
133135
];
134136

135-
$script = $this->addWithTagsScript();
136-
$scriptHash = sha1($script);
137-
$result = $client->evalSha($scriptHash, $args, 2);
138-
139-
// evalSha returns false if script not loaded (NOSCRIPT), fall back to eval
140-
if ($result === false) {
141-
$result = $client->eval($script, $args, 2);
142-
}
137+
$result = $conn->evalWithShaCache($this->addWithTagsScript(), $keys, $args);
143138

144139
return (bool) $result;
145140
});

src/cache/src/Redis/Operations/AnyTag/Decrement.php

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -122,31 +122,24 @@ private function executeCluster(string $key, int $value, array $tags): int|bool
122122
private function executeUsingLua(string $key, int $value, array $tags): int|bool
123123
{
124124
return $this->context->withConnection(function (RedisConnection $conn) use ($key, $value, $tags) {
125-
$client = $conn->client();
126125
$prefix = $this->context->prefix();
127126

128-
$args = [
127+
$keys = [
129128
$prefix . $key, // KEYS[1]
130129
$this->context->reverseIndexKey($key), // KEYS[2]
131-
$value, // ARGV[1]
132-
$this->context->fullTagPrefix(), // ARGV[2]
133-
$this->context->fullRegistryKey(), // ARGV[3]
134-
time(), // ARGV[4]
135-
$key, // ARGV[5]
136-
$this->context->tagHashSuffix(), // ARGV[6]
137-
...$tags, // ARGV[7...]
138130
];
139131

140-
$script = $this->decrementWithTagsScript();
141-
$scriptHash = sha1($script);
142-
$result = $client->evalSha($scriptHash, $args, 2);
143-
144-
// evalSha returns false if script not loaded (NOSCRIPT), fall back to eval
145-
if ($result === false) {
146-
return $client->eval($script, $args, 2);
147-
}
132+
$args = [
133+
$value, // ARGV[1]
134+
$this->context->fullTagPrefix(), // ARGV[2]
135+
$this->context->fullRegistryKey(), // ARGV[3]
136+
time(), // ARGV[4]
137+
$key, // ARGV[5]
138+
$this->context->tagHashSuffix(), // ARGV[6]
139+
...$tags, // ARGV[7...]
140+
];
148141

149-
return $result;
142+
return $conn->evalWithShaCache($this->decrementWithTagsScript(), $keys, $args);
150143
});
151144
}
152145

src/cache/src/Redis/Operations/AnyTag/Forever.php

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,14 @@ private function executeCluster(string $key, mixed $value, array $tags): bool
119119
private function executeUsingLua(string $key, mixed $value, array $tags): bool
120120
{
121121
return $this->context->withConnection(function (RedisConnection $conn) use ($key, $value, $tags) {
122-
$client = $conn->client();
123122
$prefix = $this->context->prefix();
124123

124+
$keys = [
125+
$prefix . $key, // KEYS[1]
126+
$this->context->reverseIndexKey($key), // KEYS[2]
127+
];
128+
125129
$args = [
126-
$prefix . $key, // KEYS[1]
127-
$this->context->reverseIndexKey($key), // KEYS[2]
128130
$this->serialization->serializeForLua($conn, $value), // ARGV[1]
129131
$this->context->fullTagPrefix(), // ARGV[2]
130132
$this->context->fullRegistryKey(), // ARGV[3]
@@ -133,14 +135,7 @@ private function executeUsingLua(string $key, mixed $value, array $tags): bool
133135
...$tags, // ARGV[6...]
134136
];
135137

136-
$script = $this->storeForeverWithTagsScript();
137-
$scriptHash = sha1($script);
138-
$result = $client->evalSha($scriptHash, $args, 2);
139-
140-
// evalSha returns false if script not loaded (NOSCRIPT), fall back to eval
141-
if ($result === false) {
142-
$client->eval($script, $args, 2);
143-
}
138+
$conn->evalWithShaCache($this->storeForeverWithTagsScript(), $keys, $args);
144139

145140
return true;
146141
});

src/cache/src/Redis/Operations/AnyTag/Increment.php

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -122,31 +122,24 @@ private function executeCluster(string $key, int $value, array $tags): int|bool
122122
private function executeUsingLua(string $key, int $value, array $tags): int|bool
123123
{
124124
return $this->context->withConnection(function (RedisConnection $conn) use ($key, $value, $tags) {
125-
$client = $conn->client();
126125
$prefix = $this->context->prefix();
127126

128-
$args = [
127+
$keys = [
129128
$prefix . $key, // KEYS[1]
130129
$this->context->reverseIndexKey($key), // KEYS[2]
131-
$value, // ARGV[1]
132-
$this->context->fullTagPrefix(), // ARGV[2]
133-
$this->context->fullRegistryKey(), // ARGV[3]
134-
time(), // ARGV[4]
135-
$key, // ARGV[5]
136-
$this->context->tagHashSuffix(), // ARGV[6]
137-
...$tags, // ARGV[7...]
138130
];
139131

140-
$script = $this->incrementWithTagsScript();
141-
$scriptHash = sha1($script);
142-
$result = $client->evalSha($scriptHash, $args, 2);
143-
144-
// evalSha returns false if script not loaded (NOSCRIPT), fall back to eval
145-
if ($result === false) {
146-
return $client->eval($script, $args, 2);
147-
}
132+
$args = [
133+
$value, // ARGV[1]
134+
$this->context->fullTagPrefix(), // ARGV[2]
135+
$this->context->fullRegistryKey(), // ARGV[3]
136+
time(), // ARGV[4]
137+
$key, // ARGV[5]
138+
$this->context->tagHashSuffix(), // ARGV[6]
139+
...$tags, // ARGV[7...]
140+
];
148141

149-
return $result;
142+
return $conn->evalWithShaCache($this->incrementWithTagsScript(), $keys, $args);
150143
});
151144
}
152145

src/cache/src/Redis/Operations/AnyTag/Put.php

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,14 @@ private function executeCluster(string $key, mixed $value, int $seconds, array $
136136
private function executeUsingLua(string $key, mixed $value, int $seconds, array $tags): bool
137137
{
138138
return $this->context->withConnection(function (RedisConnection $conn) use ($key, $value, $seconds, $tags) {
139-
$client = $conn->client();
140139
$prefix = $this->context->prefix();
141140

141+
$keys = [
142+
$prefix . $key, // KEYS[1]
143+
$this->context->reverseIndexKey($key), // KEYS[2]
144+
];
145+
142146
$args = [
143-
$prefix . $key, // KEYS[1]
144-
$this->context->reverseIndexKey($key), // KEYS[2]
145147
$this->serialization->serializeForLua($conn, $value), // ARGV[1]
146148
max(1, $seconds), // ARGV[2]
147149
$this->context->fullTagPrefix(), // ARGV[3]
@@ -152,14 +154,7 @@ private function executeUsingLua(string $key, mixed $value, int $seconds, array
152154
...$tags, // ARGV[8...]
153155
];
154156

155-
$script = $this->storeWithTagsScript();
156-
$scriptHash = sha1($script);
157-
$result = $client->evalSha($scriptHash, $args, 2);
158-
159-
// evalSha returns false if script not loaded (NOSCRIPT), fall back to eval
160-
if ($result === false) {
161-
$client->eval($script, $args, 2);
162-
}
157+
$conn->evalWithShaCache($this->storeWithTagsScript(), $keys, $args);
163158

164159
return true;
165160
});

src/cache/src/Redis/Operations/AnyTag/Remember.php

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -148,12 +148,11 @@ private function executeCluster(string $key, int $seconds, Closure $callback, ar
148148
private function executeUsingLua(string $key, int $seconds, Closure $callback, array $tags): array
149149
{
150150
return $this->context->withConnection(function (RedisConnection $conn) use ($key, $seconds, $callback, $tags) {
151-
$client = $conn->client();
152151
$prefix = $this->context->prefix();
153152
$prefixedKey = $prefix . $key;
154153

155154
// Try to get the cached value first
156-
$value = $client->get($prefixedKey);
155+
$value = $conn->client()->get($prefixedKey);
157156

158157
if ($value !== false && $value !== null) {
159158
return [$this->serialization->unserialize($conn, $value), true];
@@ -163,9 +162,12 @@ private function executeUsingLua(string $key, int $seconds, Closure $callback, a
163162
$value = $callback();
164163

165164
// Now use Lua script to atomically store with tags
165+
$keys = [
166+
$prefixedKey, // KEYS[1]
167+
$this->context->reverseIndexKey($key), // KEYS[2]
168+
];
169+
166170
$args = [
167-
$prefixedKey, // KEYS[1]
168-
$this->context->reverseIndexKey($key), // KEYS[2]
169171
$this->serialization->serializeForLua($conn, $value), // ARGV[1]
170172
max(1, $seconds), // ARGV[2]
171173
$this->context->fullTagPrefix(), // ARGV[3]
@@ -176,14 +178,7 @@ private function executeUsingLua(string $key, int $seconds, Closure $callback, a
176178
...$tags, // ARGV[8...]
177179
];
178180

179-
$script = $this->storeWithTagsScript();
180-
$scriptHash = sha1($script);
181-
$result = $client->evalSha($scriptHash, $args, 2);
182-
183-
// evalSha returns false if script not loaded (NOSCRIPT), fall back to eval
184-
if ($result === false) {
185-
$client->eval($script, $args, 2);
186-
}
181+
$conn->evalWithShaCache($this->storeWithTagsScript(), $keys, $args);
187182

188183
return [$value, false];
189184
});

src/cache/src/Redis/Operations/AnyTag/RememberForever.php

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -136,12 +136,11 @@ private function executeCluster(string $key, Closure $callback, array $tags): ar
136136
private function executeUsingLua(string $key, Closure $callback, array $tags): array
137137
{
138138
return $this->context->withConnection(function (RedisConnection $conn) use ($key, $callback, $tags) {
139-
$client = $conn->client();
140139
$prefix = $this->context->prefix();
141140
$prefixedKey = $prefix . $key;
142141

143142
// Try to get the cached value first
144-
$value = $client->get($prefixedKey);
143+
$value = $conn->client()->get($prefixedKey);
145144

146145
if ($value !== false && $value !== null) {
147146
return [$this->serialization->unserialize($conn, $value), true];
@@ -151,9 +150,12 @@ private function executeUsingLua(string $key, Closure $callback, array $tags): a
151150
$value = $callback();
152151

153152
// Now use Lua script to atomically store with tags (forever semantics)
153+
$keys = [
154+
$prefixedKey, // KEYS[1]
155+
$this->context->reverseIndexKey($key), // KEYS[2]
156+
];
157+
154158
$args = [
155-
$prefixedKey, // KEYS[1]
156-
$this->context->reverseIndexKey($key), // KEYS[2]
157159
$this->serialization->serializeForLua($conn, $value), // ARGV[1]
158160
$this->context->fullTagPrefix(), // ARGV[2]
159161
$this->context->fullRegistryKey(), // ARGV[3]
@@ -162,14 +164,7 @@ private function executeUsingLua(string $key, Closure $callback, array $tags): a
162164
...$tags, // ARGV[6...]
163165
];
164166

165-
$script = $this->storeForeverWithTagsScript();
166-
$scriptHash = sha1($script);
167-
$result = $client->evalSha($scriptHash, $args, 2);
168-
169-
// evalSha returns false if script not loaded (NOSCRIPT), fall back to eval
170-
if ($result === false) {
171-
$client->eval($script, $args, 2);
172-
}
167+
$conn->evalWithShaCache($this->storeForeverWithTagsScript(), $keys, $args);
173168

174169
return [$value, false];
175170
});

src/cache/src/Redis/Operations/PutMany.php

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,6 @@ private function executeCluster(array $values, int $seconds): bool
118118
private function executeUsingLua(array $values, int $seconds): bool
119119
{
120120
return $this->context->withConnection(function (RedisConnection $conn) use ($values, $seconds) {
121-
$client = $conn->client();
122121
$prefix = $this->context->prefix();
123122
$seconds = max(1, $seconds);
124123

@@ -135,19 +134,7 @@ private function executeUsingLua(array $values, int $seconds): bool
135134
$args[] = $this->serialization->serializeForLua($conn, $value);
136135
}
137136

138-
// Combine keys and args for eval/evalSha
139-
// Format: [key1, key2, ..., ttl, val1, val2, ...]
140-
$evalArgs = array_merge($keys, $args);
141-
$numKeys = count($keys);
142-
143-
$script = $this->setMultipleKeysScript();
144-
$scriptHash = sha1($script);
145-
$result = $client->evalSha($scriptHash, $evalArgs, $numKeys);
146-
147-
// evalSha returns false if script not loaded (NOSCRIPT), fall back to eval
148-
if ($result === false) {
149-
$result = $client->eval($script, $evalArgs, $numKeys);
150-
}
137+
$result = $conn->evalWithShaCache($this->setMultipleKeysScript(), $keys, $args);
151138

152139
return (bool) $result;
153140
});
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Hypervel\Redis\Exceptions;
6+
7+
use RuntimeException;
8+
9+
/**
10+
* Exception thrown when a Lua script execution fails.
11+
*/
12+
class LuaScriptException extends RuntimeException
13+
{
14+
}

src/redis/src/RedisConnection.php

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
use Generator;
88
use Hyperf\Redis\RedisConnection as HyperfRedisConnection;
9+
use Hypervel\Redis\Exceptions\LuaScriptException;
910
use Hypervel\Redis\Operations\FlushByPattern;
1011
use Hypervel\Redis\Operations\SafeScan;
1112
use Hypervel\Support\Arr;
@@ -801,6 +802,61 @@ public function client(): mixed
801802
return $this->connection;
802803
}
803804

805+
/**
806+
* Execute a Lua script using evalSha with automatic fallback to eval.
807+
*
808+
* Redis caches compiled Lua scripts by SHA1 hash. This method tries evalSha
809+
* first (uses cached compiled script), and falls back to eval if the script
810+
* isn't cached yet (NOSCRIPT error).
811+
*
812+
* Unlike naive implementations that treat any `false` return as NOSCRIPT,
813+
* this method properly distinguishes NOSCRIPT errors from other failures
814+
* (syntax errors, OOM, WRONGTYPE, etc.) and throws on non-NOSCRIPT errors.
815+
*
816+
* @param string $script The Lua script to execute
817+
* @param array<string> $keys Redis keys (passed as KEYS[] in Lua)
818+
* @param array<mixed> $args Additional arguments (passed as ARGV[] in Lua)
819+
* @return mixed The script's return value
820+
*
821+
* @throws LuaScriptException If script execution fails (non-NOSCRIPT error)
822+
*/
823+
public function evalWithShaCache(string $script, array $keys = [], array $args = []): mixed
824+
{
825+
$sha = sha1($script);
826+
$numKeys = count($keys);
827+
828+
// phpredis signature: evalSha(sha, combined_args, num_keys)
829+
// combined_args = keys first, then other args
830+
$combinedArgs = [...$keys, ...$args];
831+
832+
// Try evalSha first - uses cached compiled script
833+
$result = $this->connection->evalSha($sha, $combinedArgs, $numKeys);
834+
835+
if ($result === false) {
836+
$error = $this->connection->getLastError();
837+
838+
// NOSCRIPT means script not cached yet - fall back to eval
839+
if ($error !== null && str_contains($error, 'NOSCRIPT')) {
840+
$this->connection->clearLastError();
841+
$result = $this->connection->eval($script, $combinedArgs, $numKeys);
842+
843+
if ($result === false) {
844+
$evalError = $this->connection->getLastError();
845+
if ($evalError !== null) {
846+
throw new LuaScriptException('Lua script execution failed: ' . $evalError);
847+
}
848+
// If no error, script legitimately returned nil (which becomes false)
849+
}
850+
} elseif ($error !== null) {
851+
// Some other error (syntax, OOM, WRONGTYPE, etc.)
852+
throw new LuaScriptException('Lua script execution failed: ' . $error);
853+
}
854+
// If $error is null and $result is false, the script legitimately returned false
855+
}
856+
857+
return $result;
858+
}
859+
804860
/**
805861
* Safely scan the Redis keyspace for keys matching a pattern.
806862
*

0 commit comments

Comments
 (0)