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

@@ -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<string, Filter[]> = {};
+  private filterCache: Record<string, CacheEntry> = {};
   private pendingFetches: Record<string, Promise<Filter[]>> = {};
 
   constructor() {
     makeAutoObservable(this);
-
     this.initCommonFilters();
   }
 
-  getEventOptions = (sietId: string) => {
+  // Fixed typo: sietId -> siteId
-    return this.getFilters(sietId)
+  getEventOptions = (siteId: string): FilterOption[] => {
-      .filter((i: Filter) => i.isEvent)
+    return this.getFilters(siteId)
-      .map((i: Filter) => {
+      .filter((filter: Filter) => filter.isEvent)
-        return {
+      .map((filter: Filter) => ({
-          label: i.displayName || i.name,
+        label: filter.displayName || filter.name,
-          value: i.name,
+        value: filter.name,
-        };
+      }));
-      });
   };
 
-  setTopValues = (key: string, values: Record<string, any> | TopValue[]) => {
+  setTopValues = (
+    key: string,
+    values: Record<string, any> | TopValue[],
+  ): void => {
     const vals = Array.isArray(values) ? values : values.data;
-    this.topValues[key] = vals?.filter(
+    this.topValues[key] =
-      (value: any) => value !== null && value.value !== '',
+      vals?.filter((value: any) => value !== null && value?.value !== '') || [];
-    );
   };
 
-  resetValues = () => {
+  resetValues = (): void => {
     this.topValues = {};
   };
 
-  fetchTopValues = async (params: TopValuesParams) => {
+  fetchTopValues = async (params: TopValuesParams): Promise<TopValue[]> => {
-    const valKey = `${params.siteId}_${params.id}${params.source || ''}`;
+    const valKey = this.createTopValuesKey(params);
 
-    if (this.topValues[valKey] && this.topValues[valKey].length) {
+    // Return cached values if available
-      return Promise.resolve(this.topValues[valKey]);
+    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);
+      console.warn(`Filter not found for key: ${valKey}`);
-      return Promise.resolve([]);
+      return [];
     }
 
-    return searchService
+    try {
-      .fetchTopValues({
+      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:
+      subCategory: this.determineSubCategory(category, filter),
-        category === 'events'
-          ? filter.autoCaptured
-            ? 'autocapture'
-            : 'user'
-          : category,
       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[] => {
+  private determineSubCategory = (
-    return filters.map((filter) => ({
+    category: string | undefined,
-      ...filter,
+    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<Filter[]> => {
-    // Return cached filters with operators if available
+    // Return cached filters if available
-    if (this.filters[projectId] && this.filters[projectId].length) {
+    if (this.filters[projectId]?.length) {
-      return Promise.resolve(this.getFilters(projectId));
+      return this.getFilters(projectId);
     }
 
     this.setIsLoadingFilters(true);
 
     try {
       const response = await filterService.fetchFilters(projectId);
+      const processedFilters = this.processFilterResponse(response.data);
 
-      const processedFilters: Filter[] = [];
+      runInAction(() => {
-
+        this.setFilters(projectId, processedFilters);
-      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);
       });
 
-      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<string, any>): 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<Filter[]> => {
     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<Filter[]> => {
-    const resp = await filterService.fetchProperties(eventName, isAutoCapture);
+    try {
-    const names = resp.data.map((i: any) => i['name']);
+      const response = await filterService.fetchProperties(
+        eventName,
+        isAutoCapture,
+      );
+      const propertyNames = response.data.map((item: any) => item.name);
 
-    const activeSiteId = projectStore.activeSiteId + '';
+      const activeSiteId = String(projectStore.activeSiteId);
-    return (
+      const siteFilters = this.filters[activeSiteId] || [];
-      this.filters[activeSiteId]
+
-        ?.filter((i: any) => names.includes(i.name))
+      return siteFilters
-        .map((f: any) => ({
+        .filter((filter: Filter) => propertyNames.includes(filter.name))
-          ...f,
+        .map((filter: Filter) => ({
+          ...filter,
           eventName,
-        })) || []
+        }));
-    );
+    } catch (error) {
+      console.error('Failed to fetch property filters:', error);
+      return [];
+    }
   };
 
-  setCommonFilters = (filters: Filter[]) => {
+  setCommonFilters = (filters: Filter[]): void => {
-    this.commonFilters = filters;
+    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];
+      }
+    });
   };
 }

frontend/app/mstore/types/filterItem.ts

@@ -4,6 +4,32 @@ import { FilterProperty, Operator } from '@/mstore/types/filterConstants';
 
 type JsonData = Record<string, any>;
 
+// 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(
+      processedData.filters = data.filters.map(
-        (i: Record<string, any>) => new FilterItem(i),
+        (filterData: FilterItemData) => new FilterItem(filterData),
       );
     }
-    data.operator = data.operator || 'is';
 
-    this.merge(data);
+    this.merge(processedData);
   }
 
-  updateKey(key: string, value: any) {
+  updateKey<K extends FilterItemKeys>(key: K, value: FilterItemData[K]): void {
-    // @ts-ignore
+    if (key in this) {
-    this[key] = value;
+      (this as any)[key] = value;
+    } else {
+      console.warn(`Attempted to update invalid key: ${key}`);
+    }
   }
 
-  merge(data: any) {
+  merge(data: FilterItemData): void {
-    Object.keys(data).forEach((key) => {
+    Object.entries(data).forEach(([key, value]) => {
-      // @ts-ignore
+      if (key in this && value !== undefined) {
-      this[key] = data[key];
+        (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.name = data.name || '';
-    this.category = data.category;
+    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.name = data.type || '';
-    this.category = data.category;
+    this.category = data.category || '';
     this.subCategory = data.subCategory;
     this.operator = data.operator;
-    this.value = data.value || [''];
+    this.value = Array.isArray(data.value) ? data.value : [''];
-    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: JsonData) => new FilterItem(filterData),
+      );
+    } else {
+      this.filters = [];
+    }
 
     return this;
   }
 
-  toJson(): any {
+  toJson(): JsonData {
-    const json: any = {
+    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());
+    }
+  }
 }