upgrade to Solr 9 and stop supporting removed features; make tests mo…

…re explicit in regards to expected call counts on mocks
tsterker · Nov 27, 2023 · 4620d2f · 4620d2f
1 parent 69731fa
commit 4620d2f
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 46 deletions.
diff --git a/README.md b/README.md
@@ -11,10 +11,10 @@ Helper to manage Solr collections via Solarium
 composer install
 
 # Single node setup:
-docker-compose up -d
+docker-compose up -d --wait
 
 # OR multi-node setup:
-docker-compose -f docker-compose-multinode.yml up -d
+docker-compose -f docker-compose-multinode.yml up -d --wait
 
 # Wait for Solr to start up...
 

diff --git a/docker-compose-multinode.yml b/docker-compose-multinode.yml
@@ -4,7 +4,7 @@
 version: '3.7'
 services:
   solr1:
-    image: solr:8.7.0
+    image: solr:9.4
     container_name: solr1
     ports:
      - "8981:8983"
@@ -16,9 +16,13 @@ services:
       - zoo1
       - zoo2
       - zoo3
+    healthcheck:
+        test: ["CMD", "curl", "-f", "http://localhost:8983/solr/admin/collections?action=CLUSTERSTATUS"]
+        interval: 1s
+        retries: 30
 
   solr2:
-    image: solr:8.7.0
+    image: solr:9.4
     container_name: solr2
     ports:
      - "8982:8983"
@@ -30,9 +34,13 @@ services:
       - zoo1
       - zoo2
       - zoo3
+    healthcheck:
+        test: ["CMD", "curl", "-f", "http://localhost:8983/solr/admin/collections?action=CLUSTERSTATUS"]
+        interval: 1s
+        retries: 30
 
   solr3:
-    image: solr:8.7.0
+    image: solr:9.4
     container_name: solr3
     ports:
      - "8983:8983"
@@ -44,9 +52,13 @@ services:
       - zoo1
       - zoo2
       - zoo3
+    healthcheck:
+        test: ["CMD", "curl", "-f", "http://localhost:8983/solr/admin/collections?action=CLUSTERSTATUS"]
+        interval: 1s
+        retries: 30
 
   zoo1:
-    image: zookeeper:3.6.2
+    image: zookeeper:3.9
     container_name: zoo1
     restart: always
     hostname: zoo1
@@ -60,9 +72,13 @@ services:
       ZOO_CFG_EXTRA: "metricsProvider.className=org.apache.zookeeper.metrics.prometheus.PrometheusMetricsProvider metricsProvider.httpPort=7000 metricsProvider.exportJvmInfo=true"
     networks:
       - solr
+    healthcheck:
+        test: nc -z localhost 2181 || exit -1
+        interval: 1s
+        retries: 30
 
   zoo2:
-    image: zookeeper:3.6.2
+    image: zookeeper:3.9
     container_name: zoo2
     restart: always
     hostname: zoo2
@@ -76,9 +92,13 @@ services:
       ZOO_CFG_EXTRA: "metricsProvider.className=org.apache.zookeeper.metrics.prometheus.PrometheusMetricsProvider metricsProvider.httpPort=7000 metricsProvider.exportJvmInfo=true"
     networks:
       - solr
+    healthcheck:
+        test: nc -z localhost 2181 || exit -1
+        interval: 1s
+        retries: 30
 
   zoo3:
-    image: zookeeper:3.6.2
+    image: zookeeper:3.9
     container_name: zoo3
     restart: always
     hostname: zoo3
@@ -92,6 +112,10 @@ services:
       ZOO_CFG_EXTRA: "metricsProvider.className=org.apache.zookeeper.metrics.prometheus.PrometheusMetricsProvider metricsProvider.httpPort=7000 metricsProvider.exportJvmInfo=true"
     networks:
       - solr
+    healthcheck:
+        test: nc -z localhost 2181 || exit -1
+        interval: 1s
+        retries: 30
 
 networks:
   solr:
diff --git a/docker-compose.yml b/docker-compose.yml
@@ -3,7 +3,7 @@ version: "3.4"
 services:
 
   solr:
-    image: solr:8.4
+    image: solr:9.4
     container_name: solarium-collection-manager-solr
     ports:
       - "8983:8983"
@@ -14,3 +14,5 @@ services:
       - start
       - -cloud
       - -f  # start solr in foreground
+    healthcheck:
+        test: ["CMD", "curl", "-f", "http://localhost:8983/solr/admin/collections?action=CLUSTERSTATUS"]
diff --git a/src/CollectionManager.php b/src/CollectionManager.php
@@ -5,9 +5,7 @@
 use Solarium\Client;
 use Solarium\Core\Client\State\CollectionState;
 use Solarium\Core\Query\Result\ResultInterface;
-use Solarium\QueryType\Server\Collections\Query\Query;
 use Solarium\QueryType\Server\Collections\Result\ClusterStatusResult;
-use Solarium\QueryType\Server\Query\Action\ActionInterface;
 use TypeError;
 
 class CollectionManager implements CollectionManagerInterface
@@ -89,20 +87,20 @@ public function create(string $name, array $options = []): ResultInterface
     {
         $options = array_merge([
             'num_shards' => 1,
-            'max_shards_per_node' => 1,
-            'auto_add_replicas' => 0,
             'router_name' => 'compositeId',
             'nrt_replicas' => 1,  // alias: replication_factor
             'tlog_replicas' => 0,
             'pull_replicas' => 0,
+
+            // NOTE: maxShardsPerNode has been removed in Solr 9.0
+            // @see https://solr.apache.org/guide/solr/latest/upgrade-notes/major-changes-in-solr-9.html
+            // 'max_shards_per_node' => 1,
         ], $options);
 
         $q = $this->client->createCollections();
 
         $action = $q->createCreate()
             ->setNumShards($options['num_shards'])
-            ->setMaxShardsPerNode($options['max_shards_per_node'])
-            ->setAutoAddReplicas($options['auto_add_replicas'])
             ->setRouterName($options['router_name'])
 
             ->setNrtReplicas($options['nrt_replicas'] ?? $options['replication_factor'])

diff --git a/src/CollectionManagerInterface.php b/src/CollectionManagerInterface.php
@@ -30,10 +30,11 @@ public function ensureCollection(string $name): void;
     /**
      * Create collection
      *
-     * @var string $name
+     * @param string $name
+     * @param array<string, mixed> $options
      * @return ResultInterface|ClusterStatusResult
      */
-    public function create(string $name): ResultInterface;
+    public function create(string $name, array $options = []): ResultInterface;
 
     /**
      * Alias collection

diff --git a/tests/Integration/CollectionManagerTest.php b/tests/Integration/CollectionManagerTest.php
@@ -24,24 +24,18 @@ public function it_creates_non_replicated_collection_by_default()
         $this->assertEquals('foo.AUTOCREATED', $fooState->getConfigName());
         $this->assertEquals('compositeId', $fooState->getRouterName());
         $this->assertCount(1, $fooState->getShards());
-        $this->assertSame(1, $fooState->getMaxShardsPerNode());
         $this->assertSame(1, $fooState->getReplicationFactor());
         $this->assertSame('0', $fooState->getTlogReplicas());  // NOTE: Returned as string. Solarium Bug?
-
-        // TODO: setAutoAddReplicas not respected in CollectionManager::create?
-        $this->assertSame(true, $fooState->isAutoAddReplicas());
     }
 
     /** @test */
     public function it_creates_collection_with_options()
     {
         $this->manager->create('foo', [
             'num_shards' => 2,
-            'max_shards_per_node' => 10,  // TODO: Use smaller number that forces multiple nodes to be used? But this would depend on test setup with multiple nodes to work.
             'nrt_replicas' => 2,  // alias: replication_factor
             'pull_replicas' => 1,
             'tlog_replicas' => 1,
-            'auto_add_replicas' => true,
             'router_name' => 'compositeId',
         ]);
 
@@ -55,8 +49,6 @@ public function it_creates_collection_with_options()
         $this->assertEquals('compositeId', $fooState->getRouterName());
         $this->assertCount(2, $fooState->getShards());
         $this->assertCount(2, $fooState->getShardLeaders());
-        $this->assertSame(10, $fooState->getMaxShardsPerNode());
-        $this->assertSame(true, $fooState->isAutoAddReplicas());
         $this->assertSame(2, $fooState->getReplicationFactor());
         $this->assertSame('1', $fooState->getTlogReplicas());
         // TODO: How to get pull replica count?

diff --git a/tests/Unit/CollectionManagerTest.php b/tests/Unit/CollectionManagerTest.php
@@ -18,14 +18,12 @@ public function it_by_default_creates_non_replicated_single_shard_collection_by_
     {
         /** @var Create|MockInterface $createAction */
         $createAction = Mockery::mock(Create::class, function ($m) {
-            $m->shouldReceive('setNumShards')->with(1)->andReturnSelf();
-            $m->shouldReceive('setMaxShardsPerNode')->with(1)->andReturnSelf();
-            $m->shouldReceive('setNrtReplicas')->with(1)->andReturnSelf();
-            $m->shouldReceive('setPullReplicas')->with(0)->andReturnSelf();
-            $m->shouldReceive('setTlogReplicas')->with(0)->andReturnSelf();
-            $m->shouldReceive('setAutoAddReplicas')->with(false)->andReturnSelf();
-            $m->shouldReceive('setRouterName')->with('compositeId')->andReturnSelf();
-            $m->shouldReceive('setName')->with('foo')->andReturnSelf();
+            $m->shouldReceive('setNumShards')->once()->with(1)->andReturnSelf();
+            $m->shouldReceive('setNrtReplicas')->once()->with(1)->andReturnSelf();
+            $m->shouldReceive('setPullReplicas')->once()->with(0)->andReturnSelf();
+            $m->shouldReceive('setTlogReplicas')->once()->with(0)->andReturnSelf();
+            $m->shouldReceive('setRouterName')->once()->with('compositeId')->andReturnSelf();
+            $m->shouldReceive('setName')->once()->with('foo')->andReturnSelf();
         });
 
         $client = ClientMockBuilder::new()
@@ -43,14 +41,12 @@ public function it_accepts_options()
     {
         /** @var Create|MockInterface $createAction */
         $createAction = Mockery::mock(Create::class, function ($m) {
-            $m->shouldReceive('setNumShards')->with(111)->andReturnSelf();
-            $m->shouldReceive('setMaxShardsPerNode')->with(222)->andReturnSelf();
-            $m->shouldReceive('setNrtReplicas')->with(333)->andReturnSelf();
-            $m->shouldReceive('setTlogReplicas')->with(444)->andReturnSelf();
-            $m->shouldReceive('setPullReplicas')->with(555)->andReturnSelf();
-            $m->shouldReceive('setAutoAddReplicas')->with(true)->andReturnSelf();
-            $m->shouldReceive('setRouterName')->with('xxx')->andReturnSelf();
-            $m->shouldReceive('setName')->with('foo')->andReturnSelf();
+            $m->shouldReceive('setNumShards')->once()->with(111)->andReturnSelf();
+            $m->shouldReceive('setNrtReplicas')->once()->with(333)->andReturnSelf();
+            $m->shouldReceive('setTlogReplicas')->once()->with(444)->andReturnSelf();
+            $m->shouldReceive('setPullReplicas')->once()->with(555)->andReturnSelf();
+            $m->shouldReceive('setRouterName')->once()->with('xxx')->andReturnSelf();
+            $m->shouldReceive('setName')->once()->with('foo')->andReturnSelf();
         });
 
         $client = ClientMockBuilder::new()
@@ -60,11 +56,9 @@ public function it_accepts_options()
 
         $data = $manager->create('foo', [
             'num_shards' => 111,
-            'max_shards_per_node' => 222,
             'nrt_replicas' => 333,
             'tlog_replicas' => 444,
             'pull_replicas' => 555,
-            'auto_add_replicas' => true,
             'router_name' => 'xxx',
         ]);
 
@@ -86,9 +80,10 @@ public function __construct()
 
         $this->query = Mockery::mock(Query::class);
 
-        $this->client->shouldReceive('createCollections')->andReturn($this->query);
+        $this->client->shouldReceive('createCollections')->once()->andReturn($this->query);
 
         $this->client->shouldReceive('collections')
+            ->once()
             ->with($this->query)
             ->andReturn(Mockery::mock(
                 ClusterStatusResult::class,
@@ -103,8 +98,8 @@ public static function new(): self
 
     public function expectCreateAction(ActionInterface $action): self
     {
-        $this->query->shouldReceive('createCreate')->andReturn($action);
-        $this->query->shouldReceive('setAction')->with($action);
+        $this->query->shouldReceive('createCreate')->once()->andReturn($action);
+        $this->query->shouldReceive('setAction')->once()->with($action);
 
         return $this;
     }