-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Automatic Filter Title Fetching #8112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,14 @@ | ||
| import React from 'react'; | ||
| import { useForm, Controller, FormProvider } from 'react-hook-form'; | ||
| import React, { useEffect, useState, useRef } from 'react'; | ||
| import { useForm, Controller, FormProvider, useWatch } from 'react-hook-form'; | ||
| import { useTranslation } from 'react-i18next'; | ||
| import { useDispatch } from 'react-redux'; | ||
| import { validatePath, validateRequiredValue } from '../../helpers/validators'; | ||
|
|
||
| import { MODAL_OPEN_TIMEOUT, MODAL_TYPE } from '../../helpers/constants'; | ||
| import filtersCatalog from '../../helpers/filters/filters'; | ||
| import { FiltersList } from './FiltersList'; | ||
| import { Input } from '../ui/Controls/Input'; | ||
| import { fetchFilterTitle } from '../../actions/filtering'; | ||
|
|
||
| type FormValues = { | ||
| enabled: boolean; | ||
|
|
@@ -44,6 +46,8 @@ export const Form = ({ | |
| initialValues, | ||
| }: Props) => { | ||
| const { t } = useTranslation(); | ||
| const dispatch = useDispatch(); | ||
| const [isFetchingTitle, setIsFetchingTitle] = useState(false); | ||
|
|
||
| const methods = useForm({ | ||
| defaultValues: { | ||
|
|
@@ -52,7 +56,56 @@ export const Form = ({ | |
| }, | ||
| mode: 'onBlur', | ||
| }); | ||
| const { handleSubmit, control } = methods; | ||
| const { handleSubmit, control, setValue, getValues } = methods; | ||
|
|
||
| // Watch URL field for changes | ||
| const urlValue = useWatch({ control, name: 'url' }); | ||
| const nameValue = useWatch({ control, name: 'name' }); | ||
| const debounceTimerRef = useRef<NodeJS.Timeout | null>(null); | ||
|
|
||
| // Auto-fetch title from URL | ||
| useEffect(() => { | ||
| // Clear any existing timer | ||
| if (debounceTimerRef.current) { | ||
| clearTimeout(debounceTimerRef.current); | ||
| } | ||
|
|
||
| // Don't fetch if: | ||
| // - URL is empty | ||
| // - Name field already has a value (user has typed something) | ||
| // - We're in edit mode (initialValues provided) | ||
| // - URL doesn't pass basic validation | ||
| if (!urlValue || nameValue || initialValues?.name) { | ||
| return; | ||
| } | ||
|
|
||
| // Basic URL validation check | ||
| const isValidUrl = validatePath(urlValue) === undefined; | ||
| if (!isValidUrl) { | ||
| return; | ||
| } | ||
|
|
||
| // Debounce: wait 800ms after user stops typing | ||
| debounceTimerRef.current = setTimeout(async () => { | ||
| setIsFetchingTitle(true); | ||
| try { | ||
| const title = await dispatch(fetchFilterTitle(urlValue) as any); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
| // Only set the name if it's still empty (user hasn't typed anything) | ||
| if (!getValues('name') && title) { | ||
| setValue('name', title, { shouldValidate: false }); | ||
| } | ||
| } finally { | ||
| setIsFetchingTitle(false); | ||
| } | ||
| }, 800); | ||
|
|
||
| // Cleanup on unmount | ||
| return () => { | ||
| if (debounceTimerRef.current) { | ||
| clearTimeout(debounceTimerRef.current); | ||
| } | ||
| }; | ||
| }, [urlValue, nameValue, dispatch, setValue, getValues, initialValues]); | ||
|
|
||
| const openModal = (modalType: keyof typeof MODAL_TYPE, timeout = MODAL_OPEN_TIMEOUT) => { | ||
| toggleFilteringModal(undefined); | ||
|
|
@@ -98,7 +151,11 @@ export const Form = ({ | |
| {...field} | ||
| type="text" | ||
| data-testid="filters_name" | ||
| placeholder={t('enter_name_hint')} | ||
| placeholder={ | ||
| isFetchingTitle | ||
| ? t('fetching_name_from_url') | ||
| : t('name_auto_fetch_hint') | ||
| } | ||
| error={fieldState.error?.message} | ||
| trimOnBlur | ||
| /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -607,6 +607,34 @@ func (d *DNSFilter) readerFromURL(fltURL string) (r io.ReadCloser, err error) { | |
| return resp.Body, nil | ||
| } | ||
|
|
||
| // fetchFilterTitle downloads the beginning of a filter file from url and | ||
| // extracts the title. It returns an empty string if no title is found or if | ||
| // the URL is a file path. | ||
| func (d *DNSFilter) fetchFilterTitle(url string) (title string, err error) { | ||
| r, err := d.reader(url) | ||
| if err != nil { | ||
| // Don't wrap the error since it's informative enough as is. | ||
| return "", err | ||
| } | ||
| defer func() { err = errors.WithDeferred(err, r.Close()) }() | ||
|
|
||
| // Use a limited reader to avoid downloading large files. 4KB should be | ||
| // enough to find the title in the header comments. | ||
| const maxTitleBytes = 4096 | ||
| lr := io.LimitReader(r, maxTitleBytes) | ||
|
|
||
| bufPtr := d.bufPool.Get() | ||
| defer d.bufPool.Put(bufPtr) | ||
|
|
||
| p := rulelist.NewParser() | ||
| res, err := p.Parse(io.Discard, lr, *bufPtr) | ||
| if err != nil { | ||
| return "", fmt.Errorf("parsing filter: %w", err) | ||
|
Comment on lines
+629
to
+632
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The function downloads and parses the entire filter content (up to 4KB) even though we only need the title. Consider adding a method to rulelist.Parser that can extract just the title without parsing all rules, which would be more efficient. |
||
| } | ||
|
|
||
| return res.Title, nil | ||
| } | ||
|
|
||
| // loads filter contents from the file in dataDir | ||
| func (d *DNSFilter) load(ctx context.Context, flt *FilterYAML) (err error) { | ||
| fileName := flt.Path(d.conf.DataDir) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dependency array includes
initialValues, but the condition only checksinitialValues?.name. Consider either checking the entireinitialValuesobject in the condition or only includinginitialValues?.namein the dependency array for consistency.