From 91d4437a849eebd640617aecc32bf93f00830816 Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Mon, 28 Jul 2025 16:54:13 +0200 Subject: [PATCH 1/7] fix(search): Fix SearchComposer.php filtering logic keep the $this->providers types Test via ./occ config:app:set --value '["files","settings"]' --type array core unified_search.providers_allowed should be part of 8e570041 Signed-off-by: Misha M.-Kupriyanov --- lib/private/Search/SearchComposer.php | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/private/Search/SearchComposer.php b/lib/private/Search/SearchComposer.php index 77d7ca62311c2..a33b7d53251c1 100644 --- a/lib/private/Search/SearchComposer.php +++ b/lib/private/Search/SearchComposer.php @@ -118,7 +118,7 @@ private function loadLazyProviders(?string $targetProviderId = null): void { } } - $this->providers = $this->filterProviders($this->providers); + $this->filterProviders(); $this->loadFilters(); } @@ -211,19 +211,21 @@ function (array $providerData) use ($route, $routeParameters) { /** * Filter providers based on 'unified_search.providers_allowed' core app config array - * @param array $providers - * @return array + * Will remove providers that are not in the allowed list */ - private function filterProviders(array $providers): array { + private function filterProviders(): void { $allowedProviders = $this->appConfig->getValueArray('core', 'unified_search.providers_allowed'); if (empty($allowedProviders)) { - return $providers; + return; } - return array_values(array_filter($providers, function ($p) use ($allowedProviders) { - return in_array($p['id'], $allowedProviders); - })); + foreach (array_keys($this->providers) as $providerId) { + if (!in_array($providerId, $allowedProviders, true)) { + unset($this->providers[$providerId]); + unset($this->handlers[$providerId]); + } + } } private function fetchIcon(string $appId, string $providerId): string { From 5339e6115a016c36afae82b7ff4fb9637d0d5ee3 Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Tue, 29 Jul 2025 16:31:57 +0200 Subject: [PATCH 2/7] test(SearchComposerTest): add unit tests for SearchComposer Signed-off-by: Misha M.-Kupriyanov --- tests/lib/Search/SearchComposerTest.php | 61 +++++++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 tests/lib/Search/SearchComposerTest.php diff --git a/tests/lib/Search/SearchComposerTest.php b/tests/lib/Search/SearchComposerTest.php new file mode 100644 index 0000000000000..c47e86bad2b1f --- /dev/null +++ b/tests/lib/Search/SearchComposerTest.php @@ -0,0 +1,61 @@ +bootstrapCoordinator = $this->createMock(Coordinator::class); + $this->container = $this->createMock(ContainerInterface::class); + $this->urlGenerator = $this->createMock(IURLGenerator::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->appConfig = $this->createMock(IAppConfig::class); + + $this->searchComposer = new SearchComposer( + $this->bootstrapCoordinator, + $this->container, + $this->urlGenerator, + $this->logger, + $this->appConfig + ); + } + + private function setupEmptyRegistrationContext(): void { + $this->bootstrapCoordinator->expects($this->once()) + ->method('getRegistrationContext') + ->willReturn(null); + } + + public function testGetProvidersWithNoRegisteredProviders(): void { + $this->setupEmptyRegistrationContext(); + + $providers = $this->searchComposer->getProviders('/test/route', []); + + $this->assertIsArray($providers); + $this->assertEmpty($providers); + } +} From 09172cfe690f8b0d4178af21da8126c87055bfae Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Tue, 29 Jul 2025 16:42:34 +0200 Subject: [PATCH 3/7] test(SearchComposerTest): add test for handling unknown search provider Signed-off-by: Misha M.-Kupriyanov --- tests/lib/Search/SearchComposerTest.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/lib/Search/SearchComposerTest.php b/tests/lib/Search/SearchComposerTest.php index c47e86bad2b1f..f97ebc24c7abc 100644 --- a/tests/lib/Search/SearchComposerTest.php +++ b/tests/lib/Search/SearchComposerTest.php @@ -9,10 +9,13 @@ namespace Test\Search; +use InvalidArgumentException; use OC\AppFramework\Bootstrap\Coordinator; use OC\Search\SearchComposer; use OCP\IAppConfig; use OCP\IURLGenerator; +use OCP\IUser; +use OCP\Search\ISearchQuery; use PHPUnit\Framework\MockObject\MockObject; use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; @@ -58,4 +61,16 @@ public function testGetProvidersWithNoRegisteredProviders(): void { $this->assertIsArray($providers); $this->assertEmpty($providers); } + + public function testSearchWithUnknownProvider(): void { + $this->setupEmptyRegistrationContext(); + + $user = $this->createMock(IUser::class); + $query = $this->createMock(ISearchQuery::class); + + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Provider unknown_provider is unknown'); + + $this->searchComposer->search($user, 'unknown_provider', $query); + } } From dae4cfbd21ed47cd5edb0f2595a1614caa5b3d71 Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Tue, 29 Jul 2025 17:23:20 +0200 Subject: [PATCH 4/7] test(SearchComposerTest): add unit test for getProviders with multiple providers Signed-off-by: Misha M.-Kupriyanov --- tests/lib/Search/SearchComposerTest.php | 117 ++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/tests/lib/Search/SearchComposerTest.php b/tests/lib/Search/SearchComposerTest.php index f97ebc24c7abc..38be90c7d8145 100644 --- a/tests/lib/Search/SearchComposerTest.php +++ b/tests/lib/Search/SearchComposerTest.php @@ -11,10 +11,14 @@ use InvalidArgumentException; use OC\AppFramework\Bootstrap\Coordinator; +use OC\AppFramework\Bootstrap\RegistrationContext; +use OC\AppFramework\Bootstrap\ServiceRegistration; use OC\Search\SearchComposer; use OCP\IAppConfig; use OCP\IURLGenerator; use OCP\IUser; +use OCP\Search\IInAppSearch; +use OCP\Search\IProvider; use OCP\Search\ISearchQuery; use PHPUnit\Framework\MockObject\MockObject; use Psr\Container\ContainerInterface; @@ -45,6 +49,15 @@ protected function setUp(): void { $this->logger, $this->appConfig ); + + $this->setupUrlGenerator(); + } + + private function setupUrlGenerator(): void { + $this->urlGenerator->method('imagePath') + ->willReturnCallback(function ($appId, $imageName) { + return "/apps/$appId/img/$imageName"; + }); } private function setupEmptyRegistrationContext(): void { @@ -53,6 +66,55 @@ private function setupEmptyRegistrationContext(): void { ->willReturn(null); } + private function setupAppConfigForAllowedProviders(array $allowedProviders = []): void { + $this->appConfig->method('getValueArray') + ->with('core', 'unified_search.providers_allowed') + ->willReturn($allowedProviders); + } + + /** + * @param array $providerConfigs + * @return array{registrations: ServiceRegistration[], providers: IProvider[]} + */ + private function createMockProvidersAndRegistrations(array $providerConfigs): array { + $registrations = []; + $providers = []; + $containerMap = []; + + foreach ($providerConfigs as $providerId => $config) { + // Create registration mock + $registration = $this->createMock(ServiceRegistration::class); + $registration->method('getService')->willReturn($config['service']); + $registration->method('getAppId')->willReturn($config['appId']); + $registrations[] = $registration; + + // Create provider mock + $providerClass = $config['isInApp'] ?? false ? IInAppSearch::class : IProvider::class; + $provider = $this->createMock($providerClass); + $provider->method('getId')->willReturn($providerId); + $provider->method('getName')->willReturn("Provider $providerId"); + $provider->method('getOrder')->willReturn($config['order']); + + $providers[$providerId] = $provider; + $containerMap[] = [$config['service'], $provider]; + } + + $this->container->expects($this->exactly(count($providerConfigs))) + ->method('get') + ->willReturnMap($containerMap); + + return ['registrations' => $registrations, 'providers' => $providers]; + } + + private function setupRegistrationContextWithProviders(array $registrations): void { + $registrationContext = $this->createMock(RegistrationContext::class); + $registrationContext->method('getSearchProviders')->willReturn($registrations); + + $this->bootstrapCoordinator->expects($this->once()) + ->method('getRegistrationContext') + ->willReturn($registrationContext); + } + public function testGetProvidersWithNoRegisteredProviders(): void { $this->setupEmptyRegistrationContext(); @@ -73,4 +135,59 @@ public function testSearchWithUnknownProvider(): void { $this->searchComposer->search($user, 'unknown_provider', $query); } + + public function testGetProvidersWithMultipleProviders(): void { + $providerConfigs = [ + 'provider1' => ['service' => 'provider1_service', 'appId' => 'app1', 'order' => 10], + 'provider2' => ['service' => 'provider2_service', 'appId' => 'app2', 'order' => 5], + 'provider3' => ['service' => 'provider3_service', 'appId' => 'app3', 'order' => 15, 'isInApp' => true], + ]; + + $mockData = $this->createMockProvidersAndRegistrations($providerConfigs); + $this->setupRegistrationContextWithProviders($mockData['registrations']); + $this->setupAppConfigForAllowedProviders(); + + $providers = $this->searchComposer->getProviders('/test/route', []); + + $this->assertProvidersStructureAndSorting($providers, [ + ['id' => 'provider2', 'name' => 'Provider provider2', 'appId' => 'app2', 'order' => 5, 'inAppSearch' => false], + ['id' => 'provider1', 'name' => 'Provider provider1', 'appId' => 'app1', 'order' => 10, 'inAppSearch' => false], + ['id' => 'provider3', 'name' => 'Provider provider3', 'appId' => 'app3', 'order' => 15, 'inAppSearch' => true], + ]); + } + + /** + * Assert providers array structure and expected sorting + */ + private function assertProvidersStructureAndSorting(array $actualProviders, array $expectedProviders): void { + $this->assertIsArray($actualProviders); + $this->assertCount(count($expectedProviders), $actualProviders); + + foreach ($actualProviders as $index => $provider) { + $this->assertProviderHasRequiredFields($provider); + + $expected = $expectedProviders[$index]; + $this->assertEquals($expected['id'], $provider['id']); + $this->assertEquals($expected['name'], $provider['name']); + $this->assertEquals($expected['appId'], $provider['appId']); + $this->assertEquals($expected['order'], $provider['order']); + $this->assertEquals($expected['inAppSearch'], $provider['inAppSearch']); + } + + $this->assertProvidersAreSortedByOrder($actualProviders); + } + + private function assertProviderHasRequiredFields(array $provider): void { + $requiredFields = ['id', 'appId', 'name', 'icon', 'order', 'triggers', 'filters', 'inAppSearch']; + foreach ($requiredFields as $field) { + $this->assertArrayHasKey($field, $provider, "Provider must have '$field' field"); + } + } + + private function assertProvidersAreSortedByOrder(array $providers): void { + $orders = array_column($providers, 'order'); + $sortedOrders = $orders; + sort($sortedOrders); + $this->assertEquals($sortedOrders, $orders, 'Providers should be sorted by order'); + } } From 291c35cc47ef0d9d7f762fda94e7f30c928b730e Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Wed, 30 Jul 2025 10:06:40 +0200 Subject: [PATCH 5/7] test(SearchComposerTest): add test for provider icon generation Signed-off-by: Misha M.-Kupriyanov --- tests/lib/Search/SearchComposerTest.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/lib/Search/SearchComposerTest.php b/tests/lib/Search/SearchComposerTest.php index 38be90c7d8145..774d121308ca8 100644 --- a/tests/lib/Search/SearchComposerTest.php +++ b/tests/lib/Search/SearchComposerTest.php @@ -156,6 +156,22 @@ public function testGetProvidersWithMultipleProviders(): void { ]); } + public function testProviderIconGeneration(): void { + $providerConfigs = [ + 'provider1' => ['service' => 'provider1_service', 'appId' => 'app1', 'order' => 10], + ]; + + $mockData = $this->createMockProvidersAndRegistrations($providerConfigs); + $this->setupRegistrationContextWithProviders($mockData['registrations']); + $this->setupAppConfigForAllowedProviders(); + + $providers = $this->searchComposer->getProviders('/test/route', []); + + $this->assertCount(1, $providers); + $this->assertArrayHasKey('icon', $providers[0]); + $this->assertStringContainsString('/apps/provider1/img/provider1.svg', $providers[0]['icon']); + } + /** * Assert providers array structure and expected sorting */ From 407f7a1c743f60030ca4ebce2a61e6c4729a0068 Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Tue, 29 Jul 2025 17:29:00 +0200 Subject: [PATCH 6/7] test(SearchComposerTest): add unit tests for getProviders with allowed providers restriction and empty configuration Signed-off-by: Misha M.-Kupriyanov --- tests/lib/Search/SearchComposerTest.php | 59 +++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/lib/Search/SearchComposerTest.php b/tests/lib/Search/SearchComposerTest.php index 774d121308ca8..e53bee0b72810 100644 --- a/tests/lib/Search/SearchComposerTest.php +++ b/tests/lib/Search/SearchComposerTest.php @@ -156,6 +156,65 @@ public function testGetProvidersWithMultipleProviders(): void { ]); } + public function testGetProvidersWithEmptyAllowedProvidersConfiguration(): void { + $providerConfigs = [ + 'provider1' => ['service' => 'provider1_service', 'appId' => 'app1', 'order' => 10], + 'provider2' => ['service' => 'provider2_service', 'appId' => 'app2', 'order' => 5], + ]; + + $mockData = $this->createMockProvidersAndRegistrations($providerConfigs); + $this->setupRegistrationContextWithProviders($mockData['registrations']); + $this->setupAppConfigForAllowedProviders(); + + $providers = $this->searchComposer->getProviders('/test/route', []); + + $this->assertCount(2, $providers); + $this->assertProvidersAreSortedByOrder($providers); + $this->assertEquals('provider2', $providers[0]['id']); + $this->assertEquals('provider1', $providers[1]['id']); + } + + public function testGetProvidersWithAllowedProvidersRestriction(): void { + $providerConfigs = [ + 'provider1' => ['service' => 'provider1_service', 'appId' => 'app1', 'order' => 10], + 'provider2' => ['service' => 'provider2_service', 'appId' => 'app2', 'order' => 5], + 'provider3' => ['service' => 'provider3_service', 'appId' => 'app3', 'order' => 15], + 'provider4' => ['service' => 'provider4_service', 'appId' => 'app4', 'order' => 8], + ]; + + $mockData = $this->createMockProvidersAndRegistrations($providerConfigs); + $this->setupRegistrationContextWithProviders($mockData['registrations']); + $this->setupAppConfigForAllowedProviders(['provider1', 'provider3']); + + $providers = $this->searchComposer->getProviders('/test/route', []); + + $this->assertProvidersStructureAndSorting($providers, [ + ['id' => 'provider1', 'name' => 'Provider provider1', 'appId' => 'app1', 'order' => 10, 'inAppSearch' => false], + ['id' => 'provider3', 'name' => 'Provider provider3', 'appId' => 'app3', 'order' => 15, 'inAppSearch' => false], + ]); + + // Verify excluded providers are not present + $providerIds = array_column($providers, 'id'); + $this->assertNotContains('provider2', $providerIds); + $this->assertNotContains('provider4', $providerIds); + } + + public function testGetProvidersFiltersByAllowedProvidersCompletely(): void { + $providerConfigs = [ + 'provider1' => ['service' => 'provider1_service', 'appId' => 'app1', 'order' => 10], + 'provider2' => ['service' => 'provider2_service', 'appId' => 'app2', 'order' => 5], + ]; + + $mockData = $this->createMockProvidersAndRegistrations($providerConfigs); + $this->setupRegistrationContextWithProviders($mockData['registrations']); + $this->setupAppConfigForAllowedProviders(['provider_not_exists']); + + $providers = $this->searchComposer->getProviders('/test/route', []); + + $this->assertIsArray($providers); + $this->assertEmpty($providers); + } + public function testProviderIconGeneration(): void { $providerConfigs = [ 'provider1' => ['service' => 'provider1_service', 'appId' => 'app1', 'order' => 10], From 815003676eb993f6b5a39ee500ec369269b6ec7f Mon Sep 17 00:00:00 2001 From: "Misha M.-Kupriyanov" Date: Wed, 30 Jul 2025 11:06:04 +0200 Subject: [PATCH 7/7] test(SearchComposerTest): add unit test for getProviders with mixed order values Signed-off-by: Misha M.-Kupriyanov --- tests/lib/Search/SearchComposerTest.php | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/tests/lib/Search/SearchComposerTest.php b/tests/lib/Search/SearchComposerTest.php index e53bee0b72810..f8cf1410fad0f 100644 --- a/tests/lib/Search/SearchComposerTest.php +++ b/tests/lib/Search/SearchComposerTest.php @@ -215,6 +215,28 @@ public function testGetProvidersFiltersByAllowedProvidersCompletely(): void { $this->assertEmpty($providers); } + public function testGetProvidersWithMixedOrderValues(): void { + $providerConfigs = [ + 'provider1' => ['service' => 'provider1_service', 'appId' => 'app1', 'order' => 100], + 'provider2' => ['service' => 'provider2_service', 'appId' => 'app2', 'order' => 1], + 'provider3' => ['service' => 'provider3_service', 'appId' => 'app3', 'order' => 50], + ]; + + $mockData = $this->createMockProvidersAndRegistrations($providerConfigs); + $this->setupRegistrationContextWithProviders($mockData['registrations']); + $this->setupAppConfigForAllowedProviders(); + + $providers = $this->searchComposer->getProviders('/test/route', []); + + $this->assertCount(3, $providers); + $this->assertProvidersAreSortedByOrder($providers); + + // Verify correct ordering: provider2 (1), provider3 (50), provider1 (100) + $this->assertEquals('provider2', $providers[0]['id']); + $this->assertEquals('provider3', $providers[1]['id']); + $this->assertEquals('provider1', $providers[2]['id']); + } + public function testProviderIconGeneration(): void { $providerConfigs = [ 'provider1' => ['service' => 'provider1_service', 'appId' => 'app1', 'order' => 10],