From 07387ccebb813d6e616fff2693030140c7204cea Mon Sep 17 00:00:00 2001
From: Daniel Paul Carbone <dcarbone@ena.com>
Date: Mon, 14 May 2018 14:36:48 -0500
Subject: [PATCH] addressing #15 in latest release and cleaning up
 phpunit.xml...

---
 .travis.yml                        |   2 +-
 phpunit.local.xml                  |  54 --------------
 phpunit.xml.dist => phpunit.xml    |  16 +----
 src/KV/KVClient.php                |  47 ++++++++----
 src/Values.php                     | 111 +++++++++++++++++------------
 tests/Usage/KV/KVClientCASTest.php |  57 +++++++++++++++
 6 files changed, 161 insertions(+), 126 deletions(-)
 delete mode 100644 phpunit.local.xml
 rename phpunit.xml.dist => phpunit.xml (72%)
 create mode 100644 tests/Usage/KV/KVClientCASTest.php

diff --git a/.travis.yml b/.travis.yml
index 71d151b..7c62c04 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -22,4 +22,4 @@ install:
   - composer install --no-interaction --no-progress --no-suggest --optimize-autoloader
 
 script:
-  - ./vendor/bin/phpunit -c phpunit.xml.dist
+  - ./vendor/bin/phpunit -c phpunit.xml
diff --git a/phpunit.local.xml b/phpunit.local.xml
deleted file mode 100644
index 4a574de..0000000
--- a/phpunit.local.xml
+++ /dev/null
@@ -1,54 +0,0 @@
-<?xml version="1.0" encoding="UTF-8" ?>
-<phpunit
-        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-        xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/6.1/phpunit.xsd"
-        bootstrap="./vendor/autoload.php"
-        colors="true"
-        convertErrorsToExceptions="true"
-        convertNoticesToExceptions="true"
-        convertWarningsToExceptions="true"
-        verbose="true"
-        stopOnFailure="true"
-        processIsolation="false"
-        backupGlobals="false"
->
-
-    <php>
-        <env name="CONSUL_HTTP_ADDR" value="127.0.0.1:8500"/>
-        <env name="CONSUL_HTTP_SSL" value="0"/>
-    </php>
-
-    <testsuites>
-        <testsuite name="definition">
-            <directory>./tests/Definition</directory>
-        </testsuite>
-
-        <testsuite name="usage-config">
-            <file>./tests/Usage/ConfigUsageTest.php</file>
-        </testsuite>
-
-        <testsuite name="usage-request">
-            <file>./tests/Usage/RequestUsageTest.php</file>
-        </testsuite>
-
-        <testsuite name="usage-agent">
-            <file>./tests/Usage/Agent/AgentClientUsageTests.php</file>
-        </testsuite>
-
-        <testsuite name="usage-kv">
-            <file>./tests/usage/KV/KVClientCRUDTests.php</file>
-        </testsuite>
-    </testsuites>
-
-    <filter>
-        <whitelist addUncoveredFilesFromWhitelist="true">
-            <directory suffix=".php">./src</directory>
-        </whitelist>
-    </filter>
-
-    <logging>
-        <log type="coverage-html" target="./tmp/tests/html"/>
-        <log type="plain" target="./tmp/tests/php-consul-api-tests.log"/>
-    </logging>
-
-</phpunit>
diff --git a/phpunit.xml.dist b/phpunit.xml
similarity index 72%
rename from phpunit.xml.dist
rename to phpunit.xml
index 89064cb..f1336ee 100644
--- a/phpunit.xml.dist
+++ b/phpunit.xml
@@ -4,13 +4,8 @@
         xsi:noNamespaceSchemaLocation="http://schema.phpunit.de/6.1/phpunit.xsd"
         bootstrap="./vendor/autoload.php"
         colors="true"
-        convertErrorsToExceptions="true"
-        convertNoticesToExceptions="true"
-        convertWarningsToExceptions="true"
         verbose="true"
         stopOnFailure="true"
-        processIsolation="false"
-        backupGlobals="false"
 >
 
     <php>
@@ -22,21 +17,18 @@
         <testsuite name="definition">
             <directory>./tests/Definition</directory>
         </testsuite>
-
         <testsuite name="usage-config">
             <file>./tests/Usage/ConfigUsageTest.php</file>
         </testsuite>
-
         <testsuite name="usage-request">
             <file>./tests/Usage/RequestUsageTest.php</file>
         </testsuite>
 
         <testsuite name="usage-agent">
-            <file>./tests/Usage/Agent/AgentClientUsageTests.php</file>
+            <directory>./tests/Usage/Agent</directory>
         </testsuite>
-
         <testsuite name="usage-kv">
-            <file>./tests/usage/KV/KVClientCRUDTests.php</file>
+            <directory>./tests/Usage/KV</directory>
         </testsuite>
     </testsuites>
 
@@ -46,8 +38,4 @@
         </whitelist>
     </filter>
 
-    <logging>
-        <log type="plain" target="./tmp/tests/php-consul-api-tests.log"/>
-    </logging>
-
 </phpunit>
diff --git a/src/KV/KVClient.php b/src/KV/KVClient.php
index 9667feb..0f6b5c4 100644
--- a/src/KV/KVClient.php
+++ b/src/KV/KVClient.php
@@ -28,7 +28,7 @@
  */
 class KVClient extends AbstractClient {
     /**
-     * @param string $key Name of key to retrieve value for
+     * @param string                              $key Name of key to retrieve value for
      * @param \DCarbone\PHPConsulAPI\QueryOptions $options
      * @return array(
      * @type KVPair|null kv object or null on error
@@ -78,7 +78,7 @@ public function get($key, QueryOptions $options = null) {
     }
 
     /**
-     * @param \DCarbone\PHPConsulAPI\KV\KVPair $p
+     * @param \DCarbone\PHPConsulAPI\KV\KVPair    $p
      * @param \DCarbone\PHPConsulAPI\WriteOptions $options
      * @return array(
      * @type \DCarbone\PHPConsulAPI\WriteMeta write metadata
@@ -101,7 +101,7 @@ public function put(KVPair $p, WriteOptions $options = null) {
     }
 
     /**
-     * @param string $key
+     * @param string                                   $key
      * @param \DCarbone\PHPConsulAPI\WriteOptions|null $options
      * @return array(
      * @type \DCarbone\PHPConsulAPI\WriteMeta metadata about write
@@ -121,7 +121,7 @@ public function delete($key, WriteOptions $options = null) {
     }
 
     /**
-     * @param string $prefix
+     * @param string                                   $prefix
      * @param \DCarbone\PHPConsulAPI\QueryOptions|null $options
      * @return array(
      * @type KVPair[]|null array of KVPair objects under specified prefix
@@ -171,7 +171,7 @@ public function valueList($prefix = '', QueryOptions $options = null) {
     }
 
     /**
-     * @param string $prefix Prefix to search for.  Null returns all keys.
+     * @param string                              $prefix Prefix to search for.  Null returns all keys.
      * @param \DCarbone\PHPConsulAPI\QueryOptions $options
      * @return array(
      * @type string[]|null list of keys
@@ -211,9 +211,10 @@ public function keys($prefix = null, QueryOptions $options = null) {
     }
 
     /**
-     * @param \DCarbone\PHPConsulAPI\KV\KVPair $p
+     * @param \DCarbone\PHPConsulAPI\KV\KVPair    $p
      * @param \DCarbone\PHPConsulAPI\WriteOptions $options
      * @return array(
+     * @type bool whether the operation succeeded or not
      * @type \DCarbone\PHPConsulAPI\WriteMeta write metadata
      * @type \DCarbone\PHPConsulAPI\Error|null error, if any
      * )
@@ -225,17 +226,16 @@ public function cas(KVPair $p, WriteOptions $options = null) {
         if (0 !== $p->Flags) {
             $r->Params->set('flags', (string)$p->Flags);
         }
-
-        list($duration, $_, $err) = $this->requireOK($this->doRequest($r));
+        /** @var \Psr\Http\Message\ResponseInterface $response */
+        list($duration, $response, $err) = $this->requireOK($this->doRequest($r));
         if (null !== $err) {
             return [null, $err];
         }
-
-        return [$this->buildWriteMeta($duration), null];
+        return [0 === strpos($response->getBody()->getContents(), 'true'), $this->buildWriteMeta($duration), null];
     }
 
     /**
-     * @param \DCarbone\PHPConsulAPI\KV\KVPair $p
+     * @param \DCarbone\PHPConsulAPI\KV\KVPair    $p
      * @param \DCarbone\PHPConsulAPI\WriteOptions $options
      * @return array(
      * @type \DCarbone\PHPConsulAPI\WriteMeta write metadata
@@ -259,7 +259,28 @@ public function acquire(KVPair $p, WriteOptions $options = null) {
     }
 
     /**
-     * @param \DCarbone\PHPConsulAPI\KV\KVPair $p
+     * @param \DCarbone\PHPConsulAPI\KV\KVPair         $p
+     * @param \DCarbone\PHPConsulAPI\WriteOptions|null $options
+     * @return array(
+     * @type bool
+     * @type \DCarbone\PHPConsulAPI\WriteMeta
+     * @type \DCarbone\PHPConsulAPI\Error|null
+     * )
+     */
+    public function deleteCAS(KVPair $p, WriteOptions $options = null) {
+        $r = new Request('DELETE', sprintf('v1/kv/%s', ltrim($p->Key, "/")), $this->config);
+        $r->setWriteOptions($options);
+        $r->Params['cas'] = (string)$p->ModifyIndex;
+        /** @var \Psr\Http\Message\ResponseInterface $response */
+        list($duration, $response, $err) = $this->requireOK($this->doRequest($r));
+        if (null !== $err) {
+            return [null, null, $err];
+        }
+        return [0 === strpos($response->getBody()->getContents(), 'true'), $this->buildWriteMeta($duration), null];
+    }
+
+    /**
+     * @param \DCarbone\PHPConsulAPI\KV\KVPair    $p
      * @param \DCarbone\PHPConsulAPI\WriteOptions $options
      * @return array(
      * @type \DCarbone\PHPConsulAPI\WriteMeta write metadata
@@ -283,7 +304,7 @@ public function release(KVPair $p, WriteOptions $options = null) {
     }
 
     /**
-     * @param null|string $prefix
+     * @param null|string                         $prefix
      * @param \DCarbone\PHPConsulAPI\QueryOptions $options
      * @return array(
      * @type KVPair[]|KVTree[]|null array of trees, values, or null on error
diff --git a/src/Values.php b/src/Values.php
index 8078482..00c11a6 100644
--- a/src/Values.php
+++ b/src/Values.php
@@ -1,26 +1,21 @@
 <?php namespace DCarbone\PHPConsulAPI;
-
 /*
-   Copyright 2016-2017 Daniel Carbone (daniel.p.carbone@gmail.com)
-
+   Copyright 2016-2018 Daniel Carbone (daniel.p.carbone@gmail.com)
    Licensed under the Apache License, Version 2.0 (the "License");
    you may not use this file except in compliance with the License.
    You may obtain a copy of the License at
-
        http://www.apache.org/licenses/LICENSE-2.0
-
    Unless required by applicable law or agreed to in writing, software
    distributed under the License is distributed on an "AS IS" BASIS,
    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    See the License for the specific language governing permissions and
    limitations under the License.
 */
-
 /**
  * Class Values
  * @package DCarbone\PHPConsulAPI
  */
-class Values implements \JsonSerializable, \Countable {
+class Values implements \Iterator, \ArrayAccess, \Countable, \JsonSerializable {
     /** @var array */
     private $values = [];
 
@@ -29,14 +24,9 @@ class Values implements \JsonSerializable, \Countable {
      * @return string
      */
     public function get($key) {
-        if (!is_string($key)) {
-            throw new \InvalidArgumentException(sprintf('$key must be string, saw "%s".', gettype($key)));
-        }
-
         if (isset($this->values[$key])) {
             return $this->values[$key][0];
         }
-
         return '';
     }
 
@@ -45,14 +35,9 @@ public function get($key) {
      * @return string[]
      */
     public function getAll($key) {
-        if (!is_string($key)) {
-            throw new \InvalidArgumentException(sprintf('$key must be string, saw "%s".', gettype($key)));
-        }
-
         if (isset($this->values[$key])) {
             return $this->values[$key];
         }
-
         return [];
     }
 
@@ -61,16 +46,7 @@ public function getAll($key) {
      * @param string $value
      */
     public function set($key, $value) {
-        if (!is_string($key)) {
-            throw new \InvalidArgumentException(sprintf('$key must be string, saw "%s".', gettype($key)));
-        }
-
-        if (settype($value, 'string')) {
-            $this->values[$key] = [$value];
-        } else {
-            throw new \InvalidArgumentException(sprintf('$value must be castable to string, saw "%s".',
-                gettype($value)));
-        }
+        $this->values[$key] = [$value];
     }
 
     /**
@@ -78,19 +54,10 @@ public function set($key, $value) {
      * @param string $value
      */
     public function add($key, $value) {
-        if (!is_string($key)) {
-            throw new \InvalidArgumentException(sprintf('$key must be string, saw "%s".', gettype($key)));
-        }
-
-        if (settype($value, 'string')) {
-            if (isset($this->values[$key])) {
-                $this->values[$key][] = $value;
-            } else {
-                $this->values[$key] = [$value];
-            }
+        if (isset($this->values[$key])) {
+            $this->values[$key][] = $value;
         } else {
-            throw new \InvalidArgumentException(sprintf('$value must be castable to string, saw "%s".',
-                gettype($value)));
+            $this->values[$key] = [$value];
         }
     }
 
@@ -98,10 +65,6 @@ public function add($key, $value) {
      * @param string $key
      */
     public function delete($key) {
-        if (!is_string($key)) {
-            throw new \InvalidArgumentException(sprintf('$key must be string, saw "%s".', gettype($key)));
-        }
-
         unset($this->values[$key]);
     }
 
@@ -119,6 +82,66 @@ public function toPsr7Array() {
         return $this->values;
     }
 
+    /**
+     * @return string|array
+     */
+    public function current() {
+        return current($this->values);
+    }
+
+    public function next() {
+        next($this->values);
+    }
+
+    /**
+     * @return string
+     */
+    public function key() {
+        return key($this->values);
+    }
+
+    /**
+     * @return bool
+     */
+    public function valid() {
+        return null !== key($this->values);
+    }
+
+    public function rewind() {
+        reset($this->values);
+    }
+
+    /**
+     * @param mixed $offset
+     * @return bool
+     */
+    public function offsetExists($offset) {
+        return isset($this->values[$offset]);
+    }
+
+    /**
+     * @param string $offset
+     * @return string
+     */
+    public function offsetGet($offset) {
+        return $this->get($offset);
+    }
+
+    /**
+     * @param string $offset
+     * @param string $value
+     */
+    public function offsetSet($offset, $value) {
+        $this->set($offset, $value);
+    }
+
+    /**
+     * @param string $offset
+     */
+    public function offsetUnset($offset) {
+        $this->delete($offset);
+    }
+
     /**
      * @return array
      */
@@ -148,7 +171,7 @@ public function __toString() {
 
     /**
      * @param string $v
-     * @return string mixed
+     * @return string
      */
     protected function encode($v) {
         return $v;
diff --git a/tests/Usage/KV/KVClientCASTest.php b/tests/Usage/KV/KVClientCASTest.php
new file mode 100644
index 0000000..a1098ea
--- /dev/null
+++ b/tests/Usage/KV/KVClientCASTest.php
@@ -0,0 +1,57 @@
+<?php namespace DCarbone\PHPConsulAPITests\Usage\KV;
+
+use DCarbone\PHPConsulAPI\Config;
+use DCarbone\PHPConsulAPI\KV\KVClient;
+use DCarbone\PHPConsulAPI\KV\KVPair;
+use DCarbone\PHPConsulAPITests\ConsulManager;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Class KVClientCASTest
+ * @package DCarbone\PHPConsulAPITests\Usage\KV
+ */
+class KVClientCASTest extends TestCase {
+    const KVKey1          = 'testcaskey';
+    const KVOriginalValue = 'originalvalue';
+    const KVUpdatedValue  = 'updatedvalue';
+    const KVUpdatedValue2 = 'updatedvalue2';
+
+    protected function setUp() {
+        ConsulManager::startSingle();
+    }
+
+    protected function tearDown() {
+        ConsulManager::stopSingle();
+    }
+
+    public function testKVWithCAS() {
+        /** @var \DCarbone\PHPConsulAPI\KV\KVPair $kv */
+        /** @var \DCarbone\PHPConsulAPI\Error $err */
+        $client = new KVClient(new Config());
+        list($_, $err) = $client->put(new KVPair(['Key' => self::KVKey1, 'Value' => self::KVOriginalValue]));
+        $this->assertNull($err, sprintf('Unable to put KV: %s', $err));
+        list($kv, $_, $err) = $client->get(self::KVKey1);
+        $this->assertNull($err, sprintf('Unable to get KV: %s', $err));
+        $this->assertInstanceOf(KVPair::class, $kv);
+        $this->assertEquals(self::KVOriginalValue, $kv->Value);
+        $omi = $kv->ModifyIndex;
+        $kv->Value = self::KVUpdatedValue;
+        list($ok, $_, $err) = $client->cas($kv);
+        $this->assertNull($err, sprintf('Unable to update kv value: %s', $err));
+        $this->assertTrue($ok);
+        $kv->Value = self::KVUpdatedValue2;
+        list($ok, $_, $err) = $client->cas($kv);
+        $this->assertNull($err, sprintf('Error updating kv with old cas: %s', $err));
+        $this->assertFalse($ok, 'Expected false when trying to update key with old cas');
+        list($ok, $_, $err) = $client->deleteCAS($kv);
+        $this->assertNull($err, sprintf('Error deleting kv with old cas: %s', $err));
+        $this->assertFalse($ok, 'Expected false when trying to delete key with old cas');
+        list($kv, $_, $err) = $client->get(self::KVKey1);
+        $this->assertNull($err, sprintf('Error retrieving updated key: %s', $err));
+        $this->assertInstanceOf(KVPair::class, $kv);
+        $this->assertNotEquals($omi, $kv->ModifyIndex, 'Expected ModifyIndex to be different');
+        list($ok, $_, $err) = $client->deleteCAS($kv);
+        $this->assertNull($err, sprintf('Error deleting key: %s', $err));
+        $this->assertTrue($ok, 'Expected true when deleting key with updated cas');
+    }
+}
\ No newline at end of file