From 735bc483ff02c36a5153e49d9dd52f2168b5ca59 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 18:36:28 +0000 Subject: [PATCH 01/20] Initial plan From cc014659c2b6cb0b333c1c9584834c36b581304d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 18:48:29 +0000 Subject: [PATCH 02/20] Add search feature implementation with API, indexing, and tests Co-authored-by: lcharette <2566513+lcharette@users.noreply.github.com> --- app/src/Bakery/BakeCommandListener.php | 3 +- app/src/Bakery/SearchIndexCommand.php | 90 +++++++ app/src/Controller/SearchController.php | 76 ++++++ app/src/MyRoutes.php | 5 + app/src/Recipe.php | 19 +- app/src/Search/SearchIndex.php | 216 ++++++++++++++++ app/src/Search/SearchService.php | 235 ++++++++++++++++++ .../SearchServicesProvider.php | 31 +++ app/tests/Controller/SearchControllerTest.php | 189 ++++++++++++++ app/tests/Search/SearchIndexTest.php | 192 ++++++++++++++ app/tests/Search/SearchServiceTest.php | 182 ++++++++++++++ 11 files changed, 1236 insertions(+), 2 deletions(-) create mode 100644 app/src/Bakery/SearchIndexCommand.php create mode 100644 app/src/Controller/SearchController.php create mode 100644 app/src/Search/SearchIndex.php create mode 100644 app/src/Search/SearchService.php create mode 100644 app/src/ServicesProvider/SearchServicesProvider.php create mode 100644 app/tests/Controller/SearchControllerTest.php create mode 100644 app/tests/Search/SearchIndexTest.php create mode 100644 app/tests/Search/SearchServiceTest.php diff --git a/app/src/Bakery/BakeCommandListener.php b/app/src/Bakery/BakeCommandListener.php index 4e7c51c9..4c9090bc 100644 --- a/app/src/Bakery/BakeCommandListener.php +++ b/app/src/Bakery/BakeCommandListener.php @@ -24,7 +24,8 @@ public function __invoke(BakeCommandEvent $event): void $event->setCommands([ 'debug', 'assets:build', - 'clear-cache' + 'clear-cache', + 'search:index' ]); } } diff --git a/app/src/Bakery/SearchIndexCommand.php b/app/src/Bakery/SearchIndexCommand.php new file mode 100644 index 00000000..4939988c --- /dev/null +++ b/app/src/Bakery/SearchIndexCommand.php @@ -0,0 +1,90 @@ +setName('search:index') + ->setDescription('Build or rebuild the search index for documentation') + ->addOption( + 'version', + null, + InputOption::VALUE_OPTIONAL, + 'Documentation version to index (omit to index all versions)' + ) + ->addOption( + 'clear', + null, + InputOption::VALUE_NONE, + 'Clear the search index before rebuilding' + ); + } + + /** + * {@inheritdoc} + */ + protected function execute(InputInterface $input, OutputInterface $output): int + { + $this->io->title('Documentation Search Index'); + + /** @var string|null $version */ + $version = $input->getOption('version'); + $clear = $input->getOption('clear'); + + // Clear index if requested + if ($clear) { + $this->io->writeln('Clearing search index...'); + $this->searchIndex->clearIndex($version); + $this->io->success('Search index cleared.'); + } + + // Build index + $versionText = $version !== null ? "version {$version}" : 'all versions'; + $this->io->writeln("Building search index for {$versionText}..."); + + try { + $count = $this->searchIndex->buildIndex($version); + $this->io->success("Search index built successfully. Indexed {$count} pages."); + } catch (\Exception $e) { + $this->io->error("Failed to build search index: {$e->getMessage()}"); + + return Command::FAILURE; + } + + return Command::SUCCESS; + } +} diff --git a/app/src/Controller/SearchController.php b/app/src/Controller/SearchController.php new file mode 100644 index 00000000..a2c6559e --- /dev/null +++ b/app/src/Controller/SearchController.php @@ -0,0 +1,76 @@ +getQueryParams(); + + // Get query parameter + $query = $params['q'] ?? ''; + + if (empty($query)) { + $result = [ + 'rows' => [], + 'count' => 0, + 'count_filtered' => 0, + ]; + + $response->getBody()->write(json_encode($result, JSON_THROW_ON_ERROR)); + + return $response->withHeader('Content-Type', 'application/json'); + } + + // Get pagination parameters + $page = isset($params['page']) ? max(1, (int) $params['page']) : 1; + $size = isset($params['size']) ? min(100, max(1, (int) $params['size'])) : 10; + + // Get version parameter + $version = $params['version'] ?? null; + + // Perform search + $result = $this->searchService->search($query, $version, $page, $size); + + // Write JSON response + $response->getBody()->write(json_encode($result, JSON_THROW_ON_ERROR)); + + return $response->withHeader('Content-Type', 'application/json'); + } +} diff --git a/app/src/MyRoutes.php b/app/src/MyRoutes.php index ea707f3c..72da56cf 100644 --- a/app/src/MyRoutes.php +++ b/app/src/MyRoutes.php @@ -12,6 +12,7 @@ use Slim\App; use UserFrosting\Learn\Controller\DocumentationController; +use UserFrosting\Learn\Controller\SearchController; use UserFrosting\Learn\Middleware\TwigGlobals; use UserFrosting\Routes\RouteDefinitionInterface; @@ -19,6 +20,10 @@ class MyRoutes implements RouteDefinitionInterface { public function register(App $app): void { + // Route for search API + $app->get('/api/search', [SearchController::class, 'search']) + ->setName('api.search'); + // Route for versioned and non-versioned images $app->get('/{version:\d+\.\d+}/images/{path:.*}', [DocumentationController::class, 'imageVersioned']) ->add(TwigGlobals::class) diff --git a/app/src/Recipe.php b/app/src/Recipe.php index bf45804c..9a6ce66a 100644 --- a/app/src/Recipe.php +++ b/app/src/Recipe.php @@ -14,10 +14,13 @@ use UserFrosting\Learn\Bakery\BakeCommandListener; use UserFrosting\Learn\Bakery\DebugCommandListener; use UserFrosting\Learn\Bakery\DebugVerboseCommandListener; +use UserFrosting\Learn\Bakery\SearchIndexCommand; use UserFrosting\Learn\Bakery\SetupCommandListener; use UserFrosting\Learn\Listeners\ResourceLocatorInitiated; use UserFrosting\Learn\ServicesProvider\MarkdownService; +use UserFrosting\Learn\ServicesProvider\SearchServicesProvider; use UserFrosting\Learn\Twig\Extensions\FileTreeExtension; +use UserFrosting\Sprinkle\BakeryRecipe; use UserFrosting\Sprinkle\Core\Bakery\Event\BakeCommandEvent; use UserFrosting\Sprinkle\Core\Bakery\Event\DebugCommandEvent; use UserFrosting\Sprinkle\Core\Bakery\Event\DebugVerboseCommandEvent; @@ -35,7 +38,8 @@ class Recipe implements SprinkleRecipe, EventListenerRecipe, - TwigExtensionRecipe + TwigExtensionRecipe, + BakeryRecipe { /** * Return the Sprinkle name. @@ -104,6 +108,19 @@ public function getServices(): array { return [ MarkdownService::class, + SearchServicesProvider::class, + ]; + } + + /** + * Return an array of all registered Bakery Commands. + * + * {@inheritdoc} + */ + public function getBakeryCommands(): array + { + return [ + SearchIndexCommand::class, ]; } diff --git a/app/src/Search/SearchIndex.php b/app/src/Search/SearchIndex.php new file mode 100644 index 00000000..f80c3667 --- /dev/null +++ b/app/src/Search/SearchIndex.php @@ -0,0 +1,216 @@ +config->get('learn.versions.available', []); + foreach (array_keys($available) as $versionId) { + $versions[] = $this->versionValidator->getVersion($versionId); + } + } else { + // Index specific version + $versions[] = $this->versionValidator->getVersion($version); + } + + $totalPages = 0; + + foreach ($versions as $versionObj) { + $pages = $this->indexVersion($versionObj); + $totalPages += count($pages); + + // Store in cache + $this->cache->put( + $this->getCacheKey($versionObj->id), + $pages, + $this->getCacheTtl() + ); + } + + return $totalPages; + } + + /** + * Index all pages for a specific version. + * + * @param Version $version + * + * @return array + */ + protected function indexVersion(Version $version): array + { + $tree = $this->repository->getTree($version->id); + $pages = $this->flattenTree($tree); + + $indexed = []; + + foreach ($pages as $page) { + $indexed[] = $this->indexPage($page); + } + + return $indexed; + } + + /** + * Index a single page. + * + * @param PageResource $page + * + * @return array{title: string, slug: string, route: string, content: string, version: string} + */ + protected function indexPage(PageResource $page): array + { + // Get the HTML content and strip HTML tags to get plain text + $htmlContent = $page->getContent(); + $plainText = $this->stripHtmlTags($htmlContent); + + return [ + 'title' => $page->getTitle(), + 'slug' => $page->getSlug(), + 'route' => $page->getRoute(), + 'content' => $plainText, + 'version' => $page->getVersion()->id, + ]; + } + + /** + * Strip HTML tags from content to get searchable plain text. + * Preserves code blocks and adds spacing for better search results. + * + * @param string $html + * + * @return string + */ + protected function stripHtmlTags(string $html): string + { + // Convert HTML to plain text, preserving code blocks + // Add space before/after block elements to prevent word concatenation + $html = (string) preg_replace('/<(div|p|h[1-6]|li|pre|code|blockquote)[^>]*>/i', ' $0', $html); + $html = (string) preg_replace('/<\/(div|p|h[1-6]|li|pre|code|blockquote)>/i', '$0 ', $html); + + // Remove script and style tags with their content + $html = (string) preg_replace('/<(script|style)[^>]*>.*?<\/\1>/is', '', $html); + + // Strip remaining HTML tags + $text = strip_tags($html); + + // Decode HTML entities + $text = html_entity_decode($text, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + + // Normalize whitespace + $text = (string) preg_replace('/\s+/', ' ', $text); + + return trim($text); + } + + /** + * Flatten a tree structure into a flat array of pages. + * + * @param PageResource[] $tree + * + * @return PageResource[] + */ + protected function flattenTree(array $tree): array + { + $flat = []; + + foreach ($tree as $page) { + $flat[] = $page; + if ($page->getChildren()) { + $flat = array_merge($flat, $this->flattenTree($page->getChildren())); + } + } + + return $flat; + } + + /** + * Get the cache key for the search index of a specific version. + * + * @param string $version + * + * @return string + */ + protected function getCacheKey(string $version): string + { + $keyFormat = $this->config->get('learn.cache.key', '%s.%s'); + + return sprintf($keyFormat, 'search-index', $version); + } + + /** + * Get the cache TTL for the search index. + * + * @return int The cache TTL in seconds + */ + protected function getCacheTtl(): int + { + // Use a longer TTL for search index since it's expensive to rebuild + return $this->config->get('learn.cache.ttl', 3600) * 24; // 24 hours by default + } + + /** + * Clear the search index for a specific version or all versions. + * + * @param string|null $version The version to clear, or null for all versions + */ + public function clearIndex(?string $version = null): void + { + if ($version === null) { + // Clear all versions + $available = $this->config->get('learn.versions.available', []); + foreach (array_keys($available) as $versionId) { + $this->cache->forget($this->getCacheKey($versionId)); + } + } else { + // Clear specific version + $this->cache->forget($this->getCacheKey($version)); + } + } +} diff --git a/app/src/Search/SearchService.php b/app/src/Search/SearchService.php new file mode 100644 index 00000000..450e346a --- /dev/null +++ b/app/src/Search/SearchService.php @@ -0,0 +1,235 @@ +config->get('learn.versions.latest', '6.0'); + + // Get the index from cache + $index = $this->getIndex($versionId); + + if (empty($index)) { + return [ + 'rows' => [], + 'count' => 0, + 'count_filtered' => 0, + ]; + } + + // Search through the index + $results = $this->performSearch($query, $index); + + // Paginate results + $totalResults = count($results); + $offset = ($page - 1) * $perPage; + $paginatedResults = array_slice($results, $offset, $perPage); + + return [ + 'rows' => $paginatedResults, + 'count' => count($index), + 'count_filtered' => $totalResults, + ]; + } + + /** + * Perform the actual search and generate results with snippets. + * + * @param string $query + * @param array $index + * + * @return array + */ + protected function performSearch(string $query, array $index): array + { + $results = []; + $query = trim($query); + + if (empty($query)) { + return $results; + } + + // Determine if query contains wildcards + $hasWildcards = str_contains($query, '*') || str_contains($query, '?'); + + foreach ($index as $page) { + $matches = []; + + if ($hasWildcards) { + // Use wildcard matching + $matches = $this->searchWithWildcard($query, $page['content']); + } else { + // Use simple case-insensitive search + $matches = $this->searchPlain($query, $page['content']); + } + + if (!empty($matches)) { + $results[] = [ + 'title' => $page['title'], + 'slug' => $page['slug'], + 'route' => $page['route'], + 'snippet' => $this->generateSnippet($page['content'], $matches[0]), + 'matches' => count($matches), + 'version' => $page['version'], + ]; + } + } + + // Sort by number of matches (descending) + usort($results, fn ($a, $b) => $b['matches'] <=> $a['matches']); + + return array_slice($results, 0, self::MAX_RESULTS); + } + + /** + * Search for plain text matches (case-insensitive). + * + * @param string $query + * @param string $content + * + * @return array Array of match positions + */ + protected function searchPlain(string $query, string $content): array + { + $matches = []; + $offset = 0; + $queryLower = mb_strtolower($query); + $contentLower = mb_strtolower($content); + + while (($pos = mb_strpos($contentLower, $queryLower, $offset)) !== false) { + $matches[] = $pos; + $offset = $pos + 1; + } + + return $matches; + } + + /** + * Search for wildcard pattern matches. + * + * @param string $pattern Pattern with wildcards (* and ?) + * @param string $content + * + * @return array Array of match positions + */ + protected function searchWithWildcard(string $pattern, string $content): array + { + $matches = []; + + // Convert wildcard pattern to regex + // Escape special regex characters except * and ? + $regex = preg_quote($pattern, '/'); + $regex = str_replace(['\*', '\?'], ['.*', '.'], $regex); + $regex = '/' . $regex . '/i'; // Case-insensitive + + // Split content into words and check each word + $words = preg_split('/\s+/', $content); + $offset = 0; + + if ($words === false) { + return $matches; + } + + foreach ($words as $word) { + if (preg_match($regex, $word)) { + $matches[] = $offset; + } + $offset += mb_strlen($word) + 1; // +1 for space + } + + return $matches; + } + + /** + * Generate a snippet of text around a match position. + * + * @param string $content Full content + * @param int $matchPosition Position of the match + * + * @return string Snippet with context + */ + protected function generateSnippet(string $content, int $matchPosition): string + { + $contextLength = self::SNIPPET_CONTEXT_LENGTH; + + // Calculate start and end positions + $start = max(0, $matchPosition - $contextLength); + $end = min(mb_strlen($content), $matchPosition + $contextLength); + + // Extract snippet + $snippet = mb_substr($content, $start, $end - $start); + + // Add ellipsis if we're not at the beginning/end + if ($start > 0) { + $snippet = '...' . $snippet; + } + if ($end < mb_strlen($content)) { + $snippet .= '...'; + } + + return $snippet; + } + + /** + * Get the search index for a specific version from cache. + * + * @param string $version + * + * @return array + */ + protected function getIndex(string $version): array + { + $keyFormat = $this->config->get('learn.cache.key', '%s.%s'); + $cacheKey = sprintf($keyFormat, 'search-index', $version); + + $index = $this->cache->get($cacheKey); + + return is_array($index) ? $index : []; + } +} diff --git a/app/src/ServicesProvider/SearchServicesProvider.php b/app/src/ServicesProvider/SearchServicesProvider.php new file mode 100644 index 00000000..9a84bab7 --- /dev/null +++ b/app/src/ServicesProvider/SearchServicesProvider.php @@ -0,0 +1,31 @@ + \DI\autowire(), + SearchService::class => \DI\autowire(), + ]; + } +} diff --git a/app/tests/Controller/SearchControllerTest.php b/app/tests/Controller/SearchControllerTest.php new file mode 100644 index 00000000..e4536934 --- /dev/null +++ b/app/tests/Controller/SearchControllerTest.php @@ -0,0 +1,189 @@ +ci->get(Config::class); + $config->set('learn.versions.latest', '6.0'); + $config->set('learn.versions.available', [ + '6.0' => '6.0 Beta', + ]); + + // Use the test pages directory + /** @var ResourceLocatorInterface $locator */ + $locator = $this->ci->get(ResourceLocatorInterface::class); + $locator->removeStream('pages'); + $locator->addStream(new ResourceStream('pages', shared: true, readonly: true, path: __DIR__ . '/../pages')); + + // Build index for testing + $searchIndex = $this->ci->get(SearchIndex::class); + $searchIndex->buildIndex('6.0'); + } + + /** + * Test search API endpoint with query. + */ + public function testSearchEndpoint(): void + { + // Create request to search API + $request = $this->createRequest('GET', '/api/search?q=first'); + $response = $this->handleRequest($request); + + // Assert successful response + $this->assertResponseStatus(200, $response); + + // Parse JSON response + $body = (string) $response->getBody(); + $data = json_decode($body, true); + + $this->assertIsArray($data); + $this->assertArrayHasKey('rows', $data); + $this->assertArrayHasKey('count', $data); + $this->assertArrayHasKey('count_filtered', $data); + + // Should have some results + $this->assertGreaterThan(0, $data['count_filtered']); + $this->assertNotEmpty($data['rows']); + + // Check structure of first result + if (!empty($data['rows'])) { + $firstResult = $data['rows'][0]; + $this->assertArrayHasKey('title', $firstResult); + $this->assertArrayHasKey('slug', $firstResult); + $this->assertArrayHasKey('route', $firstResult); + $this->assertArrayHasKey('snippet', $firstResult); + $this->assertArrayHasKey('matches', $firstResult); + $this->assertArrayHasKey('version', $firstResult); + } + } + + /** + * Test search API endpoint with empty query. + */ + public function testSearchEndpointEmptyQuery(): void + { + // Create request without query + $request = $this->createRequest('GET', '/api/search'); + $response = $this->handleRequest($request); + + // Assert successful response + $this->assertResponseStatus(200, $response); + + // Parse JSON response + $body = (string) $response->getBody(); + $data = json_decode($body, true); + + $this->assertIsArray($data); + $this->assertSame(0, $data['count_filtered']); + $this->assertEmpty($data['rows']); + } + + /** + * Test search API endpoint with pagination. + */ + public function testSearchEndpointPagination(): void + { + // Create request with pagination parameters + $request = $this->createRequest('GET', '/api/search?q=page&page=1&size=2'); + $response = $this->handleRequest($request); + + // Assert successful response + $this->assertResponseStatus(200, $response); + + // Parse JSON response + $body = (string) $response->getBody(); + $data = json_decode($body, true); + + $this->assertIsArray($data); + + // Should return at most 2 results + $this->assertLessThanOrEqual(2, count($data['rows'])); + } + + /** + * Test search API endpoint with version parameter. + */ + public function testSearchEndpointWithVersion(): void + { + // Create request with version parameter + $request = $this->createRequest('GET', '/api/search?q=first&version=6.0'); + $response = $this->handleRequest($request); + + // Assert successful response + $this->assertResponseStatus(200, $response); + + // Parse JSON response + $body = (string) $response->getBody(); + $data = json_decode($body, true); + + $this->assertIsArray($data); + + // Verify results are from correct version + if (!empty($data['rows'])) { + foreach ($data['rows'] as $result) { + $this->assertSame('6.0', $result['version']); + } + } + } + + /** + * Test search API endpoint with wildcard query. + */ + public function testSearchEndpointWildcard(): void + { + // Create request with wildcard query + $request = $this->createRequest('GET', '/api/search?q=f*'); + $response = $this->handleRequest($request); + + // Assert successful response + $this->assertResponseStatus(200, $response); + + // Parse JSON response + $body = (string) $response->getBody(); + $data = json_decode($body, true); + + $this->assertIsArray($data); + $this->assertArrayHasKey('rows', $data); + } + + /** + * Test that response is valid JSON. + */ + public function testSearchEndpointReturnsJson(): void + { + $request = $this->createRequest('GET', '/api/search?q=test'); + $response = $this->handleRequest($request); + + // Check content type header + $this->assertTrue($response->hasHeader('Content-Type')); + $contentType = $response->getHeaderLine('Content-Type'); + $this->assertStringContainsString('application/json', $contentType); + } +} diff --git a/app/tests/Search/SearchIndexTest.php b/app/tests/Search/SearchIndexTest.php new file mode 100644 index 00000000..62fa8175 --- /dev/null +++ b/app/tests/Search/SearchIndexTest.php @@ -0,0 +1,192 @@ +ci->get(Config::class); + $config->set('learn.versions.latest', '6.0'); + $config->set('learn.versions.available', [ + '6.0' => '6.0 Beta', + ]); + + // Use the test pages directory + /** @var ResourceLocatorInterface $locator */ + $locator = $this->ci->get(ResourceLocatorInterface::class); + $locator->removeStream('pages'); + $locator->addStream(new ResourceStream('pages', shared: true, readonly: true, path: __DIR__ . '/../pages')); + } + + public function testBuildIndexForVersion(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + // Build index for version 6.0 + $count = $searchIndex->buildIndex('6.0'); + + // Should have indexed 9 pages (based on test data structure) + $this->assertSame(9, $count); + } + + public function testBuildIndexForAllVersions(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + // Build index for all versions + $count = $searchIndex->buildIndex(null); + + // Should have indexed 9 pages (only 6.0 has test data) + $this->assertSame(9, $count); + } + + public function testIndexPageContent(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + // Build index + $searchIndex->buildIndex('6.0'); + + // Use reflection to access protected method + $reflection = new \ReflectionClass($searchIndex); + $getCacheKeyMethod = $reflection->getMethod('getCacheKey'); + + // Get cache key and retrieve index + $cacheKey = $getCacheKeyMethod->invoke($searchIndex, '6.0'); + + /** @var \Illuminate\Cache\Repository $cache */ + $cache = $this->ci->get(\Illuminate\Cache\Repository::class); + $index = $cache->get($cacheKey); + + $this->assertIsArray($index); + $this->assertNotEmpty($index); + + // Check first page structure + $firstPage = $index[0]; + $this->assertArrayHasKey('title', $firstPage); + $this->assertArrayHasKey('slug', $firstPage); + $this->assertArrayHasKey('route', $firstPage); + $this->assertArrayHasKey('content', $firstPage); + $this->assertArrayHasKey('version', $firstPage); + + // Content should be plain text (no HTML tags) + $this->assertStringNotContainsString('<', $firstPage['content']); + $this->assertStringNotContainsString('>', $firstPage['content']); + } + + public function testStripHtmlTags(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + // Use reflection to test protected method + $reflection = new \ReflectionClass($searchIndex); + $method = $reflection->getMethod('stripHtmlTags'); + + // Test with HTML content + $html = '

Title

This is a test paragraph.

some code
'; + $plain = $method->invoke($searchIndex, $html); + + $this->assertStringNotContainsString('

', $plain); + $this->assertStringNotContainsString('

', $plain); + $this->assertStringNotContainsString('', $plain); + $this->assertStringContainsString('Title', $plain); + $this->assertStringContainsString('test', $plain); + $this->assertStringContainsString('some code', $plain); + } + + public function testClearIndex(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + // Build index + $searchIndex->buildIndex('6.0'); + + // Clear index + $searchIndex->clearIndex('6.0'); + + // Verify cache is cleared + $reflection = new \ReflectionClass($searchIndex); + $getCacheKeyMethod = $reflection->getMethod('getCacheKey'); + $cacheKey = $getCacheKeyMethod->invoke($searchIndex, '6.0'); + + /** @var \Illuminate\Cache\Repository $cache */ + $cache = $this->ci->get(\Illuminate\Cache\Repository::class); + $index = $cache->get($cacheKey); + + $this->assertNull($index); + } + + public function testClearAllIndexes(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + // Build index for all versions + $searchIndex->buildIndex(null); + + // Clear all indexes + $searchIndex->clearIndex(null); + + // Verify cache is cleared + $reflection = new \ReflectionClass($searchIndex); + $getCacheKeyMethod = $reflection->getMethod('getCacheKey'); + $cacheKey = $getCacheKeyMethod->invoke($searchIndex, '6.0'); + + /** @var \Illuminate\Cache\Repository $cache */ + $cache = $this->ci->get(\Illuminate\Cache\Repository::class); + $index = $cache->get($cacheKey); + + $this->assertNull($index); + } + + public function testFlattenTree(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + // Build index to get tree + $searchIndex->buildIndex('6.0'); + + // Use reflection to access the repository and get tree + /** @var \UserFrosting\Learn\Documentation\DocumentationRepository $repository */ + $repository = $this->ci->get(\UserFrosting\Learn\Documentation\DocumentationRepository::class); + $tree = $repository->getTree('6.0'); + + // Use reflection to test flattenTree + $reflection = new \ReflectionClass($searchIndex); + $method = $reflection->getMethod('flattenTree'); + + $flat = $method->invoke($searchIndex, $tree); + + // Should have 9 pages total + $this->assertCount(9, $flat); + + // Verify they're all PageResource objects + foreach ($flat as $page) { + $this->assertInstanceOf(\UserFrosting\Learn\Documentation\PageResource::class, $page); + } + } +} diff --git a/app/tests/Search/SearchServiceTest.php b/app/tests/Search/SearchServiceTest.php new file mode 100644 index 00000000..0386f666 --- /dev/null +++ b/app/tests/Search/SearchServiceTest.php @@ -0,0 +1,182 @@ +ci->get(Config::class); + $config->set('learn.versions.latest', '6.0'); + $config->set('learn.versions.available', [ + '6.0' => '6.0 Beta', + ]); + + // Use the test pages directory + /** @var ResourceLocatorInterface $locator */ + $locator = $this->ci->get(ResourceLocatorInterface::class); + $locator->removeStream('pages'); + $locator->addStream(new ResourceStream('pages', shared: true, readonly: true, path: __DIR__ . '/../pages')); + + // Build index for testing + $searchIndex = $this->ci->get(SearchIndex::class); + $searchIndex->buildIndex('6.0'); + } + + public function testSearchWithPlainText(): void + { + $searchService = $this->ci->get(SearchService::class); + + // Search for "first" - should match "First page" + $result = $searchService->search('first', '6.0'); + + $this->assertIsArray($result); + $this->assertArrayHasKey('rows', $result); + $this->assertArrayHasKey('count', $result); + $this->assertArrayHasKey('count_filtered', $result); + + $this->assertGreaterThan(0, $result['count_filtered']); + $this->assertNotEmpty($result['rows']); + + // Check structure of first result + $firstResult = $result['rows'][0]; + $this->assertArrayHasKey('title', $firstResult); + $this->assertArrayHasKey('slug', $firstResult); + $this->assertArrayHasKey('route', $firstResult); + $this->assertArrayHasKey('snippet', $firstResult); + $this->assertArrayHasKey('matches', $firstResult); + $this->assertArrayHasKey('version', $firstResult); + } + + public function testSearchWithEmptyQuery(): void + { + $searchService = $this->ci->get(SearchService::class); + + $result = $searchService->search('', '6.0'); + + $this->assertSame(0, $result['count_filtered']); + $this->assertEmpty($result['rows']); + } + + public function testSearchWithWildcard(): void + { + $searchService = $this->ci->get(SearchService::class); + + // Search for "f*" - should match words starting with 'f' + $result = $searchService->search('f*', '6.0'); + + $this->assertGreaterThanOrEqual(0, $result['count_filtered']); + } + + public function testSearchPagination(): void + { + $searchService = $this->ci->get(SearchService::class); + + // Search with pagination + $result = $searchService->search('page', '6.0', 1, 2); + + $this->assertLessThanOrEqual(2, count($result['rows'])); + } + + public function testSearchResultSnippet(): void + { + $searchService = $this->ci->get(SearchService::class); + + $result = $searchService->search('first', '6.0'); + + if (!empty($result['rows'])) { + $firstResult = $result['rows'][0]; + $this->assertIsString($firstResult['snippet']); + $this->assertNotEmpty($firstResult['snippet']); + } + } + + public function testSearchPlainMethod(): void + { + $searchService = $this->ci->get(SearchService::class); + + // Use reflection to test protected method + $reflection = new \ReflectionClass($searchService); + $method = $reflection->getMethod('searchPlain'); + + $content = 'This is a test content with multiple test words.'; + $matches = $method->invoke($searchService, 'test', $content); + + $this->assertIsArray($matches); + $this->assertCount(2, $matches); // Should find 2 matches + } + + public function testGenerateSnippet(): void + { + $searchService = $this->ci->get(SearchService::class); + + // Use reflection to test protected method + $reflection = new \ReflectionClass($searchService); + $method = $reflection->getMethod('generateSnippet'); + + $content = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. This is the important part. More text follows here.'; + $matchPosition = strpos($content, 'important'); + + if ($matchPosition !== false) { + $snippet = $method->invoke($searchService, $content, $matchPosition); + + $this->assertIsString($snippet); + $this->assertStringContainsString('important', $snippet); + $this->assertStringContainsString('...', $snippet); // Should have ellipsis + } + } + + public function testSearchWithNoIndex(): void + { + // Clear the index + $searchIndex = $this->ci->get(SearchIndex::class); + $searchIndex->clearIndex('6.0'); + + $searchService = $this->ci->get(SearchService::class); + $result = $searchService->search('test', '6.0'); + + $this->assertSame(0, $result['count_filtered']); + $this->assertEmpty($result['rows']); + } + + public function testSearchResultSorting(): void + { + $searchService = $this->ci->get(SearchService::class); + + // Search for a common term that might appear multiple times + $result = $searchService->search('page', '6.0'); + + if (count($result['rows']) > 1) { + // Verify results are sorted by number of matches (descending) + $firstMatches = $result['rows'][0]['matches']; + $lastMatches = $result['rows'][count($result['rows']) - 1]['matches']; + + $this->assertGreaterThanOrEqual($lastMatches, $firstMatches); + } + } +} From 07b03aedba1a01bf981aa40958eaa90990d28fd3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 18:58:17 +0000 Subject: [PATCH 03/20] Address code review feedback: optimize search performance and improve test robustness Co-authored-by: lcharette <2566513+lcharette@users.noreply.github.com> --- app/src/Search/SearchIndex.php | 17 ++++++++----- app/src/Search/SearchService.php | 25 ++++++++++-------- app/tests/Search/SearchIndexTest.php | 38 +++++++++++++++++++++++----- 3 files changed, 56 insertions(+), 24 deletions(-) diff --git a/app/src/Search/SearchIndex.php b/app/src/Search/SearchIndex.php index f80c3667..66966e78 100644 --- a/app/src/Search/SearchIndex.php +++ b/app/src/Search/SearchIndex.php @@ -129,13 +129,16 @@ protected function indexPage(PageResource $page): array */ protected function stripHtmlTags(string $html): string { - // Convert HTML to plain text, preserving code blocks - // Add space before/after block elements to prevent word concatenation - $html = (string) preg_replace('/<(div|p|h[1-6]|li|pre|code|blockquote)[^>]*>/i', ' $0', $html); - $html = (string) preg_replace('/<\/(div|p|h[1-6]|li|pre|code|blockquote)>/i', '$0 ', $html); - - // Remove script and style tags with their content - $html = (string) preg_replace('/<(script|style)[^>]*>.*?<\/\1>/is', '', $html); + // Combined regex: Add space before/after block elements to prevent word concatenation + $html = (string) preg_replace([ + '/<(div|p|h[1-6]|li|pre|code|blockquote)[^>]*>/i', // Opening tags + '/<\/(div|p|h[1-6]|li|pre|code|blockquote)>/i', // Closing tags + '/<(script|style)[^>]*>.*?<\/\1>/is', // Remove script/style with content + ], [ + ' $0', // Space before opening tags + '$0 ', // Space after closing tags + '', // Remove script/style entirely + ], $html); // Strip remaining HTML tags $text = strip_tags($html); diff --git a/app/src/Search/SearchService.php b/app/src/Search/SearchService.php index 450e346a..920890ab 100644 --- a/app/src/Search/SearchService.php +++ b/app/src/Search/SearchService.php @@ -95,15 +95,23 @@ protected function performSearch(string $query, array $index): array return $results; } - // Determine if query contains wildcards + // Determine if query contains wildcards (check once before loop) $hasWildcards = str_contains($query, '*') || str_contains($query, '?'); + // Pre-compile regex for wildcard searches to avoid recompiling in loop + $wildcardRegex = null; + if ($hasWildcards) { + $pattern = preg_quote($query, '/'); + $pattern = str_replace(['\*', '\?'], ['.*', '.'], $pattern); + $wildcardRegex = '/' . $pattern . '/i'; + } + foreach ($index as $page) { $matches = []; if ($hasWildcards) { - // Use wildcard matching - $matches = $this->searchWithWildcard($query, $page['content']); + // Use wildcard matching with pre-compiled regex + $matches = $this->searchWithWildcard($wildcardRegex, $page['content']); } else { // Use simple case-insensitive search $matches = $this->searchPlain($query, $page['content']); @@ -153,26 +161,21 @@ protected function searchPlain(string $query, string $content): array /** * Search for wildcard pattern matches. * - * @param string $pattern Pattern with wildcards (* and ?) + * @param string $regex Pre-compiled regex pattern * @param string $content * * @return array Array of match positions */ - protected function searchWithWildcard(string $pattern, string $content): array + protected function searchWithWildcard(string $regex, string $content): array { $matches = []; - // Convert wildcard pattern to regex - // Escape special regex characters except * and ? - $regex = preg_quote($pattern, '/'); - $regex = str_replace(['\*', '\?'], ['.*', '.'], $regex); - $regex = '/' . $regex . '/i'; // Case-insensitive - // Split content into words and check each word $words = preg_split('/\s+/', $content); $offset = 0; if ($words === false) { + // Log error if needed in the future, but for now just return empty return $matches; } diff --git a/app/tests/Search/SearchIndexTest.php b/app/tests/Search/SearchIndexTest.php index 62fa8175..4c32dc0b 100644 --- a/app/tests/Search/SearchIndexTest.php +++ b/app/tests/Search/SearchIndexTest.php @@ -50,8 +50,19 @@ public function testBuildIndexForVersion(): void // Build index for version 6.0 $count = $searchIndex->buildIndex('6.0'); - // Should have indexed 9 pages (based on test data structure) - $this->assertSame(9, $count); + // Should have indexed pages (at least some) + $this->assertGreaterThan(0, $count, 'Should have indexed at least one page'); + + // Verify it matches the number of test pages + /** @var \UserFrosting\Learn\Documentation\DocumentationRepository $repository */ + $repository = $this->ci->get(\UserFrosting\Learn\Documentation\DocumentationRepository::class); + + // Use reflection to get pages count + $reflection = new \ReflectionClass($repository); + $method = $reflection->getMethod('getFlattenedTree'); + $flatPages = $method->invoke($repository, '6.0'); + + $this->assertSame(count($flatPages), $count, 'Index count should match actual page count'); } public function testBuildIndexForAllVersions(): void @@ -61,8 +72,8 @@ public function testBuildIndexForAllVersions(): void // Build index for all versions $count = $searchIndex->buildIndex(null); - // Should have indexed 9 pages (only 6.0 has test data) - $this->assertSame(9, $count); + // Should have indexed pages (at least some) + $this->assertGreaterThan(0, $count, 'Should have indexed at least one page'); } public function testIndexPageContent(): void @@ -181,12 +192,27 @@ public function testFlattenTree(): void $flat = $method->invoke($searchIndex, $tree); - // Should have 9 pages total - $this->assertCount(9, $flat); + // Should have multiple pages + $this->assertGreaterThan(0, count($flat), 'Should have at least one page'); // Verify they're all PageResource objects foreach ($flat as $page) { $this->assertInstanceOf(\UserFrosting\Learn\Documentation\PageResource::class, $page); } + + // Verify flat count matches tree structure (all pages including nested) + $countTreePages = function ($pages) use (&$countTreePages) { + $count = 0; + foreach ($pages as $page) { + $count++; + if ($page->getChildren()) { + $count += $countTreePages($page->getChildren()); + } + } + return $count; + }; + + $expectedCount = $countTreePages($tree); + $this->assertSame($expectedCount, count($flat), 'Flattened tree should contain all pages'); } } From 1a1deccd12cac1a507c3332607a58e7ac220beb2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 13 Jan 2026 19:00:48 +0000 Subject: [PATCH 04/20] Fix error handling in preg_replace and cache retrieval Co-authored-by: lcharette <2566513+lcharette@users.noreply.github.com> --- app/src/Search/SearchIndex.php | 18 +++++++++++++++--- app/src/Search/SearchService.php | 7 ++++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/app/src/Search/SearchIndex.php b/app/src/Search/SearchIndex.php index 66966e78..eef4bbfc 100644 --- a/app/src/Search/SearchIndex.php +++ b/app/src/Search/SearchIndex.php @@ -130,7 +130,7 @@ protected function indexPage(PageResource $page): array protected function stripHtmlTags(string $html): string { // Combined regex: Add space before/after block elements to prevent word concatenation - $html = (string) preg_replace([ + $result = preg_replace([ '/<(div|p|h[1-6]|li|pre|code|blockquote)[^>]*>/i', // Opening tags '/<\/(div|p|h[1-6]|li|pre|code|blockquote)>/i', // Closing tags '/<(script|style)[^>]*>.*?<\/\1>/is', // Remove script/style with content @@ -140,14 +140,26 @@ protected function stripHtmlTags(string $html): string '', // Remove script/style entirely ], $html); + // Check if preg_replace failed + if ($result === null) { + // Fallback to original HTML if regex fails + $result = $html; + } + // Strip remaining HTML tags - $text = strip_tags($html); + $text = strip_tags($result); // Decode HTML entities $text = html_entity_decode($text, ENT_QUOTES | ENT_HTML5, 'UTF-8'); // Normalize whitespace - $text = (string) preg_replace('/\s+/', ' ', $text); + $text = preg_replace('/\s+/', ' ', $text); + + // Check if preg_replace failed + if ($text === null) { + // Fallback: at least decode entities from stripped HTML + $text = html_entity_decode(strip_tags($html), ENT_QUOTES | ENT_HTML5, 'UTF-8'); + } return trim($text); } diff --git a/app/src/Search/SearchService.php b/app/src/Search/SearchService.php index 920890ab..b123515d 100644 --- a/app/src/Search/SearchService.php +++ b/app/src/Search/SearchService.php @@ -233,6 +233,11 @@ protected function getIndex(string $version): array $index = $this->cache->get($cacheKey); - return is_array($index) ? $index : []; + // Ensure we return an array even if cache returns null or unexpected type + if (!is_array($index)) { + return []; + } + + return $index; } } From 0e1a628448f14046f46259ab4c4b0e39b7a1cfc6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 18 Jan 2026 00:55:37 +0000 Subject: [PATCH 05/20] Address code review feedback: add config values, validation, metadata search, and result caching Co-authored-by: lcharette <2566513+lcharette@users.noreply.github.com> --- app/config/default.php | 14 +++ app/src/Bakery/SearchIndexCommand.php | 10 ++- app/src/Controller/SearchController.php | 29 ++++-- app/src/Search/SearchIndex.php | 59 ++++++++---- app/src/Search/SearchService.php | 115 ++++++++++++++++++------ 5 files changed, 174 insertions(+), 53 deletions(-) diff --git a/app/config/default.php b/app/config/default.php index 8d0443f1..a0a66bf5 100644 --- a/app/config/default.php +++ b/app/config/default.php @@ -59,6 +59,20 @@ ], 'latest' => '6.0', ], + 'search' => [ + 'min_length' => 3, + 'default_page' => 1, + 'default_size' => 10, + 'max_size' => 100, + 'snippet_length' => 150, + 'max_results' => 1000, + 'results_cache_ttl' => 3600, + 'results_cache_key' => 'learn.search-results.%1$s.%2$s.%3$s.%4$s', + ], + 'index' => [ + 'key' => 'learn.search-index.%1$s', + 'ttl' => 86400 * 7, // 7 days + ], ], /* diff --git a/app/src/Bakery/SearchIndexCommand.php b/app/src/Bakery/SearchIndexCommand.php index 4939988c..da7582fe 100644 --- a/app/src/Bakery/SearchIndexCommand.php +++ b/app/src/Bakery/SearchIndexCommand.php @@ -16,14 +16,16 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Style\SymfonyStyle; use UserFrosting\Learn\Search\SearchIndex; -use UserFrosting\Sprinkle\Core\Bakery\BaseCommand; /** * Bakery command to rebuild the search index for documentation. */ -class SearchIndexCommand extends BaseCommand +class SearchIndexCommand extends Command { + protected SymfonyStyle $io; + /** * @param SearchIndex $searchIndex */ @@ -59,6 +61,8 @@ protected function configure(): void */ protected function execute(InputInterface $input, OutputInterface $output): int { + $this->io = new SymfonyStyle($input, $output); + $this->io->title('Documentation Search Index'); /** @var string|null $version */ @@ -66,7 +70,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int $clear = $input->getOption('clear'); // Clear index if requested - if ($clear) { + if ($clear === true) { $this->io->writeln('Clearing search index...'); $this->searchIndex->clearIndex($version); $this->io->success('Search index cleared.'); diff --git a/app/src/Controller/SearchController.php b/app/src/Controller/SearchController.php index a2c6559e..cda0032d 100644 --- a/app/src/Controller/SearchController.php +++ b/app/src/Controller/SearchController.php @@ -14,6 +14,7 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; +use UserFrosting\Config\Config; use UserFrosting\Learn\Search\SearchService; /** @@ -23,6 +24,7 @@ class SearchController { public function __construct( protected SearchService $searchService, + protected Config $config, ) { } @@ -31,10 +33,10 @@ public function __construct( * Request type: GET. * * Query parameters: - * - q: Search query (required) + * - q: Search query (required, min length from config) * - version: Documentation version to search (optional, defaults to latest) - * - page: Page number for pagination (optional, default: 1) - * - size: Number of results per page (optional, default: 10, max: 100) + * - page: Page number for pagination (optional, from config) + * - size: Number of results per page (optional, from config, max from config) * * @param Request $request * @param Response $response @@ -46,21 +48,32 @@ public function search(Request $request, Response $response): Response // Get query parameter $query = $params['q'] ?? ''; - if (empty($query)) { + // Get minimum length from config + $minLength = $this->config->get('learn.search.min_length', 3); + + // Validate query length + if ($query === '' || mb_strlen($query) < $minLength) { $result = [ 'rows' => [], 'count' => 0, 'count_filtered' => 0, + 'error' => "Query must be at least {$minLength} characters long", ]; $response->getBody()->write(json_encode($result, JSON_THROW_ON_ERROR)); - return $response->withHeader('Content-Type', 'application/json'); + return $response + ->withHeader('Content-Type', 'application/json') + ->withStatus(400); } - // Get pagination parameters - $page = isset($params['page']) ? max(1, (int) $params['page']) : 1; - $size = isset($params['size']) ? min(100, max(1, (int) $params['size'])) : 10; + // Get pagination parameters from config with fallbacks + $defaultPage = $this->config->get('learn.search.default_page', 1); + $defaultSize = $this->config->get('learn.search.default_size', 10); + $maxSize = $this->config->get('learn.search.max_size', 100); + + $page = isset($params['page']) ? max(1, (int) $params['page']) : $defaultPage; + $size = isset($params['size']) ? min($maxSize, max(1, (int) $params['size'])) : $defaultSize; // Get version parameter $version = $params['version'] ?? null; diff --git a/app/src/Search/SearchIndex.php b/app/src/Search/SearchIndex.php index eef4bbfc..f7c036ef 100644 --- a/app/src/Search/SearchIndex.php +++ b/app/src/Search/SearchIndex.php @@ -52,7 +52,7 @@ public function buildIndex(?string $version = null): int // Index all available versions $available = $this->config->get('learn.versions.available', []); foreach (array_keys($available) as $versionId) { - $versions[] = $this->versionValidator->getVersion($versionId); + $versions[] = $this->versionValidator->getVersion((string) $versionId); } } else { // Index specific version @@ -81,7 +81,7 @@ public function buildIndex(?string $version = null): int * * @param Version $version * - * @return array + * @return array */ protected function indexVersion(Version $version): array { @@ -102,20 +102,49 @@ protected function indexVersion(Version $version): array * * @param PageResource $page * - * @return array{title: string, slug: string, route: string, content: string, version: string} + * @return array{title: string, slug: string, route: string, content: string, version: string, keywords: string, metadata: string} */ protected function indexPage(PageResource $page): array { // Get the HTML content and strip HTML tags to get plain text $htmlContent = $page->getContent(); $plainText = $this->stripHtmlTags($htmlContent); + + // Get frontmatter + $frontMatter = $page->getFrontMatter(); + + // Extract keywords if present + $keywords = ''; + if (isset($frontMatter['keywords'])) { + if (is_array($frontMatter['keywords'])) { + $keywords = implode(' ', $frontMatter['keywords']); + } elseif (is_string($frontMatter['keywords'])) { + $keywords = $frontMatter['keywords']; + } + } + + // Extract other relevant metadata (description, tags, etc.) + $metadata = []; + $metadataFields = ['description', 'tags', 'category', 'author']; + foreach ($metadataFields as $field) { + if (isset($frontMatter[$field])) { + if (is_array($frontMatter[$field])) { + $metadata[] = implode(' ', $frontMatter[$field]); + } elseif (is_string($frontMatter[$field])) { + $metadata[] = $frontMatter[$field]; + } + } + } + $metadataString = implode(' ', $metadata); return [ - 'title' => $page->getTitle(), - 'slug' => $page->getSlug(), - 'route' => $page->getRoute(), - 'content' => $plainText, - 'version' => $page->getVersion()->id, + 'title' => $page->getTitle(), + 'slug' => $page->getSlug(), + 'route' => $page->getRoute(), + 'content' => $plainText, + 'version' => $page->getVersion()->id, + 'keywords' => $keywords, + 'metadata' => $metadataString, ]; } @@ -177,8 +206,9 @@ protected function flattenTree(array $tree): array foreach ($tree as $page) { $flat[] = $page; - if ($page->getChildren()) { - $flat = array_merge($flat, $this->flattenTree($page->getChildren())); + $children = $page->getChildren(); + if ($children !== null && count($children) > 0) { + $flat = array_merge($flat, $this->flattenTree($children)); } } @@ -194,9 +224,9 @@ protected function flattenTree(array $tree): array */ protected function getCacheKey(string $version): string { - $keyFormat = $this->config->get('learn.cache.key', '%s.%s'); + $keyFormat = $this->config->get('learn.index.key', 'learn.search-index.%1$s'); - return sprintf($keyFormat, 'search-index', $version); + return sprintf($keyFormat, $version); } /** @@ -206,8 +236,7 @@ protected function getCacheKey(string $version): string */ protected function getCacheTtl(): int { - // Use a longer TTL for search index since it's expensive to rebuild - return $this->config->get('learn.cache.ttl', 3600) * 24; // 24 hours by default + return $this->config->get('learn.index.ttl', 86400 * 7); } /** @@ -221,7 +250,7 @@ public function clearIndex(?string $version = null): void // Clear all versions $available = $this->config->get('learn.versions.available', []); foreach (array_keys($available) as $versionId) { - $this->cache->forget($this->getCacheKey($versionId)); + $this->cache->forget($this->getCacheKey((string) $versionId)); } } else { // Clear specific version diff --git a/app/src/Search/SearchService.php b/app/src/Search/SearchService.php index b123515d..3a490b31 100644 --- a/app/src/Search/SearchService.php +++ b/app/src/Search/SearchService.php @@ -22,15 +22,10 @@ * - Wildcard pattern matching * - Snippet extraction with context * - Pagination support + * - Result caching */ class SearchService { - /** @var int Default number of characters to show in snippet context */ - protected const SNIPPET_CONTEXT_LENGTH = 150; - - /** @var int Maximum number of results to return */ - protected const MAX_RESULTS = 1000; - public function __construct( protected Cache $cache, protected Config $config, @@ -47,15 +42,31 @@ public function __construct( * * @return array{rows: array, count: int, count_filtered: int} */ - public function search(string $query, ?string $version = null, int $page = 1, int $perPage = 10): array + public function search(string $query, ?string $version, int $page = 1, int $perPage = 10): array { // Get the version to search - $versionId = $version ?? $this->config->get('learn.versions.latest', '6.0'); + $versionId = $version ?? $this->config->get('learn.versions.latest'); + + if ($versionId === null) { + return [ + 'rows' => [], + 'count' => 0, + 'count_filtered' => 0, + ]; + } + + // Check cache for this search + $cacheKey = $this->getResultsCacheKey($query, $versionId, $page, $perPage); + $cached = $this->cache->get($cacheKey); + + if (is_array($cached)) { + return $cached; + } // Get the index from cache $index = $this->getIndex($versionId); - if (empty($index)) { + if (count($index) === 0) { return [ 'rows' => [], 'count' => 0, @@ -71,18 +82,24 @@ public function search(string $query, ?string $version = null, int $page = 1, in $offset = ($page - 1) * $perPage; $paginatedResults = array_slice($results, $offset, $perPage); - return [ + $response = [ 'rows' => $paginatedResults, 'count' => count($index), 'count_filtered' => $totalResults, ]; + + // Cache the results + $ttl = $this->config->get('learn.search.results_cache_ttl', 3600); + $this->cache->put($cacheKey, $response, $ttl); + + return $response; } /** * Perform the actual search and generate results with snippets. * - * @param string $query - * @param array $index + * @param string $query + * @param array $index * * @return array */ @@ -91,7 +108,7 @@ protected function performSearch(string $query, array $index): array $results = []; $query = trim($query); - if (empty($query)) { + if ($query === '') { return $results; } @@ -107,32 +124,60 @@ protected function performSearch(string $query, array $index): array } foreach ($index as $page) { - $matches = []; + $titleMatches = []; + $keywordMatches = []; + $metadataMatches = []; + $contentMatches = []; + // Search in different fields with priority if ($hasWildcards) { - // Use wildcard matching with pre-compiled regex - $matches = $this->searchWithWildcard($wildcardRegex, $page['content']); + $titleMatches = $this->searchWithWildcard($wildcardRegex, $page['title']); + $keywordMatches = $this->searchWithWildcard($wildcardRegex, $page['keywords']); + $metadataMatches = $this->searchWithWildcard($wildcardRegex, $page['metadata']); + $contentMatches = $this->searchWithWildcard($wildcardRegex, $page['content']); } else { - // Use simple case-insensitive search - $matches = $this->searchPlain($query, $page['content']); + $titleMatches = $this->searchPlain($query, $page['title']); + $keywordMatches = $this->searchPlain($query, $page['keywords']); + $metadataMatches = $this->searchPlain($query, $page['metadata']); + $contentMatches = $this->searchPlain($query, $page['content']); } - if (!empty($matches)) { + // Calculate weighted score: title > keywords > metadata > content + $score = count($titleMatches) * 10 + count($keywordMatches) * 5 + count($metadataMatches) * 2 + count($contentMatches); + + if ($score > 0) { + // Prefer snippet from title/keywords/metadata if found, otherwise content + $snippetPosition = 0; + if (count($titleMatches) > 0) { + $snippetPosition = $titleMatches[0]; + $snippetContent = $page['title']; + } elseif (count($keywordMatches) > 0) { + $snippetPosition = $keywordMatches[0]; + $snippetContent = $page['keywords']; + } elseif (count($metadataMatches) > 0) { + $snippetPosition = $metadataMatches[0]; + $snippetContent = $page['metadata']; + } else { + $snippetPosition = $contentMatches[0]; + $snippetContent = $page['content']; + } + $results[] = [ 'title' => $page['title'], 'slug' => $page['slug'], 'route' => $page['route'], - 'snippet' => $this->generateSnippet($page['content'], $matches[0]), - 'matches' => count($matches), + 'snippet' => $this->generateSnippet($snippetContent, $snippetPosition), + 'matches' => $score, 'version' => $page['version'], ]; } } - // Sort by number of matches (descending) + // Sort by weighted score (descending) usort($results, fn ($a, $b) => $b['matches'] <=> $a['matches']); - return array_slice($results, 0, self::MAX_RESULTS); + $maxResults = $this->config->get('learn.search.max_results', 1000); + return array_slice($results, 0, $maxResults); } /** @@ -199,7 +244,7 @@ protected function searchWithWildcard(string $regex, string $content): array */ protected function generateSnippet(string $content, int $matchPosition): string { - $contextLength = self::SNIPPET_CONTEXT_LENGTH; + $contextLength = $this->config->get('learn.search.snippet_length', 150); // Calculate start and end positions $start = max(0, $matchPosition - $contextLength); @@ -219,17 +264,33 @@ protected function generateSnippet(string $content, int $matchPosition): string return $snippet; } + /** + * Get the cache key for search results. + * + * @param string $query + * @param string $version + * @param int $page + * @param int $perPage + * + * @return string + */ + protected function getResultsCacheKey(string $query, string $version, int $page, int $perPage): string + { + $keyFormat = $this->config->get('learn.search.results_cache_key', 'learn.search-results.%1$s.%2$s.%3$s.%4$s'); + return sprintf($keyFormat, md5($query), $version, $page, $perPage); + } + /** * Get the search index for a specific version from cache. * * @param string $version * - * @return array + * @return array */ protected function getIndex(string $version): array { - $keyFormat = $this->config->get('learn.cache.key', '%s.%s'); - $cacheKey = sprintf($keyFormat, 'search-index', $version); + $keyFormat = $this->config->get('learn.index.key', 'learn.search-index.%1$s'); + $cacheKey = sprintf($keyFormat, $version); $index = $this->cache->get($cacheKey); From 09445cb623bcf073dd4d8c7d7d48c3ccac9449ae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 18 Jan 2026 00:57:15 +0000 Subject: [PATCH 06/20] Update tests for new index structure and validation behavior Co-authored-by: lcharette <2566513+lcharette@users.noreply.github.com> --- app/tests/Controller/SearchControllerTest.php | 26 +++++++++++++++++-- app/tests/Search/SearchIndexTest.php | 2 ++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/app/tests/Controller/SearchControllerTest.php b/app/tests/Controller/SearchControllerTest.php index e4536934..47138491 100644 --- a/app/tests/Controller/SearchControllerTest.php +++ b/app/tests/Controller/SearchControllerTest.php @@ -93,8 +93,8 @@ public function testSearchEndpointEmptyQuery(): void $request = $this->createRequest('GET', '/api/search'); $response = $this->handleRequest($request); - // Assert successful response - $this->assertResponseStatus(200, $response); + // Should return 400 Bad Request for invalid query + $this->assertResponseStatus(400, $response); // Parse JSON response $body = (string) $response->getBody(); @@ -103,6 +103,28 @@ public function testSearchEndpointEmptyQuery(): void $this->assertIsArray($data); $this->assertSame(0, $data['count_filtered']); $this->assertEmpty($data['rows']); + $this->assertArrayHasKey('error', $data); + } + + /** + * Test search API endpoint with query too short. + */ + public function testSearchEndpointQueryTooShort(): void + { + // Create request with query too short (less than min_length) + $request = $this->createRequest('GET', '/api/search?q=ab'); + $response = $this->handleRequest($request); + + // Should return 400 Bad Request for query too short + $this->assertResponseStatus(400, $response); + + // Parse JSON response + $body = (string) $response->getBody(); + $data = json_decode($body, true); + + $this->assertIsArray($data); + $this->assertArrayHasKey('error', $data); + $this->assertStringContainsString('at least', $data['error']); } /** diff --git a/app/tests/Search/SearchIndexTest.php b/app/tests/Search/SearchIndexTest.php index 4c32dc0b..a7898a30 100644 --- a/app/tests/Search/SearchIndexTest.php +++ b/app/tests/Search/SearchIndexTest.php @@ -104,6 +104,8 @@ public function testIndexPageContent(): void $this->assertArrayHasKey('route', $firstPage); $this->assertArrayHasKey('content', $firstPage); $this->assertArrayHasKey('version', $firstPage); + $this->assertArrayHasKey('keywords', $firstPage); + $this->assertArrayHasKey('metadata', $firstPage); // Content should be plain text (no HTML tags) $this->assertStringNotContainsString('<', $firstPage['content']); From 54b1f48a21a9d03af4cfa9c28b8d5374adb125a1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 18 Jan 2026 01:01:32 +0000 Subject: [PATCH 07/20] Integrate Sprunje for pagination and response formatting Co-authored-by: lcharette <2566513+lcharette@users.noreply.github.com> --- app/src/Controller/SearchController.php | 29 +++--- app/src/Search/SearchSprunje.php | 118 ++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 16 deletions(-) create mode 100644 app/src/Search/SearchSprunje.php diff --git a/app/src/Controller/SearchController.php b/app/src/Controller/SearchController.php index cda0032d..6c24879b 100644 --- a/app/src/Controller/SearchController.php +++ b/app/src/Controller/SearchController.php @@ -16,6 +16,7 @@ use Psr\Http\Message\ServerRequestInterface as Request; use UserFrosting\Config\Config; use UserFrosting\Learn\Search\SearchService; +use UserFrosting\Learn\Search\SearchSprunje; /** * Controller for the documentation search API. @@ -67,23 +68,19 @@ public function search(Request $request, Response $response): Response ->withStatus(400); } - // Get pagination parameters from config with fallbacks - $defaultPage = $this->config->get('learn.search.default_page', 1); - $defaultSize = $this->config->get('learn.search.default_size', 10); - $maxSize = $this->config->get('learn.search.max_size', 100); - - $page = isset($params['page']) ? max(1, (int) $params['page']) : $defaultPage; - $size = isset($params['size']) ? min($maxSize, max(1, (int) $params['size'])) : $defaultSize; + // Prepare options for Sprunje + $sprunjeOptions = [ + 'query' => $query, + 'version' => $params['version'] ?? null, + 'page' => isset($params['page']) ? (int) $params['page'] : null, + 'size' => $params['size'] ?? null, + 'format' => 'json', + ]; - // Get version parameter - $version = $params['version'] ?? null; + // Create and execute Sprunje + $sprunje = new SearchSprunje($this->searchService, $this->config, $sprunjeOptions); - // Perform search - $result = $this->searchService->search($query, $version, $page, $size); - - // Write JSON response - $response->getBody()->write(json_encode($result, JSON_THROW_ON_ERROR)); - - return $response->withHeader('Content-Type', 'application/json'); + // Return response via Sprunje + return $sprunje->toResponse($response); } } diff --git a/app/src/Search/SearchSprunje.php b/app/src/Search/SearchSprunje.php new file mode 100644 index 00000000..83b8cf2e --- /dev/null +++ b/app/src/Search/SearchSprunje.php @@ -0,0 +1,118 @@ +searchQuery = $options['query'] ?? ''; + $this->version = $options['version'] ?? null; + + // Remove search-specific options before parent processes them + unset($options['query'], $options['version']); + + parent::__construct($options); + } + + /** + * Required by Sprunje, but not used since we don't use Eloquent queries. + * Returns a dummy query builder. + * + * @return EloquentBuilderContract|QueryBuilderContract + */ + protected function baseQuery(): EloquentBuilderContract|QueryBuilderContract + { + // This is never actually used since we override getModels + // But we need to return something to satisfy the abstract method + throw new \RuntimeException('baseQuery should not be called on SearchSprunje'); + } + + /** + * Override getModels to use SearchService instead of database queries. + * + * @return array{int, int, Collection} + */ + public function getModels(): array + { + // Get pagination parameters + $page = $this->options['page'] ?? 1; + $size = $this->options['size'] ?? $this->config->get('learn.search.default_size', 10); + + // Handle 'all' size + if ($size === 'all') { + $size = $this->config->get('learn.search.max_results', 1000); + $page = 0; + } else { + $size = (int) $size; + $page = (int) $page; + } + + // Perform search via SearchService + $result = $this->searchService->search($this->searchQuery, $this->version, $page, $size); + + // Convert to Collection for compatibility + $collection = collect($result['rows']); + + return [ + $result['count'], + $result['count_filtered'], + $collection, + ]; + } + + /** + * Override validateOptions to include search-specific validation. + * + * @param mixed[] $options + */ + protected function validateOptions(array $options): void + { + // Don't validate query and version here as they're handled separately + $optionsToValidate = $options; + unset($optionsToValidate['query'], $optionsToValidate['version']); + + parent::validateOptions($optionsToValidate); + } +} From 6869f4262c24e139a660f77eea5ee216903b150b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 18 Jan 2026 01:03:53 +0000 Subject: [PATCH 08/20] Fix Sprunje integration issues: pagination, validation, and error handling Co-authored-by: lcharette <2566513+lcharette@users.noreply.github.com> --- app/src/Controller/SearchController.php | 38 ++++++++++++------------- app/src/Search/SearchSprunje.php | 23 ++++++++++----- 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/app/src/Controller/SearchController.php b/app/src/Controller/SearchController.php index 6c24879b..0a867643 100644 --- a/app/src/Controller/SearchController.php +++ b/app/src/Controller/SearchController.php @@ -49,16 +49,29 @@ public function search(Request $request, Response $response): Response // Get query parameter $query = $params['q'] ?? ''; - // Get minimum length from config - $minLength = $this->config->get('learn.search.min_length', 3); + // Validate query before creating Sprunje (Sprunje will validate length) + try { + // Prepare options for Sprunje + $sprunjeOptions = [ + 'query' => $query, + 'version' => $params['version'] ?? null, + 'page' => isset($params['page']) ? (int) $params['page'] : null, + 'size' => $params['size'] ?? null, + 'format' => 'json', + ]; + + // Create and execute Sprunje (validates query length internally) + $sprunje = new SearchSprunje($this->searchService, $this->config, $sprunjeOptions); - // Validate query length - if ($query === '' || mb_strlen($query) < $minLength) { + // Return response via Sprunje + return $sprunje->toResponse($response); + } catch (\InvalidArgumentException $e) { + // Handle validation errors consistently $result = [ 'rows' => [], 'count' => 0, 'count_filtered' => 0, - 'error' => "Query must be at least {$minLength} characters long", + 'error' => $e->getMessage(), ]; $response->getBody()->write(json_encode($result, JSON_THROW_ON_ERROR)); @@ -67,20 +80,5 @@ public function search(Request $request, Response $response): Response ->withHeader('Content-Type', 'application/json') ->withStatus(400); } - - // Prepare options for Sprunje - $sprunjeOptions = [ - 'query' => $query, - 'version' => $params['version'] ?? null, - 'page' => isset($params['page']) ? (int) $params['page'] : null, - 'size' => $params['size'] ?? null, - 'format' => 'json', - ]; - - // Create and execute Sprunje - $sprunje = new SearchSprunje($this->searchService, $this->config, $sprunjeOptions); - - // Return response via Sprunje - return $sprunje->toResponse($response); } } diff --git a/app/src/Search/SearchSprunje.php b/app/src/Search/SearchSprunje.php index 83b8cf2e..14edd389 100644 --- a/app/src/Search/SearchSprunje.php +++ b/app/src/Search/SearchSprunje.php @@ -50,6 +50,12 @@ public function __construct( $this->searchQuery = $options['query'] ?? ''; $this->version = $options['version'] ?? null; + // Validate query here for consistency + $minLength = $this->config->get('learn.search.min_length', 3); + if ($this->searchQuery === '' || mb_strlen($this->searchQuery) < $minLength) { + throw new \InvalidArgumentException("Query must be at least {$minLength} characters long"); + } + // Remove search-specific options before parent processes them unset($options['query'], $options['version']); @@ -57,16 +63,19 @@ public function __construct( } /** - * Required by Sprunje, but not used since we don't use Eloquent queries. - * Returns a dummy query builder. - * + * Required by Sprunje abstract class, but not used for search functionality. + * + * SearchSprunje uses SearchService instead of Eloquent queries, so this method + * is not called. We override getModels() to bypass the database query system. + * + * @throws \RuntimeException if accidentally called * @return EloquentBuilderContract|QueryBuilderContract */ protected function baseQuery(): EloquentBuilderContract|QueryBuilderContract { - // This is never actually used since we override getModels - // But we need to return something to satisfy the abstract method - throw new \RuntimeException('baseQuery should not be called on SearchSprunje'); + // This should never be called since we override getModels(). + // If it is called, it indicates a problem with our implementation. + throw new \RuntimeException('SearchSprunje does not use database queries. Use getModels() directly.'); } /** @@ -83,7 +92,7 @@ public function getModels(): array // Handle 'all' size if ($size === 'all') { $size = $this->config->get('learn.search.max_results', 1000); - $page = 0; + $page = 1; // Start at page 1, not 0 } else { $size = (int) $size; $page = (int) $page; From c984e86e78bd62ef93843de52c5ef81f59d30993 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 18 Jan 2026 01:05:22 +0000 Subject: [PATCH 09/20] Improve code comments for clarity and accuracy Co-authored-by: lcharette <2566513+lcharette@users.noreply.github.com> --- app/src/Controller/SearchController.php | 4 ++-- app/src/Search/SearchSprunje.php | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/src/Controller/SearchController.php b/app/src/Controller/SearchController.php index 0a867643..38ed5963 100644 --- a/app/src/Controller/SearchController.php +++ b/app/src/Controller/SearchController.php @@ -49,7 +49,7 @@ public function search(Request $request, Response $response): Response // Get query parameter $query = $params['q'] ?? ''; - // Validate query before creating Sprunje (Sprunje will validate length) + // Create Sprunje which validates query length in its constructor try { // Prepare options for Sprunje $sprunjeOptions = [ @@ -60,7 +60,7 @@ public function search(Request $request, Response $response): Response 'format' => 'json', ]; - // Create and execute Sprunje (validates query length internally) + // Create and execute Sprunje (validates query length in constructor) $sprunje = new SearchSprunje($this->searchService, $this->config, $sprunjeOptions); // Return response via Sprunje diff --git a/app/src/Search/SearchSprunje.php b/app/src/Search/SearchSprunje.php index 14edd389..cb487d6e 100644 --- a/app/src/Search/SearchSprunje.php +++ b/app/src/Search/SearchSprunje.php @@ -63,10 +63,10 @@ public function __construct( } /** - * Required by Sprunje abstract class, but not used for search functionality. + * Required by Sprunje abstract class, but should never be called in SearchSprunje. * - * SearchSprunje uses SearchService instead of Eloquent queries, so this method - * is not called. We override getModels() to bypass the database query system. + * SearchSprunje uses SearchService instead of Eloquent queries. We override + * getModels() to use SearchService directly, bypassing the database query system. * * @throws \RuntimeException if accidentally called * @return EloquentBuilderContract|QueryBuilderContract @@ -89,10 +89,10 @@ public function getModels(): array $page = $this->options['page'] ?? 1; $size = $this->options['size'] ?? $this->config->get('learn.search.default_size', 10); - // Handle 'all' size + // Handle 'all' size - return all results from first page if ($size === 'all') { $size = $this->config->get('learn.search.max_results', 1000); - $page = 1; // Start at page 1, not 0 + $page = 1; // Reset to first page when returning all results } else { $size = (int) $size; $page = (int) $page; From 4e8b2e09b264fe59090e99def55b18be2429751b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 18 Jan 2026 02:19:38 +0000 Subject: [PATCH 10/20] Major refactor: fix config structure, Sprunje integration, and address all code review feedback Co-authored-by: lcharette <2566513+lcharette@users.noreply.github.com> --- app/config/default.php | 9 +- app/src/Bakery/SearchIndexCommand.php | 10 +-- app/src/Controller/SearchController.php | 16 +--- app/src/Search/SearchIndex.php | 6 +- app/src/Search/SearchService.php | 107 ++++-------------------- app/src/Search/SearchSprunje.php | 57 +++++++------ 6 files changed, 60 insertions(+), 145 deletions(-) diff --git a/app/config/default.php b/app/config/default.php index a0a66bf5..f6035ff6 100644 --- a/app/config/default.php +++ b/app/config/default.php @@ -68,10 +68,11 @@ 'max_results' => 1000, 'results_cache_ttl' => 3600, 'results_cache_key' => 'learn.search-results.%1$s.%2$s.%3$s.%4$s', - ], - 'index' => [ - 'key' => 'learn.search-index.%1$s', - 'ttl' => 86400 * 7, // 7 days + 'index' => [ + 'key' => 'learn.search-index.%1$s', + 'ttl' => 86400 * 7, // 7 days + 'metadata_fields' => ['description', 'tags', 'category', 'author'], + ], ], ], diff --git a/app/src/Bakery/SearchIndexCommand.php b/app/src/Bakery/SearchIndexCommand.php index da7582fe..35676bec 100644 --- a/app/src/Bakery/SearchIndexCommand.php +++ b/app/src/Bakery/SearchIndexCommand.php @@ -16,7 +16,7 @@ use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; -use Symfony\Component\Console\Style\SymfonyStyle; +use UserFrosting\Bakery\WithSymfonyStyle; use UserFrosting\Learn\Search\SearchIndex; /** @@ -24,7 +24,7 @@ */ class SearchIndexCommand extends Command { - protected SymfonyStyle $io; + use WithSymfonyStyle; /** * @param SearchIndex $searchIndex @@ -43,7 +43,7 @@ protected function configure(): void $this->setName('search:index') ->setDescription('Build or rebuild the search index for documentation') ->addOption( - 'version', + 'doc-version', null, InputOption::VALUE_OPTIONAL, 'Documentation version to index (omit to index all versions)' @@ -61,12 +61,10 @@ protected function configure(): void */ protected function execute(InputInterface $input, OutputInterface $output): int { - $this->io = new SymfonyStyle($input, $output); - $this->io->title('Documentation Search Index'); /** @var string|null $version */ - $version = $input->getOption('version'); + $version = $input->getOption('doc-version'); $clear = $input->getOption('clear'); // Clear index if requested diff --git a/app/src/Controller/SearchController.php b/app/src/Controller/SearchController.php index 38ed5963..54c9006f 100644 --- a/app/src/Controller/SearchController.php +++ b/app/src/Controller/SearchController.php @@ -17,6 +17,7 @@ use UserFrosting\Config\Config; use UserFrosting\Learn\Search\SearchService; use UserFrosting\Learn\Search\SearchSprunje; +use UserFrosting\Sprinkle\Core\Exceptions\NotFoundException; /** * Controller for the documentation search API. @@ -66,19 +67,8 @@ public function search(Request $request, Response $response): Response // Return response via Sprunje return $sprunje->toResponse($response); } catch (\InvalidArgumentException $e) { - // Handle validation errors consistently - $result = [ - 'rows' => [], - 'count' => 0, - 'count_filtered' => 0, - 'error' => $e->getMessage(), - ]; - - $response->getBody()->write(json_encode($result, JSON_THROW_ON_ERROR)); - - return $response - ->withHeader('Content-Type', 'application/json') - ->withStatus(400); + // Throw NotFoundException for empty/invalid queries + throw new NotFoundException($e->getMessage()); } } } diff --git a/app/src/Search/SearchIndex.php b/app/src/Search/SearchIndex.php index f7c036ef..e61cd341 100644 --- a/app/src/Search/SearchIndex.php +++ b/app/src/Search/SearchIndex.php @@ -125,7 +125,7 @@ protected function indexPage(PageResource $page): array // Extract other relevant metadata (description, tags, etc.) $metadata = []; - $metadataFields = ['description', 'tags', 'category', 'author']; + $metadataFields = $this->config->get('learn.search.index.metadata_fields', ['description', 'tags', 'category', 'author']); foreach ($metadataFields as $field) { if (isset($frontMatter[$field])) { if (is_array($frontMatter[$field])) { @@ -224,7 +224,7 @@ protected function flattenTree(array $tree): array */ protected function getCacheKey(string $version): string { - $keyFormat = $this->config->get('learn.index.key', 'learn.search-index.%1$s'); + $keyFormat = $this->config->get('learn.search.index.key', 'learn.search-index.%1$s'); return sprintf($keyFormat, $version); } @@ -236,7 +236,7 @@ protected function getCacheKey(string $version): string */ protected function getCacheTtl(): int { - return $this->config->get('learn.index.ttl', 86400 * 7); + return $this->config->get('learn.search.index.ttl', 86400 * 7); } /** diff --git a/app/src/Search/SearchService.php b/app/src/Search/SearchService.php index 3a490b31..3fb7f9a0 100644 --- a/app/src/Search/SearchService.php +++ b/app/src/Search/SearchService.php @@ -21,7 +21,6 @@ * Performs searches against the indexed documentation content with: * - Wildcard pattern matching * - Snippet extraction with context - * - Pagination support * - Result caching */ class SearchService @@ -33,77 +32,38 @@ public function __construct( } /** - * Search for a query in the documentation for a specific version. + * Get the search index for a specific version from cache. + * Public method for use by SearchSprunje. * - * @param string $query The search query (supports wildcards: * and ?) - * @param string|null $version The version to search in, or null for latest - * @param int $page The page number (1-indexed) - * @param int $perPage Number of results per page + * @param string $version * - * @return array{rows: array, count: int, count_filtered: int} + * @return array */ - public function search(string $query, ?string $version, int $page = 1, int $perPage = 10): array + public function getIndex(string $version): array { - // Get the version to search - $versionId = $version ?? $this->config->get('learn.versions.latest'); - - if ($versionId === null) { - return [ - 'rows' => [], - 'count' => 0, - 'count_filtered' => 0, - ]; - } - - // Check cache for this search - $cacheKey = $this->getResultsCacheKey($query, $versionId, $page, $perPage); - $cached = $this->cache->get($cacheKey); - - if (is_array($cached)) { - return $cached; - } + $keyFormat = $this->config->get('learn.search.index.key', 'learn.search-index.%1$s'); + $cacheKey = sprintf($keyFormat, $version); - // Get the index from cache - $index = $this->getIndex($versionId); + $index = $this->cache->get($cacheKey); - if (count($index) === 0) { - return [ - 'rows' => [], - 'count' => 0, - 'count_filtered' => 0, - ]; + // Ensure we return an array even if cache returns null or unexpected type + if (!is_array($index)) { + return []; } - // Search through the index - $results = $this->performSearch($query, $index); - - // Paginate results - $totalResults = count($results); - $offset = ($page - 1) * $perPage; - $paginatedResults = array_slice($results, $offset, $perPage); - - $response = [ - 'rows' => $paginatedResults, - 'count' => count($index), - 'count_filtered' => $totalResults, - ]; - - // Cache the results - $ttl = $this->config->get('learn.search.results_cache_ttl', 3600); - $this->cache->put($cacheKey, $response, $ttl); - - return $response; + return $index; } /** * Perform the actual search and generate results with snippets. + * Public method for use by SearchSprunje. * * @param string $query * @param array $index * * @return array */ - protected function performSearch(string $query, array $index): array + public function performSearch(string $query, array $index): array { $results = []; $query = trim($query); @@ -263,42 +223,5 @@ protected function generateSnippet(string $content, int $matchPosition): string return $snippet; } - - /** - * Get the cache key for search results. - * - * @param string $query - * @param string $version - * @param int $page - * @param int $perPage - * - * @return string - */ - protected function getResultsCacheKey(string $query, string $version, int $page, int $perPage): string - { - $keyFormat = $this->config->get('learn.search.results_cache_key', 'learn.search-results.%1$s.%2$s.%3$s.%4$s'); - return sprintf($keyFormat, md5($query), $version, $page, $perPage); - } - - /** - * Get the search index for a specific version from cache. - * - * @param string $version - * - * @return array - */ - protected function getIndex(string $version): array - { - $keyFormat = $this->config->get('learn.index.key', 'learn.search-index.%1$s'); - $cacheKey = sprintf($keyFormat, $version); - - $index = $this->cache->get($cacheKey); - - // Ensure we return an array even if cache returns null or unexpected type - if (!is_array($index)) { - return []; - } - - return $index; - } } + diff --git a/app/src/Search/SearchSprunje.php b/app/src/Search/SearchSprunje.php index cb487d6e..d38bc45c 100644 --- a/app/src/Search/SearchSprunje.php +++ b/app/src/Search/SearchSprunje.php @@ -12,8 +12,7 @@ namespace UserFrosting\Learn\Search; -use Illuminate\Contracts\Database\Eloquent\Builder as EloquentBuilderContract; -use Illuminate\Contracts\Database\Query\Builder as QueryBuilderContract; +use Illuminate\Database\Eloquent\Builder as EloquentBuilder; use Illuminate\Support\Collection; use UserFrosting\Config\Config; use UserFrosting\Sprinkle\Core\Sprunje\Sprunje; @@ -59,23 +58,26 @@ public function __construct( // Remove search-specific options before parent processes them unset($options['query'], $options['version']); + // Call parent constructor which will initialize the query via baseQuery() parent::__construct($options); } /** - * Required by Sprunje abstract class, but should never be called in SearchSprunje. + * Required by Sprunje abstract class. Returns a dummy Eloquent builder. * - * SearchSprunje uses SearchService instead of Eloquent queries. We override - * getModels() to use SearchService directly, bypassing the database query system. - * - * @throws \RuntimeException if accidentally called - * @return EloquentBuilderContract|QueryBuilderContract + * SearchSprunje doesn't use database queries - we override getModels() + * to use SearchService directly. This builder is never actually used for queries. + * + * @return EloquentBuilder */ - protected function baseQuery(): EloquentBuilderContract|QueryBuilderContract + protected function baseQuery(): EloquentBuilder { - // This should never be called since we override getModels(). - // If it is called, it indicates a problem with our implementation. - throw new \RuntimeException('SearchSprunje does not use database queries. Use getModels() directly.'); + // Return a dummy Eloquent builder that won't be used + // We use a simple Eloquent model just to satisfy the type requirement + $model = new class extends \Illuminate\Database\Eloquent\Model { + protected $table = 'dummy'; + }; + return $model::query(); } /** @@ -85,28 +87,29 @@ protected function baseQuery(): EloquentBuilderContract|QueryBuilderContract */ public function getModels(): array { - // Get pagination parameters - $page = $this->options['page'] ?? 1; - $size = $this->options['size'] ?? $this->config->get('learn.search.default_size', 10); + // Get the version to search + $versionId = $this->version ?? $this->config->get('learn.versions.latest'); - // Handle 'all' size - return all results from first page - if ($size === 'all') { - $size = $this->config->get('learn.search.max_results', 1000); - $page = 1; // Reset to first page when returning all results - } else { - $size = (int) $size; - $page = (int) $page; + if ($versionId === null) { + return [0, 0, collect([])]; + } + + // Get the index from cache + $index = $this->searchService->getIndex($versionId); + + if (count($index) === 0) { + return [0, 0, collect([])]; } - // Perform search via SearchService - $result = $this->searchService->search($this->searchQuery, $this->version, $page, $size); + // Search through the index (without pagination - Sprunje handles that) + $results = $this->searchService->performSearch($this->searchQuery, $index); // Convert to Collection for compatibility - $collection = collect($result['rows']); + $collection = collect($results); return [ - $result['count'], - $result['count_filtered'], + count($index), + count($results), $collection, ]; } From 56f2180affe9acdedb243b190c5c65ab7892a168 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 18 Jan 2026 02:20:42 +0000 Subject: [PATCH 11/20] Update tests for NotFoundException error handling Co-authored-by: lcharette <2566513+lcharette@users.noreply.github.com> --- app/tests/Controller/SearchControllerTest.php | 25 +++---------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/app/tests/Controller/SearchControllerTest.php b/app/tests/Controller/SearchControllerTest.php index 47138491..bae6a24e 100644 --- a/app/tests/Controller/SearchControllerTest.php +++ b/app/tests/Controller/SearchControllerTest.php @@ -93,17 +93,8 @@ public function testSearchEndpointEmptyQuery(): void $request = $this->createRequest('GET', '/api/search'); $response = $this->handleRequest($request); - // Should return 400 Bad Request for invalid query - $this->assertResponseStatus(400, $response); - - // Parse JSON response - $body = (string) $response->getBody(); - $data = json_decode($body, true); - - $this->assertIsArray($data); - $this->assertSame(0, $data['count_filtered']); - $this->assertEmpty($data['rows']); - $this->assertArrayHasKey('error', $data); + // Should return 404 Not Found for invalid query (NotFoundException) + $this->assertResponseStatus(404, $response); } /** @@ -115,16 +106,8 @@ public function testSearchEndpointQueryTooShort(): void $request = $this->createRequest('GET', '/api/search?q=ab'); $response = $this->handleRequest($request); - // Should return 400 Bad Request for query too short - $this->assertResponseStatus(400, $response); - - // Parse JSON response - $body = (string) $response->getBody(); - $data = json_decode($body, true); - - $this->assertIsArray($data); - $this->assertArrayHasKey('error', $data); - $this->assertStringContainsString('at least', $data['error']); + // Should return 404 Not Found for query too short (NotFoundException) + $this->assertResponseStatus(404, $response); } /** From 60597370f0c073afc9729c064b8148616f204b7d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 18 Jan 2026 02:22:22 +0000 Subject: [PATCH 12/20] Improve SearchSprunje: use named DummySearchModel class and fix comments Co-authored-by: lcharette <2566513+lcharette@users.noreply.github.com> --- app/src/Search/DummySearchModel.php | 32 +++++++++++++++++++++++++++++ app/src/Search/SearchSprunje.php | 14 ++++++------- 2 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 app/src/Search/DummySearchModel.php diff --git a/app/src/Search/DummySearchModel.php b/app/src/Search/DummySearchModel.php new file mode 100644 index 00000000..25a02244 --- /dev/null +++ b/app/src/Search/DummySearchModel.php @@ -0,0 +1,32 @@ +newQuery(); } /** From f0b6e573bf4925de240800f7dbe20f29b85983c1 Mon Sep 17 00:00:00 2001 From: Louis Charette Date: Thu, 22 Jan 2026 22:15:45 -0500 Subject: [PATCH 13/20] Remove slop, apply better sprunje --- app/config/default.php | 59 +++-- app/src/Controller/SearchController.php | 31 +-- app/src/Search/DummySearchModel.php | 32 --- app/src/Search/IndexedPageShape.php | 45 ++++ app/src/Search/SearchIndex.php | 41 +++- app/src/Search/SearchService.php | 42 +--- app/src/Search/SearchSprunje.php | 106 +++------ app/src/Search/StaticSprunje.php | 222 ++++++++++++++++++ app/tests/Bakery/SearchIndexCommandTest.php | 184 +++++++++++++++ app/tests/Controller/SearchControllerTest.php | 21 +- app/tests/Search/SearchIndexTest.php | 11 +- app/tests/Search/SearchServiceTest.php | 76 +++--- composer.lock | 188 +++++++-------- 13 files changed, 724 insertions(+), 334 deletions(-) delete mode 100644 app/src/Search/DummySearchModel.php create mode 100644 app/src/Search/IndexedPageShape.php create mode 100644 app/src/Search/StaticSprunje.php create mode 100644 app/tests/Bakery/SearchIndexCommandTest.php diff --git a/app/config/default.php b/app/config/default.php index f6035ff6..a6080bd0 100644 --- a/app/config/default.php +++ b/app/config/default.php @@ -3,11 +3,11 @@ declare(strict_types=1); /* - * UserFrosting (http://www.userfrosting.com) + * UserFrosting Learn (http://www.userfrosting.com) * - * @link https://github.com/userfrosting/UserFrosting - * @copyright Copyright (c) 2013-2024 Alexander Weissman & Louis Charette - * @license https://github.com/userfrosting/UserFrosting/blob/master/LICENSE.md (MIT License) + * @link https://github.com/userfrosting/Learn + * @copyright Copyright (c) 2025 Alexander Weissman & Louis Charette + * @license https://github.com/userfrosting/Learn/blob/main/LICENSE.md (MIT License) */ /* @@ -26,23 +26,18 @@ ], ], - /** - * Disable cache - */ - 'cache' => [ - 'driver' => 'array', - ], + // TODO : Disable page cache by default in dev mode, but keep search cache enabled. /** - * ---------------------------------------------------------------------- - * Learn Settings - * - * Settings for the documentation application. - * - Cache : Enable/disable caching of documentation pages and menu. - * - Key : Cache key prefix for cached documentation pages and menu. - * - TTL : Time to live for cached documentation pages and menu, in seconds. - * ---------------------------------------------------------------------- - */ + * ---------------------------------------------------------------------- + * Learn Settings + * + * Settings for the documentation application. + * - Cache : Enable/disable caching of documentation pages and menu. + * - Key : Cache key prefix for cached documentation pages and menu. + * - TTL : Time to live for cached documentation pages and menu, in seconds. + * ---------------------------------------------------------------------- + */ 'learn' => [ 'cache' => [ 'key' => 'learn.%1$s.%2$s', @@ -60,17 +55,21 @@ 'latest' => '6.0', ], 'search' => [ - 'min_length' => 3, - 'default_page' => 1, - 'default_size' => 10, - 'max_size' => 100, - 'snippet_length' => 150, - 'max_results' => 1000, - 'results_cache_ttl' => 3600, - 'results_cache_key' => 'learn.search-results.%1$s.%2$s.%3$s.%4$s', - 'index' => [ - 'key' => 'learn.search-index.%1$s', - 'ttl' => 86400 * 7, // 7 days + 'min_length' => 3, // Minimum length of search query + 'default_page' => 1, // Default page number for paginated results + 'default_size' => 10, // Default number of results per page + 'max_size' => 10, // Default maximum number of results per page + 'snippet_length' => 150, // Length of content snippets in results + 'max_results' => 150, // Maximum number of results to consider for pagination + 'cache' => [ + 'key' => 'learn.search.%1$s', // %1$s = keyword hash + 'ttl' => 86400 * 30, // 30 days + ], + 'index' => [ + 'key' => 'learn.index.%1$s', // %1$s = version + 'ttl' => 86400 * 30, // 30 days + + // Metadata fields to include in the search index 'metadata_fields' => ['description', 'tags', 'category', 'author'], ], ], diff --git a/app/src/Controller/SearchController.php b/app/src/Controller/SearchController.php index 54c9006f..535b95ac 100644 --- a/app/src/Controller/SearchController.php +++ b/app/src/Controller/SearchController.php @@ -17,7 +17,6 @@ use UserFrosting\Config\Config; use UserFrosting\Learn\Search\SearchService; use UserFrosting\Learn\Search\SearchSprunje; -use UserFrosting\Sprinkle\Core\Exceptions\NotFoundException; /** * Controller for the documentation search API. @@ -27,6 +26,7 @@ class SearchController public function __construct( protected SearchService $searchService, protected Config $config, + protected SearchSprunje $sprunje, ) { } @@ -36,7 +36,6 @@ public function __construct( * * Query parameters: * - q: Search query (required, min length from config) - * - version: Documentation version to search (optional, defaults to latest) * - page: Page number for pagination (optional, from config) * - size: Number of results per page (optional, from config, max from config) * @@ -47,28 +46,12 @@ public function search(Request $request, Response $response): Response { $params = $request->getQueryParams(); - // Get query parameter - $query = $params['q'] ?? ''; + $this->sprunje->setOptions([ + 'query' => $params['q'] ?? '', + 'page' => $params['page'] ?? null, + 'size' => $params['size'] ?? null, + ]); - // Create Sprunje which validates query length in its constructor - try { - // Prepare options for Sprunje - $sprunjeOptions = [ - 'query' => $query, - 'version' => $params['version'] ?? null, - 'page' => isset($params['page']) ? (int) $params['page'] : null, - 'size' => $params['size'] ?? null, - 'format' => 'json', - ]; - - // Create and execute Sprunje (validates query length in constructor) - $sprunje = new SearchSprunje($this->searchService, $this->config, $sprunjeOptions); - - // Return response via Sprunje - return $sprunje->toResponse($response); - } catch (\InvalidArgumentException $e) { - // Throw NotFoundException for empty/invalid queries - throw new NotFoundException($e->getMessage()); - } + return $this->sprunje->toResponse($response); } } diff --git a/app/src/Search/DummySearchModel.php b/app/src/Search/DummySearchModel.php deleted file mode 100644 index 25a02244..00000000 --- a/app/src/Search/DummySearchModel.php +++ /dev/null @@ -1,32 +0,0 @@ - + */ + public function getIndex(string $version): array + { + $keyFormat = $this->config->getString('learn.search.index.key', ''); + $cacheKey = sprintf($keyFormat, $version); + + // TODO : If the cache key is empty, it should build the index first + $index = $this->cache->get($cacheKey); + + // Ensure we return an array even if cache returns null or unexpected type + if (!is_array($index)) { + return []; + } + + return $index; + } + /** * Index all pages for a specific version. * * @param Version $version * - * @return array + * @return list */ protected function indexVersion(Version $version): array { $tree = $this->repository->getTree($version->id); $pages = $this->flattenTree($tree); + /** @var list */ $indexed = []; foreach ($pages as $page) { @@ -102,17 +129,17 @@ protected function indexVersion(Version $version): array * * @param PageResource $page * - * @return array{title: string, slug: string, route: string, content: string, version: string, keywords: string, metadata: string} + * @return IndexedPage */ protected function indexPage(PageResource $page): array { // Get the HTML content and strip HTML tags to get plain text $htmlContent = $page->getContent(); $plainText = $this->stripHtmlTags($htmlContent); - + // Get frontmatter $frontMatter = $page->getFrontMatter(); - + // Extract keywords if present $keywords = ''; if (isset($frontMatter['keywords'])) { @@ -122,10 +149,10 @@ protected function indexPage(PageResource $page): array $keywords = $frontMatter['keywords']; } } - + // Extract other relevant metadata (description, tags, etc.) $metadata = []; - $metadataFields = $this->config->get('learn.search.index.metadata_fields', ['description', 'tags', 'category', 'author']); + $metadataFields = $this->config->get('learn.search.metadata_fields', ['description', 'tags', 'category', 'author']); foreach ($metadataFields as $field) { if (isset($frontMatter[$field])) { if (is_array($frontMatter[$field])) { @@ -183,7 +210,7 @@ protected function stripHtmlTags(string $html): string // Normalize whitespace $text = preg_replace('/\s+/', ' ', $text); - + // Check if preg_replace failed if ($text === null) { // Fallback: at least decode entities from stripped HTML diff --git a/app/src/Search/SearchService.php b/app/src/Search/SearchService.php index 3fb7f9a0..4a00fa4d 100644 --- a/app/src/Search/SearchService.php +++ b/app/src/Search/SearchService.php @@ -22,6 +22,9 @@ * - Wildcard pattern matching * - Snippet extraction with context * - Result caching + * + * @phpstan-import-type IndexedPage from IndexedPageShape + * @phpstan-import-type SearchResult from IndexedPageShape */ class SearchService { @@ -31,37 +34,14 @@ public function __construct( ) { } - /** - * Get the search index for a specific version from cache. - * Public method for use by SearchSprunje. - * - * @param string $version - * - * @return array - */ - public function getIndex(string $version): array - { - $keyFormat = $this->config->get('learn.search.index.key', 'learn.search-index.%1$s'); - $cacheKey = sprintf($keyFormat, $version); - - $index = $this->cache->get($cacheKey); - - // Ensure we return an array even if cache returns null or unexpected type - if (!is_array($index)) { - return []; - } - - return $index; - } - /** * Perform the actual search and generate results with snippets. * Public method for use by SearchSprunje. * - * @param string $query - * @param array $index + * @param string $query + * @param array $index * - * @return array + * @return array */ public function performSearch(string $query, array $index): array { @@ -137,6 +117,7 @@ public function performSearch(string $query, array $index): array usort($results, fn ($a, $b) => $b['matches'] <=> $a['matches']); $maxResults = $this->config->get('learn.search.max_results', 1000); + return array_slice($results, 0, $maxResults); } @@ -166,7 +147,7 @@ protected function searchPlain(string $query, string $content): array /** * Search for wildcard pattern matches. * - * @param string $regex Pre-compiled regex pattern + * @param string $regex Pre-compiled regex pattern * @param string $content * * @return array Array of match positions @@ -185,7 +166,7 @@ protected function searchWithWildcard(string $regex, string $content): array } foreach ($words as $word) { - if (preg_match($regex, $word)) { + if (preg_match($regex, $word) === 1) { $matches[] = $offset; } $offset += mb_strlen($word) + 1; // +1 for space @@ -207,8 +188,8 @@ protected function generateSnippet(string $content, int $matchPosition): string $contextLength = $this->config->get('learn.search.snippet_length', 150); // Calculate start and end positions - $start = max(0, $matchPosition - $contextLength); - $end = min(mb_strlen($content), $matchPosition + $contextLength); + $start = (int) max(0, $matchPosition - $contextLength); + $end = (int) min(mb_strlen($content), $matchPosition + $contextLength); // Extract snippet $snippet = mb_substr($content, $start, $end - $start); @@ -224,4 +205,3 @@ protected function generateSnippet(string $content, int $matchPosition): string return $snippet; } } - diff --git a/app/src/Search/SearchSprunje.php b/app/src/Search/SearchSprunje.php index 33bd6a81..04f04c66 100644 --- a/app/src/Search/SearchSprunje.php +++ b/app/src/Search/SearchSprunje.php @@ -12,8 +12,8 @@ namespace UserFrosting\Learn\Search; -use Illuminate\Database\Eloquent\Builder as EloquentBuilder; use Illuminate\Support\Collection; +use InvalidArgumentException; use UserFrosting\Config\Config; use UserFrosting\Sprinkle\Core\Sprunje\Sprunje; @@ -22,94 +22,58 @@ * * Provides a Sprunje-compatible interface for searching documentation pages. * Adapts the SearchService to work with the Sprunje API. + * + * @phpstan-import-type IndexedPage from IndexedPageShape + * @phpstan-import-type SearchResult from IndexedPageShape + * + * @extends StaticSprunje */ -class SearchSprunje extends Sprunje +class SearchSprunje extends StaticSprunje { - /** - * @var string Name of this Sprunje - */ - protected string $name = 'search'; - - /** - * @var string The search query - */ - protected string $searchQuery = ''; - - /** - * @var string|null The version to search - */ - protected ?string $version = null; - public function __construct( protected SearchService $searchService, - protected Config $config, - array $options = [] + protected SearchIndex $searchIndex, + protected Config $config ) { - // Extract search-specific options before passing to parent - $this->searchQuery = $options['query'] ?? ''; - $this->version = $options['version'] ?? null; - - // Validate query here for consistency - $minLength = $this->config->get('learn.search.min_length', 3); - if ($this->searchQuery === '' || mb_strlen($this->searchQuery) < $minLength) { - throw new \InvalidArgumentException("Query must be at least {$minLength} characters long"); - } - - // Remove search-specific options before parent processes them - unset($options['query'], $options['version']); - - // Call parent constructor - parent::__construct($options); } /** - * Required by Sprunje abstract class. Returns a dummy Eloquent builder. - * - * SearchSprunje doesn't use database queries - we override getModels() - * to use SearchService directly. This builder is only used internally - * by Sprunje for type requirements and is never actually queried. + * Get the underlying queryable object in its current state. * - * @return EloquentBuilder + * @return Collection */ - protected function baseQuery(): EloquentBuilder + public function getQuery(): Collection { - // Return a dummy Eloquent builder that won't be used for actual queries - $model = new \UserFrosting\Learn\Search\DummySearchModel(); - return $model->newQuery(); - } + // Default version if not provided + if (!isset($this->options['version']) || $this->options['version'] === null) { + $this->options['version'] = $this->config->get('learn.versions.latest'); + } - /** - * Override getModels to use SearchService instead of database queries. - * - * @return array{int, int, Collection} - */ - public function getModels(): array - { - // Get the version to search - $versionId = $this->version ?? $this->config->get('learn.versions.latest'); - - if ($versionId === null) { - return [0, 0, collect([])]; + // No version specified means no results + if ($this->options['version'] === null) { + return collect([]); } // Get the index from cache - $index = $this->searchService->getIndex($versionId); + $index = $this->searchIndex->getIndex($this->options['version']); + // No indexed pages means no results if (count($index) === 0) { - return [0, 0, collect([])]; + return collect([]); } // Search through the index (without pagination - Sprunje handles that) - $results = $this->searchService->performSearch($this->searchQuery, $index); + $results = $this->searchService->performSearch($this->options['query'], $index); // Convert to Collection for compatibility $collection = collect($results); - return [ - count($index), - count($results), - $collection, - ]; + return $collection; } /** @@ -119,10 +83,12 @@ public function getModels(): array */ protected function validateOptions(array $options): void { - // Don't validate query and version here as they're handled separately - $optionsToValidate = $options; - unset($optionsToValidate['query'], $optionsToValidate['version']); - - parent::validateOptions($optionsToValidate); + // Validate query here for consistency + $minLength = $this->config->get('learn.search.min_length', 3); + if (!is_string($options['query']) || $options['query'] === '' || mb_strlen($options['query']) < $minLength) { + throw new InvalidArgumentException("Query must be at least {$minLength} characters long"); + } + + parent::validateOptions($options); } } diff --git a/app/src/Search/StaticSprunje.php b/app/src/Search/StaticSprunje.php new file mode 100644 index 00000000..f474d11f --- /dev/null +++ b/app/src/Search/StaticSprunje.php @@ -0,0 +1,222 @@ + 'all', + 'page' => null, + ]; + + /** + * @var string[] Fields to show in output. Empty array will load all. + */ + protected array $columns = []; + + /** + * @var string Array key for the total unfiltered object count. + */ + protected string $countKey = 'count'; + + /** + * @var string Array key for the actual result set. + */ + protected string $rowsKey = 'rows'; + + /** + * Set Sprunje options. + * + * @param array $options Partial TOptions + * + * @return static + */ + public function setOptions(array $options): static + { + $this->validateOptions($options); + + // @phpstan-ignore-next-line - Can't make array_replace_recursive hint at TOptions + $this->options = array_replace_recursive($this->options, $options); + + return $this; + } + + /** + * Validate option using Validator. + * + * @param array $options + * + * @throws ValidationException + */ + protected function validateOptions(array $options): void + { + // Validation on input data + $v = new Validator($options); + $v->rule('regex', 'size', '/all|[0-9]+/i'); + $v->rule('integer', 'page'); + + if (!$v->validate()) { + $e = new ValidationException(); + $e->addErrors($v->errors()); // @phpstan-ignore-line errors returns array with no arguments + + throw $e; + } + } + + /** + * Execute the query and build the results, and append them in the appropriate format to the response. + * + * @param ResponseInterface $response + * + * @return ResponseInterface + */ + public function toResponse(ResponseInterface $response): ResponseInterface + { + $payload = json_encode($this->getArray(), JSON_THROW_ON_ERROR); + $response->getBody()->write($payload); + + return $response->withHeader('Content-Type', 'application/json'); + } + + /** + * Executes the sprunje query, applying all sorts, filters, and pagination. + * + * Returns an array containing `count` (the total number of rows, before filtering), + * and `rows` (the filtered result set). + * + * @return array + */ + public function getArray(): array + { + list($count, $rows) = $this->getModels(); + + // Return sprunjed results + return [ + $this->countKey => $count, + $this->rowsKey => $rows->values()->toArray(), + ]; + } + + /** + * Executes the sprunje query, applying all sorts, filters, and pagination. + * + * Returns the filtered, paginated result set and the counts. + * + * @return array{int, Collection} + */ + public function getModels(): array + { + $query = $this->getQuery(); + + // Count unfiltered total + $count = $this->count($query); + + // Paginate + $query = $this->applyPagination($query); + + // Execute query - only apply select if not wildcard/empty + if ($this->columns !== []) { + $query = $query->select($this->columns); // @phpstan-ignore-line + } + + $query = collect($query); + + // Perform any additional transformations on the dataset + $query = $this->applyTransformations($query); + + return [$count, $query]; + } + + /** + * Get the underlying queryable object in its current state. + * + * @return Collection + */ + abstract public function getQuery(): Collection; + + /** + * Apply pagination based on the `page` and `size` options. + * + * @param Collection $query + * + * @return Collection + */ + public function applyPagination(Collection $query): Collection + { + $page = $this->options['page']; + $size = $this->options['size']; + + if (!is_null($page) && !is_null($size) && $size !== 'all') { + $offset = (int) $size * (int) $page; + $query = $query->skip($offset)->take((int) $size); + } + + return $query; + } + + /** + * Set fields to show in output. + * + * @param string[] $columns + * + * @return static + */ + public function setColumns(array $columns): static + { + $this->columns = $columns; + + return $this; + } + + /** + * Set any transformations you wish to apply to the collection, after the query is executed. + * This method is meant to be customized in child class. + * + * @param Collection $collection + * + * @return Collection + */ + protected function applyTransformations(Collection $collection): Collection + { + return $collection; + } + + /** + * Get the unpaginated count of items (before filtering) in this query. + * + * @param Collection $query + * + * @return int + */ + protected function count(Collection $query): int + { + return $query->count(); + } +} diff --git a/app/tests/Bakery/SearchIndexCommandTest.php b/app/tests/Bakery/SearchIndexCommandTest.php new file mode 100644 index 00000000..edf52445 --- /dev/null +++ b/app/tests/Bakery/SearchIndexCommandTest.php @@ -0,0 +1,184 @@ +shouldReceive('buildIndex') + ->once() + ->with(null) + ->andReturn(42); + + // Create command and tester + $command = new SearchIndexCommand($searchIndex); + $tester = new CommandTester($command); + + // Execute command + $exitCode = $tester->execute([]); + + // Assertions + $this->assertSame(Command::SUCCESS, $exitCode); + $this->assertStringContainsString('Building search index for all versions', $tester->getDisplay()); + $this->assertStringContainsString('Indexed 42 pages', $tester->getDisplay()); + } + + /** + * Test building index for specific version. + */ + public function testBuildIndexSpecificVersion(): void + { + // Mock SearchIndex + $searchIndex = Mockery::mock(SearchIndex::class); + $searchIndex->shouldReceive('buildIndex') + ->once() + ->with('6.0') + ->andReturn(15); + + // Create command and tester + $command = new SearchIndexCommand($searchIndex); + $tester = new CommandTester($command); + + // Execute command with version option + $exitCode = $tester->execute(['--doc-version' => '6.0']); + + // Assertions + $this->assertSame(Command::SUCCESS, $exitCode); + $this->assertStringContainsString('Building search index for version 6.0', $tester->getDisplay()); + $this->assertStringContainsString('Indexed 15 pages', $tester->getDisplay()); + } + + /** + * Test clearing index before building. + */ + public function testClearIndexBeforeBuilding(): void + { + // Mock SearchIndex + $searchIndex = Mockery::mock(SearchIndex::class); + $searchIndex->shouldReceive('clearIndex') + ->once() + ->with(null); + $searchIndex->shouldReceive('buildIndex') + ->once() + ->with(null) + ->andReturn(30); + + // Create command and tester + $command = new SearchIndexCommand($searchIndex); + $tester = new CommandTester($command); + + // Execute command with clear option + $exitCode = $tester->execute(['--clear' => true]); + + // Assertions + $this->assertSame(Command::SUCCESS, $exitCode); + $this->assertStringContainsString('Clearing search index', $tester->getDisplay()); + $this->assertStringContainsString('Search index cleared', $tester->getDisplay()); + $this->assertStringContainsString('Indexed 30 pages', $tester->getDisplay()); + } + + /** + * Test clearing and building for specific version. + */ + public function testClearAndBuildSpecificVersion(): void + { + // Mock SearchIndex + $searchIndex = Mockery::mock(SearchIndex::class); + $searchIndex->shouldReceive('clearIndex') + ->once() + ->with('5.1'); + $searchIndex->shouldReceive('buildIndex') + ->once() + ->with('5.1') + ->andReturn(20); + + // Create command and tester + $command = new SearchIndexCommand($searchIndex); + $tester = new CommandTester($command); + + // Execute command with both options + $exitCode = $tester->execute([ + '--doc-version' => '5.1', + '--clear' => true, + ]); + + // Assertions + $this->assertSame(Command::SUCCESS, $exitCode); + $this->assertStringContainsString('Clearing search index', $tester->getDisplay()); + $this->assertStringContainsString('Building search index for version 5.1', $tester->getDisplay()); + $this->assertStringContainsString('Indexed 20 pages', $tester->getDisplay()); + } + + /** + * Test handling exception during index building. + */ + public function testBuildIndexException(): void + { + // Mock SearchIndex to throw exception + $searchIndex = Mockery::mock(SearchIndex::class); + $searchIndex->shouldReceive('buildIndex') + ->once() + ->with(null) + ->andThrow(new \RuntimeException('Index build failed')); + + // Create command and tester + $command = new SearchIndexCommand($searchIndex); + $tester = new CommandTester($command); + + // Execute command + $exitCode = $tester->execute([]); + + // Assertions + $this->assertSame(Command::FAILURE, $exitCode); + $this->assertStringContainsString('Failed to build search index', $tester->getDisplay()); + $this->assertStringContainsString('Index build failed', $tester->getDisplay()); + } + + /** + * Test command configuration. + */ + public function testCommandConfiguration(): void + { + $searchIndex = Mockery::mock(SearchIndex::class); + $command = new SearchIndexCommand($searchIndex); + + $this->assertSame('search:index', $command->getName()); + $this->assertStringContainsString('Build or rebuild', $command->getDescription()); + + $definition = $command->getDefinition(); + $this->assertTrue($definition->hasOption('doc-version')); + $this->assertTrue($definition->hasOption('clear')); + } +} diff --git a/app/tests/Controller/SearchControllerTest.php b/app/tests/Controller/SearchControllerTest.php index bae6a24e..94e4f245 100644 --- a/app/tests/Controller/SearchControllerTest.php +++ b/app/tests/Controller/SearchControllerTest.php @@ -66,14 +66,13 @@ public function testSearchEndpoint(): void $this->assertIsArray($data); $this->assertArrayHasKey('rows', $data); $this->assertArrayHasKey('count', $data); - $this->assertArrayHasKey('count_filtered', $data); // Should have some results - $this->assertGreaterThan(0, $data['count_filtered']); + $this->assertGreaterThan(0, $data['count']); $this->assertNotEmpty($data['rows']); // Check structure of first result - if (!empty($data['rows'])) { + if (count($data['rows']) > 0) { $firstResult = $data['rows'][0]; $this->assertArrayHasKey('title', $firstResult); $this->assertArrayHasKey('slug', $firstResult); @@ -93,10 +92,10 @@ public function testSearchEndpointEmptyQuery(): void $request = $this->createRequest('GET', '/api/search'); $response = $this->handleRequest($request); - // Should return 404 Not Found for invalid query (NotFoundException) - $this->assertResponseStatus(404, $response); + // Returns 500 because InvalidArgumentException is not caught + $this->assertResponseStatus(500, $response); } - + /** * Test search API endpoint with query too short. */ @@ -106,8 +105,8 @@ public function testSearchEndpointQueryTooShort(): void $request = $this->createRequest('GET', '/api/search?q=ab'); $response = $this->handleRequest($request); - // Should return 404 Not Found for query too short (NotFoundException) - $this->assertResponseStatus(404, $response); + // Returns 500 because InvalidArgumentException is not caught + $this->assertResponseStatus(500, $response); } /** @@ -151,7 +150,7 @@ public function testSearchEndpointWithVersion(): void $this->assertIsArray($data); // Verify results are from correct version - if (!empty($data['rows'])) { + if (count($data['rows']) > 0) { foreach ($data['rows'] as $result) { $this->assertSame('6.0', $result['version']); } @@ -163,8 +162,8 @@ public function testSearchEndpointWithVersion(): void */ public function testSearchEndpointWildcard(): void { - // Create request with wildcard query - $request = $this->createRequest('GET', '/api/search?q=f*'); + // Create request with wildcard query that meets minimum length + $request = $this->createRequest('GET', '/api/search?q=fir*'); $response = $this->handleRequest($request); // Assert successful response diff --git a/app/tests/Search/SearchIndexTest.php b/app/tests/Search/SearchIndexTest.php index a7898a30..457e5ed6 100644 --- a/app/tests/Search/SearchIndexTest.php +++ b/app/tests/Search/SearchIndexTest.php @@ -52,16 +52,16 @@ public function testBuildIndexForVersion(): void // Should have indexed pages (at least some) $this->assertGreaterThan(0, $count, 'Should have indexed at least one page'); - + // Verify it matches the number of test pages /** @var \UserFrosting\Learn\Documentation\DocumentationRepository $repository */ $repository = $this->ci->get(\UserFrosting\Learn\Documentation\DocumentationRepository::class); - + // Use reflection to get pages count $reflection = new \ReflectionClass($repository); $method = $reflection->getMethod('getFlattenedTree'); $flatPages = $method->invoke($repository, '6.0'); - + $this->assertSame(count($flatPages), $count, 'Index count should match actual page count'); } @@ -201,7 +201,7 @@ public function testFlattenTree(): void foreach ($flat as $page) { $this->assertInstanceOf(\UserFrosting\Learn\Documentation\PageResource::class, $page); } - + // Verify flat count matches tree structure (all pages including nested) $countTreePages = function ($pages) use (&$countTreePages) { $count = 0; @@ -211,9 +211,10 @@ public function testFlattenTree(): void $count += $countTreePages($page->getChildren()); } } + return $count; }; - + $expectedCount = $countTreePages($tree); $this->assertSame($expectedCount, count($flat), 'Flattened tree should contain all pages'); } diff --git a/app/tests/Search/SearchServiceTest.php b/app/tests/Search/SearchServiceTest.php index 0386f666..a77cad8f 100644 --- a/app/tests/Search/SearchServiceTest.php +++ b/app/tests/Search/SearchServiceTest.php @@ -14,6 +14,7 @@ use UserFrosting\Learn\Recipe; use UserFrosting\Learn\Search\SearchIndex; use UserFrosting\Learn\Search\SearchService; +use UserFrosting\Learn\Search\SearchSprunje; use UserFrosting\Testing\TestCase; use UserFrosting\UniformResourceLocator\ResourceLocatorInterface; use UserFrosting\UniformResourceLocator\ResourceStream; @@ -51,20 +52,17 @@ public function setUp(): void public function testSearchWithPlainText(): void { $searchService = $this->ci->get(SearchService::class); + $searchIndex = $this->ci->get(SearchIndex::class); - // Search for "first" - should match "First page" - $result = $searchService->search('first', '6.0'); - - $this->assertIsArray($result); - $this->assertArrayHasKey('rows', $result); - $this->assertArrayHasKey('count', $result); - $this->assertArrayHasKey('count_filtered', $result); + // Get index and search for "first" - should match "First page" + $index = $searchIndex->getIndex('6.0'); + $results = $searchService->performSearch('first', $index); - $this->assertGreaterThan(0, $result['count_filtered']); - $this->assertNotEmpty($result['rows']); + $this->assertIsArray($results); + $this->assertGreaterThan(0, count($results)); // Check structure of first result - $firstResult = $result['rows'][0]; + $firstResult = $results[0]; $this->assertArrayHasKey('title', $firstResult); $this->assertArrayHasKey('slug', $firstResult); $this->assertArrayHasKey('route', $firstResult); @@ -76,41 +74,55 @@ public function testSearchWithPlainText(): void public function testSearchWithEmptyQuery(): void { $searchService = $this->ci->get(SearchService::class); + $searchIndex = $this->ci->get(SearchIndex::class); - $result = $searchService->search('', '6.0'); + $index = $searchIndex->getIndex('6.0'); + $results = $searchService->performSearch('', $index); - $this->assertSame(0, $result['count_filtered']); - $this->assertEmpty($result['rows']); + $this->assertSame(0, count($results)); + $this->assertEmpty($results); } public function testSearchWithWildcard(): void { $searchService = $this->ci->get(SearchService::class); + $searchIndex = $this->ci->get(SearchIndex::class); // Search for "f*" - should match words starting with 'f' - $result = $searchService->search('f*', '6.0'); + $index = $searchIndex->getIndex('6.0'); + $results = $searchService->performSearch('f*', $index); - $this->assertGreaterThanOrEqual(0, $result['count_filtered']); + $this->assertIsArray($results); + $this->assertGreaterThanOrEqual(0, count($results)); } public function testSearchPagination(): void { - $searchService = $this->ci->get(SearchService::class); + $searchSprunje = $this->ci->get(SearchSprunje::class); + + // Search with pagination via Sprunje + $searchSprunje->setOptions([ + 'query' => 'page', + 'version' => '6.0', + 'size' => 2, + 'page' => 0, + ]); - // Search with pagination - $result = $searchService->search('page', '6.0', 1, 2); + $result = $searchSprunje->getArray(); - $this->assertLessThanOrEqual(2, count($result['rows'])); + $this->assertArrayHasKey('rows', $result); } public function testSearchResultSnippet(): void { $searchService = $this->ci->get(SearchService::class); + $searchIndex = $this->ci->get(SearchIndex::class); - $result = $searchService->search('first', '6.0'); + $index = $searchIndex->getIndex('6.0'); + $results = $searchService->performSearch('first', $index); - if (!empty($result['rows'])) { - $firstResult = $result['rows'][0]; + if (count($results) > 0) { + $firstResult = $results[0]; $this->assertIsString($firstResult['snippet']); $this->assertNotEmpty($firstResult['snippet']); } @@ -139,7 +151,8 @@ public function testGenerateSnippet(): void $reflection = new \ReflectionClass($searchService); $method = $reflection->getMethod('generateSnippet'); - $content = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit. This is the important part. More text follows here.'; + // Create long content that exceeds snippet length (default 150 chars) + $content = str_repeat('Lorem ipsum dolor sit amet, consectetur adipiscing elit. ', 10) . 'This is the important part. ' . str_repeat('More text follows here. ', 10); $matchPosition = strpos($content, 'important'); if ($matchPosition !== false) { @@ -158,23 +171,26 @@ public function testSearchWithNoIndex(): void $searchIndex->clearIndex('6.0'); $searchService = $this->ci->get(SearchService::class); - $result = $searchService->search('test', '6.0'); + $index = $searchIndex->getIndex('6.0'); + $results = $searchService->performSearch('test', $index); - $this->assertSame(0, $result['count_filtered']); - $this->assertEmpty($result['rows']); + $this->assertSame(0, count($results)); + $this->assertEmpty($results); } public function testSearchResultSorting(): void { $searchService = $this->ci->get(SearchService::class); + $searchIndex = $this->ci->get(SearchIndex::class); // Search for a common term that might appear multiple times - $result = $searchService->search('page', '6.0'); + $index = $searchIndex->getIndex('6.0'); + $results = $searchService->performSearch('page', $index); - if (count($result['rows']) > 1) { + if (count($results) > 1) { // Verify results are sorted by number of matches (descending) - $firstMatches = $result['rows'][0]['matches']; - $lastMatches = $result['rows'][count($result['rows']) - 1]['matches']; + $firstMatches = $results[0]['matches']; + $lastMatches = $results[count($results) - 1]['matches']; $this->assertGreaterThanOrEqual($lastMatches, $firstMatches); } diff --git a/composer.lock b/composer.lock index 44c7f25e..96474ea1 100644 --- a/composer.lock +++ b/composer.lock @@ -451,16 +451,16 @@ }, { "name": "doctrine/event-manager", - "version": "2.0.1", + "version": "2.1.0", "source": { "type": "git", "url": "https://github.com/doctrine/event-manager.git", - "reference": "b680156fa328f1dfd874fd48c7026c41570b9c6e" + "reference": "c07799fcf5ad362050960a0fd068dded40b1e312" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/doctrine/event-manager/zipball/b680156fa328f1dfd874fd48c7026c41570b9c6e", - "reference": "b680156fa328f1dfd874fd48c7026c41570b9c6e", + "url": "https://api.github.com/repos/doctrine/event-manager/zipball/c07799fcf5ad362050960a0fd068dded40b1e312", + "reference": "c07799fcf5ad362050960a0fd068dded40b1e312", "shasum": "" }, "require": { @@ -470,10 +470,10 @@ "doctrine/common": "<2.9" }, "require-dev": { - "doctrine/coding-standard": "^12", - "phpstan/phpstan": "^1.8.8", - "phpunit/phpunit": "^10.5", - "vimeo/psalm": "^5.24" + "doctrine/coding-standard": "^14", + "phpdocumentor/guides-cli": "^1.4", + "phpstan/phpstan": "^2.1.32", + "phpunit/phpunit": "^10.5.58" }, "type": "library", "autoload": { @@ -522,7 +522,7 @@ ], "support": { "issues": "https://github.com/doctrine/event-manager/issues", - "source": "https://github.com/doctrine/event-manager/tree/2.0.1" + "source": "https://github.com/doctrine/event-manager/tree/2.1.0" }, "funding": [ { @@ -538,7 +538,7 @@ "type": "tidelift" } ], - "time": "2024-05-22T20:47:39+00:00" + "time": "2026-01-17T22:40:21+00:00" }, { "name": "doctrine/inflector", @@ -2080,16 +2080,16 @@ }, { "name": "laravel/serializable-closure", - "version": "v2.0.7", + "version": "v2.0.8", "source": { "type": "git", "url": "https://github.com/laravel/serializable-closure.git", - "reference": "cb291e4c998ac50637c7eeb58189c14f5de5b9dd" + "reference": "7581a4407012f5f53365e11bafc520fd7f36bc9b" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/laravel/serializable-closure/zipball/cb291e4c998ac50637c7eeb58189c14f5de5b9dd", - "reference": "cb291e4c998ac50637c7eeb58189c14f5de5b9dd", + "url": "https://api.github.com/repos/laravel/serializable-closure/zipball/7581a4407012f5f53365e11bafc520fd7f36bc9b", + "reference": "7581a4407012f5f53365e11bafc520fd7f36bc9b", "shasum": "" }, "require": { @@ -2137,7 +2137,7 @@ "issues": "https://github.com/laravel/serializable-closure/issues", "source": "https://github.com/laravel/serializable-closure" }, - "time": "2025-11-21T20:52:36+00:00" + "time": "2026-01-08T16:22:46+00:00" }, { "name": "lcharette/webpack-encore-twig", @@ -2669,16 +2669,16 @@ }, { "name": "monolog/monolog", - "version": "3.9.0", + "version": "3.10.0", "source": { "type": "git", "url": "https://github.com/Seldaek/monolog.git", - "reference": "10d85740180ecba7896c87e06a166e0c95a0e3b6" + "reference": "b321dd6749f0bf7189444158a3ce785cc16d69b0" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/Seldaek/monolog/zipball/10d85740180ecba7896c87e06a166e0c95a0e3b6", - "reference": "10d85740180ecba7896c87e06a166e0c95a0e3b6", + "url": "https://api.github.com/repos/Seldaek/monolog/zipball/b321dd6749f0bf7189444158a3ce785cc16d69b0", + "reference": "b321dd6749f0bf7189444158a3ce785cc16d69b0", "shasum": "" }, "require": { @@ -2696,7 +2696,7 @@ "graylog2/gelf-php": "^1.4.2 || ^2.0", "guzzlehttp/guzzle": "^7.4.5", "guzzlehttp/psr7": "^2.2", - "mongodb/mongodb": "^1.8", + "mongodb/mongodb": "^1.8 || ^2.0", "php-amqplib/php-amqplib": "~2.4 || ^3", "php-console/php-console": "^3.1.8", "phpstan/phpstan": "^2", @@ -2756,7 +2756,7 @@ ], "support": { "issues": "https://github.com/Seldaek/monolog/issues", - "source": "https://github.com/Seldaek/monolog/tree/3.9.0" + "source": "https://github.com/Seldaek/monolog/tree/3.10.0" }, "funding": [ { @@ -2768,7 +2768,7 @@ "type": "tidelift" } ], - "time": "2025-03-24T10:02:05+00:00" + "time": "2026-01-02T08:56:05+00:00" }, { "name": "nesbot/carbon", @@ -4549,16 +4549,16 @@ }, { "name": "symfony/config", - "version": "v7.4.1", + "version": "v7.4.3", "source": { "type": "git", "url": "https://github.com/symfony/config.git", - "reference": "2c323304c354a43a48b61c5fa760fc4ed60ce495" + "reference": "800ce889e358a53a9678b3212b0c8cecd8c6aace" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/config/zipball/2c323304c354a43a48b61c5fa760fc4ed60ce495", - "reference": "2c323304c354a43a48b61c5fa760fc4ed60ce495", + "url": "https://api.github.com/repos/symfony/config/zipball/800ce889e358a53a9678b3212b0c8cecd8c6aace", + "reference": "800ce889e358a53a9678b3212b0c8cecd8c6aace", "shasum": "" }, "require": { @@ -4604,7 +4604,7 @@ "description": "Helps you find, load, combine, autofill and validate configuration values of any kind", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/config/tree/v7.4.1" + "source": "https://github.com/symfony/config/tree/v7.4.3" }, "funding": [ { @@ -4624,20 +4624,20 @@ "type": "tidelift" } ], - "time": "2025-12-05T07:52:08+00:00" + "time": "2025-12-23T14:24:27+00:00" }, { "name": "symfony/console", - "version": "v6.4.30", + "version": "v6.4.31", "source": { "type": "git", "url": "https://github.com/symfony/console.git", - "reference": "1b2813049506b39eb3d7e64aff033fd5ca26c97e" + "reference": "f9f8a889f54c264f9abac3fc0f7a371ffca51997" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/console/zipball/1b2813049506b39eb3d7e64aff033fd5ca26c97e", - "reference": "1b2813049506b39eb3d7e64aff033fd5ca26c97e", + "url": "https://api.github.com/repos/symfony/console/zipball/f9f8a889f54c264f9abac3fc0f7a371ffca51997", + "reference": "f9f8a889f54c264f9abac3fc0f7a371ffca51997", "shasum": "" }, "require": { @@ -4702,7 +4702,7 @@ "terminal" ], "support": { - "source": "https://github.com/symfony/console/tree/v6.4.30" + "source": "https://github.com/symfony/console/tree/v6.4.31" }, "funding": [ { @@ -4722,20 +4722,20 @@ "type": "tidelift" } ], - "time": "2025-12-05T13:47:41+00:00" + "time": "2025-12-22T08:30:34+00:00" }, { "name": "symfony/dependency-injection", - "version": "v7.4.2", + "version": "v7.4.3", "source": { "type": "git", "url": "https://github.com/symfony/dependency-injection.git", - "reference": "baf614f7c15b30ba6762d4b1ddabdf83dbf0d29b" + "reference": "54122901b6d772e94f1e71a75e0533bc16563499" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/dependency-injection/zipball/baf614f7c15b30ba6762d4b1ddabdf83dbf0d29b", - "reference": "baf614f7c15b30ba6762d4b1ddabdf83dbf0d29b", + "url": "https://api.github.com/repos/symfony/dependency-injection/zipball/54122901b6d772e94f1e71a75e0533bc16563499", + "reference": "54122901b6d772e94f1e71a75e0533bc16563499", "shasum": "" }, "require": { @@ -4786,7 +4786,7 @@ "description": "Allows you to standardize and centralize the way objects are constructed in your application", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/dependency-injection/tree/v7.4.2" + "source": "https://github.com/symfony/dependency-injection/tree/v7.4.3" }, "funding": [ { @@ -4806,7 +4806,7 @@ "type": "tidelift" } ], - "time": "2025-12-08T06:57:04+00:00" + "time": "2025-12-28T10:55:46+00:00" }, { "name": "symfony/deprecation-contracts", @@ -5190,16 +5190,16 @@ }, { "name": "symfony/finder", - "version": "v6.4.27", + "version": "v6.4.31", "source": { "type": "git", "url": "https://github.com/symfony/finder.git", - "reference": "a1b6aa435d2fba50793b994a839c32b6064f063b" + "reference": "5547f2e1f0ca8e2e7abe490156b62da778cfbe2b" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/finder/zipball/a1b6aa435d2fba50793b994a839c32b6064f063b", - "reference": "a1b6aa435d2fba50793b994a839c32b6064f063b", + "url": "https://api.github.com/repos/symfony/finder/zipball/5547f2e1f0ca8e2e7abe490156b62da778cfbe2b", + "reference": "5547f2e1f0ca8e2e7abe490156b62da778cfbe2b", "shasum": "" }, "require": { @@ -5234,7 +5234,7 @@ "description": "Finds files and directories via an intuitive fluent interface", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/finder/tree/v6.4.27" + "source": "https://github.com/symfony/finder/tree/v6.4.31" }, "funding": [ { @@ -5254,20 +5254,20 @@ "type": "tidelift" } ], - "time": "2025-10-15T18:32:00+00:00" + "time": "2025-12-11T14:52:17+00:00" }, { "name": "symfony/http-foundation", - "version": "v6.4.30", + "version": "v6.4.31", "source": { "type": "git", "url": "https://github.com/symfony/http-foundation.git", - "reference": "0384c62b79d96e9b22d77bc1272c9e83342ba3a6" + "reference": "a35ee6f47e4775179704d7877a8b0da3cb09241a" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/http-foundation/zipball/0384c62b79d96e9b22d77bc1272c9e83342ba3a6", - "reference": "0384c62b79d96e9b22d77bc1272c9e83342ba3a6", + "url": "https://api.github.com/repos/symfony/http-foundation/zipball/a35ee6f47e4775179704d7877a8b0da3cb09241a", + "reference": "a35ee6f47e4775179704d7877a8b0da3cb09241a", "shasum": "" }, "require": { @@ -5315,7 +5315,7 @@ "description": "Defines an object-oriented layer for the HTTP specification", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/http-foundation/tree/v6.4.30" + "source": "https://github.com/symfony/http-foundation/tree/v6.4.31" }, "funding": [ { @@ -5335,7 +5335,7 @@ "type": "tidelift" } ], - "time": "2025-12-01T20:07:31+00:00" + "time": "2025-12-17T10:10:57+00:00" }, { "name": "symfony/http-kernel", @@ -6116,16 +6116,16 @@ }, { "name": "symfony/process", - "version": "v6.4.26", + "version": "v6.4.31", "source": { "type": "git", "url": "https://github.com/symfony/process.git", - "reference": "48bad913268c8cafabbf7034b39c8bb24fbc5ab8" + "reference": "8541b7308fca001320e90bca8a73a28aa5604a6e" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/process/zipball/48bad913268c8cafabbf7034b39c8bb24fbc5ab8", - "reference": "48bad913268c8cafabbf7034b39c8bb24fbc5ab8", + "url": "https://api.github.com/repos/symfony/process/zipball/8541b7308fca001320e90bca8a73a28aa5604a6e", + "reference": "8541b7308fca001320e90bca8a73a28aa5604a6e", "shasum": "" }, "require": { @@ -6157,7 +6157,7 @@ "description": "Executes commands in sub-processes", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/process/tree/v6.4.26" + "source": "https://github.com/symfony/process/tree/v6.4.31" }, "funding": [ { @@ -6177,7 +6177,7 @@ "type": "tidelift" } ], - "time": "2025-09-11T09:57:09+00:00" + "time": "2025-12-15T19:26:35+00:00" }, { "name": "symfony/service-contracts", @@ -6359,16 +6359,16 @@ }, { "name": "symfony/translation", - "version": "v6.4.30", + "version": "v6.4.31", "source": { "type": "git", "url": "https://github.com/symfony/translation.git", - "reference": "d1fdeefd0707d15eb150c04e8837bf0b15ebea39" + "reference": "81579408ecf7dc5aa2d8462a6d5c3a430a80e6f2" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/translation/zipball/d1fdeefd0707d15eb150c04e8837bf0b15ebea39", - "reference": "d1fdeefd0707d15eb150c04e8837bf0b15ebea39", + "url": "https://api.github.com/repos/symfony/translation/zipball/81579408ecf7dc5aa2d8462a6d5c3a430a80e6f2", + "reference": "81579408ecf7dc5aa2d8462a6d5c3a430a80e6f2", "shasum": "" }, "require": { @@ -6434,7 +6434,7 @@ "description": "Provides tools to internationalize your application", "homepage": "https://symfony.com", "support": { - "source": "https://github.com/symfony/translation/tree/v6.4.30" + "source": "https://github.com/symfony/translation/tree/v6.4.31" }, "funding": [ { @@ -6454,7 +6454,7 @@ "type": "tidelift" } ], - "time": "2025-11-24T13:57:00+00:00" + "time": "2025-12-18T11:37:55+00:00" }, { "name": "symfony/translation-contracts", @@ -6540,16 +6540,16 @@ }, { "name": "symfony/var-dumper", - "version": "v7.4.0", + "version": "v7.4.3", "source": { "type": "git", "url": "https://github.com/symfony/var-dumper.git", - "reference": "41fd6c4ae28c38b294b42af6db61446594a0dece" + "reference": "7e99bebcb3f90d8721890f2963463280848cba92" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/var-dumper/zipball/41fd6c4ae28c38b294b42af6db61446594a0dece", - "reference": "41fd6c4ae28c38b294b42af6db61446594a0dece", + "url": "https://api.github.com/repos/symfony/var-dumper/zipball/7e99bebcb3f90d8721890f2963463280848cba92", + "reference": "7e99bebcb3f90d8721890f2963463280848cba92", "shasum": "" }, "require": { @@ -6603,7 +6603,7 @@ "dump" ], "support": { - "source": "https://github.com/symfony/var-dumper/tree/v7.4.0" + "source": "https://github.com/symfony/var-dumper/tree/v7.4.3" }, "funding": [ { @@ -6623,7 +6623,7 @@ "type": "tidelift" } ], - "time": "2025-10-27T20:36:44+00:00" + "time": "2025-12-18T07:04:31+00:00" }, { "name": "symfony/var-exporter", @@ -6939,16 +6939,16 @@ }, { "name": "userfrosting/framework", - "version": "6.0.0-beta.7", + "version": "6.0.0-beta.8", "source": { "type": "git", "url": "https://github.com/userfrosting/framework.git", - "reference": "a200c2e7a88e3590c43e4d0bb149221baeea3c7d" + "reference": "d6ca95119f8e2543c8046e1ebef315e1a73a0974" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/userfrosting/framework/zipball/a200c2e7a88e3590c43e4d0bb149221baeea3c7d", - "reference": "a200c2e7a88e3590c43e4d0bb149221baeea3c7d", + "url": "https://api.github.com/repos/userfrosting/framework/zipball/d6ca95119f8e2543c8046e1ebef315e1a73a0974", + "reference": "d6ca95119f8e2543c8046e1ebef315e1a73a0974", "shasum": "" }, "require": { @@ -7018,22 +7018,22 @@ ], "support": { "issues": "https://github.com/userfrosting/framework/issues", - "source": "https://github.com/userfrosting/framework/tree/6.0.0-beta.7" + "source": "https://github.com/userfrosting/framework/tree/6.0.0-beta.8" }, - "time": "2025-12-20T18:14:44+00:00" + "time": "2026-01-11T01:52:02+00:00" }, { "name": "userfrosting/sprinkle-core", - "version": "6.0.0-beta.7", + "version": "6.0.0-beta.8", "source": { "type": "git", "url": "https://github.com/userfrosting/sprinkle-core.git", - "reference": "bd72ee16cd013cca1246a9991a551457e4d54c0d" + "reference": "7fdd7ebecb7be50b9849a7b4b4433a5b63390b59" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/userfrosting/sprinkle-core/zipball/bd72ee16cd013cca1246a9991a551457e4d54c0d", - "reference": "bd72ee16cd013cca1246a9991a551457e4d54c0d", + "url": "https://api.github.com/repos/userfrosting/sprinkle-core/zipball/7fdd7ebecb7be50b9849a7b4b4433a5b63390b59", + "reference": "7fdd7ebecb7be50b9849a7b4b4433a5b63390b59", "shasum": "" }, "require": { @@ -7111,9 +7111,9 @@ ], "support": { "issues": "https://github.com/userfrosting/sprinkle-core/issues", - "source": "https://github.com/userfrosting/sprinkle-core/tree/6.0.0-beta.7" + "source": "https://github.com/userfrosting/sprinkle-core/tree/6.0.0-beta.8" }, - "time": "2025-12-30T02:31:31+00:00" + "time": "2026-01-13T00:55:55+00:00" }, { "name": "userfrosting/vite-php-twig", @@ -7773,16 +7773,16 @@ }, { "name": "friendsofphp/php-cs-fixer", - "version": "v3.92.3", + "version": "v3.92.5", "source": { "type": "git", "url": "https://github.com/PHP-CS-Fixer/PHP-CS-Fixer.git", - "reference": "2ba8f5a60f6f42fb65758cfb3768434fa2d1c7e8" + "reference": "260cc8c4a1d2f6d2f22cd4f9c70aa72e55ebac58" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/PHP-CS-Fixer/PHP-CS-Fixer/zipball/2ba8f5a60f6f42fb65758cfb3768434fa2d1c7e8", - "reference": "2ba8f5a60f6f42fb65758cfb3768434fa2d1c7e8", + "url": "https://api.github.com/repos/PHP-CS-Fixer/PHP-CS-Fixer/zipball/260cc8c4a1d2f6d2f22cd4f9c70aa72e55ebac58", + "reference": "260cc8c4a1d2f6d2f22cd4f9c70aa72e55ebac58", "shasum": "" }, "require": { @@ -7814,17 +7814,17 @@ }, "require-dev": { "facile-it/paraunit": "^1.3.1 || ^2.7", - "infection/infection": "^0.31.0", - "justinrainbow/json-schema": "^6.5", - "keradus/cli-executor": "^2.2", + "infection/infection": "^0.31", + "justinrainbow/json-schema": "^6.6", + "keradus/cli-executor": "^2.3", "mikey179/vfsstream": "^1.6.12", "php-coveralls/php-coveralls": "^2.9", "php-cs-fixer/phpunit-constraint-isidenticalstring": "^1.6", "php-cs-fixer/phpunit-constraint-xmlmatchesxsd": "^1.6", - "phpunit/phpunit": "^9.6.25 || ^10.5.53 || ^11.5.34", + "phpunit/phpunit": "^9.6.31 || ^10.5.60 || ^11.5.46", "symfony/polyfill-php85": "^1.33", - "symfony/var-dumper": "^5.4.48 || ^6.4.24 || ^7.3.2 || ^8.0", - "symfony/yaml": "^5.4.45 || ^6.4.24 || ^7.3.2 || ^8.0" + "symfony/var-dumper": "^5.4.48 || ^6.4.26 || ^7.4.0 || ^8.0", + "symfony/yaml": "^5.4.45 || ^6.4.30 || ^7.4.1 || ^8.0" }, "suggest": { "ext-dom": "For handling output formats in XML", @@ -7865,7 +7865,7 @@ ], "support": { "issues": "https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/issues", - "source": "https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/tree/v3.92.3" + "source": "https://github.com/PHP-CS-Fixer/PHP-CS-Fixer/tree/v3.92.5" }, "funding": [ { @@ -7873,7 +7873,7 @@ "type": "github" } ], - "time": "2025-12-18T10:45:02+00:00" + "time": "2026-01-08T21:57:37+00:00" }, { "name": "hamcrest/hamcrest-php", From 2e7bc01d79cfed4c1f3f3e81a04d7d165a848b52 Mon Sep 17 00:00:00 2001 From: Louis Charette Date: Sat, 24 Jan 2026 09:15:05 -0500 Subject: [PATCH 14/20] Add size & page to output, complete tests, fix default options --- app/config/default.php | 3 +- app/src/Search/SearchSprunje.php | 20 +- app/src/Search/StaticSprunje.php | 33 +- app/tests/Controller/SearchControllerTest.php | 74 ++++ app/tests/Search/StaticSprunjeTest.php | 334 ++++++++++++++++++ 5 files changed, 439 insertions(+), 25 deletions(-) create mode 100644 app/tests/Search/StaticSprunjeTest.php diff --git a/app/config/default.php b/app/config/default.php index a6080bd0..55bffbe4 100644 --- a/app/config/default.php +++ b/app/config/default.php @@ -56,9 +56,8 @@ ], 'search' => [ 'min_length' => 3, // Minimum length of search query - 'default_page' => 1, // Default page number for paginated results + 'default_page' => 0, // Default page number for paginated results 'default_size' => 10, // Default number of results per page - 'max_size' => 10, // Default maximum number of results per page 'snippet_length' => 150, // Length of content snippets in results 'max_results' => 150, // Maximum number of results to consider for pagination 'cache' => [ diff --git a/app/src/Search/SearchSprunje.php b/app/src/Search/SearchSprunje.php index 04f04c66..e1bcd43b 100644 --- a/app/src/Search/SearchSprunje.php +++ b/app/src/Search/SearchSprunje.php @@ -49,11 +49,6 @@ public function __construct( */ public function getQuery(): Collection { - // Default version if not provided - if (!isset($this->options['version']) || $this->options['version'] === null) { - $this->options['version'] = $this->config->get('learn.versions.latest'); - } - // No version specified means no results if ($this->options['version'] === null) { return collect([]); @@ -77,18 +72,21 @@ public function getQuery(): Collection } /** - * Override validateOptions to include search-specific validation. - * - * @param mixed[] $options + * {@inheritDoc} */ - protected function validateOptions(array $options): void + protected function validateOptions(array $options): array { - // Validate query here for consistency + // Validate query length $minLength = $this->config->get('learn.search.min_length', 3); if (!is_string($options['query']) || $options['query'] === '' || mb_strlen($options['query']) < $minLength) { throw new InvalidArgumentException("Query must be at least {$minLength} characters long"); } - parent::validateOptions($options); + // Define default options + $options['version'] ??= $this->config->get('learn.versions.latest'); + $options['size'] ??= $this->config->get('learn.search.default_size', 'all'); + $options['page'] ??= $this->config->get('learn.search.default_page', null); + + return parent::validateOptions($options); } } diff --git a/app/src/Search/StaticSprunje.php b/app/src/Search/StaticSprunje.php index f474d11f..4586aecf 100644 --- a/app/src/Search/StaticSprunje.php +++ b/app/src/Search/StaticSprunje.php @@ -41,16 +41,18 @@ abstract class StaticSprunje */ protected array $columns = []; - /** - * @var string Array key for the total unfiltered object count. - */ + /** @var string Array key for the total unfiltered object count. */ protected string $countKey = 'count'; - /** - * @var string Array key for the actual result set. - */ + /** @var string Array key for the actual result set. */ protected string $rowsKey = 'rows'; + /** @var string Array key for the actual result set. */ + protected string $sizeKey = 'size'; + + /** @var string Array key for the actual result set. */ + protected string $pageKey = 'page'; + /** * Set Sprunje options. * @@ -60,7 +62,7 @@ abstract class StaticSprunje */ public function setOptions(array $options): static { - $this->validateOptions($options); + $options = $this->validateOptions($options); // @phpstan-ignore-next-line - Can't make array_replace_recursive hint at TOptions $this->options = array_replace_recursive($this->options, $options); @@ -69,13 +71,16 @@ public function setOptions(array $options): static } /** - * Validate option using Validator. + * Validate option using Validator. Can also mutate options as needed, + * e.g., setting defaults. * * @param array $options * * @throws ValidationException + * + * @return array */ - protected function validateOptions(array $options): void + protected function validateOptions(array $options): array { // Validation on input data $v = new Validator($options); @@ -88,6 +93,8 @@ protected function validateOptions(array $options): void throw $e; } + + return $options; } /** @@ -99,7 +106,7 @@ protected function validateOptions(array $options): void */ public function toResponse(ResponseInterface $response): ResponseInterface { - $payload = json_encode($this->getArray(), JSON_THROW_ON_ERROR); + $payload = json_encode($this->getArray(), JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT); $response->getBody()->write($payload); return $response->withHeader('Content-Type', 'application/json'); @@ -119,8 +126,10 @@ public function getArray(): array // Return sprunjed results return [ - $this->countKey => $count, - $this->rowsKey => $rows->values()->toArray(), + $this->countKey => $count, + $this->sizeKey => (int) $this->options['size'], + $this->pageKey => (int) $this->options['page'], + $this->rowsKey => $rows->values()->toArray(), ]; } diff --git a/app/tests/Controller/SearchControllerTest.php b/app/tests/Controller/SearchControllerTest.php index 94e4f245..aaea17e9 100644 --- a/app/tests/Controller/SearchControllerTest.php +++ b/app/tests/Controller/SearchControllerTest.php @@ -126,9 +126,17 @@ public function testSearchEndpointPagination(): void $data = json_decode($body, true); $this->assertIsArray($data); + $this->assertArrayHasKey('count', $data); + $this->assertArrayHasKey('page', $data); + $this->assertArrayHasKey('size', $data); + $this->assertArrayHasKey('rows', $data); // Should return at most 2 results $this->assertLessThanOrEqual(2, count($data['rows'])); + + // Check that page and size are correct + $this->assertSame(1, $data['page']); + $this->assertSame(2, $data['size']); } /** @@ -190,4 +198,70 @@ public function testSearchEndpointReturnsJson(): void $contentType = $response->getHeaderLine('Content-Type'); $this->assertStringContainsString('application/json', $contentType); } + + /** + * Test search API endpoint with no version and no default version in config. + */ + public function testSearchEndpointNoVersion(): void + { + // Temporarily unset the default version + /** @var Config $config */ + $config = $this->ci->get(Config::class); + $originalVersion = $config->get('learn.versions.latest'); + $config->set('learn.versions.latest', null); + + // Create request without version parameter + $request = $this->createRequest('GET', '/api/search?q=test'); + $response = $this->handleRequest($request); + + // Restore original version + $config->set('learn.versions.latest', $originalVersion); + + // Assert successful response but empty results + $this->assertResponseStatus(200, $response); + + // Parse JSON response + $body = (string) $response->getBody(); + $data = json_decode($body, true); + + $this->assertIsArray($data); + $this->assertArrayHasKey('rows', $data); + $this->assertArrayHasKey('count', $data); + + // Should have no results when version is null + $this->assertSame(0, $data['count']); + $this->assertEmpty($data['rows']); + } + + /** + * Test search API endpoint with no indexed pages for version. + */ + public function testSearchEndpointNoIndexedPages(): void + { + // Clear the index for version 6.0 + $searchIndex = $this->ci->get(SearchIndex::class); + $searchIndex->clearIndex('6.0'); + + // Create request to search + $request = $this->createRequest('GET', '/api/search?q=test&version=6.0'); + $response = $this->handleRequest($request); + + // Assert successful response but empty results + $this->assertResponseStatus(200, $response); + + // Parse JSON response + $body = (string) $response->getBody(); + $data = json_decode($body, true); + + $this->assertIsArray($data); + $this->assertArrayHasKey('rows', $data); + $this->assertArrayHasKey('count', $data); + + // Should have no results when index is empty + $this->assertSame(0, $data['count']); + $this->assertEmpty($data['rows']); + + // Rebuild index for other tests + $searchIndex->buildIndex('6.0'); + } } diff --git a/app/tests/Search/StaticSprunjeTest.php b/app/tests/Search/StaticSprunjeTest.php new file mode 100644 index 00000000..c26486be --- /dev/null +++ b/app/tests/Search/StaticSprunjeTest.php @@ -0,0 +1,334 @@ +createSprunje(); + + $sprunje->setOptions([ + 'size' => 10, + 'page' => 2, + ]); + + $result = $sprunje->getArray(); + + $this->assertSame(10, $result['size']); + $this->assertSame(2, $result['page']); + } + + public function testSetOptionsWithDefaults(): void + { + $sprunje = $this->createSprunje(); + + // Don't set any options + $result = $sprunje->getArray(); + + $this->assertSame(0, $result['size']); // 'all' converts to 0 + $this->assertSame(0, $result['page']); // null converts to 0 + } + + public function testValidateOptionsWithValidSize(): void + { + $sprunje = $this->createSprunje(); + + $sprunje->setOptions(['size' => '25']); + $result = $sprunje->getArray(); + + $this->assertSame(25, $result['size']); + } + + public function testValidateOptionsWithAllSize(): void + { + $sprunje = $this->createSprunje(); + + $sprunje->setOptions(['size' => 'all']); + $result = $sprunje->getArray(); + + // 'all' should be preserved and convert to 0 in output + $this->assertSame(0, $result['size']); + } + + public function testValidateOptionsWithInvalidSize(): void + { + $sprunje = $this->createSprunje(); + + $this->expectException(ValidationException::class); + $sprunje->setOptions(['size' => 'invalid']); + } + + public function testValidateOptionsWithValidPage(): void + { + $sprunje = $this->createSprunje(); + + $sprunje->setOptions(['page' => 3]); + $result = $sprunje->getArray(); + + $this->assertSame(3, $result['page']); + } + + public function testValidateOptionsWithInvalidPage(): void + { + $sprunje = $this->createSprunje(); + + $this->expectException(ValidationException::class); + $sprunje->setOptions(['page' => 'invalid']); + } + + public function testGetArray(): void + { + $sprunje = $this->createSprunje(); + + $result = $sprunje->getArray(); + + $this->assertIsArray($result); // @phpstan-ignore-line + $this->assertArrayHasKey('count', $result); + $this->assertArrayHasKey('size', $result); + $this->assertArrayHasKey('page', $result); + $this->assertArrayHasKey('rows', $result); + } + + public function testGetArrayWithData(): void + { + $sprunje = $this->createSprunje(); + + $result = $sprunje->getArray(); + + $this->assertSame(5, $result['count']); // Test data has 5 items + $this->assertIsArray($result['rows']); + $this->assertCount(5, $result['rows']); + } + + public function testGetModels(): void + { + $sprunje = $this->createSprunje(); + + list($count, $rows) = $sprunje->getModels(); + + $this->assertSame(5, $count); + $this->assertInstanceOf(Collection::class, $rows); // @phpstan-ignore-line + $this->assertCount(5, $rows); + } + + public function testApplyPaginationWithPageAndSize(): void + { + $sprunje = $this->createSprunje(); + + $sprunje->setOptions([ + 'size' => 2, + 'page' => 1, // Second page (0-indexed) + ]); + + $result = $sprunje->getArray(); + + // Should return 2 items starting from offset 2 + $this->assertCount(2, $result['rows']); + $this->assertSame('Item 3', $result['rows'][0]['name']); // Third item (0-indexed) + } + + public function testApplyPaginationWithFirstPage(): void + { + $sprunje = $this->createSprunje(); + + $sprunje->setOptions([ + 'size' => 2, + 'page' => 0, // First page + ]); + + $result = $sprunje->getArray(); + + $this->assertCount(2, $result['rows']); + $this->assertSame('Item 1', $result['rows'][0]['name']); + $this->assertSame('Item 2', $result['rows'][1]['name']); + } + + public function testApplyPaginationWithAllSize(): void + { + $sprunje = $this->createSprunje(); + + $sprunje->setOptions([ + 'size' => 'all', + 'page' => 0, + ]); + + $result = $sprunje->getArray(); + + // Should return all items when size is 'all' + $this->assertCount(5, $result['rows']); + } + + public function testApplyPaginationWithNullPage(): void + { + $sprunje = $this->createSprunje(); + + $sprunje->setOptions([ + 'size' => 2, + 'page' => null, + ]); + + $result = $sprunje->getArray(); + + // Should return all items when page is null + $this->assertCount(5, $result['rows']); + } + + public function testSetColumns(): void + { + $sprunje = $this->createSprunje(); + + $sprunje->setColumns(['name']); + + // Columns don't affect static collections (no select() support) + // but we can verify the method works + $result = $sprunje->getArray(); + + $this->assertIsArray($result['rows']); + } + + public function testApplyTransformations(): void + { + $sprunje = new TestableStaticSprunjeWithTransformation(); + + $result = $sprunje->getArray(); + + // Transformation adds '_transformed' to each name + $this->assertStringEndsWith('_transformed', $result['rows'][0]['name']); + } + + public function testCount(): void + { + $sprunje = $this->createSprunje(); + + $result = $sprunje->getArray(); + + $this->assertSame(5, $result['count']); + } + + public function testToResponse(): void + { + $sprunje = $this->createSprunje(); + + // Create a response using Slim's ResponseFactory + $responseFactory = new \Slim\Psr7\Factory\ResponseFactory(); + $response = $responseFactory->createResponse(); + + $response = $sprunje->toResponse($response); + + $this->assertInstanceOf(ResponseInterface::class, $response); // @phpstan-ignore-line + $this->assertTrue($response->hasHeader('Content-Type')); + $this->assertStringContainsString('application/json', $response->getHeaderLine('Content-Type')); + + // Verify JSON is valid - need to rewind the stream after writing + $response->getBody()->rewind(); + $body = (string) $response->getBody(); + $data = json_decode($body, true); + + $this->assertIsArray($data); + $this->assertArrayHasKey('count', $data); + $this->assertArrayHasKey('rows', $data); + } + + public function testToResponseWithPrettyPrint(): void + { + $sprunje = $this->createSprunje(); + + // Create a response using Slim's ResponseFactory + $responseFactory = new \Slim\Psr7\Factory\ResponseFactory(); + $response = $responseFactory->createResponse(); + $response = $sprunje->toResponse($response); + + // Rewind the stream after writing + $response->getBody()->rewind(); + $body = (string) $response->getBody(); + + // Should have pretty print formatting (newlines and indentation) + $this->assertStringContainsString("\n", $body); + } + + public function testPaginationBeyondAvailableItems(): void + { + $sprunje = $this->createSprunje(); + + $sprunje->setOptions([ + 'size' => 2, + 'page' => 10, // Way beyond available items + ]); + + $result = $sprunje->getArray(); + + // Should return empty rows when page is beyond available items + $this->assertEmpty($result['rows']); + $this->assertSame(5, $result['count']); // Total count should still be 5 + } +} + +/** + * Concrete implementation of StaticSprunje for testing. + * + * @extends StaticSprunje + */ +class TestableStaticSprunje extends StaticSprunje +{ + public function getQuery(): Collection + { + // Return a simple collection for testing + return collect([ + ['id' => 1, 'name' => 'Item 1'], + ['id' => 2, 'name' => 'Item 2'], + ['id' => 3, 'name' => 'Item 3'], + ['id' => 4, 'name' => 'Item 4'], + ['id' => 5, 'name' => 'Item 5'], + ]); + } +} + +/** + * Concrete implementation with custom transformation for testing. + */ +class TestableStaticSprunjeWithTransformation extends TestableStaticSprunje +{ + protected function applyTransformations(Collection $collection): Collection + { + return $collection->map(function ($item) { + $item['name'] .= '_transformed'; + + return $item; + }); + } +} From 2def405b63b8548d286257b0bef82acaa844edec Mon Sep 17 00:00:00 2001 From: Louis Charette Date: Sun, 25 Jan 2026 11:03:12 -0500 Subject: [PATCH 15/20] Replace options magic keys with proper getters and setters --- app/config/default.php | 1 - app/src/Controller/SearchController.php | 23 +++--- app/src/Search/SearchSprunje.php | 89 +++++++++++++------- app/src/Search/StaticSprunje.php | 91 +++++++++++--------- app/tests/Search/SearchServiceTest.php | 11 ++- app/tests/Search/StaticSprunjeTest.php | 105 +++++++++++------------- 6 files changed, 179 insertions(+), 141 deletions(-) diff --git a/app/config/default.php b/app/config/default.php index 55bffbe4..d000879c 100644 --- a/app/config/default.php +++ b/app/config/default.php @@ -56,7 +56,6 @@ ], 'search' => [ 'min_length' => 3, // Minimum length of search query - 'default_page' => 0, // Default page number for paginated results 'default_size' => 10, // Default number of results per page 'snippet_length' => 150, // Length of content snippets in results 'max_results' => 150, // Maximum number of results to consider for pagination diff --git a/app/src/Controller/SearchController.php b/app/src/Controller/SearchController.php index 535b95ac..e47a979b 100644 --- a/app/src/Controller/SearchController.php +++ b/app/src/Controller/SearchController.php @@ -14,8 +14,6 @@ use Psr\Http\Message\ResponseInterface as Response; use Psr\Http\Message\ServerRequestInterface as Request; -use UserFrosting\Config\Config; -use UserFrosting\Learn\Search\SearchService; use UserFrosting\Learn\Search\SearchSprunje; /** @@ -24,8 +22,6 @@ class SearchController { public function __construct( - protected SearchService $searchService, - protected Config $config, protected SearchSprunje $sprunje, ) { } @@ -36,8 +32,9 @@ public function __construct( * * Query parameters: * - q: Search query (required, min length from config) - * - page: Page number for pagination (optional, from config) - * - size: Number of results per page (optional, from config, max from config) + * - page: Page number for pagination (optional, default 1) + * - size: Number of results per page (optional, default from config, null means all results) + * - version: Documentation version (optional, defaults to latest) * * @param Request $request * @param Response $response @@ -46,11 +43,15 @@ public function search(Request $request, Response $response): Response { $params = $request->getQueryParams(); - $this->sprunje->setOptions([ - 'query' => $params['q'] ?? '', - 'page' => $params['page'] ?? null, - 'size' => $params['size'] ?? null, - ]); + $this->sprunje + ->setQuery($params['q'] ?? '') + ->setVersion($params['version'] ?? null) + ->setPage((int) ($params['page'] ?? 1)); + + // Only set size if explicitly provided + if (isset($params['size'])) { + $this->sprunje->setSize((int) $params['size']); + } return $this->sprunje->toResponse($response); } diff --git a/app/src/Search/SearchSprunje.php b/app/src/Search/SearchSprunje.php index e1bcd43b..29f451cf 100644 --- a/app/src/Search/SearchSprunje.php +++ b/app/src/Search/SearchSprunje.php @@ -26,20 +26,72 @@ * @phpstan-import-type IndexedPage from IndexedPageShape * @phpstan-import-type SearchResult from IndexedPageShape * - * @extends StaticSprunje + * @extends StaticSprunje */ class SearchSprunje extends StaticSprunje { + /** @var string|null Documentation version to search */ + protected ?string $version = null; + + /** @var string Search query */ + protected string $query = ''; + public function __construct( protected SearchService $searchService, protected SearchIndex $searchIndex, protected Config $config ) { + // Set default size from config + $defaultSize = $this->config->getInt('learn.search.default_size'); + if ($defaultSize !== null) { + $this->size = $defaultSize; + } + } + + /** + * Set the search query. + * + * @param string $query Search query + * + * @throws InvalidArgumentException + * + * @return static + */ + public function setQuery(string $query): static + { + // Validate query length + $minLength = $this->config->getInt('learn.search.min_length', 3); + if ($query === '' || mb_strlen($query) < $minLength) { + throw new InvalidArgumentException("Query must be at least {$minLength} characters long"); + } + + $this->query = $query; + + return $this; + } + + /** + * Set the documentation version. + * + * @param string|null $version Documentation version + * + * @return static + */ + public function setVersion(?string $version): static + { + $this->version = $version ?? $this->config->get('learn.versions.latest'); + + return $this; + } + + /** + * Get the documentation version. + * + * @return string|null + */ + public function getVersion(): ?string + { + return $this->version; } /** @@ -50,12 +102,12 @@ public function __construct( public function getQuery(): Collection { // No version specified means no results - if ($this->options['version'] === null) { + if ($this->version === null) { return collect([]); } // Get the index from cache - $index = $this->searchIndex->getIndex($this->options['version']); + $index = $this->searchIndex->getIndex($this->version); // No indexed pages means no results if (count($index) === 0) { @@ -63,30 +115,11 @@ public function getQuery(): Collection } // Search through the index (without pagination - Sprunje handles that) - $results = $this->searchService->performSearch($this->options['query'], $index); + $results = $this->searchService->performSearch($this->query, $index); // Convert to Collection for compatibility $collection = collect($results); return $collection; } - - /** - * {@inheritDoc} - */ - protected function validateOptions(array $options): array - { - // Validate query length - $minLength = $this->config->get('learn.search.min_length', 3); - if (!is_string($options['query']) || $options['query'] === '' || mb_strlen($options['query']) < $minLength) { - throw new InvalidArgumentException("Query must be at least {$minLength} characters long"); - } - - // Define default options - $options['version'] ??= $this->config->get('learn.versions.latest'); - $options['size'] ??= $this->config->get('learn.search.default_size', 'all'); - $options['page'] ??= $this->config->get('learn.search.default_page', null); - - return parent::validateOptions($options); - } } diff --git a/app/src/Search/StaticSprunje.php b/app/src/Search/StaticSprunje.php index 4586aecf..f0b681a0 100644 --- a/app/src/Search/StaticSprunje.php +++ b/app/src/Search/StaticSprunje.php @@ -15,26 +15,19 @@ use Illuminate\Support\Collection; use Psr\Http\Message\ResponseInterface; use UserFrosting\Sprinkle\Core\Exceptions\ValidationException; -use Valitron\Validator; /** * Implements a simple version of Sprunje for paginating a static collection. * - * @template TOptions of array{ - * size: string|int|null, - * page: string|int|null, - * } * @template TItem */ abstract class StaticSprunje { - /** - * @var TOptions Default HTTP request parameters. - */ - protected array $options = [ - 'size' => 'all', - 'page' => null, - ]; + /** @var int|null Number of results per page. Null means return all results. */ + protected ?int $size = null; + + /** @var int Page number (1-based). */ + protected int $page = 1; /** * @var string[] Fields to show in output. Empty array will load all. @@ -54,47 +47,69 @@ abstract class StaticSprunje protected string $pageKey = 'page'; /** - * Set Sprunje options. + * Set the page size. * - * @param array $options Partial TOptions + * @param int|null $size Number of results per page, or null for all results + * + * @throws ValidationException * * @return static */ - public function setOptions(array $options): static + public function setSize(?int $size): static { - $options = $this->validateOptions($options); + if ($size !== null && $size < 1) { + $e = new ValidationException(); + $e->addErrors(['size' => ['Size must be null or at least 1']]); + + throw $e; + } - // @phpstan-ignore-next-line - Can't make array_replace_recursive hint at TOptions - $this->options = array_replace_recursive($this->options, $options); + $this->size = $size; return $this; } /** - * Validate option using Validator. Can also mutate options as needed, - * e.g., setting defaults. + * Get the page size. + * + * @return int|null + */ + public function getSize(): ?int + { + return $this->size; + } + + /** + * Set the page number (1-based). * - * @param array $options + * @param int $page Page number * * @throws ValidationException * - * @return array + * @return static */ - protected function validateOptions(array $options): array + public function setPage(int $page): static { - // Validation on input data - $v = new Validator($options); - $v->rule('regex', 'size', '/all|[0-9]+/i'); - $v->rule('integer', 'page'); - - if (!$v->validate()) { + if ($page < 1) { $e = new ValidationException(); - $e->addErrors($v->errors()); // @phpstan-ignore-line errors returns array with no arguments + $e->addErrors(['page' => ['Page must be at least 1']]); throw $e; } - return $options; + $this->page = $page; + + return $this; + } + + /** + * Get the page number. + * + * @return int + */ + public function getPage(): int + { + return $this->page; } /** @@ -127,8 +142,8 @@ public function getArray(): array // Return sprunjed results return [ $this->countKey => $count, - $this->sizeKey => (int) $this->options['size'], - $this->pageKey => (int) $this->options['page'], + $this->sizeKey => $this->size ?? 0, + $this->pageKey => $this->page, $this->rowsKey => $rows->values()->toArray(), ]; } @@ -179,12 +194,10 @@ abstract public function getQuery(): Collection; */ public function applyPagination(Collection $query): Collection { - $page = $this->options['page']; - $size = $this->options['size']; - - if (!is_null($page) && !is_null($size) && $size !== 'all') { - $offset = (int) $size * (int) $page; - $query = $query->skip($offset)->take((int) $size); + if ($this->size !== null) { + // Page is 1-based, so subtract 1 for offset calculation + $offset = $this->size * ($this->page - 1); + $query = $query->skip($offset)->take($this->size); } return $query; diff --git a/app/tests/Search/SearchServiceTest.php b/app/tests/Search/SearchServiceTest.php index a77cad8f..936dfcd3 100644 --- a/app/tests/Search/SearchServiceTest.php +++ b/app/tests/Search/SearchServiceTest.php @@ -101,12 +101,11 @@ public function testSearchPagination(): void $searchSprunje = $this->ci->get(SearchSprunje::class); // Search with pagination via Sprunje - $searchSprunje->setOptions([ - 'query' => 'page', - 'version' => '6.0', - 'size' => 2, - 'page' => 0, - ]); + $searchSprunje + ->setQuery('page') + ->setVersion('6.0') + ->setSize(2) + ->setPage(1); $result = $searchSprunje->getArray(); diff --git a/app/tests/Search/StaticSprunjeTest.php b/app/tests/Search/StaticSprunjeTest.php index c26486be..581e644c 100644 --- a/app/tests/Search/StaticSprunjeTest.php +++ b/app/tests/Search/StaticSprunjeTest.php @@ -32,14 +32,11 @@ protected function createSprunje(): TestableStaticSprunje return new TestableStaticSprunje(); } - public function testSetOptions(): void + public function testSetSizeAndPage(): void { $sprunje = $this->createSprunje(); - $sprunje->setOptions([ - 'size' => 10, - 'page' => 2, - ]); + $sprunje->setSize(10)->setPage(2); $result = $sprunje->getArray(); @@ -47,62 +44,88 @@ public function testSetOptions(): void $this->assertSame(2, $result['page']); } - public function testSetOptionsWithDefaults(): void + public function testGettersAndSetters(): void + { + $sprunje = $this->createSprunje(); + + $sprunje->setSize(25)->setPage(3); + + $this->assertSame(25, $sprunje->getSize()); + $this->assertSame(3, $sprunje->getPage()); + } + + public function testDefaultValues(): void { $sprunje = $this->createSprunje(); // Don't set any options $result = $sprunje->getArray(); - $this->assertSame(0, $result['size']); // 'all' converts to 0 - $this->assertSame(0, $result['page']); // null converts to 0 + $this->assertSame(0, $result['size']); // null converts to 0 + $this->assertSame(1, $result['page']); // Default is 1 } - public function testValidateOptionsWithValidSize(): void + public function testSetSizeWithValidInteger(): void { $sprunje = $this->createSprunje(); - $sprunje->setOptions(['size' => '25']); + $sprunje->setSize(25); $result = $sprunje->getArray(); $this->assertSame(25, $result['size']); } - public function testValidateOptionsWithAllSize(): void + public function testSetSizeWithNull(): void { $sprunje = $this->createSprunje(); - $sprunje->setOptions(['size' => 'all']); + $sprunje->setSize(null); $result = $sprunje->getArray(); - // 'all' should be preserved and convert to 0 in output + // null should convert to 0 in output (meaning all results) $this->assertSame(0, $result['size']); } - public function testValidateOptionsWithInvalidSize(): void + public function testSetSizeWithInvalidValue(): void { $sprunje = $this->createSprunje(); $this->expectException(ValidationException::class); - $sprunje->setOptions(['size' => 'invalid']); + $sprunje->setSize(0); } - public function testValidateOptionsWithValidPage(): void + public function testSetSizeWithNegativeValue(): void { $sprunje = $this->createSprunje(); - $sprunje->setOptions(['page' => 3]); + $this->expectException(ValidationException::class); + $sprunje->setSize(-1); + } + + public function testSetPageWithValidInteger(): void + { + $sprunje = $this->createSprunje(); + + $sprunje->setPage(3); $result = $sprunje->getArray(); $this->assertSame(3, $result['page']); } - public function testValidateOptionsWithInvalidPage(): void + public function testSetPageWithInvalidValue(): void { $sprunje = $this->createSprunje(); $this->expectException(ValidationException::class); - $sprunje->setOptions(['page' => 'invalid']); + $sprunje->setPage(0); + } + + public function testSetPageWithNegativeValue(): void + { + $sprunje = $this->createSprunje(); + + $this->expectException(ValidationException::class); + $sprunje->setPage(-1); } public function testGetArray(): void @@ -144,26 +167,20 @@ public function testApplyPaginationWithPageAndSize(): void { $sprunje = $this->createSprunje(); - $sprunje->setOptions([ - 'size' => 2, - 'page' => 1, // Second page (0-indexed) - ]); + $sprunje->setSize(2)->setPage(2); // Second page (1-based) $result = $sprunje->getArray(); // Should return 2 items starting from offset 2 $this->assertCount(2, $result['rows']); - $this->assertSame('Item 3', $result['rows'][0]['name']); // Third item (0-indexed) + $this->assertSame('Item 3', $result['rows'][0]['name']); // Third item (0-indexed array) } public function testApplyPaginationWithFirstPage(): void { $sprunje = $this->createSprunje(); - $sprunje->setOptions([ - 'size' => 2, - 'page' => 0, // First page - ]); + $sprunje->setSize(2)->setPage(1); // First page (1-based) $result = $sprunje->getArray(); @@ -172,33 +189,15 @@ public function testApplyPaginationWithFirstPage(): void $this->assertSame('Item 2', $result['rows'][1]['name']); } - public function testApplyPaginationWithAllSize(): void - { - $sprunje = $this->createSprunje(); - - $sprunje->setOptions([ - 'size' => 'all', - 'page' => 0, - ]); - - $result = $sprunje->getArray(); - - // Should return all items when size is 'all' - $this->assertCount(5, $result['rows']); - } - - public function testApplyPaginationWithNullPage(): void + public function testApplyPaginationWithNullSize(): void { $sprunje = $this->createSprunje(); - $sprunje->setOptions([ - 'size' => 2, - 'page' => null, - ]); + $sprunje->setSize(null)->setPage(1); $result = $sprunje->getArray(); - // Should return all items when page is null + // Should return all items when size is null $this->assertCount(5, $result['rows']); } @@ -279,10 +278,7 @@ public function testPaginationBeyondAvailableItems(): void { $sprunje = $this->createSprunje(); - $sprunje->setOptions([ - 'size' => 2, - 'page' => 10, // Way beyond available items - ]); + $sprunje->setSize(2)->setPage(10); // Way beyond available items $result = $sprunje->getArray(); @@ -296,9 +292,6 @@ public function testPaginationBeyondAvailableItems(): void * Concrete implementation of StaticSprunje for testing. * * @extends StaticSprunje From 47bd291a96a604aec54e04d0952806cf98ab3214 Mon Sep 17 00:00:00 2001 From: Louis Charette Date: Sun, 25 Jan 2026 11:09:14 -0500 Subject: [PATCH 16/20] Refactor StaticSprunje: Remove getArray, getModels -> getResultSet and getQuery -> getItems --- app/src/Search/SearchSprunje.php | 4 +- app/src/Search/StaticSprunje.php | 70 +++++++++++--------------- app/tests/Search/SearchServiceTest.php | 2 +- app/tests/Search/StaticSprunjeTest.php | 45 +++++++---------- 4 files changed, 48 insertions(+), 73 deletions(-) diff --git a/app/src/Search/SearchSprunje.php b/app/src/Search/SearchSprunje.php index 29f451cf..1cb63973 100644 --- a/app/src/Search/SearchSprunje.php +++ b/app/src/Search/SearchSprunje.php @@ -95,11 +95,11 @@ public function getVersion(): ?string } /** - * Get the underlying queryable object in its current state. + * Get the base collection of items to process. * * @return Collection */ - public function getQuery(): Collection + public function getItems(): Collection { // No version specified means no results if ($this->version === null) { diff --git a/app/src/Search/StaticSprunje.php b/app/src/Search/StaticSprunje.php index f0b681a0..04f55ee9 100644 --- a/app/src/Search/StaticSprunje.php +++ b/app/src/Search/StaticSprunje.php @@ -121,86 +121,72 @@ public function getPage(): int */ public function toResponse(ResponseInterface $response): ResponseInterface { - $payload = json_encode($this->getArray(), JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT); + $payload = json_encode($this->getResultSet(), JSON_THROW_ON_ERROR | JSON_PRETTY_PRINT); $response->getBody()->write($payload); return $response->withHeader('Content-Type', 'application/json'); } /** - * Executes the sprunje query, applying all sorts, filters, and pagination. + * Execute processing to get the complete result set with metadata. * - * Returns an array containing `count` (the total number of rows, before filtering), - * and `rows` (the filtered result set). + * Returns an array containing `count` (total items), `size` (page size), + * `page` (current page), and `rows` (the filtered result set). * * @return array */ - public function getArray(): array + public function getResultSet(): array { - list($count, $rows) = $this->getModels(); - - // Return sprunjed results - return [ - $this->countKey => $count, - $this->sizeKey => $this->size ?? 0, - $this->pageKey => $this->page, - $this->rowsKey => $rows->values()->toArray(), - ]; - } - - /** - * Executes the sprunje query, applying all sorts, filters, and pagination. - * - * Returns the filtered, paginated result set and the counts. - * - * @return array{int, Collection} - */ - public function getModels(): array - { - $query = $this->getQuery(); + $collection = $this->getItems(); // Count unfiltered total - $count = $this->count($query); + $count = $this->count($collection); // Paginate - $query = $this->applyPagination($query); + $collection = $this->applyPagination($collection); // Execute query - only apply select if not wildcard/empty if ($this->columns !== []) { - $query = $query->select($this->columns); // @phpstan-ignore-line + $collection = $collection->select($this->columns); // @phpstan-ignore-line } - $query = collect($query); + $collection = collect($collection); // Perform any additional transformations on the dataset - $query = $this->applyTransformations($query); + $collection = $this->applyTransformations($collection); - return [$count, $query]; + // Return complete result set with metadata + return [ + $this->countKey => $count, + $this->sizeKey => $this->size ?? 0, + $this->pageKey => $this->page, + $this->rowsKey => $collection->values()->toArray(), + ]; } /** - * Get the underlying queryable object in its current state. + * Get the base collection of items to process. * * @return Collection */ - abstract public function getQuery(): Collection; + abstract public function getItems(): Collection; /** * Apply pagination based on the `page` and `size` options. * - * @param Collection $query + * @param Collection $collection * * @return Collection */ - public function applyPagination(Collection $query): Collection + public function applyPagination(Collection $collection): Collection { if ($this->size !== null) { // Page is 1-based, so subtract 1 for offset calculation $offset = $this->size * ($this->page - 1); - $query = $query->skip($offset)->take($this->size); + $collection = $collection->skip($offset)->take($this->size); } - return $query; + return $collection; } /** @@ -231,14 +217,14 @@ protected function applyTransformations(Collection $collection): Collection } /** - * Get the unpaginated count of items (before filtering) in this query. + * Get the unpaginated count of items in the collection. * - * @param Collection $query + * @param Collection $collection * * @return int */ - protected function count(Collection $query): int + protected function count(Collection $collection): int { - return $query->count(); + return $collection->count(); } } diff --git a/app/tests/Search/SearchServiceTest.php b/app/tests/Search/SearchServiceTest.php index 936dfcd3..14d9d01b 100644 --- a/app/tests/Search/SearchServiceTest.php +++ b/app/tests/Search/SearchServiceTest.php @@ -107,7 +107,7 @@ public function testSearchPagination(): void ->setSize(2) ->setPage(1); - $result = $searchSprunje->getArray(); + $result = $searchSprunje->getResultSet(); $this->assertArrayHasKey('rows', $result); } diff --git a/app/tests/Search/StaticSprunjeTest.php b/app/tests/Search/StaticSprunjeTest.php index 581e644c..0e80e562 100644 --- a/app/tests/Search/StaticSprunjeTest.php +++ b/app/tests/Search/StaticSprunjeTest.php @@ -38,7 +38,7 @@ public function testSetSizeAndPage(): void $sprunje->setSize(10)->setPage(2); - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); $this->assertSame(10, $result['size']); $this->assertSame(2, $result['page']); @@ -59,7 +59,7 @@ public function testDefaultValues(): void $sprunje = $this->createSprunje(); // Don't set any options - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); $this->assertSame(0, $result['size']); // null converts to 0 $this->assertSame(1, $result['page']); // Default is 1 @@ -70,7 +70,7 @@ public function testSetSizeWithValidInteger(): void $sprunje = $this->createSprunje(); $sprunje->setSize(25); - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); $this->assertSame(25, $result['size']); } @@ -80,7 +80,7 @@ public function testSetSizeWithNull(): void $sprunje = $this->createSprunje(); $sprunje->setSize(null); - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); // null should convert to 0 in output (meaning all results) $this->assertSame(0, $result['size']); @@ -107,7 +107,7 @@ public function testSetPageWithValidInteger(): void $sprunje = $this->createSprunje(); $sprunje->setPage(3); - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); $this->assertSame(3, $result['page']); } @@ -128,11 +128,11 @@ public function testSetPageWithNegativeValue(): void $sprunje->setPage(-1); } - public function testGetArray(): void + public function testGetResultSet(): void { $sprunje = $this->createSprunje(); - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); $this->assertIsArray($result); // @phpstan-ignore-line $this->assertArrayHasKey('count', $result); @@ -141,35 +141,24 @@ public function testGetArray(): void $this->assertArrayHasKey('rows', $result); } - public function testGetArrayWithData(): void + public function testGetResultSetWithData(): void { $sprunje = $this->createSprunje(); - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); $this->assertSame(5, $result['count']); // Test data has 5 items $this->assertIsArray($result['rows']); $this->assertCount(5, $result['rows']); } - public function testGetModels(): void - { - $sprunje = $this->createSprunje(); - - list($count, $rows) = $sprunje->getModels(); - - $this->assertSame(5, $count); - $this->assertInstanceOf(Collection::class, $rows); // @phpstan-ignore-line - $this->assertCount(5, $rows); - } - public function testApplyPaginationWithPageAndSize(): void { $sprunje = $this->createSprunje(); $sprunje->setSize(2)->setPage(2); // Second page (1-based) - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); // Should return 2 items starting from offset 2 $this->assertCount(2, $result['rows']); @@ -182,7 +171,7 @@ public function testApplyPaginationWithFirstPage(): void $sprunje->setSize(2)->setPage(1); // First page (1-based) - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); $this->assertCount(2, $result['rows']); $this->assertSame('Item 1', $result['rows'][0]['name']); @@ -195,7 +184,7 @@ public function testApplyPaginationWithNullSize(): void $sprunje->setSize(null)->setPage(1); - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); // Should return all items when size is null $this->assertCount(5, $result['rows']); @@ -209,7 +198,7 @@ public function testSetColumns(): void // Columns don't affect static collections (no select() support) // but we can verify the method works - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); $this->assertIsArray($result['rows']); } @@ -218,7 +207,7 @@ public function testApplyTransformations(): void { $sprunje = new TestableStaticSprunjeWithTransformation(); - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); // Transformation adds '_transformed' to each name $this->assertStringEndsWith('_transformed', $result['rows'][0]['name']); @@ -228,7 +217,7 @@ public function testCount(): void { $sprunje = $this->createSprunje(); - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); $this->assertSame(5, $result['count']); } @@ -280,7 +269,7 @@ public function testPaginationBeyondAvailableItems(): void $sprunje->setSize(2)->setPage(10); // Way beyond available items - $result = $sprunje->getArray(); + $result = $sprunje->getResultSet(); // Should return empty rows when page is beyond available items $this->assertEmpty($result['rows']); @@ -298,7 +287,7 @@ public function testPaginationBeyondAvailableItems(): void */ class TestableStaticSprunje extends StaticSprunje { - public function getQuery(): Collection + public function getItems(): Collection { // Return a simple collection for testing return collect([ From baa146ced58b54161b70bcf6647439e0b7d6be67 Mon Sep 17 00:00:00 2001 From: Louis Charette Date: Sun, 25 Jan 2026 16:08:13 -0500 Subject: [PATCH 17/20] Replace magic array with objects --- app/src/Search/IndexedPage.php | 30 +++++++++++++++++ app/src/Search/IndexedPageShape.php | 45 ------------------------- app/src/Search/SearchIndex.php | 22 ++++++------ app/src/Search/SearchResult.php | 46 ++++++++++++++++++++++++++ app/src/Search/SearchService.php | 45 ++++++++++++------------- app/src/Search/SearchSprunje.php | 3 -- app/tests/Search/SearchIndexTest.php | 17 +++++----- app/tests/Search/SearchServiceTest.php | 21 ++++++------ 8 files changed, 126 insertions(+), 103 deletions(-) create mode 100644 app/src/Search/IndexedPage.php delete mode 100644 app/src/Search/IndexedPageShape.php create mode 100644 app/src/Search/SearchResult.php diff --git a/app/src/Search/IndexedPage.php b/app/src/Search/IndexedPage.php new file mode 100644 index 00000000..2057db8b --- /dev/null +++ b/app/src/Search/IndexedPage.php @@ -0,0 +1,30 @@ +getContent(); @@ -164,15 +162,15 @@ protected function indexPage(PageResource $page): array } $metadataString = implode(' ', $metadata); - return [ - 'title' => $page->getTitle(), - 'slug' => $page->getSlug(), - 'route' => $page->getRoute(), - 'content' => $plainText, - 'version' => $page->getVersion()->id, - 'keywords' => $keywords, - 'metadata' => $metadataString, - ]; + return new IndexedPage( + title: $page->getTitle(), + slug: $page->getSlug(), + route: $page->getRoute(), + content: $plainText, + version: $page->getVersion()->id, + keywords: $keywords, + metadata: $metadataString, + ); } /** diff --git a/app/src/Search/SearchResult.php b/app/src/Search/SearchResult.php new file mode 100644 index 00000000..e4405cc4 --- /dev/null +++ b/app/src/Search/SearchResult.php @@ -0,0 +1,46 @@ + + */ + public function jsonSerialize(): array + { + return [ + 'title' => $this->title, + 'slug' => $this->slug, + 'route' => $this->route, + 'snippet' => $this->snippet, + 'matches' => $this->matches, + 'version' => $this->version, + ]; + } +} diff --git a/app/src/Search/SearchService.php b/app/src/Search/SearchService.php index 4a00fa4d..7617e682 100644 --- a/app/src/Search/SearchService.php +++ b/app/src/Search/SearchService.php @@ -22,9 +22,6 @@ * - Wildcard pattern matching * - Snippet extraction with context * - Result caching - * - * @phpstan-import-type IndexedPage from IndexedPageShape - * @phpstan-import-type SearchResult from IndexedPageShape */ class SearchService { @@ -71,15 +68,15 @@ public function performSearch(string $query, array $index): array // Search in different fields with priority if ($hasWildcards) { - $titleMatches = $this->searchWithWildcard($wildcardRegex, $page['title']); - $keywordMatches = $this->searchWithWildcard($wildcardRegex, $page['keywords']); - $metadataMatches = $this->searchWithWildcard($wildcardRegex, $page['metadata']); - $contentMatches = $this->searchWithWildcard($wildcardRegex, $page['content']); + $titleMatches = $this->searchWithWildcard($wildcardRegex, $page->title); + $keywordMatches = $this->searchWithWildcard($wildcardRegex, $page->keywords); + $metadataMatches = $this->searchWithWildcard($wildcardRegex, $page->metadata); + $contentMatches = $this->searchWithWildcard($wildcardRegex, $page->content); } else { - $titleMatches = $this->searchPlain($query, $page['title']); - $keywordMatches = $this->searchPlain($query, $page['keywords']); - $metadataMatches = $this->searchPlain($query, $page['metadata']); - $contentMatches = $this->searchPlain($query, $page['content']); + $titleMatches = $this->searchPlain($query, $page->title); + $keywordMatches = $this->searchPlain($query, $page->keywords); + $metadataMatches = $this->searchPlain($query, $page->metadata); + $contentMatches = $this->searchPlain($query, $page->content); } // Calculate weighted score: title > keywords > metadata > content @@ -90,31 +87,31 @@ public function performSearch(string $query, array $index): array $snippetPosition = 0; if (count($titleMatches) > 0) { $snippetPosition = $titleMatches[0]; - $snippetContent = $page['title']; + $snippetContent = $page->title; } elseif (count($keywordMatches) > 0) { $snippetPosition = $keywordMatches[0]; - $snippetContent = $page['keywords']; + $snippetContent = $page->keywords; } elseif (count($metadataMatches) > 0) { $snippetPosition = $metadataMatches[0]; - $snippetContent = $page['metadata']; + $snippetContent = $page->metadata; } else { $snippetPosition = $contentMatches[0]; - $snippetContent = $page['content']; + $snippetContent = $page->content; } - $results[] = [ - 'title' => $page['title'], - 'slug' => $page['slug'], - 'route' => $page['route'], - 'snippet' => $this->generateSnippet($snippetContent, $snippetPosition), - 'matches' => $score, - 'version' => $page['version'], - ]; + $results[] = new SearchResult( + title: $page->title, + slug: $page->slug, + route: $page->route, + snippet: $this->generateSnippet($snippetContent, $snippetPosition), + matches: $score, + version: $page->version, + ); } } // Sort by weighted score (descending) - usort($results, fn ($a, $b) => $b['matches'] <=> $a['matches']); + usort($results, fn ($a, $b) => $b->matches <=> $a->matches); $maxResults = $this->config->get('learn.search.max_results', 1000); diff --git a/app/src/Search/SearchSprunje.php b/app/src/Search/SearchSprunje.php index 1cb63973..ebdbc6f2 100644 --- a/app/src/Search/SearchSprunje.php +++ b/app/src/Search/SearchSprunje.php @@ -23,9 +23,6 @@ * Provides a Sprunje-compatible interface for searching documentation pages. * Adapts the SearchService to work with the Sprunje API. * - * @phpstan-import-type IndexedPage from IndexedPageShape - * @phpstan-import-type SearchResult from IndexedPageShape - * * @extends StaticSprunje */ class SearchSprunje extends StaticSprunje diff --git a/app/tests/Search/SearchIndexTest.php b/app/tests/Search/SearchIndexTest.php index 457e5ed6..7c730a35 100644 --- a/app/tests/Search/SearchIndexTest.php +++ b/app/tests/Search/SearchIndexTest.php @@ -99,17 +99,16 @@ public function testIndexPageContent(): void // Check first page structure $firstPage = $index[0]; - $this->assertArrayHasKey('title', $firstPage); - $this->assertArrayHasKey('slug', $firstPage); - $this->assertArrayHasKey('route', $firstPage); - $this->assertArrayHasKey('content', $firstPage); - $this->assertArrayHasKey('version', $firstPage); - $this->assertArrayHasKey('keywords', $firstPage); - $this->assertArrayHasKey('metadata', $firstPage); + $this->assertInstanceOf(\UserFrosting\Learn\Search\IndexedPage::class, $firstPage); + $this->assertNotEmpty($firstPage->title); + $this->assertNotEmpty($firstPage->slug); + $this->assertNotEmpty($firstPage->route); + $this->assertNotEmpty($firstPage->content); + $this->assertNotEmpty($firstPage->version); // Content should be plain text (no HTML tags) - $this->assertStringNotContainsString('<', $firstPage['content']); - $this->assertStringNotContainsString('>', $firstPage['content']); + $this->assertStringNotContainsString('<', $firstPage->content); + $this->assertStringNotContainsString('>', $firstPage->content); } public function testStripHtmlTags(): void diff --git a/app/tests/Search/SearchServiceTest.php b/app/tests/Search/SearchServiceTest.php index 14d9d01b..26f2c7b2 100644 --- a/app/tests/Search/SearchServiceTest.php +++ b/app/tests/Search/SearchServiceTest.php @@ -63,12 +63,13 @@ public function testSearchWithPlainText(): void // Check structure of first result $firstResult = $results[0]; - $this->assertArrayHasKey('title', $firstResult); - $this->assertArrayHasKey('slug', $firstResult); - $this->assertArrayHasKey('route', $firstResult); - $this->assertArrayHasKey('snippet', $firstResult); - $this->assertArrayHasKey('matches', $firstResult); - $this->assertArrayHasKey('version', $firstResult); + $this->assertInstanceOf(\UserFrosting\Learn\Search\SearchResult::class, $firstResult); + $this->assertNotEmpty($firstResult->title); + $this->assertNotEmpty($firstResult->slug); + $this->assertNotEmpty($firstResult->route); + $this->assertNotEmpty($firstResult->snippet); + $this->assertGreaterThan(0, $firstResult->matches); + $this->assertNotEmpty($firstResult->version); } public function testSearchWithEmptyQuery(): void @@ -122,8 +123,8 @@ public function testSearchResultSnippet(): void if (count($results) > 0) { $firstResult = $results[0]; - $this->assertIsString($firstResult['snippet']); - $this->assertNotEmpty($firstResult['snippet']); + $this->assertIsString($firstResult->snippet); + $this->assertNotEmpty($firstResult->snippet); } } @@ -188,8 +189,8 @@ public function testSearchResultSorting(): void if (count($results) > 1) { // Verify results are sorted by number of matches (descending) - $firstMatches = $results[0]['matches']; - $lastMatches = $results[count($results) - 1]['matches']; + $firstMatches = $results[0]->matches; + $lastMatches = $results[count($results) - 1]->matches; $this->assertGreaterThanOrEqual($lastMatches, $firstMatches); } From 3bd361e9c0561440e83695b4aefcc1a9d11ff276 Mon Sep 17 00:00:00 2001 From: Louis Charette Date: Sun, 25 Jan 2026 16:24:18 -0500 Subject: [PATCH 18/20] Index should be fetched if cache is empty --- app/src/Search/SearchIndex.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/src/Search/SearchIndex.php b/app/src/Search/SearchIndex.php index 4abe9788..fc2452e9 100644 --- a/app/src/Search/SearchIndex.php +++ b/app/src/Search/SearchIndex.php @@ -89,9 +89,14 @@ public function getIndex(string $version): array $keyFormat = $this->config->getString('learn.search.index.key', ''); $cacheKey = sprintf($keyFormat, $version); - // TODO : If the cache key is empty, it should build the index first $index = $this->cache->get($cacheKey); + // If cache is empty, build the index first + if (!is_array($index)) { + $this->buildIndex($version); + $index = $this->cache->get($cacheKey); + } + // Ensure we return an array even if cache returns null or unexpected type if (!is_array($index)) { return []; From 6af0323256aff8298b6a149baa3935838b24b8b4 Mon Sep 17 00:00:00 2001 From: Louis Charette Date: Sun, 25 Jan 2026 16:45:49 -0500 Subject: [PATCH 19/20] Simplify SearchIndex & SearchService --- .../Documentation/DocumentationRepository.php | 2 +- app/src/Search/SearchIndex.php | 123 +++++-------- app/src/Search/SearchService.php | 170 ++++++++++++------ app/tests/Search/SearchIndexTest.php | 23 ++- 4 files changed, 170 insertions(+), 148 deletions(-) diff --git a/app/src/Documentation/DocumentationRepository.php b/app/src/Documentation/DocumentationRepository.php index 70eb226b..6c5f8d31 100644 --- a/app/src/Documentation/DocumentationRepository.php +++ b/app/src/Documentation/DocumentationRepository.php @@ -283,7 +283,7 @@ protected function getAdjacentPage(PageResource $page, int $offset): ?PageResour * * @return array Array keyed by page slug */ - protected function getFlattenedTree(?string $version = null): array + public function getFlattenedTree(?string $version = null): array { $tree = $this->getTree($version); $flat = []; diff --git a/app/src/Search/SearchIndex.php b/app/src/Search/SearchIndex.php index fc2452e9..0da37b8e 100644 --- a/app/src/Search/SearchIndex.php +++ b/app/src/Search/SearchIndex.php @@ -91,7 +91,7 @@ public function getIndex(string $version): array $index = $this->cache->get($cacheKey); - // If cache is empty, build the index first + // If cache is empty, try to build the index first if (!is_array($index)) { $this->buildIndex($version); $index = $this->cache->get($cacheKey); @@ -114,8 +114,7 @@ public function getIndex(string $version): array */ protected function indexVersion(Version $version): array { - $tree = $this->repository->getTree($version->id); - $pages = $this->flattenTree($tree); + $pages = $this->repository->getFlattenedTree($version->id); /** @var list */ $indexed = []; @@ -136,113 +135,81 @@ protected function indexVersion(Version $version): array */ protected function indexPage(PageResource $page): IndexedPage { - // Get the HTML content and strip HTML tags to get plain text - $htmlContent = $page->getContent(); - $plainText = $this->stripHtmlTags($htmlContent); - - // Get frontmatter $frontMatter = $page->getFrontMatter(); - // Extract keywords if present - $keywords = ''; - if (isset($frontMatter['keywords'])) { - if (is_array($frontMatter['keywords'])) { - $keywords = implode(' ', $frontMatter['keywords']); - } elseif (is_string($frontMatter['keywords'])) { - $keywords = $frontMatter['keywords']; - } - } - - // Extract other relevant metadata (description, tags, etc.) - $metadata = []; - $metadataFields = $this->config->get('learn.search.metadata_fields', ['description', 'tags', 'category', 'author']); - foreach ($metadataFields as $field) { - if (isset($frontMatter[$field])) { - if (is_array($frontMatter[$field])) { - $metadata[] = implode(' ', $frontMatter[$field]); - } elseif (is_string($frontMatter[$field])) { - $metadata[] = $frontMatter[$field]; - } - } - } - $metadataString = implode(' ', $metadata); - return new IndexedPage( title: $page->getTitle(), slug: $page->getSlug(), route: $page->getRoute(), - content: $plainText, + content: $this->stripHtmlTags($page->getContent()), version: $page->getVersion()->id, - keywords: $keywords, - metadata: $metadataString, + keywords: $this->extractFieldAsString($frontMatter, 'keywords'), + metadata: $this->extractMetadata($frontMatter), ); } /** - * Strip HTML tags from content to get searchable plain text. - * Preserves code blocks and adds spacing for better search results. + * Extract a frontmatter field as string. * - * @param string $html + * @param array $frontMatter + * @param string $field * * @return string */ - protected function stripHtmlTags(string $html): string + protected function extractFieldAsString(array $frontMatter, string $field): string { - // Combined regex: Add space before/after block elements to prevent word concatenation - $result = preg_replace([ - '/<(div|p|h[1-6]|li|pre|code|blockquote)[^>]*>/i', // Opening tags - '/<\/(div|p|h[1-6]|li|pre|code|blockquote)>/i', // Closing tags - '/<(script|style)[^>]*>.*?<\/\1>/is', // Remove script/style with content - ], [ - ' $0', // Space before opening tags - '$0 ', // Space after closing tags - '', // Remove script/style entirely - ], $html); - - // Check if preg_replace failed - if ($result === null) { - // Fallback to original HTML if regex fails - $result = $html; + if (!isset($frontMatter[$field])) { + return ''; } - // Strip remaining HTML tags - $text = strip_tags($result); + $value = $frontMatter[$field]; - // Decode HTML entities - $text = html_entity_decode($text, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + return is_array($value) ? implode(' ', $value) : (string) $value; + } - // Normalize whitespace - $text = preg_replace('/\s+/', ' ', $text); + /** + * Extract metadata fields as concatenated string. + * + * @param array $frontMatter + * + * @return string + */ + protected function extractMetadata(array $frontMatter): string + { + $fields = $this->config->get('learn.search.metadata_fields', []); + $values = []; - // Check if preg_replace failed - if ($text === null) { - // Fallback: at least decode entities from stripped HTML - $text = html_entity_decode(strip_tags($html), ENT_QUOTES | ENT_HTML5, 'UTF-8'); + foreach ($fields as $field) { + $value = $this->extractFieldAsString($frontMatter, $field); + if ($value !== '') { + $values[] = $value; + } } - return trim($text); + return implode(' ', $values); } /** - * Flatten a tree structure into a flat array of pages. + * Strip HTML tags from content to get searchable plain text. + * Preserves code blocks and adds spacing for better search results. * - * @param PageResource[] $tree + * @param string $html * - * @return PageResource[] + * @return string */ - protected function flattenTree(array $tree): array + protected function stripHtmlTags(string $html): string { - $flat = []; + // Remove script/style tags with content + $html = preg_replace('/<(script|style)[^>]*>.*?<\/\1>/is', '', $html) ?? $html; - foreach ($tree as $page) { - $flat[] = $page; - $children = $page->getChildren(); - if ($children !== null && count($children) > 0) { - $flat = array_merge($flat, $this->flattenTree($children)); - } - } + // Add spaces around block elements to prevent word concatenation + $html = preg_replace('/<(div|p|h[1-6]|li|pre|code|blockquote)[^>]*>/i', ' ', $html) ?? $html; + + // Strip all remaining HTML tags and decode entities + $text = html_entity_decode(strip_tags($html), ENT_QUOTES | ENT_HTML5, 'UTF-8'); - return $flat; + // Normalize whitespace + return trim(preg_replace('/\s+/', ' ', $text) ?? $text); } /** diff --git a/app/src/Search/SearchService.php b/app/src/Search/SearchService.php index 7617e682..116d0fe3 100644 --- a/app/src/Search/SearchService.php +++ b/app/src/Search/SearchService.php @@ -42,71 +42,21 @@ public function __construct( */ public function performSearch(string $query, array $index): array { - $results = []; $query = trim($query); - if ($query === '') { - return $results; + return []; } - // Determine if query contains wildcards (check once before loop) $hasWildcards = str_contains($query, '*') || str_contains($query, '?'); + $wildcardRegex = $hasWildcards ? $this->buildWildcardRegex($query) : null; - // Pre-compile regex for wildcard searches to avoid recompiling in loop - $wildcardRegex = null; - if ($hasWildcards) { - $pattern = preg_quote($query, '/'); - $pattern = str_replace(['\*', '\?'], ['.*', '.'], $pattern); - $wildcardRegex = '/' . $pattern . '/i'; - } - + $results = []; foreach ($index as $page) { - $titleMatches = []; - $keywordMatches = []; - $metadataMatches = []; - $contentMatches = []; - - // Search in different fields with priority - if ($hasWildcards) { - $titleMatches = $this->searchWithWildcard($wildcardRegex, $page->title); - $keywordMatches = $this->searchWithWildcard($wildcardRegex, $page->keywords); - $metadataMatches = $this->searchWithWildcard($wildcardRegex, $page->metadata); - $contentMatches = $this->searchWithWildcard($wildcardRegex, $page->content); - } else { - $titleMatches = $this->searchPlain($query, $page->title); - $keywordMatches = $this->searchPlain($query, $page->keywords); - $metadataMatches = $this->searchPlain($query, $page->metadata); - $contentMatches = $this->searchPlain($query, $page->content); - } - - // Calculate weighted score: title > keywords > metadata > content - $score = count($titleMatches) * 10 + count($keywordMatches) * 5 + count($metadataMatches) * 2 + count($contentMatches); + $matches = $this->searchInPage($page, $query, $wildcardRegex); + $score = $this->calculateScore($matches); if ($score > 0) { - // Prefer snippet from title/keywords/metadata if found, otherwise content - $snippetPosition = 0; - if (count($titleMatches) > 0) { - $snippetPosition = $titleMatches[0]; - $snippetContent = $page->title; - } elseif (count($keywordMatches) > 0) { - $snippetPosition = $keywordMatches[0]; - $snippetContent = $page->keywords; - } elseif (count($metadataMatches) > 0) { - $snippetPosition = $metadataMatches[0]; - $snippetContent = $page->metadata; - } else { - $snippetPosition = $contentMatches[0]; - $snippetContent = $page->content; - } - - $results[] = new SearchResult( - title: $page->title, - slug: $page->slug, - route: $page->route, - snippet: $this->generateSnippet($snippetContent, $snippetPosition), - matches: $score, - version: $page->version, - ); + $results[] = $this->createSearchResult($page, $matches, $score); } } @@ -118,6 +68,114 @@ public function performSearch(string $query, array $index): array return array_slice($results, 0, $maxResults); } + /** + * Build wildcard regex pattern from query. + * + * @param string $query + * + * @return string + */ + protected function buildWildcardRegex(string $query): string + { + $pattern = preg_quote($query, '/'); + $pattern = str_replace(['\*', '\?'], ['.*', '.'], $pattern); + + return '/' . $pattern . '/i'; + } + + /** + * Search in all page fields. + * + * @param IndexedPage $page + * @param string $query + * @param string|null $wildcardRegex + * + * @return array> + */ + protected function searchInPage(IndexedPage $page, string $query, ?string $wildcardRegex): array + { + if ($wildcardRegex !== null) { + return [ + 'title' => $this->searchWithWildcard($wildcardRegex, $page->title), + 'keywords' => $this->searchWithWildcard($wildcardRegex, $page->keywords), + 'metadata' => $this->searchWithWildcard($wildcardRegex, $page->metadata), + 'content' => $this->searchWithWildcard($wildcardRegex, $page->content), + ]; + } + + return [ + 'title' => $this->searchPlain($query, $page->title), + 'keywords' => $this->searchPlain($query, $page->keywords), + 'metadata' => $this->searchPlain($query, $page->metadata), + 'content' => $this->searchPlain($query, $page->content), + ]; + } + + /** + * Calculate weighted score from matches. + * + * @param array> $matches + * + * @return int + */ + protected function calculateScore(array $matches): int + { + return count($matches['title']) * 10 + + count($matches['keywords']) * 5 + + count($matches['metadata']) * 2 + + count($matches['content']); + } + + /** + * Create search result with snippet. + * + * @param IndexedPage $page + * @param array> $matches + * @param int $score + * + * @return SearchResult + */ + protected function createSearchResult(IndexedPage $page, array $matches, int $score): SearchResult + { + // Determine best snippet source by priority + $snippetData = $this->selectSnippetSource($page, $matches); + + return new SearchResult( + title: $page->title, + slug: $page->slug, + route: $page->route, + snippet: $this->generateSnippet($snippetData['content'], $snippetData['position']), + matches: $score, + version: $page->version, + ); + } + + /** + * Select the best snippet source from matches. + * + * @param IndexedPage $page + * @param array> $matches + * + * @return array{content: string, position: int} + */ + protected function selectSnippetSource(IndexedPage $page, array $matches): array + { + $priority = [ + 'title' => $page->title, + 'keywords' => $page->keywords, + 'metadata' => $page->metadata, + 'content' => $page->content, + ]; + + foreach ($priority as $field => $content) { + if (isset($matches[$field]) && count($matches[$field]) > 0) { + return ['content' => $content, 'position' => $matches[$field][0]]; + } + } + + return ['content' => '', 'position' => 0]; + } + /** * Search for plain text matches (case-insensitive). * diff --git a/app/tests/Search/SearchIndexTest.php b/app/tests/Search/SearchIndexTest.php index 7c730a35..5d710ab3 100644 --- a/app/tests/Search/SearchIndexTest.php +++ b/app/tests/Search/SearchIndexTest.php @@ -11,6 +11,8 @@ namespace UserFrosting\Tests\Learn\Search; use UserFrosting\Config\Config; +use UserFrosting\Learn\Documentation\DocumentationRepository; +use UserFrosting\Learn\Documentation\PageResource; use UserFrosting\Learn\Recipe; use UserFrosting\Learn\Search\SearchIndex; use UserFrosting\Testing\TestCase; @@ -54,8 +56,8 @@ public function testBuildIndexForVersion(): void $this->assertGreaterThan(0, $count, 'Should have indexed at least one page'); // Verify it matches the number of test pages - /** @var \UserFrosting\Learn\Documentation\DocumentationRepository $repository */ - $repository = $this->ci->get(\UserFrosting\Learn\Documentation\DocumentationRepository::class); + /** @var DocumentationRepository $repository */ + $repository = $this->ci->get(DocumentationRepository::class); // Use reflection to get pages count $reflection = new \ReflectionClass($repository); @@ -182,26 +184,21 @@ public function testFlattenTree(): void // Build index to get tree $searchIndex->buildIndex('6.0'); - // Use reflection to access the repository and get tree - /** @var \UserFrosting\Learn\Documentation\DocumentationRepository $repository */ - $repository = $this->ci->get(\UserFrosting\Learn\Documentation\DocumentationRepository::class); - $tree = $repository->getTree('6.0'); - - // Use reflection to test flattenTree - $reflection = new \ReflectionClass($searchIndex); - $method = $reflection->getMethod('flattenTree'); - - $flat = $method->invoke($searchIndex, $tree); + // Use the repository's public getFlattenedTree method + /** @var DocumentationRepository $repository */ + $repository = $this->ci->get(DocumentationRepository::class); + $flat = $repository->getFlattenedTree('6.0'); // Should have multiple pages $this->assertGreaterThan(0, count($flat), 'Should have at least one page'); // Verify they're all PageResource objects foreach ($flat as $page) { - $this->assertInstanceOf(\UserFrosting\Learn\Documentation\PageResource::class, $page); + $this->assertInstanceOf(PageResource::class, $page); // @phpstan-ignore-line } // Verify flat count matches tree structure (all pages including nested) + $tree = $repository->getTree('6.0'); $countTreePages = function ($pages) use (&$countTreePages) { $count = 0; foreach ($pages as $page) { From 99d41fc289f7f6d949f34ab7b38fc5ea2aa66942 Mon Sep 17 00:00:00 2001 From: Louis Charette Date: Sun, 25 Jan 2026 22:02:02 -0500 Subject: [PATCH 20/20] Review tests --- app/src/Search/SearchService.php | 18 +- app/src/Search/SearchSprunje.php | 14 +- app/tests/Controller/SearchControllerTest.php | 194 ++------------- app/tests/Search/SearchIndexTest.php | 224 ++++++++++++++---- app/tests/Search/SearchServiceTest.php | 165 ++++++++++--- app/tests/Search/SearchSprunjeTest.php | 158 ++++++++++++ .../pages/6.0/02.second/01.alpha/docs.md | 4 +- app/tests/pages/6.0/02.second/02.beta/docs.md | 3 + 8 files changed, 498 insertions(+), 282 deletions(-) create mode 100644 app/tests/Search/SearchSprunjeTest.php diff --git a/app/src/Search/SearchService.php b/app/src/Search/SearchService.php index 116d0fe3..b079f54e 100644 --- a/app/src/Search/SearchService.php +++ b/app/src/Search/SearchService.php @@ -13,6 +13,7 @@ namespace UserFrosting\Learn\Search; use Illuminate\Cache\Repository as Cache; +use InvalidArgumentException; use UserFrosting\Config\Config; /** @@ -43,8 +44,11 @@ public function __construct( public function performSearch(string $query, array $index): array { $query = trim($query); - if ($query === '') { - return []; + + // Validate query length + $minLength = $this->config->getInt('learn.search.min_length', 3); + if ($query === '' || mb_strlen($query) < $minLength) { + throw new InvalidArgumentException("Query must be at least {$minLength} characters long"); } $hasWildcards = str_contains($query, '*') || str_contains($query, '?'); @@ -211,15 +215,11 @@ protected function searchWithWildcard(string $regex, string $content): array { $matches = []; - // Split content into words and check each word - $words = preg_split('/\s+/', $content); + // Split content into words and check each word (default to empty array if preg_split fails) + // @phpstan-ignore-next-line : preg_split can return false, but only if an error occurs, which we can ignore here. + $words = preg_split('/\s+/', $content) ?: []; $offset = 0; - if ($words === false) { - // Log error if needed in the future, but for now just return empty - return $matches; - } - foreach ($words as $word) { if (preg_match($regex, $word) === 1) { $matches[] = $offset; diff --git a/app/src/Search/SearchSprunje.php b/app/src/Search/SearchSprunje.php index ebdbc6f2..5eefe26a 100644 --- a/app/src/Search/SearchSprunje.php +++ b/app/src/Search/SearchSprunje.php @@ -38,11 +38,9 @@ public function __construct( protected SearchIndex $searchIndex, protected Config $config ) { - // Set default size from config - $defaultSize = $this->config->getInt('learn.search.default_size'); - if ($defaultSize !== null) { - $this->size = $defaultSize; - } + // Set default size and version from config + $this->size = $this->config->getInt('learn.search.default_size', 10); + $this->version = $this->config->getString('learn.versions.latest'); } /** @@ -56,12 +54,6 @@ public function __construct( */ public function setQuery(string $query): static { - // Validate query length - $minLength = $this->config->getInt('learn.search.min_length', 3); - if ($query === '' || mb_strlen($query) < $minLength) { - throw new InvalidArgumentException("Query must be at least {$minLength} characters long"); - } - $this->query = $query; return $this; diff --git a/app/tests/Controller/SearchControllerTest.php b/app/tests/Controller/SearchControllerTest.php index aaea17e9..521c4360 100644 --- a/app/tests/Controller/SearchControllerTest.php +++ b/app/tests/Controller/SearchControllerTest.php @@ -48,7 +48,7 @@ public function setUp(): void } /** - * Test search API endpoint with query. + * Test search API endpoint returns successful response with valid structure. */ public function testSearchEndpoint(): void { @@ -59,209 +59,45 @@ public function testSearchEndpoint(): void // Assert successful response $this->assertResponseStatus(200, $response); - // Parse JSON response - $body = (string) $response->getBody(); - $data = json_decode($body, true); - - $this->assertIsArray($data); - $this->assertArrayHasKey('rows', $data); - $this->assertArrayHasKey('count', $data); - - // Should have some results - $this->assertGreaterThan(0, $data['count']); - $this->assertNotEmpty($data['rows']); - - // Check structure of first result - if (count($data['rows']) > 0) { - $firstResult = $data['rows'][0]; - $this->assertArrayHasKey('title', $firstResult); - $this->assertArrayHasKey('slug', $firstResult); - $this->assertArrayHasKey('route', $firstResult); - $this->assertArrayHasKey('snippet', $firstResult); - $this->assertArrayHasKey('matches', $firstResult); - $this->assertArrayHasKey('version', $firstResult); - } - } - - /** - * Test search API endpoint with empty query. - */ - public function testSearchEndpointEmptyQuery(): void - { - // Create request without query - $request = $this->createRequest('GET', '/api/search'); - $response = $this->handleRequest($request); - - // Returns 500 because InvalidArgumentException is not caught - $this->assertResponseStatus(500, $response); - } - - /** - * Test search API endpoint with query too short. - */ - public function testSearchEndpointQueryTooShort(): void - { - // Create request with query too short (less than min_length) - $request = $this->createRequest('GET', '/api/search?q=ab'); - $response = $this->handleRequest($request); - - // Returns 500 because InvalidArgumentException is not caught - $this->assertResponseStatus(500, $response); - } - - /** - * Test search API endpoint with pagination. - */ - public function testSearchEndpointPagination(): void - { - // Create request with pagination parameters - $request = $this->createRequest('GET', '/api/search?q=page&page=1&size=2'); - $response = $this->handleRequest($request); - - // Assert successful response - $this->assertResponseStatus(200, $response); - - // Parse JSON response - $body = (string) $response->getBody(); - $data = json_decode($body, true); - - $this->assertIsArray($data); - $this->assertArrayHasKey('count', $data); - $this->assertArrayHasKey('page', $data); - $this->assertArrayHasKey('size', $data); - $this->assertArrayHasKey('rows', $data); - - // Should return at most 2 results - $this->assertLessThanOrEqual(2, count($data['rows'])); - - // Check that page and size are correct - $this->assertSame(1, $data['page']); - $this->assertSame(2, $data['size']); - } - - /** - * Test search API endpoint with version parameter. - */ - public function testSearchEndpointWithVersion(): void - { - // Create request with version parameter - $request = $this->createRequest('GET', '/api/search?q=first&version=6.0'); - $response = $this->handleRequest($request); - - // Assert successful response - $this->assertResponseStatus(200, $response); - - // Parse JSON response - $body = (string) $response->getBody(); - $data = json_decode($body, true); - - $this->assertIsArray($data); - - // Verify results are from correct version - if (count($data['rows']) > 0) { - foreach ($data['rows'] as $result) { - $this->assertSame('6.0', $result['version']); - } - } - } - - /** - * Test search API endpoint with wildcard query. - */ - public function testSearchEndpointWildcard(): void - { - // Create request with wildcard query that meets minimum length - $request = $this->createRequest('GET', '/api/search?q=fir*'); - $response = $this->handleRequest($request); - - // Assert successful response - $this->assertResponseStatus(200, $response); - - // Parse JSON response - $body = (string) $response->getBody(); - $data = json_decode($body, true); - - $this->assertIsArray($data); - $this->assertArrayHasKey('rows', $data); - } - - /** - * Test that response is valid JSON. - */ - public function testSearchEndpointReturnsJson(): void - { - $request = $this->createRequest('GET', '/api/search?q=test'); - $response = $this->handleRequest($request); - // Check content type header $this->assertTrue($response->hasHeader('Content-Type')); $contentType = $response->getHeaderLine('Content-Type'); $this->assertStringContainsString('application/json', $contentType); - } - - /** - * Test search API endpoint with no version and no default version in config. - */ - public function testSearchEndpointNoVersion(): void - { - // Temporarily unset the default version - /** @var Config $config */ - $config = $this->ci->get(Config::class); - $originalVersion = $config->get('learn.versions.latest'); - $config->set('learn.versions.latest', null); - - // Create request without version parameter - $request = $this->createRequest('GET', '/api/search?q=test'); - $response = $this->handleRequest($request); - - // Restore original version - $config->set('learn.versions.latest', $originalVersion); - - // Assert successful response but empty results - $this->assertResponseStatus(200, $response); // Parse JSON response $body = (string) $response->getBody(); $data = json_decode($body, true); + // Verify response structure $this->assertIsArray($data); $this->assertArrayHasKey('rows', $data); $this->assertArrayHasKey('count', $data); - - // Should have no results when version is null - $this->assertSame(0, $data['count']); - $this->assertEmpty($data['rows']); + $this->assertArrayHasKey('page', $data); + $this->assertArrayHasKey('size', $data); } /** - * Test search API endpoint with no indexed pages for version. + * Test search API endpoint with size parameter. */ - public function testSearchEndpointNoIndexedPages(): void + public function testSearchEndpointWithSize(): void { - // Clear the index for version 6.0 - $searchIndex = $this->ci->get(SearchIndex::class); - $searchIndex->clearIndex('6.0'); - - // Create request to search - $request = $this->createRequest('GET', '/api/search?q=test&version=6.0'); + // Create request with size parameter + $request = $this->createRequest('GET', '/api/search?q=page&size=2'); $response = $this->handleRequest($request); - // Assert successful response but empty results + // Assert successful response $this->assertResponseStatus(200, $response); // Parse JSON response $body = (string) $response->getBody(); $data = json_decode($body, true); - $this->assertIsArray($data); - $this->assertArrayHasKey('rows', $data); - $this->assertArrayHasKey('count', $data); - - // Should have no results when index is empty - $this->assertSame(0, $data['count']); - $this->assertEmpty($data['rows']); + // Verify size is correctly set and respected + $this->assertSame(2, $data['size']); - // Rebuild index for other tests - $searchIndex->buildIndex('6.0'); + // If there are results, should not exceed size + if ($data['count'] > 0) { + $this->assertLessThanOrEqual(2, count($data['rows'])); + } } } diff --git a/app/tests/Search/SearchIndexTest.php b/app/tests/Search/SearchIndexTest.php index 5d710ab3..f82e744b 100644 --- a/app/tests/Search/SearchIndexTest.php +++ b/app/tests/Search/SearchIndexTest.php @@ -12,8 +12,8 @@ use UserFrosting\Config\Config; use UserFrosting\Learn\Documentation\DocumentationRepository; -use UserFrosting\Learn\Documentation\PageResource; use UserFrosting\Learn\Recipe; +use UserFrosting\Learn\Search\IndexedPage; use UserFrosting\Learn\Search\SearchIndex; use UserFrosting\Testing\TestCase; use UserFrosting\UniformResourceLocator\ResourceLocatorInterface; @@ -58,11 +58,7 @@ public function testBuildIndexForVersion(): void // Verify it matches the number of test pages /** @var DocumentationRepository $repository */ $repository = $this->ci->get(DocumentationRepository::class); - - // Use reflection to get pages count - $reflection = new \ReflectionClass($repository); - $method = $reflection->getMethod('getFlattenedTree'); - $flatPages = $method->invoke($repository, '6.0'); + $flatPages = $repository->getFlattenedTree('6.0'); $this->assertSame(count($flatPages), $count, 'Index count should match actual page count'); } @@ -74,8 +70,16 @@ public function testBuildIndexForAllVersions(): void // Build index for all versions $count = $searchIndex->buildIndex(null); + // Verify it matches the number of test pages + /** @var DocumentationRepository $repository */ + $repository = $this->ci->get(DocumentationRepository::class); + $flatPages = $repository->getFlattenedTree('6.0'); + // Should have indexed pages (at least some) $this->assertGreaterThan(0, $count, 'Should have indexed at least one page'); + + // Should have more pages than just version 6.0 alone (if multiple versions exist) + $this->assertSame(count($flatPages), $count, 'Index count should match actual page count across all versions'); } public function testIndexPageContent(): void @@ -101,7 +105,7 @@ public function testIndexPageContent(): void // Check first page structure $firstPage = $index[0]; - $this->assertInstanceOf(\UserFrosting\Learn\Search\IndexedPage::class, $firstPage); + $this->assertInstanceOf(IndexedPage::class, $firstPage); $this->assertNotEmpty($firstPage->title); $this->assertNotEmpty($firstPage->slug); $this->assertNotEmpty($firstPage->route); @@ -137,21 +141,27 @@ public function testClearIndex(): void { $searchIndex = $this->ci->get(SearchIndex::class); - // Build index - $searchIndex->buildIndex('6.0'); - - // Clear index - $searchIndex->clearIndex('6.0'); - - // Verify cache is cleared + // Get cache key $reflection = new \ReflectionClass($searchIndex); $getCacheKeyMethod = $reflection->getMethod('getCacheKey'); $cacheKey = $getCacheKeyMethod->invoke($searchIndex, '6.0'); /** @var \Illuminate\Cache\Repository $cache */ $cache = $this->ci->get(\Illuminate\Cache\Repository::class); + + // Build index + $searchIndex->buildIndex('6.0'); + + // Make sure index exists $index = $cache->get($cacheKey); + $this->assertIsArray($index); + $this->assertNotEmpty($index); + + // Clear index + $searchIndex->clearIndex('6.0'); + // Verify cache is cleared + $index = $cache->get($cacheKey); $this->assertNull($index); } @@ -159,59 +169,173 @@ public function testClearAllIndexes(): void { $searchIndex = $this->ci->get(SearchIndex::class); - // Build index for all versions - $searchIndex->buildIndex(null); - - // Clear all indexes - $searchIndex->clearIndex(null); - - // Verify cache is cleared + // Get cache key $reflection = new \ReflectionClass($searchIndex); $getCacheKeyMethod = $reflection->getMethod('getCacheKey'); $cacheKey = $getCacheKeyMethod->invoke($searchIndex, '6.0'); + // Build index for all versions + $searchIndex->buildIndex(null); + + // Make sure index exists /** @var \Illuminate\Cache\Repository $cache */ $cache = $this->ci->get(\Illuminate\Cache\Repository::class); $index = $cache->get($cacheKey); + $this->assertIsArray($index); + $this->assertNotEmpty($index); + // Clear all indexes + $searchIndex->clearIndex(null); + $index = $cache->get($cacheKey); $this->assertNull($index); } - public function testFlattenTree(): void + public function testGetIndex(): void { $searchIndex = $this->ci->get(SearchIndex::class); - // Build index to get tree + // Build index for version 6.0 so we can retrieve it $searchIndex->buildIndex('6.0'); - // Use the repository's public getFlattenedTree method + // Verify it matches the number of test pages /** @var DocumentationRepository $repository */ $repository = $this->ci->get(DocumentationRepository::class); - $flat = $repository->getFlattenedTree('6.0'); - - // Should have multiple pages - $this->assertGreaterThan(0, count($flat), 'Should have at least one page'); - - // Verify they're all PageResource objects - foreach ($flat as $page) { - $this->assertInstanceOf(PageResource::class, $page); // @phpstan-ignore-line - } - - // Verify flat count matches tree structure (all pages including nested) - $tree = $repository->getTree('6.0'); - $countTreePages = function ($pages) use (&$countTreePages) { - $count = 0; - foreach ($pages as $page) { - $count++; - if ($page->getChildren()) { - $count += $countTreePages($page->getChildren()); - } - } - - return $count; - }; - - $expectedCount = $countTreePages($tree); - $this->assertSame($expectedCount, count($flat), 'Flattened tree should contain all pages'); + $flatPages = $repository->getFlattenedTree('6.0'); + + // Retrieve index + $index = $searchIndex->getIndex('6.0'); + $this->assertIsArray($index); + $this->assertNotEmpty($index); + $this->assertSame(count($flatPages), count($index), 'Index should contain the right number of pages'); + } + + public function testGetIndexRebuildsWhenCacheEmpty(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + // Make sure cache is cleared + $searchIndex->clearIndex('6.0'); + + // getIndex should automatically rebuild when cache is empty + $index = $searchIndex->getIndex('6.0'); + + $this->assertIsArray($index); + $this->assertNotEmpty($index, 'Should rebuild index when cache is empty'); + $this->assertContainsOnlyInstancesOf(IndexedPage::class, $index); + } + + public function testExtractMetadataSkipsEmptyFields(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + // Set metadata fields in config + /** @var Config $config */ + $config = $this->ci->get(Config::class); + $config->set('learn.search.metadata_fields', ['description', 'tags', 'category', 'author']); + + // Use reflection to test protected method + $reflection = new \ReflectionClass($searchIndex); + $method = $reflection->getMethod('extractMetadata'); + + // Test with mixed empty and non-empty fields + $frontMatter = [ + 'description' => 'Test description', + 'tags' => '', // Empty string - should be skipped + 'category' => [], // Empty array - should be skipped + 'author' => 'Test Author', + ]; + + $metadata = $method->invoke($searchIndex, $frontMatter); + + // Should only include non-empty fields + $this->assertSame('Test description Test Author', $metadata); + } + + public function testExtractMetadataWithAllEmptyFields(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + // Set metadata fields in config + /** @var Config $config */ + $config = $this->ci->get(Config::class); + $config->set('learn.search.metadata_fields', ['description', 'tags']); + + // Use reflection to test protected method + $reflection = new \ReflectionClass($searchIndex); + $method = $reflection->getMethod('extractMetadata'); + + // Test with all empty or missing fields + $frontMatter = [ + 'description' => '', + // 'tags' is missing + ]; + + $metadata = $method->invoke($searchIndex, $frontMatter); + + // Should return empty string + $this->assertSame('', $metadata); + } + + public function testExtractMetadataWithArrayFields(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + // Set metadata fields in config + /** @var Config $config */ + $config = $this->ci->get(Config::class); + $config->set('learn.search.metadata_fields', ['tags', 'keywords']); + + // Use reflection to test protected method + $reflection = new \ReflectionClass($searchIndex); + $method = $reflection->getMethod('extractMetadata'); + + // Test with array fields + $frontMatter = [ + 'tags' => ['php', 'framework', 'web'], + 'keywords' => ['userfrosting', 'tutorial'], + ]; + + $metadata = $method->invoke($searchIndex, $frontMatter); + + // Should join array values with spaces + $this->assertStringContainsString('php framework web', $metadata); + $this->assertStringContainsString('userfrosting tutorial', $metadata); + } + + public function testGetIndexReturnsEmptyArrayWhenCacheCorrupted(): void + { + $searchIndex = $this->ci->get(SearchIndex::class); + + /** @var \Illuminate\Cache\Repository $cache */ + $cache = $this->ci->get(\Illuminate\Cache\Repository::class); + + // Mock cache to always return non-array value, even after rebuild attempt + // Use reflection to get cache key + $reflection = new \ReflectionClass($searchIndex); + $getCacheKeyMethod = $reflection->getMethod('getCacheKey'); + $cacheKey = $getCacheKeyMethod->invoke($searchIndex, '6.0'); + + // Create a mock cache that always returns corrupted data + $mockCache = $this->getMockBuilder(\Illuminate\Cache\Repository::class) + ->disableOriginalConstructor() + ->onlyMethods(['get', 'put', 'forget']) + ->getMock(); + + $mockCache->method('get') + ->with($cacheKey) + ->willReturn('corrupted-data'); // Always return non-array + + $mockCache->method('put') + ->willReturn(true); + + // Use reflection to replace the cache instance + $cacheProperty = $reflection->getProperty('cache'); + $cacheProperty->setValue($searchIndex, $mockCache); + + // getIndex should return empty array when cache consistently returns non-array + $index = $searchIndex->getIndex('6.0'); + + $this->assertIsArray($index); + $this->assertEmpty($index, 'Should return empty array when cache persistently returns non-array data'); } } diff --git a/app/tests/Search/SearchServiceTest.php b/app/tests/Search/SearchServiceTest.php index 26f2c7b2..8bf7458b 100644 --- a/app/tests/Search/SearchServiceTest.php +++ b/app/tests/Search/SearchServiceTest.php @@ -10,11 +10,12 @@ namespace UserFrosting\Tests\Learn\Search; +use InvalidArgumentException; use UserFrosting\Config\Config; use UserFrosting\Learn\Recipe; use UserFrosting\Learn\Search\SearchIndex; +use UserFrosting\Learn\Search\SearchResult; use UserFrosting\Learn\Search\SearchService; -use UserFrosting\Learn\Search\SearchSprunje; use UserFrosting\Testing\TestCase; use UserFrosting\UniformResourceLocator\ResourceLocatorInterface; use UserFrosting\UniformResourceLocator\ResourceStream; @@ -63,7 +64,7 @@ public function testSearchWithPlainText(): void // Check structure of first result $firstResult = $results[0]; - $this->assertInstanceOf(\UserFrosting\Learn\Search\SearchResult::class, $firstResult); + $this->assertInstanceOf(SearchResult::class, $firstResult); $this->assertNotEmpty($firstResult->title); $this->assertNotEmpty($firstResult->slug); $this->assertNotEmpty($firstResult->route); @@ -77,40 +78,36 @@ public function testSearchWithEmptyQuery(): void $searchService = $this->ci->get(SearchService::class); $searchIndex = $this->ci->get(SearchIndex::class); - $index = $searchIndex->getIndex('6.0'); - $results = $searchService->performSearch('', $index); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Query must be at least 3 characters long'); - $this->assertSame(0, count($results)); - $this->assertEmpty($results); + $index = $searchIndex->getIndex('6.0'); + $searchService->performSearch('', $index); } - public function testSearchWithWildcard(): void + public function testSearchWithShortQuery(): void { $searchService = $this->ci->get(SearchService::class); $searchIndex = $this->ci->get(SearchIndex::class); - // Search for "f*" - should match words starting with 'f' - $index = $searchIndex->getIndex('6.0'); - $results = $searchService->performSearch('f*', $index); + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('Query must be at least 3 characters long'); - $this->assertIsArray($results); - $this->assertGreaterThanOrEqual(0, count($results)); + $index = $searchIndex->getIndex('6.0'); + $searchService->performSearch('ab', $index); // Only 2 characters } - public function testSearchPagination(): void + public function testSearchWithWildcard(): void { - $searchSprunje = $this->ci->get(SearchSprunje::class); - - // Search with pagination via Sprunje - $searchSprunje - ->setQuery('page') - ->setVersion('6.0') - ->setSize(2) - ->setPage(1); + $searchService = $this->ci->get(SearchService::class); + $searchIndex = $this->ci->get(SearchIndex::class); - $result = $searchSprunje->getResultSet(); + // Search for "pag*" - should match words starting with 'pag' + $index = $searchIndex->getIndex('6.0'); + $results = $searchService->performSearch('pag*', $index); - $this->assertArrayHasKey('rows', $result); + $this->assertIsArray($results); + $this->assertGreaterThanOrEqual(0, count($results)); } public function testSearchResultSnippet(): void @@ -152,7 +149,8 @@ public function testGenerateSnippet(): void $method = $reflection->getMethod('generateSnippet'); // Create long content that exceeds snippet length (default 150 chars) - $content = str_repeat('Lorem ipsum dolor sit amet, consectetur adipiscing elit. ', 10) . 'This is the important part. ' . str_repeat('More text follows here. ', 10); + $content = str_repeat('Lorem ipsum dolor sit amet, consectetur adipiscing elit. ', 10) . + 'This is the important part. ' . str_repeat('More text follows here. ', 10); $matchPosition = strpos($content, 'important'); if ($matchPosition !== false) { @@ -164,15 +162,10 @@ public function testGenerateSnippet(): void } } - public function testSearchWithNoIndex(): void + public function testSearchWithEmptyIndex(): void { - // Clear the index - $searchIndex = $this->ci->get(SearchIndex::class); - $searchIndex->clearIndex('6.0'); - $searchService = $this->ci->get(SearchService::class); - $index = $searchIndex->getIndex('6.0'); - $results = $searchService->performSearch('test', $index); + $results = $searchService->performSearch('first', []); $this->assertSame(0, count($results)); $this->assertEmpty($results); @@ -192,7 +185,115 @@ public function testSearchResultSorting(): void $firstMatches = $results[0]->matches; $lastMatches = $results[count($results) - 1]->matches; - $this->assertGreaterThanOrEqual($lastMatches, $firstMatches); + $this->assertGreaterThan($lastMatches, $firstMatches); + } + + // First tree pages should be in order : Alpha Page, Beta Page & First Page + $this->assertSame('Alpha Page', $results[0]->title); + $this->assertSame('Beta Page', $results[1]->title); + $this->assertSame('First page', $results[2]->title); + } + + public function testSelectSnippetSourceWithNoMatches(): void + { + $searchService = $this->ci->get(SearchService::class); + + // Use reflection to test protected method + $reflection = new \ReflectionClass($searchService); + $method = $reflection->getMethod('selectSnippetSource'); + + // Create a page with some content + $page = new \UserFrosting\Learn\Search\IndexedPage( + title: 'Test Page', + slug: 'test-page', + route: '/test-page', + content: 'Test content', + version: '6.0', + keywords: 'test keywords', + metadata: 'test metadata', + ); + + // Create matches array with all empty arrays (no matches) + $matches = [ + 'title' => [], + 'keywords' => [], + 'metadata' => [], + 'content' => [], + ]; + + $result = $method->invoke($searchService, $page, $matches); + + // Should return empty content and zero position as fallback + $this->assertIsArray($result); + $this->assertArrayHasKey('content', $result); + $this->assertArrayHasKey('position', $result); + $this->assertSame('', $result['content']); + $this->assertSame(0, $result['position']); + } + + public function testSearchWithWildcardMethod(): void + { + $searchService = $this->ci->get(SearchService::class); + + // Use reflection to test protected method + $reflection = new \ReflectionClass($searchService); + $method = $reflection->getMethod('searchWithWildcard'); + + // Test normal wildcard search + $content = 'This is a test with testing and tested words like attest and attestation.'; + $regex = '/test.*/i'; // Matches test, testing, tested, attest, attestation + + $matches = $method->invoke($searchService, $regex, $content); + + $this->assertIsArray($matches); + $this->assertSame(5, count($matches)); + } + + public function testSearchWithWildcardEmptyContent(): void + { + $searchService = $this->ci->get(SearchService::class); + + // Use reflection to test protected method + $reflection = new \ReflectionClass($searchService); + $method = $reflection->getMethod('searchWithWildcard'); + + // Test with empty content - should not fail and return empty array + $content = ''; + $regex = '/test.*/i'; + + $matches = $method->invoke($searchService, $regex, $content); + + $this->assertIsArray($matches); + $this->assertEmpty($matches, 'Should return empty array for empty content'); + } + + public function testSearchWithWildcardPregSplitFailure(): void + { + $searchService = $this->ci->get(SearchService::class); + + // Use reflection to test protected method + $reflection = new \ReflectionClass($searchService); + $method = $reflection->getMethod('searchWithWildcard'); + + // Save current PCRE settings + $originalBacktrackLimit = ini_get('pcre.backtrack_limit'); + + // Set a very low backtrack limit to force preg_split to fail + ini_set('pcre.backtrack_limit', '1'); + + // Create content that would exceed the backtrack limit + $content = str_repeat('a ', 1000); // 1000 words of 'a' + $regex = '/test.*/i'; + + $matches = $method->invoke($searchService, $regex, $content); + + // Restore original setting + if ($originalBacktrackLimit !== false) { + ini_set('pcre.backtrack_limit', $originalBacktrackLimit); } + + // Should return empty array when preg_split fails + $this->assertIsArray($matches); + $this->assertEmpty($matches, 'Should return empty array when preg_split fails'); } } diff --git a/app/tests/Search/SearchSprunjeTest.php b/app/tests/Search/SearchSprunjeTest.php new file mode 100644 index 00000000..2ecd1a70 --- /dev/null +++ b/app/tests/Search/SearchSprunjeTest.php @@ -0,0 +1,158 @@ +ci->get(Config::class); + $config->set('learn.versions.latest', '6.0'); + $config->set('learn.versions.available', [ + '6.0' => '6.0 Beta', + ]); + + // Use the test pages directory + /** @var ResourceLocatorInterface $locator */ + $locator = $this->ci->get(ResourceLocatorInterface::class); + $locator->removeStream('pages'); + $locator->addStream(new ResourceStream('pages', shared: true, readonly: true, path: __DIR__ . '/../pages')); + + // Build index for testing + $searchIndex = $this->ci->get(SearchIndex::class); + $searchIndex->buildIndex('6.0'); + } + + public function testSearchPagination(): void + { + $searchSprunje = $this->ci->get(SearchSprunje::class); + + // Search with pagination via Sprunje + $searchSprunje + ->setQuery('page') + ->setVersion('6.0') + ->setSize(2) + ->setPage(1); + + $result = $searchSprunje->getResultSet(); + + $this->assertArrayHasKey('rows', $result); + + // Make sure we have 2 results as per size + $this->assertCount(2, $result['rows']); + $this->assertSame(2, $result['size']); + $this->assertSame(1, $result['page']); + } + + public function testForDefaultSizePageAndVersion(): void + { + $searchSprunje = $this->ci->get(SearchSprunje::class); + + // Search with all default parameters via Sprunje + $searchSprunje->setQuery('page'); + $result = $searchSprunje->getResultSet(); + + $this->assertArrayHasKey('rows', $result); + + // Make sure we have 2 results as per size + $this->assertGreaterThanOrEqual(2, $result['rows']); + $this->assertSame(10, $result['size']); + $this->assertSame(1, $result['page']); + } + + public function testSearchSprunjeWithEmptyIndex(): void + { + $searchSprunje = $this->ci->get(SearchSprunje::class); + $searchIndex = $this->ci->get(SearchIndex::class); + $cache = $this->ci->get(\Illuminate\Cache\Repository::class); + + // Mock the SearchIndex to return empty array + $reflection = new \ReflectionClass($searchIndex); + $getCacheKeyMethod = $reflection->getMethod('getCacheKey'); + $cacheKey = $getCacheKeyMethod->invoke($searchIndex, '6.0'); + + // Put empty array in cache to simulate no indexed pages + $cache->put($cacheKey, [], 60); + + // Set query and version + $searchSprunje + ->setQuery('test') + ->setVersion('6.0'); + + // getItems should return empty collection when index is empty + $items = $searchSprunje->getItems(); + + $this->assertInstanceOf(\Illuminate\Support\Collection::class, $items); + $this->assertSame(0, $items->count(), 'Should return empty collection when index is empty'); + } + + public function testSearchSprunjeWithNullVersion(): void + { + // Version can be null if not defined in config + $config = $this->ci->get(Config::class); + $config->set('learn.versions.latest', null); + + $searchSprunje = $this->ci->get(SearchSprunje::class); + + // Search with all default parameters via Sprunje + $searchSprunje->setQuery('page'); + $result = $searchSprunje->getResultSet(); + + $this->assertArrayHasKey('rows', $result); + + // Make sure we have 2 results as per size + $this->assertSame(0, $result['count']); + $this->assertCount(0, $result['rows']); + } + + public function testSearchSprunjeGetVersionDefault(): void + { + $searchSprunje = $this->ci->get(SearchSprunje::class); + + // getVersion should be default before setting + $this->assertSame('6.0', $searchSprunje->getVersion()); + } + + public function testSearchSprunjeGetVersionAfterSet(): void + { + $searchSprunje = $this->ci->get(SearchSprunje::class); + + // Set version and verify getVersion returns it + $searchSprunje->setVersion('5.92'); + $this->assertSame('5.92', $searchSprunje->getVersion()); + } + + public function testSearchSprunjeGetVersionAfterSetNull(): void + { + $searchSprunje = $this->ci->get(SearchSprunje::class); + + // Set version to null, should default to latest from config + $searchSprunje->setVersion(null); + $this->assertSame('6.0', $searchSprunje->getVersion(), 'Should use latest version from config when set to null'); + } +} diff --git a/app/tests/pages/6.0/02.second/01.alpha/docs.md b/app/tests/pages/6.0/02.second/01.alpha/docs.md index d7bc37e0..f9c90dad 100644 --- a/app/tests/pages/6.0/02.second/01.alpha/docs.md +++ b/app/tests/pages/6.0/02.second/01.alpha/docs.md @@ -2,8 +2,10 @@ title: Alpha Page metadata: description: The alpha page description +keywords: + - page --- -## Title +## Page Title Lorem ipsum dolor sit amet, consectetur adipiscing elit. diff --git a/app/tests/pages/6.0/02.second/02.beta/docs.md b/app/tests/pages/6.0/02.second/02.beta/docs.md index 6086b0ca..e3819f6a 100644 --- a/app/tests/pages/6.0/02.second/02.beta/docs.md +++ b/app/tests/pages/6.0/02.second/02.beta/docs.md @@ -2,6 +2,9 @@ title: Beta Page metadata: description: Beta Page Description. +keywords: + - page + - test --- Lorem ipsum dolor sit amet, consectetur adipiscing elit.