Merge 9f443a1 into 2469b53

.coveralls.yml

@@ -1,3 +1,4 @@
-coverage_clover: tests/App/build/clover.xml
-service_name: travis-ci
-json_path: tests/App/build/coveralls.json
+# todo: deprecated - remove this file, it was used with travis ci
+#coverage_clover: tests/App/build/clover.xml
+#service_name: travis-ci
+#json_path: tests/App/build/coveralls.json

.env

@@ -0,0 +1,5 @@
+# Override for the ES port to use for the docker container
+ELASTICSEARCH_PORT=9201
+
+# Override for the ES version to use for the docker container
+#ELASTICSEARCH_VERSION=8.8.0

.github/workflows/phpunit-tests.yml

@@ -0,0 +1,102 @@
+name: PHPUnit tests
+
+#concurrency:
+#    group: phpunit-tests-${{ github.ref_name }}
+#    cancel-in-progress: true
+
+on: ['push', 'pull_request', 'workflow_dispatch']
+
+env:
+    ELASTICSEARCH_PORT: 9201
+
+jobs:
+    phpunit:
+        runs-on: 'ubuntu-20.04'
+        name: 'PHPUnit (PHP ${{ matrix.php }}, Symfony ${{ matrix.symfony }}, ES ${{ matrix.elasticsearch }})'
+        timeout-minutes: 30
+        continue-on-error: ${{ matrix.experimental }}
+        strategy:
+            matrix:
+                experimental:
+                    - false
+                dependencies:
+                    - 'highest'
+                php:
+                    - '8.1'
+                    - '8.2'
+                elasticsearch:
+                    - '7.17.13'
+                symfony:
+                    - '~5.0'
+                include:
+                    - php: '8.1'
+                      symfony: '~5.0'
+                      elasticsearch: '8.0.1'
+                      experimental: false
+                    - php: '8.1'
+                      symfony: '~5.0'
+                      elasticsearch: '8.1.3' # there are some bc in minor version https://www.elastic.co/guide/en/elasticsearch/reference/current/migrating-8.1.html#breaking-changes-8.1
+                      experimental: false
+                    - php: '8.1'
+                      symfony: '~5.0'
+                      elasticsearch: '8.5.3' # there are some bc in minor version https://www.elastic.co/guide/en/elasticsearch/reference/current/migrating-8.5.html
+                      experimental: false
+                    - php: '8.1'
+                      symfony: '~5.0'
+                      elasticsearch: '8.6.2' # there are no bc in minor version https://www.elastic.co/guide/en/elasticsearch/reference/current/migrating-8.6.html
+                      experimental: false
+                    - php: '8.1'
+                      symfony: '~5.0'
+                      elasticsearch: '8.7.1' # there are no bc in minor version https://www.elastic.co/guide/en/elasticsearch/reference/current/migrating-8.7.html
+                      experimental: false
+                    - php: '8.1'
+                      symfony: '~5.0'
+                      elasticsearch: '8.8.0' # there are no bc in minor version https://www.elastic.co/guide/en/elasticsearch/reference/current/migrating-8.8.html
+                      experimental: false
+                    - php: '8.1'
+                      symfony: '~6.0'
+                      elasticsearch: '8.10.2' # newest version
+                      experimental: false
+            fail-fast: false
+        steps:
+            - name: 'Checkout'
+              uses: 'actions/checkout@v4'
+
+            - name: 'Setup PHP'
+              uses: 'shivammathur/setup-php@v2'
+              with:
+                  php-version: '${{ matrix.php }}'
+                  coverage: 'pcov'
+                  tools: 'composer:v2'
+                  extensions: 'curl, json, mbstring, openssl'
+                  ini-values: 'memory_limit=256M'
+
+            - name: 'Validate composer.json and composer.lock'
+              run: composer validate
+
+            - name: 'Fix symfony/framework-bundle version'
+              run: composer require --no-update symfony/framework-bundle:${{ matrix.symfony }}
+
+            - name: 'Install dependencies with Composer'
+              uses: 'ramsey/composer-install@v2'
+              with:
+                  dependency-versions: '${{ matrix.dependencies }}'
+                  composer-options: '--prefer-dist'
+
+            - name: 'Dump composer autoloader'
+              run: composer dump-autoload --classmap-authoritative --no-ansi --no-interaction --no-scripts
+
+            - name: 'Setup Elasticsearch'
+              env:
+                  ELASTICSEARCH_VERSION: ${{ matrix.elasticsearch }}
+              run: docker compose up --detach --wait ; curl -XGET 'http://localhost:'"$ELASTICSEARCH_PORT"
+
+            - name: 'Run phpunit tests'
+              run: |
+                  vendor/bin/simple-phpunit --coverage-clover=tests/App/build/clover.xml 2>/dev/null
+
+            - name: Upload coverage results to Coveralls
+              env:
+                  COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }}
+              run: |
+                  vendor/bin/php-coveralls --coverage_clover=tests/App/build/clover.xml --json_path=tests/App/build/coveralls.json -v

README.md

@@ -16,7 +16,7 @@
 
 ## Documentation
 
-Installation instructions and documentation of the bundle can be found [here](src/docs/index.md).
+Installation instructions and documentation of the bundle can be found [here](docs/index.md).
 
 ## Version matrix
 

composer.json

@@ -32,6 +32,7 @@
         "symfony/stopwatch": "^4.4 || ^5.0",
         "symfony/phpunit-bridge": "^4.4 || ^5.0",
         "symfony/browser-kit": "^4.4 || ^5.0",
+        "symfony/dotenv": "^4.4 || ^5.0",
         "doctrine/orm": "^2.6.3",
 
         "monolog/monolog": "^1.0|^2.0|^3.0",
@@ -50,8 +51,7 @@
     "autoload": {
         "psr-4": {
             "Sineflow\\ElasticsearchBundle\\": "src/"
-        },
-        "exclude-from-classmap": ["/tests/"]
+        }
     },
     "autoload-dev": {
         "psr-4": {

docker-compose.yml

@@ -0,0 +1,34 @@
+services:
+  elasticsearch:
+    image: "docker.elastic.co/elasticsearch/elasticsearch:${ELASTICSEARCH_VERSION:-7.17.13}"
+    container_name: sfes_elasticsearch
+    environment:
+      - discovery.type=single-node
+      - bootstrap.memory_lock=true
+      - "ES_JAVA_OPTS=-Xms${ELASTICSEARCH_JAVA_MEM:-2048m} -Xmx${ELASTICSEARCH_JAVA_MEM:-2048m}"
+      - cluster.name=sfes
+      - node.name=sfes1
+      - xpack.security.enabled=false
+      - indices.id_field_data.enabled=true # TODO: temporary requirement for ES8 until tests are fixed to not do sorting/aggregations on _id (https://github.com/elastic/elasticsearch/issues/43599)
+    ulimits:
+      memlock:
+        soft: -1
+        hard: -1
+    volumes:
+      - elasticsearch-data:/usr/share/elasticsearch/data
+    ports:
+      - "${ELASTICSEARCH_PORT:-9200}:9200"
+    networks:
+      - elastic
+    healthcheck:
+      test: curl -s http://sfes_elasticsearch:$${ELASTICSEARCH_PORT:-9200} > /dev/null || exit 1
+      interval: 1s
+      timeout: 5s
+      retries: 120
+
+volumes:
+  elasticsearch-data:
+
+networks:
+  elastic:
+    driver: bridge

phpunit.xml.dist

@@ -25,6 +25,7 @@
     </testsuites>
 
     <php>
+        <server name="APP_ENV" value="test" force="true" />
         <server name="KERNEL_CLASS" value="Sineflow\ElasticsearchBundle\Tests\App\AppKernel" />
         <server name="SHELL_VERBOSITY" value="-1" />
         <env name="SYMFONY_PHPUNIT_VERSION" value="9" />

src/Manager/ConnectionManager.php

@@ -216,16 +216,16 @@ public function commit(bool $forceRefresh = true)
 
         $this->bulkQueries = [];
 
+        if ($this->eventDispatcher) {
+            $this->eventDispatcher->dispatch(new PostCommitEvent($response, $this), Events::POST_COMMIT);
+        }
+
         if ($response['errors']) {
             $errorCount = $this->logBulkRequestErrors($response['items']);
             $e = new BulkRequestException(\sprintf('Bulk request failed with %s error(s)', $errorCount));
             $e->setBulkResponseItems($response['items'], $bulkRequest);
             throw $e;
         }
-
-        if ($this->eventDispatcher) {
-            $this->eventDispatcher->dispatch(new PostCommitEvent($response, $this), Events::POST_COMMIT);
-        }
     }
 
     /**

src/Subscriber/EntityTrackerSubscriber.php

@@ -67,13 +67,18 @@ public function onPostCommit(PostCommitEvent $postCommitEvent)
 
         // Update the ids of persisted entity objects
         foreach ($this->entitiesData[$postCommitEvent->getConnectionName()] as $bulkOperationIndex => $entityData) {
-            $idValue = \current($postCommitEvent->getBulkResponse()['items'][$bulkOperationIndex])['_id'];
-            $idPropertyMetadata = $entityData['metadata']['_id'];
-            $entity = $entityData['entity'];
-            if (DocumentMetadata::PROPERTY_ACCESS_PRIVATE === $idPropertyMetadata['propertyAccess']) {
-                $entity->{$idPropertyMetadata['methods']['setter']}($idValue);
-            } else {
-                $entity->{$idPropertyMetadata['propertyName']} = $idValue;
+            $bulkResponseItem = $postCommitEvent->getBulkResponse()['items'][$bulkOperationIndex];
+            $operation = \key($bulkResponseItem);
+            $bulkResponseItemValue = \current($bulkResponseItem);
+            if (in_array($operation, ['create', 'index']) && !isset($bulkResponseItemValue['error'])) {
+                $idValue = $bulkResponseItemValue['_id'];
+                $idPropertyMetadata = $entityData['metadata']['_id'];
+                $entity = $entityData['entity'];
+                if (DocumentMetadata::PROPERTY_ACCESS_PRIVATE === $idPropertyMetadata['propertyAccess']) {
+                    $entity->{$idPropertyMetadata['methods']['setter']}($idValue);
+                } else {
+                    $entity->{$idPropertyMetadata['propertyName']} = $idValue;
+                }
             }
         }
 

tests/App/config/config_test.yml

@@ -1,6 +1,6 @@
 parameters:
     elasticsearch_hosts:
-        - '127.0.0.1:9200'
+        - '127.0.0.1:%env(ELASTICSEARCH_PORT)%'
 
 # Framework Configuration
 framework:

tests/Functional/Subscriber/EntityTrackerSubscriberTest.php

@@ -2,6 +2,7 @@
 
 namespace Sineflow\ElasticsearchBundle\Tests\Functional\Subscriber;
 
+use Sineflow\ElasticsearchBundle\Exception\BulkRequestException;
 use Sineflow\ElasticsearchBundle\Finder\Finder;
 use Sineflow\ElasticsearchBundle\Result\DocumentConverter;
 use Sineflow\ElasticsearchBundle\Tests\AbstractElasticsearchTestCase;
@@ -13,6 +14,50 @@
  */
 class EntityTrackerSubscriberTest extends AbstractElasticsearchTestCase
 {
+    /**
+     * Test executing 2 separate bulk requests, where there's an error in the first one,
+     * to make sure the subscriber properly keeps track of bulk request items.
+     */
+    public function testTwoBulkRequestWithErrorInFirstOne()
+    {
+        $imWithAliases = $this->getIndexManager('customer');
+        $customer1 = new Customer();
+        $customer1->name = 'batman';
+        $customer1->setActive('invalid value');
+        $customer2 = new Customer();
+        $customer2->name = 'robin';
+        $imWithAliases->persist($customer1);
+        $imWithAliases->persist($customer2);
+        try {
+            $imWithAliases->getConnection()->commit();
+        } catch (BulkRequestException $e) {
+            // ignore the exception
+        }
+
+        $customer3 = new Customer();
+        $customer3->name = 'superman';
+        $imWithAliases->persist($customer3);
+        $imWithAliases->getConnection()->commit();
+
+        $this->assertNull($customer1->id, 'id should not have been set');
+        $this->assertNotNull($customer2->id, 'id should have been set');
+        $this->assertNotNull($customer3->id, 'id should have been set');
+
+        // Make sure that the correct id was assigned to the object, not the id of another customer
+        // Get the customer from ES by name
+        $finder = $this->getContainer()->get(Finder::class);
+
+        $docs = $finder->find(['AcmeFooBundle:Customer'], ['query' => ['match' => ['name' => 'robin']]]);
+        $this->assertCount(1, $docs);
+        $retrievedCustomer2 = $docs->current();
+        $this->assertEquals($customer2->id, $retrievedCustomer2->id);
+
+        $docs = $finder->find(['AcmeFooBundle:Customer'], ['query' => ['match' => ['name' => 'superman']]]);
+        $this->assertCount(1, $docs);
+        $retrievedCustomer3 = $docs->current();
+        $this->assertEquals($customer3->id, $retrievedCustomer3->id);
+    }
+
     /**
      * Test populating persisted entity ids after a bulk operation with several operations
      */

tests/Unit/DependencyInjection/ElasticsearchExtensionTest.php

@@ -107,6 +107,7 @@ public function getData()
                 'profiling' => false,
                 'logging' => false,
                 'bulk_batch_size' => 123,
+                'ssl_verification' => null,
             ],
         ];
 

tests/tests.bootstrap.php

@@ -1,3 +1,12 @@
 <?php
 
+use Symfony\Component\Dotenv\Dotenv;
+
 require __DIR__.'/../vendor/autoload.php';
+
+/*
+ * @see https://symfony.com/doc/5.4/testing/bootstrap.html
+ */
+if (method_exists(Dotenv::class, 'bootEnv')) {
+    (new Dotenv())->bootEnv(dirname(__DIR__).'/.env');
+}