From 81dfdacb33d25d91bce7e2069e7119272f29b80c Mon Sep 17 00:00:00 2001 From: Bruno Carlin Date: Wed, 15 Jan 2025 01:50:41 +0100 Subject: [PATCH] feat: Load* and Save functions panic if data is not a pointer to a struct --- CHANGELOG.md | 3 ++ config.go | 43 ++++++++++++++++++++++++-- config_test.go | 82 ++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 122 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 35454da..6fb29e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ - Add support for TOML configuration files - Add support for HCL configuration files - Use stdlib for tests instead of convey +- Public functions now panic when the data to be marshaled or unmarshaled is not + a pointer to a struct. These errors should be caught during deelopment (with + unit tests). - Update golangci-lint configuration ## v0.3.0 (2025-01-14) diff --git a/config.go b/config.go index bb3dde4..8a556b3 100644 --- a/config.go +++ b/config.go @@ -15,14 +15,30 @@ import ( "fmt" "os" "path/filepath" + "reflect" ) -// ErrUnsupportedFileType is returned when the type of the config file is not -// supported. -var ErrUnsupportedFileType = errors.New("unsupported config type") +var ( + // ErrUnsupportedFileType is returned when the type of the config file is + // not supported. + ErrUnsupportedFileType = errors.New("unsupported config type") + + // ErrInvalidMarshalData is returned when the marshaled value is not a + // struct. + ErrInvalidMarshalData = errors.New("the marshaled value must be a struct") + + // ErrInvalidUnmarshalData is returned when the marshaled value is not a + // struct. + ErrInvalidUnmarshalData = errors.New( + "the unmarshaled value must be a pointer to a struct", + ) +) // LoadFile reads the file at path, parses its json content and fills the struct // with the content of the file. +// +// LoadFile panics with [ErrInvalidUnmarshalData] if data is not a pointer to a +// struct, as this error should be caught during dev. func LoadFile(path string, data any) error { return read(path, data) } @@ -33,6 +49,9 @@ func LoadFile(path string, data any) error { // // It returns an error only if the content of a file is invalid, i.e. it // cannot be unmarshaled to the struct. +// +// LoadFiles panics with [ErrInvalidUnmarshalData] if data is not a pointer to a +// struct, as this error should be caught during dev. func LoadFiles(data any, paths ...string) error { for _, p := range paths { err := read(p, data) @@ -45,6 +64,9 @@ func LoadFiles(data any, paths ...string) error { } // SaveFile writes the given data serialized in JSON in the given path. +// +// SaveFile panics with [ErrInvalidMarshalData] if data is not a struct, as +// this error should be caught during dev. func SaveFile(path string, data any) error { return write(path, data) } @@ -59,6 +81,9 @@ func SaveFile(path string, data any) error { // // An error is returned only if the config file cannot be // written. +// +// LoadAndUpdateFile panics with [ErrInvalidUnmarshalData] if data is not a +// pointer to a struct, as this error should be caught during dev. func LoadAndUpdateFile(path string, data any) error { if _, err := os.Stat(path); !os.IsNotExist(err) { err2 := read(path, data) @@ -85,6 +110,13 @@ type Updater interface { } func read(path string, data any) error { + val := reflect.ValueOf(data) + indVal := reflect.Indirect(val) + + if val.Kind() != reflect.Ptr || indVal.Kind() != reflect.Struct { + panic(ErrInvalidUnmarshalData) + } + content, err := os.ReadFile(filepath.Clean(path)) if err != nil { return fmt.Errorf("cannot read config file: %w", err) @@ -94,6 +126,11 @@ func read(path string, data any) error { } func write(path string, data any) error { + v := reflect.Indirect(reflect.ValueOf(data)) + if v.Kind() != reflect.Struct { + panic(ErrInvalidMarshalData) + } + content, err := marshal(getType(path), data) if err != nil { return err diff --git a/config_test.go b/config_test.go index 930788e..d4d1a15 100644 --- a/config_test.go +++ b/config_test.go @@ -117,6 +117,31 @@ func testLoadFile(t *testing.T, ext string) { assert.Equal(t, "should not change", c.Invariant) }) + t.Run("with a valid file and invalid data", func(t *testing.T) { + t.Parallel() + + file := "test_data/valid." + ext + + assert.PanicsWithError(t, conf.ErrInvalidUnmarshalData.Error(), func() { + conf.LoadFile(file, func() {}) + }) + }) + + t.Run("with a valid file and data is not a pointer", func(t *testing.T) { + t.Parallel() + + file := "test_data/valid." + ext + c := testconf{ + String: "default string", + Int: 1, + Invariant: "should not change", + } + + assert.PanicsWithError(t, conf.ErrInvalidUnmarshalData.Error(), func() { + conf.LoadFile(file, c) + }) + }) + t.Run("with an invalid file", func(t *testing.T) { t.Parallel() @@ -206,6 +231,31 @@ func testLoadFiles(t *testing.T, ext string) { assert.Equal(t, "", c.String) }) + + t.Run("with a valid file and invalid data", func(t *testing.T) { + t.Parallel() + + file := "test_data/valid." + ext + + assert.PanicsWithError(t, conf.ErrInvalidUnmarshalData.Error(), func() { + conf.LoadFiles(func() {}, file) + }) + }) + + t.Run("with a valid file and data is not a pointer", func(t *testing.T) { + t.Parallel() + + file := "test_data/valid." + ext + c := testconf{ + String: "default string", + Int: 1, + Invariant: "should not change", + } + + assert.PanicsWithError(t, conf.ErrInvalidUnmarshalData.Error(), func() { + conf.LoadFiles(c, file) + }) + }) }) } @@ -245,8 +295,9 @@ func testSaveFile(t *testing.T, ext string) { tmpDir := t.TempDir() file := filepath.Join(tmpDir, "test."+ext) - err := conf.SaveFile(file, func() error { return nil }) - require.Error(t, err) + assert.PanicsWithError(t, conf.ErrInvalidMarshalData.Error(), func() { + conf.SaveFile(file, func() error { return nil }) + }) assert.NoFileExists(t, file) }) @@ -437,7 +488,32 @@ func testLoadAndUpdateFile(t *testing.T, ext string) { require.NoError(t, err) assert.NotContains(t, string(newContent), "Unknown") - assert.True(t, updated) + assert.True(t, updated, "the config file has not been updated") + }) + + t.Run("with a valid file and invalid data", func(t *testing.T) { + t.Parallel() + + file := "test_data/valid." + ext + + assert.PanicsWithError(t, conf.ErrInvalidUnmarshalData.Error(), func() { + conf.LoadAndUpdateFile(file, func() {}) + }) + }) + + t.Run("with a valid file and data is not a pointer", func(t *testing.T) { + t.Parallel() + + file := "test_data/valid." + ext + c := testconf{ + String: "default string", + Int: 1, + Invariant: "should not change", + } + + assert.PanicsWithError(t, conf.ErrInvalidUnmarshalData.Error(), func() { + conf.LoadAndUpdateFile(file, c) + }) }) }) }