Merge pull request #428 from RandallFlagg/filter

Filter refactoring and warning removal

Author: Hirogen

src/.editorconfig

@@ -580,7 +580,7 @@ dotnet_code_quality.CA1001.excluded_type_names_with_derived_types =
 
 # CA1002: Do not expose generic lists
 dotnet_diagnostic.CA1002.severity = suggestion
-dotnet_code_quality.CA1002.api_surface = public, internal
+dotnet_code_quality.CA1002.api_surface = public
 
 # CA1003: Use generic event handler instances
 dotnet_diagnostic.CA1003.severity = warning

src/LogExpert.Core/Classes/Filter/Filter.cs

@@ -1,9 +1,9 @@
 using LogExpert.Core.Callback;
 using LogExpert.Core.Classes;
-using LogExpert.Core.Classes.Filter;
+
 using NLog;
 
-namespace LogExpert.Classes.Filter;
+namespace LogExpert.Core.Classes.Filter;
 
 internal delegate void FilterFx (FilterParams filterParams, List<int> filterResultLines, List<int> lastFilterResultLines, List<int> filterHitList);
 
@@ -13,7 +13,7 @@ internal class Filter
 
     private const int PROGRESS_BAR_MODULO = 1000;
     private const int SPREAD_MAX = 50;
-    private static readonly ILogger _logger = LogManager.GetCurrentClassLogger();
+    private static readonly Logger _logger = LogManager.GetCurrentClassLogger();
 
     private readonly ColumnizerCallback _callback;
 
@@ -35,11 +35,8 @@ public Filter (ColumnizerCallback callback)
     #region Properties
 
     public List<int> FilterResultLines { get; }
-
     public List<int> LastFilterLinesList { get; }
-
     public List<int> FilterHitList { get; }
-
     public bool ShouldCancel { get; set; }
 
     #endregion
@@ -72,7 +69,7 @@ private int DoFilter (FilterParams filterParams, int startLine, int maxCount, Li
                     return count;
                 }
 
-                ILogLine line = _callback.GetLogLine(lineNum);
+                var line = _callback.GetLogLine(lineNum);
 
                 if (line == null)
                 {
@@ -83,7 +80,7 @@ private int DoFilter (FilterParams filterParams, int startLine, int maxCount, Li
 
                 if (Util.TestFilterCondition(filterParams, line, _callback))
                 {
-                    AddFilterLine(lineNum, false, filterParams, filterResultLines, lastFilterLinesList, filterHitList);
+                    AddFilterLine(lineNum, filterParams, filterResultLines, lastFilterLinesList, filterHitList);
                 }
 
                 lineNum++;
@@ -99,6 +96,7 @@ private int DoFilter (FilterParams filterParams, int startLine, int maxCount, Li
         catch (Exception ex)
         {
             _logger.Error(ex, "Exception while filtering. Please report to developer");
+            throw;
             //TODO: This information should be handled from the LogExpert project and not from LogExpert.Core.
             //MessageBox.Show(null,
             //    "Exception while filtering. Please report to developer: \n\n" + ex + "\n\n" + ex.StackTrace,
@@ -108,10 +106,10 @@ private int DoFilter (FilterParams filterParams, int startLine, int maxCount, Li
         return count;
     }
 
-    private void AddFilterLine (int lineNum, bool immediate, FilterParams filterParams, List<int> filterResultLines, List<int> lastFilterLinesList, List<int> filterHitList)
+    private void AddFilterLine (int lineNum, FilterParams filterParams, List<int> filterResultLines, List<int> lastFilterLinesList, List<int> filterHitList)
     {
         filterHitList.Add(lineNum);
-        IList<int> filterResult = GetAdditionalFilterResults(filterParams, lineNum, lastFilterLinesList);
+        var filterResult = GetAdditionalFilterResults(filterParams, lineNum, lastFilterLinesList);
 
         filterResultLines.AddRange(filterResult);
 
@@ -123,7 +121,6 @@ private void AddFilterLine (int lineNum, bool immediate, FilterParams filterPara
         }
     }
 
-
     /// <summary>
     ///  Returns a list with 'additional filter results'. This is the given line number
     ///  and (if back spread and/or fore spread is enabled) some additional lines.
@@ -170,6 +167,7 @@ private IList<int> GetAdditionalFilterResults (FilterParams filterParams, int li
                 }
             }
         }
+
         return resultList;
     }
 

src/LogExpert.Core/Classes/Filter/FilterCancelHandler.cs

@@ -4,7 +4,7 @@
 
 using NLog;
 
-namespace LogExpert.Classes.Filter;
+namespace LogExpert.Core.Classes.Filter;
 
 public class FilterCancelHandler : IBackgroundProcessCancelHandler
 {

src/LogExpert.Core/Classes/Filter/FilterParams.cs

@@ -1,4 +1,5 @@
 using System.Collections;
+using System.Collections.ObjectModel;
 using System.Drawing;
 using System.Text.Json.Serialization;
 using System.Text.RegularExpressions;
@@ -9,38 +10,18 @@ namespace LogExpert.Core.Classes.Filter;
 public class FilterParams : ICloneable
 {
     #region Fields
-
-    private string _rangeSearchText = string.Empty;
-    private string _searchText = string.Empty;
-
     //public List<string> historyList = new List<string>();
     //public List<string> rangeHistoryList = new List<string>();
 
     #endregion
 
     #region Properties
 
-    public string SearchText
-    {
-        get => _searchText;
-        set
-        {
-            _searchText = value;
-            LowerSearchText = _searchText.ToLowerInvariant();
-        }
-    }
+    public string SearchText { get; set; }
 
-    public string RangeSearchText
-    {
-        get => _rangeSearchText;
-        set
-        {
-            _rangeSearchText = value;
-            LowerRangeSearchText = _rangeSearchText.ToLowerInvariant();
-        }
-    }
+    public string RangeSearchText { get; set; }
 
-    public bool SpreadEnabled => SpreadBefore > 0 || SpreadBehind > 0;
+    //public bool SpreadEnabled => SpreadBefore > 0 || SpreadBehind > 0;
 
     public bool IsCaseSensitive { get; set; }
 
@@ -69,7 +50,7 @@ public string RangeSearchText
     public bool IsRegex { get; set; }
 
     // list of columns in which to search
-    public List<int> ColumnList { get; set; } = [];
+    public Collection<int> ColumnList { get; } = [];
 
     [JsonIgnore]
     [field: NonSerialized]
@@ -91,11 +72,13 @@ public string RangeSearchText
     [field: NonSerialized]
     public bool LastResult { get; set; }
 
-    [field: NonSerialized]
-    public string LowerRangeSearchText { get; set; } = string.Empty;
+    ///Returns RangeSearchText.ToUpperInvariant
+    [JsonIgnore]
+    internal string NormalizedRangeSearchText => RangeSearchText.ToUpperInvariant();
 
-    [field: NonSerialized]
-    public string LowerSearchText { get; set; } = string.Empty;
+    ///Returns SearchText.ToUpperInvariant
+    [JsonIgnore]
+    internal string NormalizedSearchText => SearchText.ToUpperInvariant();
 
     [field: NonSerialized]
     public Regex RangeRex { get; set; }
@@ -125,8 +108,6 @@ public FilterParams CloneWithCurrentColumnizer ()
     public void Init ()
     {
         LastNonEmptyCols = [];
-        LowerRangeSearchText = RangeSearchText.ToLowerInvariant();
-        LowerSearchText = SearchText.ToLowerInvariant();
         LastLine = string.Empty;
     }
 

src/LogExpert.Core/Classes/Filter/FilterPipe.cs

@@ -72,8 +72,11 @@ public void CloseFile ()
         }
     }
 
+    //TOOD: check if the callers are checking for null before calling
     public bool WriteToPipe (ILogLine textLine, int orgLineNum)
     {
+        ArgumentNullException.ThrowIfNull(textLine, nameof(textLine));
+
         try
         {
             lock (_fileNameLock)

src/LogExpert.Core/Classes/Filter/FilterStarter.cs

@@ -1,11 +1,10 @@
 using System.Globalization;
 
 using LogExpert.Core.Callback;
-using LogExpert.Core.Classes.Filter;
 
 using NLog;
 
-namespace LogExpert.Classes.Filter;
+namespace LogExpert.Core.Classes.Filter;
 
 public delegate void ProgressCallback (int lineCount);
 
@@ -86,7 +85,6 @@ public async void DoFilter (FilterParams filterParams, int startLine, int maxCou
         }
 
         var workStartLine = startLine;
-        List<WaitHandle> handleList = [];
         _progressLineCount = 0;
         _progressCallback = progressCallback;
         while (workStartLine < startLine + maxCount)
@@ -101,17 +99,11 @@ public async void DoFilter (FilterParams filterParams, int startLine, int maxCou
             }
             _logger.Info(CultureInfo.InvariantCulture, "FilterStarter starts worker for line {0}, lineCount {1}", workStartLine, interval);
 
-            await Task.Run(() => DoWork(filterParams, workStartLine, interval, ThreadProgressCallback)).ContinueWith(FilterDoneCallback);
+            var filter = await Task.Run(() => DoWork(filterParams, workStartLine, interval, ThreadProgressCallback)).ConfigureAwait(false);
+            FilterDoneCallback(filter);
             workStartLine += interval;
         }
 
-        WaitHandle[] handles = [.. handleList];
-        // wait for worker threads completion
-        if (handles.Length > 0)
-        {
-            WaitHandle.WaitAll(handles);
-        }
-
         MergeResults();
     }
 
@@ -147,46 +139,40 @@ private Filter DoWork (FilterParams filterParams, int startLine, int maxCount, P
         _logger.Info(CultureInfo.InvariantCulture, "Started Filter worker [{0}] for line {1}", Environment.CurrentManagedThreadId, startLine);
 
         // Give every thread own copies of ColumnizerCallback and FilterParams, because the state of the objects changes while filtering
-        FilterParams threadFilterParams = filterParams.CloneWithCurrentColumnizer();
+        var threadFilterParams = filterParams.CloneWithCurrentColumnizer();
         Filter filter = new((ColumnizerCallback)_callback.Clone());
         lock (_filterWorkerList)
         {
             _filterWorkerList.Add(filter);
         }
 
-        if (_shouldStop)
+        if (!_shouldStop)
         {
-            return filter;
-        }
 
-        _ = filter.DoFilter(threadFilterParams, startLine, maxCount, progressCallback);
-        _logger.Info(CultureInfo.InvariantCulture, "Filter worker [{0}] for line {1} has completed.", Environment.CurrentManagedThreadId, startLine);
+            _ = filter.DoFilter(threadFilterParams, startLine, maxCount, progressCallback);
+            _logger.Info(CultureInfo.InvariantCulture, "Filter worker [{0}] for line {1} has completed.", Environment.CurrentManagedThreadId, startLine);
 
-        lock (_filterReadyList)
-        {
-            _filterReadyList.Add(filter);
+            lock (_filterReadyList)
+            {
+                _filterReadyList.Add(filter);
+            }
         }
 
         return filter;
     }
 
-    private void FilterDoneCallback (Task<Filter> filterTask)
+    private void FilterDoneCallback (Filter filter)
     {
-        if (filterTask.IsCompleted)
+        lock (_filterReadyList)
         {
-            Filter filter = filterTask.Result;
-
-            lock (_filterReadyList)
-            {
-                _filterReadyList.Add(filter);
-            }
+            _filterReadyList.Add(filter);
         }
     }
 
     private void MergeResults ()
     {
         _logger.Info(CultureInfo.InvariantCulture, "Merging filter results.");
-        foreach (Filter filter in _filterReadyList)
+        foreach (var filter in _filterReadyList)
         {
             foreach (var lineNum in filter.FilterHitList)
             {
@@ -195,13 +181,15 @@ private void MergeResults ()
                     _filterHitDict.Add(lineNum, lineNum);
                 }
             }
+
             foreach (var lineNum in filter.FilterResultLines)
             {
                 if (!_filterResultDict.ContainsKey(lineNum))
                 {
                     _filterResultDict.Add(lineNum, lineNum);
                 }
             }
+
             foreach (var lineNum in filter.LastFilterLinesList)
             {
                 if (!_lastFilterLinesDict.ContainsKey(lineNum))
@@ -210,6 +198,7 @@ private void MergeResults ()
                 }
             }
         }
+
         FilterHitList.AddRange(_filterHitDict.Keys);
         FilterResultLines.AddRange(_filterResultDict.Keys);
         LastFilterLinesList.AddRange(_lastFilterLinesDict.Keys);