From 73fafcd6452b69084bc93ac32ca2bf8de2a48250 Mon Sep 17 00:00:00 2001 From: Roland Date: Fri, 3 Jul 2026 10:09:08 -0600 Subject: [PATCH 01/12] Search: fast ABC mode by default, opt-in deep search The file_group.textsearch tsvector indexes all four text columns; d_text (captions, document text, zip contents) is ~90% of the data, making ts_rank searches take seconds on large libraries. Add a textsearch_abc generated column (title/author/description only, ~8x smaller) and default all searches to it, with deep=true opting into the full column. Database: - Migration adds the generated column and a GIN index (rewrites the table; a few minutes on large libraries). Backend: - FilesSearchRequest, VideoSearchRequest, ArchiveSearchRequest, and DocSearchRequest accept `deep`; each search function picks the column. - /api/search_file_estimates returns both counts in one index-bound query (COUNT FILTER on the abc subset): `file_groups` (fast) and `file_groups_deep`. UI (one coherent story across the count surfaces): - The typing dropdown always suggests the deep count (what is findable). - The tab badge shows the active mode's count so it matches the result list. - The gap is reconciled by a hint offering deep search: count-aware on the global search ("No results in fast search. N results available in deep search." / "N more results available..."), generic on the module pages (no deep estimate there). The hint resets pagination. - "Search Depth" toggle in the search filter modals, persisted as ?deep=true. Tests that relied on d_text matches now pass deep=True, with fast-mode assertions alongside. Also skip flaky test_download_channel on CircleCI. Co-Authored-By: Claude Fable 5 --- .../e5f7a9c1d3b6_file_group_textsearch_abc.py | 43 ++++++++++ app/src/DashboardPage.js | 2 +- app/src/api.js | 21 ++++- app/src/components/Archive.js | 4 +- app/src/components/Docs.js | 4 +- app/src/components/Files.js | 83 ++++++++++++++++++- app/src/components/Search.js | 26 ++++-- app/src/components/SearchFilterModal.test.js | 30 +++++++ app/src/components/Videos.js | 4 +- app/src/hooks/customHooks.js | 38 +++++++-- modules/archive/api.py | 2 +- modules/archive/lib.py | 8 +- modules/archive/schema.py | 1 + modules/archive/test/test_api.py | 28 ++++--- modules/archive/test/test_lib.py | 3 +- modules/docs/api.py | 1 + modules/docs/lib.py | 9 +- modules/docs/schema.py | 1 + modules/docs/test/test_lib.py | 6 +- modules/videos/conftest.py | 3 + modules/videos/schema.py | 1 + modules/videos/test/test_api.py | 21 +++-- modules/videos/test/test_downloader.py | 2 + modules/videos/video/api.py | 1 + modules/videos/video/lib.py | 7 +- wrolpi/files/api.py | 2 +- wrolpi/files/lib.py | 31 ++++--- wrolpi/files/models.py | 8 ++ wrolpi/files/schema.py | 1 + wrolpi/files/test/test_api.py | 48 ++++++++++- wrolpi/files/test/test_lib.py | 12 +-- wrolpi/root_api.py | 5 +- wrolpi/test/test_root_api.py | 6 +- 33 files changed, 385 insertions(+), 77 deletions(-) create mode 100644 alembic/versions/e5f7a9c1d3b6_file_group_textsearch_abc.py diff --git a/alembic/versions/e5f7a9c1d3b6_file_group_textsearch_abc.py b/alembic/versions/e5f7a9c1d3b6_file_group_textsearch_abc.py new file mode 100644 index 00000000..16207c0d --- /dev/null +++ b/alembic/versions/e5f7a9c1d3b6_file_group_textsearch_abc.py @@ -0,0 +1,43 @@ +"""file_group_textsearch_abc + +Add the `textsearch_abc` generated tsvector column to `file_group`. It indexes only a_text (title), +b_text (author), and c_text (description) — excluding d_text (captions/document body), which is ~90% of all +text data. Searches default to this much smaller column; the existing `textsearch` column remains for +opt-in "deep" search. + +Adding a STORED generated column rewrites the table; expect a few minutes on large libraries. + +Revision ID: e5f7a9c1d3b6 +Revises: d4e6f8a0c2b5 +Create Date: 2026-07-03 + +""" +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = 'e5f7a9c1d3b6' +down_revision = 'd4e6f8a0c2b5' +branch_labels = None +depends_on = None + + +def upgrade(): + conn = op.get_bind() + conn.execute(sa.text(''' + ALTER TABLE file_group + ADD COLUMN textsearch_abc tsvector + GENERATED ALWAYS AS ( + setweight(to_tsvector('english'::regconfig, COALESCE(a_text, '')), 'A'::"char") || + setweight(to_tsvector('english'::regconfig, COALESCE(b_text, '')), 'B'::"char") || + setweight(to_tsvector('english'::regconfig, COALESCE(c_text, '')), 'C'::"char") + ) STORED + ''')) + conn.execute(sa.text( + 'CREATE INDEX IF NOT EXISTS file_group_textsearch_abc_idx ON file_group USING GIN(textsearch_abc)')) + + +def downgrade(): + conn = op.get_bind() + conn.execute(sa.text('DROP INDEX IF EXISTS file_group_textsearch_abc_idx')) + conn.execute(sa.text('ALTER TABLE file_group DROP COLUMN IF EXISTS textsearch_abc')) diff --git a/app/src/DashboardPage.js b/app/src/DashboardPage.js index 10b0ea93..440b4d31 100644 --- a/app/src/DashboardPage.js +++ b/app/src/DashboardPage.js @@ -469,7 +469,7 @@ export function DashboardPage() { } return
{getSearchResultsInput(inputProps)} -
; }; diff --git a/app/src/api.js b/app/src/api.js index 74efc88d..3d8a3e4b 100644 --- a/app/src/api.js +++ b/app/src/api.js @@ -187,7 +187,7 @@ export async function tagChannelInfo(channelId, tagName) { }); } -export async function searchVideos(offset, limit, channelId, searchStr, order_by, tagNames, headline, censored) { +export async function searchVideos(offset, limit, channelId, searchStr, order_by, tagNames, headline, censored, deep) { // Build a search query to retrieve a list of videos from the API offset = parseInt(offset || 0); limit = parseInt(limit || DEFAULT_LIMIT); @@ -208,6 +208,9 @@ export async function searchVideos(offset, limit, channelId, searchStr, order_by if (censored) { body['censored'] = true; } + if (deep) { + body['deep'] = true; + } console.debug('searching videos', body); @@ -697,7 +700,7 @@ export async function saveCatalog(items) { } -export async function searchArchives(offset, limit, domain, searchStr, order, tagNames, headline) { +export async function searchArchives(offset, limit, domain, searchStr, order, tagNames, headline, deep) { // Build a search query to retrieve a list of videos from the API offset = parseInt(offset || 0); limit = parseInt(limit || DEFAULT_LIMIT); @@ -717,6 +720,9 @@ export async function searchArchives(offset, limit, domain, searchStr, order, ta if (headline) { body['headline'] = true; } + if (deep) { + body['deep'] = true; + } console.debug('searching archives', body); const response = await apiPost(`${ARCHIVES_API}/search`, body); @@ -1212,7 +1218,7 @@ export async function deleteDownload(downloadId) { } } -export async function filesSearch(offset, limit, searchStr, mimetypes, model, tagNames, headline, months, fromYear, toYear, anyTag, order, suffix, path) { +export async function filesSearch(offset, limit, searchStr, mimetypes, model, tagNames, headline, months, fromYear, toYear, anyTag, order, suffix, path, deep) { const body = {search_str: searchStr, offset: parseInt(offset), limit: parseInt(limit), any_tag: anyTag}; if (suffix) { body['suffix'] = suffix; @@ -1244,6 +1250,9 @@ export async function filesSearch(offset, limit, searchStr, mimetypes, model, ta if (order) { body['order'] = order; } + if (deep) { + body['deep'] = true; + } console.info('searching files', body); const response = await apiPost(`${API_URI}/files/search`, body); @@ -1988,6 +1997,7 @@ export async function searchEstimateFiles(search_str, tagNames, mimetypes, month return { fileGroups: content.file_groups, + fileGroupsDeep: content.file_groups_deep, } } else { console.error('Failed to get file search estimates!'); @@ -2220,7 +2230,7 @@ export async function getDoc(fileGroupId) { } -export async function searchDocs(offset, limit, searchStr, order, tagNames, author, subject, mimetype) { +export async function searchDocs(offset, limit, searchStr, order, tagNames, author, subject, mimetype, deep) { offset = parseInt(offset || 0); limit = parseInt(limit || DEFAULT_LIMIT); let body = {offset, limit}; @@ -2242,6 +2252,9 @@ export async function searchDocs(offset, limit, searchStr, order, tagNames, auth if (mimetype) { body['mimetype'] = mimetype; } + if (deep) { + body['deep'] = true; + } console.debug('searching docs', body); const response = await apiPost(`${DOCS_API}/search`, body); diff --git a/app/src/components/Archive.js b/app/src/components/Archive.js index 67b59837..9416c1b4 100644 --- a/app/src/components/Archive.js +++ b/app/src/components/Archive.js @@ -77,7 +77,7 @@ import { useSearchDomain, useSearchOrder } from "../hooks/customHooks"; -import {FileCards, FileRowTagIcon, FilesView, SearchControlBar} from "./Files"; +import {DeepSearchHint, FileCards, FileRowTagIcon, FilesView, SearchControlBar} from "./Files"; import Grid from "semantic-ui-react/dist/commonjs/collections/Grid"; import _ from "lodash"; import {ThemeContext} from "../contexts/contexts"; @@ -1259,8 +1259,10 @@ function ArchivesPage() { inputRef={searchInputRef} viewButton={viewButton} sorts={archiveOrders} + showDeep={true} /> {body} + {paginator} {body} + {paginator} { @@ -582,6 +586,7 @@ export function SearchFilterModal( setDraftMonths(seededMonths); setDraftRange(seededRange); setDraftCensored(censored || false); + setDraftDeep(deep || false); } }, [open]); @@ -622,6 +627,9 @@ export function SearchFilterModal( if (showCensored && draftCensored !== censored) { params.censored = draftCensored ? 'true' : null; } + if (showDeep && draftDeep !== deep) { + params.deep = draftDeep ? 'true' : null; + } if (Object.keys(params).length > 0) { params.o = 0; updateQuery(params); @@ -641,6 +649,7 @@ export function SearchFilterModal( setDraftMonths([]); setDraftRange(emptyRange); setDraftCensored(false); + setDraftDeep(false); } // Sort section (operates on the draft). @@ -754,6 +763,18 @@ export function SearchFilterModal( } + {showDeep && + + + + + + + } + {showDates && @@ -783,6 +804,7 @@ export function SearchFilterButton( showTags = true, showLimit = true, showCensored = false, + showDeep = false, limitOptions = [12, 24, 48, 96], size = 'medium', content = 'Filter', @@ -794,6 +816,7 @@ export function SearchFilterButton( const {dateRange, months} = useSearchDate(); const {activeTags, anyTag} = useSearch(); const {censored} = useSearchCensored(); + const {deep} = useSearchDeep(); let count = 0; if (fileFilterOptions && filter) { @@ -811,6 +834,9 @@ export function SearchFilterButton( if (showCensored && censored) { count += 1; } + if (showDeep && deep) { + count += 1; + } const active = count > 0; return <> @@ -829,6 +855,7 @@ export function SearchFilterButton( showTags={showTags} showLimit={showLimit} showCensored={showCensored} + showDeep={showDeep} limitOptions={limitOptions} /> @@ -849,6 +876,7 @@ export function SearchControlBar( fileFilterOptions = null, showDates = false, showCensored = false, + showDeep = false, }, ) { const [localSearchStr, setLocalSearchStr] = useState(searchStr || ''); @@ -866,17 +894,39 @@ export function SearchControlBar( {viewButton}
{searchInput}
+ showCensored={showCensored} showDeep={showDeep}/> } +// Offers deep search when a fast search found nothing. Used by the module pages (Videos/Archives/Docs), +// which have no deep estimate; FilesSearchView has a count-aware variant. +export function DeepSearchHint({searchStr, files}) { + const {deep} = useSearchDeep(); + const {updateQuery} = useContext(QueryContext); + + if (!searchStr || deep || !files || files.length > 0) { + return null; + } + + return + Not finding what you need? + + +} + export function FilesSearchView({ showView = true, emptySearch = false, model, + deepEstimate = null, }) { - const {searchFiles, pages} = useSearchFiles(24, emptySearch, model); + const {searchFiles, total, searchStr, pages} = useSearchFiles(24, emptySearch, model); + const {deep} = useSearchDeep(); + const {updateQuery} = useContext(QueryContext); const {body, paginator, viewButton} = FilesView( { @@ -891,9 +941,38 @@ export function FilesSearchView({ }, ); + // The default search only matches titles/authors/descriptions; offer deep search when it would find + // more. `deepEstimate` comes from the search estimates; without it, fall back to a generic hint on + // zero results. + let deepHint = null; + if (Boolean(searchStr) && !deep && searchFiles) { + const moreCount = deepEstimate !== null && deepEstimate !== undefined && total !== null + ? deepEstimate - total : null; + // Enable deep search and reset pagination in a single update (like the filter modal does), so the + // user starts at page 1 of the deep results. + const deepButton = ; + if (searchFiles.length === 0 && (moreCount === null || moreCount > 0)) { + deepHint = + {moreCount > 0 + ? `No results in fast search. ${moreCount} result${moreCount === 1 ? '' : 's'} available in deep search.` + : 'Not finding what you need?'} + {deepButton} + ; + } else if (searchFiles.length > 0 && moreCount > 0) { + deepHint = + {`${moreCount} more result${moreCount === 1 ? '' : 's'} available in deep search.`} + {deepButton} + ; + } + } + return <> {showView &&
{viewButton}
} {body} + {deepHint} {paginator} } diff --git a/app/src/components/Search.js b/app/src/components/Search.js index addbc6ac..ab3d2de4 100644 --- a/app/src/components/Search.js +++ b/app/src/components/Search.js @@ -1,7 +1,7 @@ import React, {useState} from "react"; import {Link, Route, Routes, useLocation, useNavigate} from "react-router"; import {FilesSearchView, SearchViewButton} from "./Files"; -import {useLatestRequest, usePages, useSearchChannels, useSearchDate, useSearchFilter} from "../hooks/customHooks"; +import {useLatestRequest, usePages, useSearchChannels, useSearchDate, useSearchDeep, useSearchFilter} from "../hooks/customHooks"; import {ShortcutHint} from "./ShortcutHint"; import {ZimSearchView} from "./Zim"; import {MapSearchView} from "./MapSearchView"; @@ -159,6 +159,7 @@ export function useSuggestions(searchStr, tagNames, filter, anyTag) { sendGeneralReqeust(async () => await searchSuggestions(searchStr)); sendMapRequest(async () => await searchEstimateMap(searchStr)); } + // The files estimate always uses deep search server-side, so users know results are possible. sendFilesRequest(async () => await searchEstimateFiles(searchStr, tagNames, mimetypes, months, dateRange, anyTag)); sendZimRequest(async () => await searchEstimateZims(searchStr, tagNames)); sendOtherRequest(async () => await searchEstimateOthers(tagNames)); @@ -191,7 +192,7 @@ export function useSuggestions(searchStr, tagNames, filter, anyTag) { React.useEffect(() => { if (!_.isEmpty(filesData)) { setSuggestions((prevState) => { - return {...prevState, fileGroups: filesData.fileGroups} + return {...prevState, fileGroups: filesData.fileGroups, fileGroupsDeep: filesData.fileGroupsDeep} }); } }, [setSuggestions, filesData]); @@ -232,6 +233,7 @@ export function useSearchSuggestions(defaultSearchStr, defaultTagNames, anyTag) const {SingleTag, fuzzyMatchTagsByName} = React.useContext(TagsContext); const {dateRange, months, setDates, clearDate} = useSearchDate(); const {suggestions, loading} = useSuggestions(searchStr, searchTags, filter, anyTag); + const {deep} = useSearchDeep(); const {getLocationStr} = React.useContext(QueryContext); // The results that will be displayed by . @@ -251,19 +253,25 @@ export function useSearchSuggestions(defaultSearchStr, defaultTagNames, anyTag) let results = {}; + // The dropdown always suggests the deep count so users know what is findable. The search results + // page shows the active mode's count, and offers deep search when it found less than promised. + const dropdownEstimate = newSuggestions.fileGroupsDeep ?? newSuggestions.fileGroups; + // The tab badge shows the count matching the active search mode, so it agrees with the result list. + const fileGroupsEstimate = deep ? newSuggestions.fileGroupsDeep : newSuggestions.fileGroups; + // Suggested results are ordered. - if (newSuggestions.fileGroups > 0) { + if (dropdownEstimate > 0) { results.fileGroups = { name: 'Files', results: [ { - title: newSuggestions.fileGroups.toString(), + title: dropdownEstimate.toString(), type: 'files', // Add search query onto current location. location: getLocationStr({q: searchStr}, '/search'), } ] }; - } else if (newSuggestions.fileGroups === 0) { + } else if (dropdownEstimate === 0) { // Tell the user there are no files. results.fileGroups = {name: 'Files', results: noResults}; } @@ -389,7 +397,8 @@ export function useSearchSuggestions(defaultSearchStr, defaultTagNames, anyTag) setSuggestionsResults(results); setSuggestionsSums({ - fileGroups: newSuggestions.fileGroups, + fileGroups: fileGroupsEstimate, + fileGroupsDeep: newSuggestions.fileGroupsDeep, zims: zimSum, otherSum: otherSum, mapPlaces: newSuggestions.mapPlaces || 0, @@ -419,7 +428,7 @@ export function useSearchSuggestions(defaultSearchStr, defaultTagNames, anyTag) normalizeSuggestionsResults(suggestions); } - }, [JSON.stringify(suggestions)]); + }, [JSON.stringify(suggestions), deep]); // Clear timer on unmount. React.useEffect(() => { @@ -495,7 +504,8 @@ export function SearchView({suggestions, suggestionsSums, loading}) { return : null}/> - }/> + }/> }/> }/> }/> diff --git a/app/src/components/SearchFilterModal.test.js b/app/src/components/SearchFilterModal.test.js index 5e919dfb..679e4daf 100644 --- a/app/src/components/SearchFilterModal.test.js +++ b/app/src/components/SearchFilterModal.test.js @@ -146,6 +146,36 @@ describe('SearchFilterButton', () => { expect(screen.getByText('1')).toBeInTheDocument(); }); + it('counts the deep filter and applies it on close when showDeep is set', () => { + const spy = jest.fn(); + renderButton({sorts: videoOrders, showDeep: true}, {}, spy); + + // No deep badge initially. + expect(screen.queryByText('1')).toBeNull(); + + fireEvent.click(screen.getByText('Filter')); + // The Search Depth section/toggle is shown. + expect(screen.getByText('Search Depth')).toBeInTheDocument(); + fireEvent.mouseUp(screen.getByTestId('toggle')); + // Draft only — nothing applied yet. + expect(spy).not.toHaveBeenCalled(); + + fireEvent.click(screen.getByText('Done')); + expect(spy).toHaveBeenCalledTimes(1); + expect(spy.mock.calls[0][0]).toMatchObject({deep: 'true'}); + }); + + it('does not show the Search Depth section unless showDeep is set', () => { + renderButton({sorts: videoOrders}); + fireEvent.click(screen.getByText('Filter')); + expect(screen.queryByText('Search Depth')).toBeNull(); + }); + + it('shows a count badge when the deep filter is active in the URL', () => { + renderButton({sorts: videoOrders, showDeep: true}, {deep: 'true'}); + expect(screen.getByText('1')).toBeInTheDocument(); + }); + it('shows the File Type section when fileFilterOptions are provided', () => { renderButton({ sorts: videoOrders, diff --git a/app/src/components/Videos.js b/app/src/components/Videos.js index 50c75aed..b3e434d1 100644 --- a/app/src/components/Videos.js +++ b/app/src/components/Videos.js @@ -45,7 +45,7 @@ import { TableCell } from "semantic-ui-react"; import {useChannel, useSearchOrder, useSearchVideos, useVideo, useVideoStatistics} from "../hooks/customHooks"; -import {FileRowTagIcon, FilesView, SearchControlBar} from "./Files"; +import {DeepSearchHint, FileRowTagIcon, FilesView, SearchControlBar} from "./Files"; import Grid from "semantic-ui-react/dist/commonjs/collections/Grid"; import Icon from "semantic-ui-react/dist/commonjs/elements/Icon"; import {Button, Card, Header, Loader, Modal, Placeholder, Popup, Segment, Statistic} from "./Theme"; @@ -241,8 +241,10 @@ export function VideosPage() { viewButton={viewButton} sorts={videoOrders} showCensored={true} + showDeep={true} /> {body} + {paginator} { const activeTags = searchParams.getAll('tag'); const {view} = useSearchView(); const headline = view === 'headline'; + const {deep} = useSearchDeep(); const [archives, setArchives] = useState(null); const [totalPages, setTotalPages] = useState(0); @@ -519,7 +520,8 @@ export const useSearchArchives = (defaultLimit) => { setArchives(null); setTotalPages(0); try { - let [archives_, total] = await searchArchives(offset, limit, domain, searchStr, order, activeTags, headline); + let [archives_, total] = await searchArchives(offset, limit, domain, searchStr, order, activeTags, headline, + deep); setArchives(archives_); setTotalPages(calculateTotalPages(total, limit)); } catch (e) { @@ -536,7 +538,7 @@ export const useSearchArchives = (defaultLimit) => { useEffect(() => { localSearchArchives(); - }, [searchStr, limit, domain, order, offset, JSON.stringify(activeTags), headline]); + }, [searchStr, limit, domain, order, offset, JSON.stringify(activeTags), headline, deep]); const setSearchStr = (value) => { updateQuery({q: value, o: 0, order: undefined}); @@ -580,6 +582,7 @@ export const useSearchDocs = (defaultLimit) => { const author = searchParams.get('author') || ''; const subject = searchParams.get('subject') || ''; const filter = searchParams.get('filter') || ''; + const {deep} = useSearchDeep(); const [docs, setDocs] = useState(null); const [totalPages, setTotalPages] = useState(0); @@ -589,7 +592,8 @@ export const useSearchDocs = (defaultLimit) => { setTotalPages(0); try { const mimetype = docFilterToMimetype(filter); - let [docs_, total] = await searchDocs(offset, limit, searchStr, order, activeTags, author, subject, mimetype); + let [docs_, total] = await searchDocs(offset, limit, searchStr, order, activeTags, author, subject, + mimetype, deep); setDocs(docs_); setTotalPages(calculateTotalPages(total, limit)); } catch (e) { @@ -600,7 +604,7 @@ export const useSearchDocs = (defaultLimit) => { useEffect(() => { localSearchDocs(); - }, [searchStr, limit, order, offset, JSON.stringify(activeTags), author, subject, filter]); + }, [searchStr, limit, order, offset, JSON.stringify(activeTags), author, subject, filter, deep]); const setSearchStr = (value) => { updateQuery({q: value, o: 0, order: undefined}); @@ -639,6 +643,7 @@ export const useSearchVideos = (defaultLimit, channelId, order_by) => { const headline = view === 'headline'; const anyTag = searchParams.get('anyTag') === 'true'; const censored = searchParams.get('censored') === 'true'; + const {deep} = useSearchDeep(); const [videos, setVideos] = useState(null); const [totalPages, setTotalPages] = useState(0); @@ -649,7 +654,8 @@ export const useSearchVideos = (defaultLimit, channelId, order_by) => { setLoading(true); setTotalPages(0); try { - let [videos_, total] = await searchVideos(offset, limit, channelId, searchStr, order, activeTags, headline, censored); + let [videos_, total] = await searchVideos(offset, limit, channelId, searchStr, order, activeTags, headline, + censored, deep); setVideos(videos_); setTotalPages(calculateTotalPages(total, limit)); } catch (e) { @@ -662,7 +668,7 @@ export const useSearchVideos = (defaultLimit, channelId, order_by) => { useEffect(() => { localSearchVideos(); - }, [searchStr, limit, channelId, offset, order_by, JSON.stringify(activeTags), headline, censored]); + }, [searchStr, limit, channelId, offset, order_by, JSON.stringify(activeTags), headline, censored, deep]); const setSearchStr = (value) => { updateQuery({q: value, o: 0, order: undefined}); @@ -912,8 +918,10 @@ export const useSearchFiles = (defaultLimit = 48, emptySearch = false, model) => dateRange, } = useSearch(defaultLimit, emptySearch, model); const {view} = useSearchView(); + const {deep} = useSearchDeep(); const [searchFiles, setSearchFiles] = useState(null); + const [total, setTotal] = useState(null); const [loading, setLoading] = useState(false); const headline = view === 'headline'; @@ -932,12 +940,14 @@ export const useSearchFiles = (defaultLimit = 48, emptySearch = false, model) => setLoading(true); console.log('localSearchFiles', fromDate, toDate, months); try { - let [file_groups, total] = await filesSearch( + let [file_groups, total_] = await filesSearch( pages.offset, pages.limit, searchStr, mimetypes, model || model_, activeTags, headline, - months, fromDate, toDate, anyTag); + months, fromDate, toDate, anyTag, null, null, null, deep); setSearchFiles(file_groups); - pages.setTotal(total); + setTotal(total_); + pages.setTotal(total_); } catch (e) { + setTotal(null); pages.setTotal(0); console.error(e); toast({ @@ -974,10 +984,12 @@ export const useSearchFiles = (defaultLimit = 48, emptySearch = false, model) => JSON.stringify(months), JSON.stringify(dateRange), anyTag, + deep, ]); return { searchFiles, + total, searchStr, filter, setSearchStr, @@ -1655,6 +1667,14 @@ export const useSearchCensored = () => { return {censored, setCensored} } +export const useSearchDeep = () => { + // Search all text including captions/document contents (?deep=true). Slower, but more thorough. + const [value, setValue] = useOneQuery('deep'); + const deep = value === 'true'; + const setDeep = (newValue) => setValue(newValue ? 'true' : undefined); + return {deep, setDeep} +} + export const useSearchDate = () => { // Filters files using // ?month=1&month=2&month=3 diff --git a/modules/archive/api.py b/modules/archive/api.py index 506ecaea..ebaa9725 100644 --- a/modules/archive/api.py +++ b/modules/archive/api.py @@ -61,7 +61,7 @@ async def search_archives(_: Request, body: schema.ArchiveSearchRequest): offset = body.offset or 0 file_groups, total = lib.search_archives(search_str, domain, limit, offset, body.order_by, body.tag_names, - body.headline) + body.headline, body.deep) ret = dict(file_groups=file_groups, totals=dict(file_groups=total)) return json_response(ret) diff --git a/modules/archive/lib.py b/modules/archive/lib.py index b122456a..778e1de2 100644 --- a/modules/archive/lib.py +++ b/modules/archive/lib.py @@ -1400,7 +1400,7 @@ NO_NULL_ORDERS = { def search_archives(search_str: str, domain: str, limit: int, offset: int, order: str, tag_names: List[str], - headline: bool = False) \ + headline: bool = False, deep: bool = False) \ -> Tuple[List[dict], int]: # Always filter FileGroups to Archives. wheres = [] @@ -1412,9 +1412,11 @@ def search_archives(search_str: str, domain: str, limit: int, offset: int, order select_columns = '' if search_str: # A search_str was provided by the user, modify the query to filter by it. - select_columns = 'ts_rank(fg.textsearch, websearch_to_tsquery(%(search_str)s)) AS rank,' \ + # `textsearch` includes d_text (page contents); `textsearch_abc` is much smaller and faster. + ts_col = 'textsearch' if deep else 'textsearch_abc' + select_columns = f'ts_rank(fg.{ts_col}, websearch_to_tsquery(%(search_str)s)) AS rank,' \ ' COUNT(*) OVER() AS total' - wheres.append('fg.textsearch @@ websearch_to_tsquery(%(search_str)s)') + wheres.append(f'fg.{ts_col} @@ websearch_to_tsquery(%(search_str)s)') params['search_str'] = search_str group_by = f'{group_by}, rank' diff --git a/modules/archive/schema.py b/modules/archive/schema.py index 7e9b5cd0..b963a545 100644 --- a/modules/archive/schema.py +++ b/modules/archive/schema.py @@ -24,6 +24,7 @@ class ArchiveSearchRequest: order_by: Optional[str] = None tag_names: List[str] = field(default_factory=list) headline: bool = False + deep: bool = False # When True, search all text including page contents (d_text) @dataclass diff --git a/modules/archive/test/test_api.py b/modules/archive/test/test_api.py index 05f434c5..c92df3d1 100644 --- a/modules/archive/test/test_api.py +++ b/modules/archive/test/test_api.py @@ -50,36 +50,43 @@ async def test_archives_search(test_session, archive_directory, archive_factory, archive_factory('example.org') # has no contents test_session.commit() + # Contents are d_text, so a deep search is required to match them. # 1 and 2 contain "foo". - data = {'search_str': 'foo'} + data = {'search_str': 'foo', 'deep': True} await check_results(async_client, data, [2, 1]) + # The default (fast) search does not match contents. + data = {'search_str': 'foo'} + await check_results(async_client, data, []) + # 2 and 3 contain "baz". - data = {'search_str': 'baz'} + data = {'search_str': 'baz', 'deep': True} await check_results(async_client, data, [3, 2]) # 1 contains "bar". - data = {'search_str': 'bar'} + data = {'search_str': 'bar', 'deep': True} await check_results(async_client, data, [1, ]) # No archives contain "huzzah" - data = {'search_str': 'huzzah'} + data = {'search_str': 'huzzah', 'deep': True} await check_results(async_client, data, []) # Only 3 contains "baz" and is in domain "example.org" - data = {'search_str': 'baz', 'domain': 'example.org'} + data = {'search_str': 'baz', 'domain': 'example.org', 'deep': True} await check_results(async_client, data, [3, ]) # 1's title contains "my", this is ignored by Postgres. - data = {'search_str': 'my'} + data = {'search_str': 'my', 'deep': True} await check_results(async_client, data, []) - # 3's title contains "third". + # 3's title contains "third", found by the default (fast) search and the deep search. data = {'search_str': 'third'} await check_results(async_client, data, [3, ]) + data = {'search_str': 'third', 'deep': True} + await check_results(async_client, data, [3, ]) # All contents contain "qux", but they contain different amounts. They are ordered by the amount. - data = {'search_str': 'qux'} + data = {'search_str': 'qux', 'deep': True} await check_results(async_client, data, [3, 2, 1]) data = {'search_str': 'qux', 'order_by': 'bad order_by'} @@ -116,7 +123,8 @@ async def test_archives_search_headline(test_session, archive_directory, archive archive_factory('example.org') # has no contents test_session.commit() - content = dict(search_str='foo') + # "foo" is only in the contents (d_text), so a deep search is required to match it. + content = dict(search_str='foo', deep=True) request, response = await async_client.post('/api/archive/search', content=json.dumps(content)) assert response.status_code == HTTPStatus.OK @@ -124,7 +132,7 @@ async def test_archives_search_headline(test_session, archive_directory, archive assert response.json['file_groups'][0]['d_headline'] is None assert response.json['file_groups'][1]['d_headline'] is None - content = dict(search_str='foo', headline=True) + content = dict(search_str='foo', headline=True, deep=True) request, response = await async_client.post('/api/archive/search', content=json.dumps(content)) assert response.status_code == HTTPStatus.OK diff --git a/modules/archive/test/test_lib.py b/modules/archive/test/test_lib.py index 5b99e37b..625782d0 100644 --- a/modules/archive/test/test_lib.py +++ b/modules/archive/test/test_lib.py @@ -571,7 +571,8 @@ async def test_refresh_archives(test_session, test_directory, async_client, make assert (test_directory / 'archive/example.com/2021-10-05-16-20-10_NA.html').is_file() assert test_session.query(Archive).count() == 2 - content = json.dumps({'search_str': 'text', 'model': 'archive'}) + # "text" is only in the readability contents (d_text), so a deep search is required. + content = json.dumps({'search_str': 'text', 'model': 'archive', 'deep': True}) request, response = await async_client.post('/api/files/search', content=content) assert response.status_code == HTTPStatus.OK assert response.json['file_groups'], 'No files matched "text"' diff --git a/modules/docs/api.py b/modules/docs/api.py index c69c1cb4..2aeeded3 100644 --- a/modules/docs/api.py +++ b/modules/docs/api.py @@ -56,6 +56,7 @@ async def search_docs(_: Request, body: schema.DocSearchRequest): offset=offset, order_by=body.order_by, tag_names=body.tag_names, + deep=body.deep, ) ret = dict(file_groups=file_groups, totals=dict(file_groups=total)) return json_response(ret) diff --git a/modules/docs/lib.py b/modules/docs/lib.py index 708b3d70..79060638 100644 --- a/modules/docs/lib.py +++ b/modules/docs/lib.py @@ -347,15 +347,18 @@ def _get_doc(session, file_group_id: int): def _search_docs(search_str=None, author=None, subject=None, language=None, mimetype=None, - limit=20, offset=0, order_by='published_datetime', tag_names=None): + limit=20, offset=0, order_by='published_datetime', tag_names=None, deep=False): from .models import Doc from wrolpi.files.models import FileGroup + # `textsearch` includes d_text (document contents); `textsearch_abc` is much smaller and faster. + ts_col = FileGroup.textsearch if deep else FileGroup.textsearch_abc + with get_db_session() as session: query = session.query(FileGroup).join(Doc, Doc.file_group_id == FileGroup.id) if search_str: - query = query.filter(FileGroup.textsearch.op('@@')(func.websearch_to_tsquery(search_str))) + query = query.filter(ts_col.op('@@')(func.websearch_to_tsquery(search_str))) if author: query = query.filter(FileGroup.b_text.ilike(f'%{author}%')) @@ -382,7 +385,7 @@ def _search_docs(search_str=None, author=None, subject=None, language=None, mime # Ordering. if order_by == 'rank' and search_str: query = query.order_by( - desc(func.ts_rank(FileGroup.textsearch, func.websearch_to_tsquery(search_str))), + desc(func.ts_rank(ts_col, func.websearch_to_tsquery(search_str))), desc(FileGroup.id), ) elif order_by == 'published_datetime': diff --git a/modules/docs/schema.py b/modules/docs/schema.py index a2ad7c8a..c95a09d9 100644 --- a/modules/docs/schema.py +++ b/modules/docs/schema.py @@ -13,6 +13,7 @@ class DocSearchRequest: order_by: Optional[str] = 'published_datetime' limit: int = 20 offset: int = 0 + deep: bool = False # When True, search all text including document contents (d_text) @dataclasses.dataclass diff --git a/modules/docs/test/test_lib.py b/modules/docs/test/test_lib.py index c6dc661a..2db26844 100644 --- a/modules/docs/test/test_lib.py +++ b/modules/docs/test/test_lib.py @@ -289,7 +289,11 @@ async def test_search_docs_attaches_section_hint_epub(test_session, test_directo (2, 'Chapter 2', 'unrelated content about pottery'), ]) + # "mullen" is only in the document body (d_text), so a deep search is required to match it. results, total = _search_docs(search_str='mullen', mimetype='application/epub') + assert total == 0, 'Document contents are not searched unless deep=True' + + results, total = _search_docs(search_str='mullen', mimetype='application/epub', deep=True) assert total == 1 assert results[0]['id'] == fg.id hint = results[0]['section_hint'] @@ -311,7 +315,7 @@ async def test_search_docs_attaches_section_hint_pdf(test_session, test_director (3, 'Page 3', 'here is where mullen shows up'), ]) - results, total = _search_docs(search_str='mullen', mimetype='application/pdf') + results, total = _search_docs(search_str='mullen', mimetype='application/pdf', deep=True) assert total == 1 assert results[0]['id'] == fg.id hint = results[0]['section_hint'] diff --git a/modules/videos/conftest.py b/modules/videos/conftest.py index 7241b255..5f42e5e8 100644 --- a/modules/videos/conftest.py +++ b/modules/videos/conftest.py @@ -336,6 +336,7 @@ def assert_video_search(async_client): order_by: str = None, channel_id: int = None, censored: bool = None, + deep: bool = None, ): content = dict() if search_str is not None: @@ -352,6 +353,8 @@ def assert_video_search(async_client): content['channel_id'] = channel_id if censored is not None: content['censored'] = censored + if deep is not None: + content['deep'] = deep request, response = await async_client.post('/api/videos/search', content=json.dumps(content)) diff --git a/modules/videos/schema.py b/modules/videos/schema.py index 7da8ae3e..711de62e 100644 --- a/modules/videos/schema.py +++ b/modules/videos/schema.py @@ -178,6 +178,7 @@ class VideoSearchRequest: tag_names: List[str] = field(default_factory=list) headline: bool = False censored: bool = False + deep: bool = False # When True, search all text including captions (d_text) @dataclass diff --git a/modules/videos/test/test_api.py b/modules/videos/test/test_api.py index 6f714996..2a496c0b 100644 --- a/modules/videos/test/test_api.py +++ b/modules/videos/test/test_api.py @@ -195,31 +195,36 @@ async def test_search_videos_file(test_session, test_directory, video_with_searc video_with_search_factory(title=title, d_text=caption) test_session.commit() + # Captions are d_text, so a deep search is required to match them. # Repeated runs should return the same result for _ in range(2): # Only videos with a b are returned, ordered by the amount of b's - await assert_video_search(search_str='b', assert_ids=[1, 2, 3, 4]) + await assert_video_search(search_str='b', deep=True, assert_ids=[1, 2, 3, 4]) + + # The default (fast) search does not match captions. + await assert_video_search(search_str='b', assert_ids=[]) # Only two captions have e - await assert_video_search(search_str='e', assert_ids=[4, 1]) + await assert_video_search(search_str='e', deep=True, assert_ids=[4, 1]) # Only 2 contains 2 - await assert_video_search(search_str='2', assert_ids=[2, ]) + await assert_video_search(search_str='2', deep=True, assert_ids=[2, ]) # Only two captions have d - await assert_video_search(search_str='d', assert_ids=[1, 2]) + await assert_video_search(search_str='d', deep=True, assert_ids=[1, 2]) - # 5 can be gotten by it's title + # 5 can be gotten by it's title, in either mode. await assert_video_search(search_str='5', assert_ids=[5, ]) + await assert_video_search(search_str='5', deep=True, assert_ids=[5, ]) # only video 1 has e and d - await assert_video_search(search_str='e d', assert_ids=[1, ]) + await assert_video_search(search_str='e d', deep=True, assert_ids=[1, ]) # video 1 and 4 have b and e, but 1 has more - await assert_video_search(search_str='b e', assert_ids=[1, 4]) + await assert_video_search(search_str='b e', deep=True, assert_ids=[1, 4]) # Check totals are correct even with a limit - await assert_video_search(search_str='b', limit=2, assert_ids=[1, 2], assert_total=4) + await assert_video_search(search_str='b', deep=True, limit=2, assert_ids=[1, 2], assert_total=4) @pytest.mark.asyncio diff --git a/modules/videos/test/test_downloader.py b/modules/videos/test/test_downloader.py index 83dd5421..375ee632 100644 --- a/modules/videos/test/test_downloader.py +++ b/modules/videos/test/test_downloader.py @@ -16,6 +16,7 @@ from modules.videos.models import Channel, Video from wrolpi.conftest import test_directory, await_switches from wrolpi.downloader import Download, DownloadResult, get_download_manager_config from wrolpi.errors import InvalidDownload, UnrecoverableDownloadError +from wrolpi.test.common import skip_circleci from wrolpi.vars import PROJECT_DIR example_video_json = { @@ -130,6 +131,7 @@ async def test_download_video_tags(test_session, video_download_manager, video_f assert [i.tag.name for i in video.file_group.tag_files] == [tag1.name, tag2.name] +@skip_circleci @pytest.mark.asyncio async def test_download_channel(test_session, test_directory, simple_channel, video_download_manager, video_file, mock_video_extract_info, mock_video_prepare_filename, await_switches, diff --git a/modules/videos/video/api.py b/modules/videos/video/api.py index 4f9f3dd9..4a4b96b6 100644 --- a/modules/videos/video/api.py +++ b/modules/videos/video/api.py @@ -66,6 +66,7 @@ async def search_videos(_: Request, body: schema.VideoSearchRequest): body.tag_names, body.headline, body.censored, + body.deep, ) ret = {'file_groups': list(file_groups), 'totals': {'file_groups': videos_total}} diff --git a/modules/videos/video/lib.py b/modules/videos/video/lib.py index 9de2065c..5e30f463 100644 --- a/modules/videos/video/lib.py +++ b/modules/videos/video/lib.py @@ -93,6 +93,7 @@ def search_videos( tag_names: List[str] = None, headline: bool = False, censored: bool = False, + deep: bool = False, ) -> Tuple[List[dict], int]: tag_names = tag_names or [] # Search videos and audio-only files. @@ -112,8 +113,10 @@ def search_videos( if search_str: # A search_str was provided by the user, modify the query to filter by it. - select_columns = 'fg.id, ts_rank(fg.textsearch, websearch_to_tsquery(%(search_str)s)), COUNT(*) OVER() AS total' - wheres.append('fg.textsearch @@ websearch_to_tsquery(%(search_str)s)') + # `textsearch` includes d_text (captions); `textsearch_abc` is much smaller and faster. + ts_col = 'textsearch' if deep else 'textsearch_abc' + select_columns = f'fg.id, ts_rank(fg.{ts_col}, websearch_to_tsquery(%(search_str)s)), COUNT(*) OVER() AS total' + wheres.append(f'fg.{ts_col} @@ websearch_to_tsquery(%(search_str)s)') params['search_str'] = search_str else: # No search_str provided. Get id and total only. diff --git a/wrolpi/files/api.py b/wrolpi/files/api.py index 9463ec3e..2a8875f4 100644 --- a/wrolpi/files/api.py +++ b/wrolpi/files/api.py @@ -165,7 +165,7 @@ async def post_search_files(_: Request, body: schema.FilesSearchRequest): with timer('Searching all files', 'info', logger__=logger): file_groups, total = lib.search_files(body.search_str, body.limit, body.offset, body.mimetypes, body.model, body.tag_names, body.headline, body.months, body.from_year, body.to_year, - body.any_tag, body.order, body.url, body.suffix, body.path) + body.any_tag, body.order, body.url, body.suffix, body.path, body.deep) return json_response(dict(file_groups=file_groups, totals=dict(file_groups=total))) diff --git a/wrolpi/files/lib.py b/wrolpi/files/lib.py index c461eb6a..b2fa0c12 100644 --- a/wrolpi/files/lib.py +++ b/wrolpi/files/lib.py @@ -1039,7 +1039,7 @@ async def search_directories_by_name(session: Session, name: str, excluded: List def search_files(search_str: str, limit: int, offset: int, mimetypes: List[str] = None, model: str = None, tag_names: List[str] = None, headline: bool = False, months: List[int] = None, from_year: int = None, to_year: int = None, any_tag: bool = False, order: str = None, - url: str = None, suffix: str = None, path: str = None) -> \ + url: str = None, suffix: str = None, path: str = None, deep: bool = False) -> \ Tuple[List[dict], int]: """Search the FileGroup table. @@ -1061,6 +1061,7 @@ def search_files(search_str: str, limit: int, offset: int, mimetypes: List[str] @param url: Filter by URL using case-insensitive partial match (ILIKE). @param suffix: Only return files whose primary file has this suffix (e.g. ".bin"), case-insensitive. @param path: Filter by primary_path using case-insensitive partial match (ILIKE). + @param deep: Search all text including captions/body (d_text); slower but more thorough. """ params = dict(offset=offset, limit=limit, search_str=search_str, url_search_str=f'%{search_str}%') wheres = [] @@ -1069,9 +1070,10 @@ def search_files(search_str: str, limit: int, offset: int, mimetypes: List[str] joins = [] if search_str: - # Search by textsearch column. - wheres.append('textsearch @@ websearch_to_tsquery(%(search_str)s)') - selects.append('ts_rank(textsearch, websearch_to_tsquery(%(search_str)s))') + # `textsearch` includes d_text (captions/body); `textsearch_abc` is much smaller and faster. + ts_col = 'textsearch' if deep else 'textsearch_abc' + wheres.append(f'{ts_col} @@ websearch_to_tsquery(%(search_str)s)') + selects.append(f'ts_rank({ts_col}, websearch_to_tsquery(%(search_str)s))') order_by = '2 DESC' wheres, params = mimetypes_to_sql_wheres(wheres, params, mimetypes) @@ -1479,9 +1481,13 @@ def mimetypes_to_sql_wheres(wheres: List[str], params: dict, mimetypes: List[str async def search_file_suggestion_count(search_str: str, tag_names: List[str], mimetypes: List[str], months: List[int] = None, from_year: int = None, to_year: int = None, - any_tag: bool = False): + any_tag: bool = False) -> dict: """ - Return FileGroup count of what will be returned if the search is actually performed. + Return FileGroup counts of what will be returned if the search is actually performed. + + Returns both the fast (`textsearch_abc`) and deep (`textsearch`) counts so the UI can show the count + matching the current search mode, and offer deep search when it has more results. Both are cheap + index-bound COUNTs (no ts_rank). """ wheres = [] joins = list() @@ -1490,8 +1496,13 @@ async def search_file_suggestion_count(search_str: str, tag_names: List[str], mi having = '' if search_str: - # Search by textsearch, and by matching url. + # `textsearch` matches are a superset of `textsearch_abc`, so filter by the deep column and count + # the fast subset with FILTER in the same pass. wheres.append('fg.textsearch @@ websearch_to_tsquery(%(search_str)s)') + selects = ('COUNT(*) FILTER (WHERE fg.textsearch_abc @@ websearch_to_tsquery(%(search_str)s)) AS estimate,' + ' COUNT(*) AS deep_estimate') + else: + selects = 'COUNT(*) AS estimate, COUNT(*) AS deep_estimate' wheres, params = tag_append_sub_select_where(wheres, params, tag_names, any_tag) wheres, params = mimetypes_to_sql_wheres(wheres, params, mimetypes) @@ -1501,7 +1512,7 @@ async def search_file_suggestion_count(search_str: str, tag_names: List[str], mi joins = "\n".join(joins) wheres = 'WHERE ' + "\nAND ".join(wheres) if wheres else '' stmt = f''' - SELECT COUNT(*) OVER() AS estimate + SELECT {selects} FROM file_group fg {joins} {wheres} @@ -1514,8 +1525,8 @@ async def search_file_suggestion_count(search_str: str, tag_names: List[str], mi curs.execute(stmt, params) result = curs.fetchone() if result: - return int(result['estimate']) - return 0 + return dict(file_groups=int(result['estimate']), file_groups_deep=int(result['deep_estimate'])) + return dict(file_groups=0, file_groups_deep=0) MOVE_CHUNK_SIZE = 100 diff --git a/wrolpi/files/models.py b/wrolpi/files/models.py index 63e59b10..f48f4c34 100644 --- a/wrolpi/files/models.py +++ b/wrolpi/files/models.py @@ -79,6 +79,7 @@ class FileGroup(ModelHelper, Base): Index('file_group_published_datetime_idx', 'published_datetime'), Index('file_group_published_modified_datetime_idx', 'published_modified_datetime'), Index('file_group_size_ix', 'size'), + Index('file_group_textsearch_abc_idx', 'textsearch_abc'), Index('file_group_textsearch_idx', 'textsearch'), Index('file_group_url_idx', 'url'), Index('file_group_viewed_idx', 'viewed'), @@ -126,6 +127,13 @@ class FileGroup(ModelHelper, Base): setweight(to_tsvector('english'::regconfig, COALESCE(c_text, '')), 'C'::"char") || setweight(to_tsvector('english'::regconfig, COALESCE(d_text, '')), 'D'::"char") '''))) + # Excludes d_text (captions/body); much smaller than `textsearch`, used for the default fast search. + textsearch_abc = deferred( + Column(tsvector, Computed(''' + setweight(to_tsvector('english'::regconfig, COALESCE(a_text, '')), 'A'::"char") || + setweight(to_tsvector('english'::regconfig, COALESCE(b_text, '')), 'B'::"char") || + setweight(to_tsvector('english'::regconfig, COALESCE(c_text, '')), 'C'::"char") + '''))) def __repr__(self): m = f'model={self.model}' if self.model else f'mimetype={self.mimetype}' diff --git a/wrolpi/files/schema.py b/wrolpi/files/schema.py index 2f085e46..0eac4945 100644 --- a/wrolpi/files/schema.py +++ b/wrolpi/files/schema.py @@ -44,6 +44,7 @@ class FilesSearchRequest: url: Optional[str] = None # Filter by URL (partial match) suffix: Optional[str] = None # Filter by primary file suffix (e.g. ".bin"), case-insensitive exact match path: Optional[str] = None # Filter by primary_path (case-insensitive partial match) + deep: bool = False # When True, search all text including captions/body (d_text) def __post_init__(self): if self.any_tag and self.tag_names: diff --git a/wrolpi/files/test/test_api.py b/wrolpi/files/test/test_api.py index eb7b10bf..ae999fe6 100644 --- a/wrolpi/files/test/test_api.py +++ b/wrolpi/files/test/test_api.py @@ -315,6 +315,51 @@ async def test_files_search(test_session, async_client, make_files_structure, as await assert_files_search('nothing', []) +@pytest.mark.asyncio +async def test_files_search_deep(test_session, async_client, make_files_structure): + """Search defaults to the fast ABC column (title/author/description); `deep` opts into d_text + (captions/file contents).""" + guide, lecture = make_files_structure([ + 'gardening guide.txt', + 'lecture.txt', + ]) + guide.write_text('planting tips') + # "gardening" only appears in this file's contents (d_text), not its name. + lecture.write_text('a lecture about gardening') + + request, response = await async_client.post('/api/files/refresh') + assert response.status_code == HTTPStatus.NO_CONTENT + + async def search(**kwargs): + request, response = await async_client.post('/api/files/search', json=kwargs) + assert response.status_code == HTTPStatus.OK + return response.json['file_groups'] + + # Default (fast) search only matches the title. + results = await search(search_str='gardening') + assert [i['primary_path'] for i in results] == ['gardening guide.txt'] + + # Deep search also matches file contents; the title match ranks higher (weight A vs D). + results = await search(search_str='gardening', deep=True) + assert [i['primary_path'] for i in results] == ['gardening guide.txt', 'lecture.txt'] + + # Headlines work in both modes. + results = await search(search_str='gardening', headline=True) + assert [i['primary_path'] for i in results] == ['gardening guide.txt'] + assert 'gardening' in results[0]['title_headline'] + results = await search(search_str='gardening', deep=True, headline=True) + assert [i['primary_path'] for i in results] == ['gardening guide.txt', 'lecture.txt'] + assert 'gardening' in results[1]['d_headline'] + + # The estimate endpoint returns both counts: `file_groups` matches the default fast search, and + # `file_groups_deep` shows what deep search would find, so the UI can offer deep search. + request, response = await async_client.post('/api/search_file_estimates', + json=dict(search_str='gardening')) + assert response.status_code == HTTPStatus.OK + assert response.json['file_groups'] == 1 + assert response.json['file_groups_deep'] == 2 + + @pytest.mark.asyncio async def test_files_search_by_suffix(test_session, async_client, make_files_structure): """Files can be filtered by their primary file's suffix (used by the firmware flasher to find .bin files).""" @@ -367,7 +412,8 @@ async def test_files_search_attaches_doc_section_hint(async_client, test_session bug where only the docs-specific endpoint (/api/docs/search) attached hints.""" await refresh_files() - body = dict(search_str='chapter') + # "chapter" only appears in the EPUB body (d_text), so deep search is required to match it. + body = dict(search_str='chapter', deep=True) request, response = await async_client.post('/api/files/search', json=body) assert response.status_code == HTTPStatus.OK diff --git a/wrolpi/files/test/test_lib.py b/wrolpi/files/test/test_lib.py index 52d9748a..a2da0071 100644 --- a/wrolpi/files/test/test_lib.py +++ b/wrolpi/files/test/test_lib.py @@ -580,18 +580,20 @@ async def test_files_indexer(async_client, test_session, make_files_structure, t # Video are indexed by the modeler, not by an indexer. assert video_file.mimetype == 'video/mp4' and video_file.indexer == indexers.DefaultIndexer - # File are indexed by their titles and contents. + # File are indexed by their titles and contents. Contents (d_text) require a deep search. files, total = lib.search_files('file', 10, 0) assert total == 5, 'All files contain "file" in their file name. The associated video file is hidden.' files, total = lib.search_files('image', 10, 0) assert total == 1 and files[0]['title'] == 'an image file.jpeg', 'The image file title contains "image".' - files, total = lib.search_files('contents', 10, 0) + files, total = lib.search_files('contents', 10, 0, deep=True) assert total == 1 and files[0]['title'] == 'a text file.txt', 'The text file contains "contents".' + files, total = lib.search_files('contents', 10, 0) + assert total == 0, 'File contents are not searched unless deep=True.' files, total = lib.search_files('video', 10, 0) assert total == 1 and {i['title'] for i in files} == {'a video file'}, 'The video file contains "video".' - files, total = lib.search_files('yawn', 10, 0) + files, total = lib.search_files('yawn', 10, 0, deep=True) assert total == 1 and files[0]['title'] == 'a video file', 'The video file captions contain "yawn".' - files, total = lib.search_files('bunny', 10, 0) + files, total = lib.search_files('bunny', 10, 0, deep=True) assert total == 1 and {i['title'] for i in files} == {'a zip file.zip'}, \ 'The zip file contains a file with "bunny" in the title.' @@ -603,7 +605,7 @@ async def test_files_indexer(async_client, test_session, make_files_structure, t text_path.write_text('new text contents') await refresh_files() await await_background_tasks() - files, total = lib.search_files('new', 10, 0) + files, total = lib.search_files('new', 10, 0, deep=True) assert total == 1 diff --git a/wrolpi/root_api.py b/wrolpi/root_api.py index 506bfd86..cb5cdcc0 100644 --- a/wrolpi/root_api.py +++ b/wrolpi/root_api.py @@ -768,7 +768,7 @@ async def post_search_suggestions(request: Request, body: schema.SearchSuggestio ) async def post_search_file_estimates(_: Request, body: schema.SearchFileEstimateRequest): """Used by the Global search to suggest FileGroup count to the user.""" - file_groups = await search_file_suggestion_count( + counts = await search_file_suggestion_count( body.search_str, body.tag_names, body.mimetypes, @@ -779,7 +779,8 @@ async def post_search_file_estimates(_: Request, body: schema.SearchFileEstimate ) ret = dict( - file_groups=file_groups, + file_groups=counts['file_groups'], + file_groups_deep=counts['file_groups_deep'], ) return json_response(ret) diff --git a/wrolpi/test/test_root_api.py b/wrolpi/test/test_root_api.py index a608ff75..1f22c275 100644 --- a/wrolpi/test/test_root_api.py +++ b/wrolpi/test/test_root_api.py @@ -666,7 +666,8 @@ async def test_search_file_estimates(test_session, async_client, archive_factory request, response = await async_client.post('/api/search_file_estimates', json=body) assert response.status_code == HTTPStatus.OK - assert response.json['file_groups'] == expected_file_groups or 0 + # The deep count includes contents/captions matches; the fast (ABC) count is in `file_groups`. + assert response.json['file_groups_deep'] == expected_file_groups or 0 await assert_results(dict(search_str='foo'), 1) @@ -727,7 +728,8 @@ async def test_search_file_estimates_any_tag(test_session, async_client, archive request, response = await async_client.post('/api/search_file_estimates', json=body) assert response.status_code == HTTPStatus.OK - assert response.json['file_groups'] == expected_file_groups or 0 + # The deep count includes contents/captions matches; the fast (ABC) count is in `file_groups`. + assert response.json['file_groups_deep'] == expected_file_groups or 0 await assert_results(dict(search_str='bunny'), 2) await assert_results(dict(search_str='bunny', any_tag=True), 1) From 2162396212071914ae9e0191b03292f7afe9d21c Mon Sep 17 00:00:00 2001 From: Roland Date: Fri, 3 Jul 2026 10:46:22 -0600 Subject: [PATCH 02/12] Module pages: always show the deep-search reminder during fast searches The DeepSearchHint only appeared on zero results, so a fast search with matches gave no indication that captions/contents were not searched. Module searches have no deep estimate, so show a generic reminder whenever a fast search is active: "More results may be available in deep search." with results, "Not finding what you need?" without. Co-Authored-By: Claude Fable 5 --- app/src/components/Files.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/app/src/components/Files.js b/app/src/components/Files.js index 5f29489d..9ef66cc4 100644 --- a/app/src/components/Files.js +++ b/app/src/components/Files.js @@ -898,18 +898,19 @@ export function SearchControlBar( } -// Offers deep search when a fast search found nothing. Used by the module pages (Videos/Archives/Docs), -// which have no deep estimate; FilesSearchView has a count-aware variant. +// Reminds the user that the fast search only matches titles/authors/descriptions. Used by the module +// pages (Videos/Archives/Docs), which have no deep estimate, so it shows whenever a fast search is +// active; FilesSearchView has a count-aware variant. export function DeepSearchHint({searchStr, files}) { const {deep} = useSearchDeep(); const {updateQuery} = useContext(QueryContext); - if (!searchStr || deep || !files || files.length > 0) { + if (!searchStr || deep || !Array.isArray(files)) { return null; } return - Not finding what you need? + {files.length === 0 ? 'Not finding what you need?' : 'More results may be available in deep search.'} + setQrOpen(false)} + onOpen={() => setQrOpen(true)} + open={qrOpen} + trigger={; - const timezoneOptions = Intl.supportedValuesOf('timeZone').map(tz => ( {key: tz, value: tz, text: tz} )); @@ -619,54 +593,6 @@ export function SettingsPage() { - - -
Hotspot Settings
- - setState({...state, hotspot_ssid: i.value})} - style={{marginBottom: '0.5em'}} - /> - setState({...state, hotspot_password: i.value})} - style={{marginBottom: '0.5em'}} - /> - handleInputChange(e, 'hotspot_device', i.value)} - style={{marginBottom: '0.5em'}} - /> - - setQrOpen(false)} - onOpen={handleQrOpen} - open={qrOpen} - trigger={qrButton} - > - - Scan this code to join the hotspot - - -
- -
- - - - - diff --git a/controller/api/admin.py b/controller/api/admin.py index 83ba5b53..3d12d563 100644 --- a/controller/api/admin.py +++ b/controller/api/admin.py @@ -8,6 +8,9 @@ from controller.api.schemas import ( BluetoothActionResponse, BluetoothStatusResponse, HotspotActionResponse, + HotspotDevicesResponse, + HotspotSettingsRequest, + HotspotSettingsResponse, HotspotStatusResponse, RestartServicesResponse, SambaShareAddRequest, @@ -30,13 +33,18 @@ from controller.lib.admin import ( enable_hotspot, enable_throttle, get_bluetooth_status_dict, + get_hotspot_device, + get_hotspot_password, + get_hotspot_ssid, get_hotspot_status_dict, get_throttle_status_dict, get_timezone_status_dict, + get_wifi_devices, reboot_system, restart_all_services, set_timezone, shutdown_system, + update_hotspot_settings, ) from controller.lib.config import is_docker_mode from controller.lib.samba import ( @@ -56,6 +64,33 @@ async def hotspot_status() -> HotspotStatusResponse: return HotspotStatusResponse(**get_hotspot_status_dict()) +@router.get("/api/hotspot/devices", response_model=HotspotDevicesResponse) +async def hotspot_devices() -> HotspotDevicesResponse: + """List WiFi devices which could host the hotspot.""" + return HotspotDevicesResponse(devices=get_wifi_devices()) + + +@router.get("/api/hotspot/settings", response_model=HotspotSettingsResponse) +async def hotspot_settings() -> HotspotSettingsResponse: + """Get WiFi hotspot settings.""" + return HotspotSettingsResponse( + device=get_hotspot_device(), + ssid=get_hotspot_ssid(), + password=get_hotspot_password(), + ) + + +@router.post("/api/hotspot/settings", response_model=HotspotSettingsResponse) +async def hotspot_settings_update(request: HotspotSettingsRequest) -> HotspotSettingsResponse: + """Update WiFi hotspot settings; they are saved in controller.yaml.""" + result = update_hotspot_settings(device=request.device, ssid=request.ssid, password=request.password) + if not result.get("success"): + error = result.get("error", "Failed") + status_code = 500 if "Docker" in error else 400 + raise HTTPException(status_code=status_code, detail=error) + return HotspotSettingsResponse(device=result["device"], ssid=result["ssid"], password=result["password"]) + + @router.post("/api/hotspot/enable", response_model=HotspotActionResponse) async def hotspot_enable() -> HotspotActionResponse: """Enable WiFi hotspot.""" diff --git a/controller/api/schemas.py b/controller/api/schemas.py index eaf0b312..dc681157 100644 --- a/controller/api/schemas.py +++ b/controller/api/schemas.py @@ -157,6 +157,28 @@ class HotspotStatusResponse(BaseModel): device: Optional[str] = Field(default=None, description="WiFi device name") +class HotspotDevicesResponse(BaseModel): + """Response model for /api/hotspot/devices endpoint.""" + + devices: list[str] = Field(description="Available WiFi device names") + + +class HotspotSettingsRequest(BaseModel): + """Request model for updating hotspot settings; only provided values are changed.""" + + device: Optional[str] = Field(default=None, description="WiFi device name") + ssid: Optional[str] = Field(default=None, description="Hotspot SSID") + password: Optional[str] = Field(default=None, description="Hotspot password (8 to 63 characters)") + + +class HotspotSettingsResponse(BaseModel): + """Response model for /api/hotspot/settings endpoint.""" + + device: str = Field(description="WiFi device name") + ssid: str = Field(description="Hotspot SSID") + password: str = Field(description="Hotspot password") + + class HotspotActionResponse(BaseModel): """Response model for hotspot enable/disable actions.""" diff --git a/controller/api/status.py b/controller/api/status.py index fb53a2d2..44137b91 100644 --- a/controller/api/status.py +++ b/controller/api/status.py @@ -12,7 +12,8 @@ wrol_mode) remain in the main WROLPi API at /api/status. from fastapi import APIRouter, Request from fastapi.responses import JSONResponse -from controller.lib.admin import get_bluetooth_status, get_hotspot_status, get_throttle_status, BluetoothStatus, HotspotStatus +from controller.lib.admin import get_bluetooth_status, get_hotspot_ssid, get_hotspot_status, get_throttle_status, \ + BluetoothStatus, HotspotStatus from controller.lib.config import is_docker_mode, is_rpi, is_rpi4, is_rpi5 from controller.lib.status import ( get_cpu_status, @@ -36,16 +37,12 @@ async def get_system_info() -> dict: Get static system info that doesn't need frequent updates. These are collected on each request as they rarely change. """ - from controller.lib.config import get_config - hotspot_status = get_hotspot_status() bluetooth_status = await get_bluetooth_status() throttle_status = get_throttle_status() # Get hotspot SSID from config if hotspot is connected - config = get_config() - hotspot_config = config.get("hotspot", {}) - hotspot_ssid = hotspot_config.get("ssid", "WROLPi") if hotspot_status == HotspotStatus.connected else None + hotspot_ssid = get_hotspot_ssid() if hotspot_status == HotspotStatus.connected else None return { "dockerized": is_docker_mode(), diff --git a/controller/conftest.py b/controller/conftest.py index 05b1f983..3685f316 100644 --- a/controller/conftest.py +++ b/controller/conftest.py @@ -49,6 +49,14 @@ async def _noop_status_worker(app, base_sleep: float = 5.0): raise +@pytest.fixture(autouse=True) +def _isolate_media_directory(tmp_path_factory, monkeypatch): + """Point the media directory at an empty temp dir so tests never read a real + /media/wrolpi (e.g. hotspot settings from wrolpi.yaml).""" + media_dir = tmp_path_factory.mktemp("media") + monkeypatch.setenv("MEDIA_DIRECTORY", str(media_dir)) + + @pytest.fixture(autouse=True) def _patch_status_worker(): """Replace the real status worker with a no-op for every test. diff --git a/controller/lib/admin.py b/controller/lib/admin.py index ff77a9da..fc8b366a 100644 --- a/controller/lib/admin.py +++ b/controller/lib/admin.py @@ -13,6 +13,7 @@ from pathlib import Path import yaml +from controller.lib import config as config_lib from controller.lib.config import is_docker_mode, get_config_value, get_media_directory logger = logging.getLogger(__name__) @@ -45,13 +46,141 @@ class GovernorStatus(enum.Enum): unknown = enum.auto() +def get_hotspot_device() -> str: + return get_config_value('hotspot.device', 'wlan0') + + +def get_hotspot_ssid() -> str: + return get_config_value('hotspot.ssid', 'WROLPi') + + +def get_hotspot_password() -> str: + return get_config_value('hotspot.password', 'wrolpi hotspot') + + +def update_hotspot_settings(device: str = None, ssid: str = None, password: str = None) -> dict: + """ + Update hotspot settings in controller.yaml. Only provided values are changed. + + Returns: + dict with success status and the current settings + """ + if is_docker_mode(): + return {"success": False, "error": "Not available in Docker mode"} + + if device is not None and not device.strip(): + return {"success": False, "error": "Hotspot device must not be empty"} + if ssid is not None and not ssid.strip(): + return {"success": False, "error": "Hotspot SSID must not be empty"} + # nmcli requires a WPA password of 8 to 63 characters. + if password is not None and not 8 <= len(password) <= 63: + return {"success": False, "error": "Hotspot password must be 8 to 63 characters"} + + if device: + config_lib.update_config('hotspot.device', device) + if ssid: + config_lib.update_config('hotspot.ssid', ssid) + if password: + config_lib.update_config('hotspot.password', password) + + try: + config_lib.save_config() + except RuntimeError as e: + return {"success": False, "error": str(e)} + + logger.info("Hotspot settings updated (device=%s, ssid=%s)", get_hotspot_device(), get_hotspot_ssid()) + return { + "success": True, + "device": get_hotspot_device(), + "ssid": get_hotspot_ssid(), + "password": get_hotspot_password(), + } + + +def migrate_hotspot_settings_from_wrolpi_config(): + """ + One-time copy of hotspot settings from wrolpi.yaml into controller.yaml. + + Hotspot settings used to be saved in wrolpi.yaml by the Settings page; the + Controller now owns them in controller.yaml. Copy any values the user set + in wrolpi.yaml unless controller.yaml already sets them explicitly. + Called on Controller startup after the primary drive is mounted. + """ + if not config_lib.CONFIG_PATH_ON_DRIVE.exists(): + return + + try: + with open(config_lib.CONFIG_PATH_ON_DRIVE) as f: + controller_config = yaml.safe_load(f) or {} + except (IOError, yaml.YAMLError) as e: + logger.warning("Failed to read controller.yaml for hotspot migration: %s", e) + return + + wrolpi_config_path = get_media_directory() / 'config' / 'wrolpi.yaml' + if not wrolpi_config_path.exists(): + return + + try: + with open(wrolpi_config_path) as f: + wrolpi_config = yaml.safe_load(f) or {} + except (IOError, yaml.YAMLError) as e: + logger.warning("Failed to read wrolpi.yaml for hotspot migration: %s", e) + return + + explicit = controller_config.get('hotspot') or {} + key_map = {'device': 'hotspot_device', 'ssid': 'hotspot_ssid', 'password': 'hotspot_password'} + changed = False + for controller_key, wrolpi_key in key_map.items(): + value = wrolpi_config.get(wrolpi_key) + if value and controller_key not in explicit and value != get_config_value(f'hotspot.{controller_key}'): + config_lib.update_config(f'hotspot.{controller_key}', value) + changed = True + + if changed: + try: + config_lib.save_config() + logger.info("Migrated hotspot settings from wrolpi.yaml to controller.yaml") + except RuntimeError as e: + logger.warning("Could not save migrated hotspot settings: %s", e) + + +def get_wifi_devices() -> list[str]: + """ + List WiFi device names (e.g. wlan0, wlp2s0) using nmcli. + + Returns an empty list in Docker mode or when nmcli is unavailable. + """ + if is_docker_mode(): + return [] + + try: + result = subprocess.run( + ["nmcli", "-t", "-f", "DEVICE,TYPE", "device"], + capture_output=True, + text=True, + timeout=5, + ) + if result.returncode != 0: + return [] + + devices = [] + for line in result.stdout.strip().splitlines(): + parts = line.split(":") + # Exact "wifi" match excludes wifi-p2p devices. + if len(parts) >= 2 and parts[1] == "wifi": + devices.append(parts[0]) + return devices + except (FileNotFoundError, subprocess.TimeoutExpired, subprocess.SubprocessError): + return [] + + def get_current_ssid(interface: str = None) -> str | None: """ Returns the name of the SSID that is currently connected. Returns None if Hotspot is active, or no Wi-Fi network is being used. """ if interface is None: - interface = get_config_value('hotspot.device', 'wlan0') + interface = get_hotspot_device() try: result = subprocess.run( ['iwgetid', interface, '--raw'], @@ -81,7 +210,7 @@ def get_hotspot_status() -> HotspotStatus: if is_docker_mode(): return HotspotStatus.unknown - device = get_config_value('hotspot.device', 'wlan0') + device = get_hotspot_device() # Check if device is connected to a Wi-Fi network (not hotspot) if get_current_ssid(device): @@ -128,7 +257,7 @@ def get_hotspot_status_dict() -> dict: ssid: Optional[str] - Hotspot SSID when enabled device: Optional[str] - WiFi device name """ - device = get_config_value('hotspot.device', 'wlan0') + device = get_hotspot_device() status = get_hotspot_status() # Map enum status to enabled/available flags @@ -147,7 +276,7 @@ def get_hotspot_status_dict() -> dict: "enabled": enabled, "available": available, "reason": reason, - "ssid": get_config_value('hotspot.ssid', 'WROLPi') if enabled else None, + "ssid": get_hotspot_ssid() if enabled else None, "device": device, } @@ -162,9 +291,9 @@ def enable_hotspot() -> dict: if is_docker_mode(): return {"success": False, "error": "Not available in Docker mode"} - device = get_config_value('hotspot.device', 'wlan0') - ssid = get_config_value('hotspot.ssid', 'WROLPi') - password = get_config_value('hotspot.password', 'wrolpi hotspot') + device = get_hotspot_device() + ssid = get_hotspot_ssid() + password = get_hotspot_password() try: # First ensure radio is on diff --git a/controller/main.py b/controller/main.py index d6732819..7f4a8a9b 100644 --- a/controller/main.py +++ b/controller/main.py @@ -87,9 +87,12 @@ async def lifespan(app: FastAPI): logger.info("Using default configuration (no controller.yaml found)") # Apply timezone from wrolpi.yaml to the system. - from controller.lib.admin import apply_timezone_from_config + from controller.lib.admin import apply_timezone_from_config, migrate_hotspot_settings_from_wrolpi_config apply_timezone_from_config() + # Copy legacy hotspot settings from wrolpi.yaml into controller.yaml. + migrate_hotspot_settings_from_wrolpi_config() + # Apply Samba config if enabled. from controller.lib.samba import apply_samba_from_config apply_samba_from_config() @@ -108,8 +111,9 @@ async def lifespan(app: FastAPI): except RuntimeError as e: logger.warning("Failed to create controller.yaml: %s", e) - from controller.lib.admin import apply_timezone_from_config + from controller.lib.admin import apply_timezone_from_config, migrate_hotspot_settings_from_wrolpi_config apply_timezone_from_config() + migrate_hotspot_settings_from_wrolpi_config() from controller.lib.samba import apply_samba_from_config apply_samba_from_config() diff --git a/controller/templates/index.html b/controller/templates/index.html index e27cb92b..c526d4a7 100644 --- a/controller/templates/index.html +++ b/controller/templates/index.html @@ -959,6 +959,23 @@ +
+ Hotspot Settings +
+ + + + +
+
{% endif %}
@@ -1372,6 +1389,48 @@ await updateHotspotButton(); } + async function loadHotspotSettings() { + const ssidInput = document.getElementById('hotspot-ssid'); + if (!ssidInput) return; + try { + const [settings, devices] = await Promise.all([ + apiCall('api/hotspot/settings'), + apiCall('api/hotspot/devices'), + ]); + ssidInput.value = settings.ssid; + document.getElementById('hotspot-password').value = settings.password; + const select = document.getElementById('hotspot-device'); + select.innerHTML = ''; + // Keep the saved device selectable even if it is not currently present. + const names = [...new Set([...(devices.devices || []), settings.device])].filter(Boolean); + for (const name of names) { + const option = document.createElement('option'); + option.value = name; + option.textContent = name; + option.selected = name === settings.device; + select.appendChild(option); + } + } catch (e) { + console.error('Failed to load hotspot settings', e); + } + } + + async function saveHotspotSettings() { + const btn = event.target; + btn.disabled = true; + try { + await apiCall('api/hotspot/settings', 'POST', { + ssid: document.getElementById('hotspot-ssid').value, + password: document.getElementById('hotspot-password').value, + device: document.getElementById('hotspot-device').value, + }); + await updateHotspotButton(); + } catch (e) { + alert(`Failed to save hotspot settings: ${e.message}`); + } + btn.disabled = false; + } + // --- Bluetooth --- let bluetoothEnabled = null; @@ -2759,6 +2818,7 @@ // Fetch hardware button status immediately updateHotspotButton(); updateBluetoothButton(); + loadHotspotSettings(); // Initial update after a short delay (to not overlap with initial load) setTimeout(() => { diff --git a/controller/test/test_admin.py b/controller/test/test_admin.py index 374e3b7b..9b4c2202 100644 --- a/controller/test/test_admin.py +++ b/controller/test/test_admin.py @@ -5,7 +5,9 @@ Unit tests for controller.lib.admin module. from unittest import mock import pytest +import yaml +from controller.lib.config import reload_config_from_drive from controller.lib.admin import ( apply_timezone_from_config, disable_bluetooth, @@ -19,10 +21,16 @@ from controller.lib.admin import ( get_hotspot_status_dict, get_throttle_status, get_timezone_status_dict, + get_hotspot_device, + get_hotspot_password, + get_hotspot_ssid, + get_wifi_devices, + migrate_hotspot_settings_from_wrolpi_config, reboot_system, restart_all_services, set_timezone, shutdown_system, + update_hotspot_settings, BluetoothStatus, HotspotStatus, GovernorStatus, @@ -295,6 +303,155 @@ class TestGetHotspotStatusDict: assert result["enabled"] is True +class TestHotspotSettingsMigration: + """Legacy hotspot settings in wrolpi.yaml are copied into controller.yaml on startup.""" + + @staticmethod + def _write_wrolpi_yaml(test_config_directory, **values): + lines = ''.join(f'{key}: {value}\n' for key, value in values.items()) + (test_config_directory / 'wrolpi.yaml').write_text(lines) + + def test_migrates_wrolpi_yaml_values(self, test_directory, test_config_directory, mock_config_path, + reset_runtime_config): + """wrolpi.yaml hotspot settings should be copied to controller.yaml.""" + mock_config_path.write_text("{}\n") + self._write_wrolpi_yaml(test_config_directory, hotspot_device='wlp2s0', hotspot_ssid='RafaelPi', + hotspot_password='password123') + + with mock.patch("controller.lib.admin.get_media_directory", return_value=test_directory): + migrate_hotspot_settings_from_wrolpi_config() + + assert get_hotspot_device() == 'wlp2s0' + assert get_hotspot_ssid() == 'RafaelPi' + assert get_hotspot_password() == 'password123' + # The migrated values are persisted in controller.yaml. + saved = yaml.safe_load(mock_config_path.read_text()) + assert saved['hotspot'] == {'device': 'wlp2s0', 'ssid': 'RafaelPi', 'password': 'password123'} + + def test_does_not_overwrite_explicit_controller_values(self, test_directory, test_config_directory, + mock_config_path, reset_runtime_config): + """Explicit controller.yaml values win over legacy wrolpi.yaml values.""" + mock_config_path.write_text("hotspot:\n device: wlan1\n") + reload_config_from_drive() + self._write_wrolpi_yaml(test_config_directory, hotspot_device='wlp2s0', hotspot_ssid='RafaelPi') + + with mock.patch("controller.lib.admin.get_media_directory", return_value=test_directory): + migrate_hotspot_settings_from_wrolpi_config() + + # The explicit device is kept, but the SSID is still migrated. + assert get_hotspot_device() == 'wlan1' + assert get_hotspot_ssid() == 'RafaelPi' + + def test_noop_without_wrolpi_yaml(self, test_directory, test_config_directory, mock_config_path, + reset_runtime_config): + """Nothing happens when wrolpi.yaml does not exist.""" + mock_config_path.write_text("{}\n") + + with mock.patch("controller.lib.admin.get_media_directory", return_value=test_directory): + migrate_hotspot_settings_from_wrolpi_config() + + assert get_hotspot_device() == 'wlan0' + assert yaml.safe_load(mock_config_path.read_text()) == {} + + def test_noop_when_drive_not_mounted(self, test_directory, test_config_directory, mock_config_path, + reset_runtime_config): + """Nothing happens when controller.yaml does not exist (drive not mounted).""" + self._write_wrolpi_yaml(test_config_directory, hotspot_device='wlp2s0') + + with mock.patch("controller.lib.admin.get_media_directory", return_value=test_directory): + migrate_hotspot_settings_from_wrolpi_config() + + assert get_hotspot_device() == 'wlan0' + + +class TestUpdateHotspotSettings: + """Tests for update_hotspot_settings function.""" + + def test_updates_and_saves(self, mock_config_path, reset_runtime_config): + """New settings should be applied to the runtime config and saved to controller.yaml.""" + with mock.patch("controller.lib.admin.is_docker_mode", return_value=False): + result = update_hotspot_settings(device='wlp2s0', ssid='RafaelPi', password='password123') + + assert result["success"] is True + assert get_hotspot_device() == 'wlp2s0' + saved = yaml.safe_load(mock_config_path.read_text()) + assert saved['hotspot'] == {'device': 'wlp2s0', 'ssid': 'RafaelPi', 'password': 'password123'} + + def test_partial_update(self, mock_config_path, reset_runtime_config): + """Only provided values are changed.""" + with mock.patch("controller.lib.admin.is_docker_mode", return_value=False): + result = update_hotspot_settings(device='wlp2s0') + + assert result["success"] is True + assert result["device"] == 'wlp2s0' + assert result["ssid"] == 'WROLPi' + assert result["password"] == 'wrolpi hotspot' + + def test_rejects_short_password(self, mock_config_path, reset_runtime_config): + """nmcli requires a WPA password of 8 to 63 characters.""" + with mock.patch("controller.lib.admin.is_docker_mode", return_value=False): + result = update_hotspot_settings(password='short') + + assert result["success"] is False + assert "8" in result["error"] + assert get_hotspot_password() == 'wrolpi hotspot' + + def test_rejects_in_docker_mode(self, reset_runtime_config): + """Hotspot settings cannot be changed in Docker mode.""" + with mock.patch("controller.lib.admin.is_docker_mode", return_value=True): + result = update_hotspot_settings(device='wlp2s0') + + assert result["success"] is False + assert "Docker" in result["error"] + + def test_error_when_drive_not_mounted(self, tmp_path, reset_runtime_config): + """Saving fails cleanly when the primary drive is not mounted.""" + missing_path = tmp_path / 'missing' / 'controller.yaml' + with mock.patch("controller.lib.admin.is_docker_mode", return_value=False): + with mock.patch("controller.lib.config.CONFIG_PATH_ON_DRIVE", missing_path): + result = update_hotspot_settings(device='wlp2s0') + + assert result["success"] is False + assert "not mounted" in result["error"] + + +class TestGetWifiDevices: + """Tests for get_wifi_devices function.""" + + # Real terse nmcli output from an x86 PC: `nmcli -t -f DEVICE,TYPE device` + NMCLI_TERSE_DEVICES = ( + "eno1:ethernet\n" + "wlp2s0:wifi\n" + "p2p-dev-wlp2s0:wifi-p2p\n" + "lo:loopback\n" + ) + + def test_returns_wifi_devices_only(self): + """Should return only wifi-type devices, excluding wifi-p2p and ethernet.""" + mock_result = mock.Mock(returncode=0, stdout=self.NMCLI_TERSE_DEVICES) + with mock.patch("controller.lib.admin.is_docker_mode", return_value=False): + with mock.patch("subprocess.run", return_value=mock_result): + assert get_wifi_devices() == ["wlp2s0"] + + def test_returns_empty_in_docker_mode(self): + """Should return an empty list in Docker mode.""" + with mock.patch("controller.lib.admin.is_docker_mode", return_value=True): + assert get_wifi_devices() == [] + + def test_returns_empty_when_nmcli_missing(self): + """Should return an empty list when nmcli is not installed.""" + with mock.patch("controller.lib.admin.is_docker_mode", return_value=False): + with mock.patch("subprocess.run", side_effect=FileNotFoundError()): + assert get_wifi_devices() == [] + + def test_returns_empty_when_nmcli_fails(self): + """Should return an empty list when nmcli returns an error.""" + mock_result = mock.Mock(returncode=8, stdout="") + with mock.patch("controller.lib.admin.is_docker_mode", return_value=False): + with mock.patch("subprocess.run", return_value=mock_result): + assert get_wifi_devices() == [] + + class TestDockerModeErrors: """Admin functions should return an error result when running in Docker mode.""" diff --git a/controller/test/test_admin_api.py b/controller/test/test_admin_api.py index d5a3cafc..b0026d82 100644 --- a/controller/test/test_admin_api.py +++ b/controller/test/test_admin_api.py @@ -34,6 +34,54 @@ class TestStatusEndpoints: assert data["available"] is False +class TestHotspotDevicesEndpoint: + """GET /api/hotspot/devices lists WiFi interfaces for the Settings dropdown.""" + + def test_devices_endpoint_shape(self, test_client): + """Should return the WiFi devices found by nmcli.""" + from unittest import mock + with mock.patch("controller.api.admin.get_wifi_devices", return_value=["wlan0", "wlp2s0"]): + response = test_client.get("/api/hotspot/devices") + assert response.status_code == 200 + assert response.json() == {"devices": ["wlan0", "wlp2s0"]} + + def test_devices_empty_in_docker(self, test_client_docker_mode): + """Should return an empty list in Docker mode.""" + response = test_client_docker_mode.get("/api/hotspot/devices") + assert response.status_code == 200 + assert response.json() == {"devices": []} + + +class TestHotspotSettingsEndpoint: + """GET/POST /api/hotspot/settings manage hotspot settings in controller.yaml.""" + + def test_get_settings(self, test_client): + """Should return the current hotspot settings.""" + response = test_client.get("/api/hotspot/settings") + assert response.status_code == 200 + assert response.json() == {"device": "wlan0", "ssid": "WROLPi", "password": "wrolpi hotspot"} + + def test_post_settings(self, test_client, mock_config_path): + """Should update settings and persist them to controller.yaml.""" + response = test_client.post( + "/api/hotspot/settings", + json={"device": "wlp2s0", "ssid": "RafaelPi", "password": "password123"}, + ) + assert response.status_code == 200 + assert response.json() == {"device": "wlp2s0", "ssid": "RafaelPi", "password": "password123"} + assert "wlp2s0" in mock_config_path.read_text() + + def test_post_rejects_short_password(self, test_client, mock_config_path): + """Should reject a password shorter than 8 characters.""" + response = test_client.post("/api/hotspot/settings", json={"password": "short"}) + assert response.status_code == 400 + + def test_post_rejected_in_docker(self, test_client_docker_mode): + """Should reject settings changes in Docker mode.""" + response = test_client_docker_mode.post("/api/hotspot/settings", json={"device": "wlp2s0"}) + assert response.status_code == 500 + + class TestDockerModeRejectsAdminActions: """Admin action endpoints should reject requests when running in Docker.""" From 2887309aae8117184f4b9f0dbc5e50a5e0c6c0f2 Mon Sep 17 00:00:00 2001 From: Roland Date: Sat, 4 Jul 2026 08:56:12 -0600 Subject: [PATCH 10/12] Hotspot: address review comments. * POST /api/hotspot/settings checks is_docker_mode() directly instead of string-matching the error message (still 500, matching the sibling hotspot/bluetooth/throttle endpoints). * Escape \ ; , " in the WiFi QR code per the WPA QR format. * Skip the controller.yaml write when a settings update changes nothing. Co-Authored-By: Claude Fable 5 --- app/src/components/admin/ControllerPage.js | 4 +++- controller/api/admin.py | 7 ++++--- controller/lib/admin.py | 13 +++++++++---- controller/test/test_admin.py | 11 +++++++++++ 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/app/src/components/admin/ControllerPage.js b/app/src/components/admin/ControllerPage.js index 1df31229..9cf0cc27 100644 --- a/app/src/components/admin/ControllerPage.js +++ b/app/src/components/admin/ControllerPage.js @@ -1395,7 +1395,9 @@ function HotspotSettingsForm() { .map(device => ({key: device, value: device, text: device})); // Special string which allows a mobile device to connect to a specific Wi-Fi. - const qrCodeValue = `WIFI:S:${form.ssid};T:WPA;P:${form.password};;`; + // The WPA QR format requires backslash-escaping of \ ; , " in the SSID and password. + const escapeWifi = (s) => s.replace(/([\\;,"])/g, '\\$1'); + const qrCodeValue = `WIFI:S:${escapeWifi(form.ssid)};T:WPA;P:${escapeWifi(form.password)};;`; return
diff --git a/controller/api/admin.py b/controller/api/admin.py index 3d12d563..4e28bae9 100644 --- a/controller/api/admin.py +++ b/controller/api/admin.py @@ -83,11 +83,12 @@ async def hotspot_settings() -> HotspotSettingsResponse: @router.post("/api/hotspot/settings", response_model=HotspotSettingsResponse) async def hotspot_settings_update(request: HotspotSettingsRequest) -> HotspotSettingsResponse: """Update WiFi hotspot settings; they are saved in controller.yaml.""" + # 500 matches the other subsystem endpoints (hotspot/bluetooth/throttle) in Docker mode. + if is_docker_mode(): + raise HTTPException(status_code=500, detail="Not available in Docker mode") result = update_hotspot_settings(device=request.device, ssid=request.ssid, password=request.password) if not result.get("success"): - error = result.get("error", "Failed") - status_code = 500 if "Docker" in error else 400 - raise HTTPException(status_code=status_code, detail=error) + raise HTTPException(status_code=400, detail=result.get("error", "Failed")) return HotspotSettingsResponse(device=result["device"], ssid=result["ssid"], password=result["password"]) diff --git a/controller/lib/admin.py b/controller/lib/admin.py index fc8b366a..f4da130c 100644 --- a/controller/lib/admin.py +++ b/controller/lib/admin.py @@ -76,17 +76,22 @@ def update_hotspot_settings(device: str = None, ssid: str = None, password: str if password is not None and not 8 <= len(password) <= 63: return {"success": False, "error": "Hotspot password must be 8 to 63 characters"} + changed = False if device: config_lib.update_config('hotspot.device', device) + changed = True if ssid: config_lib.update_config('hotspot.ssid', ssid) + changed = True if password: config_lib.update_config('hotspot.password', password) + changed = True - try: - config_lib.save_config() - except RuntimeError as e: - return {"success": False, "error": str(e)} + if changed: + try: + config_lib.save_config() + except RuntimeError as e: + return {"success": False, "error": str(e)} logger.info("Hotspot settings updated (device=%s, ssid=%s)", get_hotspot_device(), get_hotspot_ssid()) return { diff --git a/controller/test/test_admin.py b/controller/test/test_admin.py index 9b4c2202..a6b69dec 100644 --- a/controller/test/test_admin.py +++ b/controller/test/test_admin.py @@ -404,6 +404,17 @@ class TestUpdateHotspotSettings: assert result["success"] is False assert "Docker" in result["error"] + def test_noop_does_not_save(self, tmp_path, reset_runtime_config): + """An empty update succeeds without writing controller.yaml.""" + missing_path = tmp_path / 'missing' / 'controller.yaml' + with mock.patch("controller.lib.admin.is_docker_mode", return_value=False): + # save_config() would raise RuntimeError here; success proves it was skipped. + with mock.patch("controller.lib.config.CONFIG_PATH_ON_DRIVE", missing_path): + result = update_hotspot_settings() + + assert result["success"] is True + assert not missing_path.exists() + def test_error_when_drive_not_mounted(self, tmp_path, reset_runtime_config): """Saving fails cleanly when the primary drive is not mounted.""" missing_path = tmp_path / 'missing' / 'controller.yaml' From 43dac5d63d0342beb452c91e9cae0d3786c56967 Mon Sep 17 00:00:00 2001 From: Roland Date: Sat, 4 Jul 2026 09:16:44 -0600 Subject: [PATCH 11/12] Controller page: group hotspot settings in a segment below the toggles. Co-Authored-By: Claude Fable 5 --- app/src/components/admin/ControllerPage.js | 105 ++++++++++++--------- 1 file changed, 59 insertions(+), 46 deletions(-) diff --git a/app/src/components/admin/ControllerPage.js b/app/src/components/admin/ControllerPage.js index 9cf0cc27..83117264 100644 --- a/app/src/components/admin/ControllerPage.js +++ b/app/src/components/admin/ControllerPage.js @@ -1386,7 +1386,13 @@ function HotspotSettingsForm() { }; if (loading) { - return ; + return +
+ + Hotspot Settings +
+ + ; } // Keep the saved device selectable even if it is not currently present @@ -1399,50 +1405,56 @@ function HotspotSettingsForm() { const escapeWifi = (s) => s.replace(/([\\;,"])/g, '\\$1'); const qrCodeValue = `WIFI:S:${escapeWifi(form.ssid)};T:WPA;P:${escapeWifi(form.password)};;`; - return - - setForm({...form, ssid: value})} - /> - setForm({...form, password: value})} - /> - setForm({...form, device: value})} - /> - - - setQrOpen(false)} - onOpen={() => setQrOpen(true)} - open={qrOpen} - trigger={ + setQrOpen(false)} + onOpen={() => setQrOpen(true)} + open={qrOpen} + trigger={