Skip to content

Commit 3f23583

Browse files
dcarbonedcarbone
committed
Mild logic improvements in "KV::valueList" and "KV::keys"
- Added fail test for invalid prefix in "KV::valueList"
1 parent 007e59f commit 3f23583

File tree

2 files changed

+106
-19
lines changed

2 files changed

+106
-19
lines changed

‎src/KV/KVClient.php‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,11 @@ public function get($key, QueryOptions $queryOptions = null) {
8787
* )
8888
*/
8989
public function valueList($prefix = '', QueryOptions $queryOptions = null) {
90-
if (!is_string($prefix)) {
90+
if (null === $prefix) {
91+
$r = new Request('get', 'v1/kv/', $this->c);
92+
} else if (is_string($prefix)) {
93+
$r = new Request('get', sprintf('v1/kv/%s', $prefix), $this->c);
94+
} else {
9195
return [null,
9296
null,
9397
new Error(sprintf(
@@ -97,12 +101,6 @@ public function valueList($prefix = '', QueryOptions $queryOptions = null) {
97101
))];
98102
}
99103

100-
if ('' === $prefix) {
101-
$r = new Request('get', 'v1/kv/', $this->c);
102-
} else {
103-
$r = new Request('get', sprintf('v1/kv/%s', $prefix), $this->c);
104-
}
105-
106104
$r->setQueryOptions($queryOptions);
107105
$r->params->set('recurse', '');
108106

@@ -139,7 +137,11 @@ public function valueList($prefix = '', QueryOptions $queryOptions = null) {
139137
* )
140138
*/
141139
public function keys($prefix = null, QueryOptions $queryOptions = null) {
142-
if (null !== $prefix && !is_string($prefix)) {
140+
if (null === $prefix) {
141+
$r = new Request('get', 'v1/kv/', $this->c);
142+
} else if (is_string($prefix)) {
143+
$r = new Request('get', sprintf('v1/kv/%s', $prefix), $this->c);
144+
} else {
143145
return [null,
144146
null,
145147
new Error(sprintf(
@@ -149,12 +151,6 @@ public function keys($prefix = null, QueryOptions $queryOptions = null) {
149151
))];
150152
}
151153

152-
if (null === $prefix) {
153-
$r = new Request('get', 'v1/kv/', $this->c);
154-
} else {
155-
$r = new Request('get', sprintf('v1/kv/%s', $prefix), $this->c);
156-
}
157-
158154
$r->setQueryOptions($queryOptions);
159155
$r->params->set('keys', 'true');
160156

@@ -172,7 +168,7 @@ public function keys($prefix = null, QueryOptions $queryOptions = null) {
172168
}
173169

174170
/**
175-
* @param KVPair $p
171+
* @param \DCarbone\PHPConsulAPI\KV\KVPair $p
176172
* @param \DCarbone\PHPConsulAPI\WriteOptions $writeOptions
177173
* @return array(
178174
* @type \DCarbone\PHPConsulAPI\WriteMeta write metadata
@@ -215,7 +211,7 @@ public function delete($key, WriteOptions $writeOptions = null) {
215211
}
216212

217213
/**
218-
* @param KVPair $p
214+
* @param \DCarbone\PHPConsulAPI\KV\KVPair $p
219215
* @param \DCarbone\PHPConsulAPI\WriteOptions $writeOptions
220216
* @return array(
221217
* @type \DCarbone\PHPConsulAPI\WriteMeta write metadata
@@ -239,7 +235,7 @@ public function cas(KVPair $p, WriteOptions $writeOptions = null) {
239235
}
240236

241237
/**
242-
* @param KVPair $p
238+
* @param \DCarbone\PHPConsulAPI\KV\KVPair $p
243239
* @param \DCarbone\PHPConsulAPI\WriteOptions $writeOptions
244240
* @return array(
245241
* @type \DCarbone\PHPConsulAPI\WriteMeta write metadata
@@ -263,7 +259,7 @@ public function acquire(KVPair $p, WriteOptions $writeOptions = null) {
263259
}
264260

265261
/**
266-
* @param KVPair $p
262+
* @param \DCarbone\PHPConsulAPI\KV\KVPair $p
267263
* @param \DCarbone\PHPConsulAPI\WriteOptions $writeOptions
268264
* @return array(
269265
* @type \DCarbone\PHPConsulAPI\WriteMeta write metadata

‎tests/Usage/KV/KVClientUsageTest.php‎

Lines changed: 92 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*/
1818

1919
use DCarbone\PHPConsulAPI\Config;
20+
use DCarbone\PHPConsulAPI\Error;
2021
use DCarbone\PHPConsulAPI\KV\KVClient;
2122
use DCarbone\PHPConsulAPI\KV\KVPair;
2223
use DCarbone\PHPConsulAPI\QueryMeta;
@@ -108,10 +109,27 @@ public function testCanDeleteKey() {
108109
));
109110
}
110111

112+
public function testListReturnsErrorWithInvalidPrefix() {
113+
$client = new KVClient(new Config());
114+
list($_, $_, $err) = $client->valueList(12345);
115+
$this->assertInstanceOf(
116+
Error::class,
117+
$err,
118+
sprintf(
119+
'Expected $err to be instanceof "%s", saw "%s"',
120+
Error::class,
121+
is_object($err) ? get_class($err) : gettype($err)
122+
));
123+
}
124+
111125
/**
112126
* @depends testCanPutKey
113127
*/
114128
public function testCanGetNoPrefixList() {
129+
/** @var \DCarbone\PHPConsulAPI\KV\KVPair[] $list */
130+
/** @var \DCarbone\PHPConsulAPI\QueryMeta $qm */
131+
/** @var \DCarbone\PHPConsulAPI\Error $err */
132+
115133
$client = new KVClient(new Config());
116134
$client->put(new KVPair(['Key' => self::KVKey1, 'Value' => self::KVValue1]));
117135
$client->put(new KVPair(['Key' => self::KVKey2, 'Value' => self::KVValue2]));
@@ -122,6 +140,32 @@ public function testCanGetNoPrefixList() {
122140
$this->assertInstanceOf(QueryMeta::class, $qm);
123141
$this->assertInternalType('array', $list);
124142
$this->assertCount(3, $list);
143+
144+
$key1found = false;
145+
$key2found = false;
146+
$key3found = false;
147+
148+
foreach ($list as $kv) {
149+
if (self::KVValue1 === $kv->Value) {
150+
$key1found = true;
151+
} else if (self::KVValue2 === $kv->Value) {
152+
$key2found = true;
153+
} else if (self::KVValue3 === $kv->Value) {
154+
$key3found = true;
155+
}
156+
}
157+
158+
try {
159+
$this->assertTrue($key1found, 'Key1 not found in list!');
160+
$this->assertTrue($key2found, 'Key2 not found in list!');
161+
$this->assertTrue($key3found, 'Key3 not found in list!');
162+
} catch (\PHPUnit_Framework_AssertionFailedError $e) {
163+
echo "\nno prefix \$list value:\n";
164+
var_dump($list);
165+
echo "\n";
166+
167+
throw $e;
168+
}
125169
}
126170

127171
/**
@@ -163,7 +207,54 @@ public function testCanGetPrefixList() {
163207
$this->assertTrue($key2found, 'Key2 not found in list!');
164208
$this->assertTrue($key3found, 'Key3 not found in list!');
165209
} catch (\PHPUnit_Framework_AssertionFailedError $e) {
166-
echo "\n\$list value:\n";
210+
echo "\nprefix \$list value:\n";
211+
var_dump($list);
212+
echo "\n";
213+
214+
throw $e;
215+
}
216+
}
217+
218+
/**
219+
* @depends testCanPutKey
220+
*/
221+
public function testCanGetNoPrefixKeys() {
222+
/** @var string[] $list */
223+
/** @var \DCarbone\PHPConsulAPI\QueryMeta $qm */
224+
/** @var \DCarbone\PHPConsulAPI\Error $err */
225+
226+
$client = new KVClient(new Config());
227+
$client->put(new KVPair(['Key' => self::KVKey1, 'Value' => self::KVValue1]));
228+
$client->put(new KVPair(['Key' => self::KVKey2, 'Value' => self::KVValue2]));
229+
$client->put(new KVPair(['Key' => self::KVKey3, 'Value' => self::KVValue3]));
230+
231+
list($list, $qm, $err) = $client->keys();
232+
$this->assertNull($err, sprintf('KV::keys returned error: %s', $err));
233+
$this->assertInstanceOf(QueryMeta::class, $qm);
234+
$this->assertInternalType('array', $list);
235+
$this->assertCount(3, $list);
236+
$this->assertContainsOnly('string', $list, true);
237+
238+
$key1found = false;
239+
$key2found = false;
240+
$key3found = false;
241+
242+
foreach ($list as $key) {
243+
if (self::KVKey1 === $key) {
244+
$key1found = true;
245+
} else if (self::KVKey2 === $key) {
246+
$key2found = true;
247+
} else if (self::KVKey3 === $key) {
248+
$key3found = true;
249+
}
250+
}
251+
252+
try {
253+
$this->assertTrue($key1found, 'Key1 not found in list!');
254+
$this->assertTrue($key2found, 'Key2 not found in list!');
255+
$this->assertTrue($key3found, 'Key3 not found in list!');
256+
} catch (\PHPUnit_Framework_AssertionFailedError $e) {
257+
echo "\nprefix \$list value:\n";
167258
var_dump($list);
168259
echo "\n";
169260

0 commit comments

Comments
 (0)