Fix issue with groups, add factory for testing and add tests

lib/Connectors/BaseConnector.php

@@ -23,18 +23,14 @@
 
 use Elastica\Client;
 use Elastica\Index;
-use Elastica\Mapping;
 use Elastica\Request;
-use Elastica\Search;
-use Elastica\Document;
-use Elastica\Bulk;
-use Elastica\Query;
 use Elastica\Result;
 use Elastica\ResultSet;
 use OC\Files\Cache\Cache;
 use OC\Files\Filesystem;
 use OC\Files\View;
 use OCA\Search_Elastic\SearchElasticConfigService;
+use OCA\Search_Elastic\Connectors\ElasticaFactory;
 use OC\Share\Constants;
 use OCP\Files\Node;
 use OCP\Files\Folder;
@@ -61,6 +57,8 @@
 abstract class BaseConnector implements IConnector {
 	/** @var Client */
 	private $client;
+	/** @var ElasticaFactory */
+	private $factory;
 	/** @var SearchElasticConfigService */
 	private $esConfig;
 	/** @var IGroupManager */
@@ -78,12 +76,14 @@ abstract class BaseConnector implements IConnector {
 	 */
 	public function __construct(
 		Client $client,
+		ElasticaFactory $factory,
 		SearchElasticConfigService $esConfig,
 		IGroupManager $groupManager,
 		IUserManager $userManager,
 		ILogger $logger
 	) {
 		$this->client = $client;
+		$this->factory = $factory;
 		$this->esConfig = $esConfig;
 		$this->groupManager = $groupManager;
 		$this->userManager = $userManager;
@@ -97,7 +97,7 @@ public function __construct(
 	 */
 	final protected function getIndex() {
 		if (!isset($this->index)) {
-			$this->index = new Index($this->client, $this->getIndexName());
+			$this->index = $this->factory->getNewIndex($this->client, $this->getIndexName());
 		}
 		return $this->index;
 	}
@@ -210,7 +210,7 @@ final public function prepareIndex() {
 
 		$mappingData = $this->getMappingPropertiesConf();
 		if (!empty($mappingData)) {
-			$mapping = new Mapping();
+			$mapping = $this->factory->getNewMapping();
 			$mapping->setProperties($mappingData);
 			$mapping->send($index);
 		}
@@ -260,7 +260,7 @@ final public function indexNode(string $userId, Node $node, bool $extractContent
 		$access = $this->getUsersWithReadPermission($node, $userId);
 		$extractedData = $this->extractNodeData($node, $access);
 
-		$doc = new Document((string)$node->getId());
+		$doc = $this->factory->getNewDocument((string)$node->getId());
 		foreach ($extractedData as $key => $value) {
 			$doc->set($key, $value);
 		}
@@ -279,7 +279,7 @@ final public function indexNode(string $userId, Node $node, bool $extractContent
 
 			// this is a workaround to acutally be able to use parameters when setting a document
 			// see: https://github.com/ruflin/Elastica/issues/1248
-			$bulk = new Bulk($index->getClient());
+			$bulk = $this->factory->getNewBulk($index->getClient());
 			$bulk->setIndex($index);
 			$bulk->setRequestParam('pipeline', $this->getProcessorName());
 			$bulk->addDocuments([$doc]);
@@ -423,10 +423,10 @@ final public function fetchResults(string $userId, string $query, int $limit, in
 		$proposedEsQuery = $this->getElasticSearchQuery($query, $opts);
 
 		$this->logger->info('query: ' . json_encode($proposedEsQuery), ['search_elastic']); // TODO: Reduce log level to debug
-		$es_query = new Query();
+		$es_query = $this->factory->getNewQuery();
 		$es_query->setRawQuery($proposedEsQuery);
 
-		$search = new Search($this->client);
+		$search = $this->factory->getNewSearch($this->client);
 		$search->addIndex($this->getIndex());
 		return $search->search($es_query);
 	}

lib/Connectors/ConnectorLegacy.php

@@ -23,6 +23,7 @@
 
 use Elastica\Client;
 use OCA\Search_Elastic\SearchElasticConfigService;
+use OCA\Search_Elastic\Connectors\ElasticaFactory;
 use OCP\Files\Node;
 use OCP\IGroupManager;
 use OCP\IUserManager;
@@ -31,12 +32,13 @@
 class ConnectorLegacy extends BaseConnector {
 	public function __construct(
 		Client $client,
+		ElasticaFactory $factory,
 		SearchElasticConfigService $esConfig,
 		IGroupManager $groupManager,
 		IUserManager $userManager,
 		ILogger $logger
 	) {
-		parent::__construct($client, $esConfig, $groupManager, $userManager, $logger);
+		parent::__construct($client, $factory, $esConfig, $groupManager, $userManager, $logger);
 	}
 
 	/**
@@ -54,8 +56,8 @@ protected function extractNodeData(Node $node, array $access): array {
 	}
 
 	protected function getElasticSearchQuery(string $query, array $opts): array {
-		$users = \implode(' OR ', $opts['access']['users']);
-		$groups = \implode(' OR ', $opts['access']['groups']);
+		$users = $opts['access']['users'] ?? [];
+		$groups = $opts['access']['groups'] ?? [];
 		$size = $opts['size'] ?? 30;
 		$from = $opts['from'] ?? 0;
 
@@ -66,16 +68,8 @@ protected function getElasticSearchQuery(string $query, array $opts): array {
 						[
 							'bool' => [
 								'should' => [
-									[
-										'match' => [
-											'users' => $users,
-										]
-									],
-									[
-										'match' => [
-											'groups' => $groups,
-										],
-									],
+									// assume there will be matches to be included here
+									// coming from users and groups
 								],
 							],
 						],
@@ -101,6 +95,13 @@ protected function getElasticSearchQuery(string $query, array $opts): array {
 			'from' => $from,
 		];
 
+		foreach ($users as $user) {
+			$es_query['query']['bool']['filter'][0]['bool']['should'][] = ['match' => ['users' => $user]];
+		}
+		foreach ($groups as $group) {
+			$es_query['query']['bool']['filter'][0]['bool']['should'][] = ['match' => ['groups' => $group]];
+		}
+
 		if ($opts['searchContent']) {
 			$es_query['query']['bool']['should'][] = [
 				'query_string' => [

lib/Connectors/ConnectorRelevanceV2.php

@@ -24,6 +24,7 @@
 use Elastica\Client;
 use Elastica\Result;
 use OCA\Search_Elastic\SearchElasticConfigService;
+use OCA\Search_Elastic\Connectors\ElasticaFactory;
 use OCP\Files\Node;
 use OCP\Files\FileInfo;
 use OCP\IGroupManager;
@@ -33,12 +34,13 @@
 class ConnectorRelevanceV2 extends BaseConnector {
 	public function __construct(
 		Client $client,
+		ElasticaFactory $factory,
 		SearchElasticConfigService $esConfig,
 		IGroupManager $groupManager,
 		IUserManager $userManager,
 		ILogger $logger
 	) {
-		parent::__construct($client, $esConfig, $groupManager, $userManager, $logger);
+		parent::__construct($client, $factory, $esConfig, $groupManager, $userManager, $logger);
 	}
 
 	protected function getIndexSettingsConf(): array {
@@ -199,8 +201,8 @@ protected function extractNodeData(Node $node, array $access): array {
 	}
 
 	protected function getElasticSearchQuery(string $query, array $opts): array {
-		$users = \implode(' OR ', $opts['access']['users']);
-		$groups = \implode(' OR ', $opts['access']['groups']);
+		$users = $opts['access']['users'] ?? [];
+		$groups = $opts['access']['groups'] ?? [];
 		$size = $opts['size'] ?? 30;
 		$from = $opts['from'] ?? 0;
 
@@ -267,12 +269,12 @@ protected function getElasticSearchQuery(string $query, array $opts): array {
 									'bool' => [
 										'should' => [
 											[
-												'match' => [
+												'terms' => [
 													'users' => $users,
 												]
 											],
 											[
-												'match' => [
+												'terms' => [
 													'groups' => $groups,
 												],
 											],

lib/Connectors/ElasticaFactory.php

@@ -0,0 +1,61 @@
+<?php
+/**
+ * @copyright Copyright (c) 2023, ownCloud GmbH
+ * @license GPL-2.0
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+namespace OCA\Search_Elastic\Connectors;
+
+use Elastica\Client;
+use Elastica\Index;
+use Elastica\Mapping;
+use Elastica\Document;
+use Elastica\Bulk;
+use Elastica\Query;
+use Elastica\Search;
+
+/**
+ * A class dedicated to create Elastica objects. This is intended
+ * to help with the unit tests by injecting a mock of this class
+ * instead of creating the objects directly
+ */
+class ElasticaFactory {
+	public function getNewIndex(Client $client, string $name) {
+		return new Index($client, $name);
+	}
+
+	public function getNewMapping() {
+		return new Mapping();
+	}
+
+	public function getNewDocument(string $id) {
+		return new Document($id);
+	}
+
+	public function getNewBulk(Client $client) {
+		return new Bulk($client);
+	}
+
+	public function getNewQuery() {
+		return new Query();
+	}
+
+	public function getNewSearch(Client $client) {
+		return new Search($client);
+	}
+}