From d7de88398ca67d7213fb849db60e5963fd3bc32f Mon Sep 17 00:00:00 2001 From: René 'Necoro' Neumann Date: Fri, 1 May 2020 16:42:31 +0200 Subject: Remove pointers from feed options. Detect unknown fields in feed and group options. --- internal/feed/feed.go | 4 +- internal/feed/parse.go | 8 +-- internal/feed/state.go | 4 +- pkg/config/config.go | 64 +++++---------------- pkg/config/feed.go | 2 +- pkg/config/yaml.go | 80 +++++++++++++++++++++----- pkg/config/yaml_test.go | 146 +++++++++++++++++++++++++++++------------------- 7 files changed, 178 insertions(+), 130 deletions(-) diff --git a/internal/feed/feed.go b/internal/feed/feed.go index f111cd6..ddc91ed 100644 --- a/internal/feed/feed.go +++ b/internal/feed/feed.go @@ -53,10 +53,10 @@ func (feed *Feed) descriptor() feedDescriptor { } func (feed *Feed) NeedsUpdate(updateTime time.Time) bool { - if *feed.MinFreq == 0 { // shortcut + if feed.MinFreq == 0 { // shortcut return true } - if !updateTime.IsZero() && int(time.Since(updateTime).Hours()) < *feed.MinFreq { + if !updateTime.IsZero() && int(time.Since(updateTime).Hours()) < feed.MinFreq { log.Printf("Feed '%s' does not need updating, skipping.", feed.Name) return false } diff --git a/internal/feed/parse.go b/internal/feed/parse.go index 539cad0..fe3374a 100644 --- a/internal/feed/parse.go +++ b/internal/feed/parse.go @@ -14,14 +14,14 @@ import ( // share HTTP clients var ( - stdHTTPClient *http.Client - unsafeHTTPClient *http.Client + stdHTTPClient *http.Client + unsafeHTTPClient *http.Client ) func init() { // std stdHTTPClient = &http.Client{Transport: http.DefaultTransport} - + // unsafe tlsConfig := &tls.Config{InsecureSkipVerify: true} transport := http.DefaultTransport.(*http.Transport).Clone() @@ -45,7 +45,7 @@ func parseFeed(feed *Feed) error { defer cancel() fp := gofeed.NewParser() - fp.Client = httpClient(*feed.NoTLS) + fp.Client = httpClient(feed.NoTLS) parsedFeed, err := fp.ParseURLWithContext(feed.Url, ctx) if err != nil { diff --git a/internal/feed/state.go b/internal/feed/state.go index a060a77..cc9dd94 100644 --- a/internal/feed/state.go +++ b/internal/feed/state.go @@ -82,7 +82,7 @@ func filterFeed(feed *Feed) { origLen := len(feed.items) log.Debugf("Filtering %s. Starting with %d items", feed.Name, origLen) - items := feed.cached.filterItems(feed.items, *feed.Options.IgnHash, *feed.Options.AlwaysNew) + items := feed.cached.filterItems(feed.items, feed.IgnHash, feed.AlwaysNew) feed.items = items newLen := len(feed.items) @@ -122,7 +122,7 @@ func NewState(cfg *config.Config) *State { func (state *State) RemoveUndue() { for name, feed := range state.feeds { - if *feed.Options.Disable || !feed.NeedsUpdate(feed.cached.Last()) { + if feed.Disable || !feed.NeedsUpdate(feed.cached.Last()) { delete(state.feeds, name) } } diff --git a/pkg/config/config.go b/pkg/config/config.go index 885b80e..de8e4ad 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -34,56 +34,29 @@ var DefaultGlobalOptions = GlobalOptions{ } // Per feed options +// NB: Always specify a yaml name, as it is later used in processing type Options struct { - MinFreq *int `yaml:"min-frequency"` - InclImages *bool `yaml:"include-images"` - Disable *bool `yaml:"disable"` - IgnHash *bool `yaml:"ignore-hash"` - AlwaysNew *bool `yaml:"always-new"` - NoTLS *bool `yaml:"tls-no-verify"` -} - -func (opt *Options) mergeFrom(other Options) { - if opt.MinFreq == nil { - opt.MinFreq = other.MinFreq - } - if opt.InclImages == nil { - opt.InclImages = other.InclImages - } - if opt.IgnHash == nil { - opt.IgnHash = other.IgnHash - } - if opt.AlwaysNew == nil { - opt.AlwaysNew = other.AlwaysNew - } - if opt.Disable == nil { - opt.Disable = other.Disable - } - if opt.NoTLS == nil { - opt.NoTLS = other.NoTLS - } + MinFreq int `yaml:"min-frequency"` + InclImages bool `yaml:"include-images"` + Disable bool `yaml:"disable"` + IgnHash bool `yaml:"ignore-hash"` + AlwaysNew bool `yaml:"always-new"` + NoTLS bool `yaml:"tls-no-verify"` } // Default feed options -var DefaultFeedOptions Options - -func init() { - one := 1 - fal := false - DefaultFeedOptions = Options{ - MinFreq: &one, - InclImages: &fal, - IgnHash: &fal, - AlwaysNew: &fal, - Disable: &fal, - NoTLS: &fal, - } +var DefaultFeedOptions = Options{ + MinFreq: 1, + InclImages: false, + IgnHash: false, + AlwaysNew: false, + Disable: false, + NoTLS: false, } // Config holds the global configuration options and the configured feeds type Config struct { GlobalOptions `yaml:",inline"` - GlobalConfig Map `yaml:",inline"` FeedOptions Options `yaml:"options"` Feeds Feeds `yaml:"-"` } @@ -93,7 +66,6 @@ func WithDefault() *Config { return &Config{ GlobalOptions: DefaultGlobalOptions, FeedOptions: DefaultFeedOptions, - GlobalConfig: Map{}, Feeds: Feeds{}, } } @@ -140,17 +112,9 @@ func Load(path string) (*Config, error) { return nil, fmt.Errorf("while parsing: %w", err) } - cfg.pushFeedOptions() - return cfg, nil } -func (cfg *Config) pushFeedOptions() { - for _, feed := range cfg.Feeds { - feed.Options.mergeFrom(cfg.FeedOptions) - } -} - func hostname() (hostname string) { hostname, err := os.Hostname() if err != nil { diff --git a/pkg/config/feed.go b/pkg/config/feed.go index a861e2e..9a88fc9 100644 --- a/pkg/config/feed.go +++ b/pkg/config/feed.go @@ -5,7 +5,7 @@ type Feed struct { Name string Target []string `yaml:"-"` Url string - Options `yaml:",inline"` + Options `yaml:"-"` // not parsed directly } // Convenience type for all feeds diff --git a/pkg/config/yaml.go b/pkg/config/yaml.go index 6d48099..93f8c10 100644 --- a/pkg/config/yaml.go +++ b/pkg/config/yaml.go @@ -2,8 +2,12 @@ package config import ( "fmt" + "reflect" + "strings" "gopkg.in/yaml.v3" + + "github.com/Necoro/feed2imap-go/pkg/log" ) const ( @@ -14,7 +18,7 @@ const ( type config struct { *Config `yaml:",inline"` - GlobalConfig Map `yaml:",inline"` // need to be duplicated, because the Map in Config is not filled + GlobalConfig Map `yaml:",inline"` Feeds []configGroupFeed } @@ -24,9 +28,10 @@ type group struct { } type configGroupFeed struct { - Target yaml.Node - Feed Feed `yaml:",inline"` - Group group `yaml:",inline"` + Target yaml.Node + Feed Feed `yaml:",inline"` + Group group `yaml:",inline"` + Options Map `yaml:",inline"` } func (grpFeed *configGroupFeed) isGroup() bool { @@ -65,12 +70,6 @@ func unmarshal(buf []byte, cfg *Config) (config, error) { } //fmt.Printf("--- parsedCfg:\n%+v\n\n", parsedCfg) - if parsedCfg.GlobalConfig == nil { - cfg.GlobalConfig = Map{} - } else { - cfg.GlobalConfig = parsedCfg.GlobalConfig // need to copy the map explicitly - } - return parsedCfg, nil } @@ -84,7 +83,7 @@ func (cfg *Config) parse(buf []byte) error { return fmt.Errorf("while unmarshalling: %w", err) } - if err := buildFeeds(parsedCfg.Feeds, []string{}, cfg.Feeds); err != nil { + if err := buildFeeds(parsedCfg.Feeds, []string{}, cfg.Feeds, &cfg.FeedOptions); err != nil { return fmt.Errorf("while parsing: %w", err) } @@ -104,8 +103,50 @@ func appTarget(target []string, app string) []string { } } +func buildOptions(globalFeedOptions *Options, options Map) (feedOptions Options, unknownFields []string) { + if options == nil { + // no options set for the feed: copy global options and be done + return *globalFeedOptions, unknownFields + } + + fv := reflect.ValueOf(&feedOptions).Elem() + gv := reflect.ValueOf(globalFeedOptions).Elem() + + n := gv.NumField() + for i := 0; i < n; i++ { + val := fv.Field(i) + f := fv.Type().Field(i) + + if f.PkgPath != "" && !f.Anonymous { + continue + } + + tag := f.Tag.Get("yaml") + if tag == "" { + continue + } + + name := strings.Split(tag, ",")[0] + + set, ok := options[name] + if ok { // in the map -> copy and delete + val.Set(reflect.ValueOf(set)) + delete(options, name) + } else { // not in the map -> copy from global + val.Set(gv.Field(i)) + } + } + + // remaining fields are unknown + for k := range options { + unknownFields = append(unknownFields, k) + } + + return feedOptions, unknownFields +} + // Fetch the group structure and populate the `targetStr` fields in the feeds -func buildFeeds(cfg []configGroupFeed, target []string, feeds Feeds) error { +func buildFeeds(cfg []configGroupFeed, target []string, feeds Feeds, globalFeedOptions *Options) error { for _, f := range cfg { target := appTarget(target, f.target()) switch { @@ -118,17 +159,28 @@ func buildFeeds(cfg []configGroupFeed, target []string, feeds Feeds) error { if name == "" { return fmt.Errorf("Unnamed feed") } - if _, ok := feeds[name]; ok { return fmt.Errorf("Duplicate Feed Name '%s'", name) } + + opt, unknown := buildOptions(globalFeedOptions, f.Options) + + for _, optName := range unknown { + log.Warnf("Unknown option '%s' for feed '%s'. Ignored!", optName, name) + } + + feedCopy.Options = opt feedCopy.Target = target feeds[name] = &feedCopy case f.isGroup(): - if err := buildFeeds(f.Group.Feeds, target, feeds); err != nil { + if err := buildFeeds(f.Group.Feeds, target, feeds, globalFeedOptions); err != nil { return err } + + for optName := range f.Options { + log.Warnf("Unknown option '%s' for group '%s'. Ignored!", optName, f.Group.Group) + } } } diff --git a/pkg/config/yaml_test.go b/pkg/config/yaml_test.go index 575927f..de20c9f 100644 --- a/pkg/config/yaml_test.go +++ b/pkg/config/yaml_test.go @@ -1,6 +1,7 @@ package config import ( + "sort" "strings" "testing" @@ -8,8 +9,6 @@ import ( "gopkg.in/yaml.v3" ) -func i(i int) *int { return &i } -func b(b bool) *bool { return &b } func t(s string) []string { if s == "" { return []string{} @@ -21,6 +20,45 @@ func n(s string) (n yaml.Node) { return } +func TestBuildOptions(tst *testing.T) { + tests := []struct { + name string + inp Map + opts Options + out Options + unknowns []string + }{ + {"Empty", nil, Options{}, Options{}, nil}, + {"Simple copy", nil, Options{MinFreq: 75}, Options{MinFreq: 75}, nil}, + {"Unknowns", Map{"foo": 1}, Options{}, Options{}, []string{"foo"}}, + {"Override", Map{"include-images": true}, Options{InclImages: false}, Options{InclImages: true}, nil}, + {"Mixed", Map{"min-frequency": 24}, Options{MinFreq: 6, InclImages: true}, Options{MinFreq: 24, InclImages: true}, nil}, + {"All", + Map{"max-frequency": 12, "include-images": true, "ignore-hash": true, "obsolete": 54}, + Options{MinFreq: 6, InclImages: true, IgnHash: false}, + Options{MinFreq: 6, InclImages: true, IgnHash: true}, + []string{"max-frequency", "obsolete"}, + }, + } + + for _, tt := range tests { + tst.Run(tt.name, func(tst *testing.T) { + out, unk := buildOptions(&tt.opts, tt.inp) + + if diff := cmp.Diff(out, tt.out); diff != "" { + tst.Error(diff) + } + + sort.Strings(unk) + sort.Strings(tt.unknowns) + + if diff := cmp.Diff(unk, tt.unknowns); diff != "" { + tst.Error(diff) + } + }) + } +} + func TestBuildFeeds(tst *testing.T) { tests := []struct { name string @@ -126,7 +164,8 @@ func TestBuildFeeds(tst *testing.T) { for _, tt := range tests { tst.Run(tt.name, func(tst *testing.T) { var feeds = Feeds{} - err := buildFeeds(tt.feeds, t(tt.target), feeds) + var opts = Options{} + err := buildFeeds(tt.feeds, t(tt.target), feeds, &opts) if (err != nil) != tt.wantErr { tst.Errorf("buildFeeds() error = %v, wantErr %v", err, tt.wantErr) return @@ -140,9 +179,6 @@ func TestBuildFeeds(tst *testing.T) { func defaultConfig(feeds []configGroupFeed, global Map) config { defCfg := WithDefault() - if global != nil { - defCfg.GlobalConfig = global - } return config{ Config: defCfg, Feeds: feeds, @@ -150,7 +186,6 @@ func defaultConfig(feeds []configGroupFeed, global Map) config { } } -//noinspection GoNilness,GoNilness func TestUnmarshal(tst *testing.T) { tests := []struct { name string @@ -174,66 +209,64 @@ func TestUnmarshal(tst *testing.T) { inp: "whatever: 2\ntimeout: 60\noptions:\n min-frequency: 6", wantErr: false, config: func() config { c := defaultConfig(nil, Map{"whatever": 2}) c.Timeout = 60 - c.FeedOptions.MinFreq = i(6) + c.FeedOptions.MinFreq = 6 return c }()}, {name: "Config with feed", inp: ` something: 1 feeds: - - name: Foo - url: whatever - target: bar - include-images: true - unknown-option: foo + - name: Foo + url: whatever + target: bar + include-images: true + unknown-option: foo `, wantErr: false, - config: defaultConfig([]configGroupFeed{ - {Target: n("bar"), Feed: Feed{ + config: defaultConfig([]configGroupFeed{{ + Target: n("bar"), + Feed: Feed{ Name: "Foo", Url: "whatever", - Options: Options{ - MinFreq: nil, - InclImages: b(true), - }, - }}}, Map{"something": 1})}, + }, + Options: Map{"include-images": true, "unknown-option": "foo"}, + }}, Map{"something": 1})}, {name: "Feeds", inp: ` feeds: - - name: Foo - url: whatever - min-frequency: 2 - - name: Shrubbery - url: google.de - target: bla - include-images: false + - name: Foo + url: whatever + min-frequency: 2 + - name: Shrubbery + url: google.de + target: bla + include-images: false `, wantErr: false, config: defaultConfig([]configGroupFeed{ - {Feed: Feed{ - Name: "Foo", - Url: "whatever", - Options: Options{ - MinFreq: i(2), - InclImages: nil, + { + Feed: Feed{ + Name: "Foo", + Url: "whatever", }, - }}, - {Target: n("bla"), Feed: Feed{ - Name: "Shrubbery", - Url: "google.de", - Options: Options{ - MinFreq: nil, - InclImages: b(false), + Options: Map{"min-frequency": 2}, + }, + { + Target: n("bla"), + Feed: Feed{ + Name: "Shrubbery", + Url: "google.de", }, - }}, + Options: Map{"include-images": false}, + }, }, nil), }, {name: "Empty Group", inp: ` feeds: - - group: Foo - target: bla + - group: Foo + target: bla `, wantErr: false, config: defaultConfig([]configGroupFeed{{Target: n("bla"), Group: group{"Foo", nil}}}, nil), @@ -241,18 +274,18 @@ feeds: {name: "Feeds and Groups", inp: ` feeds: - - name: Foo - url: whatever - - group: G1 - target: target - feeds: - - group: G2 - target: "" - feeds: - - name: F1 - url: google.de - - name: F2 - - group: G3 + - name: Foo + url: whatever + - group: G1 + target: target + feeds: + - group: G2 + target: "" + feeds: + - name: F1 + url: google.de + - name: F2 + - group: G3 `, wantErr: false, config: defaultConfig([]configGroupFeed{ @@ -291,8 +324,7 @@ feeds: } if err == nil { - diff := cmp.Diff(got, tt.config, eqNode) - if diff != "" { + if diff := cmp.Diff(got, tt.config, eqNode); diff != "" { tst.Error(diff) } } -- cgit v1.2.3-54-g00ecf