Remove pointers from feed options. Detect unknown fields in feed and group options.

4 files changed, 170 insertions, 122 deletions

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)
 				}
 			}