From 9294f7840a0299e81b19d5b6efe63f0ce3968109 Mon Sep 17 00:00:00 2001 From: Shekar Siri Date: Mon, 2 Jun 2025 13:26:40 +0200 Subject: [PATCH] Refactor FilterStore with type safety and caching improvements - Add TypeScript interfaces and proper type annotations - Implement cache TTL and LRU eviction for better memory management - Improve error handling with try-catch blocks and graceful fallbacks - Extract helper methods and fix parameter naming (sietId -> siteId) - Add utility methods to FilterItem class for validation and cloning --- frontend/app/mstore/filterStore.ts | 246 ++++++++++++++++-------- frontend/app/mstore/types/filterItem.ts | 141 +++++++++++--- 2 files changed, 279 insertions(+), 108 deletions(-) diff --git a/frontend/app/mstore/filterStore.ts b/frontend/app/mstore/filterStore.ts index 9d36db727..2eae6fd6f 100644 --- a/frontend/app/mstore/filterStore.ts +++ b/frontend/app/mstore/filterStore.ts @@ -26,67 +26,96 @@ interface TopValuesParams { isEvent?: boolean; } +interface FilterOption { + label: string; + value: string; +} + +interface CacheEntry { + data: Filter[]; + timestamp: number; +} + +// Constants +const CACHE_TTL = 5 * 60 * 1000; // 5 minutes +const MAX_CACHE_SIZE = 100; + export default class FilterStore { topValues: TopValues = {}; filters: ProjectFilters = {}; commonFilters: Filter[] = []; isLoadingFilters: boolean = true; - filterCache: Record = {}; + private filterCache: Record = {}; private pendingFetches: Record> = {}; constructor() { makeAutoObservable(this); - this.initCommonFilters(); } - getEventOptions = (sietId: string) => { - return this.getFilters(sietId) - .filter((i: Filter) => i.isEvent) - .map((i: Filter) => { - return { - label: i.displayName || i.name, - value: i.name, - }; - }); + // Fixed typo: sietId -> siteId + getEventOptions = (siteId: string): FilterOption[] => { + return this.getFilters(siteId) + .filter((filter: Filter) => filter.isEvent) + .map((filter: Filter) => ({ + label: filter.displayName || filter.name, + value: filter.name, + })); }; - setTopValues = (key: string, values: Record | TopValue[]) => { + setTopValues = ( + key: string, + values: Record | TopValue[], + ): void => { const vals = Array.isArray(values) ? values : values.data; - this.topValues[key] = vals?.filter( - (value: any) => value !== null && value.value !== '', - ); + this.topValues[key] = + vals?.filter((value: any) => value !== null && value?.value !== '') || []; }; - resetValues = () => { + resetValues = (): void => { this.topValues = {}; }; - fetchTopValues = async (params: TopValuesParams) => { - const valKey = `${params.siteId}_${params.id}${params.source || ''}`; + fetchTopValues = async (params: TopValuesParams): Promise => { + const valKey = this.createTopValuesKey(params); - if (this.topValues[valKey] && this.topValues[valKey].length) { - return Promise.resolve(this.topValues[valKey]); + // Return cached values if available + if (this.topValues[valKey]?.length) { + return this.topValues[valKey]; } - const filter = this.filters[params.siteId + '']?.find( - (i) => i.id === params.id, - ); + + const filter = this.findFilterById(params.siteId || '', params.id || ''); if (!filter) { - console.error('Filter not found in store:', valKey); - return Promise.resolve([]); + console.warn(`Filter not found for key: ${valKey}`); + return []; } - return searchService - .fetchTopValues({ + try { + const response = await searchService.fetchTopValues({ [params.isEvent ? 'eventName' : 'propertyName']: filter.name, - }) - .then((response: []) => { + }); + + runInAction(() => { this.setTopValues(valKey, response); }); + + return this.topValues[valKey] || []; + } catch (error) { + console.error('Failed to fetch top values:', error); + return []; + } }; - setFilters = (projectId: string, filters: Filter[]) => { + private createTopValuesKey = (params: TopValuesParams): string => { + return `${params.siteId}_${params.id}${params.source || ''}`; + }; + + private findFilterById = (siteId: string, id: string): Filter | undefined => { + return this.filters[siteId]?.find((filter) => filter.id === id); + }; + + setFilters = (projectId: string, filters: Filter[]): void => { this.filters[projectId] = filters; }; @@ -95,12 +124,18 @@ export default class FilterStore { return this.addOperatorsToFilters(filters); }; - setIsLoadingFilters = (loading: boolean) => { + setIsLoadingFilters = (loading: boolean): void => { this.isLoadingFilters = loading; }; - resetFilters = () => { + resetFilters = (): void => { this.filters = {}; + this.clearCache(); + }; + + private clearCache = (): void => { + this.filterCache = {}; + this.pendingFetches = {}; }; processFilters = (filters: Filter[], category?: string): Filter[] => { @@ -110,12 +145,7 @@ export default class FilterStore { filter.possibleTypes?.map((type) => type.toLowerCase()) || [], dataType: filter.dataType || 'string', category: category || 'custom', - subCategory: - category === 'events' - ? filter.autoCaptured - ? 'autocapture' - : 'user' - : category, + subCategory: this.determineSubCategory(category, filter), displayName: filter.displayName || filter.name, icon: FilterKey.LOCATION, // TODO - use actual icons isEvent: category === 'events', @@ -125,67 +155,84 @@ export default class FilterStore { })); }; - addOperatorsToFilters = (filters: Filter[]): Filter[] => { - return filters.map((filter) => ({ - ...filter, - })); + private determineSubCategory = ( + category: string | undefined, + filter: Filter, + ): string | undefined => { + if (category === 'events') { + return filter.autoCaptured ? 'autocapture' : 'user'; + } + return category; + }; + + addOperatorsToFilters = (filters: Filter[]): Filter[] => { + // Currently just returns filters as-is, but keeping for future enhancements + return filters.map((filter) => ({ ...filter })); }; - // Modified to not add operators in cache fetchFilters = async (projectId: string): Promise => { - // Return cached filters with operators if available - if (this.filters[projectId] && this.filters[projectId].length) { - return Promise.resolve(this.getFilters(projectId)); + // Return cached filters if available + if (this.filters[projectId]?.length) { + return this.getFilters(projectId); } this.setIsLoadingFilters(true); try { const response = await filterService.fetchFilters(projectId); + const processedFilters = this.processFilterResponse(response.data); - const processedFilters: Filter[] = []; - - Object.keys(response.data).forEach((category: string) => { - const { list, total } = response.data[category] || { - list: [], - total: 0, - }; - const filters = this.processFilters(list, category); - processedFilters.push(...filters); + runInAction(() => { + this.setFilters(projectId, processedFilters); }); - this.setFilters(projectId, processedFilters); - return this.getFilters(projectId); } catch (error) { console.error('Failed to fetch filters:', error); throw error; } finally { - this.setIsLoadingFilters(false); + runInAction(() => { + this.setIsLoadingFilters(false); + }); } }; - initCommonFilters = () => { + private processFilterResponse = (data: Record): Filter[] => { + const processedFilters: Filter[] = []; + + Object.entries(data).forEach(([category, categoryData]) => { + const { list = [], total = 0 } = categoryData || {}; + const filters = this.processFilters(list, category); + processedFilters.push(...filters); + }); + + return processedFilters; + }; + + initCommonFilters = (): void => { this.commonFilters = [...COMMON_FILTERS]; }; getAllFilters = (projectId: string): Filter[] => { const projectFilters = this.filters[projectId] || []; - // return this.addOperatorsToFilters([...this.commonFilters, ...projectFilters]); return this.addOperatorsToFilters([...projectFilters]); }; getCurrentProjectFilters = (): Filter[] => { - return this.getAllFilters(projectStore.activeSiteId + ''); + return this.getAllFilters(String(projectStore.activeSiteId)); }; getEventFilters = async (eventName: string): Promise => { const cacheKey = `${projectStore.activeSiteId}_${eventName}`; - if (this.filterCache[cacheKey]) { - return this.filterCache[cacheKey]; + + // Check cache with TTL + const cachedEntry = this.filterCache[cacheKey]; + if (cachedEntry && this.isCacheValid(cachedEntry)) { + return cachedEntry.data; } - if (await this.pendingFetches[cacheKey]) { + // Return pending fetch if in progress + if (this.pendingFetches[cacheKey]) { return this.pendingFetches[cacheKey]; } @@ -193,39 +240,78 @@ export default class FilterStore { this.pendingFetches[cacheKey] = this.fetchAndProcessPropertyFilters(eventName); const filters = await this.pendingFetches[cacheKey]; - console.log('filters', filters); runInAction(() => { - this.filterCache[cacheKey] = filters; + this.setCacheEntry(cacheKey, filters); }); - delete this.pendingFetches[cacheKey]; return filters; } catch (error) { - delete this.pendingFetches[cacheKey]; + console.error('Failed to fetch event filters:', error); throw error; + } finally { + delete this.pendingFetches[cacheKey]; } }; + private isCacheValid = (entry: CacheEntry): boolean => { + return Date.now() - entry.timestamp < CACHE_TTL; + }; + + private setCacheEntry = (key: string, data: Filter[]): void => { + // Implement simple LRU by removing oldest entries + if (Object.keys(this.filterCache).length >= MAX_CACHE_SIZE) { + const oldestKey = Object.keys(this.filterCache)[0]; + delete this.filterCache[oldestKey]; + } + + this.filterCache[key] = { + data, + timestamp: Date.now(), + }; + }; + private fetchAndProcessPropertyFilters = async ( eventName: string, isAutoCapture?: boolean, ): Promise => { - const resp = await filterService.fetchProperties(eventName, isAutoCapture); - const names = resp.data.map((i: any) => i['name']); + try { + const response = await filterService.fetchProperties( + eventName, + isAutoCapture, + ); + const propertyNames = response.data.map((item: any) => item.name); - const activeSiteId = projectStore.activeSiteId + ''; - return ( - this.filters[activeSiteId] - ?.filter((i: any) => names.includes(i.name)) - .map((f: any) => ({ - ...f, + const activeSiteId = String(projectStore.activeSiteId); + const siteFilters = this.filters[activeSiteId] || []; + + return siteFilters + .filter((filter: Filter) => propertyNames.includes(filter.name)) + .map((filter: Filter) => ({ + ...filter, eventName, - })) || [] - ); + })); + } catch (error) { + console.error('Failed to fetch property filters:', error); + return []; + } }; - setCommonFilters = (filters: Filter[]) => { - this.commonFilters = filters; + setCommonFilters = (filters: Filter[]): void => { + this.commonFilters = [...filters]; + }; + + // Cleanup method for memory management + cleanup = (): void => { + this.clearExpiredCacheEntries(); + }; + + private clearExpiredCacheEntries = (): void => { + const now = Date.now(); + Object.entries(this.filterCache).forEach(([key, entry]) => { + if (now - entry.timestamp > CACHE_TTL) { + delete this.filterCache[key]; + } + }); }; } diff --git a/frontend/app/mstore/types/filterItem.ts b/frontend/app/mstore/types/filterItem.ts index b806353a0..0950a9983 100644 --- a/frontend/app/mstore/types/filterItem.ts +++ b/frontend/app/mstore/types/filterItem.ts @@ -4,6 +4,32 @@ import { FilterProperty, Operator } from '@/mstore/types/filterConstants'; type JsonData = Record; +// Define a proper interface for initialization data +interface FilterItemData { + id?: string; + name?: string; + displayName?: string; + description?: string; + possibleTypes?: string[]; + autoCaptured?: boolean; + metadataName?: string; + category?: string; + subCategory?: string; + type?: string; + icon?: string; + properties?: FilterProperty[]; + operator?: string; + operators?: Operator[]; + isEvent?: boolean; + value?: string[]; + propertyOrder?: string; + filters?: FilterItemData[]; + autoOpen?: boolean; +} + +// Define valid keys that can be updated +type FilterItemKeys = keyof FilterItemData; + export default class FilterItem { id: string = ''; name: string = ''; @@ -12,9 +38,9 @@ export default class FilterItem { possibleTypes?: string[]; autoCaptured?: boolean; metadataName?: string; - category: string; // 'event' | 'filter' | 'action' | etc. + category: string = ''; subCategory?: string; - type?: string; // 'number' | 'string' | 'boolean' | etc. + type?: string; icon?: string; properties?: FilterProperty[]; operator?: string; @@ -25,67 +51,108 @@ export default class FilterItem { filters?: FilterItem[]; autoOpen?: boolean; - constructor(data: any = {}) { + constructor(data: FilterItemData = {}) { makeAutoObservable(this); + this.initializeFromData(data); + } + private initializeFromData(data: FilterItemData): void { + // Set default operator if not provided + const processedData = { + ...data, + operator: data.operator || 'is', + }; + + // Handle filters array transformation if (Array.isArray(data.filters)) { - data.filters = data.filters.map( - (i: Record) => new FilterItem(i), + processedData.filters = data.filters.map( + (filterData: FilterItemData) => new FilterItem(filterData), ); } - data.operator = data.operator || 'is'; - this.merge(data); + this.merge(processedData); } - updateKey(key: string, value: any) { - // @ts-ignore - this[key] = value; + updateKey(key: K, value: FilterItemData[K]): void { + if (key in this) { + (this as any)[key] = value; + } else { + console.warn(`Attempted to update invalid key: ${key}`); + } } - merge(data: any) { - Object.keys(data).forEach((key) => { - // @ts-ignore - this[key] = data[key]; + merge(data: FilterItemData): void { + Object.entries(data).forEach(([key, value]) => { + if (key in this && value !== undefined) { + (this as any)[key] = value; + } }); } - fromData(data: any) { + fromData(data: FilterItemData): FilterItem { + if (!data) { + console.warn('fromData called with null/undefined data'); + return this; + } + Object.assign(this, data); this.type = 'string'; - this.name = data.type; - this.category = data.category; + this.name = data.name || ''; + this.category = data.category || ''; this.subCategory = data.subCategory; this.operator = data.operator; - this.filters = data.filters.map((i: JsonData) => new FilterItem(i)); + + // Safely handle filters array + if (Array.isArray(data.filters)) { + this.filters = data.filters.map( + (filterData: FilterItemData) => new FilterItem(filterData), + ); + } else { + this.filters = []; + } return this; } - fromJson(data: JsonData) { + fromJson(data: JsonData): FilterItem { + if (!data) { + console.warn('fromJson called with null/undefined data'); + return this; + } + this.type = 'string'; - this.name = data.type; - this.category = data.category; + this.name = data.type || ''; + this.category = data.category || ''; this.subCategory = data.subCategory; this.operator = data.operator; - this.value = data.value || ['']; - this.filters = data.filters.map((i: JsonData) => new FilterItem(i)); + this.value = Array.isArray(data.value) ? data.value : ['']; + + // Safely handle filters array + if (Array.isArray(data.filters)) { + this.filters = data.filters.map( + (filterData: JsonData) => new FilterItem(filterData), + ); + } else { + this.filters = []; + } return this; } - toJson(): any { - const json: any = { + toJson(): JsonData { + const json: JsonData = { type: this.name, isEvent: Boolean(this.isEvent), - value: this.value?.map((i: any) => (i ? i.toString() : '')) || [], + value: + this.value?.map((item: any) => (item ? item.toString() : '')) || [], operator: this.operator, source: this.name, filters: Array.isArray(this.filters) - ? this.filters.map((i) => i.toJson()) + ? this.filters.map((filter) => filter.toJson()) : [], }; + // Handle metadata category const isMetadata = this.category === FilterCategory.METADATA; if (isMetadata) { json.type = FilterKey.METADATA; @@ -93,10 +160,28 @@ export default class FilterItem { json.sourceOperator = this.operator; } + // Handle duration type if (this.type === FilterKey.DURATION) { - json.value = this.value?.map((i: any) => (!i ? 0 : i)); + json.value = this.value?.map((item: any) => (item ? Number(item) : 0)); } return json; } + + // Additional utility methods + isValid(): boolean { + return Boolean(this.name && this.category); + } + + clone(): FilterItem { + return new FilterItem(JSON.parse(JSON.stringify(this.toJson()))); + } + + reset(): void { + this.value = ['']; + this.operator = 'is'; + if (this.filters) { + this.filters.forEach((filter) => filter.reset()); + } + } }