From 7d1de56797db6e4a1fc689a295c4d0f8933555c4 Mon Sep 17 00:00:00 2001 From: plastikfan Date: Fri, 15 Dec 2023 08:12:06 +0000 Subject: [PATCH] feat(nav): add poly filter (#381) --- xfs/nav/filter-defs.go | 24 +++ xfs/nav/filter-init.go | 8 + xfs/nav/filter-poly.go | 77 +++++++++ xfs/nav/filter-poly_test.go | 308 ++++++++++++++++++++++++++++++++++++ xfs/nav/helpers_test.go | 6 + xfs/nav/new-filter.go | 26 ++- xfs/nav/traverse-options.go | 10 +- 7 files changed, 456 insertions(+), 3 deletions(-) create mode 100644 xfs/nav/filter-poly.go create mode 100644 xfs/nav/filter-poly_test.go diff --git a/xfs/nav/filter-defs.go b/xfs/nav/filter-defs.go index 82f7da3..f807026 100644 --- a/xfs/nav/filter-defs.go +++ b/xfs/nav/filter-defs.go @@ -68,6 +68,9 @@ const ( // FilterTypeCustomEn client definable filter FilterTypeCustomEn + + // FilterTypePolyEn poly filter + FilterTypePolyEn ) type allOrderedFilterScopeEnums collections.OrderedKeysMap[FilterScopeBiEnum, string] @@ -103,6 +106,16 @@ func (f FilterScopeBiEnum) String() string { return strings.Join(result, "|") } +// Set sets the bit position indicated by mask +func (f *FilterScopeBiEnum) Set(mask FilterScopeBiEnum) { + *f |= mask +} + +// Clear clears the bit position indicated by mask +func (f *FilterScopeBiEnum) Clear(mask FilterScopeBiEnum) { + *f &^= mask +} + // TraverseFilter filter that can be applied to file system entries. When specified, // the callback will only be invoked for file system nodes that pass the filter. type TraverseFilter interface { @@ -152,6 +165,17 @@ type FilterDef struct { // its the client's responsibility to restore this themselves (see // PersistenceRestorer) Custom TraverseFilter `json:"-"` + + // Poly allows for the definition of a PolyFilter which contains separate + // filters that target files and folders separately. If present, then + // all other fields are redundant, since the filter definitions inside + // Poly should be referred to instead. + Poly *PolyFilterDef +} + +type PolyFilterDef struct { + File FilterDef + Folder FilterDef } // CompoundTraverseFilter filter that can be applied to a folder's collection of entries diff --git a/xfs/nav/filter-init.go b/xfs/nav/filter-init.go index 6d447cc..924c633 100644 --- a/xfs/nav/filter-init.go +++ b/xfs/nav/filter-init.go @@ -1,5 +1,7 @@ package nav +import "fmt" + // InitFiltersHookFn is the default filter initialiser. This can be overridden or extended // by the client if the need arises. To extend this behaviour rather than replace it, // call this function from inside the custom function set on o.Hooks.Filter. To @@ -16,6 +18,12 @@ func InitFiltersHookFn(o *TraverseOptions, frame *navigationFrame) { frame.filters = &NavigationFilters{} if o.isFilteringActive() { + if o.Store.FilterDefs.Node.Poly != nil { + if (o.Store.Subscription == SubscribeFolders) || (o.Store.Subscription == SubscribeFoldersWithFiles) { + panic(fmt.Errorf("invalid subscription type for poly filter")) + } + } + o.useExtendHook() applyNodeFilterDecoration(&o.Store.FilterDefs.Node, frame) } diff --git a/xfs/nav/filter-poly.go b/xfs/nav/filter-poly.go new file mode 100644 index 0000000..25e032e --- /dev/null +++ b/xfs/nav/filter-poly.go @@ -0,0 +1,77 @@ +package nav + +import ( + "fmt" +) + +// PolyFilter is a dual filter that allows files and folders to be filtered +// independently. The Folder filter only applies when the current item +// is a file. This is because, filtering doesn't affect navigation, it only +// controls wether the client callback is invoked or not. That is to say, if +// a particular folder fails to pass a filter, the callback will not be +// invoked for that folder, but we still descend into it and navigate its +// children. This is the reason why the poly filter is only active when the +// the current item is a filter as the client callback will only be invoked +// for the file if its parent folder passes the poly folder filter and +// the file passes the poly file filter. +type PolyFilter struct { + // File is the filter that applies to a file. Note that the client does + // not have to set the File scope as this is enforced automatically as + // well as ensuring that the Folder scope has not been set. The client is + // still free to set other scopes. + File TraverseFilter + + // Folder is the filter that applies to a folder. Note that the client does + // not have to set the Folder scope as this is enforced automatically as + // well as ensuring that the File scope has not been set. The client is + // still free to set other scopes. + Folder TraverseFilter +} + +// Description +func (f *PolyFilter) Description() string { + return fmt.Sprintf("Poly - FILE: '%v', FOLDER: '%v'", + f.File.Description(), f.Folder.Description(), + ) +} + +// Validate ensures that both filters definition are valid, panics when invalid +func (f *PolyFilter) Validate() { + f.File.Validate() + f.Folder.Validate() +} + +// Source returns the Sources of both the File and Folder filters separated +// by a '##' +func (f *PolyFilter) Source() string { + return fmt.Sprintf("%v##%v", + f.File.Source(), f.Folder.Source(), + ) +} + +// IsMatch returns true if the current item is a file and both the current +// file matches the poly file filter and the file's parent folder matches +// the poly folder filter. Returns true of the current item is a folder. +func (f *PolyFilter) IsMatch(item *TraverseItem) bool { + if !item.IsDir() { + return f.Folder.IsMatch(item.Parent) && f.File.IsMatch(item) + } + + return true +} + +// IsApplicable returns the result of applying IsApplicable to +// the poly Filter filter if the current item is a file, returns false +// for folders. +func (f *PolyFilter) IsApplicable(item *TraverseItem) bool { + if !item.IsDir() { + return f.File.IsApplicable(item) + } + + return false +} + +// Scope is a bitwise OR combination of both filters +func (f *PolyFilter) Scope() FilterScopeBiEnum { + return f.File.Scope() | f.Folder.Scope() +} diff --git a/xfs/nav/filter-poly_test.go b/xfs/nav/filter-poly_test.go new file mode 100644 index 0000000..6430f84 --- /dev/null +++ b/xfs/nav/filter-poly_test.go @@ -0,0 +1,308 @@ +package nav_test + +import ( + "fmt" + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/samber/lo" + + . "github.com/snivilised/extendio/i18n" + "github.com/snivilised/extendio/internal/helpers" + "github.com/snivilised/extendio/xfs/nav" +) + +var _ = Describe("FilterPoly", Ordered, func() { + var root string + + BeforeAll(func() { + root = musico() + }) + + BeforeEach(func() { + if err := Use(func(o *UseOptions) { + o.Tag = DefaultLanguage.Get() + }); err != nil { + Fail(err.Error()) + } + }) + + Context("FilterScopeBiEnum", func() { + Context("Clear", func() { + It("should: clear bit", func() { + scope := nav.ScopeFolderEn | nav.ScopeLeafEn + scope.Clear(nav.ScopeFolderEn) + Expect(scope).To(Equal(nav.ScopeLeafEn)) + }) + }) + + Context("Set", func() { + It("should: set bit", func() { + scope := nav.ScopeLeafEn + scope.Set(nav.ScopeFolderEn) + Expect(scope).To(Equal(nav.ScopeFolderEn | nav.ScopeLeafEn)) + }) + }) + }) + + DescribeTable("PolyFilter", + func(entry *polyTE) { + recording := make(recordingMap) + filterDefs := &nav.FilterDefinitions{ + Node: nav.FilterDef{ + Type: nav.FilterTypePolyEn, + Poly: &nav.PolyFilterDef{ + File: entry.file, + Folder: entry.folder, + }, + }, + } + var filter nav.TraverseFilter + + path := helpers.Path(root, entry.relative) + optionFn := func(o *nav.TraverseOptions) { + o.Notify.OnBegin = func(state *nav.NavigationState) { + GinkgoWriter.Printf( + "---> 🛡️ [traverse-navigator-test:BEGIN], root: '%v'\n", state.Root, + ) + filter = state.Filters.Node + } + + o.Store.Subscription = entry.subscription + o.Store.FilterDefs = filterDefs + o.Callback = &nav.LabelledTraverseCallback{ + Label: "test poly filter callback", + Fn: func(item *nav.TraverseItem) error { + GinkgoWriter.Printf( + "===> ⚗️ Poly Filter(%v) source: '%v', item-name: '%v', item-scope(fs): '%v(%v)'\n", + filter.Description(), + filter.Source(), + item.Extension.Name, + item.Extension.NodeScope, + filter.Scope(), + ) + if lo.Contains(entry.mandatory, item.Extension.Name) { + Expect(item).Should(MatchCurrentRegexFilter(filter)) + } + + recording[item.Extension.Name] = len(item.Children) + return nil + }, + } + } + result, err := nav.New().Primary(&nav.Prime{ + Path: path, + OptionsFn: optionFn, + }).Run() + + if entry.mandatory != nil { + for _, name := range entry.mandatory { + _, found := recording[name] + Expect(found).To(BeTrue(), helpers.Reason(name)) + } + } + + if entry.prohibited != nil { + for _, name := range entry.prohibited { + _, found := recording[name] + Expect(found).To(BeFalse(), helpers.Reason(name)) + } + } + + Expect(err).Error().To(BeNil()) + Expect(result.Metrics.Count(nav.MetricNoFilesInvokedEn)).To(Equal(entry.expectedNoOf.files), + helpers.BecauseQuantity("Incorrect no of files", + int(entry.expectedNoOf.files), + int(result.Metrics.Count(nav.MetricNoFilesInvokedEn)), + ), + ) + + Expect(result.Metrics.Count(nav.MetricNoFoldersInvokedEn)).To(Equal(entry.expectedNoOf.folders), + helpers.BecauseQuantity("Incorrect no of folders", + int(entry.expectedNoOf.folders), + int(result.Metrics.Count(nav.MetricNoFoldersInvokedEn)), + ), + ) + }, + func(entry *polyTE) string { + return fmt.Sprintf("🧪 ===> given: '%v'", entry.message) + }, + + Entry(nil, &polyTE{ + naviTE: naviTE{ + message: "poly - files:regex; folders:glob", + relative: "RETRO-WAVE", + subscription: nav.SubscribeAny, + expectedNoOf: directoryQuantities{ + // file is 2 not 3 because *i* is case sensitive so Innerworld is not a match + // The next(not this one) regex test case, fixes this because folder regex has better + // control over case sensitivity + files: 2, + folders: 8, + }, + }, + file: nav.FilterDef{ + Type: nav.FilterTypeRegexEn, + Description: "files: starts with vinyl", + Pattern: "^vinyl", + Scope: nav.ScopeFileEn, + }, + folder: nav.FilterDef{ + Type: nav.FilterTypeGlobEn, + Description: "folders: contains i (case sensitive)", + Pattern: "*i*", + Scope: nav.ScopeFolderEn | nav.ScopeLeafEn, + }, + }), + + Entry(nil, &polyTE{ + naviTE: naviTE{ + message: "poly - files:regex; folders:regex", + relative: "RETRO-WAVE", + subscription: nav.SubscribeAny, + expectedNoOf: directoryQuantities{ + files: 3, + folders: 8, + }, + }, + file: nav.FilterDef{ + Type: nav.FilterTypeRegexEn, + Description: "files: starts with vinyl", + Pattern: "^vinyl", + Scope: nav.ScopeFileEn, + }, + folder: nav.FilterDef{ + Type: nav.FilterTypeRegexEn, + Description: "folders: contains i (case insensitive)", + Pattern: "[iI]", + Scope: nav.ScopeFolderEn | nav.ScopeLeafEn, + }, + }), + + // For the poly filter, the file/folder scopes must be set correctly, but because + // they can be set automatically, the client is not forced to set them. This test + // checks that when the file/folder scopes are not set, then poly filtering still works + // properly. + Entry(nil, &polyTE{ + naviTE: naviTE{ + message: "poly(scopes omitted) - files:regex; folders:regex", + relative: "RETRO-WAVE", + subscription: nav.SubscribeAny, + expectedNoOf: directoryQuantities{ + files: 3, + folders: 8, + }, + }, + file: nav.FilterDef{ + Type: nav.FilterTypeRegexEn, + Description: "files: starts with vinyl", + Pattern: "^vinyl", + // file scope omitted + }, + folder: nav.FilterDef{ + Type: nav.FilterTypeRegexEn, + Description: "folders: contains i (case insensitive)", + Pattern: "[iI]", + Scope: nav.ScopeLeafEn, // folder scope omitted + }, + }), + + Entry(nil, &polyTE{ + naviTE: naviTE{ + message: "poly(subscribe:files)", + relative: "RETRO-WAVE", + subscription: nav.SubscribeFiles, + expectedNoOf: directoryQuantities{ + files: 3, + folders: 0, + }, + }, + file: nav.FilterDef{ + Type: nav.FilterTypeRegexEn, + Description: "files: starts with vinyl", + Pattern: "^vinyl", + }, + folder: nav.FilterDef{ + Type: nav.FilterTypeRegexEn, + Description: "folders: contains i", + Pattern: "[iI]", + Scope: nav.ScopeLeafEn, + }, + }), + ) + + DescribeTable("Panic: PolyFilter", + func(entry *polyTE) { + defer func() { + pe := recover() + if err, ok := pe.(error); !ok || !strings.Contains(err.Error(), + "invalid subscription type for poly filter") { + Fail("incorrect panic") + } + }() + + fileDef := nav.FilterDef{ + Type: nav.FilterTypeRegexEn, + Description: "files: starts with vinyl", + Pattern: "^vinyl", + } + folderDef := nav.FilterDef{ + Type: nav.FilterTypeRegexEn, + Description: "folders: contains i", + Pattern: "[iI]", + Scope: nav.ScopeLeafEn, + } + + filterDefs := &nav.FilterDefinitions{ + Node: nav.FilterDef{ + Type: nav.FilterTypePolyEn, + Poly: &nav.PolyFilterDef{ + File: fileDef, + Folder: folderDef, + }, + }, + } + + path := helpers.Path(root, entry.relative) + optionFn := func(o *nav.TraverseOptions) { + o.Store.Subscription = entry.subscription + o.Store.FilterDefs = filterDefs + o.Callback = &nav.LabelledTraverseCallback{ + Label: "(panic): test poly filter callback", + Fn: func(item *nav.TraverseItem) error { + return nil + }, + } + } + _, _ = nav.New().Primary(&nav.Prime{ + Path: path, + OptionsFn: optionFn, + }).Run() + + Fail("❌ expected panic due to invalid subscription type for poly filter") + }, + func(entry *polyTE) string { + return fmt.Sprintf("🧪 ===> given: '%v'", entry.message) + }, + + // Poly filtering is not valid when files are not subscribed to + // a warning is issued in the logs to indicate that this + // scenario is invalid + Entry(nil, &polyTE{ + naviTE: naviTE{ + message: "poly(subscribe:folders/invalid)", + relative: "RETRO-WAVE", + subscription: nav.SubscribeFolders, + }, + }), + + Entry(nil, &polyTE{ + naviTE: naviTE{ + message: "poly(subscribe:folders/invalid)", + relative: "RETRO-WAVE", + subscription: nav.SubscribeFoldersWithFiles, + }, + }), + ) +}) diff --git a/xfs/nav/helpers_test.go b/xfs/nav/helpers_test.go index e628988..c98fbe4 100644 --- a/xfs/nav/helpers_test.go +++ b/xfs/nav/helpers_test.go @@ -81,6 +81,12 @@ type filterTE struct { ifNotApplicable nav.TriStateBoolEnum } +type polyTE struct { + naviTE + file nav.FilterDef + folder nav.FilterDef +} + type marshalTE struct { naviTE errorContains string diff --git a/xfs/nav/new-filter.go b/xfs/nav/new-filter.go index f819d6f..4841de9 100644 --- a/xfs/nav/new-filter.go +++ b/xfs/nav/new-filter.go @@ -51,16 +51,37 @@ func newNodeFilter(def *FilterDef) TraverseFilter { filter = def.Custom + case FilterTypePolyEn: + filter = newPolyFilter(def.Poly) + default: panic(fmt.Sprintf("Filter definition for '%v' is missing the Type field", def.Description)) } - filter.Validate() + if def.Type != FilterTypePolyEn { + filter.Validate() + } + + return filter +} + +func newPolyFilter(polyDef *PolyFilterDef) TraverseFilter { + // lets enforce the correct filter scopes + // + polyDef.File.Scope.Set(ScopeFileEn) // file scope must be set for files + polyDef.File.Scope.Clear(ScopeFolderEn) // folder scope must NOT be set for files + + polyDef.Folder.Scope.Set(ScopeFolderEn) // folder scope must be set for folders + polyDef.Folder.Scope.Clear(ScopeFileEn) // file scope must NOT be set for folders + + filter := &PolyFilter{ + File: newNodeFilter(&polyDef.File), + Folder: newNodeFilter(&polyDef.Folder), + } return filter } -// newCompoundFilter exported for testing purposes only (do not use) func newCompoundFilter(def *CompoundFilterDef) CompoundTraverseFilter { var filter CompoundTraverseFilter @@ -91,6 +112,7 @@ func newCompoundFilter(def *CompoundFilterDef) CompoundTraverseFilter { filter = def.Custom case FilterTypeUndefinedEn: + case FilterTypePolyEn: } filter.Validate() diff --git a/xfs/nav/traverse-options.go b/xfs/nav/traverse-options.go index 5674a97..bdf34b7 100644 --- a/xfs/nav/traverse-options.go +++ b/xfs/nav/traverse-options.go @@ -276,7 +276,15 @@ func composeTraverseOptions(fn ...TraverseOptionFn) *TraverseOptions { } func (o *TraverseOptions) isFilteringActive() bool { - return o.Store.FilterDefs != nil && (o.Store.FilterDefs.Node.Pattern != "" || o.Store.FilterDefs.Node.Custom != nil) + if o.Store.FilterDefs != nil { + patternDefined := o.Store.FilterDefs.Node.Pattern != "" + customDefined := o.Store.FilterDefs.Node.Custom != nil + polyDefined := o.Store.FilterDefs.Node.Poly != nil + + return patternDefined || customDefined || polyDefined + } + + return false } func (o *TraverseOptions) afterUserOptions() {