From 4620d2f6f0ae98a9e3ca3ee65f461a13618358d9 Mon Sep 17 00:00:00 2001 From: Tim Sterker Date: Mon, 27 Nov 2023 22:50:32 +0100 Subject: [PATCH] upgrade to Solr 9 and stop supporting removed features; make tests more explicit in regards to expected call counts on mocks --- README.md | 4 +-- docker-compose-multinode.yml | 36 ++++++++++++++++---- docker-compose.yml | 4 ++- src/CollectionManager.php | 10 +++--- src/CollectionManagerInterface.php | 5 +-- tests/Integration/CollectionManagerTest.php | 8 ----- tests/Unit/CollectionManagerTest.php | 37 +++++++++------------ 7 files changed, 58 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index ac91819..399a252 100644 --- 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 index 9d1e1c0..5f49927 100644 --- 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 index 52efd7f..a9d7729 100644 --- 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 index 405b283..68dc7fd 100644 --- 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 index 2f28488..014dcae 100644 --- 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 $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 index 24961ea..4f90b56 100644 --- a/tests/Integration/CollectionManagerTest.php +++ b/tests/Integration/CollectionManagerTest.php @@ -24,12 +24,8 @@ 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 */ @@ -37,11 +33,9 @@ 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 index acf78f9..ec630b6 100644 --- 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; }