feat(cluster): add a ProtocolVersion enum for withProtocolVersion()

CHANGELOG.md

@@ -45,6 +45,10 @@
   carries no setting by default and `isIdempotent()` then returns `null`. A per-call
   `idempotent` option overrides the statement. For a batch the driver reads the flag off the
   batch, so a flag on a statement added to the batch has no effect.
+- `Cassandra\ProtocolVersion`, an int-backed enum with the cases `V1` to `V5`. The backing value
+  is the protocol version byte, so it matches `CASS_PROTOCOL_VERSION_V1` to
+  `CASS_PROTOCOL_VERSION_V5`. `Cluster\Builder::withProtocolVersion()` accepts a case of this
+  enum, and still accepts a plain integer for a version the enum does not name.
 - `Rows::wasApplied(): bool`, which reads the `[applied]` column of a lightweight transaction
   result. A statement with no condition has no such column, and the method then returns `true`,
   so you can call it on any result. The `[applied]` column stays readable through `first()` and
@@ -132,6 +136,8 @@
 
 ### Changed
 
+- Casting a `Cluster\Builder` to an array now gives a `Cassandra\ProtocolVersion` case for the
+  `protocolVersion` key, was an integer. A value that no case names stays an integer.
 - The `scylla-cpp` backend now builds the vendored driver in `third-party/cpp-driver` and links
   it into `cassandra.so`. Upstream no longer maintains the ScyllaDB cpp-driver, so it ships with
   the extension and gets fixed here. A checkout builds with no C/C++ driver installed. To link a

docs/Cassandra/Cluster/Builder.php

@@ -210,7 +210,7 @@ public function withPersistentSessions($enabled) { }
      *
      * NOTE: Apache Cassandra 3.x supports protocol version 3 and 4 only
      *
-     * @param int $version The protocol version
+     * @param \Cassandra\ProtocolVersion|int $version The protocol version
      *
      * @return \Cassandra\Cluster\Builder self
      */

include/php_scylladb_types.h

@@ -812,6 +812,7 @@ extern PHP_SCYLLADB_API zend_class_entry *php_scylladb_core_ce;
 extern PHP_SCYLLADB_API zend_class_entry *php_scylladb_cluster_ce;
 extern PHP_SCYLLADB_API zend_class_entry *php_scylladb_default_cluster_ce;
 extern PHP_SCYLLADB_API zend_class_entry *php_scylladb_cluster_builder_ce;
+extern PHP_SCYLLADB_API zend_class_entry *php_scylladb_protocol_version_ce;
 extern PHP_SCYLLADB_API zend_class_entry *php_scylladb_ssl_options_builder_ce;
 extern PHP_SCYLLADB_API zend_class_entry *php_scylladb_future_ce;
 extern PHP_SCYLLADB_API zend_class_entry *php_scylladb_future_prepared_statement_ce;

src/CMakeLists.txt

@@ -21,6 +21,7 @@ target_sources(src PRIVATE
         UuidGen.c
         DefaultSession.c
         PreparedStatement.c
+        ProtocolVersion.c
         Set.c
         SimpleStatement.c
         Tuple.c
@@ -89,6 +90,7 @@ php_scylladb_generate_arginfo(src
         Blob.stub.php
         Map.stub.php
         Cassandra.stub.php
+        ProtocolVersion.stub.php
         PreparedStatement.stub.php
         Collection.stub.php
         Session.stub.php
@@ -127,6 +129,7 @@ php_scylladb_generate_descriptor(src
         Tuple.stub.php
         UserTypeValue.stub.php
         Cassandra.stub.php
+        ProtocolVersion.stub.php
         Future.stub.php
         FuturePreparedStatement.stub.php
         FutureRows.stub.php

src/Cluster/Builder.c

@@ -19,6 +19,7 @@
 #include <cassandra.h>
 
 #include <php_scylladb.h>
+#include <Zend/zend_enum.h>
 #include <php_scylladb_cache_key.h>
 #include <SSLOptions/SSLOptions.h>
 #include <php_scylladb_globals.h>
@@ -689,22 +690,29 @@ ZEND_METHOD(Cassandra_Cluster_Builder, withPersistentSessions)
 }
 ZEND_METHOD(Cassandra_Cluster_Builder, withProtocolVersion)
 {
-    zend_long version;
+    zend_object *versionCase = nullptr;
+    zend_long version = 0;
 
     ZEND_PARSE_PARAMETERS_START(1, 1)
-    Z_PARAM_LONG(version)
+    Z_PARAM_OBJ_OF_CLASS_OR_LONG(versionCase, php_scylladb_protocol_version_ce, version)
     ZEND_PARSE_PARAMETERS_END();
 
-    if (version < 1)
+    if (versionCase != nullptr)
     {
+        version = Z_LVAL_P(zend_enum_fetch_case_value(versionCase));
+    }
+    else if (version < 1)
+    {
+        /* An int still gets through for versions the enum does not name, such
+         * as the DSE ones — so it keeps its own range check. */
         zval val;
         ZVAL_LONG(&val, version);
         throw_invalid_argument(&val, "version", "a positive number");
         return;
     }
 
     auto self = PHP_SCYLLADB_GET_CLUSTER_BUILDER(getThis());
-    self->protocol_version = version;
+    self->protocol_version = (uint32_t)version;
 
     RETURN_ZVAL(getThis(), 1, 0);
 }

src/Cluster/Builder.stub.php

@@ -96,7 +96,7 @@ public function withPersistentSessions(bool $enabled = true): static
         {
         }
 
-        public function withProtocolVersion(int $version): static
+        public function withProtocolVersion(\Cassandra\ProtocolVersion|int $version): static
         {
         }
 

src/Cluster/BuilderHandlers.c

@@ -1,4 +1,5 @@
 #include <php.h>
+#include <Zend/zend_enum.h>
 
 #include <php_scylladb.h>
 #include <php_scylladb_globals.h>
@@ -199,7 +200,19 @@ HashTable *php_scylladb_cluster_builder_properties(zend_object *object)
     }
 
     ZVAL_BOOL(&usePersistentSessions, self->persist);
-    ZVAL_LONG(&protocolVersion, self->protocol_version);
+
+    zend_object *protocolVersionCase = nullptr;
+    if (zend_enum_get_case_by_value(&protocolVersionCase, php_scylladb_protocol_version_ce,
+                                    (zend_long)self->protocol_version, nullptr, true) == SUCCESS
+        && protocolVersionCase != nullptr)
+    {
+        ZVAL_OBJ_COPY(&protocolVersion, protocolVersionCase);
+    }
+    else
+    {
+        ZVAL_LONG(&protocolVersion, self->protocol_version);
+    }
+
     ZVAL_LONG(&ioThreads, self->io_threads);
     ZVAL_LONG(&coreConnectionPerHost, self->core_connections_per_host);
     ZVAL_LONG(&maxConnectionsPerHost, self->max_connections_per_host);

src/ProtocolVersion.c

@@ -0,0 +1,13 @@
+#include <cassandra.h>
+
+#include <php_scylladb.h>
+#include <php_scylladb_types.h>
+
+/* gen_stub ignores docblocks on enum cases, so `@cvalue` cannot bind a case to
+ * a CASS_* macro the way it does for class constants. The cases carry literal
+ * values and these assertions hold them to the driver header instead. */
+static_assert(CASS_PROTOCOL_VERSION_V1 == 1, "Cassandra\\ProtocolVersion::V1 no longer matches CASS_PROTOCOL_VERSION_V1");
+static_assert(CASS_PROTOCOL_VERSION_V2 == 2, "Cassandra\\ProtocolVersion::V2 no longer matches CASS_PROTOCOL_VERSION_V2");
+static_assert(CASS_PROTOCOL_VERSION_V3 == 3, "Cassandra\\ProtocolVersion::V3 no longer matches CASS_PROTOCOL_VERSION_V3");
+static_assert(CASS_PROTOCOL_VERSION_V4 == 4, "Cassandra\\ProtocolVersion::V4 no longer matches CASS_PROTOCOL_VERSION_V4");
+static_assert(CASS_PROTOCOL_VERSION_V5 == 5, "Cassandra\\ProtocolVersion::V5 no longer matches CASS_PROTOCOL_VERSION_V5");

src/ProtocolVersion.stub.php

@@ -0,0 +1,24 @@
+<?php
+
+/**
+ * @generate-class-entries
+ */
+
+declare(strict_types=1);
+
+namespace Cassandra {
+    /**
+     * Native protocol version used on the wire.
+     *
+     * The backing value is the version byte the protocol itself defines, so it
+     * matches CASS_PROTOCOL_VERSION_V1 … CASS_PROTOCOL_VERSION_V5.
+     */
+    enum ProtocolVersion: int
+    {
+        case V1 = 1;
+        case V2 = 2;
+        case V3 = 3;
+        case V4 = 4;
+        case V5 = 5;
+    }
+}

tests/Unit/ClusterBuilderProtocolVersionTest.php

@@ -0,0 +1,70 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Cassandra\Tests\Unit;
+
+use Cassandra;
+use Cassandra\ProtocolVersion;
+
+describe('Cassandra\ProtocolVersion', function () {
+
+    it('is an int-backed enum whose cases match the protocol version byte', function () {
+        expect(ProtocolVersion::V1->value)->toBe(1);
+        expect(ProtocolVersion::V2->value)->toBe(2);
+        expect(ProtocolVersion::V3->value)->toBe(3);
+        expect(ProtocolVersion::V4->value)->toBe(4);
+        expect(ProtocolVersion::V5->value)->toBe(5);
+    });
+
+    it('exposes exactly the five native protocol versions', function () {
+        expect(array_column(ProtocolVersion::cases(), 'name'))
+            ->toBe(['V1', 'V2', 'V3', 'V4', 'V5']);
+    });
+
+    it('resolves a case from its backing value', function () {
+        expect(ProtocolVersion::from(4))->toBe(ProtocolVersion::V4);
+        expect(ProtocolVersion::tryFrom(9))->toBeNull();
+    });
+});
+
+describe('Cassandra\Cluster\Builder protocol version', function () {
+
+    it('defaults to v4', function () {
+        $props = (array) Cassandra::cluster();
+
+        expect($props['protocolVersion'])->toBe(ProtocolVersion::V4);
+    });
+
+    it('stores the version passed as an enum case', function () {
+        $props = (array) Cassandra::cluster()->withProtocolVersion(ProtocolVersion::V5);
+
+        expect($props['protocolVersion'])->toBe(ProtocolVersion::V5);
+    });
+
+    it('returns the builder so calls chain', function () {
+        $builder = Cassandra::cluster();
+
+        expect($builder->withProtocolVersion(ProtocolVersion::V3))->toBe($builder);
+    });
+
+    it('still accepts a plain integer', function () {
+        $props = (array) Cassandra::cluster()->withProtocolVersion(3);
+
+        expect($props['protocolVersion'])->toBe(ProtocolVersion::V3);
+    });
+
+    it('keeps an integer the enum does not name', function () {
+        $props = (array) Cassandra::cluster()->withProtocolVersion(0x41);
+
+        expect($props['protocolVersion'])->toBe(0x41);
+    });
+
+    it('rejects a non-positive integer', function () {
+        Cassandra::cluster()->withProtocolVersion(0);
+    })->throws(\Cassandra\Exception\InvalidArgumentException::class);
+
+    it('rejects a value that is neither an int nor a ProtocolVersion', function () {
+        Cassandra::cluster()->withProtocolVersion('v4');
+    })->throws(\TypeError::class);
+});

tests/Unit/IniConfigurationTest.php

@@ -53,7 +53,7 @@ function builderProps(array $ini): ?array
         expect($props['contactPoints'])->toBe('10.0.0.1,10.0.0.2')
             ->and($props['defaultConsistency'])->toBe(\Cassandra::CONSISTENCY_LOCAL_QUORUM)
             ->and($props['defaultPageSize'])->toBe(250)
-            ->and($props['protocolVersion'])->toBe(3)
+            ->and($props['protocolVersion'])->toBe(\Cassandra\ProtocolVersion::V3)
             ->and($props['ioThreads'])->toBe(4)
             ->and($props['coreConnectionPerHost'])->toBe(2)
             ->and($props['maxConnectionsPerHost'])->toBe(8)
@@ -77,7 +77,7 @@ function builderProps(array $ini): ?array
         expect($props['contactPoints'])->toBe('127.0.0.1')
             ->and($props['defaultConsistency'])->toBe(\Cassandra::CONSISTENCY_LOCAL_QUORUM)
             ->and($props['defaultPageSize'])->toBe(5000)
-            ->and($props['protocolVersion'])->toBe(4)
+            ->and($props['protocolVersion'])->toBe(\Cassandra\ProtocolVersion::V4)
             ->and($props['usePersistentSessions'])->toBeTrue();
     });