Implement whitebox testing of constructor arguments (#24)

Implement a framework for retrieving the protobuf ConnectionRequest message
that the PHP ValkeyGlide and ValkeyGlideCluster constructors generate.

Add tests to validate various command inputs by examining the message
deserialized into PHP protobuf classes.

Fix various bugs:
* When using multiple addresses only the first address was used.
* Reconnection strategy was not being used.
* Client AZ was not being used.
* Connection timeout was not being used.
* Disabling periodic checks did not disable it correctly.
* Made defaults for connection timeout and reconnection strategy consistent with Rust client.

Integrate tests into CI:
* Register the test class
* Install PHP dependencies through composer in CI
* Generate PHP protobuf objects in CI
* Add dependency on symfony-polyfill to resolve issues finding ctype_* functions

Signed-off-by: James Duong <[email protected]>

Author: jduo

.github/workflows/build-php-wrapper/action.yml

@@ -45,6 +45,7 @@ runs:
             libtool \
             pkg-config \
             libssl-dev \
+            php-bcmath \
             unzip \
             clang-format \
             cppcheck \

.github/workflows/php.yml

@@ -140,6 +140,21 @@ jobs:
           github-token: ${{ secrets.GITHUB_TOKEN }}
           engine-version: ${{ matrix.engine.version }}
 
+      - name: Generate test protobuf PHP classes
+        run: |
+          protoc --proto_path=./valkey-glide/glide-core/src/protobuf --php_out=./tests/ ./valkey-glide/glide-core/src/protobuf/connection_request.proto
+          echo "tests directory contents:"
+          cd tests
+          ls -la
+
+      - name: Install PHP dependencies
+        run: |
+          composer install --no-interaction --prefer-dist --optimize-autoloader
+          echo "Composer install completed. Checking for composer.lock:"
+          ls -la composer.lock || echo "Warning: composer.lock not found"
+          echo "Current directory contents:"
+          ls -la
+
       - name: Start Valkey servers
         working-directory: tests
         run: |

.gitignore

@@ -81,19 +81,23 @@ src/**
 include/**
 lib/**
 .libs/**
+tests/.libs/**
 *.dep
 *.lo
 valkey_glide.la
 valkey_glide_arginfo.h
 valkey_glide_cluster_arginfo.h
 valkey_glide_cluster_legacy_arginfo.h
 valkey_glide_legacy_arginfo.h
+logger_arginfo.h
+logger_legacy_arginfo.h
 run-tests.php
 modules/valkey_glide.so
 libtool
 cluster_scan_cursor_legacy_arginfo.h
 cluster_scan_cursor_arginfo.h
 tests/valkey_data/*/valkey.log
+composer.lock
 configure
 configure.ac
 config.h.in
@@ -106,3 +110,8 @@ Makefile.objects
 autom4te.cache
 config.h.in~
 configure~
+tests/Connection_request/**
+tests/GPBMetadata/**
+tests/client_constructor_mock_arginfo.h
+tests/client_constructor_mock_legacy_arginfo.h
+vendor/**

DEVELOPER.md

@@ -55,7 +55,7 @@ Continue with **Install protobuf compiler** and **Install `ziglang` and `zigbuil
 
 ```bash
 sudo yum update -y
-sudo yum install -y php-devel php-cli git gcc make autoconf automake libtool pkgconfig openssl openssl-devel unzip
+sudo yum install -y php-devel php-cli git gcc make autoconf automake libtool pkgconfig openssl openssl-devel unzip php-bcmath
 # Install rust
 curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh
 source "$HOME/.cargo/env"
@@ -190,6 +190,8 @@ Before starting this step, make sure you've installed all software requirements.
 To run unit tests, use:
 
 ```bash
+protoc --proto_path=./valkey-glide/glide-core/src/protobuf --php_out=./tests/ ./valkey-glide/glide-core/src/protobuf/connection_request.proto
+composer install --no-interaction --prefer-dist --optimize-autoloader
 make install
 cd tests
 ./start_valkey_with_replicas.sh 

Makefile.frag

@@ -74,13 +74,17 @@ cluster_scan_cursor_arginfo.h: cluster_scan_cursor.stub.php
 	@echo "Generating arginfo from cluster_scan_cursor.stub.php"
 	$(PHP_EXECUTABLE) build/gen_stub.php --no-legacy-arginfo cluster_scan_cursor.stub.php
 
-ARGINFO_HEADERS = valkey_glide_arginfo.h valkey_glide_cluster_arginfo.h cluster_scan_cursor_arginfo.h logger_arginfo.h
+tests/client_constructor_mock_arginfo.h: tests/client_constructor_mock.stub.php
+	@echo "Generating arginfo from tests/client_constructor_mock_arginfo.stub.php"
+	$(PHP_EXECUTABLE) build/gen_stub.php --no-legacy-arginfo tests/client_constructor_mock.stub.php
+
+ARGINFO_HEADERS = valkey_glide_arginfo.h valkey_glide_cluster_arginfo.h cluster_scan_cursor_arginfo.h logger_arginfo.h tests/client_constructor_mock_arginfo.h
 
 all: $(ARGINFO_HEADERS)
 
 .PHONY: build-modules-pre
 
-build-modules-pre: valkey_glide_arginfo.h valkey_glide_cluster_arginfo.h cluster_scan_cursor_arginfo.h logger_arginfo.h
+build-modules-pre: valkey_glide_arginfo.h valkey_glide_cluster_arginfo.h cluster_scan_cursor_arginfo.h logger_arginfo.h tests/client_constructor_mock_arginfo.h
 	@$(MAKE) generate-proto
 	@$(MAKE) generate-bindings
 

README.md

@@ -68,13 +68,14 @@ Before installing Valkey GLIDE PHP extension, ensure you have the following depe
 - protoc (protobuf compiler) >= v3.20.0
 - openssl and openssl-dev
 - rustup (Rust toolchain)
+- php-bcmath (Protobuf PHP dependency, needed only for testing)
 
 ### Installing Dependencies
 
 **Ubuntu/Debian:**
 ```bash
 sudo apt update -y
-sudo apt install -y php-dev php-cli git gcc make autotools-dev pkg-config openssl libssl-dev unzip
+sudo apt install -y php-dev php-cli git gcc make autotools-dev pkg-config openssl libssl-dev unzip php-bcmath
 # Install rust
 curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh
 source "$HOME/.cargo/env"
@@ -83,7 +84,7 @@ source "$HOME/.cargo/env"
 **CentOS/RHEL:**
 ```bash
 sudo yum update -y
-sudo yum install -y php-devel php-cli git gcc make pkgconfig openssl openssl-devel unzip
+sudo yum install -y php-devel php-cli git gcc make pkgconfig openssl openssl-devel unzip php-bcmath
 # Install rust
 curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh
 source "$HOME/.cargo/env"
@@ -150,8 +151,18 @@ protoc --version
     ```ini
     extension=valkey_glide
     ```
-
-6. Execute the tests:
+   
+6. Generate PHP protobuf classes used for testing purposes:
+   ```bash
+   protoc --proto_path=./valkey-glide/glide-core/src/protobuf --php_out=./tests/ ./valkey-glide/glide-core/src/protobuf/connection_request.proto
+   ```
+
+7. Install PHP dependencies with composer:
+   ```bash
+   composer install --no-interaction --prefer-dist --optimize-autoloader
+   ```
+
+8. Execute the tests:
     ```
     make test
     ```

common.h

@@ -67,19 +67,28 @@ typedef struct {
     char* username; /* Optional */
 } valkey_glide_server_credentials_t;
 
+/* Default values for connection configuration options. */
+#define VALKEY_GLIDE_DEFAULT_NUM_OF_RETRIES 5
+#define VALKEY_GLIDE_DEFAULT_FACTOR 100
+#define VALKEY_GLIDE_DEFAULT_EXPONENT_BASE 2
+#define VALKEY_GLIDE_DEFAULT_JITTER_PERCENTAGE 20
+
+#define VALKEY_GLIDE_DEFAULT_CONNECTION_TIMEOUT 250
+
+
 typedef struct {
     int num_of_retries;
     int factor;
     int exponent_base;
-    int jitter_percent; /* -1 if not set */
+    int jitter_percent;
 } valkey_glide_backoff_strategy_t;
 
 typedef struct {
     bool use_insecure_tls; /* false if not set */
 } valkey_glide_tls_advanced_configuration_t;
 
 typedef struct {
-    int                                        connection_timeout; /* -1 if not set */
+    int                                        connection_timeout; /* In milliseconds. */
     valkey_glide_tls_advanced_configuration_t* tls_config;         /* NULL if not set */
 } valkey_glide_advanced_base_client_configuration_t;
 

composer.json

@@ -15,18 +15,16 @@
     },
     "require-dev": {
         "squizlabs/php_codesniffer": "^3.7",
-        "phpstan/phpstan": "^1.10"
+        "phpstan/phpstan": "^1.10",
+        "google/protobuf": "^v4.32.0",
+        "protobuf-php/protobuf-plugin": "^0.1.3",
+        "symfony/polyfill-ctype": "^1.32.0"
     },
     "autoload": {
         "psr-4": {
             "ValkeyGlide\\": "src/"
         }
     },
-    "autoload-dev": {
-        "psr-4": {
-            "ValkeyGlide\\Tests\\": "tests/"
-        }
-    },
     "scripts": {
         "lint": [
             "phpcs --standard=phpcs.xml"

config.m4

@@ -52,7 +52,7 @@ if test "$PHP_VALKEY_GLIDE" != "no"; then
     LDFLAGS="$LDFLAGS $PHP_VALKEY_GLIDE_LDFLAGS"
   fi
   PHP_NEW_EXTENSION(valkey_glide,
-    valkey_glide.c valkey_glide_cluster.c cluster_scan_cursor.c command_response.c logger.c valkey_glide_commands.c valkey_glide_commands_2.c valkey_glide_commands_3.c valkey_glide_core_commands.c valkey_glide_core_common.c valkey_glide_expire_commands.c valkey_glide_geo_commands.c valkey_glide_geo_common.c valkey_glide_hash_common.c valkey_glide_list_common.c valkey_glide_s_common.c valkey_glide_str_commands.c valkey_glide_x_commands.c valkey_glide_x_common.c valkey_glide_z.c valkey_glide_z_common.c valkey_z_php_methods.c src/command_request.pb-c.c src/connection_request.pb-c.c src/response.pb-c.c,
+    valkey_glide.c valkey_glide_cluster.c cluster_scan_cursor.c command_response.c logger.c valkey_glide_commands.c valkey_glide_commands_2.c valkey_glide_commands_3.c valkey_glide_core_commands.c valkey_glide_core_common.c valkey_glide_expire_commands.c valkey_glide_geo_commands.c valkey_glide_geo_common.c valkey_glide_hash_common.c valkey_glide_list_common.c valkey_glide_s_common.c valkey_glide_str_commands.c valkey_glide_x_commands.c valkey_glide_x_common.c valkey_glide_z.c valkey_glide_z_common.c valkey_z_php_methods.c src/command_request.pb-c.c src/connection_request.pb-c.c src/response.pb-c.c tests/client_constructor_mock.c,
     $ext_shared)
 
   EXTRA_DIST="$EXTRA_DIST valkey_glide.stub.php valkey_glide_cluster.stub.php logger.stub.php"

php_build.sh

@@ -7,6 +7,8 @@ cd valkey-glide/ffi
 cargo build --release
 cd ../../
 
+protoc --proto_path=./valkey-glide/glide-core/src/protobuf --php_out=./tests/generated ./valkey-glide/glide-core/src/protobuf/connection_request.proto
+
 phpize
 ./configure --enable-valkey-glide
 make -j4 build-modules-pre