[Draft] Add fix and test it

.github/workflows/continuous-integration.yaml

@@ -1,74 +1,74 @@
-name: 'Continuous integration'
-on: ['push', 'pull_request']
+name: "Continuous integration"
+on: ["push", "pull_request"]
 jobs:
   cs:
-    runs-on: 'ubuntu-20.04'
-    name: 'Coding style'
+    runs-on: "ubuntu-24.04"
+    name: "Coding style"
     steps:
-      - name: 'Checkout'
-        uses: 'actions/checkout@v3'
+      - name: "Checkout"
+        uses: "actions/checkout@v3"
 
-      - name: 'Setup PHP'
-        uses: 'shivammathur/setup-php@v2'
+      - name: "Setup PHP"
+        uses: "shivammathur/setup-php@v2"
         with:
-          php-version: '7.4'
-          coverage: 'none'
-          extensions: 'json, mbstring, tokenizer'
-          tools: 'composer-normalize:2.28.0, php-cs-fixer:3.9.5'
+          php-version: "7.4"
+          coverage: "none"
+          extensions: "json, mbstring, tokenizer"
+          tools: "composer-normalize:2.28.0, php-cs-fixer:3.9.5"
 
-      - name: 'Check PHP code'
+      - name: "Check PHP code"
         run: |
           php-cs-fixer fix --diff --dry-run --allow-risky=yes --using-cache=no
 
-      - name: 'Check composer.json'
+      - name: "Check composer.json"
         run: |
           composer-normalize --diff --dry-run --indent-size=4 --indent-style=space --no-update-lock
 
   phpunit:
-    runs-on: 'ubuntu-20.04'
-    name: 'PHPUnit (PHP ${{ matrix.php }}, ES ${{ matrix.elasticsearch }})'
+    runs-on: "ubuntu-24.04"
+    name: "PHPUnit (PHP ${{ matrix.php }}, ES ${{ matrix.elasticsearch }})"
     timeout-minutes: 10
     strategy:
       matrix:
         php:
-          - '7.2'
-          - '7.3'
-          - '7.4'
-          - '8.0'
-          - '8.1'
+          - "7.2"
+          - "7.3"
+          - "7.4"
+          - "8.0"
+          - "8.1"
         dependencies:
-          - 'highest'
+          - "highest"
         elasticsearch:
-          - '7.15.2'
+          - "7.15.2"
         include:
           # Test with the lowest set of dependencies
-          - dependencies: 'lowest'
-            php: '7.2'
-            elasticsearch: '7.15.2'
+          - dependencies: "lowest"
+            php: "7.2"
+            elasticsearch: "7.15.2"
       fail-fast: false
     steps:
-      - name: 'Checkout'
-        uses: 'actions/checkout@v3'
+      - name: "Checkout"
+        uses: "actions/checkout@v3"
 
-      - name: 'Setup PHP'
-        uses: 'shivammathur/setup-php@v2'
+      - name: "Setup PHP"
+        uses: "shivammathur/setup-php@v2"
         with:
-          php-version: '${{ matrix.php }}'
-          coverage: 'pcov'
-          tools: 'pecl, composer:v2'
-          extensions: 'curl, json, mbstring, openssl'
+          php-version: "${{ matrix.php }}"
+          coverage: "pcov"
+          tools: "pecl, composer:v2"
+          extensions: "curl, json, mbstring, openssl"
 
-      - name: 'Install dependencies with Composer'
-        uses: 'ramsey/composer-install@v2'
+      - name: "Install dependencies with Composer"
+        uses: "ramsey/composer-install@v2"
         with:
-          dependency-versions: '${{ matrix.dependencies }}'
-          composer-options: '--prefer-dist'
+          dependency-versions: "${{ matrix.dependencies }}"
+          composer-options: "--prefer-dist"
 
-      - name: 'Run unit tests'
+      - name: "Run unit tests"
         run: |
           vendor/bin/phpunit --group unit --coverage-clover=build/coverage/unit-coverage.xml
 
-      - name: 'Setup Elasticsearch'
+      - name: "Setup Elasticsearch"
         run: |
           sudo swapoff -a
           sudo sysctl -w vm.swappiness=1
@@ -78,44 +78,43 @@ jobs:
           docker run --rm --network=docker_elastic curlimages/curl --max-time 120 --retry-max-time 120 --retry 120 --retry-delay 5 --retry-all-errors --show-error --silent http://es01:9200
           docker run --rm --network=docker_elastic curlimages/curl --max-time 120 --retry-max-time 120 --retry 120 --retry-delay 5 --retry-all-errors --show-error --silent http://es02:9200
 
-      - name: 'Run functional tests'
+      - name: "Run functional tests"
         run: |
           vendor/bin/phpunit --group functional --coverage-clover=build/coverage/functional-coverage.xml
 
-      - name: 'Upload coverage to Codecov'
+      - name: "Upload coverage to Codecov"
         uses: codecov/codecov-action@v2
         with:
-            files: build/coverage/unit-coverage.xml,build/coverage/functional-coverage.xml
+          files: build/coverage/unit-coverage.xml,build/coverage/functional-coverage.xml
 
   phpstan:
-    runs-on: 'ubuntu-20.04'
-    name: 'PHPStan (PHP ${{ matrix.php }}, ES ${{ matrix.elasticsearch }})'
+    runs-on: "ubuntu-24.04"
+    name: "PHPStan (PHP ${{ matrix.php }}, ES ${{ matrix.elasticsearch }})"
     timeout-minutes: 10
     strategy:
       matrix:
         php:
-          - '8.1'
+          - "8.1"
         elasticsearch:
-          - '7.15.2'
+          - "7.15.2"
       fail-fast: false
     steps:
-      - name: 'Checkout'
-        uses: 'actions/checkout@v3'
+      - name: "Checkout"
+        uses: "actions/checkout@v3"
 
-      - name: 'Setup PHP'
-        uses: 'shivammathur/setup-php@v2'
+      - name: "Setup PHP"
+        uses: "shivammathur/setup-php@v2"
         with:
-          php-version: '${{ matrix.php }}'
-          coverage: 'none'
-          tools: 'pecl, composer:v2'
-          extensions: 'curl, json, mbstring, openssl'
+          php-version: "${{ matrix.php }}"
+          coverage: "none"
+          tools: "pecl, composer:v2"
+          extensions: "curl, json, mbstring, openssl"
 
-
-      - name: 'Install dependencies with Composer'
-        uses: 'ramsey/composer-install@v2'
+      - name: "Install dependencies with Composer"
+        uses: "ramsey/composer-install@v2"
         with:
-          composer-options: '--prefer-dist'
+          composer-options: "--prefer-dist"
 
-      - name: 'Run phpstan'
+      - name: "Run phpstan"
         run: |
           vendor/bin/phpstan analyse --no-progress --error-format=github

phive.xml

@@ -1,5 +1,6 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <phive xmlns="https://phar.io/phive">
-  <phar name="php-cs-fixer" version="^3.0" installed="3.9.5" location="./tools/php-cs-fixer.phar" copy="true"/>
+  <phar name="php-cs-fixer" version="^3.84.0" installed="3.84.0" location="./tools/php-cs-fixer" copy="true"/>
   <phar name="php-coveralls" version="^2.2" installed="2.2.0" location="./tools/php-coveralls.phar" copy="true"/>
+  <phar name="composer-normalize" version="2.28.0" installed="2.28.0" location="./tools/composer-normalize" copy="true"/>
 </phive>

src/Bulk.php

@@ -131,8 +131,12 @@ public function getActions(): array
      */
     public function addDocument(Document $document, ?string $opType = null): self
     {
-        if (!$document->hasRetryOnConflict() && $this->_client->hasConnection() && $this->_client->getConnection()->hasParam('retryOnConflict') && ($retry = $this->_client->getConnection()->getParam('retryOnConflict')) > 0) {
-            $document->setRetryOnConflict($retry);
+        if (!$document->hasRetryOnConflict()) {
+            $retry = $this->_client->getConfigValue('retryOnConflict', 0);
+
+            if ($retry > 0) {
+                $document->setRetryOnConflict($retry);
+            }
         }
 
         $action = AbstractDocumentAction::create($document, $opType);
@@ -159,8 +163,12 @@ public function addDocuments(array $documents, ?string $opType = null): self
      */
     public function addScript(AbstractScript $script, ?string $opType = null): self
     {
-        if (!$script->hasRetryOnConflict() && $this->_client->hasConnection() && $this->_client->getConnection()->hasParam('retryOnConflict') && ($retry = $this->_client->getConnection()->getParam('retryOnConflict')) > 0) {
-            $script->setRetryOnConflict($retry);
+        if (!$script->hasRetryOnConflict()) {
+            $retry = $this->_client->getConfigValue('retryOnConflict', 0);
+
+            if ($retry > 0) {
+                $script->setRetryOnConflict($retry);
+            }
         }
 
         $action = AbstractDocumentAction::create($script, $opType);

tests/BulkTest.php

@@ -277,6 +277,7 @@ public function testAddRawData(): void
 
     /**
      * @group unit
+     *
      * @dataProvider invalidRawDataProvider
      *
      * @param mixed $rawData
@@ -703,8 +704,8 @@ public function testRetry(): void
         $metadata = $actions[0]->getMetadata();
         $this->assertEquals(5, $metadata['retry_on_conflict']);
 
-        // Test retry via client
-        $client->getConnection()->setParam('retryOnConflict', 5);
+        // Test retry via client configuration (fixed implementation)
+        $client->setConfigValue('retryOnConflict', 5);
         $doc2 = new Document('2', ['name' => 'Invisible Woman']);
         $doc2->setOpType(Action::OP_TYPE_UPDATE);
 
@@ -819,4 +820,102 @@ public function testSendRequestEntityTooLargeExceptionIf413Response(): void
         $this->expectException(RequestEntityTooLargeException::class);
         $bulk->send();
     }
+
+    /**
+     * @group unit
+     * Test that verifies the fix for issue #2219 - hasConnection() error
+     */
+    public function testRetryOnConflictFromClientConfig(): void
+    {
+        $client = $this->_getClient();
+
+        // Set retryOnConflict in client configuration
+        $client->setConfigValue('retryOnConflict', 3);
+
+        // Create documents without explicit retryOnConflict
+        $doc1 = new Document('1', ['name' => 'Test Document 1']);
+        $doc2 = new Document('2', ['name' => 'Test Document 2']);
+
+        // Create scripts without explicit retryOnConflict
+        $script1 = new Script('ctx._source.name = "Updated"');
+        $script2 = new Script('ctx._source.status = "active"');
+
+        // Test that addDocument correctly applies retryOnConflict from client config
+        $bulk = new Bulk($client);
+        $bulk->addDocument($doc1);
+        $bulk->addScript($script1);
+
+        $actions = $bulk->getActions();
+
+        // Verify that retry_on_conflict is set from client configuration
+        $this->assertCount(2, $actions);
+
+        $docMetadata = $actions[0]->getMetadata();
+        $this->assertEquals(3, $docMetadata['retry_on_conflict']);
+
+        $scriptMetadata = $actions[1]->getMetadata();
+        $this->assertEquals(3, $scriptMetadata['retry_on_conflict']);
+    }
+
+    /**
+     * @group unit
+     * Test that verifies documents with explicit retryOnConflict are not overridden
+     */
+    public function testRetryOnConflictExplicitValueNotOverridden(): void
+    {
+        $client = $this->_getClient();
+
+        // Set retryOnConflict in client configuration
+        $client->setConfigValue('retryOnConflict', 5);
+
+        // Create document with explicit retryOnConflict
+        $doc = new Document('1', ['name' => 'Test Document']);
+        $doc->setRetryOnConflict(2); // Explicit value
+
+        // Create script with explicit retryOnConflict
+        $script = new Script('ctx._source.name = "Updated"');
+        $script->setRetryOnConflict(1); // Explicit value
+
+        $bulk = new Bulk($client);
+        $bulk->addDocument($doc);
+        $bulk->addScript($script);
+
+        $actions = $bulk->getActions();
+
+        // Verify that explicit values are preserved
+        $docMetadata = $actions[0]->getMetadata();
+        $this->assertEquals(2, $docMetadata['retry_on_conflict']);
+
+        $scriptMetadata = $actions[1]->getMetadata();
+        $this->assertEquals(1, $scriptMetadata['retry_on_conflict']);
+    }
+
+    /**
+     * @group unit
+     * Test that verifies zero retryOnConflict from client config is not applied
+     */
+    public function testRetryOnConflictZeroNotApplied(): void
+    {
+        $client = $this->_getClient();
+
+        // Set retryOnConflict to 0 in client configuration
+        $client->setConfigValue('retryOnConflict', 0);
+
+        // Create document without explicit retryOnConflict
+        $doc = new Document('1', ['name' => 'Test Document']);
+        $script = new Script('ctx._source.name = "Updated"');
+
+        $bulk = new Bulk($client);
+        $bulk->addDocument($doc);
+        $bulk->addScript($script);
+
+        $actions = $bulk->getActions();
+
+        // Verify that retry_on_conflict is not set when value is 0
+        $docMetadata = $actions[0]->getMetadata();
+        $this->assertArrayNotHasKey('retry_on_conflict', $docMetadata);
+
+        $scriptMetadata = $actions[1]->getMetadata();
+        $this->assertArrayNotHasKey('retry_on_conflict', $scriptMetadata);
+    }
 }